Skip to content

remove support for Ssl2 #64322

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

Merged
wfurt merged 4 commits into from
Jan 28, 2022
Merged

remove support for Ssl2 #64322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ababac5
remove support for Ssl2
wfurt Jan 26, 2022
b87f3a9
update test
wfurt Jan 26, 2022
cb8c208
update exeception type
wfurt Jan 26, 2022
01d2df9
remove Sslv2 test
wfurt Jan 26, 2022
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
263 changes: 22 additions & 241 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 @@ -21,18 +21,6 @@ public partial class SslStream
private int _nestedAuth;
private bool _isRenego;

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

// This is set on the first packet to figure out the framing style.
private Framing _framing = Framing.Unknown;

private TlsFrameHelper.TlsFrameInfo _lastFrame;

private object _handshakeLock => _sslAuthenticationOptions!;
Expand Down Expand Up @@ -460,27 +448,12 @@ private async ValueTask<ProtocolToken> ReceiveBlobAsync<TIOAdapter>(TIOAdapter a
where TIOAdapter : IReadWriteAdapter
{
await FillHandshakeBufferAsync(adapter, SecureChannel.ReadHeaderSize).ConfigureAwait(false);
if (_framing == Framing.Unified || _framing == Framing.Unknown)
{
_framing = DetectFraming(_handshakeBuffer.ActiveReadOnlySpan);
}

if (_framing != Framing.SinceSSL3)
{
#pragma warning disable 0618
_lastFrame.Header.Version = SslProtocols.Ssl2;
#pragma warning restore 0618
_lastFrame.Header.Length = GetFrameSize(_handshakeBuffer.ActiveReadOnlySpan) - TlsFrameHelper.HeaderSize;
}
else
{
TlsFrameHelper.TryGetFrameHeader(_handshakeBuffer.ActiveReadOnlySpan, ref _lastFrame.Header);
}
TlsFrameHelper.TryGetFrameHeader(_handshakeBuffer.ActiveReadOnlySpan, ref _lastFrame.Header);

if (_lastFrame.Header.Length < 0)
{
if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, "invalid TLS frame size");
throw new IOException(SR.net_frame_read_size);
throw new AuthenticationException(SR.net_frame_read_size);
Copy link
Member

@karelz karelz Jan 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why to change this exception when you leave IOException in GetFrameSize?
In other words: What is the rule to throw IOException vs. AuthenticationException?

Copy link
Member Author

@wfurt wfurt Jan 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ServerAsyncAuthenticate_InvalidHello_Throws checks for the exception type. In the past we can get one or the other depending on connection management. So I had to choose. Since getting invalid length did not feel like IO problem, I decided to use AuthenticationException

}

// Header length is content only so we must add header size as well.
Expand Down Expand Up @@ -560,29 +533,26 @@ private ProtocolToken ProcessBlob(int frameSize)
// ActiveSpan will exclude the "discarded" data.
_handshakeBuffer.Discard(frameSize);

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

if (!TlsFrameHelper.TryGetFrameHeader(_handshakeBuffer.ActiveReadOnlySpan, ref nextHeader))
{
break;
}
TlsFrameHeader nextHeader = default;

frameSize = nextHeader.Length + TlsFrameHelper.HeaderSize;
// Can process more handshake frames in single step, but we should avoid processing too much so as to preserve API boundary between handshake and I/O.
if ((nextHeader.Type != TlsContentType.Handshake && nextHeader.Type != TlsContentType.ChangeCipherSpec) || 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;
}

chunkSize += frameSize;
_handshakeBuffer.Discard(frameSize);
frameSize = nextHeader.Length + TlsFrameHelper.HeaderSize;
// Can process more handshake frames in single step, but we should avoid processing too much so as to preserve API boundary between handshake and I/O.
if ((nextHeader.Type != TlsContentType.Handshake && nextHeader.Type != TlsContentType.ChangeCipherSpec) || 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);
}

return _context!.NextMessage(availableData.Slice(0, chunkSize));
Expand Down Expand Up @@ -1199,204 +1169,15 @@ private void ResetReadBuffer()
}
}

// We need at least 5 bytes to determine what we have.
private Framing DetectFraming(ReadOnlySpan<byte> bytes)
{
/* PCTv1.0 Hello starts with
* RECORD_LENGTH_MSB (ignore)
* RECORD_LENGTH_LSB (ignore)
* PCT1_CLIENT_HELLO (must be equal)
* PCT1_CLIENT_VERSION_MSB (if version greater than PCTv1)
* PCT1_CLIENT_VERSION_LSB (if version greater than PCTv1)
*
* ... PCT hello ...
*/

/* Microsoft Unihello starts with
* RECORD_LENGTH_MSB (ignore)
* RECORD_LENGTH_LSB (ignore)
* SSL2_CLIENT_HELLO (must be equal)
* SSL2_CLIENT_VERSION_MSB (if version greater than SSLv2) ( or v3)
* SSL2_CLIENT_VERSION_LSB (if version greater than SSLv2) ( or v3)
*
* ... SSLv2 Compatible Hello ...
*/

/* SSLv2 CLIENT_HELLO starts with
* RECORD_LENGTH_MSB (ignore)
* RECORD_LENGTH_LSB (ignore)
* SSL2_CLIENT_HELLO (must be equal)
* SSL2_CLIENT_VERSION_MSB (if version greater than SSLv2) ( or v3)
* SSL2_CLIENT_VERSION_LSB (if version greater than SSLv2) ( or v3)
*
* ... SSLv2 CLIENT_HELLO ...
*/

/* SSLv2 SERVER_HELLO starts with
* RECORD_LENGTH_MSB (ignore)
* RECORD_LENGTH_LSB (ignore)
* SSL2_SERVER_HELLO (must be equal)
* SSL2_SESSION_ID_HIT (ignore)
* SSL2_CERTIFICATE_TYPE (ignore)
* SSL2_CLIENT_VERSION_MSB (if version greater than SSLv2) ( or v3)
* SSL2_CLIENT_VERSION_LSB (if version greater than SSLv2) ( or v3)
*
* ... SSLv2 SERVER_HELLO ...
*/

/* SSLv3 Type 2 Hello starts with
* RECORD_LENGTH_MSB (ignore)
* RECORD_LENGTH_LSB (ignore)
* SSL2_CLIENT_HELLO (must be equal)
* SSL2_CLIENT_VERSION_MSB (if version greater than SSLv3)
* SSL2_CLIENT_VERSION_LSB (if version greater than SSLv3)
*
* ... SSLv2 Compatible Hello ...
*/

/* SSLv3 Type 3 Hello starts with
* 22 (HANDSHAKE MESSAGE)
* VERSION MSB
* VERSION LSB
* RECORD_LENGTH_MSB (ignore)
* RECORD_LENGTH_LSB (ignore)
* HS TYPE (CLIENT_HELLO)
* 3 bytes HS record length
* HS Version
* HS Version
*/

/* SSLv2 message codes
* SSL_MT_ERROR 0
* SSL_MT_CLIENT_HELLO 1
* SSL_MT_CLIENT_MASTER_KEY 2
* SSL_MT_CLIENT_FINISHED 3
* SSL_MT_SERVER_HELLO 4
* SSL_MT_SERVER_VERIFY 5
* SSL_MT_SERVER_FINISHED 6
* SSL_MT_REQUEST_CERTIFICATE 7
* SSL_MT_CLIENT_CERTIFICATE 8
*/

int version = -1;

Debug.Assert(bytes.Length != 0, "Header buffer is not allocated.");

// If the first byte is SSL3 HandShake, then check if we have a SSLv3 Type3 client hello.
if (bytes[0] == (byte)TlsContentType.Handshake || bytes[0] == (byte)TlsContentType.AppData
|| bytes[0] == (byte)TlsContentType.Alert)
{
if (bytes.Length < 3)
{
return Framing.Invalid;
}

version = (bytes[1] << 8) | bytes[2];
if (version < 0x300 || version >= 0x500)
{
return Framing.Invalid;
}

//
// This is an SSL3 Framing
//
return Framing.SinceSSL3;
}

if (bytes.Length < 3)
{
return Framing.Invalid;
}

if (bytes[2] > 8)
{
return Framing.Invalid;
}

if (bytes[2] == 0x1) // SSL_MT_CLIENT_HELLO
{
if (bytes.Length >= 5)
{
version = (bytes[3] << 8) | bytes[4];
}
}
else if (bytes[2] == 0x4) // SSL_MT_SERVER_HELLO
{
if (bytes.Length >= 7)
{
version = (bytes[5] << 8) | bytes[6];
}
}

if (version != -1)
{
// If this is the first packet, the client may start with an SSL2 packet
// but stating that the version is 3.x, so check the full range.
// For the subsequent packets we assume that an SSL2 packet should have a 2.x version.
if (_framing == Framing.Unknown)
{
if (version != 0x0002 && (version < 0x200 || version >= 0x500))
{
return Framing.Invalid;
}
}
else
{
if (version != 0x0002)
{
return Framing.Invalid;
}
}
}

// When server has replied the framing is already fixed depending on the prior client packet
if (!_context!.IsServer || _framing == Framing.Unified)
{
return Framing.BeforeSSL3;
}

return Framing.Unified; // Will use Ssl2 just for this frame.
}

// Returns TLS Frame size.
// Returns TLS Frame size including header size.
private int GetFrameSize(ReadOnlySpan<byte> buffer)
{
int payloadSize;
switch (_framing)
if (buffer.Length < SecureChannel.ReadHeaderSize)
{
case Framing.Unified:
case Framing.BeforeSSL3:
if (buffer.Length < 2)
{
throw new IOException(SR.net_ssl_io_frame);
}
// Note: Cannot detect version mismatch for <= SSL2

if ((buffer[0] & 0x80) != 0)
{
// Two bytes
payloadSize = (((buffer[0] & 0x7f) << 8) | buffer[1]) + 2;
}
else
{
// Three bytes
payloadSize = (((buffer[0] & 0x3f) << 8) | buffer[1]) + 3;
}

break;
case Framing.SinceSSL3:
if (buffer.Length < 5)
{
throw new IOException(SR.net_ssl_io_frame);
}

payloadSize = ((buffer[3] << 8) | buffer[4]) + 5;
break;
default:
throw new IOException(SR.net_frame_read_size);
throw new IOException(SR.net_ssl_io_frame);
}

return payloadSize;
return ((buffer[3] << 8) | buffer[4]) + SecureChannel.ReadHeaderSize;
}
}
}
14 changes: 0 additions & 14 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 @@ -52,20 +52,6 @@ public async Task ClientAsyncAuthenticate_EachSupportedProtocol_Success(SslProto
await ClientAsyncSslHelper(protocol, protocol);
}

[Fact]
[PlatformSpecific(TestPlatforms.Windows)]
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.SupportsSsl2)
{
#pragma warning disable 0618
await ClientAsyncSslHelper(SslProtocols.Ssl2, SslProtocols.Ssl2);
#pragma warning restore 0618
}
}

[Theory]
[MemberData(nameof(ProtocolMismatchData))]
public async Task ClientAsyncAuthenticate_MismatchProtocols_Fails(
Expand Down
9 changes: 1 addition & 8 deletions src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,14 +317,7 @@ public async Task ServerAsyncAuthenticate_InvalidHello_Throws(bool close)
await t2.WaitAsync(TestConfiguration.PassingTestTimeout);
}

if (close)
{
await Assert.ThrowsAsync<IOException>(() => t1);
}
else
{
await Assert.ThrowsAsync<AuthenticationException>(() => t1);
}
await Assert.ThrowsAsync<AuthenticationException>(() => t1);
}
}

Expand Down