fix(comm): require TLS 1.3 minimum for websocket p2p client connections - #1631
Conversation
|
Hey @adecaro 👋 thanks for the PR! This comment updates automatically as you push changes -- think of it as your PR's live scoreboard! PR Checks✅ DCO Sign-off -- All commits have valid sign-offs. Nice work! ❌ GPG Signature -- Heads up! The following commits don't have a verified GPG signature:
You'll need to sign your commits with GPG (e.g. ✅ Merge Conflicts -- No merge conflicts detected. Smooth sailing! ✅ Issue Link -- Linked to #1630 (assigned to you). ⏳ All checks must pass before this PR can be reviewed. You've got this! |
github-actions
Bot
added
the
status: needs revision
mbrandenburger
left a comment
mbrandenburger
left a comment
There was a problem hiding this comment.
Thank you @adecaro for this PR.
| clientTLSConfig := &tls.Config{ | ||
| MinVersion: tls.VersionTLS12, | ||
| MinVersion: tls.VersionTLS13, |
There was a problem hiding this comment.
We probably should add a negative handshake test where the client indeed tries to connect with TLS12 and gets rejected. WDYT?
There was a problem hiding this comment.
Good call — added TestRejectsTLS12ClientHandshake in 88a879c, which forces the client's MinVersion/MaxVersion down to TLS 1.2 and asserts the handshake against the server fails with protocol version not supported, the server-side callback is never invoked, and no client connection is registered.
Addresses review feedback on #1631: verify that a client which only offers TLS 1.2 fails the handshake against the websocket p2p server, which now pins MinVersion to TLS 1.3 on both sides. Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
Addresses review feedback on #1631: verify that a client which only offers TLS 1.2 fails the handshake against the websocket p2p server, which now pins MinVersion to TLS 1.3 on both sides. Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
88a879c to
b2c81dc
Compare
The websocket server TLS config already pinned MinVersion to TLS 1.3, but the client config still allowed negotiating down to TLS 1.2. Align the client with the server so both sides of the mTLS handshake reject anything below TLS 1.3, and update the docs and test fixtures to match. Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
Addresses review feedback on #1631: verify that a client which only offers TLS 1.2 fails the handshake against the websocket p2p server, which now pins MinVersion to TLS 1.3 on both sides. Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>
b2c81dc to
eeecbfd
Compare
|
Tick the box to add this pull request to the merge queue (same as
|
2 participants
Summary
MinVersion: tls.VersionTLS13, but the client config still allowed negotiating down totls.VersionTLS12. This aligns the client with the server so both sides of the mTLS handshake reject anything below TLS 1.3.wspackage test fixtures (multiplexed_provider_test.go) that also allowed TLS 1.2 as a minimum, for consistency with the enforced policy.docs/platform/view/services/comm/websocket.md.Fixes #1630
Test plan
go build ./platform/view/services/comm/host/websocket/...go vet ./platform/view/services/comm/host/websocket/...go test ./platform/view/services/comm/host/websocket/...(bothwebsocketandwebsocket/wspackages pass)