Skip to content

Commit 973237d

Browse files
authored
use also SslCertificateTrust when constructing CertificateContext (#103372)
* use also SslCertificateTrust when constructing CertificateContext * 'build * feedback
1 parent 4e1ad9c commit 973237d

7 files changed

+46
-8
lines changed

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Android.cs

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ namespace System.Net.Security
99
public partial class SslStreamCertificateContext
1010
{
1111
private const bool TrimRootCertificate = true;
12+
private const bool ChainBuildNeedsTrustedRoot = true;
1213

1314
private SslStreamCertificateContext(X509Certificate2 target, ReadOnlyCollection<X509Certificate2> intermediates, SslCertificateTrust? trust)
1415
{

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public partial class SslStreamCertificateContext
2424
internal static TimeSpan RefreshAfterFailureBackOffInterval => TimeSpan.FromSeconds(5);
2525

2626
private const bool TrimRootCertificate = true;
27+
private const bool ChainBuildNeedsTrustedRoot = false;
2728
internal ConcurrentDictionary<SslProtocols, SafeSslContextHandle> SslContexts
2829
{
2930
get

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.OSX.cs

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ public partial class SslStreamCertificateContext
1010
{
1111
// No leaf, no root.
1212
private const bool TrimRootCertificate = true;
13+
private const bool ChainBuildNeedsTrustedRoot = false;
1314

1415
private SslStreamCertificateContext(X509Certificate2 target, ReadOnlyCollection<X509Certificate2> intermediates, SslCertificateTrust? trust)
1516
{

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Windows.cs

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ public partial class SslStreamCertificateContext
1010
{
1111
// No leaf, include root.
1212
private const bool TrimRootCertificate = false;
13+
private const bool ChainBuildNeedsTrustedRoot = false;
1314

1415
internal static SslStreamCertificateContext Create(X509Certificate2 target)
1516
{

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs

+26-2
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,19 @@ internal static SslStreamCertificateContext Create(
5151
{
5252
if (additionalCertificates != null)
5353
{
54-
foreach (X509Certificate cert in additionalCertificates)
54+
chain.ChainPolicy.ExtraStore.AddRange(additionalCertificates);
55+
}
56+
57+
if (trust != null)
58+
{
59+
chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
60+
if (trust._store != null)
61+
{
62+
chain.ChainPolicy.CustomTrustStore.AddRange(trust._store.Certificates);
63+
}
64+
if (trust._trustList != null)
5565
{
56-
chain.ChainPolicy.ExtraStore.Add(cert);
66+
chain.ChainPolicy.CustomTrustStore.AddRange(trust._trustList);
5767
}
5868
}
5969

@@ -67,6 +77,20 @@ internal static SslStreamCertificateContext Create(
6777
NetEventSource.Error(null, $"Failed to build chain for {target.Subject}");
6878
}
6979

80+
if (!chainStatus && ChainBuildNeedsTrustedRoot && additionalCertificates != null)
81+
{
82+
// Some platforms like Android may not be able to build the chain unless the chain root is trusted.
83+
// We can try to rebuild the chain with making all extra certificates trused.
84+
// We do not try to evaluate trust here, we jsut need to construct the chain so it should not matter.
85+
chain.ChainPolicy.CustomTrustStore.AddRange(additionalCertificates);
86+
chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
87+
chainStatus = chain.Build(target);
88+
if (!chainStatus && NetEventSource.Log.IsEnabled())
89+
{
90+
NetEventSource.Error(null, $"Failed to build chain for {target.Subject} while trusting additional certificates");
91+
}
92+
}
93+
7094
int count = chain.ChainElements.Count - 1;
7195

7296
// Some platforms (e.g. Android) can't ignore all verification and will return zero

src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs

+13-3
Original file line numberDiff line numberDiff line change
@@ -914,19 +914,29 @@ public async Task SslStream_ClientCertificate_SendsChain()
914914
}
915915
}
916916

917-
[Fact]
917+
[Theory]
918+
[InlineData(true)]
919+
[InlineData(false)]
918920
[ActiveIssue("https://github.com/dotnet/runtime/issues/68206", TestPlatforms.Android)]
919-
public async Task SslStream_ClientCertificateContext_SendsChain()
921+
public async Task SslStream_ClientCertificateContext_SendsChain(bool useTrust)
920922
{
921923
(X509Certificate2 clientCertificate, X509Certificate2Collection clientChain) = Configuration.Certificates.GenerateCertificates(nameof(SslStream_ClientCertificateContext_SendsChain), serverCertificate: false);
922924
TestHelper.CleanupCertificates(nameof(SslStream_ClientCertificateContext_SendsChain));
923925

926+
SslCertificateTrust? trust = null;
927+
if (useTrust)
928+
{
929+
// This is simplification. We make all the intermediates trusted,
930+
// normally just the root would go here.
931+
trust = SslCertificateTrust.CreateForX509Collection(clientChain, false);
932+
}
933+
924934
var clientOptions = new SslClientAuthenticationOptions()
925935
{
926936
TargetHost = "localhost",
927937
};
928938
clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true;
929-
clientOptions.ClientCertificateContext = SslStreamCertificateContext.Create(clientCertificate, clientChain);
939+
clientOptions.ClientCertificateContext = SslStreamCertificateContext.Create(clientCertificate, useTrust ? null : clientChain, offline:true, trust);
930940

931941
await SslStream_ClientSendsChain_Core(clientOptions, clientChain);
932942

src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj

+3-3
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
<Compile Include="..\..\src\System\Net\Security\SslStreamCertificateContext.Linux.cs" />
5555
<Compile Include="..\..\src\System\Net\Security\Pal.Managed\SafeChannelBindingHandle.cs" />
5656
<Compile Include="..\..\src\System\Net\Security\CipherSuitesPolicyPal.Linux.cs" />
57-
<Compile Include="..\..\src\System\Net\Security\SslCertificateTrust.cs" />
5857
<Compile Include="..\..\src\System\Net\Security\CipherSuitesPolicy.cs" />
5958

6059
<!-- these are not specific to linux, but they are not necessary for tests on other platforms -->
@@ -145,7 +144,6 @@
145144
Link="Common\System\Net\Security\TlsAlertMessage.cs" />
146145
<Compile Include="$(CommonPath)System\Net\Security\TargetHostNameHelper.cs"
147146
Link="Common\System\Net\Security\TargetHostNameHelper.cs" />
148-
149147
<Compile Include="..\..\src\System\Net\SecurityStatusPal.cs"
150148
Link="ProductionCode\System\Net\SecurityStatusPal.cs" />
151149
<Compile Include="..\..\src\System\Net\Security\NetEventSource.Security.cs"
@@ -167,7 +165,9 @@
167165
<Compile Include="..\..\src\System\Net\Security\ReadWriteAdapter.cs"
168166
Link="ProductionCode\System\Net\Security\ReadWriteAdapter.cs" />
169167
<Compile Include="..\..\src\System\Net\SslStreamContext.cs"
170-
Link="ProductionCode\System\Net\SslStreamContext.cs" />
168+
Link="ProductionCode\System\Net\SslStreamContext.cs" />
169+
<Compile Include="..\..\src\System\Net\Security\SslCertificateTrust.cs"
170+
Link="ProductionCode\System\Net\Security\SslCertificateTrust.cs" />
171171
<Compile Include="$(CommonPath)System\Net\SecurityProtocol.cs"
172172
Link="ProductionCode\Common\System\Net\SecurityProtocol.cs" />
173173
<Compile Include="..\..\src\System\Net\Security\TlsAlertType.cs"

0 commit comments

Comments
 (0)