Downgrade to HTTP/1.1 on older Windows versions #22859

JunTaoLuo · 2020-06-12T01:56:58Z

Addresses #16811

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

JunTaoLuo · 2020-06-12T02:26:38Z

@Tratcher I'm guessing the only testing we are going to need to do here is test it out on a Windows 8 machine?

Tratcher

For testing, there's automated tests for Win7, you should be able to mimic those for Win8.

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

src/Servers/Kestrel/Core/src/CoreStrings.resx

JunTaoLuo · 2020-06-12T03:49:24Z

We don't run any tests on Windows 7/Windows 8 anywhere in our CI, public or private. @HaoK I remember we removed all the different OSes from our Helix runs, is there a plan to ever bring those back?

HaoK · 2020-06-12T06:59:24Z

You should still these on the helix-matrix runs that run in master: https://dev.azure.com/dnceng/public/_build?definitionId=837

You can also turn them on in your PR if you want if you flip /p:IsHelixDaily=true on in the ci.yaml temporarily

https://github.com/dotnet/aspnetcore/blob/master/.azure/pipelines/helix-matrix.yml#L34

JunTaoLuo · 2020-06-12T17:50:12Z

Ah that's the missing piece, I couldn't find where the win7/win81 queues were being run so I thought we weren't running them at all. As long as we're running them somewhere I'll add tests.

JunTaoLuo · 2020-06-13T07:53:18Z

Hard to track down test status but according to https://helix.dot.net/api/2019-06-17/jobs/4e6d3e82-7d9a-4b49-928c-edea02bacab0/workitems/InMemory.FunctionalTests--net5.0/console the tests are passing. I'm going to also use this PR to fix/skip a bunch of tests on Windows 7/8

JunTaoLuo · 2020-06-13T18:39:56Z

Given the tests were verified in https://dev.azure.com/dnceng/public/_build/results?buildId=686284&view=results with remaining test failures on helix caused by unrelated issues: #22917. I'm marking this PR as ready.

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

src/Servers/Kestrel/test/FunctionalTests/Http2/HandshakeTests.cs

src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs

.azure/pipelines/ci.yml

halter73 · 2020-06-16T00:18:59Z

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

            // This configuration will always fail per-request, preemptively fail it here. See HttpConnection.SelectProtocol().
            if (options.HttpProtocols == HttpProtocols.Http2)
            {
                if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
                {
                    throw new NotSupportedException(CoreStrings.HTTP2NoTlsOsx);
                }
-                else if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && Environment.OSVersion.Version < new Version(6, 2))


Is there any risk that someone got ALPN working on Windows 8 somehow and this change breaks them despite the AppContext switch?

This probably warrants a breaking changes announcement.

This definitely warrants a breaking change announcement.

If someone does have ALPN working on Windows 8, changing < new Version(6, 2) to < new Version(6, 3) would completely break them and not even the AppContext switch would work around the issue.

ALPN support was added in Windows 8.1 so I don't think that was ever a scenario that worked. I made the change since I noticed that Windows 8.1 which was where ALPN support was first added is version 6.3 instead of 6.2. Unless there was an actual reason for allowing Http2 on Windows 8?

Ping @Tratcher

No reason to allow it on Win8 without ALPN, it's the same as Win7.

halter73 · 2020-06-16T00:19:55Z

src/Servers/Kestrel/Core/src/CoreStrings.resx

@@ -605,6 +602,12 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
  <data name="HttpsConnectionEstablished" xml:space="preserve">
    <value>Connection "{connectionId}" established using the following protocol: {protocol}</value>
  </data>
+  <data name="HTTP2DefaultCiphersInsufficient" xml:space="preserve">
+    <value>HTTP/2 over TLS is not supported on Windows versions older than Windows 10 and Windows Server 2016 due to incompatible ciphers or missing ALPN support. Falling back to HTTP/1.1 instead.</value>


Should we mention how to change the enabled ciphers and re-enable HTTP/2 on older Windows versions?

I think the best we can do is get a aka.ms link to something like https://docs.microsoft.com/en-us/windows/win32/secauthn/tls-cipher-suites-in-windows-8-1, would that be sufficient?

A breaking change announcement should be sufficient. We can follow up with anyone that's actually trying to make it work.

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

src/Servers/Kestrel/Core/src/CoreStrings.resx

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs

Tratcher · 2020-06-16T21:14:47Z

src/Servers/Kestrel/Core/src/CoreStrings.resx

@@ -605,6 +602,12 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
  <data name="HttpsConnectionEstablished" xml:space="preserve">
    <value>Connection "{connectionId}" established using the following protocol: {protocol}</value>
  </data>
+  <data name="HTTP2DefaultCiphersInsufficient" xml:space="preserve">
+    <value>HTTP/2 over TLS is not supported on Windows versions older than Windows 10 and Windows Server 2016 due to incompatible ciphers or missing ALPN support. Falling back to HTTP/1.1 instead.</value>


A breaking change announcement should be sufficient. We can follow up with anyone that's actually trying to make it work.

Tratcher · 2020-06-16T21:19:01Z

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

            // This configuration will always fail per-request, preemptively fail it here. See HttpConnection.SelectProtocol().
            if (options.HttpProtocols == HttpProtocols.Http2)
            {
                if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
                {
                    throw new NotSupportedException(CoreStrings.HTTP2NoTlsOsx);
                }
-                else if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && Environment.OSVersion.Version < new Version(6, 2))


