From 6859f260bcc56b5e5e91e31090ca6aa59609f5c0 Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Tue, 8 Apr 2025 15:55:24 -0700 Subject: [PATCH 1/3] Prepare crypto tests for new signing algorithms --- .../X509Certificates/CertificateAuthority.cs | 330 ++++++++++++++---- .../TestUtilities/System/AssertExtensions.cs | 6 + .../CertificateRequestChainTests.cs | 54 +-- .../CertificateRequestLoadTests.cs | 100 ++++++ .../CertificateCreation/CrlBuilderTests.cs | 310 +++++++++++----- .../PrivateKeyAssociationTests.cs | 226 ++++++++++++ 6 files changed, 840 insertions(+), 186 deletions(-) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs index 38b9ff44c09230..516d2c5910b35c 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs @@ -2,8 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics; using System.Formats.Asn1; using System.Linq; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using Xunit; namespace System.Security.Cryptography.X509Certificates.Tests.Common @@ -42,6 +45,7 @@ internal sealed class CertificateAuthority : IDisposable private static readonly Asn1Tag s_context0 = new Asn1Tag(TagClass.ContextSpecific, 0); private static readonly Asn1Tag s_context1 = new Asn1Tag(TagClass.ContextSpecific, 1); private static readonly Asn1Tag s_context2 = new Asn1Tag(TagClass.ContextSpecific, 2); + private static readonly KeyFactory[] s_variantKeyFactories = KeyFactory.BuildVariantFactories(); private static readonly X500DistinguishedName s_nonParticipatingName = new X500DistinguishedName("CN=The Ghost in the Machine"); @@ -148,7 +152,7 @@ internal void Revoke(X509Certificate2 certificate, DateTimeOffset revocationTime internal X509Certificate2 CreateSubordinateCA( string subject, - RSA publicKey, + PublicKey publicKey, int? depthLimit = null) { return CreateCertificate( @@ -164,7 +168,7 @@ internal X509Certificate2 CreateSubordinateCA( s_caKeyUsage }); } - internal X509Certificate2 CreateEndEntity(string subject, RSA publicKey, X509ExtensionCollection extensions) + internal X509Certificate2 CreateEndEntity(string subject, PublicKey publicKey, X509ExtensionCollection extensions) { return CreateCertificate( subject, @@ -174,6 +178,13 @@ internal X509Certificate2 CreateEndEntity(string subject, RSA publicKey, X509Ext } internal X509Certificate2 CreateOcspSigner(string subject, RSA publicKey) + { + return CreateOcspSigner( + subject, + X509SignatureGenerator.CreateForRSA(publicKey, RSASignaturePadding.Pkcs1).PublicKey); + } + + internal X509Certificate2 CreateOcspSigner(string subject, PublicKey publicKey) { return CreateCertificate( subject, @@ -207,7 +218,7 @@ private void RebuildRootWithRevocation(X509Extension cdpExtension, X509Extension throw new InvalidOperationException(); } - var req = new CertificateRequest(subjectName, _cert.PublicKey, HashAlgorithmName.SHA256); + var req = new CertificateRequest(subjectName, _cert.PublicKey, HashAlgorithmIfNeeded(_cert.GetKeyAlgorithm())); foreach (X509Extension ext in _cert.Extensions) { @@ -222,21 +233,21 @@ private void RebuildRootWithRevocation(X509Extension cdpExtension, X509Extension X509Certificate2 dispose = _cert; using (dispose) - using (RSA rsa = _cert.GetRSAPrivateKey()) + using (KeyHolder key = new KeyHolder(_cert)) using (X509Certificate2 tmp = req.Create( subjectName, - X509SignatureGenerator.CreateForRSA(rsa, RSASignaturePadding.Pkcs1), + key.GetGenerator(), new DateTimeOffset(_cert.NotBefore), new DateTimeOffset(_cert.NotAfter), serial)) { - _cert = tmp.CopyWithPrivateKey(rsa); + _cert = key.OntoCertificate(tmp); } } private X509Certificate2 CreateCertificate( string subject, - RSA publicKey, + PublicKey publicKey, TimeSpan nestingBuffer, X509ExtensionCollection extensions, bool ocspResponder = false) @@ -257,9 +268,9 @@ private X509Certificate2 CreateCertificate( } CertificateRequest request = new CertificateRequest( - subject, + new X500DistinguishedName(subject), publicKey, - HashAlgorithmName.SHA256, + HashAlgorithmIfNeeded(_cert.GetKeyAlgorithm()), RSASignaturePadding.Pkcs1); foreach (X509Extension extension in extensions) @@ -282,11 +293,15 @@ private X509Certificate2 CreateCertificate( byte[] serial = new byte[sizeof(long)]; RandomNumberGenerator.Fill(serial); - return request.Create( - _cert, - _cert.NotBefore.Add(nestingBuffer), - _cert.NotAfter.Subtract(nestingBuffer), - serial); + using (KeyHolder key = new KeyHolder(_cert)) + { + return request.Create( + _cert.SubjectName, + key.GetGenerator(), + _cert.NotBefore.Add(nestingBuffer), + _cert.NotAfter.Subtract(nestingBuffer), + serial); + } } internal byte[] GetCertData() @@ -337,14 +352,14 @@ internal byte[] GetCrl() nextUpdate = newExpiry; } - using (RSA key = _cert.GetRSAPrivateKey()) + using (KeyHolder key = new KeyHolder(_cert)) { crl = builder.Build( CorruptRevocationIssuerName ? s_nonParticipatingName : _cert.SubjectName, - X509SignatureGenerator.CreateForRSA(key, RSASignaturePadding.Pkcs1), + key.GetGenerator(), _crlNumber, nextUpdate, - HashAlgorithmName.SHA256, + HashAlgorithmIfNeeded(key.ToPublicKey().Oid.Value), _akidExtension, thisUpdate); } @@ -366,16 +381,10 @@ private byte[] BuildCrlManually( DateTimeOffset newExpiry, X509AuthorityKeyIdentifierExtension akidExtension) { - AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); - - using (writer.PushSequence()) - { - writer.WriteObjectIdentifier("1.2.840.113549.1.1.11"); - writer.WriteNull(); - } + using KeyHolder key = new KeyHolder(_cert); + byte[] signatureAlgId = key.GetSignatureAlgorithmIdentifier(); - byte[] signatureAlgId = writer.Encode(); - writer.Reset(); + AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); // TBSCertList using (writer.PushSequence()) @@ -473,17 +482,11 @@ private byte[] BuildCrlManually( byte[] tbsCertList = writer.Encode(); writer.Reset(); - byte[] signature; + byte[] signature = key.Sign(tbsCertList); - using (RSA key = _cert.GetRSAPrivateKey()) + if (CorruptRevocationSignature) { - signature = - key.SignData(tbsCertList, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); - - if (CorruptRevocationSignature) - { - signature[5] ^= 0xFF; - } + signature[5] ^= 0xFF; } // CertificateList @@ -568,7 +571,7 @@ singleExtensions [1] EXPLICIT Extensions OPTIONAL } { writer.PushSequence(s_context1); - // Fracational seconds "MUST NOT" be used here. Android and macOS 13+ enforce this and + // Fractional seconds "MUST NOT" be used here. Android and macOS 13+ enforce this and // reject GeneralizedTime's with fractional seconds, so omit them. // RFC 6960: 4.2.2.1: // The format for GeneralizedTime is as specified in Section 4.1.2.5.2 of [RFC5280]. @@ -630,18 +633,11 @@ certs [0] EXPLICIT SEQUENCE OF Certificate OPTIONAL } { writer.WriteEncodedValue(tbsResponseData); - using (writer.PushSequence()) + using (KeyHolder key = new KeyHolder(responder)) { - writer.WriteObjectIdentifier("1.2.840.113549.1.1.11"); - writer.WriteNull(); - } + writer.WriteEncodedValue(key.GetSignatureAlgorithmIdentifier()); - using (RSA rsa = responder.GetRSAPrivateKey()) - { - byte[] signature = rsa.SignData( - tbsResponseData, - HashAlgorithmName.SHA256, - RSASignaturePadding.Pkcs1); + byte[] signature = key.Sign(tbsResponseData); if (CorruptRevocationSignature) { @@ -794,6 +790,7 @@ private enum CertStatus Revoked, } + [OverloadResolutionPriority(-1)] internal static void BuildPrivatePki( PkiOptions pkiOptions, out RevocationResponder responder, @@ -807,6 +804,35 @@ internal static void BuildPrivatePki( string subjectName = null, int keySize = DefaultKeySize, X509ExtensionCollection extensions = null) + { + BuildPrivatePki( + pkiOptions, + out responder, + out rootAuthority, + out intermediateAuthorities, + out endEntityCert, + intermediateAuthorityCount, + testName, + registerAuthorities, + pkiOptionsInSubject, + subjectName, + KeyFactory.RSASize(keySize), + extensions); + } + + internal static void BuildPrivatePki( + PkiOptions pkiOptions, + out RevocationResponder responder, + out CertificateAuthority rootAuthority, + out CertificateAuthority[] intermediateAuthorities, + out X509Certificate2 endEntityCert, + int intermediateAuthorityCount, + string testName = null, + bool registerAuthorities = true, + bool pkiOptionsInSubject = false, + string subjectName = null, + KeyFactory keyFactory = null, + X509ExtensionCollection extensions = null) { bool rootDistributionViaHttp = !pkiOptions.HasFlag(PkiOptions.NoRootCertDistributionUri); bool issuerRevocationViaCrl = pkiOptions.HasFlag(PkiOptions.IssuerRevocationViaCrl); @@ -823,14 +849,38 @@ internal static void BuildPrivatePki( // default to client extensions ??= new X509ExtensionCollection() { s_eeConstraints, s_eeKeyUsage, s_tlsClientEku }; - using (RSA rootKey = RSA.Create(keySize)) - using (RSA eeKey = RSA.Create(keySize)) + if (keyFactory is null) + { + // This could use any of the NC-hashes, but that complicates the code sharing for this file, + // so use IncrementalHash(SHA256) as it's inbox. + // + // System.HashCode isn't suitable because it's randomized, and we want the algorithm to + // be consistent for any given test from run to run. + using (IncrementalHash hasher = IncrementalHash.CreateHash(HashAlgorithmName.SHA256)) + { + // The use of AsBytes means that the hash value will differ between Big and Little Endian + // platforms, but that's OK: a failing test in a given configuration will continue to fail + // in that configuration. + hasher.AppendData(MemoryMarshal.AsBytes(new ReadOnlySpan(ref pkiOptions))); + hasher.AppendData(MemoryMarshal.AsBytes(new ReadOnlySpan(ref intermediateAuthorityCount))); + hasher.AppendData(MemoryMarshal.AsBytes(testName.AsSpan())); + hasher.AppendData(MemoryMarshal.AsBytes(subjectName.AsSpan())); + + Span hash = stackalloc byte[256 / 8]; + int written = hasher.GetCurrentHash(hash); + Debug.Assert(written == hash.Length); + + // Using mod here will create an imbalance any time s_variantKeyFactories isn't a power of 2, + // but that's OK. + keyFactory = s_variantKeyFactories[hash[0] % s_variantKeyFactories.Length]; + } + } + + using (KeyHolder rootKey = KeyHolder.CreateKey(keyFactory)) + using (KeyHolder eeKey = KeyHolder.CreateKey(keyFactory)) { - var rootReq = new CertificateRequest( - BuildSubject("A Revocation Test Root", testName, pkiOptions, pkiOptionsInSubject), - rootKey, - HashAlgorithmName.SHA256, - RSASignaturePadding.Pkcs1); + CertificateRequest rootReq = rootKey.CreateRequest( + BuildSubject("A Revocation Test Root", testName, pkiOptions, pkiOptionsInSubject)); X509BasicConstraintsExtension caConstraints = new X509BasicConstraintsExtension(true, false, 0, true); @@ -862,7 +912,7 @@ internal static void BuildPrivatePki( for (int intermediateIndex = 0; intermediateIndex < intermediateAuthorityCount; intermediateIndex++) { - using RSA intermediateKey = RSA.Create(keySize); + using KeyHolder intermediateKey = KeyHolder.CreateKey(keyFactory); // Don't dispose this, it's being transferred to the CertificateAuthority X509Certificate2 intermedCert; @@ -870,8 +920,8 @@ internal static void BuildPrivatePki( { X509Certificate2 intermedPub = issuingAuthority.CreateSubordinateCA( BuildSubject($"A Revocation Test CA {intermediateIndex}", testName, pkiOptions, pkiOptionsInSubject), - intermediateKey); - intermedCert = intermedPub.CopyWithPrivateKey(intermediateKey); + intermediateKey.ToPublicKey()); + intermedCert = intermediateKey.OntoCertificate(intermedPub); intermedPub.Dispose(); } @@ -894,11 +944,11 @@ internal static void BuildPrivatePki( endEntityCert = issuingAuthority.CreateEndEntity( BuildSubject(subjectName ?? "A Revocation Test Cert", testName, pkiOptions, pkiOptionsInSubject), - eeKey, + eeKey.ToPublicKey(), extensions); X509Certificate2 tmp = endEntityCert; - endEntityCert = endEntityCert.CopyWithPrivateKey(eeKey); + endEntityCert = eeKey.OntoCertificate(endEntityCert); tmp.Dispose(); } @@ -913,6 +963,7 @@ internal static void BuildPrivatePki( } } + [OverloadResolutionPriority(-1)] internal static void BuildPrivatePki( PkiOptions pkiOptions, out RevocationResponder responder, @@ -926,7 +977,6 @@ internal static void BuildPrivatePki( int keySize = DefaultKeySize, X509ExtensionCollection extensions = null) { - BuildPrivatePki( pkiOptions, out responder, @@ -944,6 +994,36 @@ internal static void BuildPrivatePki( intermediateAuthority = intermediateAuthorities.Single(); } + internal static void BuildPrivatePki( + PkiOptions pkiOptions, + out RevocationResponder responder, + out CertificateAuthority rootAuthority, + out CertificateAuthority intermediateAuthority, + out X509Certificate2 endEntityCert, + string testName = null, + bool registerAuthorities = true, + bool pkiOptionsInSubject = false, + string subjectName = null, + KeyFactory keyFactory = null, + X509ExtensionCollection extensions = null) + { + BuildPrivatePki( + pkiOptions, + out responder, + out rootAuthority, + out CertificateAuthority[] intermediateAuthorities, + out endEntityCert, + intermediateAuthorityCount: 1, + testName: testName, + registerAuthorities: registerAuthorities, + pkiOptionsInSubject: pkiOptionsInSubject, + subjectName: subjectName, + keyFactory: keyFactory, + extensions: extensions); + + intermediateAuthority = intermediateAuthorities.Single(); + } + private static string BuildSubject( string cn, string testName, @@ -955,5 +1035,137 @@ private static string BuildSubject( return $"CN=\"{cn}\"" + testNamePart + pkiOptionsPart; } + + private static HashAlgorithmName HashAlgorithmIfNeeded(string publicKeyOid) + { + const string Rsa = "1.2.840.113549.1.1.1"; + const string RsaPss = "1.2.840.113549.1.1.10"; + const string EcPublicKey = "1.2.840.10045.2.1"; + const string Dsa = "1.2.840.10040.4.1"; + + return publicKeyOid switch + { + Rsa or RsaPss or EcPublicKey or Dsa => HashAlgorithmName.SHA256, + _ => default, + }; + } + + internal static X509Certificate2 CloneWithPrivateKey(X509Certificate2 cert, object key) + { + return key switch + { + RSA rsa => cert.CopyWithPrivateKey(rsa), + ECDsa ecdsa => cert.CopyWithPrivateKey(ecdsa), + DSA dsa => cert.CopyWithPrivateKey(dsa), + _ => throw new InvalidOperationException( + $"Had no handler for key of type {key?.GetType().FullName ?? "null"}") + }; + } + + internal sealed class KeyFactory + { + internal static KeyFactory RSA { get; } = + new(() => Cryptography.RSA.Create(DefaultKeySize)); + + internal static KeyFactory ECDsa { get; } = + new(() => Cryptography.ECDsa.Create(ECCurve.NamedCurves.nistP384)); + + private Func _factory; + + private KeyFactory(Func factory) + { + _factory = factory; + } + + internal IDisposable CreateKey() + { + return _factory(); + } + + internal static KeyFactory RSASize(int keySize) + { + return new KeyFactory(() => Cryptography.RSA.Create(keySize)); + } + + internal static KeyFactory[] BuildVariantFactories() + { + return [RSA, ECDsa]; + } + } + + private sealed class KeyHolder : IDisposable + { + private readonly IDisposable _key; + private X509SignatureGenerator _generator; + + internal KeyHolder(IDisposable key) + { + _key = key; + } + + internal KeyHolder(X509Certificate2 cert) + { + // We're always in the context of signing something, so EC-DH does not apply. + _key = + cert.GetRSAPrivateKey() ?? + cert.GetECDsaPrivateKey() ?? + (IDisposable)cert.GetDSAPrivateKey() ?? + throw new NotSupportedException(); + } + + public void Dispose() + { + _key?.Dispose(); + } + + internal static KeyHolder CreateKey(KeyFactory factory) + { + return new KeyHolder(factory.CreateKey()); + } + + internal CertificateRequest CreateRequest(string subject) + { + return _key switch + { + RSA rsa => new CertificateRequest(subject, rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1), + ECDsa ecdsa => new CertificateRequest(subject, ecdsa, HashAlgorithmName.SHA256), + _ => throw new NotSupportedException(), + }; + } + + internal X509SignatureGenerator GetGenerator() + { + return _generator ??= _key switch + { + RSA rsa => X509SignatureGenerator.CreateForRSA(rsa, RSASignaturePadding.Pkcs1), + ECDsa ecdsa => X509SignatureGenerator.CreateForECDsa(ecdsa), + _ => throw new NotSupportedException(), + }; + } + + internal PublicKey ToPublicKey() + { + return GetGenerator().PublicKey; + } + + internal X509Certificate2 OntoCertificate(X509Certificate2 cert) + { + return CloneWithPrivateKey(cert, _key); + } + + internal byte[] Sign(byte[] data) + { + X509SignatureGenerator generator = GetGenerator(); + return generator.SignData(data, HashAlgorithmIfNeeded(generator.PublicKey.Oid.Value)); + } + + internal byte[] GetSignatureAlgorithmIdentifier() + { + X509SignatureGenerator generator = GetGenerator(); + + return generator.GetSignatureAlgorithmIdentifier( + HashAlgorithmIfNeeded(generator.PublicKey.Oid.Value)); + } + } } } diff --git a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs index f8e8b3cc9d3626..f5e4d72ca7fcb7 100644 --- a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs +++ b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs @@ -45,6 +45,12 @@ public static void Throws(Action action, string expectedMessage) Assert.Equal(expectedMessage, Assert.Throws(action).Message); } + public static void ThrowsContains(string paramName, Action action, string expectedMessageContent) + where T : ArgumentException + { + Assert.Contains(expectedMessageContent, Assert.Throws(paramName, action).Message); + } + public static void ThrowsContains(Action action, string expectedMessageContent) where T : Exception { diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestChainTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestChainTests.cs index 008402ee37b4e2..a13d9ff6287391 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestChainTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestChainTests.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Linq; -using Test.Cryptography; using Xunit; namespace System.Security.Cryptography.X509Certificates.Tests.CertificateCreation @@ -203,7 +202,7 @@ public static void ChainCertRequirements(bool useIntermed, bool? isCA, X509KeyUs private static CertificateRequest OpenCertRequest( string dn, - AsymmetricAlgorithm key, + object key, HashAlgorithmName hashAlgorithm) { X500DistinguishedName x500dn = new X500DistinguishedName(dn); @@ -216,25 +215,20 @@ private static CertificateRequest OpenCertRequest( }; } - private static X509SignatureGenerator OpenGenerator(AsymmetricAlgorithm key) + private static X509SignatureGenerator OpenGenerator(object key) { - RSA rsa = key as RSA; - - if (rsa != null) - return X509SignatureGenerator.CreateForRSA(rsa, RSASignaturePadding.Pkcs1); - - ECDsa ecdsa = key as ECDsa; - - if (ecdsa != null) - return X509SignatureGenerator.CreateForECDsa(ecdsa); - - throw new InvalidOperationException( - $"Had no handler for key of type {key?.GetType().FullName ?? "null"}"); + return key switch + { + RSA rsa => X509SignatureGenerator.CreateForRSA(rsa, RSASignaturePadding.Pkcs1), + ECDsa ecdsa => X509SignatureGenerator.CreateForECDsa(ecdsa), + _ => throw new InvalidOperationException( + $"Had no handler for key of type {key?.GetType().FullName ?? "null"}") + }; } private static CertificateRequest CreateChainRequest( string dn, - AsymmetricAlgorithm key, + object key, HashAlgorithmName hashAlgorithm, bool isCa, int? pathLen) @@ -323,32 +317,16 @@ private static void DisposeChainCerts(X509Chain chain) } } - private static X509Certificate2 CloneWithPrivateKey(X509Certificate2 cert, AsymmetricAlgorithm key) + private static X509Certificate2 CloneWithPrivateKey(X509Certificate2 cert, object key) { - RSA rsa = key as RSA; - - if (rsa != null) - return cert.CopyWithPrivateKey(rsa); - - ECDsa ecdsa = key as ECDsa; - - if (ecdsa != null) - return cert.CopyWithPrivateKey(ecdsa); - - DSA dsa = key as DSA; - - if (dsa != null) - return cert.CopyWithPrivateKey(dsa); - - throw new InvalidOperationException( - $"Had no handler for key of type {key?.GetType().FullName ?? "null"}"); + return Common.CertificateAuthority.CloneWithPrivateKey(cert, key); } private static void CreateAndTestChain( - AsymmetricAlgorithm rootPrivKey, - AsymmetricAlgorithm intermed1PrivKey, - AsymmetricAlgorithm intermed2PrivKey, - AsymmetricAlgorithm leafPubKey) + object rootPrivKey, + object intermed1PrivKey, + object intermed2PrivKey, + object leafPubKey) { const string RootDN = "CN=Experimental Root Certificate"; const string Intermed1DN = "CN=First Intermediate Certificate, O=Experimental"; diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestLoadTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestLoadTests.cs index b29429f9e80a9f..51255cccf8f9aa 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestLoadTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CertificateRequestLoadTests.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.Formats.Asn1; using System.Net; using Test.Cryptography; using Xunit; @@ -718,6 +719,105 @@ public static void LoadRequestWithAttributeValues() Assert.Equal("0C053132333435", attr.RawData.ByteArrayToHex()); } + [Fact] + public static void LoadCreate_MatchesCreate_RSAPkcs1() + { + using (RSA key = RSA.Create(2048)) + { + LoadCreate_MatchesCreate( + new CertificateRequest( + "CN=Roundtrip, O=RSA, OU=PKCS1", + key, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1), + X509SignatureGenerator.CreateForRSA(key, RSASignaturePadding.Pkcs1), + deterministicSignature: true); + } + } + + [Fact] + public static void LoadCreate_MatchesCreate_RSAPss() + { + using (RSA key = RSA.Create(2048)) + { + LoadCreate_MatchesCreate( + new CertificateRequest( + "CN=Roundtrip, O=RSA, OU=PSS", + key, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pss), + X509SignatureGenerator.CreateForRSA(key, RSASignaturePadding.Pss), + deterministicSignature: false); + } + } + + [Fact] + public static void LoadCreate_MatchesCreate_ECDsa() + { + using (ECDsa key = ECDsa.Create(ECCurve.NamedCurves.nistP384)) + { + LoadCreate_MatchesCreate( + new CertificateRequest( + "CN=Roundtrip, O=EC-DSA", + key, + HashAlgorithmName.SHA256), + X509SignatureGenerator.CreateForECDsa(key), + deterministicSignature: false); + } + } + + private static void LoadCreate_MatchesCreate( + CertificateRequest request, + X509SignatureGenerator generator, + bool deterministicSignature) + { + DateTimeOffset now = DateTimeOffset.UtcNow; + DateTimeOffset notBefore = now.AddMonths(-1); + DateTimeOffset notAfter = now.AddMonths(1); + byte[] serial = new byte[] { 0x02, 0x04, 0x06, 0x08, 0x07, 0x05, 0x03, 0x01 }; + + request.CertificateExtensions.Add(new X509SubjectKeyIdentifierExtension(request.PublicKey, false)); + request.CertificateExtensions.Add(new X509EnhancedKeyUsageExtension(new OidCollection { new Oid("0.0.1", null) }, false)); + + byte[] pkcs10 = request.CreateSigningRequest(generator); + CertificateRequest loaded = CertificateRequest.LoadSigningRequest( + pkcs10, + HashAlgorithmName.SHA256, + CertificateRequestLoadOptions.UnsafeLoadCertificateExtensions); + + using (X509Certificate2 one = request.Create(request.SubjectName, generator, notBefore, notAfter, serial)) + using (X509Certificate2 two = loaded.Create(request.SubjectName, generator, notBefore, notAfter, serial)) + { + if (deterministicSignature) + { + AssertExtensions.SequenceEqual(one.RawDataMemory.Span, two.RawDataMemory.Span); + } + else + { + // tbsCertificate and signatureAlgorithm should match, signature should not. + // + // Certificate ::= SEQUENCE { + // tbsCertificate TBSCertificate, + // signatureAlgorithm AlgorithmIdentifier, + // signature BIT STRING } + + AsnValueReader readerOne = new AsnValueReader(one.RawDataMemory.Span, AsnEncodingRules.DER); + AsnValueReader readerTwo = new AsnValueReader(two.RawDataMemory.Span, AsnEncodingRules.DER); + + AsnValueReader certOne = readerOne.ReadSequence(); + AsnValueReader certTwo = readerTwo.ReadSequence(); + readerOne.ThrowIfNotEmpty(); + readerTwo.ThrowIfNotEmpty(); + + AssertExtensions.SequenceEqual(certOne.ReadEncodedValue(), certTwo.ReadEncodedValue()); + AssertExtensions.SequenceEqual(certOne.ReadEncodedValue(), certTwo.ReadEncodedValue()); + AssertExtensions.SequenceNotEqual(certOne.ReadEncodedValue(), certTwo.ReadEncodedValue()); + certOne.ThrowIfNotEmpty(); + certTwo.ThrowIfNotEmpty(); + } + } + } + private static void VerifyBigExponentRequest( CertificateRequest req, CertificateRequestLoadOptions options) diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CrlBuilderTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CrlBuilderTests.cs index 563502efbda656..1acd06fca487ce 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CrlBuilderTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CrlBuilderTests.cs @@ -16,6 +16,20 @@ public static class CrlBuilderTests { private const string CertParam = "issuerCertificate"; + public enum CertKind + { + ECDsa, + RsaPkcs1, + RsaPss, + } + + public static IEnumerable SupportedCertKinds() + { + yield return new object[] { CertKind.ECDsa }; + yield return new object[] { CertKind.RsaPkcs1 }; + yield return new object[] { CertKind.RsaPss }; + } + [Fact] public static void AddEntryArgumentValidation() { @@ -153,63 +167,102 @@ public static void BuildWithNextUpdateBeforeThisUpdate() }); } - [Fact] - public static void BuildWithNoHashAlgorithm() + [Theory] + [MemberData(nameof(SupportedCertKinds))] + public static void BuildWithNoHashAlgorithm(CertKind certKind) { BuildCertificateAndRun( + certKind, new X509Extension[] { X509BasicConstraintsExtension.CreateForCertificateAuthority(), }, - static (cert, now) => + static (certKind, cert, now) => { HashAlgorithmName hashAlg = default; CertificateRevocationListBuilder builder = new CertificateRevocationListBuilder(); - Assert.Throws( - "hashAlgorithm", - () => builder.Build(cert, 0, now.AddMinutes(5), hashAlg, null, now)); + Action certBuild = () => builder.Build(cert, 0, now.AddMinutes(5), hashAlg, null, now); - using (ECDsa key = cert.GetECDsaPrivateKey()) + if (RequiresHashAlgorithm(certKind)) { - X509SignatureGenerator gen = X509SignatureGenerator.CreateForECDsa(key); - X500DistinguishedName dn = cert.SubjectName; + Assert.Throws("hashAlgorithm", certBuild); + } + else + { + // Assert.NoThrow + certBuild(); + } + + X509SignatureGenerator gen = GetSignatureGenerator(certKind, cert, out IDisposable key); - Assert.Throws( - "hashAlgorithm", - () => builder.Build(dn, gen, 0, now.AddMinutes(5), hashAlg, null, now)); + using (key) + { + X500DistinguishedName dn = cert.SubjectName; + X509AuthorityKeyIdentifierExtension akid = + X509AuthorityKeyIdentifierExtension.CreateFromCertificate(cert, true, false); + + Action genBuild = () => builder.Build(dn, gen, 0, now.AddMinutes(5), hashAlg, akid, now); + + if (RequiresHashAlgorithm(certKind)) + { + Assert.Throws("hashAlgorithm", genBuild); + } + else + { + // Assert.NoThrow + genBuild(); + } } }); } - [Fact] - public static void BuildWithEmptyHashAlgorithm() + [Theory] + [MemberData(nameof(SupportedCertKinds))] + public static void BuildWithEmptyHashAlgorithm(CertKind certKind) { BuildCertificateAndRun( + certKind, new X509Extension[] { X509BasicConstraintsExtension.CreateForCertificateAuthority(), }, - static (cert, now) => + static (certKind, cert, now) => { HashAlgorithmName hashAlg = new HashAlgorithmName(""); CertificateRevocationListBuilder builder = new CertificateRevocationListBuilder(); - ArgumentException e = Assert.Throws( - "hashAlgorithm", - () => builder.Build(cert, 0, now.AddMinutes(5), hashAlg, null, now)); - Assert.Contains("empty", e.Message); + Action certAction = () => builder.Build(cert, 0, now.AddMinutes(5), hashAlg, null, now); - using (ECDsa key = cert.GetECDsaPrivateKey()) + if (RequiresHashAlgorithm(certKind)) { - X509SignatureGenerator gen = X509SignatureGenerator.CreateForECDsa(key); - X500DistinguishedName dn = cert.SubjectName; + AssertExtensions.ThrowsContains("hashAlgorithm", certAction, "empty"); + } + else + { + // Assert.NoThrow + certAction(); + } - e = Assert.Throws( - "hashAlgorithm", - () => builder.Build(dn, gen, 0, now.AddMinutes(5), hashAlg, null, now)); + X509SignatureGenerator gen = GetSignatureGenerator(certKind, cert, out IDisposable key); - Assert.Contains("empty", e.Message); + using (key) + { + X500DistinguishedName dn = cert.SubjectName; + X509AuthorityKeyIdentifierExtension akid = + X509AuthorityKeyIdentifierExtension.CreateFromCertificate(cert, true, false); + + Action genAction = () => builder.Build(dn, gen, 0, now.AddMinutes(5), hashAlg, akid, now); + + if (RequiresHashAlgorithm(certKind)) + { + Assert.Throws("hashAlgorithm", genAction); + } + else + { + // Assert.NoThrow + genAction(); + } } }); } @@ -349,7 +402,7 @@ public static void BuildWithGeneratorArgumentValidation() } [Fact] - public static void BuildEmpty() + public static void BuildEmptyRsaPkcs1() { BuildRsaCertificateAndRun( new X509Extension[] @@ -371,7 +424,8 @@ public static void BuildEmpty() // In fact, because RSASSA-PKCS1 is a deterministic algorithm, we can check it for a fixed output. AssertExtensions.SequenceEqual(BuildEmptyExpectedCrl, built); - }); + }, + callerName: "BuildEmpty"); } [Theory] @@ -421,20 +475,24 @@ public static void BuildEmptyRsaPss(string hashName) }); } - [Fact] - public static void BuildEmptyEcdsa() + [Theory] + [MemberData(nameof(SupportedCertKinds))] + public static void BuildEmpty(CertKind certKind) { BuildCertificateAndRun( + certKind, new X509Extension[] { X509BasicConstraintsExtension.CreateForCertificateAuthority(), }, - (cert, now) => + (certKind, cert, now) => { CertificateRevocationListBuilder builder = new CertificateRevocationListBuilder(); DateTimeOffset nextUpdate = now.AddHours(1); - byte[] crl = builder.Build(cert, 2, nextUpdate, HashAlgorithmName.SHA256); + HashAlgorithmName hashAlg = RequiresHashAlgorithm(certKind) ? HashAlgorithmName.SHA256 : default; + + byte[] crl = builder.Build(cert, 2, nextUpdate, hashAlg, GetRsaPadding(certKind)); AsnReader reader = new AsnReader(crl, AsnEncodingRules.DER); reader = reader.ReadSequence(); @@ -444,16 +502,7 @@ public static void BuildEmptyEcdsa() byte[] signature = reader.ReadBitString(out _); reader.ThrowIfNotEmpty(); - using (ECDsa pubKey = cert.GetECDsaPublicKey()) - { - Assert.True( - pubKey.VerifyData( - tbs.Span, - signature, - HashAlgorithmName.SHA256, - DSASignatureFormat.Rfc3279DerSequence), - "Certificate public key verifies CRL"); - } + VerifySignature(certKind, cert, tbs.Span, signature, hashAlg); VerifyCrlFields( crl, @@ -465,26 +514,30 @@ public static void BuildEmptyEcdsa() }); } - [Fact] - public static void BuildEmptyEcdsa_NoSubjectKeyIdentifier() + [Theory] + [MemberData(nameof(SupportedCertKinds))] + public static void BuildEmpty_NoSubjectKeyIdentifier(CertKind certKind) { BuildCertificateAndRun( + certKind, new X509Extension[] { X509BasicConstraintsExtension.CreateForCertificateAuthority(), }, - (cert, now) => + (certKind, cert, now) => { CertificateRevocationListBuilder builder = new CertificateRevocationListBuilder(); DateTimeOffset nextUpdate = now.AddHours(1); DateTimeOffset thisUpdate = now; + HashAlgorithmName hashAlg = RequiresHashAlgorithm(certKind) ? HashAlgorithmName.SHA256 : default; byte[] crl = builder.Build( cert, 2, nextUpdate, - HashAlgorithmName.SHA256, - thisUpdate: thisUpdate); + hashAlg, + GetRsaPadding(certKind), + thisUpdate); AsnReader reader = new AsnReader(crl, AsnEncodingRules.DER); reader = reader.ReadSequence(); @@ -494,16 +547,7 @@ public static void BuildEmptyEcdsa_NoSubjectKeyIdentifier() byte[] signature = reader.ReadBitString(out _); reader.ThrowIfNotEmpty(); - using (ECDsa pubKey = cert.GetECDsaPublicKey()) - { - Assert.True( - pubKey.VerifyData( - tbs.Span, - signature, - HashAlgorithmName.SHA256, - DSASignatureFormat.Rfc3279DerSequence), - "Certificate public key verifies CRL"); - } + VerifySignature(certKind, cert, tbs.Span, signature, hashAlg); VerifyCrlFields( crl, @@ -1430,17 +1474,34 @@ public static void LoadAndResignPublicCrl() } private static void BuildCertificateAndRun( + CertKind certKind, IEnumerable extensions, - Action action, + Action action, bool addSubjectKeyIdentifier = true, [CallerMemberName] string callerName = null) { - using (ECDsa key = ECDsa.Create()) + string subjectName = $"CN=\"{callerName}\""; + CertificateRequest req; + IDisposable key = null; + + try { - CertificateRequest req = new CertificateRequest( - $"CN=\"{callerName}\"", - key, - HashAlgorithmName.SHA384); + if (certKind == CertKind.ECDsa) + { + ECDsa ecdsa = ECDsa.Create(); + key = ecdsa; + req = new CertificateRequest(subjectName, ecdsa, HashAlgorithmName.SHA384); + } + else if (certKind == CertKind.RsaPkcs1 || certKind == CertKind.RsaPss) + { + RSA rsa = RSA.Create(TestData.RsaBigExponentParams); + key = rsa; + req = new CertificateRequest(subjectName, rsa, HashAlgorithmName.SHA384, GetRsaPadding(certKind)); + } + else + { + throw new NotSupportedException($"Unsupported CertKind: {certKind}"); + } if (addSubjectKeyIdentifier) { @@ -1456,42 +1517,41 @@ private static void BuildCertificateAndRun( using (X509Certificate2 cert = req.CreateSelfSigned(now.AddMonths(-1), now.AddMonths(1))) { - action(cert, now); + action(certKind, cert, now); } } + finally + { + key?.Dispose(); + } } - private static void BuildRsaCertificateAndRun( + private static void BuildCertificateAndRun( IEnumerable extensions, Action action, bool addSubjectKeyIdentifier = true, [CallerMemberName] string callerName = null) { - using (RSA key = RSA.Create(TestData.RsaBigExponentParams)) - { - CertificateRequest req = new CertificateRequest( - $"CN=\"{callerName}\"", - key, - HashAlgorithmName.SHA384, - RSASignaturePadding.Pkcs1); - - if (addSubjectKeyIdentifier) - { - req.CertificateExtensions.Add(new X509SubjectKeyIdentifierExtension(req.PublicKey, false)); - } - - foreach (X509Extension ext in extensions) - { - req.CertificateExtensions.Add(ext); - } - - DateTimeOffset now = DateTimeOffset.UtcNow; + BuildCertificateAndRun( + CertKind.ECDsa, + extensions, + (certKind, cert, now) => action(cert, now), + addSubjectKeyIdentifier, + callerName); + } - using (X509Certificate2 cert = req.CreateSelfSigned(now.AddMonths(-1), now.AddMonths(1))) - { - action(cert, now); - } - } + private static void BuildRsaCertificateAndRun( + IEnumerable extensions, + Action action, + bool addSubjectKeyIdentifier = true, + [CallerMemberName] string callerName = null) + { + BuildCertificateAndRun( + CertKind.RsaPkcs1, + extensions, + (certKind, cert, now) => action(cert, now), + addSubjectKeyIdentifier, + callerName); } private static void VerifyCrlFields( @@ -1579,6 +1639,78 @@ private static DateTimeOffset ReadX509Time(AsnReader reader) return reader.ReadGeneralizedTime(); } + private static X509SignatureGenerator GetSignatureGenerator( + CertKind certKind, + X509Certificate2 cert, + out IDisposable key) + { + if (certKind == CertKind.RsaPkcs1 || certKind == CertKind.RsaPss) + { + RSA rsa = cert.GetRSAPrivateKey(); + key = rsa; + return X509SignatureGenerator.CreateForRSA(rsa, GetRsaPadding(certKind)); + } + else if (certKind == CertKind.ECDsa) + { + ECDsa ecdsa = cert.GetECDsaPrivateKey(); + key = ecdsa; + return X509SignatureGenerator.CreateForECDsa(ecdsa); + } + else + { + throw new NotSupportedException($"Unsupported CertKind: {certKind}"); + } + } + + private static void VerifySignature( + CertKind certKind, + X509Certificate2 cert, + ReadOnlySpan data, + ReadOnlySpan signature, + HashAlgorithmName hashAlgorithm) + { + bool signatureValid; + + if (certKind == CertKind.RsaPkcs1 || certKind == CertKind.RsaPss) + { + using RSA rsa = cert.GetRSAPublicKey(); + signatureValid = rsa.VerifyData(data, signature, hashAlgorithm, GetRsaPadding(certKind)); + } + else if (certKind == CertKind.ECDsa) + { + using ECDsa ecdsa = cert.GetECDsaPublicKey(); + signatureValid = ecdsa.VerifyData(data, signature, hashAlgorithm, DSASignatureFormat.Rfc3279DerSequence); + } + else + { + throw new NotSupportedException($"Unsupported CertKind: {certKind}"); + } + + if (!signatureValid) + { + Assert.Fail($"{certKind} signature validation failed when it should have succeeded."); + } + } + + private static bool RequiresHashAlgorithm(CertKind certKind) + { + return certKind switch + { + CertKind.ECDsa or CertKind.RsaPkcs1 or CertKind.RsaPss => true, + _ => throw new NotSupportedException(certKind.ToString()) + }; + } + + private static RSASignaturePadding GetRsaPadding(CertKind certKind) + { + return certKind switch + { + CertKind.RsaPkcs1 => RSASignaturePadding.Pkcs1, + CertKind.RsaPss => RSASignaturePadding.Pss, + _ => null, + }; + } + private static ReadOnlySpan BuildEmptyExpectedCrl => new byte[] { 0x30, 0x82, 0x01, 0x8E, 0x30, 0x78, 0x02, 0x01, 0x01, 0x30, 0x0D, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/PrivateKeyAssociationTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/PrivateKeyAssociationTests.cs index 79dca40a0a8c6d..1f8ed857140234 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/PrivateKeyAssociationTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/PrivateKeyAssociationTests.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 Test.Cryptography; using Xunit; @@ -550,5 +551,230 @@ public static void ThirdPartyProvider_ECDsa() Assert.True(ecdsaOther.VerifyData(data, signature, hashAlgorithm)); } } + + [Fact] + public static void CheckCopyWithPrivateKey_RSA() + { + using (X509Certificate2 withKey = X509CertificateLoader.LoadPkcs12(TestData.PfxData, TestData.PfxDataPassword)) + using (X509Certificate2 pubOnly = X509CertificateLoader.LoadCertificate(withKey.RawDataMemory.Span)) + using (RSA privKey = withKey.GetRSAPrivateKey()) + using (X509Certificate2 wrongAlg = X509Certificate2.CreateFromPem(TestData.EcDhCertificate)) + { + CheckCopyWithPrivateKey( + pubOnly, + wrongAlg, + privKey, + [ + () => RSA.Create(2048), + () => RSA.Create(4096) + ], + RSACertificateExtensions.CopyWithPrivateKey, + RSACertificateExtensions.GetRSAPublicKey, + RSACertificateExtensions.GetRSAPrivateKey, + (priv, pub) => + { + byte[] data = new byte[RandomNumberGenerator.GetInt32(97)]; + RandomNumberGenerator.Fill(data); + + byte[] signature = priv.SignData(data, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + Assert.True(pub.VerifyData(data, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1)); + }); + } + } + + [Fact] + [SkipOnPlatform(PlatformSupport.MobileAppleCrypto, "DSA is not available")] + public static void CheckCopyWithPrivateKey_DSA() + { + using (X509Certificate2 withKey = X509CertificateLoader.LoadPkcs12(TestData.Dsa1024Pfx, TestData.Dsa1024PfxPassword)) + using (X509Certificate2 pubOnly = X509CertificateLoader.LoadCertificate(withKey.RawDataMemory.Span)) + using (DSA privKey = withKey.GetDSAPrivateKey()) + using (X509Certificate2 wrongAlg = X509Certificate2.CreateFromPem(TestData.EcDhCertificate)) + { + CheckCopyWithPrivateKey( + pubOnly, + wrongAlg, + privKey, + [ + () => + { + DSA dsa = DSA.Create(); + dsa.ImportParameters(TestData.GetDSA1024Params()); + return dsa; + }, + () => + { + DSA dsa = DSA.Create(); + + if (Dsa.Tests.DSASignVerify.SupportsFips186_3) + { + dsa.ImportParameters(Dsa.Tests.DSATestData.GetDSA2048Params()); + } + else + { + dsa.ImportParameters(Dsa.Tests.DSATestData.Dsa576Parameters); + } + + return dsa; + } + ], + DSACertificateExtensions.CopyWithPrivateKey, + DSACertificateExtensions.GetDSAPublicKey, + DSACertificateExtensions.GetDSAPrivateKey, + (priv, pub) => + { + byte[] data = new byte[RandomNumberGenerator.GetInt32(97)]; + RandomNumberGenerator.Fill(data); + + byte[] signature = priv.SignData(data, HashAlgorithmName.SHA1); + Assert.True(pub.VerifyData(data, signature, HashAlgorithmName.SHA1)); + }); + } + } + + [Fact] + public static void CheckCopyWithPrivateKey_ECDSA() + { + // A plain "ecPublicKey" cert can be either ECDSA or ECDH, but EcDhCertificate has a KeyUsage that + // says it is not suitable for being ECDSA. + // that stop them from being interchangeable, making them a much better test case than (e.g.) RSA + using (X509Certificate2 pubOnly = X509Certificate2.CreateFromPem(TestData.ECDsaCertificate)) + using (ECDsa privKey = ECDsa.Create()) + using (X509Certificate2 wrongAlg = X509Certificate2.CreateFromPem(TestData.EcDhCertificate)) + { + privKey.ImportFromPem(TestData.ECDsaECPrivateKey); + + CheckCopyWithPrivateKey( + pubOnly, + wrongAlg, + privKey, + [ + () => ECDsa.Create(ECCurve.NamedCurves.nistP256), + () => ECDsa.Create(ECCurve.NamedCurves.nistP384), + () => ECDsa.Create(ECCurve.NamedCurves.nistP521), + ], + ECDsaCertificateExtensions.CopyWithPrivateKey, + ECDsaCertificateExtensions.GetECDsaPublicKey, + ECDsaCertificateExtensions.GetECDsaPrivateKey, + (priv, pub) => + { + byte[] data = new byte[RandomNumberGenerator.GetInt32(97)]; + RandomNumberGenerator.Fill(data); + + byte[] signature = priv.SignData(data, HashAlgorithmName.SHA256); + Assert.True(pub.VerifyData(data, signature, HashAlgorithmName.SHA256)); + }); + } + } + + [Fact] + public static void CheckCopyWithPrivateKey_ECDH() + { + // The ECDH methods don't reject certs that lack the KeyAgreement KU, so test EC-DH vs RSA. + using (X509Certificate2 pubOnly = X509Certificate2.CreateFromPem(TestData.EcDhCertificate)) + using (ECDiffieHellman privKey = ECDiffieHellman.Create()) + using (X509Certificate2 wrongAlg = X509CertificateLoader.LoadCertificate(TestData.CertWithEnhancedKeyUsage)) + { + privKey.ImportFromPem(TestData.EcDhPkcs8Key); + + CheckCopyWithPrivateKey( + pubOnly, + wrongAlg, + privKey, + [ + () => ECDiffieHellman.Create(ECCurve.NamedCurves.nistP256), + () => ECDiffieHellman.Create(ECCurve.NamedCurves.nistP384), + () => ECDiffieHellman.Create(ECCurve.NamedCurves.nistP521), + ], + (cert, ecdh) => cert.CopyWithPrivateKey(ecdh), + cert => cert.GetECDiffieHellmanPublicKey(), + cert => cert.GetECDiffieHellmanPrivateKey(), + (priv, pub) => + { + ECParameters ecParams = pub.ExportParameters(false); + + using (ECDiffieHellman other = ECDiffieHellman.Create(ecParams.Curve)) + using (ECDiffieHellmanPublicKey otherPub = other.PublicKey) + using (ECDiffieHellmanPublicKey usPub = pub.PublicKey) + { + byte[] otherToUs = other.DeriveKeyFromHash(usPub, HashAlgorithmName.SHA256); + byte[] usToOther = priv.DeriveKeyFromHash(otherPub, HashAlgorithmName.SHA256); + + AssertExtensions.SequenceEqual(otherToUs, usToOther); + } + }); + } + } + + private static void CheckCopyWithPrivateKey( + X509Certificate2 cert, + X509Certificate2 wrongAlgorithmCert, + TKey correctPrivateKey, + IEnumerable> incorrectKeys, + Func copyWithPrivateKey, + Func getPublicKey, + Func getPrivateKey, + Action keyProver) + where TKey : class, IDisposable + { + AssertExtensions.ThrowsContains( + null, + () => copyWithPrivateKey(wrongAlgorithmCert, correctPrivateKey), + "algorithm"); + + List generatedKeys = new(); + + foreach (Func func in incorrectKeys) + { + TKey incorrectKey = func(); + generatedKeys.Add(incorrectKey); + + AssertExtensions.ThrowsContains( + "privateKey", + () => copyWithPrivateKey(cert, incorrectKey), + "key does not match the public key for this certificate"); + } + + using (X509Certificate2 withKey = copyWithPrivateKey(cert, correctPrivateKey)) + { + AssertExtensions.ThrowsContains( + () => copyWithPrivateKey(withKey, correctPrivateKey), + "already has an associated private key"); + + foreach (TKey incorrectKey in generatedKeys) + { + AssertExtensions.ThrowsContains( + () => copyWithPrivateKey(withKey, incorrectKey), + "already has an associated private key"); + } + + using (TKey pub = getPublicKey(withKey)) + using (TKey pub2 = getPublicKey(withKey)) + using (TKey pubOnly = getPublicKey(cert)) + using (TKey priv = getPrivateKey(withKey)) + using (TKey priv2 = getPrivateKey(withKey)) + { + Assert.NotSame(pub, pub2); + Assert.NotSame(pub, pubOnly); + Assert.NotSame(pub2, pubOnly); + Assert.NotSame(priv, priv2); + + keyProver(priv, pub2); + keyProver(priv2, pub); + keyProver(priv, pubOnly); + + priv.Dispose(); + pub2.Dispose(); + + keyProver(priv2, pub); + keyProver(priv2, pubOnly); + } + } + + foreach (TKey incorrectKey in generatedKeys) + { + incorrectKey.Dispose(); + } + } } } From 8be84910226dc81774a934039f08ea56a4de5659 Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Wed, 9 Apr 2025 14:40:47 -0700 Subject: [PATCH 2/3] Apply feedback --- .../Net/Configuration.Certificates.Dynamic.cs | 2 +- .../X509Certificates/CertificateAuthority.cs | 120 +++++++++--------- .../TestUtilities/System/AssertExtensions.cs | 6 - .../CertificateValidationRemoteServer.cs | 2 +- .../SslStreamCertificateContextTests.cs | 2 +- .../CertificateCreation/CrlBuilderTests.cs | 4 +- .../PrivateKeyAssociationTests.cs | 36 +++--- 7 files changed, 86 insertions(+), 86 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Configuration.Certificates.Dynamic.cs b/src/libraries/Common/tests/System/Net/Configuration.Certificates.Dynamic.cs index 829bcfbe42f402..fa20d1f374ce36 100644 --- a/src/libraries/Common/tests/System/Net/Configuration.Certificates.Dynamic.cs +++ b/src/libraries/Common/tests/System/Net/Configuration.Certificates.Dynamic.cs @@ -139,7 +139,7 @@ public static (X509Certificate2 certificate, X509Certificate2Collection) Generat intermediateAuthorityCount: longChain ? 3 : 1, subjectName: targetName, testName: testName, - keySize: keySize, + keyFactory: CertificateAuthority.KeyFactory.RSASize(keySize), extensions: extensions); // Walk the intermediates backwards so we build the chain collection as diff --git a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs index 516d2c5910b35c..737e3e2ddb3758 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs @@ -790,35 +790,35 @@ private enum CertStatus Revoked, } - [OverloadResolutionPriority(-1)] - internal static void BuildPrivatePki( - PkiOptions pkiOptions, - out RevocationResponder responder, - out CertificateAuthority rootAuthority, - out CertificateAuthority[] intermediateAuthorities, - out X509Certificate2 endEntityCert, - int intermediateAuthorityCount, - string testName = null, - bool registerAuthorities = true, - bool pkiOptionsInSubject = false, - string subjectName = null, - int keySize = DefaultKeySize, - X509ExtensionCollection extensions = null) - { - BuildPrivatePki( - pkiOptions, - out responder, - out rootAuthority, - out intermediateAuthorities, - out endEntityCert, - intermediateAuthorityCount, - testName, - registerAuthorities, - pkiOptionsInSubject, - subjectName, - KeyFactory.RSASize(keySize), - extensions); - } + //[OverloadResolutionPriority(-1)] + //internal static void BuildPrivatePki( + // PkiOptions pkiOptions, + // out RevocationResponder responder, + // out CertificateAuthority rootAuthority, + // out CertificateAuthority[] intermediateAuthorities, + // out X509Certificate2 endEntityCert, + // int intermediateAuthorityCount, + // string testName = null, + // bool registerAuthorities = true, + // bool pkiOptionsInSubject = false, + // string subjectName = null, + // int keySize = DefaultKeySize, + // X509ExtensionCollection extensions = null) + //{ + // BuildPrivatePki( + // pkiOptions, + // out responder, + // out rootAuthority, + // out intermediateAuthorities, + // out endEntityCert, + // intermediateAuthorityCount, + // testName, + // registerAuthorities, + // pkiOptionsInSubject, + // subjectName, + // KeyFactory.RSASize(keySize), + // extensions); + //} internal static void BuildPrivatePki( PkiOptions pkiOptions, @@ -851,7 +851,7 @@ internal static void BuildPrivatePki( if (keyFactory is null) { - // This could use any of the NC-hashes, but that complicates the code sharing for this file, + // This could use any of the non-cryptographic hashes, but that complicates the code sharing for this file, // so use IncrementalHash(SHA256) as it's inbox. // // System.HashCode isn't suitable because it's randomized, and we want the algorithm to @@ -963,36 +963,36 @@ internal static void BuildPrivatePki( } } - [OverloadResolutionPriority(-1)] - internal static void BuildPrivatePki( - PkiOptions pkiOptions, - out RevocationResponder responder, - out CertificateAuthority rootAuthority, - out CertificateAuthority intermediateAuthority, - out X509Certificate2 endEntityCert, - string testName = null, - bool registerAuthorities = true, - bool pkiOptionsInSubject = false, - string subjectName = null, - int keySize = DefaultKeySize, - X509ExtensionCollection extensions = null) - { - BuildPrivatePki( - pkiOptions, - out responder, - out rootAuthority, - out CertificateAuthority[] intermediateAuthorities, - out endEntityCert, - intermediateAuthorityCount: 1, - testName: testName, - registerAuthorities: registerAuthorities, - pkiOptionsInSubject: pkiOptionsInSubject, - subjectName: subjectName, - keySize: keySize, - extensions: extensions); - - intermediateAuthority = intermediateAuthorities.Single(); - } + //[OverloadResolutionPriority(-1)] + //internal static void BuildPrivatePki( + // PkiOptions pkiOptions, + // out RevocationResponder responder, + // out CertificateAuthority rootAuthority, + // out CertificateAuthority intermediateAuthority, + // out X509Certificate2 endEntityCert, + // string testName = null, + // bool registerAuthorities = true, + // bool pkiOptionsInSubject = false, + // string subjectName = null, + // int keySize = DefaultKeySize, + // X509ExtensionCollection extensions = null) + //{ + // BuildPrivatePki( + // pkiOptions, + // out responder, + // out rootAuthority, + // out CertificateAuthority[] intermediateAuthorities, + // out endEntityCert, + // intermediateAuthorityCount: 1, + // testName: testName, + // registerAuthorities: registerAuthorities, + // pkiOptionsInSubject: pkiOptionsInSubject, + // subjectName: subjectName, + // keySize: keySize, + // extensions: extensions); + + // intermediateAuthority = intermediateAuthorities.Single(); + //} internal static void BuildPrivatePki( PkiOptions pkiOptions, diff --git a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs index f5e4d72ca7fcb7..f8e8b3cc9d3626 100644 --- a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs +++ b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs @@ -45,12 +45,6 @@ public static void Throws(Action action, string expectedMessage) Assert.Equal(expectedMessage, Assert.Throws(action).Message); } - public static void ThrowsContains(string paramName, Action action, string expectedMessageContent) - where T : ArgumentException - { - Assert.Contains(expectedMessageContent, Assert.Throws(paramName, action).Message); - } - public static void ThrowsContains(Action action, string expectedMessageContent) where T : Exception { diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationRemoteServer.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationRemoteServer.cs index 589ae369c449e0..54152711daeb36 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationRemoteServer.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationRemoteServer.cs @@ -209,7 +209,7 @@ private async Task ConnectWithRevocation_WithCallback_Core( testName: testName, intermediateAuthorityCount: noIntermediates ? 0 : 1, subjectName: serverName, - keySize: 2048, + keyFactory: CertificateAuthority.KeyFactory.RSASize(2048), extensions: Configuration.Certificates.BuildTlsServerCertExtensions(serverName)); CertificateAuthority issuingAuthority = noIntermediates ? rootAuthority : intermediateAuthorities[0]; diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateContextTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateContextTests.cs index d2dceacce55651..f0ec352e083750 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateContextTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamCertificateContextTests.cs @@ -29,7 +29,7 @@ public static async Task Create_OcspDoesNotReturnOrCacheInvalidStapleData() testName: nameof(Create_OcspDoesNotReturnOrCacheInvalidStapleData), intermediateAuthorityCount: 1, subjectName: serverName, - keySize: 2048, + keyFactory: CertificateAuthority.KeyFactory.RSASize(2048), extensions: Configuration.Certificates.BuildTlsServerCertExtensions(serverName)); using (responder) diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CrlBuilderTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CrlBuilderTests.cs index 1acd06fca487ce..eea7fd280be081 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CrlBuilderTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/CrlBuilderTests.cs @@ -236,7 +236,9 @@ public static void BuildWithEmptyHashAlgorithm(CertKind certKind) if (RequiresHashAlgorithm(certKind)) { - AssertExtensions.ThrowsContains("hashAlgorithm", certAction, "empty"); + Exception e = AssertExtensions.Throws("hashAlgorithm", certAction); + + Assert.Contains("empty", e.Message); } else { diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/PrivateKeyAssociationTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/PrivateKeyAssociationTests.cs index 1f8ed857140234..4a8c97aed39910 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/PrivateKeyAssociationTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/PrivateKeyAssociationTests.cs @@ -548,7 +548,7 @@ public static void ThirdPartyProvider_ECDsa() Assert.InRange(pfxBytes.Length, 100, int.MaxValue); } - Assert.True(ecdsaOther.VerifyData(data, signature, hashAlgorithm)); + AssertExtensions.TrueExpression(ecdsaOther.VerifyData(data, signature, hashAlgorithm)); } } @@ -577,7 +577,7 @@ public static void CheckCopyWithPrivateKey_RSA() RandomNumberGenerator.Fill(data); byte[] signature = priv.SignData(data, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); - Assert.True(pub.VerifyData(data, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1)); + AssertExtensions.TrueExpression(pub.VerifyData(data, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1)); }); } } @@ -627,7 +627,7 @@ public static void CheckCopyWithPrivateKey_DSA() RandomNumberGenerator.Fill(data); byte[] signature = priv.SignData(data, HashAlgorithmName.SHA1); - Assert.True(pub.VerifyData(data, signature, HashAlgorithmName.SHA1)); + AssertExtensions.TrueExpression(pub.VerifyData(data, signature, HashAlgorithmName.SHA1)); }); } } @@ -662,7 +662,7 @@ public static void CheckCopyWithPrivateKey_ECDSA() RandomNumberGenerator.Fill(data); byte[] signature = priv.SignData(data, HashAlgorithmName.SHA256); - Assert.True(pub.VerifyData(data, signature, HashAlgorithmName.SHA256)); + AssertExtensions.TrueExpression(pub.VerifyData(data, signature, HashAlgorithmName.SHA256)); }); } } @@ -717,10 +717,11 @@ private static void CheckCopyWithPrivateKey( Action keyProver) where TKey : class, IDisposable { - AssertExtensions.ThrowsContains( + Exception e = AssertExtensions.Throws( null, - () => copyWithPrivateKey(wrongAlgorithmCert, correctPrivateKey), - "algorithm"); + () => copyWithPrivateKey(wrongAlgorithmCert, correctPrivateKey)); + + Assert.Contains("algorithm", e.Message); List generatedKeys = new(); @@ -729,23 +730,26 @@ private static void CheckCopyWithPrivateKey( TKey incorrectKey = func(); generatedKeys.Add(incorrectKey); - AssertExtensions.ThrowsContains( + e = AssertExtensions.Throws( "privateKey", - () => copyWithPrivateKey(cert, incorrectKey), - "key does not match the public key for this certificate"); + () => copyWithPrivateKey(cert, incorrectKey)); + + Assert.Contains("key does not match the public key for this certificate", e.Message); } using (X509Certificate2 withKey = copyWithPrivateKey(cert, correctPrivateKey)) { - AssertExtensions.ThrowsContains( - () => copyWithPrivateKey(withKey, correctPrivateKey), - "already has an associated private key"); + e = AssertExtensions.Throws( + () => copyWithPrivateKey(withKey, correctPrivateKey)); + + Assert.Contains("already has an associated private key", e.Message); foreach (TKey incorrectKey in generatedKeys) { - AssertExtensions.ThrowsContains( - () => copyWithPrivateKey(withKey, incorrectKey), - "already has an associated private key"); + e = AssertExtensions.Throws( + () => copyWithPrivateKey(withKey, incorrectKey)); + + Assert.Contains("already has an associated private key", e.Message); } using (TKey pub = getPublicKey(withKey)) From b2a42760b8dffbb110d05af65e451789cb7f0fb6 Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Wed, 9 Apr 2025 15:26:54 -0700 Subject: [PATCH 3/3] Remove commented-out code --- .../X509Certificates/CertificateAuthority.cs | 62 ------------------- 1 file changed, 62 deletions(-) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs index 737e3e2ddb3758..110bfc05144ddc 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs @@ -5,7 +5,6 @@ using System.Diagnostics; using System.Formats.Asn1; using System.Linq; -using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using Xunit; @@ -790,36 +789,6 @@ private enum CertStatus Revoked, } - //[OverloadResolutionPriority(-1)] - //internal static void BuildPrivatePki( - // PkiOptions pkiOptions, - // out RevocationResponder responder, - // out CertificateAuthority rootAuthority, - // out CertificateAuthority[] intermediateAuthorities, - // out X509Certificate2 endEntityCert, - // int intermediateAuthorityCount, - // string testName = null, - // bool registerAuthorities = true, - // bool pkiOptionsInSubject = false, - // string subjectName = null, - // int keySize = DefaultKeySize, - // X509ExtensionCollection extensions = null) - //{ - // BuildPrivatePki( - // pkiOptions, - // out responder, - // out rootAuthority, - // out intermediateAuthorities, - // out endEntityCert, - // intermediateAuthorityCount, - // testName, - // registerAuthorities, - // pkiOptionsInSubject, - // subjectName, - // KeyFactory.RSASize(keySize), - // extensions); - //} - internal static void BuildPrivatePki( PkiOptions pkiOptions, out RevocationResponder responder, @@ -963,37 +932,6 @@ internal static void BuildPrivatePki( } } - //[OverloadResolutionPriority(-1)] - //internal static void BuildPrivatePki( - // PkiOptions pkiOptions, - // out RevocationResponder responder, - // out CertificateAuthority rootAuthority, - // out CertificateAuthority intermediateAuthority, - // out X509Certificate2 endEntityCert, - // string testName = null, - // bool registerAuthorities = true, - // bool pkiOptionsInSubject = false, - // string subjectName = null, - // int keySize = DefaultKeySize, - // X509ExtensionCollection extensions = null) - //{ - // BuildPrivatePki( - // pkiOptions, - // out responder, - // out rootAuthority, - // out CertificateAuthority[] intermediateAuthorities, - // out endEntityCert, - // intermediateAuthorityCount: 1, - // testName: testName, - // registerAuthorities: registerAuthorities, - // pkiOptionsInSubject: pkiOptionsInSubject, - // subjectName: subjectName, - // keySize: keySize, - // extensions: extensions); - - // intermediateAuthority = intermediateAuthorities.Single(); - //} - internal static void BuildPrivatePki( PkiOptions pkiOptions, out RevocationResponder responder,