From ca3fc19a8b26dc952b770d055ff1b8ecdfea12e1 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 2 May 2025 14:48:39 -0400 Subject: [PATCH 1/2] Attempt to load X.509 keys as ECDH keys first. Windows CNG EC keys always have a key usage attached to them. ECDH keys can be treated as either ECDH or ECDSA. However, a CNG key with ECDSA usage can only be used as ECDSA. When we import a PEM aggregate with an ECC private key, we should attempt to import it as ECDH first, if the certificate's key usage permits it. This will allow the imported key to act as either ECDH or ECDSA. However, if we attempt to import as ECDSA first, it will succeed however not have the correct key usage, preventing it from being used as ECDH. --- .../X509Certificates/X509Certificate2.cs | 24 ++++----- .../X509Certificate2PemTests.cs | 50 +++++++++++++++++++ 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Certificate2.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Certificate2.cs index d67935833010f7..315ac1acdb694a 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Certificate2.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Certificate2.cs @@ -1172,18 +1172,18 @@ public static X509Certificate2 CreateFromPem(ReadOnlySpan certPem, ReadOnl s_DsaPublicKeyPrivateKeyLabels, static keyPem => CreateAndImport(keyPem, DSA.Create), certificate.CopyWithPrivateKey), - Oids.EcPublicKey when IsECDsa(certificate) => - ExtractKeyFromPem( - keyPem, - s_EcPublicKeyPrivateKeyLabels, - static keyPem => CreateAndImport(keyPem, ECDsa.Create), - certificate.CopyWithPrivateKey), Oids.EcPublicKey when IsECDiffieHellman(certificate) => ExtractKeyFromPem( keyPem, s_EcPublicKeyPrivateKeyLabels, static keyPem => CreateAndImport(keyPem, ECDiffieHellman.Create), certificate.CopyWithPrivateKey), + Oids.EcPublicKey when IsECDsa(certificate) => + ExtractKeyFromPem( + keyPem, + s_EcPublicKeyPrivateKeyLabels, + static keyPem => CreateAndImport(keyPem, ECDsa.Create), + certificate.CopyWithPrivateKey), Oids.MlKem512 or Oids.MlKem768 or Oids.MlKem1024 => ExtractKeyFromPem( keyPem, @@ -1259,18 +1259,18 @@ public static X509Certificate2 CreateFromEncryptedPem(ReadOnlySpan certPem password, static (keyPem, password) => CreateAndImportEncrypted(keyPem, password, DSA.Create), certificate.CopyWithPrivateKey), - Oids.EcPublicKey when IsECDsa(certificate) => - ExtractKeyFromEncryptedPem( - keyPem, - password, - static (keyPem, password) => CreateAndImportEncrypted(keyPem, password, ECDsa.Create), - certificate.CopyWithPrivateKey), Oids.EcPublicKey when IsECDiffieHellman(certificate) => ExtractKeyFromEncryptedPem( keyPem, password, static (keyPem, password) => CreateAndImportEncrypted(keyPem, password, ECDiffieHellman.Create), certificate.CopyWithPrivateKey), + Oids.EcPublicKey when IsECDsa(certificate) => + ExtractKeyFromEncryptedPem( + keyPem, + password, + static (keyPem, password) => CreateAndImportEncrypted(keyPem, password, ECDsa.Create), + certificate.CopyWithPrivateKey), Oids.MlKem512 or Oids.MlKem768 or Oids.MlKem1024 => ExtractKeyFromEncryptedPem( keyPem, diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509Certificate2PemTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509Certificate2PemTests.cs index bf786fc7dc2b1c..75a626ff95f3c7 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509Certificate2PemTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509Certificate2PemTests.cs @@ -268,6 +268,56 @@ public static void CreateFromPem_ECDH_Pkcs8_Success() } } + [Fact] + public static void CreateFromPem_EC_Pkcs8_Success() + { + // ecPublicKey certificates that have no key usage restrictions should be allowed to be used as both + // an ECDsa key and an ECDiffieHellman key. + + // For purposes of creating the certificate, it doesn't matter if we use an ECDSA or ECDH key, but starting + // with ECDSA means we can make a self-signed cert. + using ECDsa key = ECDsa.Create(); + key.ImportFromPem(TestData.EcDhPkcs8Key); + CertificateRequest req = new("CN=radish", key, HashAlgorithmName.SHA256); + using X509Certificate2 cert = req.CreateSelfSigned(DateTimeOffset.Now, DateTimeOffset.Now.AddYears(1)); + using X509Certificate2 pubOnly = X509CertificateLoader.LoadCertificate(cert.RawDataMemory.Span); + string pemAggregate = $"{pubOnly.ExportCertificatePem()}\n{TestData.EcDhPkcs8Key}"; + using X509Certificate2 reLoaded = X509Certificate2.CreateFromPem(pemAggregate, pemAggregate); + + AssertKeysMatch(TestData.EcDhPkcs8Key, reLoaded.GetECDiffieHellmanPrivateKey); + AssertKeysMatch(TestData.EcDhPkcs8Key, reLoaded.GetECDsaPrivateKey); + AssertExtensions.SequenceEqual(cert.SerialNumberBytes.Span, reLoaded.SerialNumberBytes.Span); + } + + [Fact] + public static void CreateFromEncryptedPem_EC_Pkcs8_Success() + { + // ecPublicKey certificates that have no key usage restrictions should be allowed to be used as both + // an ECDsa key and an ECDiffieHellman key. + + // For purposes of creating the certificate, it doesn't matter if we use an ECDSA or ECDH key, but starting + // with ECDSA means we can make a self-signed cert. + using ECDsa key = ECDsa.Create(); + key.ImportFromPem(TestData.EcDhPkcs8Key); + CertificateRequest req = new("CN=radish", key, HashAlgorithmName.SHA256); + using X509Certificate2 cert = req.CreateSelfSigned(DateTimeOffset.Now, DateTimeOffset.Now.AddYears(1)); + using X509Certificate2 pubOnly = X509CertificateLoader.LoadCertificate(cert.RawDataMemory.Span); + + PbeParameters pbe = new(PbeEncryptionAlgorithm.Aes128Cbc, HashAlgorithmName.SHA1, 32); + const string Password = "PLACEHOLDER"; + + string encryptedPrivateKey = PemEncoding.WriteString( + "ENCRYPTED PRIVATE KEY", + key.ExportEncryptedPkcs8PrivateKey(Password, pbe)); + + string pemAggregate = $"{pubOnly.ExportCertificatePem()}\n{encryptedPrivateKey}"; + using X509Certificate2 reLoaded = X509Certificate2.CreateFromEncryptedPem(pemAggregate, pemAggregate, Password); + + AssertKeysMatch(encryptedPrivateKey, reLoaded.GetECDiffieHellmanPrivateKey, Password); + AssertKeysMatch(encryptedPrivateKey, reLoaded.GetECDsaPrivateKey, Password); + AssertExtensions.SequenceEqual(cert.SerialNumberBytes.Span, reLoaded.SerialNumberBytes.Span); + } + [ConditionalFact(typeof(MLKem), nameof(MLKem.IsSupported))] public static void CreateFromPem_MLKem_Pkcs8_Success() { From 3a398df0424ef0ea17f1d257cd8f70742f5c5fc8 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 2 May 2025 17:18:57 -0400 Subject: [PATCH 2/2] Code review feedback --- .../tests/X509Certificates/X509Certificate2PemTests.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509Certificate2PemTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509Certificate2PemTests.cs index 75a626ff95f3c7..f31fd688e79381 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509Certificate2PemTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509Certificate2PemTests.cs @@ -280,8 +280,7 @@ public static void CreateFromPem_EC_Pkcs8_Success() key.ImportFromPem(TestData.EcDhPkcs8Key); CertificateRequest req = new("CN=radish", key, HashAlgorithmName.SHA256); using X509Certificate2 cert = req.CreateSelfSigned(DateTimeOffset.Now, DateTimeOffset.Now.AddYears(1)); - using X509Certificate2 pubOnly = X509CertificateLoader.LoadCertificate(cert.RawDataMemory.Span); - string pemAggregate = $"{pubOnly.ExportCertificatePem()}\n{TestData.EcDhPkcs8Key}"; + string pemAggregate = $"{cert.ExportCertificatePem()}\n{TestData.EcDhPkcs8Key}"; using X509Certificate2 reLoaded = X509Certificate2.CreateFromPem(pemAggregate, pemAggregate); AssertKeysMatch(TestData.EcDhPkcs8Key, reLoaded.GetECDiffieHellmanPrivateKey); @@ -301,7 +300,6 @@ public static void CreateFromEncryptedPem_EC_Pkcs8_Success() key.ImportFromPem(TestData.EcDhPkcs8Key); CertificateRequest req = new("CN=radish", key, HashAlgorithmName.SHA256); using X509Certificate2 cert = req.CreateSelfSigned(DateTimeOffset.Now, DateTimeOffset.Now.AddYears(1)); - using X509Certificate2 pubOnly = X509CertificateLoader.LoadCertificate(cert.RawDataMemory.Span); PbeParameters pbe = new(PbeEncryptionAlgorithm.Aes128Cbc, HashAlgorithmName.SHA1, 32); const string Password = "PLACEHOLDER"; @@ -310,7 +308,7 @@ public static void CreateFromEncryptedPem_EC_Pkcs8_Success() "ENCRYPTED PRIVATE KEY", key.ExportEncryptedPkcs8PrivateKey(Password, pbe)); - string pemAggregate = $"{pubOnly.ExportCertificatePem()}\n{encryptedPrivateKey}"; + string pemAggregate = $"{cert.ExportCertificatePem()}\n{encryptedPrivateKey}"; using X509Certificate2 reLoaded = X509Certificate2.CreateFromEncryptedPem(pemAggregate, pemAggregate, Password); AssertKeysMatch(encryptedPrivateKey, reLoaded.GetECDiffieHellmanPrivateKey, Password);