use also SslCertificateTrust when constructing CertificateContext (do…

…tnet#103372) * use also SslCertificateTrust when constructing CertificateContext * 'build * feedback
simonrozsival · Jul 8, 2024 · 9f1b0c4 · 9f1b0c4
1 parent 787a48b
commit 9f1b0c4
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 5 deletions.
diff --git a/...raries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Android.cs b/...raries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Android.cs
@@ -9,6 +9,7 @@ namespace System.Net.Security
     public partial class SslStreamCertificateContext
     {
         private const bool TrimRootCertificate = true;
+        private const bool ChainBuildNeedsTrustedRoot = true;
 
         private SslStreamCertificateContext(X509Certificate2 target, ReadOnlyCollection<X509Certificate2> intermediates, SslCertificateTrust? trust)
         {

diff --git a/...ibraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs b/...ibraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs
@@ -18,6 +18,7 @@ namespace System.Net.Security
     public partial class SslStreamCertificateContext
     {
         private const bool TrimRootCertificate = true;
+        private const bool ChainBuildNeedsTrustedRoot = false;
         internal readonly ConcurrentDictionary<SslProtocols, SafeSslContextHandle> SslContexts;
         internal readonly SafeX509Handle CertificateHandle;
         internal readonly SafeEvpPKeyHandle KeyHandle;

diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.OSX.cs
@@ -10,6 +10,7 @@ public partial class SslStreamCertificateContext
     {
         // No leaf, no root.
         private const bool TrimRootCertificate = true;
+        private const bool ChainBuildNeedsTrustedRoot = false;
 
         private SslStreamCertificateContext(X509Certificate2 target, ReadOnlyCollection<X509Certificate2> intermediates, SslCertificateTrust? trust)
         {

diff --git a/...raries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Windows.cs b/...raries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Windows.cs
@@ -10,6 +10,7 @@ public partial class SslStreamCertificateContext
     {
         // No leaf, include root.
         private const bool TrimRootCertificate = false;
+        private const bool ChainBuildNeedsTrustedRoot = false;
 
         internal static SslStreamCertificateContext Create(X509Certificate2 target)
         {

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
@@ -51,9 +51,19 @@ internal static SslStreamCertificateContext Create(
             {
                 if (additionalCertificates != null)
                 {
-                    foreach (X509Certificate cert in additionalCertificates)
+                    chain.ChainPolicy.ExtraStore.AddRange(additionalCertificates);
+                }
+
+                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.ExtraStore.Add(cert);
+                        chain.ChainPolicy.CustomTrustStore.AddRange(trust._trustList);
                     }
                 }
 
@@ -67,6 +77,20 @@ internal static SslStreamCertificateContext Create(
                     NetEventSource.Error(null, $"Failed to build chain for {target.Subject}");
                 }
 
+                if (!chainStatus && ChainBuildNeedsTrustedRoot && additionalCertificates != null)
+                {
+                    // 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.
+                    // We do not try to evaluate trust here, we jsut need to construct the chain so it should not matter.
+                    chain.ChainPolicy.CustomTrustStore.AddRange(additionalCertificates);
+                    chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
+                    chainStatus = chain.Build(target);
+                    if (!chainStatus && NetEventSource.Log.IsEnabled())
+                    {
+                        NetEventSource.Error(null, $"Failed to build chain for {target.Subject} while trusting additional certificates");
+                    }
+                }
+
                 int count = chain.ChainElements.Count - 1;
 
                 // Some platforms (e.g. Android) can't ignore all verification and will return zero

diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs
@@ -914,19 +914,29 @@ public async Task SslStream_ClientCertificate_SendsChain()
             }
         }
 
-        [Fact]
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
         [ActiveIssue("https://github.com/dotnet/runtime/issues/68206", TestPlatforms.Android)]
-        public async Task SslStream_ClientCertificateContext_SendsChain()
+        public async Task SslStream_ClientCertificateContext_SendsChain(bool useTrust)
         {
             (X509Certificate2 clientCertificate, X509Certificate2Collection clientChain) = TestHelper.GenerateCertificates(nameof(SslStream_ClientCertificateContext_SendsChain), serverCertificate: false);
             TestHelper.CleanupCertificates(nameof(SslStream_ClientCertificateContext_SendsChain));
 
+            SslCertificateTrust? trust = null;
+            if (useTrust)
+            {
+                // This is simplification. We make all the intermediates trusted,
+                // normally just the root would go here.
+                trust = SslCertificateTrust.CreateForX509Collection(clientChain, false);
+            }
+
             var clientOptions = new SslClientAuthenticationOptions()
             {
                 TargetHost = "localhost",
             };
             clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true;
-            clientOptions.ClientCertificateContext = SslStreamCertificateContext.Create(clientCertificate, clientChain);
+            clientOptions.ClientCertificateContext = SslStreamCertificateContext.Create(clientCertificate, useTrust ? null : clientChain, offline:true, trust);
 
             await SslStream_ClientSendsChain_Core(clientOptions, clientChain);
 

diff --git a/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj b/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj
@@ -74,6 +74,8 @@
              Link="ProductionCode\System\Net\Security\ReadWriteAdapter.cs" />
     <Compile Include="..\..\src\System\Net\SslStreamContext.cs"
              Link="ProductionCode\System\Net\SslStreamContext.cs" />
+    <Compile Include="..\..\src\System\Net\Security\SslCertificateTrust.cs"
+             Link="ProductionCode\System\Net\Security\SslCertificateTrust.cs" />
     <Compile Include="$(CommonPath)System\Net\SecurityProtocol.cs"
              Link="ProductionCode\Common\System\Net\SecurityProtocol.cs" />
     <Compile Include="..\..\src\System\Net\Security\TlsAlertType.cs"