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.

Sign up for GitHub

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

fix handling of Ssl2 and enable disabled tests #36098

Merged
merged 8 commits into from
May 19, 2020
+28 −28
Merged

fix handling of Ssl2 and enable disabled tests #36098

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
eef75b3
attempt to fix ssl2
May 8, 2020
08b6ae3
adjust length calculation
May 8, 2020
fa3191c
Merge branch 'master' of https://github.com/dotnet/runtime into sslv1
May 8, 2020
5cb8571
enable tests
May 8, 2020
114627d
fix ReadAsyncInternal
May 9, 2020
a7d5638
Merge branch 'master' of https://github.com/dotnet/runtime into sslv2
May 9, 2020
556ea10
use PlatformDetection.SupportsSsl2
May 9, 2020
532f945
feedback from review
May 10, 2020
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
4 changes: 1 addition & 3 deletions src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.SslProtocols.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ public static IEnumerable<object[]> GetAsync_AllowedSSLVersion_Succeeds_MemberDa

[Theory]
[MemberData(nameof(GetAsync_AllowedSSLVersion_Succeeds_MemberData))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/36100", TestPlatforms.Windows)]
public async Task GetAsync_AllowedSSLVersion_Succeeds(SslProtocols acceptedProtocol, bool requestOnlyThisProtocol)
{
using (HttpClientHandler handler = CreateHttpClientHandler())
Expand Down Expand Up @@ -198,7 +197,7 @@ public async Task GetAsync_SupportedSSLVersion_Succeeds(SslProtocols sslProtocol
public static IEnumerable<object[]> NotSupportedSSLVersionServers()
{
#pragma warning disable 0618
if (PlatformDetection.IsWindows10Version1607OrGreater)
if (PlatformDetection.SupportsSsl2)
{
yield return new object[] { SslProtocols.Ssl2, Configuration.Http.SSLv2RemoteServer };
}
Expand Down Expand Up @@ -251,7 +250,6 @@ await TestHelper.WhenAllCompletedOrAnyFailed(
[InlineData(SslProtocols.Tls11 | SslProtocols.Tls12, SslProtocols.Tls)] // Skip this on WinHttpHandler.
[InlineData(SslProtocols.Tls12, SslProtocols.Tls11)]
[InlineData(SslProtocols.Tls, SslProtocols.Tls12)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/36100", TestPlatforms.Windows)]
public async Task GetAsync_AllowedClientSslVersionDiffersFromServer_ThrowsException(
SslProtocols allowedClientProtocols, SslProtocols acceptedServerProtocols)
{
Expand Down
8 changes: 4 additions & 4 deletions src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,12 +352,12 @@ private async ValueTask<ProtocolToken> ReceiveBlobAsync<TIOAdapter>(TIOAdapter a
_framing = DetectFraming(_handshakeBuffer.ActiveReadOnlySpan);
}

if (_framing == Framing.BeforeSSL3)
if (_framing == Framing.BeforeSSL3 || _framing == Framing.Unified)
wfurt marked this conversation as resolved.
Show resolved Hide resolved
{
#pragma warning disable 0618
_lastFrame.Header.Version = SslProtocols.Ssl2;
#pragma warning restore 0618
_lastFrame.Header.Length = GetFrameSize(_handshakeBuffer.ActiveReadOnlySpan);
_lastFrame.Header.Length = GetFrameSize(_handshakeBuffer.ActiveReadOnlySpan) - TlsFrameHelper.HeaderSize;
}
else
{
Expand Down Expand Up @@ -410,7 +410,7 @@ private ProtocolToken ProcessBlob(int frameSize)
_handshakeBuffer.Discard(frameSize);

// Often more TLS messages fit into same packet. Get as many complete frames as we can.
while (_handshakeBuffer.ActiveLength > TlsFrameHelper.HeaderSize)
while (_handshakeBuffer.ActiveLength > TlsFrameHelper.HeaderSize && _framing != Framing.BeforeSSL3 && _framing == Framing.Unified)
wfurt marked this conversation as resolved.
Show resolved Hide resolved
{
TlsFrameHeader nextHeader = default;

Expand Down Expand Up @@ -684,7 +684,7 @@ private async ValueTask<int> ReadAsyncInternal<TIOAdapter>(TIOAdapter adapter, M
Debug.Assert(_internalBufferCount >= SecureChannel.ReadHeaderSize);

// Parse the frame header to determine the payload size (which includes the header size).
int payloadBytes = TlsFrameHelper.GetFrameSize(_internalBuffer.AsSpan(_internalOffset));
int payloadBytes = GetFrameSize(_internalBuffer.AsSpan(_internalOffset));
if (payloadBytes < 0)
{
throw new IOException(SR.net_frame_read_size);
Expand Down
3 changes: 1 addition & 2 deletions src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,11 @@ public async Task ClientAsyncAuthenticate_EachSupportedProtocol_Success(SslProto

[Fact]
[PlatformSpecific(TestPlatforms.Windows)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/36100")]
public async Task ClientAsyncAuthenticate_Ssl2WithSelf_Success()
{
// Test Ssl2 against itself. This is a standalone test as even on versions where Windows supports Ssl2,
// it appears to have rules around not using it when other protocols are mentioned.
if (!PlatformDetection.IsWindows10Version1607OrGreater)
if (PlatformDetection.SupportsSsl2)
{
#pragma warning disable 0618
await ClientAsyncSslHelper(SslProtocols.Ssl2, SslProtocols.Ssl2);
Expand Down