From e301ec16723437561c84f7b9a8c773def67b81fd Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 28 Apr 2020 10:19:00 -0400 Subject: [PATCH] Handle CriticalExtensions and WeakSignature on 509Chain for macOS (#35548) * 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. * 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 | 5 +- .../tests/ChainTests.cs | 64 +++++++++++++++++++ .../tests/DynamicChainTests.cs | 37 +++++++++++ 3 files changed, 105 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 9103292ff294a..7daa486ea7a5a 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,12 +176,15 @@ 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. } 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 751b35ee65dde..6ed015272b4a9 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs @@ -1202,6 +1202,70 @@ public static void BuildChainForCertificateSignedWithDisallowedKey() } } + [Fact] + public static void BuildChainForCertificateWithMD5Signature() + { + byte[] issuerCert = 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 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.ExtraStore.Add(issuer); + + // 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( diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs index 2fb3882674524..0c494a17038be 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() {