From e18dd2251209fd7c5a67e49527b7becabd55290c Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 26 Jun 2024 10:51:00 +0200 Subject: [PATCH 1/4] Check if certificate collections are not empty before changing trust mode to custom root trust --- .../Security/SslStreamCertificateContext.cs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs index 2bcc96b4d9a62e..64b44fb5674bc7 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs @@ -54,17 +54,15 @@ internal static SslStreamCertificateContext Create( chain.ChainPolicy.ExtraStore.AddRange(additionalCertificates); } - if (trust != null) + if (trust?._store?.Certificates.Count > 0) { chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; - if (trust._store != null) - { - chain.ChainPolicy.CustomTrustStore.AddRange(trust._store.Certificates); - } - if (trust._trustList != null) - { - chain.ChainPolicy.CustomTrustStore.AddRange(trust._trustList); - } + chain.ChainPolicy.CustomTrustStore.AddRange(trust._store.Certificates); + } + if (trust?._trustList?.Count > 0) + { + chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + chain.ChainPolicy.CustomTrustStore.AddRange(trust._trustList); } chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllFlags; @@ -77,7 +75,7 @@ internal static SslStreamCertificateContext Create( NetEventSource.Error(null, $"Failed to build chain for {target.Subject}"); } - if (!chainStatus && ChainBuildNeedsTrustedRoot && additionalCertificates != null) + if (!chainStatus && ChainBuildNeedsTrustedRoot && additionalCertificates?.Count > 0) { // Some platforms like Android may not be able to build the chain unless the chain root is trusted. // We can try to rebuild the chain with making all extra certificates trused. From 7b8a000fea6c9104ca7608e9a1cf71102264e728 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 26 Jun 2024 11:45:43 +0200 Subject: [PATCH 2/4] Enable SslStream_ClientCertificateContext_SendsChain test on Android --- .../tests/FunctionalTests/SslStreamNetworkStreamTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index d7b69f953f1dfe..1de06b18683e04 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -917,7 +917,6 @@ public async Task SslStream_ClientCertificate_SendsChain() [Theory] [InlineData(true)] [InlineData(false)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/68206", TestPlatforms.Android)] public async Task SslStream_ClientCertificateContext_SendsChain(bool useTrust) { (X509Certificate2 clientCertificate, X509Certificate2Collection clientChain) = Configuration.Certificates.GenerateCertificates(nameof(SslStream_ClientCertificateContext_SendsChain), serverCertificate: false); From ea7ad9aa9ff57a2ad9830aa74999897d3db77ac3 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 27 Jun 2024 09:33:34 +0200 Subject: [PATCH 3/4] Apply suggestions from reviews --- .../System/Net/Security/SslStreamCertificateContext.cs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs index 64b44fb5674bc7..b1ce3a28b2a0d6 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs @@ -54,15 +54,11 @@ internal static SslStreamCertificateContext Create( chain.ChainPolicy.ExtraStore.AddRange(additionalCertificates); } - if (trust?._store?.Certificates.Count > 0) + chain.ChainPolicy.CustomTrustStore.AddRange(trust?._store?.Certificates ?? []); + chain.ChainPolicy.CustomTrustStore.AddRange(trust?._trustList ?? []); + if (chain.ChainPolicy.CustomTrustStore.Count > 0) { chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; - chain.ChainPolicy.CustomTrustStore.AddRange(trust._store.Certificates); - } - if (trust?._trustList?.Count > 0) - { - chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; - chain.ChainPolicy.CustomTrustStore.AddRange(trust._trustList); } chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllFlags; From 6d9d1bb9449653d3140218abd260fa907aa5c2ad Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 27 Jun 2024 10:49:51 +0200 Subject: [PATCH 4/4] Avoid unnecessary allocations --- .../Security/SslStreamCertificateContext.cs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs index b1ce3a28b2a0d6..b3540d646a1d25 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs @@ -54,11 +54,22 @@ internal static SslStreamCertificateContext Create( chain.ChainPolicy.ExtraStore.AddRange(additionalCertificates); } - chain.ChainPolicy.CustomTrustStore.AddRange(trust?._store?.Certificates ?? []); - chain.ChainPolicy.CustomTrustStore.AddRange(trust?._trustList ?? []); - if (chain.ChainPolicy.CustomTrustStore.Count > 0) + if (trust != null) { - chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + if (trust._store != null) + { + chain.ChainPolicy.CustomTrustStore.AddRange(trust._store.Certificates); + } + + if (trust._trustList != null) + { + chain.ChainPolicy.CustomTrustStore.AddRange(trust._trustList); + } + + if (chain.ChainPolicy.CustomTrustStore.Count > 0) + { + chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + } } chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllFlags;