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

fix handling of Ssl2 and enable disabled tests #36098

Merged
merged 8 commits into from
May 19, 2020
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ public partial class SslStream

private enum Framing
{
Unknown = 0,
BeforeSSL3,
SinceSSL3,
Unified,
Invalid
Unknown = 0, // Initial before any frame is processd.
BeforeSSL3, // SSlv2
SinceSSL3, // SSlv3 & TLS
Unified, // Intermediate on first frame until response is processes.
Invalid // Somthing is wrong.
}

// This is set on the first packet to figure out the framing style.
Expand Down 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.SinceSSL3)
{
#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 @@ -409,25 +409,28 @@ private ProtocolToken ProcessBlob(int frameSize)
// ActiveSpan will exclude the "discarded" data.
_handshakeBuffer.Discard(frameSize);

// Often more TLS messages fit into same packet. Get as many complete frames as we can.
while (_handshakeBuffer.ActiveLength > TlsFrameHelper.HeaderSize)
if (_framing == Framing.SinceSSL3)
{
TlsFrameHeader nextHeader = default;

if (!TlsFrameHelper.TryGetFrameHeader(_handshakeBuffer.ActiveReadOnlySpan, ref nextHeader))
// Often more TLS messages fit into same packet. Get as many complete frames as we can.
while (_handshakeBuffer.ActiveLength > TlsFrameHelper.HeaderSize)
{
break;
}
TlsFrameHeader nextHeader = default;

frameSize = nextHeader.Length + TlsFrameHelper.HeaderSize;
if (nextHeader.Type == TlsContentType.AppData || frameSize > _handshakeBuffer.ActiveLength)
{
// We don't have full frame left or we already have app data which needs to be processed by decrypt.
break;
}
if (!TlsFrameHelper.TryGetFrameHeader(_handshakeBuffer.ActiveReadOnlySpan, ref nextHeader))
{
break;
}

frameSize = nextHeader.Length + TlsFrameHelper.HeaderSize;
if (nextHeader.Type == TlsContentType.AppData || frameSize > _handshakeBuffer.ActiveLength)
{
// We don't have full frame left or we already have app data which needs to be processed by decrypt.
break;
}

chunkSize += frameSize;
_handshakeBuffer.Discard(frameSize);
chunkSize += frameSize;
_handshakeBuffer.Discard(frameSize);
}
}

return _context!.NextMessage(availableData.Slice(0, chunkSize));
Expand Down Expand Up @@ -684,7 +687,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
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