Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not call RemoteCertificateValidationCallback on the same certificate again #42836

Merged
merged 4 commits into from
Sep 30, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,12 @@ public static IEnumerable<object[]> GetAsync_AllowedSSLVersion_Succeeds_MemberDa
[MemberData(nameof(GetAsync_AllowedSSLVersion_Succeeds_MemberData))]
public async Task GetAsync_AllowedSSLVersion_Succeeds(SslProtocols acceptedProtocol, bool requestOnlyThisProtocol)
{
int count = 0;
using (HttpClientHandler handler = CreateHttpClientHandler())
using (HttpClient client = CreateHttpClient(handler))
{
handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates;
handler.ServerCertificateCustomValidationCallback =
(request, cert, chain, errors) => { count++; return true; };

if (requestOnlyThisProtocol)
{
Expand Down Expand Up @@ -131,6 +133,8 @@ await TestHelper.WhenAllCompletedOrAnyFailed(
server.AcceptConnectionSendResponseAndCloseAsync(),
client.GetAsync(url));
}, options);

Assert.Equal(1, count);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Runtime.ExceptionServices;
using System.Security;
Expand Down Expand Up @@ -954,7 +955,17 @@ internal bool VerifyRemoteCertificate(RemoteCertificateValidationCallback? remot

try
{
_remoteCertificate = CertificateValidationPal.GetRemoteCertificate(_securityContext, out remoteCertificateStore);
X509Certificate2? certificate = CertificateValidationPal.GetRemoteCertificate(_securityContext, out remoteCertificateStore);

if (_remoteCertificate != null && certificate != null &&
Enumerable.SequenceEqual(certificate.RawData, _remoteCertificate.RawData))
wfurt marked this conversation as resolved.
Show resolved Hide resolved
{
// This is renegotiation or TLS 1.3 and the certificate did not change.
// There is no reason to process callback again as we already established trust.
return true;
}

_remoteCertificate = certificate;

if (_remoteCertificate == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ public async Task SslStream_NestedAuth_Throws()
public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName)
{
string targetName = useEmptyName ? string.Empty : Guid.NewGuid().ToString("N");
int count = 0;

(Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams();
using (clientStream)
Expand All @@ -248,7 +249,7 @@ public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName)
{
SslStream stream = (SslStream)sender;
Assert.Equal(targetName, stream.TargetHostName);

count++;
return true;
};

Expand All @@ -265,8 +266,12 @@ public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName)
await TestConfiguration.WhenAllOrAnyFailedWithTimeout(
client.AuthenticateAsClientAsync(clientOptions),
server.AuthenticateAsServerAsync(serverOptions));

await TestHelper.PingPong(client, server);

Assert.Equal(targetName, client.TargetHostName);
Assert.Equal(targetName, server.TargetHostName);
Assert.Equal(1, count);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
using System.Security.Cryptography.X509Certificates.Tests.Common;
using System.Runtime.CompilerServices;
using System.Text;
using System.Threading.Tasks;
using Xunit;

namespace System.Net.Security.Tests
{
Expand All @@ -30,6 +32,9 @@ public static class TestHelper
private static readonly X509BasicConstraintsExtension s_eeConstraints =
new X509BasicConstraintsExtension(false, false, 0, false);

private static readonly byte[] s_ping = Encoding.UTF8.GetBytes("PING");
private static readonly byte[] s_pong = Encoding.UTF8.GetBytes("PONG");

public static (SslStream ClientStream, SslStream ServerStream) GetConnectedSslStreams()
{
(Stream clientStream, Stream serverStream) = GetConnectedStreams();
Expand Down Expand Up @@ -151,5 +156,33 @@ internal static (X509Certificate2 certificate, X509Certificate2Collection) Gener

return (endEntity, chain);
}

internal static async Task PingPong(SslStream client, SslStream server)
{
byte[] buffer = new byte[s_ping.Length];
ValueTask t = client.WriteAsync(s_ping);

int remains = s_ping.Length;
while (remains > 0)
{
int readLength = await server.ReadAsync(buffer, buffer.Length - remains, remains);
Assert.True(readLength > 0);
remains -= readLength;
}
Assert.Equal(s_ping, buffer);
await t;

t = server.WriteAsync(s_pong);
remains = s_pong.Length;
while (remains > 0)
{
int readLength = await client.ReadAsync(buffer, buffer.Length - remains, remains);
Assert.True(readLength > 0);
remains -= readLength;
}

Assert.Equal(s_pong, buffer);
await t;
}
}
}