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

Force secure WebSocket connections to use http/1.1 #10754

Merged
merged 5 commits into from Apr 25, 2024
Merged

Conversation

jeremyg484
Copy link
Contributor

@jeremyg484 jeremyg484 commented Apr 23, 2024

ConnectionManager is updated to use a separate SSLContext for WebSocket client connections that will only advertise http/1.1 in the list of supported protocols in the ALPN section of the TLS handshake.

WebSocket is not currently supported over HTTP 2 connections, thus if an HTTP 2 connection is established through ALPN, the subsequent upgrade to the WebSocket protocol would fail.

This resolves #10744

Connection manager is updated to use a separate SSLContext for
WebSocket connections that will only advertise http/1.1 in the list
of supported protocols in the ALPN section of the TLS handshake.

WebSocket is not currently supported over HTTP 2 connections, thus
if an HTTP 2 connection is established through ALPN, the subsequent
upgrade to the WebSocket protocol would fail.

This resolves #10744
@jeremyg484 jeremyg484 self-assigned this Apr 23, 2024
@jeremyg484 jeremyg484 requested a review from yawkat April 24, 2024 11:50
Copy link
Member

@yawkat yawkat left a comment

Choose a reason for hiding this comment

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

Actually no. we need the release() call

@jeremyg484
Copy link
Contributor Author

Actually no. we need the release() call

@yawkat Ok, so I've split the difference and gone back to making the context a volatile field, but lazily initialized in buildWebsocketSslContext with double-checked locking. This allows it to be explicitly released as necessary in the same manner as the main SslContext.

Copy link

sonarcloud bot commented Apr 24, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
1 New Bugs (required ≤ 0)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@yawkat
Copy link
Member

yawkat commented Apr 25, 2024

thanks!

@yawkat yawkat merged commit dc3e41f into 4.4.x Apr 25, 2024
16 of 17 checks passed
@yawkat yawkat deleted the wss-connection branch April 25, 2024 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants