use new API on new windows to get TLS13 #37888

wfurt · 2020-06-15T07:30:35Z

Update SslStream to use SCH_CREDENTIALS on new Windows builds. It does not allow to enforce EncryptionPolicy.NoEncryption so when requested, we will fall-back to legacy API and protocols.

It will negotiate TLS 1.3 on Windows insider build and it will pass all tests (but the one bellow)

fixes #1720

SslStream_StreamToStream_ClientInitiatedCloseNotify_Ok test was failing. After some experiments, it can pass if server passes some data before shutdown. This is probably OS bug and ticket was opened to track it. Completion of TLS 1.3 piggy backs on application frames but this should work IMHO. I updated the test to run with and without extra data to make it more visible. We can decided what to do when we get new windows build to CI.

ghost · 2020-06-15T07:30:37Z

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs

src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs

…Win_1720

wfurt · 2020-07-01T18:34:09Z

this is ready for another review round.

src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs

stephentoub · 2020-07-03T00:22:22Z

src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs

+            public int cCreds;
+
+            // This is pointer to arry of CERT_CONTEXT*
+            // We do not use it directly in .NET. Instead, we wrap returned OS pointer in safe handle.


Is this comment needed? I'm not even sure what it means; we do use it.

the meaning was that we do not dereference or use CERT_CONTEXT in .NET. So it is pointer to a structure but that is hidden to PAL.

I added that comment after your questions about IntPtr vs something strongly typed.

So the "it" in we dot not use it" is referring to CERT_CONTEXT rather than to paCred? That was my confusion.

But, even so, we are kind of using it, aren't we? X509Certificate.Handle must be a CERT_CONTEXT*; we're storing that into paCred, effectively storing a pointer to an array of one CERT_CONTEXT. So while we're not naming it as such, we are using it.

yes. I assumed the "directly" would be a hint but obviously it is not clear. I was also thinking about also adding CERT_CONTEXT so we can see details in debugger but that is somewhat complicated with more pointers inside.
Maybe Windbg could do cast to native but I failed so far with VS.

src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs

stephentoub · 2020-07-03T00:33:26Z

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs

+                credential.pTlsParameters = &tlsParameters;
+            }
+
+            return AcquireCredentialsHandle(direction, &credential);


This credential is storing a pointer to something on the stack. I assume that whatever AcquireCredentialsHandle returns doesn't expect that to still be alive?

not as far as I know. I can verify that as well. I expect tile time to be same as the structure.

…Win_1720

scalablecory

looks good, thanks.

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs

src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs

wfurt · 2020-07-15T16:51:13Z

/azp run runtime-libraries

azure-pipelines · 2020-07-15T16:51:18Z

No pipelines are associated with this pull request.

wfurt · 2020-07-15T16:59:03Z

/azp help

azure-pipelines · 2020-07-15T16:59:07Z

Supported commands help: Get descriptions, examples and documentation about supported commands Example: help "command_name" list: List all pipelines for this repository using a comment. Example: "list" run: Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run. Example: "run" or "run pipeline_name, pipeline_name, pipeline_name" where: Report back the Azure DevOps orgs that are related to this repository and org Example: "where" See additional documentation.

wfurt · 2020-07-15T17:00:09Z

/azp list

azure-pipelines · 2020-07-15T17:01:11Z

… tls13Win_1720

safern · 2020-07-15T22:33:17Z

@wfurt I know CI is acting weird. However this was merged without CI run. There was only the cla that was triggered.

wfurt · 2020-07-15T22:34:01Z

oops. I let it sit and did something else and then I saw it green.

wfurt added 2 commits June 14, 2020 20:12

use new API on new windows to get TLS13

58a4493

use unmanaged ref to avoid copy

881e77d

wfurt added area-System.Net.Security os-windows labels Jun 15, 2020

wfurt self-assigned this Jun 15, 2020

davidsh reviewed Jun 15, 2020

View reviewed changes

src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs Outdated Show resolved Hide resolved

wfurt mentioned this pull request Jun 15, 2020

reduce use of TRACE_VERBOSE in SslStream #37917

