From 74098ff7b9596da1e63853a6dd46f4163efcc9b7 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Mon, 27 Apr 2020 20:06:18 -0400 Subject: [PATCH 1/3] Support unknown critical extensions on macOS. If a certificate contains an unprocessable critical extension in a certificate, map the "CriticalExtensions" status to HasNotSupportedCriticalExtension instead of throwing an exception. --- .../pal_x509chain.c | 2 + .../tests/DynamicChainTests.cs | 37 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c index 9103292ff294af..41b12468a31043 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c @@ -176,6 +176,8 @@ static void MergeStatusCodes(CFTypeRef key, CFTypeRef value, void* context) *pStatus |= PAL_X509ChainRevocationStatusUnknown; else if (CFEqual(keyString, CFSTR("MissingIntermediate"))) *pStatus |= PAL_X509ChainPartialChain; + else if (CFEqual(keyString, CFSTR("CriticalExtensions"))) + *pStatus |= PAL_X509ChainHasNotSupportedCriticalExtension; else if (CFEqual(keyString, CFSTR("UnparseableExtension"))) { // 10.15 introduced new status code value which is not reported by Windows. Ignoring for now. diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs index 2fb3882674524b..0c494a17038be0 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Linq; using System.Runtime.InteropServices; using Test.Cryptography; using Xunit; @@ -281,6 +282,42 @@ public static void BasicConstraints_ViolatesCaFalse() } } + [Fact] + public static void TestLeafCertificateWithUnknownCriticalExtension() + { + using (RSA key = RSA.Create()) + { + CertificateRequest certReq = new CertificateRequest( + new X500DistinguishedName("CN=Cert"), + key, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + const string PrecertificatePoisonExtensionOid = "1.3.6.1.4.1.11129.2.4.3"; + certReq.CertificateExtensions.Add(new X509Extension( + new AsnEncodedData( + new Oid(PrecertificatePoisonExtensionOid), + new byte[] { 5, 0 }), + critical: true)); + + DateTimeOffset notBefore = DateTimeOffset.UtcNow.AddDays(-1); + DateTimeOffset notAfter = notBefore.AddDays(30); + + using (X509Certificate2 cert = certReq.CreateSelfSigned(notBefore, notAfter)) + using (ChainHolder holder = new ChainHolder()) + { + X509Chain chain = holder.Chain; + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + Assert.False(chain.Build(cert)); + + X509ChainElement certElement = chain.ChainElements.OfType().Single(); + const X509ChainStatusFlags ExpectedFlag = X509ChainStatusFlags.HasNotSupportedCriticalExtension; + X509ChainStatusFlags actualFlags = certElement.AllStatusFlags(); + Assert.True((actualFlags & ExpectedFlag) == ExpectedFlag, $"Has expected flag {ExpectedFlag} but was {actualFlags}"); + } + } + } + [Fact] public static void TestInvalidAia() { From 5b81ee5a79dbdc90b28bd451977ab1dd6217a94e Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Mon, 27 Apr 2020 20:54:11 -0400 Subject: [PATCH 2/3] Ignore WeakSignature chain status on macOS. X509Chain on Windows will not check for modern signatures, so we will let macOS do the same thing. --- .../pal_x509chain.c | 3 +- .../tests/ChainTests.cs | 65 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c index 41b12468a31043..7daa486ea7a5a8 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c @@ -183,7 +183,8 @@ static void MergeStatusCodes(CFTypeRef key, CFTypeRef value, void* context) // 10.15 introduced new status code value which is not reported by Windows. Ignoring for now. } else if (CFEqual(keyString, CFSTR("WeakLeaf")) || CFEqual(keyString, CFSTR("WeakIntermediates")) || - CFEqual(keyString, CFSTR("WeakRoot")) || CFEqual(keyString, CFSTR("WeakKeySize"))) + CFEqual(keyString, CFSTR("WeakRoot")) || CFEqual(keyString, CFSTR("WeakKeySize")) || + CFEqual(keyString, CFSTR("WeakSignature"))) { // Because we won't report this out of a chain built by .NET on Windows, // don't report it here. diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index 751b35ee65dded..2fd2d9324fbb35 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -1202,6 +1202,71 @@ public static void BuildChainForCertificateSignedWithDisallowedKey() } } + [Fact] + public static void BuildChainForCertificateWithMD5Signature() + { + byte[] rootCert = Convert.FromBase64String(@" +MIIDgzCCAmsCFGTFpNWP/ick4s4VCF1MafVWpWr+MA0GCSqGSIb3DQEBCwUAMH0x +CzAJBgNVBAYTAlVTMR0wGwYDVQQIDBREaXN0cmljdCBvZiBDb2x1bWJpYTETMBEG +A1UEBwwKV2FzaGluZ3RvbjEQMA4GA1UECgwHVGVzdCBDQTEUMBIGA1UECwwLRGV2 +ZWxvcG1lbnQxEjAQBgNVBAMMCVRlc3QgUm9vdDAgFw0yMDA0MjgwMDQwNDZaGA8y +MTIwMDQwNDAwNDA0NlowfTELMAkGA1UEBhMCVVMxHTAbBgNVBAgMFERpc3RyaWN0 +IG9mIENvbHVtYmlhMRMwEQYDVQQHDApXYXNoaW5ndG9uMRAwDgYDVQQKDAdUZXN0 +IENBMRQwEgYDVQQLDAtEZXZlbG9wbWVudDESMBAGA1UEAwwJVGVzdCBSb290MIIB +IjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsUBPPdECxj8DWbmjkhtnjxjd +LZluHyRpb0+favXLuXHeFtTy/92wuHUSHTr45TGKDxI0qevMLaNqEiy9yBkjNPTz +ctjZTHwbOhxuGEz3Mv2n3IJ7XoIPcn2ZQbhEcTDI/FeF06B+OQKNLigYMHR+L/qd +KlNBpaUaGG0FNpZ0zGJl1n+CizECWOh3PYaVLKmIS9RjEmNmOMqP737u8W6d3sRZ +vb6etsNKRmwRjpWUdk4/LzjSJSiNbIQv5c/cGSv6sXFKDixXxIugwreQ/F/JwJ3/ +x2xTIJt0nHSKbK8zVKIGkSmZ3+bdeve889Mjwu0kN7EW+labAuf8VwzQ0c9qUQID +AQABMA0GCSqGSIb3DQEBCwUAA4IBAQAjGr0pZOxiCa+52S94WvR0WQVNMje3ZL+m +f4/FyaaDUCqrNv8Tt4m3vYtr8bkT+0uC4rcYx5/9iwLzI6oK1+JddoprAQ+17ZPw +Cg8ISgn8PuBzvaOQJxpc1nvWvvQpOiYxpsZPWABdE4xl3YAdcuu43x1mtFphn7Aw +KFTcvxF03RVZPSuZ0k6l1WBRNZFJFoTo2XlhUiLXN4vjxIEDXTCyi/kOzlYu98kZ +pzDlSoMBAu6CHSBygS51IaimM48qtdQjxZIYVZhFL9QaBa2zQ+qsEF0gz+mG0an9 +0BMCvSA9GZA0VBrQJjQLQLjv0rpZkw0i9FypOicu2Zv9d5UF+IXZ"); + + byte[] md5SignedLeafCert = Convert.FromBase64String(@" +MIIEezCCA2OgAwIBAgIUf1ubwalzwcn4DQ2X5hUbYPqateowDQYJKoZIhvcNAQEE +BQAwfTELMAkGA1UEBhMCVVMxHTAbBgNVBAgMFERpc3RyaWN0IG9mIENvbHVtYmlh +MRMwEQYDVQQHDApXYXNoaW5ndG9uMRAwDgYDVQQKDAdUZXN0IENBMRQwEgYDVQQL +DAtEZXZlbG9wbWVudDESMBAGA1UEAwwJVGVzdCBSb290MCAXDTIwMDQyODAwNDI1 +OFoYDzIxMTYwMjI1MDA0MjU4WjBoMQswCQYDVQQGEwJVUzEdMBsGA1UECAwURGlz +dHJpY3Qgb2YgQ29sdW1iaWExEDAOBgNVBAoMB1Rlc3QgQ0ExFDASBgNVBAsMC0Rl +dmVsb3BtZW50MRIwEAYDVQQDDAlUZXN0IExlYWYwggEiMA0GCSqGSIb3DQEBAQUA +A4IBDwAwggEKAoIBAQDGHz60IeCSpN1qQbdLHO2VSlQbOn2fBV5qGK/82+a4xiZf +xO0wZ6p9Tb7/rOnF7P7YlaOrY9zc6O5vPxatcv2FcZxwrR8zDnslWUg39WzFnz4M +8eiiGBpxlbUfcUq8FvqfGQ6MxMAwA0kgUjegaVXN1Zgq+J+HcLSJm8EADNOD46nS +TkTVXvEMCBmrl17LyYEnxLogUgWve9QMNz0+XpJq90MlygPmxuvUnWduDGnVgrJq +blkwFqaLIh94vmc8rQJ9WSy+1FRknDoDcy3KveYW3ii9uD9B7YvXdmFVEjGXPcUv +9aFoRiqq/E8oYUhWJXTr9omyA3iJawcn0Kkl3IT7AgMBAAGjggEEMIIBADAJBgNV +HRMEAjAAMCwGCWCGSAGG+EIBDQQfFh1PcGVuU1NMIEdlbmVyYXRlZCBDZXJ0aWZp +Y2F0ZTAdBgNVHQ4EFgQUE9CCY2wzRzuH3lvEENkvBxDRFx8wgaUGA1UdIwSBnTCB +mqGBgaR/MH0xCzAJBgNVBAYTAlVTMR0wGwYDVQQIDBREaXN0cmljdCBvZiBDb2x1 +bWJpYTETMBEGA1UEBwwKV2FzaGluZ3RvbjEQMA4GA1UECgwHVGVzdCBDQTEUMBIG +A1UECwwLRGV2ZWxvcG1lbnQxEjAQBgNVBAMMCVRlc3QgUm9vdIIUZMWk1Y/+JyTi +zhUIXUxp9Valav4wDQYJKoZIhvcNAQEEBQADggEBAGbrB50Gf9FQ1lTbtKQBlrpF +M01/mHvqDioqjP6hcvDRMvxWcnX8kIq7Idb2uv1fByBPQdBTH2yzGc1adCXtBqrb +ueIjvYVDoXZMqRa7vZjaMA+8szK9lgm2dzSfa3xFKCIT7Twfq6FKGJ7o4TRbopmr +3MsjTMLjfGUnKdxtcYb/FGxB4NRdIyCaaRgtYOIFkOGgA3UTEAJuOAqwY8RdQywR +lHBlkA0wrbydD3FzxYHUJgx0HGO6CcyAzXJLhZVbuBW4expq4Qhi0jDV4d8Otskv +LjCvFGJ+RiZCbxIZfUZEuJ5vAH5WOa2S0tYoEAeyfzuLMIqY9xK74nlZ/vzz1cY="); + + using (X509Certificate2 root = new X509Certificate2(rootCert)) + using (X509Certificate2 cert = new X509Certificate2(md5SignedLeafCert)) + using (ChainHolder chainHolder = new ChainHolder()) + { + X509Chain chain = chainHolder.Chain; + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); + chain.ChainPolicy.CustomTrustStore.Add(root); + chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + + // Should not throw, don't care about the validity of the chain. + chain.Build(cert); + } + } + internal static X509ChainStatusFlags AllStatusFlags(this X509Chain chain) { return chain.ChainStatus.Aggregate( From a684df300a20d17f64f693ecb6f204b969ec42a4 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Mon, 27 Apr 2020 22:43:27 -0400 Subject: [PATCH 3/3] Remove use of CustomTrustStore. --- .../tests/ChainTests.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs index 2fd2d9324fbb35..6ed015272b4a9a 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -1205,7 +1205,7 @@ public static void BuildChainForCertificateSignedWithDisallowedKey() [Fact] public static void BuildChainForCertificateWithMD5Signature() { - byte[] rootCert = Convert.FromBase64String(@" + byte[] issuerCert = Convert.FromBase64String(@" MIIDgzCCAmsCFGTFpNWP/ick4s4VCF1MafVWpWr+MA0GCSqGSIb3DQEBCwUAMH0x CzAJBgNVBAYTAlVTMR0wGwYDVQQIDBREaXN0cmljdCBvZiBDb2x1bWJpYTETMBEG A1UEBwwKV2FzaGluZ3RvbjEQMA4GA1UECgwHVGVzdCBDQTEUMBIGA1UECwwLRGV2 @@ -1252,15 +1252,14 @@ public static void BuildChainForCertificateWithMD5Signature() lHBlkA0wrbydD3FzxYHUJgx0HGO6CcyAzXJLhZVbuBW4expq4Qhi0jDV4d8Otskv LjCvFGJ+RiZCbxIZfUZEuJ5vAH5WOa2S0tYoEAeyfzuLMIqY9xK74nlZ/vzz1cY="); - using (X509Certificate2 root = new X509Certificate2(rootCert)) + using (X509Certificate2 issuer = new X509Certificate2(issuerCert)) using (X509Certificate2 cert = new X509Certificate2(md5SignedLeafCert)) using (ChainHolder chainHolder = new ChainHolder()) { X509Chain chain = chainHolder.Chain; chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); - chain.ChainPolicy.CustomTrustStore.Add(root); - chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + chain.ChainPolicy.ExtraStore.Add(issuer); // Should not throw, don't care about the validity of the chain. chain.Build(cert);