diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs index 32d0ac6501d34..c90b43d52435b 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs @@ -1,7 +1,9 @@ // 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 System.Formats.Asn1; +using System.IO; using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -832,6 +834,91 @@ public static void PolicyConstraints_Mapped() } } + public enum BuildChainWithNotSignatureValidTest : int + { + TrustedRoot, + UntrustedRoot, + PartialChain, + TrustedRootEndCertNotSignatureValid, + PartialChainEndCertNotSignatureValid + } + + public static IEnumerable BuildChainWithNotSignatureValidData() + { + yield return new object[] { true, X509ChainStatusFlags.NoError, BuildChainWithNotSignatureValidTest.TrustedRoot }; + yield return new object[] { false, X509ChainStatusFlags.UntrustedRoot | X509ChainStatusFlags.NotSignatureValid, BuildChainWithNotSignatureValidTest.UntrustedRoot }; + yield return new object[] { false, X509ChainStatusFlags.PartialChain, BuildChainWithNotSignatureValidTest.PartialChain }; + yield return new object[] { false, X509ChainStatusFlags.NotSignatureValid, BuildChainWithNotSignatureValidTest.TrustedRootEndCertNotSignatureValid }; + yield return new object[] { false, X509ChainStatusFlags.PartialChain | X509ChainStatusFlags.NotSignatureValid, BuildChainWithNotSignatureValidTest.PartialChainEndCertNotSignatureValid }; + } + + [Theory] + [MemberData(nameof(BuildChainWithNotSignatureValidData))] + [PlatformSpecific(TestPlatforms.Linux)] // NotSignatureValid gets ignored on the root certificate. + public static void BuildChainWithNotSignatureValid( + bool chainBuildsSuccessfully, + X509ChainStatusFlags chainFlags, + BuildChainWithNotSignatureValidTest test) + { + TestDataGenerator.MakeTestChain3( + out X509Certificate2 endCert, + out X509Certificate2 intermediateCert, + out X509Certificate2 rootCert, + testName: test.ToString()); + + X509Certificate2 tampered; + switch (test) + { + case BuildChainWithNotSignatureValidTest.TrustedRootEndCertNotSignatureValid: + case BuildChainWithNotSignatureValidTest.PartialChainEndCertNotSignatureValid: + tampered = TamperSignature(endCert); + endCert.Dispose(); + endCert = tampered; + break; + default: + // Make root cert signature invalid. + tampered = TamperSignature(rootCert); + rootCert.Dispose(); + rootCert = tampered; + break; + } + + using (endCert) + using (intermediateCert) + using (rootCert) + using (ChainHolder chainHolder = new ChainHolder()) + { + X509Chain chainTest = chainHolder.Chain; + chainTest.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + chainTest.ChainPolicy.VerificationTime = endCert.NotBefore.AddSeconds(1); + chainTest.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + + switch (test) + { + case BuildChainWithNotSignatureValidTest.TrustedRoot: + case BuildChainWithNotSignatureValidTest.TrustedRootEndCertNotSignatureValid: + chainTest.ChainPolicy.ExtraStore.Add(intermediateCert); + chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); + break; + case BuildChainWithNotSignatureValidTest.PartialChain: + chainTest.ChainPolicy.CustomTrustStore.Add(rootCert); + break; + case BuildChainWithNotSignatureValidTest.PartialChainEndCertNotSignatureValid: + chainTest.ChainPolicy.ExtraStore.Add(intermediateCert); + break; + case BuildChainWithNotSignatureValidTest.UntrustedRoot: + chainTest.ChainPolicy.ExtraStore.Add(intermediateCert); + chainTest.ChainPolicy.ExtraStore.Add(rootCert); + break; + default: + throw new InvalidDataException(); + } + + Assert.Equal(chainBuildsSuccessfully, chainTest.Build(endCert)); + Assert.Equal(chainFlags, chainTest.AllStatusFlags()); + } + } + private static X509ChainStatusFlags PlatformBasicConstraints(X509ChainStatusFlags flags) { if (OperatingSystem.IsAndroid()) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs index 906f1edb8eb84..8d61518b29d01 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs @@ -904,7 +904,7 @@ private X509ChainElement[] BuildChainElements( overallStatus = null; List? statusBuilder = null; - + bool overallHasNotSignatureValid = false; using (SafeX509StackHandle chainStack = Interop.Crypto.X509StoreCtxGetChain(_storeCtx)) { int chainSize = Interop.Crypto.GetX509StackFieldCount(chainStack); @@ -921,7 +921,20 @@ private X509ChainElement[] BuildChainElements( statusBuilder ??= new List(); overallStatus ??= new List(); - AddElementStatus(elementErrors.Value, statusBuilder, overallStatus); + bool hadSignatureNotValid = overallHasNotSignatureValid; + AddElementStatus(elementErrors.Value, statusBuilder, overallStatus, ref overallHasNotSignatureValid); + + // Clear NotSignatureValid for the last element when overall chain is not PartialChain or UntrustedRoot. + bool isLastElement = i == chainSize - 1; + if (isLastElement && !hadSignatureNotValid && overallHasNotSignatureValid) + { + if (!ContainsStatus(overallStatus, X509ChainStatusFlags.PartialChain) && + !ContainsStatus(overallStatus, X509ChainStatusFlags.UntrustedRoot)) + { + RemoveStatus(statusBuilder, X509ChainStatusFlags.NotSignatureValid); + RemoveStatus(overallStatus, X509ChainStatusFlags.NotSignatureValid); + } + } status = statusBuilder.ToArray(); statusBuilder.Clear(); } @@ -1013,11 +1026,12 @@ private static void ProcessPolicy( private static void AddElementStatus( ErrorCollection errorCodes, List elementStatus, - List overallStatus) + List overallStatus, + ref bool overallHasNotSignatureValid) { - foreach (var errorCode in errorCodes) + foreach (Interop.Crypto.X509VerifyStatusCode errorCode in errorCodes) { - AddElementStatus(errorCode, elementStatus, overallStatus); + AddElementStatus(errorCode, elementStatus, overallStatus, ref overallHasNotSignatureValid); } foreach (X509ChainStatus element in elementStatus) @@ -1042,7 +1056,8 @@ private static void AddElementStatus( private static void AddElementStatus( Interop.Crypto.X509VerifyStatusCode errorCode, List elementStatus, - List overallStatus) + List overallStatus, + ref bool overallHasNotSignatureValid) { X509ChainStatusFlags statusFlag = MapVerifyErrorToChainStatus(errorCode); @@ -1069,6 +1084,11 @@ private static void AddElementStatus( elementStatus.Add(chainStatus); AddUniqueStatus(overallStatus, ref chainStatus); + + if (statusFlag == X509ChainStatusFlags.NotSignatureValid) + { + overallHasNotSignatureValid = true; + } } private static void AddUniqueStatus(List list, ref X509ChainStatus status) @@ -1086,6 +1106,31 @@ private static void AddUniqueStatus(List list, ref X509ChainSta list.Add(status); } + private static bool ContainsStatus(List list, X509ChainStatusFlags statusCode) + { + for (int i = 0; i < list.Count; i++) + { + if (list[i].Status == statusCode) + { + return true; + } + } + + return false; + } + + private static void RemoveStatus(List list, X509ChainStatusFlags statusCode) + { + for (int i = 0; i < list.Count; i++) + { + if (list[i].Status == statusCode) + { + list.RemoveAt(i); + return; + } + } + } + private static X509ChainStatusFlags MapVerifyErrorToChainStatus(Interop.Crypto.X509VerifyStatusCode code) { switch (code.UniversalCode)