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 TLS13 procesing on windows #34181

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -114,7 +114,7 @@ public static bool IsNonZeroLowerBoundArraySupported
public static bool SupportsClientAlpn => SupportsAlpn || (IsOSX && PlatformDetection.OSXVersion > new Version(10, 12));

// OpenSSL 1.1.1 and above.
public static bool SupportsTls13 => !IsWindows && !IsOSX && (OpenSslVersion.CompareTo(new Version(1,1,1)) >= 0);
public static bool SupportsTls13 => GetTls13Support();

private static Lazy<bool> s_largeArrayIsNotSupported = new Lazy<bool>(IsLargeArrayNotSupported);

Expand Down Expand Up @@ -204,6 +204,38 @@ private static bool GetSsl3Support()
return (IsOSX || (IsLinux && OpenSslVersion < new Version(1, 0, 2) && !IsDebian));
}

private static bool GetTls13Support()
{
if (IsWindows)
{
if (!IsWindows10Version2004OrGreater)
{
return false;
}

string clientKey = @"HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\SecurityProviders\SCHANNEL\Protocols\TLS 1.3\Client";
string serverKey = @"HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\SecurityProviders\SCHANNEL\Protocols\TLS 1.3\Server";

object client, server;
try
{
client = Registry.GetValue(clientKey, "Enabled", null);
server = Registry.GetValue(serverKey, "Enabled", null);
if (client is int c && server is int s)
{
return c == 1 && s == 1;
}
}
catch { };
// assume no if key is missing or on error.
return false;
}
else
{
return !IsOSX && (OpenSslVersion.CompareTo(new Version(1,1,1)) >= 0);
Copy link
Member

Choose a reason for hiding this comment

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

macOS doesn't support TLS 1.3 on any version?

Copy link
Member Author

Choose a reason for hiding this comment

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

not in .NET. Tracked by #1979

Copy link
Member

Choose a reason for hiding this comment

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

In that case can you please change this to something like:

else if (IsOSX)
{
    // [ActiveIssue("https://github.com/dotnet/runtime/issues/1979")]
    return false;
}
else
{
    return OpenSslVersion >= new Version(1,1,1);
}

? Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

Might be a little bit more understandable as:

Suggested change
return !IsOSX && (OpenSslVersion.CompareTo(new Version(1,1,1)) >= 0);
return !IsOSX && OpenSslVersion >= new Version(1,1,1);

}
}