Merged

stephentoub reviewed Jun 15, 2020

View reviewed changes

src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jun 15, 2020

View reviewed changes

src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jun 15, 2020

View reviewed changes

src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jun 15, 2020

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jun 15, 2020

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jun 15, 2020

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs Outdated Show resolved Hide resolved

wfurt added 2 commits June 16, 2020 22:40

Merge branch 'master' of https://github.com/dotnet/runtime into tls13…

21d39d3

…Win_1720

feedback from review

28fd688

bartonjs reviewed Jul 2, 2020

View reviewed changes

src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs Show resolved Hide resolved

stephentoub reviewed Jul 3, 2020

View reviewed changes

src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jul 3, 2020

View reviewed changes

src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jul 3, 2020

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jul 3, 2020

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jul 3, 2020

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jul 3, 2020

View reviewed changes

wfurt added 3 commits July 6, 2020 20:14

feedback from review

ad23a53

Merge branch 'master' of https://github.com/dotnet/runtime into tls13…

72d944c

…Win_1720

Merge branch 'master' of https://github.com/dotnet/runtime into tls13…

496f726

…Win_1720

scalablecory approved these changes Jul 14, 2020

View reviewed changes

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs Outdated Show resolved Hide resolved

src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs Outdated Show resolved Hide resolved

feedback from review

60c550a

wfurt closed this Jul 15, 2020

wfurt reopened this Jul 15, 2020

wfurt closed this Jul 15, 2020

wfurt reopened this Jul 15, 2020

wfurt added 2 commits July 15, 2020 10:04

kick the build

fcb81d1

Merge branch 'tls13Win_1720' of https://github.com/wfurt/runtime into…

3a14532

… tls13Win_1720

safern closed this Jul 15, 2020

safern reopened this Jul 15, 2020

wfurt merged commit a4051a5 into dotnet:master Jul 15, 2020

wfurt mentioned this pull request Jul 22, 2020

cleanup schannel pal #39755

Merged

karelz added this to the 5.0.0 milestone Aug 18, 2020

ghost locked as resolved and limited conversation to collaborators Dec 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use new API on new windows to get TLS13 #37888

use new API on new windows to get TLS13 #37888

wfurt commented Jun 15, 2020

ghost commented Jun 15, 2020

wfurt commented Jul 1, 2020

stephentoub Jul 3, 2020

wfurt Jul 3, 2020

wfurt Jul 3, 2020

stephentoub Jul 3, 2020

wfurt Jul 3, 2020

stephentoub Jul 3, 2020

wfurt Jul 3, 2020

scalablecory left a comment

wfurt commented Jul 15, 2020

azure-pipelines bot commented Jul 15, 2020

wfurt commented Jul 15, 2020

azure-pipelines bot commented Jul 15, 2020

wfurt commented Jul 15, 2020

azure-pipelines bot commented Jul 15, 2020

safern commented Jul 15, 2020

wfurt commented Jul 15, 2020

use new API on new windows to get TLS13 #37888

use new API on new windows to get TLS13 #37888

Conversation

wfurt commented Jun 15, 2020

ghost commented Jun 15, 2020

wfurt commented Jul 1, 2020

stephentoub Jul 3, 2020

Choose a reason for hiding this comment

wfurt Jul 3, 2020

Choose a reason for hiding this comment

wfurt Jul 3, 2020

Choose a reason for hiding this comment

stephentoub Jul 3, 2020

Choose a reason for hiding this comment

wfurt Jul 3, 2020

Choose a reason for hiding this comment

stephentoub Jul 3, 2020

Choose a reason for hiding this comment

wfurt Jul 3, 2020

Choose a reason for hiding this comment

scalablecory left a comment

Choose a reason for hiding this comment

wfurt commented Jul 15, 2020

azure-pipelines bot commented Jul 15, 2020

wfurt commented Jul 15, 2020

azure-pipelines bot commented Jul 15, 2020

wfurt commented Jul 15, 2020

azure-pipelines bot commented Jul 15, 2020

safern commented Jul 15, 2020

wfurt commented Jul 15, 2020