From 644417a9f0bc9d4146d4b01d9929f9e70002f5ff Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 2 Nov 2022 15:10:10 -0400 Subject: [PATCH 1/7] Fix EC curve case sensitivity on Windows --- .../ECDiffieHellmanTests.ImportExport.cs | 16 +++++++++++++- .../ECDsa/ECDsaImportExport.cs | 14 ++++++++++++ .../System/Security/Cryptography/CngKey.EC.cs | 5 +++-- .../System/Security/Cryptography/Helpers.cs | 3 ++- .../CertificateRequestUsageTests.cs | 22 +++++++++++++++++++ 5 files changed, 56 insertions(+), 4 deletions(-) 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..df0a83b5d6cae4 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 @@ -14,7 +14,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 +416,20 @@ public static void ImportFromPrivateOnlyKey() } } + [Theory] + [InlineData("NISTP256", "1.2.840.10045.3.1.7")] + [InlineData("NISTP384", "1.3.132.0.34")] + [InlineData("NISTP521", "1.3.132.0.35")] + public static void OidPresentOnCurveMiscased(string curveName, string expectedOid) + { + using (ECDiffieHellman ecdh = ECDiffieHellmanFactory.Create()) + { + ecdh.GenerateKey(ECCurve.CreateFromFriendlyName(curveName)); + ECParameters exportedParameters = ecdh.ExportParameters(false); + Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); + } + } + 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..58eb7f6339f43f 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 @@ -353,6 +353,20 @@ public static void ImportFromPrivateOnlyKey() } } + [Theory] + [InlineData("NISTP256", "1.2.840.10045.3.1.7")] + [InlineData("NISTP384", "1.3.132.0.34")] + [InlineData("NISTP521", "1.3.132.0.35")] + public static void OidPresentOnCurveMiscased(string curveName, string expectedOid) + { + using (ECDsa ecdsa = ECDsaFactory.Create()) + { + ecdsa.GenerateKey(ECCurve.CreateFromFriendlyName(curveName)); + ECParameters exportedParameters = ecdsa.ExportParameters(false); + Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); + } + } + 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..9dfad855d2d5ad 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) 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 f2a4addbca6a5d..4f216e07170a98 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/tests/X509Certificates/CertificateCreation/CertificateRequestUsageTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestUsageTests.cs index 9eb846a2ccb90b..9fb3a751e4e1f1 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,28 @@ public static void CreateSigningRequestWithDuplicateAttributes(bool reversed) Assert.Equal(ExpectedPem, output); } + [Theory] + [InlineData("NISTP256", "SHA256")] + [InlineData("NISTP384", "SHA384")] + [InlineData("NISTP521", "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; From 9b34e76f621f41b698d3ff4e1c551820614d87da Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 3 Nov 2022 22:46:29 -0400 Subject: [PATCH 2/7] Change assertions to handle Windows prior to 10 --- .../ECDiffieHellmanTests.ImportExport.cs | 15 ++++++++++++--- .../ECDsa/ECDsaImportExport.cs | 15 ++++++++++++--- .../CertificateRequestUsageTests.cs | 2 +- 3 files changed, 25 insertions(+), 7 deletions(-) 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 df0a83b5d6cae4..e0f38ee2f23f39 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 @@ -424,9 +424,18 @@ public static void OidPresentOnCurveMiscased(string curveName, string expectedOi { using (ECDiffieHellman ecdh = ECDiffieHellmanFactory.Create()) { - ecdh.GenerateKey(ECCurve.CreateFromFriendlyName(curveName)); - ECParameters exportedParameters = ecdh.ExportParameters(false); - Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); + ECCurve curve = ECCurve.CreateFromFriendlyName(curveName); + + if (PlatformDetection.IsWindows10OrLater) + { + ecdh.GenerateKey(curve); + ECParameters exportedParameters = ecdh.ExportParameters(false); + Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); + } + else + { + Assert.Throws(() => ecdh.GenerateKey(curve)); + } } } 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 58eb7f6339f43f..13ef802ecef5c8 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 @@ -361,9 +361,18 @@ public static void OidPresentOnCurveMiscased(string curveName, string expectedOi { using (ECDsa ecdsa = ECDsaFactory.Create()) { - ecdsa.GenerateKey(ECCurve.CreateFromFriendlyName(curveName)); - ECParameters exportedParameters = ecdsa.ExportParameters(false); - Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); + ECCurve curve = ECCurve.CreateFromFriendlyName(curveName); + + if (PlatformDetection.IsWindows10OrLater) + { + ecdsa.GenerateKey(curve); + ECParameters exportedParameters = ecdsa.ExportParameters(false); + Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); + } + else + { + Assert.Throws(() => ecdsa.GenerateKey(curve)); + } } } 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 9fb3a751e4e1f1..5e8fde3600f986 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestUsageTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestUsageTests.cs @@ -1094,7 +1094,7 @@ public static void CreateSigningRequestWithDuplicateAttributes(bool reversed) Assert.Equal(ExpectedPem, output); } - [Theory] + [ConditionalTheory(nameof(PlatformDetection), nameof(PlatformDetection.IsWindows10OrLater))] [InlineData("NISTP256", "SHA256")] [InlineData("NISTP384", "SHA384")] [InlineData("NISTP521", "SHA512")] From 8f997ad767cfd3d84158843b9c28d950fc274f27 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 4 Nov 2022 12:21:49 -0400 Subject: [PATCH 3/7] Handle additional curve case sensitivity scenarios. --- .../Cryptography/ECCng.ImportExport.cs | 57 +--------------- .../ECDiffieHellmanTests.ImportExport.cs | 21 +++--- .../ECDsa/ECDsaImportExport.cs | 23 ++++--- .../System/Security/Cryptography/CngKey.EC.cs | 65 +++++++++---------- .../CertificateRequestUsageTests.cs | 5 +- 5 files changed, 59 insertions(+), 112 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/ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs index e0f38ee2f23f39..75e76dec8e669d 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 @@ -420,22 +420,21 @@ public static void ImportFromPrivateOnlyKey() [InlineData("NISTP256", "1.2.840.10045.3.1.7")] [InlineData("NISTP384", "1.3.132.0.34")] [InlineData("NISTP521", "1.3.132.0.35")] + [InlineData("ecdh_P256", "1.2.840.10045.3.1.7")] + [InlineData("ecdh_P384", "1.3.132.0.34")] + [InlineData("ecdh_P521", "1.3.132.0.35")] public static void OidPresentOnCurveMiscased(string curveName, string expectedOid) { + ECCurve curve = ECCurve.CreateFromFriendlyName(curveName); + using (ECDiffieHellman ecdh = ECDiffieHellmanFactory.Create()) { - ECCurve curve = ECCurve.CreateFromFriendlyName(curveName); + ecdh.GenerateKey(curve); + ECParameters exportedParameters = ecdh.ExportParameters(false); + Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); - if (PlatformDetection.IsWindows10OrLater) - { - ecdh.GenerateKey(curve); - ECParameters exportedParameters = ecdh.ExportParameters(false); - Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); - } - else - { - Assert.Throws(() => ecdh.GenerateKey(curve)); - } + exportedParameters.Curve = curve; + ecdh.ImportParameters(exportedParameters); } } 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 13ef802ecef5c8..5433cea8b93c5e 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 @@ -357,22 +357,21 @@ public static void ImportFromPrivateOnlyKey() [InlineData("NISTP256", "1.2.840.10045.3.1.7")] [InlineData("NISTP384", "1.3.132.0.34")] [InlineData("NISTP521", "1.3.132.0.35")] - public static void OidPresentOnCurveMiscased(string curveName, string expectedOid) + [InlineData("ecdsa_P256", "1.2.840.10045.3.1.7")] + [InlineData("ecdsa_P384", "1.3.132.0.34")] + [InlineData("ecdsa_P521", "1.3.132.0.35")] + public static void GenerateKey_OidPresentOnCurveMiscased(string curveName, string expectedOid) { + ECCurve curve = ECCurve.CreateFromFriendlyName(curveName); + using (ECDsa ecdsa = ECDsaFactory.Create()) { - ECCurve curve = ECCurve.CreateFromFriendlyName(curveName); + ecdsa.GenerateKey(curve); + ECParameters exportedParameters = ecdsa.ExportParameters(false); + Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); - if (PlatformDetection.IsWindows10OrLater) - { - ecdsa.GenerateKey(curve); - ECParameters exportedParameters = ecdsa.ExportParameters(false); - Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); - } - else - { - Assert.Throws(() => ecdsa.GenerateKey(curve)); - } + exportedParameters.Curve = curve; + ecdsa.ImportParameters(exportedParameters); } } 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 9dfad855d2d5ad..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 @@ -82,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/tests/X509Certificates/CertificateCreation/CertificateRequestUsageTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestUsageTests.cs index 5e8fde3600f986..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,10 +1094,13 @@ public static void CreateSigningRequestWithDuplicateAttributes(bool reversed) Assert.Equal(ExpectedPem, output); } - [ConditionalTheory(nameof(PlatformDetection), nameof(PlatformDetection.IsWindows10OrLater))] + [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); From bff87b39007529de60db86f20becaf359c947e64 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 4 Nov 2022 17:10:29 -0400 Subject: [PATCH 4/7] Fix FriendlyName lookup for ECDH on non-Windows platforms --- .../src/System/Security/Cryptography/OidLookup.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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"); From 96e1e836cb2def230682eebc37db77ec99794725 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Sat, 5 Nov 2022 14:45:48 -0400 Subject: [PATCH 5/7] Add tests with brainpool curves. Brainpool curves are 'true' named cuves and get no special mapping, so use these as additional tests. --- .../EC/EccTestBase.cs | 16 +++++++ .../ECDiffieHellmanTests.ImportExport.cs | 43 ++++++++++++----- .../ECDsa/ECDsaImportExport.cs | 47 ++++++++++++++----- 3 files changed, 82 insertions(+), 24 deletions(-) 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..ebcb83fd52eb9c 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,22 @@ 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++) + { + destination[i] = str[i] switch + { + char c and >= 'A' and <= 'Z' => char.ToLowerInvariant(c), + char c and >= 'a' and <= 'z' => char.ToUpperInvariant(c), + char c => 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 75e76dec8e669d..f517695513dc01 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; @@ -417,27 +418,47 @@ public static void ImportFromPrivateOnlyKey() } [Theory] - [InlineData("NISTP256", "1.2.840.10045.3.1.7")] - [InlineData("NISTP384", "1.3.132.0.34")] - [InlineData("NISTP521", "1.3.132.0.35")] - [InlineData("ecdh_P256", "1.2.840.10045.3.1.7")] - [InlineData("ecdh_P384", "1.3.132.0.34")] - [InlineData("ecdh_P521", "1.3.132.0.35")] - public static void OidPresentOnCurveMiscased(string curveName, string expectedOid) + [MemberData(nameof(NamedCurves))] + public static void OidPresentOnCurveMiscased(ECCurve curve) { - ECCurve curve = ECCurve.CreateFromFriendlyName(curveName); + 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(curve); + ecdh.GenerateKey(miscasedCurve); ECParameters exportedParameters = ecdh.ExportParameters(false); - Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); + Assert.Equal(curve.Oid.Value, exportedParameters.Curve.Oid.Value); - exportedParameters.Curve = curve; + exportedParameters.Curve = miscasedCurve; 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 5433cea8b93c5e..513ec1e09d5d0e 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 { @@ -354,27 +355,47 @@ public static void ImportFromPrivateOnlyKey() } [Theory] - [InlineData("NISTP256", "1.2.840.10045.3.1.7")] - [InlineData("NISTP384", "1.3.132.0.34")] - [InlineData("NISTP521", "1.3.132.0.35")] - [InlineData("ecdsa_P256", "1.2.840.10045.3.1.7")] - [InlineData("ecdsa_P384", "1.3.132.0.34")] - [InlineData("ecdsa_P521", "1.3.132.0.35")] - public static void GenerateKey_OidPresentOnCurveMiscased(string curveName, string expectedOid) + [MemberData(nameof(NamedCurves))] + public static void OidPresentOnCurveMiscased(ECCurve curve) { - ECCurve curve = ECCurve.CreateFromFriendlyName(curveName); + 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(curve); + ecdsa.GenerateKey(miscasedCurve); ECParameters exportedParameters = ecdsa.ExportParameters(false); - Assert.Equal(expectedOid, exportedParameters.Curve.Oid.Value); + Assert.Equal(curve.Oid.Value, exportedParameters.Curve.Oid.Value); - exportedParameters.Curve = curve; + exportedParameters.Curve = miscasedCurve; 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 (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, ECDsa ec, int keySize, bool includePrivate) { parameters.Validate(); From be08425144de37eb705a2ba4c2942609aa209c24 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 9 Nov 2022 10:54:35 -0500 Subject: [PATCH 6/7] Some test improvements --- .../AlgorithmImplementations/EC/EccTestBase.cs | 8 ++------ .../AlgorithmImplementations/ECDsa/ECDsaImportExport.cs | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) 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 ebcb83fd52eb9c..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 @@ -256,12 +256,8 @@ internal static string InvertStringCase(string str) { for (int i = 0; i < str.Length; i++) { - destination[i] = str[i] switch - { - char c and >= 'A' and <= 'Z' => char.ToLowerInvariant(c), - char c and >= 'a' and <= 'z' => char.ToUpperInvariant(c), - char c => c - }; + char c = str[i]; + destination[i] = char.IsAsciiLetter(c) ? (char)(c ^ 0b0100000) : c; } }); } 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 513ec1e09d5d0e..e5ca87aaa10336 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 @@ -384,12 +384,12 @@ public static IEnumerable NamedCurves yield return new object[] { ECCurve.CreateFromFriendlyName("ECDSA_P384") }; yield return new object[] { ECCurve.CreateFromFriendlyName("ECDSA_P521") }; - if (ECDiffieHellmanFactory.IsCurveValid(ECCurve.NamedCurves.brainpoolP160r1.Oid)) + if (ECDsaFactory.IsCurveValid(ECCurve.NamedCurves.brainpoolP160r1.Oid)) { yield return new object[] { ECCurve.NamedCurves.brainpoolP160r1 }; } - if (ECDiffieHellmanFactory.IsCurveValid(ECCurve.NamedCurves.brainpoolP160t1.Oid)) + if (ECDsaFactory.IsCurveValid(ECCurve.NamedCurves.brainpoolP160t1.Oid)) { yield return new object[] { ECCurve.NamedCurves.brainpoolP160t1 }; } From 3243164c9d6249864ff063c159b455a681a5178e Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 9 Nov 2022 13:28:58 -0500 Subject: [PATCH 7/7] Code review feedback --- .../ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs | 2 ++ .../AlgorithmImplementations/ECDsa/ECDsaImportExport.cs | 2 ++ 2 files changed, 4 insertions(+) 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 f517695513dc01..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 @@ -432,6 +432,8 @@ public static void OidPresentOnCurveMiscased(ECCurve curve) 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); } } 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 e5ca87aaa10336..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 @@ -369,6 +369,8 @@ public static void OidPresentOnCurveMiscased(ECCurve curve) 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); } }