private static bool GetIsRunningOnMonoInterpreter()
{
// This is a temporary solution because mono does not support interpreter detection
Expand Down
Expand Up @@ -234,8 +234,8 @@ private async Task ReplyOnReAuthenticationAsync<TIOAdapter>(TIOAdapter adapter,
private async Task ForceAuthenticationAsync<TIOAdapter>(TIOAdapter adapter, bool receiveFirst, byte[]? reAuthenticationData, bool isApm = false)
where TIOAdapter : ISslIOAdapter
{
_framing = Framing.Unknown;
ProtocolToken message;
bool handshakeCompleted = false;

if (reAuthenticationData == null)
{
Expand All @@ -262,10 +262,22 @@ private async Task ForceAuthenticationAsync<TIOAdapter>(TIOAdapter adapter, bool
// tracing done in NextMessage()
throw new AuthenticationException(SR.net_auth_SSPI, message.GetException());
}

if (message.Status.ErrorCode == SecurityStatusPalErrorCode.OK)
{
// We can finish renegotiation without doing any read.
handshakeCompleted = true;
}
}

_handshakeBuffer = new ArrayBuffer(InitialHandshakeBufferSize);
do
if (!handshakeCompleted)
{
// get ready to receive first frame
_handshakeBuffer = new ArrayBuffer(InitialHandshakeBufferSize);
_framing = Framing.Unknown;
}

while (!handshakeCompleted)
{
message = await ReceiveBlobAsync(adapter).ConfigureAwait(false);
if (message.Size > 0)
Expand All @@ -278,7 +290,12 @@ private async Task ForceAuthenticationAsync<TIOAdapter>(TIOAdapter adapter, bool
{
throw new AuthenticationException(SR.net_auth_SSPI, message.GetException());
}
} while (message.Status.ErrorCode != SecurityStatusPalErrorCode.OK);
else if (message.Status.ErrorCode == SecurityStatusPalErrorCode.OK)
{
// We can finish renegotiation without doing any read.
handshakeCompleted = true;
}
}

if (_handshakeBuffer.ActiveLength > 0)
{
Expand Down Expand Up @@ -659,10 +676,18 @@ private async ValueTask<int> ReadAsyncInternal<TIOAdapter>(TIOAdapter adapter, M
// To handle this we call DecryptData() under lock and we create TCS waiter.
// EncryptData() checks that under same lock and if it exist it will not call low-level crypto.
// Instead it will wait synchronously or asynchronously and it will try again after the wait.
// The result will be set when ReplyOnReAuthenticationAsync() is finished e.g. lsass business is over
// or if we bail to continue. If either one happen before EncryptData(), _handshakeWaiter will be set to null
// The result will be set when ReplyOnReAuthenticationAsync() is finished e.g. lsass business is over.
// If that happen before EncryptData() runs, _handshakeWaiter will be set to null
// and EncryptData() will work normally e.g. no waiting, just exclusion with DecryptData()
_handshakeWaiter = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);


if (_sslAuthenticationOptions!.AllowRenegotiation || SslProtocol == SslProtocols.Tls13)
{
// create TCS only if we plan to proceed. If not, we will throw in block bellow outside of the lock.
// Tls1.3 does not have renegotiation. However on Windows this error code is used
// for session management e.g. anything lsass needs to see.
_handshakeWaiter = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
}
}
}

Expand All @@ -686,11 +711,9 @@ private async ValueTask<int> ReadAsyncInternal<TIOAdapter>(TIOAdapter adapter, M

if (status.ErrorCode == SecurityStatusPalErrorCode.Renegotiate)
{
if (!_sslAuthenticationOptions!.AllowRenegotiation)
// We determine above that we will not process it.
if (_handshakeWaiter == null)
{
_handshakeWaiter!.SetResult(false);
_handshakeWaiter = null;

if (NetEventSource.IsEnabled) NetEventSource.Fail(this, "Renegotiation was requested but it is disallowed");
throw new IOException(SR.net_ssl_io_renego);
}
Expand Down
Expand Up @@ -60,7 +60,7 @@ public async Task ServerNoEncryption_ClientAllowNoEncryption_ConnectWithNoEncryp

using (var sslStream = new SslStream(client.GetStream(), false, AllowAnyServerCertificate, null, EncryptionPolicy.AllowNoEncryption))
{
await sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocolSupport.DefaultSslProtocols, false);
await sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12, false);

_log.WriteLine("Client authenticated to server({0}) with encryption cipher: {1} {2}-bit strength",
serverNoEncryption.RemoteEndPoint, sslStream.CipherAlgorithm, sslStream.CipherStrength);
Expand Down
Expand Up @@ -715,13 +715,17 @@ private static NegotiatedParams ConnectAndGetNegotiatedParams(ConnectionParams s
{
// send some bytes, make sure they can talk
byte[] data = new byte[] { 1, 2, 3 };
server.WriteAsync(data, 0, data.Length);

byte[] receivedData = new byte[1];
Task serverTask = server.WriteAsync(data, 0, data.Length);
for (int i = 0; i < data.Length; i++)
{
Assert.Equal(data[i], client.ReadByte());
Assert.True(client.ReadAsync(receivedData, 0, 1).Wait(TestConfiguration.PassingTestTimeoutMilliseconds),
$"Read task failed to finish in {TestConfiguration.PassingTestTimeoutMilliseconds}ms.");
Assert.Equal(data[i], receivedData[0]);
}

Assert.True(serverTask.Wait(TestConfiguration.PassingTestTimeoutMilliseconds),
$"WriteTask failed to finish in {TestConfiguration.PassingTestTimeoutMilliseconds}ms.");
return new NegotiatedParams(server, client);
}
else
Expand Down