From 4820105ba85531da5b388e7b2e98fdcdae9ced07 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 10 Nov 2022 21:45:00 -0500 Subject: [PATCH] Fix elliptic curve name case sensitivity on Windows --- .../Cryptography/ECCng.ImportExport.cs | 57 +-------------- .../EC/EccTestBase.cs | 12 ++++ .../ECDiffieHellmanTests.ImportExport.cs | 47 ++++++++++++- .../ECDsa/ECDsaImportExport.cs | 49 ++++++++++++- .../System/Security/Cryptography/CngKey.EC.cs | 70 +++++++++---------- .../System/Security/Cryptography/Helpers.cs | 3 +- .../System/Security/Cryptography/OidLookup.cs | 8 +-- .../CertificateRequestUsageTests.cs | 25 +++++++ 8 files changed, 173 insertions(+), 98 deletions(-) diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs b/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs index 8e8726fb3cd2c0..dcd36ca435cdc0 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs @@ -384,7 +384,7 @@ private static bool IsMagicValueOfKeyPublic(KeyBlobMagicNumber magic) /// that don't have the named curve functionality. /// private static KeyBlobMagicNumber EcdsaCurveNameToMagicNumber(string? name, bool includePrivateParameters) => - EcdsaCurveNameToAlgorithm(name) switch + CngKey.EcdsaCurveNameToAlgorithm(name).Algorithm switch { AlgorithmName.ECDsaP256 => includePrivateParameters ? KeyBlobMagicNumber.BCRYPT_ECDSA_PRIVATE_P256_MAGIC : @@ -409,7 +409,7 @@ private static KeyBlobMagicNumber EcdsaCurveNameToMagicNumber(string? name, bool /// that don't have the named curve functionality. /// private static KeyBlobMagicNumber EcdhCurveNameToMagicNumber(string? name, bool includePrivateParameters) => - EcdhCurveNameToAlgorithm(name) switch + CngKey.EcdhCurveNameToAlgorithm(name).Algorithm switch { AlgorithmName.ECDHP256 => includePrivateParameters ? KeyBlobMagicNumber.BCRYPT_ECDH_PRIVATE_P256_MAGIC : @@ -513,58 +513,5 @@ ref MemoryMarshal.GetReference(keyBlob), return keyHandle; } - - /// - /// Map a curve name to algorithm. This enables curves that worked pre-Win10 - /// to work with newer APIs for import and export. - /// - internal static string EcdsaCurveNameToAlgorithm(string? algorithm) - { - switch (algorithm) - { - case "nistP256": - case "ECDSA_P256": - return AlgorithmName.ECDsaP256; - - case "nistP384": - case "ECDSA_P384": - return AlgorithmName.ECDsaP384; - - case "nistP521": - case "ECDSA_P521": - return AlgorithmName.ECDsaP521; - } - - // All other curves are new in Win10 so use generic algorithm - return AlgorithmName.ECDsa; - } - - /// - /// Map a curve name to algorithm. This enables curves that worked pre-Win10 - /// to work with newer APIs for import and export. - /// - internal static string EcdhCurveNameToAlgorithm(string? algorithm) - { - switch (algorithm) - { - case "nistP256": - case "ECDH_P256": - case "ECDSA_P256": - return AlgorithmName.ECDHP256; - - case "nistP384": - case "ECDH_P384": - case "ECDSA_P384": - return AlgorithmName.ECDHP384; - - case "nistP521": - case "ECDH_P521": - case "ECDSA_P521": - return AlgorithmName.ECDHP521; - } - - // All other curves are new in Win10 so use generic algorithm - return AlgorithmName.ECDH; - } } } diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/EC/EccTestBase.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/EC/EccTestBase.cs index 2f347469e12cb9..5f9dae4c8fcefb 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/EC/EccTestBase.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/EC/EccTestBase.cs @@ -249,6 +249,18 @@ internal static void CompareCurve(in ECCurve c1, in ECCurve c2) } } } + + internal static string InvertStringCase(string str) + { + return string.Create(str.Length, str, static (destination, str) => + { + for (int i = 0; i < str.Length; i++) + { + char c = str[i]; + destination[i] = char.IsAsciiLetter(c) ? (char)(c ^ 0b0100000) : c; + } + }); + } #endif } } diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs index 0c06a3f12f1516..82f78094bb1000 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Security.Cryptography.Tests; using Test.Cryptography; using Xunit; @@ -14,7 +15,7 @@ public partial class ECDiffieHellmanTests // probe for this capability before depending on it. internal static bool ECDsa224Available => ECDiffieHellmanFactory.IsCurveValid(new Oid(ECDSA_P224_OID_VALUE)); - + internal static bool CanDeriveNewPublicKey { get; } = EcDiffieHellman.Tests.ECDiffieHellmanFactory.CanDeriveNewPublicKey; @@ -416,6 +417,50 @@ public static void ImportFromPrivateOnlyKey() } } + [Theory] + [MemberData(nameof(NamedCurves))] + public static void OidPresentOnCurveMiscased(ECCurve curve) + { + ECCurve miscasedCurve = ECCurve.CreateFromFriendlyName(InvertStringCase(curve.Oid.FriendlyName)); + Assert.NotEqual(miscasedCurve.Oid.FriendlyName, curve.Oid.FriendlyName); + Assert.Equal(miscasedCurve.Oid.FriendlyName, curve.Oid.FriendlyName, ignoreCase: true); + + using (ECDiffieHellman ecdh = ECDiffieHellmanFactory.Create()) + { + ecdh.GenerateKey(miscasedCurve); + ECParameters exportedParameters = ecdh.ExportParameters(false); + Assert.Equal(curve.Oid.Value, exportedParameters.Curve.Oid.Value); + + exportedParameters.Curve = miscasedCurve; + + // Assert.NoThrow. Make sure we can import the mis-cased curve. + ecdh.ImportParameters(exportedParameters); + } + } + + public static IEnumerable NamedCurves + { + get + { + yield return new object[] { ECCurve.NamedCurves.nistP256 }; + yield return new object[] { ECCurve.NamedCurves.nistP384 }; + yield return new object[] { ECCurve.NamedCurves.nistP521 }; + yield return new object[] { ECCurve.CreateFromFriendlyName("ECDH_P256") }; + yield return new object[] { ECCurve.CreateFromFriendlyName("ECDH_P384") }; + yield return new object[] { ECCurve.CreateFromFriendlyName("ECDH_P521") }; + + if (ECDiffieHellmanFactory.IsCurveValid(ECCurve.NamedCurves.brainpoolP160r1.Oid)) + { + yield return new object[] { ECCurve.NamedCurves.brainpoolP160r1 }; + } + + if (ECDiffieHellmanFactory.IsCurveValid(ECCurve.NamedCurves.brainpoolP160t1.Oid)) + { + yield return new object[] { ECCurve.NamedCurves.brainpoolP160t1 }; + } + } + } + private static void VerifyNamedCurve(ECParameters parameters, ECDiffieHellman ec, int keySize, bool includePrivate) { parameters.Validate(); diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaImportExport.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaImportExport.cs index bff75c20922933..1925320e66ab5a 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaImportExport.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaImportExport.cs @@ -1,10 +1,11 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using Xunit; +using System.Collections.Generic; using System.Security.Cryptography.Tests; -using Test.Cryptography; using System.Security.Cryptography.EcDiffieHellman.Tests; +using Test.Cryptography; +using Xunit; namespace System.Security.Cryptography.EcDsa.Tests { @@ -353,6 +354,50 @@ public static void ImportFromPrivateOnlyKey() } } + [Theory] + [MemberData(nameof(NamedCurves))] + public static void OidPresentOnCurveMiscased(ECCurve curve) + { + ECCurve miscasedCurve = ECCurve.CreateFromFriendlyName(InvertStringCase(curve.Oid.FriendlyName)); + Assert.NotEqual(miscasedCurve.Oid.FriendlyName, curve.Oid.FriendlyName); + Assert.Equal(miscasedCurve.Oid.FriendlyName, curve.Oid.FriendlyName, ignoreCase: true); + + using (ECDsa ecdsa = ECDsaFactory.Create()) + { + ecdsa.GenerateKey(miscasedCurve); + ECParameters exportedParameters = ecdsa.ExportParameters(false); + Assert.Equal(curve.Oid.Value, exportedParameters.Curve.Oid.Value); + + exportedParameters.Curve = miscasedCurve; + + // Assert.NoThrow. Make sure we can import the mis-cased curve. + ecdsa.ImportParameters(exportedParameters); + } + } + + public static IEnumerable NamedCurves + { + get + { + yield return new object[] { ECCurve.NamedCurves.nistP256 }; + yield return new object[] { ECCurve.NamedCurves.nistP384 }; + yield return new object[] { ECCurve.NamedCurves.nistP521 }; + yield return new object[] { ECCurve.CreateFromFriendlyName("ECDSA_P256") }; + yield return new object[] { ECCurve.CreateFromFriendlyName("ECDSA_P384") }; + yield return new object[] { ECCurve.CreateFromFriendlyName("ECDSA_P521") }; + + if (ECDsaFactory.IsCurveValid(ECCurve.NamedCurves.brainpoolP160r1.Oid)) + { + yield return new object[] { ECCurve.NamedCurves.brainpoolP160r1 }; + } + + if (ECDsaFactory.IsCurveValid(ECCurve.NamedCurves.brainpoolP160t1.Oid)) + { + yield return new object[] { ECCurve.NamedCurves.brainpoolP160t1 }; + } + } + } + private static void VerifyNamedCurve(ECParameters parameters, ECDsa ec, int keySize, bool includePrivate) { parameters.Validate(); diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.EC.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.EC.cs index c47f427b4cf1d6..045de45192b06c 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.EC.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.EC.cs @@ -26,8 +26,9 @@ internal static bool IsECNamedCurve(string algorithm) { if (IsECNamedCurve()) { - oidValue = null; - return _keyHandle.GetPropertyAsString(KeyPropertyName.ECCCurveName, CngPropertyOptions.None); + string? curveName = _keyHandle.GetPropertyAsString(KeyPropertyName.ECCCurveName, CngPropertyOptions.None); + oidValue = curveName is null ? null : OidLookup.ToOid(curveName, OidGroup.PublicKeyAlgorithm, fallBackToAllGroups: false); + return curveName; } // Use hard-coded values (for use with pre-Win10 APIs) @@ -81,53 +82,52 @@ internal static CngProperty GetPropertyFromNamedCurve(ECCurve curve) /// Map a curve name to algorithm. This enables curves that worked pre-Win10 /// to work with newer APIs for import and export. /// - internal static CngAlgorithm EcdsaCurveNameToAlgorithm(string name) + internal static CngAlgorithm EcdsaCurveNameToAlgorithm(ReadOnlySpan name) { - switch (name) - { - case "nistP256": - case "ECDSA_P256": - return CngAlgorithm.ECDsaP256; - - case "nistP384": - case "ECDSA_P384": - return CngAlgorithm.ECDsaP384; + const int MaxCurveNameLength = 16; + Span nameLower = stackalloc char[MaxCurveNameLength]; + int written = name.ToLowerInvariant(nameLower); - case "nistP521": - case "ECDSA_P521": - return CngAlgorithm.ECDsaP521; + // Either it is empty or too big for the buffer, and the buffer is large enough to hold all mapped + // curve names, so return the generic algorithm. + if (written < 1) + { + return CngAlgorithm.ECDsa; } - // All other curves are new in Win10 so use generic algorithm - return CngAlgorithm.ECDsa; + return nameLower.Slice(0, written) switch + { + "nistp256" or "ecdsa_p256" => CngAlgorithm.ECDsaP256, + "nistp384" or "ecdsa_p384" => CngAlgorithm.ECDsaP384, + "nistp521" or "ecdsa_p521" => CngAlgorithm.ECDsaP521, + _ => CngAlgorithm.ECDsa, // All other curves are new in Win10 so use generic algorithm + }; } /// /// Map a curve name to algorithm. This enables curves that worked pre-Win10 /// to work with newer APIs for import and export. /// - internal static CngAlgorithm EcdhCurveNameToAlgorithm(string name) + internal static CngAlgorithm EcdhCurveNameToAlgorithm(ReadOnlySpan name) { - switch (name) + const int MaxCurveNameLength = 16; + Span nameLower = stackalloc char[MaxCurveNameLength]; + int written = name.ToLowerInvariant(nameLower); + + // Either it is empty or too big for the buffer, and the buffer is large enough to hold all mapped + // curve names, so return the generic algorithm. + if (written < 1) { - case "nistP256": - case "ECDH_P256": - case "ECDSA_P256": - return CngAlgorithm.ECDiffieHellmanP256; - - case "nistP384": - case "ECDH_P384": - case "ECDSA_P384": - return CngAlgorithm.ECDiffieHellmanP384; - - case "nistP521": - case "ECDH_P521": - case "ECDSA_P521": - return CngAlgorithm.ECDiffieHellmanP521; + return CngAlgorithm.ECDiffieHellman; } - // All other curves are new in Win10 so use generic algorithm - return CngAlgorithm.ECDiffieHellman; + return nameLower.Slice(0, written) switch + { + "nistp256" or "ecdsa_p256" or "ecdh_p256" => CngAlgorithm.ECDiffieHellmanP256, + "nistp384" or "ecdsa_p384" or "ecdh_p384" => CngAlgorithm.ECDiffieHellmanP384, + "nistp521" or "ecdsa_p521" or "ecdh_p521" => CngAlgorithm.ECDiffieHellmanP521, + _ => CngAlgorithm.ECDiffieHellman, // All other curves are new in Win10 so use generic algorithm + }; } internal static CngKey Create(ECCurve curve, Func algorithmResolver) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Helpers.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Helpers.cs index 12ebdeec85ce4d..f2f0e9f5ca6347 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Helpers.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Helpers.cs @@ -204,7 +204,8 @@ internal static bool AreSamePublicECParameters(ECParameters aParameters, ECParam if (aCurve.IsNamed) { // On Windows we care about FriendlyName, on Unix we care about Value - return (aCurve.Oid.Value == bCurve.Oid.Value && aCurve.Oid.FriendlyName == bCurve.Oid.FriendlyName); + return aCurve.Oid.Value == bCurve.Oid.Value && + string.Equals(aCurve.Oid.FriendlyName, bCurve.Oid.FriendlyName, StringComparison.OrdinalIgnoreCase); } if (!aCurve.IsExplicit) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/OidLookup.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/OidLookup.cs index 80134ead051d4d..18e9dc392070d4 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/OidLookup.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/OidLookup.cs @@ -96,7 +96,7 @@ internal static partial class OidLookup } /// Expected size of . - private const int FriendlyNameToOidCount = 111; + private const int FriendlyNameToOidCount = 114; /// Expected size of . private const int OidToFriendlyNameCount = 103; @@ -206,9 +206,9 @@ static void AddEntry(string oid, string primaryFriendlyName, string[]? additiona AddEntry("1.3.133.16.840.63.0.2", "ECDH_STD_SHA1_KDF"); AddEntry("1.3.132.1.11.1", "ECDH_STD_SHA256_KDF"); AddEntry("1.3.132.1.11.2", "ECDH_STD_SHA384_KDF"); - AddEntry("1.2.840.10045.3.1.7", "ECDSA_P256", new[] { "nistP256", "secP256r1", "x962P256v1" } ); - AddEntry("1.3.132.0.34", "ECDSA_P384", new[] { "nistP384", "secP384r1" }); - AddEntry("1.3.132.0.35", "ECDSA_P521", new[] { "nistP521", "secP521r1" }); + AddEntry("1.2.840.10045.3.1.7", "ECDSA_P256", new[] { "nistP256", "secP256r1", "x962P256v1", "ECDH_P256" } ); + AddEntry("1.3.132.0.34", "ECDSA_P384", new[] { "nistP384", "secP384r1", "ECDH_P384" }); + AddEntry("1.3.132.0.35", "ECDSA_P521", new[] { "nistP521", "secP521r1", "ECDH_P521" }); AddEntry("1.2.840.113549.1.9.16.3.5", "ESDH"); AddEntry("2.5.4.42", "G"); AddEntry("2.5.4.43", "I"); diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestUsageTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestUsageTests.cs index 9eb846a2ccb90b..e65a0445b09b28 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestUsageTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestUsageTests.cs @@ -1094,6 +1094,31 @@ public static void CreateSigningRequestWithDuplicateAttributes(bool reversed) Assert.Equal(ExpectedPem, output); } + [Theory] + [InlineData("NISTP256", "SHA256")] + [InlineData("NISTP384", "SHA384")] + [InlineData("NISTP521", "SHA512")] + [InlineData("EcDsA_p256", "SHA256")] + [InlineData("EcDsA_p384", "SHA384")] + [InlineData("EcDsA_p521", "SHA512")] + public static void CreateSelfSigned_CurveNameIsCaseInsensitive(string curveName, string hashName) + { + ECCurve ecCurve = ECCurve.CreateFromFriendlyName(curveName); + + using (ECDsa ecdsa = ECDsa.Create(ecCurve)) + { + CertificateRequest request = new("CN=blah", ecdsa, new HashAlgorithmName(hashName)); + DateTimeOffset notBefore = DateTimeOffset.UtcNow; + DateTimeOffset notAfter = notBefore.AddDays(30); + + using (X509Certificate2 selfSignedCert = request.CreateSelfSigned(notBefore, notAfter)) + using (ECDsa publicKey = selfSignedCert.GetECDsaPublicKey()) + { + Assert.NotNull(publicKey); + } + } + } + private class InvalidSignatureGenerator : X509SignatureGenerator { private readonly byte[] _signatureAlgBytes;