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

Proritize HTTP/1.1 over HTTP/2. #17886

Merged
merged 2 commits into from
Nov 1, 2022

Conversation

mdwn
Copy link
Contributor

@mdwn mdwn commented Oct 27, 2022

Due to a bug in Chrome, secure websockets in Teleport work intermittently on the Chrome browser when using HTTP/2. Prioritizing HTTP/1.1 fixes this issue, though the reasons for this are not 100% clear. When or if https://bugs.chromium.org/p/chromium/issues/detail?id=1379017 is resolved, we should be able to revert this. When golang/go#49918 is implemented, we may be able to revert this if enabling HTTP/2 websockets fixes the issue on our end.

Should address #16031.

Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

@mdwn I think we should test Kubernetes Access and make sure to prefer h2 for it - our ALPN router relies on Kube Access requests having server name prefixed with "kube." so we should be able to utilize GetConfigForClient? cc @tigrato

@mdwn
Copy link
Contributor Author

mdwn commented Oct 27, 2022

@mdwn I think we should test Kubernetes Access and make sure to prefer h2 for it - our ALPN router relies on Kube Access requests having server name prefixed with "kube." so we should be able to utilize GetConfigForClient? cc @tigrato

I'm working on testing/implementing this as we speak.

@mdwn
Copy link
Contributor Author

mdwn commented Oct 27, 2022

@r0mant I don't believe anything is actually necessary here because the kube ALPN protocol uses ForwardTLS, skipping the prioritization configured within ALPN. With this PR as is, doing a kubectl get pods against minikube, I get the following in the kubernetes API server:

I1027 19:37:22.097113       1 trace.go:205] Trace[321091873]: "Get" url:/api/v1/namespaces/teleport-agent/pods/teleport-agent-0/log,user-agent:kubectl/v1.25.0 (darwin/arm64) kubernetes/a866cbe,audit-id:aaca85e9-82c3-4cb0-8b96-4e26f312cff0,client:172.17.0.1,accept:application/json, */*,protocol:HTTP/2.0 (27-Oct-2022 19:37:15.448) (total time: 6648ms):

Doing a raw curl of the kube endpoint gets:

➜  teleport git:(mike.wilson/prioritize-http11-instead-of-http2) ✗ curl -k https://kube.mw-test.teleport -v    
*   Trying 192.168.1.232:443...
* Connected to kube.mw-test.teleport (192.168.1.232) port 443 (#0)
* ALPN: offers h2
* ALPN: offers http/1.1
* (304) (OUT), TLS handshake, Client hello (1):
...
* ALPN: server accepted h2
...
* Using HTTP2, server supports multiplexing
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
...
> GET / HTTP/2

Additionally, doing a curl of the front page of Teleport gets us:

➜  teleport git:(mike.wilson/prioritize-http11-instead-of-http2) ✗ curl -k https://mw-test.teleport -v 
*   Trying 192.168.1.232:443...
* Connected to mw-test.teleport (192.168.1.232) port 443 (#0)
* ALPN: offers h2
* ALPN: offers http/1.1
...
* ALPN: server accepted http/1.1
...
> GET / HTTP/1.1

@tigrato Does this sound right?

@mdwn mdwn force-pushed the mike.wilson/prioritize-http11-instead-of-http2 branch from d82eb37 to 0c05ecd Compare October 28, 2022 15:43
@mdwn mdwn force-pushed the mike.wilson/prioritize-http11-instead-of-http2 branch 2 times, most recently from aa9592b to 673b399 Compare October 31, 2022 15:58
lib/service/service.go Outdated Show resolved Hide resolved
lib/srv/alpnproxy/common/protocols.go Outdated Show resolved Hide resolved
@mdwn mdwn enabled auto-merge (squash) November 1, 2022 02:26
@mdwn mdwn force-pushed the mike.wilson/prioritize-http11-instead-of-http2 branch from 484d0c9 to cd5c568 Compare November 1, 2022 02:26
Mike Wilson added 2 commits November 1, 2022 11:20
Due to a bug in Chrome, secure websockets in Teleport work intermittently on
the Chrome browser when using HTTP/2. Prioritizing HTTP/1.1 fixes this issue,
though the reasons for this are not 100% clear. When or if
https://bugs.chromium.org/p/chromium/issues/detail?id=1379017 is resolved, we
should be able to revert this. When golang/go#49918
is implemented, we may be able to revert this if enabling HTTP/2 websockets
fixes the issue on our end.
@mdwn mdwn force-pushed the mike.wilson/prioritize-http11-instead-of-http2 branch from cd5c568 to 0dbf0db Compare November 1, 2022 15:20
@github-actions github-actions bot removed the request for review from xacrimon November 1, 2022 15:20
@mdwn mdwn merged commit fb3a336 into master Nov 1, 2022
@github-actions
Copy link

github-actions bot commented Nov 1, 2022

@mdwn See the table below for backport results.

Branch Result
branch/v10 Create PR
branch/v11 Create PR
branch/v9 Failed

@zmb3 zmb3 deleted the mike.wilson/prioritize-http11-instead-of-http2 branch May 7, 2024 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants