From 24257fb9efa8ce2eea9ea900bcc6fd50f150c016 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 6 Jul 2020 15:15:42 -0700 Subject: [PATCH 1/5] fix failing tests --- .../X509Certificates/CertificateAuthority.cs | 39 ++++++++++++++----- .../tests/FunctionalTests/TestHelper.cs | 5 ++- 2 files changed, 32 insertions(+), 12 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 1cb4e8a290f1df..a94e403636bb64 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs @@ -25,6 +25,7 @@ public enum PkiOptions IssuerRevocationViaOcsp = 1 << 1, EndEntityRevocationViaCrl = 1 << 2, EndEntityRevocationViaOcsp = 1 << 3, + EndEntityIsServer = 1 << 4, CrlEverywhere = IssuerRevocationViaCrl | EndEntityRevocationViaCrl, OcspEverywhere = IssuerRevocationViaOcsp | EndEntityRevocationViaOcsp, @@ -106,6 +107,11 @@ internal sealed class CertificateAuthority : IDisposable internal DateTimeOffset? RevocationExpiration { get; set; } internal bool CorruptRevocationIssuerName { get; set; } + // All keys created in this method are smaller than recommended, + // but they only live for a few seconds (at most), + // and never communicate out of process. + const int DefaultKeySize = 1024; + internal CertificateAuthority( X509Certificate2 cert, string aiaHttpUrl, @@ -169,6 +175,18 @@ internal X509Certificate2 CreateSubordinateCA( } internal X509Certificate2 CreateEndEntity(string subject, RSA publicKey, X509Extension altName) + { + return CreateCertificate( + subject, + publicKey, + TimeSpan.FromSeconds(2), + s_eeConstraints, + s_eeKeyUsage, + s_tlsClientEku, + altName: altName); + } + + internal X509Certificate2 CreateServerEndEntity(string subject, RSA publicKey, X509Extension altName) { return CreateCertificate( subject, @@ -841,7 +859,8 @@ internal static void BuildPrivatePki( string testName = null, bool registerAuthorities = true, bool pkiOptionsInSubject = false, - string subjectName = null) + string subjectName = null, + int KeySize = DefaultKeySize) { bool rootDistributionViaHttp = !pkiOptions.HasFlag(PkiOptions.NoRootCertDistributionUri); bool issuerRevocationViaCrl = pkiOptions.HasFlag(PkiOptions.IssuerRevocationViaCrl); @@ -849,17 +868,13 @@ internal static void BuildPrivatePki( bool issuerDistributionViaHttp = !pkiOptions.HasFlag(PkiOptions.NoIssuerCertDistributionUri); bool endEntityRevocationViaCrl = pkiOptions.HasFlag(PkiOptions.EndEntityRevocationViaCrl); bool endEntityRevocationViaOcsp = pkiOptions.HasFlag(PkiOptions.EndEntityRevocationViaOcsp); + bool endEntityIsServer = pkiOptions.HasFlag(PkiOptions.EndEntityIsServer); Assert.True( issuerRevocationViaCrl || issuerRevocationViaOcsp || endEntityRevocationViaCrl || endEntityRevocationViaOcsp, "At least one revocation mode is enabled"); - // All keys created in this method are smaller than recommended, - // but they only live for a few seconds (at most), - // and never communicate out of process. - const int KeySize = 1024; - using (RSA rootKey = RSA.Create(KeySize)) using (RSA intermedKey = RSA.Create(KeySize)) using (RSA eeKey = RSA.Create(KeySize)) @@ -929,10 +944,14 @@ internal static void BuildPrivatePki( altName = builder.Build(); } - endEntityCert = intermediateAuthority.CreateEndEntity( - BuildSubject(subjectName ?? "A Revocation Test Cert", testName, pkiOptions, pkiOptionsInSubject), - eeKey, - altName); + endEntityCert = endEntityIsServer ? + intermediateAuthority.CreateServerEndEntity( + BuildSubject(subjectName ?? "A Revocation Test Cert", testName, pkiOptions, pkiOptionsInSubject), + eeKey, altName) : + intermediateAuthority.CreateEndEntity( + BuildSubject(subjectName ?? "A Revocation Test Cert", testName, pkiOptions, pkiOptionsInSubject), + eeKey, altName); + endEntityCert = endEntityCert.CopyWithPrivateKey(eeKey); } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs index 5d95d0826ec622..53288b7b5f9a76 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs @@ -55,13 +55,14 @@ internal static (X509Certificate2 certificate, X509Certificate2Collection) Gener X509Certificate2Collection chain = new X509Certificate2Collection(); CertificateAuthority.BuildPrivatePki( - PkiOptions.IssuerRevocationViaCrl, + PkiOptions.IssuerRevocationViaCrl | PkiOptions.EndEntityIsServer, out RevocationResponder responder, out CertificateAuthority root, out CertificateAuthority intermediate, out X509Certificate2 endEntity, subjectName: name, - testName: testName); + testName: testName, + KeySize: 2048); chain.Add(intermediate.CloneIssuerCert()); chain.Add(root.CloneIssuerCert()); From ea256cf53cb0980868b41926fda64ef58db121d4 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 6 Jul 2020 22:04:12 -0700 Subject: [PATCH 2/5] fix casing --- .../Cryptography/X509Certificates/CertificateAuthority.cs | 8 ++++---- .../tests/FunctionalTests/TestHelper.cs | 2 +- 2 files changed, 5 insertions(+), 5 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 a94e403636bb64..1ff0d0e4f57130 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs @@ -860,7 +860,7 @@ internal static void BuildPrivatePki( bool registerAuthorities = true, bool pkiOptionsInSubject = false, string subjectName = null, - int KeySize = DefaultKeySize) + int keySize = DefaultKeySize) { bool rootDistributionViaHttp = !pkiOptions.HasFlag(PkiOptions.NoRootCertDistributionUri); bool issuerRevocationViaCrl = pkiOptions.HasFlag(PkiOptions.IssuerRevocationViaCrl); @@ -875,9 +875,9 @@ internal static void BuildPrivatePki( endEntityRevocationViaCrl || endEntityRevocationViaOcsp, "At least one revocation mode is enabled"); - using (RSA rootKey = RSA.Create(KeySize)) - using (RSA intermedKey = RSA.Create(KeySize)) - using (RSA eeKey = RSA.Create(KeySize)) + using (RSA rootKey = RSA.Create(keySize)) + using (RSA intermedKey = RSA.Create(keySize)) + using (RSA eeKey = RSA.Create(keySize)) { var rootReq = new CertificateRequest( BuildSubject("A Revocation Test Root", testName, pkiOptions, pkiOptionsInSubject), diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs index 53288b7b5f9a76..9595d5a8021802 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs @@ -62,7 +62,7 @@ internal static (X509Certificate2 certificate, X509Certificate2Collection) Gener out X509Certificate2 endEntity, subjectName: name, testName: testName, - KeySize: 2048); + keySize: 2048); chain.Add(intermediate.CloneIssuerCert()); chain.Add(root.CloneIssuerCert()); From 52cec0ce0d6585a791eeb06f61aae67bccb1cc1a Mon Sep 17 00:00:00 2001 From: wfurt Date: Tue, 7 Jul 2020 12:51:45 -0700 Subject: [PATCH 3/5] feedback from review --- .../X509Certificates/CertificateAuthority.cs | 94 +++++-------------- .../tests/FunctionalTests/TestHelper.cs | 28 +++++- 2 files changed, 53 insertions(+), 69 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 1ff0d0e4f57130..44da79213edb72 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs @@ -70,14 +70,6 @@ internal sealed class CertificateAuthority : IDisposable }, critical: false); - private static readonly X509EnhancedKeyUsageExtension s_tlsServerEku = - new X509EnhancedKeyUsageExtension( - new OidCollection - { - new Oid("1.3.6.1.5.5.7.3.1", null) - }, - false); - private static readonly X509EnhancedKeyUsageExtension s_tlsClientEku = new X509EnhancedKeyUsageExtension( new OidCollection @@ -165,37 +157,22 @@ internal X509Certificate2 CreateSubordinateCA( subject, publicKey, TimeSpan.FromMinutes(1), - new X509BasicConstraintsExtension( - certificateAuthority: true, - depthLimit.HasValue, - depthLimit.GetValueOrDefault(), - critical: true), - s_caKeyUsage, - ekuExtension: null); + new X509ExtensionCollection() { + new X509BasicConstraintsExtension( + certificateAuthority: true, + depthLimit.HasValue, + depthLimit.GetValueOrDefault(), + critical: true), + s_caKeyUsage }); } - internal X509Certificate2 CreateEndEntity(string subject, RSA publicKey, X509Extension altName) + internal X509Certificate2 CreateEndEntity(string subject, RSA publicKey, X509ExtensionCollection extensions) { return CreateCertificate( subject, publicKey, TimeSpan.FromSeconds(2), - s_eeConstraints, - s_eeKeyUsage, - s_tlsClientEku, - altName: altName); - } - - internal X509Certificate2 CreateServerEndEntity(string subject, RSA publicKey, X509Extension altName) - { - return CreateCertificate( - subject, - publicKey, - TimeSpan.FromSeconds(2), - s_eeConstraints, - s_eeKeyUsage, - s_tlsServerEku, - altName: altName); + extensions); } internal X509Certificate2 CreateOcspSigner(string subject, RSA publicKey) @@ -204,9 +181,7 @@ internal X509Certificate2 CreateOcspSigner(string subject, RSA publicKey) subject, publicKey, TimeSpan.FromSeconds(1), - s_eeConstraints, - s_eeKeyUsage, - s_ocspResponderEku, + new X509ExtensionCollection() { s_eeConstraints, s_eeKeyUsage, s_ocspResponderEku}, ocspResponder: true); } @@ -266,11 +241,8 @@ private X509Certificate2 CreateCertificate( string subject, RSA publicKey, TimeSpan nestingBuffer, - X509BasicConstraintsExtension basicConstraints, - X509KeyUsageExtension keyUsage, - X509EnhancedKeyUsageExtension ekuExtension, - bool ocspResponder = false, - X509Extension altName = null) + X509ExtensionCollection extensions, + bool ocspResponder = false) { if (_cdpExtension == null && CdpUri != null) { @@ -293,8 +265,10 @@ private X509Certificate2 CreateCertificate( HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); - request.CertificateExtensions.Add(basicConstraints); - request.CertificateExtensions.Add(keyUsage); + foreach (X509Extension extension in extensions) + { + request.CertificateExtensions.Add(extension); + } // Windows does not accept OCSP Responder certificates which have // a CDP extension, or an AIA extension with an OCSP endpoint. @@ -308,16 +282,6 @@ private X509Certificate2 CreateCertificate( request.CertificateExtensions.Add( new X509SubjectKeyIdentifierExtension(request.PublicKey, false)); - if (ekuExtension != null) - { - request.CertificateExtensions.Add(ekuExtension); - } - - if (altName != null) - { - request.CertificateExtensions.Add(altName); - } - byte[] serial = new byte[sizeof(long)]; RandomNumberGenerator.Fill(serial); @@ -860,7 +824,8 @@ internal static void BuildPrivatePki( bool registerAuthorities = true, bool pkiOptionsInSubject = false, string subjectName = null, - int keySize = DefaultKeySize) + int keySize = DefaultKeySize, + X509ExtensionCollection extensions = null) { bool rootDistributionViaHttp = !pkiOptions.HasFlag(PkiOptions.NoRootCertDistributionUri); bool issuerRevocationViaCrl = pkiOptions.HasFlag(PkiOptions.IssuerRevocationViaCrl); @@ -875,6 +840,12 @@ internal static void BuildPrivatePki( endEntityRevocationViaCrl || endEntityRevocationViaOcsp, "At least one revocation mode is enabled"); + if (extensions == null) + { + // default to client + extensions = new X509ExtensionCollection() { s_eeConstraints, s_eeKeyUsage, s_tlsClientEku }; + } + using (RSA rootKey = RSA.Create(keySize)) using (RSA intermedKey = RSA.Create(keySize)) using (RSA eeKey = RSA.Create(keySize)) @@ -935,22 +906,9 @@ internal static void BuildPrivatePki( endEntityRevocationViaCrl ? cdpUrl : null, endEntityRevocationViaOcsp ? ocspUrl : null); - X509Extension altName = null; - - if (!String.IsNullOrEmpty(subjectName)) - { - SubjectAlternativeNameBuilder builder = new SubjectAlternativeNameBuilder(); - builder.AddDnsName(subjectName); - altName = builder.Build(); - } - - endEntityCert = endEntityIsServer ? - intermediateAuthority.CreateServerEndEntity( - BuildSubject(subjectName ?? "A Revocation Test Cert", testName, pkiOptions, pkiOptionsInSubject), - eeKey, altName) : - intermediateAuthority.CreateEndEntity( + endEntityCert = intermediateAuthority.CreateEndEntity( BuildSubject(subjectName ?? "A Revocation Test Cert", testName, pkiOptions, pkiOptionsInSubject), - eeKey, altName); + eeKey, extensions); endEntityCert = endEntityCert.CopyWithPrivateKey(eeKey); } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs index 9595d5a8021802..19cd534cd8a5c4 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs @@ -14,6 +14,22 @@ namespace System.Net.Security.Tests { public static class TestHelper { + private static readonly X509KeyUsageExtension s_eeKeyUsage = + new X509KeyUsageExtension( + X509KeyUsageFlags.DigitalSignature | X509KeyUsageFlags.KeyEncipherment | X509KeyUsageFlags.DataEncipherment, + critical: false); + + private static readonly X509EnhancedKeyUsageExtension s_tlsServerEku = + new X509EnhancedKeyUsageExtension( + new OidCollection + { + new Oid("1.3.6.1.5.5.7.3.1", null) + }, + false); + + private static readonly X509BasicConstraintsExtension s_eeConstraints = + new X509BasicConstraintsExtension(false, false, 0, false); + public static (Stream ClientStream, Stream ServerStream) GetConnectedStreams() { if (Capability.SecurityForceSocketStreams()) @@ -54,6 +70,15 @@ internal static (X509Certificate2 certificate, X509Certificate2Collection) Gener { X509Certificate2Collection chain = new X509Certificate2Collection(); + X509ExtensionCollection extensions = new X509ExtensionCollection(); + + SubjectAlternativeNameBuilder builder = new SubjectAlternativeNameBuilder(); + builder.AddDnsName(name); + extensions.Add(builder.Build()); + extensions.Add(s_eeConstraints); + extensions.Add(s_eeKeyUsage); + extensions.Add(s_tlsServerEku); + CertificateAuthority.BuildPrivatePki( PkiOptions.IssuerRevocationViaCrl | PkiOptions.EndEntityIsServer, out RevocationResponder responder, @@ -62,7 +87,8 @@ internal static (X509Certificate2 certificate, X509Certificate2Collection) Gener out X509Certificate2 endEntity, subjectName: name, testName: testName, - keySize: 2048); + keySize: 2048, + extensions: extensions); chain.Add(intermediate.CloneIssuerCert()); chain.Add(root.CloneIssuerCert()); From 1b0fa73b45ccd8a9b9c2e4ebe3bdd8cfc4ccbfa7 Mon Sep 17 00:00:00 2001 From: wfurt Date: Tue, 7 Jul 2020 12:56:35 -0700 Subject: [PATCH 4/5] style update --- .../Cryptography/X509Certificates/CertificateAuthority.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 44da79213edb72..9f2d78c6460ddb 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs @@ -908,7 +908,8 @@ internal static void BuildPrivatePki( endEntityCert = intermediateAuthority.CreateEndEntity( BuildSubject(subjectName ?? "A Revocation Test Cert", testName, pkiOptions, pkiOptionsInSubject), - eeKey, extensions); + eeKey, + extensions); endEntityCert = endEntityCert.CopyWithPrivateKey(eeKey); } From ba4eca913ce9a6ad9e235ebf11beb62835fdfb94 Mon Sep 17 00:00:00 2001 From: wfurt Date: Tue, 7 Jul 2020 13:00:54 -0700 Subject: [PATCH 5/5] retire EndEntityIsServer --- .../Cryptography/X509Certificates/CertificateAuthority.cs | 2 -- .../System.Net.Security/tests/FunctionalTests/TestHelper.cs | 2 +- 2 files changed, 1 insertion(+), 3 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 9f2d78c6460ddb..45f7a784fdd4fd 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs @@ -25,7 +25,6 @@ public enum PkiOptions IssuerRevocationViaOcsp = 1 << 1, EndEntityRevocationViaCrl = 1 << 2, EndEntityRevocationViaOcsp = 1 << 3, - EndEntityIsServer = 1 << 4, CrlEverywhere = IssuerRevocationViaCrl | EndEntityRevocationViaCrl, OcspEverywhere = IssuerRevocationViaOcsp | EndEntityRevocationViaOcsp, @@ -833,7 +832,6 @@ internal static void BuildPrivatePki( bool issuerDistributionViaHttp = !pkiOptions.HasFlag(PkiOptions.NoIssuerCertDistributionUri); bool endEntityRevocationViaCrl = pkiOptions.HasFlag(PkiOptions.EndEntityRevocationViaCrl); bool endEntityRevocationViaOcsp = pkiOptions.HasFlag(PkiOptions.EndEntityRevocationViaOcsp); - bool endEntityIsServer = pkiOptions.HasFlag(PkiOptions.EndEntityIsServer); Assert.True( issuerRevocationViaCrl || issuerRevocationViaOcsp || diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs index 19cd534cd8a5c4..2aac8a37a05422 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs @@ -80,7 +80,7 @@ internal static (X509Certificate2 certificate, X509Certificate2Collection) Gener extensions.Add(s_tlsServerEku); CertificateAuthority.BuildPrivatePki( - PkiOptions.IssuerRevocationViaCrl | PkiOptions.EndEntityIsServer, + PkiOptions.IssuerRevocationViaCrl, out RevocationResponder responder, out CertificateAuthority root, out CertificateAuthority intermediate,