Skip to content

Commit ce2fdb4

Browse files
simonrozsivalwfurtvitek-karas
authored
[release/8.0-staging] use also SslCertificateTrust when constructing CertificateContext (#104541)
Backport of #103372 and #104016 to release/8.0-staging ## Customer Impact - [X] Customer reported (#100602) - [ ] Found internally Customers developing Android apps are currently unable to use mutual TLS authentication in certain cases as the `SslStreamCertificateContext.Create(...)` method will fail to build an X509Chain instance if the certificate isn't trusted by the OS due to the limitations of the Android platform. ## Regression - [ ] Yes - [X] No ## Testing Unit tests and manual testing on Android emulator. ## Risk Low. The change is mostly limited to Android where this API doesn't currently work in many cases. --------- Co-authored-by: Tomas Weinfurt <tweinfurt@yahoo.com> Co-authored-by: Vitek Karas <10670590+vitek-karas@users.noreply.github.com>
1 parent 09784cd commit ce2fdb4

7 files changed

+50
-6
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
@@ -18,6 +18,7 @@ namespace System.Net.Security
1818
public partial class SslStreamCertificateContext
1919
{
2020
private const bool TrimRootCertificate = true;
21+
private const bool ChainBuildNeedsTrustedRoot = false;
2122
internal readonly ConcurrentDictionary<SslProtocols, SafeSslContextHandle> SslContexts;
2223
internal readonly SafeX509Handle CertificateHandle;
2324
internal readonly SafeEvpPKeyHandle KeyHandle;

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

+31-2
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,24 @@ 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+
if (trust._store != null)
60+
{
61+
chain.ChainPolicy.CustomTrustStore.AddRange(trust._store.Certificates);
62+
}
63+
64+
if (trust._trustList != null)
65+
{
66+
chain.ChainPolicy.CustomTrustStore.AddRange(trust._trustList);
67+
}
68+
69+
if (chain.ChainPolicy.CustomTrustStore.Count > 0)
5570
{
56-
chain.ChainPolicy.ExtraStore.Add(cert);
71+
chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
5772
}
5873
}
5974

@@ -67,6 +82,20 @@ internal static SslStreamCertificateContext Create(
6782
NetEventSource.Error(null, $"Failed to build chain for {target.Subject}");
6883
}
6984

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

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

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

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

917-
[Fact]
918-
[ActiveIssue("https://github.com/dotnet/runtime/issues/68206", TestPlatforms.Android)]
919-
public async Task SslStream_ClientCertificateContext_SendsChain()
917+
[Theory]
918+
[InlineData(true)]
919+
[InlineData(false)]
920+
public async Task SslStream_ClientCertificateContext_SendsChain(bool useTrust)
920921
{
921922
(X509Certificate2 clientCertificate, X509Certificate2Collection clientChain) = TestHelper.GenerateCertificates(nameof(SslStream_ClientCertificateContext_SendsChain), serverCertificate: false);
922923
TestHelper.CleanupCertificates(nameof(SslStream_ClientCertificateContext_SendsChain));
923924

925+
SslCertificateTrust? trust = null;
926+
if (useTrust)
927+
{
928+
// This is simplification. We make all the intermediates trusted,
929+
// normally just the root would go here.
930+
trust = SslCertificateTrust.CreateForX509Collection(clientChain, false);
931+
}
932+
924933
var clientOptions = new SslClientAuthenticationOptions()
925934
{
926935
TargetHost = "localhost",
927936
};
928937
clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true;
929-
clientOptions.ClientCertificateContext = SslStreamCertificateContext.Create(clientCertificate, clientChain);
938+
clientOptions.ClientCertificateContext = SslStreamCertificateContext.Create(clientCertificate, useTrust ? null : clientChain, offline:true, trust);
930939

931940
await SslStream_ClientSendsChain_Core(clientOptions, clientChain);
932941

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

+2
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@
7474
Link="ProductionCode\System\Net\Security\ReadWriteAdapter.cs" />
7575
<Compile Include="..\..\src\System\Net\SslStreamContext.cs"
7676
Link="ProductionCode\System\Net\SslStreamContext.cs" />
77+
<Compile Include="..\..\src\System\Net\Security\SslCertificateTrust.cs"
78+
Link="ProductionCode\System\Net\Security\SslCertificateTrust.cs" />
7779
<Compile Include="$(CommonPath)System\Net\SecurityProtocol.cs"
7880
Link="ProductionCode\Common\System\Net\SecurityProtocol.cs" />
7981
<Compile Include="..\..\src\System\Net\Security\TlsAlertType.cs"

0 commit comments

Comments
 (0)