diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs index e82dd608eac8f2..01ef62006d269f 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs @@ -45,6 +45,32 @@ private JavaProxy.RemoteCertificateValidationResult VerifyRemoteCertificate(IntP }; } + private static bool CanQueryConnectionInfoDuringCertificateValidation() + { + if (OperatingSystem.IsAndroidVersionAtLeast(29)) + { + // API 29+ can read the negotiated cipher from the handshake session. + return true; + } + + if (OperatingSystem.IsAndroidVersionAtLeast(27)) + { + // API 27 can dereference a null cipher and cause SIGSEGV. + // API 28 reports SSL_NULL_WITH_NULL_NULL instead. + // Both must wait for handshake completion. + return false; + } + + if (OperatingSystem.IsAndroidVersionAtLeast(26)) + { + // API 26 exposes an intermediate handshake session with a valid cipher. + return true; + } + + // API 24-25 report (NONE) before certificate validation completes. + return false; + } + private bool ShouldRespectPlatformValidation() { // Android platform trust is part of default/callback-only validation, but explicit diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index a3651646741316..dfe7e480f168a5 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -1372,11 +1372,11 @@ internal static bool VerifyRemoteCertificateCore( // which may access properties like SslProtocol or CipherAlgorithm. // During inline cert validation the handshake hasn't completed yet, so // connectionInfo may not have been set by ProcessHandshakeSuccess. - // Android 8.1's Conscrypt dereferences a null current cipher during this - // phase of the handshake. Defer its connection info query until handshake - // completion; earlier and later Android versions do not have this bug. - if (connectionInfo.Protocol == 0 && securityContext is not null && - (!OperatingSystem.IsAndroidVersionAtLeast(27) || OperatingSystem.IsAndroidVersionAtLeast(28))) + if (connectionInfo.Protocol == 0 && securityContext is not null +#if TARGET_ANDROID + && CanQueryConnectionInfoDuringCertificateValidation() +#endif + ) { SslStreamPal.QueryContextConnectionInfo(securityContext, ref connectionInfo); } diff --git a/src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/AndroidPlatformTrustTests.cs b/src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/AndroidPlatformTrustTests.cs index 912d53cbf08bcc..a8359713e7204d 100644 --- a/src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/AndroidPlatformTrustTests.cs +++ b/src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/AndroidPlatformTrustTests.cs @@ -1,6 +1,7 @@ // 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.IO; using System.Net.Sockets; using System.Security.Authentication; @@ -28,8 +29,23 @@ namespace System.Net.Security.Tests /// public class AndroidPlatformTrustTests { - [Fact] - public async Task SslStream_CustomRootTrust_NoChainErrors() + public static IEnumerable ConnectionInfoProtocols() + { + foreach (SslProtocols protocol in new[] { SslProtocols.Tls12, SslProtocols.None, SslProtocols.Tls13 }) + { + if (protocol != SslProtocols.Tls13 || PlatformDetection.SupportsTls13) + { + foreach (bool mutualAuthentication in new[] { false, true }) + { + yield return new object[] { protocol, mutualAuthentication }; + } + } + } + } + + [Theory] + [MemberData(nameof(ConnectionInfoProtocols))] + public async Task SslStream_CustomRootTrust_NoChainErrors(SslProtocols protocol, bool mutualAuthentication) { // The server uses testservereku.contoso.com.pfx signed by the NDX Test Root CA. // CustomRootTrust is configured with the NDX root so managed validation accepts. @@ -37,8 +53,10 @@ public async Task SslStream_CustomRootTrust_NoChainErrors() // pre-seed RemoteCertificateChainErrors. using X509Certificate2 rootCertificate = GetRootCertificate(); + using X509Certificate2 clientCertificate = Configuration.Certificates.GetSelfSignedClientCertificate(); SslPolicyErrors? reportedErrors = null; int callbacks = 0; + int serverCallbacks = 0; (SslStream client, SslStream server) = GetConnectedSslStreams(); using (client) @@ -48,33 +66,25 @@ public async Task SslStream_CustomRootTrust_NoChainErrors() var serverOptions = new SslServerAuthenticationOptions { ServerCertificate = serverCertificate, - EnabledSslProtocols = SslProtocols.Tls12, + EnabledSslProtocols = protocol, }; var clientOptions = new SslClientAuthenticationOptions { TargetHost = "testservereku.contoso.com", - EnabledSslProtocols = SslProtocols.Tls12, + EnabledSslProtocols = protocol, RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => { - callbacks++; reportedErrors = sslPolicyErrors; Assert.Same(client, sender); Assert.NotNull(certificate); Assert.Equal(serverCertificate.RawData, certificate.GetRawCertData()); Assert.NotNull(chain); + Assert.Equal(SslPolicyErrors.None, sslPolicyErrors); - if (OperatingSystem.IsAndroidVersionAtLeast(27) && !OperatingSystem.IsAndroidVersionAtLeast(28)) - { - Assert.Throws(() => client.SslProtocol); - Assert.Throws(() => client.NegotiatedCipherSuite); - } - else - { - Assert.Equal(SslProtocols.Tls12, client.SslProtocol); - _ = client.NegotiatedCipherSuite; - } + AssertConnectionInfo(client); + callbacks++; return sslPolicyErrors == SslPolicyErrors.None; }, CertificateChainPolicy = new X509ChainPolicy @@ -85,21 +95,88 @@ public async Task SslStream_CustomRootTrust_NoChainErrors() }, }; - await Task.WhenAll( - client.AuthenticateAsClientAsync(clientOptions), - server.AuthenticateAsServerAsync(serverOptions)).WaitAsync(TimeSpan.FromSeconds(30)); + if (mutualAuthentication) + { + clientOptions.RemoteCertificateValidationCallback = null; + clientOptions.ClientCertificates = new X509CertificateCollection { clientCertificate }; + serverOptions.ClientCertificateRequired = true; + serverOptions.CertificateChainPolicy = new X509ChainPolicy + { + RevocationMode = X509RevocationMode.NoCheck, + TrustMode = X509ChainTrustMode.CustomRootTrust, + CustomTrustStore = { clientCertificate }, + }; + serverOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, errors) => + { + reportedErrors = errors; + Assert.Same(server, sender); + Assert.NotNull(certificate); + Assert.Equal(clientCertificate.RawData, certificate.GetRawCertData()); + Assert.NotNull(chain); + Assert.Equal(SslPolicyErrors.None, errors); + AssertConnectionInfo(server); + serverCallbacks++; + return errors == SslPolicyErrors.None; + }; + } + + Task clientAuthentication = client.AuthenticateAsClientAsync(clientOptions); + Task serverAuthentication = server.AuthenticateAsServerAsync(serverOptions); + await TaskTimeoutExtensions.WhenAllOrAnyFailed( + clientAuthentication, serverAuthentication, millisecondsTimeout: 30_000); - Assert.Equal(1, callbacks); Assert.True(client.IsAuthenticated); Assert.True(server.IsAuthenticated); - Assert.Equal(SslProtocols.Tls12, client.SslProtocol); + Assert.NotEqual(SslProtocols.None, client.SslProtocol); + if (protocol != SslProtocols.None) + { + Assert.Equal(protocol, client.SslProtocol); + } + Assert.Equal(client.SslProtocol, server.SslProtocol); Assert.NotEqual(default, client.NegotiatedCipherSuite); Assert.True(Enum.IsDefined(client.NegotiatedCipherSuite)); Assert.Equal(server.NegotiatedCipherSuite, client.NegotiatedCipherSuite); + + await VerifyDataExchange(client, server); + await VerifyDataExchange(server, client); + + Assert.Equal(mutualAuthentication ? 0 : 1, callbacks); + Assert.Equal(mutualAuthentication ? 1 : 0, serverCallbacks); } Assert.NotNull(reportedErrors); Assert.Equal(SslPolicyErrors.None, reportedErrors.Value & SslPolicyErrors.RemoteCertificateChainErrors); + + void AssertConnectionInfo(SslStream stream) + { + if (!OperatingSystem.IsAndroidVersionAtLeast(26) || + (OperatingSystem.IsAndroidVersionAtLeast(27) && !OperatingSystem.IsAndroidVersionAtLeast(29))) + { + Assert.Throws(() => stream.SslProtocol); + Assert.Throws(() => stream.NegotiatedCipherSuite); + } + else + { + Assert.NotEqual(SslProtocols.None, stream.SslProtocol); + if (protocol != SslProtocols.None) + { + Assert.Equal(protocol, stream.SslProtocol); + } + Assert.NotEqual(default, stream.NegotiatedCipherSuite); + Assert.True(Enum.IsDefined(stream.NegotiatedCipherSuite)); + } + } + + static async Task VerifyDataExchange(SslStream sender, SslStream receiver) + { + byte[] expected = { 1, 2, 3, 4 }; + byte[] actual = new byte[expected.Length]; + await TaskTimeoutExtensions.WhenAllOrAnyFailed( + sender.WriteAsync(expected).AsTask(), + receiver.ReadExactlyAsync(actual).AsTask(), + millisecondsTimeout: 30_000); + Assert.Equal(expected, actual); + } } [Fact] diff --git a/src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/System.Net.Security.AndroidPlatformTrust.Tests.csproj b/src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/System.Net.Security.AndroidPlatformTrust.Tests.csproj index 27e884dd90bc57..5f09179d6f930b 100644 --- a/src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/System.Net.Security.AndroidPlatformTrust.Tests.csproj +++ b/src/libraries/System.Net.Security/tests/AndroidPlatformTrustTests/System.Net.Security.AndroidPlatformTrust.Tests.csproj @@ -17,6 +17,8 @@ Link="Common\System\Net\Configuration.Certificates.cs" /> +