No reason to allow it on Win8 without ALPN, it's the same as Win7.

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

.azure/pipelines/ci.yml

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

This applies to any versions strictly older than Windows 10 or Windows Server 2016

Co-authored-by: Chris Ross <[email protected]>

…are.cs Co-authored-by: Chris Ross <[email protected]>

…ionMiddlewareTests.cs Co-authored-by: Chris Ross <[email protected]>

ulrichb · 2020-09-26T14:50:14Z

Could you downport this to the 3.1 LTS?

Tratcher · 2020-09-26T16:45:41Z

@ulrichb the workaround is to disable HTTP/2. Issues with a simple workaround don't usually qualify for downports.
https://docs.microsoft.com/en-us/aspnet/core/fundamentals/servers/kestrel?view=aspnetcore-3.1#listenoptionsprotocols

ghost added the area-servers label Jun 12, 2020

JunTaoLuo commented Jun 12, 2020

View reviewed changes

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs Outdated Show resolved Hide resolved

JunTaoLuo commented Jun 12, 2020

View reviewed changes

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs Outdated Show resolved Hide resolved

Tratcher reviewed Jun 12, 2020

View reviewed changes

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs Outdated Show resolved Hide resolved

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs Outdated Show resolved Hide resolved

src/Servers/Kestrel/Core/src/CoreStrings.resx Outdated Show resolved Hide resolved

JunTaoLuo force-pushed the johluo/http2-downgrade branch 2 times, most recently from 687d225 to c637623 Compare June 13, 2020 18:37

JunTaoLuo marked this pull request as ready for review June 13, 2020 18:37

JunTaoLuo requested review from halter73 and jkotalik as code owners June 13, 2020 18:37

JunTaoLuo mentioned this pull request Jun 13, 2020

Fix helix-matrix tests #22918

Merged

JunTaoLuo requested a review from Tratcher June 15, 2020 17:52

Tratcher reviewed Jun 15, 2020

View reviewed changes

JunTaoLuo commented Jun 15, 2020

View reviewed changes

.azure/pipelines/ci.yml Outdated Show resolved Hide resolved

halter73 reviewed Jun 16, 2020

View reviewed changes

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs Outdated Show resolved Hide resolved

JamesNK reviewed Jun 16, 2020

View reviewed changes

JunTaoLuo force-pushed the johluo/http2-downgrade branch from cc38498 to c883fb2 Compare June 16, 2020 02:59

jaredpar mentioned this pull request Jun 16, 2020

OSX machines are de-provisioned during CI / PR runs leading to failures dotnet/runtime#34472

Closed

Tratcher approved these changes Jun 16, 2020

View reviewed changes

halter73 approved these changes Jun 16, 2020

View reviewed changes

JamesNK approved these changes Jun 16, 2020

View reviewed changes

John Luo added 2 commits June 16, 2020 15:56

Downgrade to HTTP/1.1 on older Windows versions

74252ca

This applies to any versions strictly older than Windows 10 or Windows Server 2016

Extract logger extensions

0f7025f

John Luo and others added 10 commits June 16, 2020 15:57

Update src/Servers/Kestrel/Core/src/CoreStrings.resx

8943fd4

Co-authored-by: Chris Ross <[email protected]>

Update src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddlew…

b80d492

…are.cs Co-authored-by: Chris Ross <[email protected]>

Feedback

b6e7762

Add tests

c975412

Fix affected tests

abb36a0

Update src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnect…

d487550

…ionMiddlewareTests.cs Co-authored-by: Chris Ross <[email protected]>

Wip

8f8c286

Feedback

4124533

Comments

2b8034b

Turn off helix matrix

6f56163

JunTaoLuo force-pushed the johluo/http2-downgrade branch from 2385f27 to 6f56163 Compare June 16, 2020 22:58

JunTaoLuo merged commit 45e6571 into master Jun 17, 2020

JunTaoLuo deleted the johluo/http2-downgrade branch June 17, 2020 23:50

JunTaoLuo mentioned this pull request Jun 18, 2020

Fix Windows 7 Kestrel tests #23069

Merged

Tratcher added this to the 5.0.0-preview7 milestone Sep 26, 2020

amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023

Downgrade to HTTP/1.1 on older Windows versions #22859

Downgrade to HTTP/1.1 on older Windows versions #22859

Uh oh!

Conversation

JunTaoLuo commented Jun 12, 2020

Uh oh!

Uh oh!

Uh oh!

JunTaoLuo commented Jun 12, 2020

Uh oh!

Tratcher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JunTaoLuo commented Jun 12, 2020

Uh oh!

HaoK commented Jun 12, 2020

Uh oh!

JunTaoLuo commented Jun 12, 2020

Uh oh!

JunTaoLuo commented Jun 13, 2020

Uh oh!

JunTaoLuo commented Jun 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

halter73 Jun 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ulrichb commented Sep 26, 2020

Uh oh!

Tratcher commented Sep 26, 2020

Uh oh!

Uh oh!

JunTaoLuo commented Jun 13, 2020 •

edited

Loading

halter73 Jun 16, 2020 •

edited

Loading