diff --git a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs index 230e4cb4276a42..f2a9fa6333ca66 100644 --- a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs +++ b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs @@ -397,6 +397,20 @@ public static void GreaterThanOrEqualTo(T actual, T greaterThanOrEqualTo, str throw new XunitException(AddOptionalUserMessage($"Expected: {actual} to be greater than or equal to {greaterThanOrEqualTo}", userMessage)); } + /// + /// Validate that a given enum value has the expected flag set. + /// + /// The enum type. + /// The flag which should be present in . + /// The value which should contain the flag . + public static void HasFlag(T expected, T actual, string userMessage = null) where T : Enum + { + if (!actual.HasFlag(expected)) + { + throw new XunitException(AddOptionalUserMessage($"Expected: Value {actual} (of enum type {typeof(T).FullName}) to have the flag {expected} set.", userMessage)); + } + } + // NOTE: Consider using SequenceEqual below instead, as it will give more useful information about what // the actual differences are, especially for large arrays/spans. /// diff --git a/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.netcoreapp.cs b/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.netcoreapp.cs index e5ef61d996a88b..2e1911ba11d3ea 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.netcoreapp.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.netcoreapp.cs @@ -399,7 +399,10 @@ public static void AddSigner_RSA_EphemeralKey() { ContentInfo content = new ContentInfo(new byte[] { 1, 2, 3 }); SignedCms cms = new SignedCms(content, false); - CmsSigner signer = new CmsSigner(certWithEphemeralKey); + CmsSigner signer = new CmsSigner(certWithEphemeralKey) + { + IncludeOption = X509IncludeOption.EndCertOnly + }; cms.ComputeSignature(signer); } } @@ -429,7 +432,8 @@ public static void AddSigner_DSA_EphemeralKey() SignedCms cms = new SignedCms(content, false); CmsSigner signer = new CmsSigner(certWithEphemeralKey) { - DigestAlgorithm = new Oid(Oids.Sha1, Oids.Sha1) + DigestAlgorithm = new Oid(Oids.Sha1, Oids.Sha1), + IncludeOption = X509IncludeOption.EndCertOnly }; cms.ComputeSignature(signer); } @@ -458,7 +462,10 @@ public static void AddSigner_ECDSA_EphemeralKey() { ContentInfo content = new ContentInfo(new byte[] { 1, 2, 3 }); SignedCms cms = new SignedCms(content, false); - CmsSigner signer = new CmsSigner(certWithEphemeralKey); + CmsSigner signer = new CmsSigner(certWithEphemeralKey) + { + IncludeOption = X509IncludeOption.EndCertOnly + }; cms.ComputeSignature(signer); } } diff --git a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx index e447937b7b6c39..1bd529a087a6ac 100644 --- a/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography/src/Resources/Strings.resx @@ -834,4 +834,7 @@ There was a problem with the PKCS12 (PFX) without a supplied password. See https://go.microsoft.com/fwlink/?linkid=2233907 for more information. + + An unknown chain building error occurred. + diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/LocalAppContextSwitches.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/LocalAppContextSwitches.cs index 0af05ab604b677..be8c3d44bf8fb4 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/LocalAppContextSwitches.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/LocalAppContextSwitches.cs @@ -11,6 +11,8 @@ internal static partial class LocalAppContextSwitches internal static long Pkcs12UnspecifiedPasswordIterationLimit { get; } = InitializePkcs12UnspecifiedPasswordIterationLimit(); + internal static bool X509ChainBuildThrowOnInternalError { get; } = InitializeX509ChainBuildThrowOnInternalError(); + private static long InitializePkcs12UnspecifiedPasswordIterationLimit() { object? data = AppContext.GetData("System.Security.Cryptography.Pkcs12UnspecifiedPasswordIterationLimit"); @@ -29,5 +31,12 @@ private static long InitializePkcs12UnspecifiedPasswordIterationLimit() return DefaultPkcs12UnspecifiedPasswordIterationLimit; } } + + private static bool InitializeX509ChainBuildThrowOnInternalError() + { + // n.b. the switch defaults to TRUE if it has not been explicitly set. + return AppContext.TryGetSwitch("System.Security.Cryptography.ThrowOnX509ChainBuildInternalError", out bool isEnabled) + ? isEnabled : true; + } } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Chain.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Chain.cs index 50eaba6eb91b95..fdc0b02b2d6e37 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Chain.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Chain.cs @@ -137,26 +137,62 @@ internal bool Build(X509Certificate2 certificate, bool throwOnException) chainPolicy.UrlRetrievalTimeout, chainPolicy.DisableCertificateDownloads); - if (_pal == null) - return false; - - _chainElements = new X509ChainElementCollection(_pal.ChainElements!); - - Exception? verificationException; - bool? verified = _pal.Verify(chainPolicy.VerificationFlags, out verificationException); - if (!verified.HasValue) + bool success = false; + if (_pal is not null) { - if (throwOnException) - { - throw verificationException!; - } - else + _chainElements = new X509ChainElementCollection(_pal.ChainElements!); + + Exception? verificationException; + bool? verified = _pal.Verify(chainPolicy.VerificationFlags, out verificationException); + if (!verified.HasValue) { - verified = false; + if (throwOnException) + { + throw verificationException!; + } + else + { + verified = false; + } } + + success = verified.Value; + } + + // There are two reasons success might be false here. + // + // The most common reason is that we built the chain but the chain appears to run + // afoul of policy. This is represented by BuildChain returning a non-null object + // and storing potential policy violations in the chain structure. The public Build + // method returns false to the caller, and the caller can inspect the ChainStatus + // and ChainElements properties and evaluate the failure reason against app-level + // policies. If the caller does not care about these policy violations, they can + // choose to ignore them and to treat chain building as successful. + // + // The other type of failure is that BuildChain simply can't build the chain at all. + // Perhaps something within the certificate is not valid or is unsupported, or perhaps + // there's an internal failure within the OS layer we're invoking, etc. Whatever the + // reason, we're not meaningfully able to initialize the ChainStatus property, which + // means callers may observe a non-empty list of policy violations. Depending on the + // caller's logic, they might incorrectly interpret this as there being no policy + // violations at all, which means they might treat this condition as success. + // + // To avoid callers misinterpeting this latter condition as success, we'll throw an + // exception, which matches general .NET API behavior when a method cannot complete + // its objective. A compat switch is provided to normalize this back to a 'false' + // return value for callers who cannot handle an exception here. If throwOnException + // is false, it means the caller explicitly wants to suppress exceptions and normalize + // them to a false return value. + + if (!success + && throwOnException + && _pal?.ChainStatus is not { Length: > 0 } + && LocalAppContextSwitches.X509ChainBuildThrowOnInternalError) + { + throw new CryptographicException(SR.Cryptography_X509_ChainBuildingFailed); } - return verified.Value; + return success; } } diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs index 23bef2b46c250a..92cc1abb1b6292 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs @@ -1269,6 +1269,58 @@ public static void BuildChainForSelfSignedSha3Certificate() } } + [Fact] + public static void BuildChainForSelfSignedCertificate_WithSha256RsaSignature() + { + using (ChainHolder chainHolder = new ChainHolder()) + using (X509Certificate2 cert = new X509Certificate2(TestData.SelfSignedCertSha256RsaBytes)) + { + X509Chain chain = chainHolder.Chain; + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); + + // No custom root of trust store means that this self-signed cert will at + // minimum be marked UntrustedRoot. + + Assert.False(chain.Build(cert)); + AssertExtensions.HasFlag(X509ChainStatusFlags.UntrustedRoot, chain.AllStatusFlags()); + } + } + + [Fact] + public static void BuildChainForSelfSignedCertificate_WithUnknownOidSignature() + { + using (ChainHolder chainHolder = new ChainHolder()) + using (X509Certificate2 cert = new X509Certificate2(TestData.SelfSignedCertDummyOidBytes)) + { + X509Chain chain = chainHolder.Chain; + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + chain.ChainPolicy.VerificationTime = cert.NotBefore.AddHours(2); + + // This tests a self-signed cert whose signature block contains a garbage signing alg OID. + // Some platforms return NotSignatureValid to indicate that they cannot understand the + // signature block. Other platforms return PartialChain to indicate that they think the + // bad signature block might correspond to some unknown, untrusted signer. Yet other + // platforms simply fail the operation; e.g., Windows's CertGetCertificateChain API returns + // NTE_BAD_ALGID, which we bubble up as CryptographicException. + + if (PlatformDetection.UsesAppleCrypto) + { + Assert.False(chain.Build(cert)); + AssertExtensions.HasFlag(X509ChainStatusFlags.PartialChain, chain.AllStatusFlags()); + } + else if (PlatformDetection.IsOpenSslSupported) + { + Assert.False(chain.Build(cert)); + AssertExtensions.HasFlag(X509ChainStatusFlags.NotSignatureValid, chain.AllStatusFlags()); + } + else + { + Assert.ThrowsAny(() => chain.Build(cert)); + } + } + } + internal static X509ChainStatusFlags AllStatusFlags(this X509Chain chain) { return chain.ChainStatus.Aggregate( diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/TestData.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/TestData.cs index e3deb00b354d06..cf14136eb6e058 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/TestData.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/TestData.cs @@ -4224,5 +4224,54 @@ internal static DSAParameters GetDSA1024Params() "09463C6E50BCA36EB3F8BCB00D8A415D2D0DB5AE08303B301F300706052B0E03" + "021A0414A57105D833610A6D07EBFBE51E5486CD3F8BCE0D0414DB32290CC077" + "37E9D9446E37F104FA876C861C0102022710").HexToByteArray(); + + // Used for chain building tests (CN=Test chain building) + internal static readonly byte[] SelfSignedCertSha256RsaBytes = ( + "308202BD308201A5A003020102020900EF79C10DFD657168300D06092A864886F70D0101" + + "0B0500301E311C301A060355040313135465737420636861696E206275696C64696E6730" + + "1E170D3231313031333231353735335A170D3232313031333231353735335A301E311C30" + + "1A060355040313135465737420636861696E206275696C64696E6730820122300D06092A" + + "864886F70D01010105000382010F003082010A0282010100E3B5BBF862313DEAA9172788" + + "278B26A3EAB61B9B0326F5CEA91B1A6C6DFD156836A2363BFAC5B0F4A78F4CFF5A11F35A" + + "831C6D7935D1DFD13DD81DA29AA0645CBA9F4D20BF991C625E6D61CF396C15914DEE41F6" + + "1190E97B52BFF7AE52B79FD0E2EEE3319EC23C30D27A52A2E8A963557B12BEC0664ADEF9" + + "3C520B587EC5DABFBC70980DB7473414B4B6BF982EA9AA0969F2A76AA085464AE78DFB2B" + + "F04BDE7192874679193119C2AABEC04D360F61925921660BF09A0489B7C53464F5FC35B8" + + "612F5B993D544475C20AC46CD350A34551FEA0ACBD138D8B72F79052BF0EB3BD794A426C" + + "0117CB77B4F311FFD1C628F8E438E5474509AD51FA035558771546310203010001300D06" + + "092A864886F70D01010B050003820101000A12CE2FC3DC854113D179725E9D9ADD013A42" + + "D66340CEA7A465D54EC357AD8FED1828862D8B5C32EB3D21FC8B26A7CFA9D9FB36F593CC" + + "6AD30C25C96E8100C3F07B1B51430245EE995864749C53B409260B4040705654710C236F" + + "D9B7DE3F3BE5E6E5047712C5E506419106A57C5290BB206A97F6A3FCC4B4C83E25C3FC6D" + + "2BAB03B941374086265EE08A90A8C72A63A4053044B9FA3ABD5ED5785CFDDB15A6A327BD" + + "C0CC2B115B9D33BD6E528E35670E5A6A8D9CF52199F8D693315C60D9ADAD54EF7FDCED36" + + "0C8C79E84D42AB5CB6355A70951B1ABF1F2B3FB8BEB7E3A8D6BA2293C0DB8C86B0BB060F" + + "0D6DB9939E88B998662A27F092634BBF21F58EEAAA").HexToByteArray(); + + // This is nearly identical to the cert in Pkcs7SelfSignedCertSha256RsaBytes, + // but we've replaced the OID (1.2.840.113549.1.1.11 sha256RSA) with a dummy OID + // 1.3.9999.1234.5678.1234. The cert should load properly into an X509Certificate2 + // object but will cause chain building to fail. + internal static readonly byte[] SelfSignedCertDummyOidBytes = ( + "308202BD308201A5A003020102020900EF79C10DFD657168300D06092A864886F70D0101" + + "0B0500301E311C301A060355040313135465737420636861696E206275696C64696E6730" + + "1E170D3231313031333231353735335A170D3232313031333231353735335A301E311C30" + + "1A060355040313135465737420636861696E206275696C64696E6730820122300D06092A" + + "864886F70D01010105000382010F003082010A0282010100E3B5BBF862313DEAA9172788" + + "278B26A3EAB61B9B0326F5CEA91B1A6C6DFD156836A2363BFAC5B0F4A78F4CFF5A11F35A" + + "831C6D7935D1DFD13DD81DA29AA0645CBA9F4D20BF991C625E6D61CF396C15914DEE41F6" + + "1190E97B52BFF7AE52B79FD0E2EEE3319EC23C30D27A52A2E8A963557B12BEC0664ADEF9" + + "3C520B587EC5DABFBC70980DB7473414B4B6BF982EA9AA0969F2A76AA085464AE78DFB2B" + + "F04BDE7192874679193119C2AABEC04D360F61925921660BF09A0489B7C53464F5FC35B8" + + "612F5B993D544475C20AC46CD350A34551FEA0ACBD138D8B72F79052BF0EB3BD794A426C" + + "0117CB77B4F311FFD1C628F8E438E5474509AD51FA035558771546310203010001300D06" + + "092BCE0F8952AC2E8952050003820101000A12CE2FC3DC854113D179725E9D9ADD013A42" + + "D66340CEA7A465D54EC357AD8FED1828862D8B5C32EB3D21FC8B26A7CFA9D9FB36F593CC" + + "6AD30C25C96E8100C3F07B1B51430245EE995864749C53B409260B4040705654710C236F" + + "D9B7DE3F3BE5E6E5047712C5E506419106A57C5290BB206A97F6A3FCC4B4C83E25C3FC6D" + + "2BAB03B941374086265EE08A90A8C72A63A4053044B9FA3ABD5ED5785CFDDB15A6A327BD" + + "C0CC2B115B9D33BD6E528E35670E5A6A8D9CF52199F8D693315C60D9ADAD54EF7FDCED36" + + "0C8C79E84D42AB5CB6355A70951B1ABF1F2B3FB8BEB7E3A8D6BA2293C0DB8C86B0BB060F" + + "0D6DB9939E88B998662A27F092634BBF21F58EEAAA").HexToByteArray(); } }