Skip to content

CI matrix: Add windows 20H2 - fix TLS1.3 issue - #62435

Closed
aik-jahoda wants to merge 9 commits into
mainfrom
jajahoda/cimatrix20h2
Closed

CI matrix: Add windows 20H2 - fix TLS1.3 issue#62435
aik-jahoda wants to merge 9 commits into
mainfrom
jajahoda/cimatrix20h2

Conversation

@aik-jahoda

@aik-jahoda aik-jahoda commented Dec 6, 2021

Copy link
Copy Markdown
Contributor

When employ Windows server 20H2 into CI we got buch of TLS 1.3 errors:

System.ComponentModel.Win32Exception : The client and server cannot communicate, because they do not possess a common algorithm.

Usually it means the server/client doesn't support TLS 1.3

https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-heads-revert-62098-c4c66b16e5dd4304a8/System.Net.Security.Tests/1/console.77465e3d.log?%3F%253Fsv%253D2019-07-07%2526se%253D2021-12-21T09%25253A56%25253A18Z%2526sr%253Dc%2526sp%253Drl%2526sig%253D6ptrdfy%25252FY91GLeQ4bm8kc%25252FtyN0EXeNccWwtsbnhIAIw%25253D

replace #60054

part of #57947

@ghost

ghost commented Dec 6, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: aik-jahoda
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -
@aik-jahoda

Copy link
Copy Markdown
Contributor Author

Based on https://docs.microsoft.com/en-us/windows/win32/secauthn/protocols-in-tls-ssl--schannel-ssp- the TLS 1.3 is not supported (not explicitly, but there is no support for Windows 20H2)

Probably there is an issue with Windows version check.
/cc @wfurt

client = Registry.GetValue(clientKey, "Enabled", null);
server = Registry.GetValue(serverKey, "Enabled", null);
Console.WriteLine($"client {client}");
Console.WriteLine($"client {client}");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be server?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it served its purpose for getting the CI machine info. Reverting...

@wfurt

wfurt commented Dec 7, 2021

Copy link
Copy Markdown
Member

The support check will respect registry. Based on the output somebody/something enabled TLS 1.3 even if not officially supported. (The code was there for a while just not enabled by default)
My recommendation would be to find what enables it and remove that code.

@wfurt

wfurt commented Dec 7, 2021

Copy link
Copy Markdown
Member

BTW this may go back to base Helix image. I don't know who and how created that.
cc: @MattGal

@MattGal

MattGal commented Dec 7, 2021

Copy link
Copy Markdown
Member

BTW this may go back to base Helix image. I don't know who and how created that. cc: @MattGal

This is Helix-Server-DataCenter-20h2-11-09-2021, created I believe by @ChulHul . If you can list a specific modification it needs, I'm happy to help but that's unclear from this issue.

@wfurt

wfurt commented Dec 7, 2021

Copy link
Copy Markdown
Member

Thanks for quick update @MattGal.
It seems like from the debug and side conversion with @aik-jahoda KEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\SecurityProviders\SCHANNEL\Protocols\TLS 1.3\Client\Enabled and KEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\SecurityProviders\SCHANNEL\Protocols\TLS 1.3\Server\Enabled are set , forcing TLS 1.3 on platform that officially does not support it.

Can we either flip them to 0 or simply leave them empty?

@aik-jahoda

Copy link
Copy Markdown
Contributor Author

The Windows version is 10.0.19042 , however the check in Platform detection https://github.com/dotnet/runtime/blob/main/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs#L413
assumes the TLS 1.3 is available from 10.0.19041.

@wfurt, shouldn't be safer to change the IsWindows10Version2004OrGreater (20H1) to is 20H2 or greater (10.0.19044)? It would prevent this error when the registry are set but the Windows doesn't support it

@aik-jahoda

Copy link
Copy Markdown
Contributor Author

New helix machine will be available 12/15

@aik-jahoda

Copy link
Copy Markdown
Contributor Author
@ericstj

ericstj commented Jan 10, 2022

Copy link
Copy Markdown
Member

@safern do we need to rerun this to make sure the test results are still OK from outerloop or is it OK to merge?

@wfurt

wfurt commented Jan 10, 2022

Copy link
Copy Markdown
Member

The failing tests were innerloop in the past. We can run outer loop but I don't think it is necessary.

@danmoseley danmoseley closed this Jan 10, 2022
@danmoseley danmoseley reopened this Jan 10, 2022
@danmoseley

Copy link
Copy Markdown
Contributor

No harm in re-running at least the regular checks since it's been a while..

@ericstj

ericstj commented Feb 28, 2022

Copy link
Copy Markdown
Member

Closing this since the PR is now stale and the overall change is tracked by #57947

@ericstj ericstj closed this Feb 28, 2022
@jkotas
jkotas deleted the jajahoda/cimatrix20h2 branch March 4, 2022 16:08
@ghost ghost locked as resolved and limited conversation to collaborators Apr 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.