Repository navigation
Conversation
ApplyPreset generated a key for every classical key share a spec offered but kept only the first as KeyShareKeys.Ecdhe, and the client's key exchange used that key for whichever group the server selected. Firefox offers X25519 and P-256 shares, and the Firefox 105, 120 and 148 parrots follow it, so a server selecting P-256 made the client derive the shared secret with an X25519 key and the handshake failed with "invalid server key share" on the client and a bad record MAC on the server, with or without ECH. Keep every classical share's key by group in KeyShareKeys.Ecdhes, Ecdhe staying the first one, and let the ECDHE key exchange use the key for its own group when there is one. Keys generated by the Go path or by a HelloRetryRequest carry no map and behave as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ApplyPresetgenerates a key for every classical key share a spec offers but keeps only the first one, asKeyShareKeys.Ecdhe("only do this once for the first non-grease curve"), andecdhKeyExchange.clientSharedSecretuses that key for whichever group the server selected. Firefox offers X25519 and P-256 shares alongside its hybrid one (the Firefox 158 capture in #427 showsX25519MLKEM768,X25519,P-256), and the Firefox 105, 120 and 148 parrots do the same. A server that prefers P-256 selects it directly, without a HelloRetryRequest, and the client then derives the shared secret with its X25519 key: the client fails withtls: invalid server key share(the ECDH curves do not match) and the server with a bad record MAC. This happens with or without ECH; it came up while testing ECH with a HelloRetryRequest against a P-256-only server.This change keeps the key of every classical share, by group, in a new
KeyShareKeys.Ecdhesmap, withEcdhestill the first one as before, and makes the ECDHE key exchange use the key for its own group when the map has one. Keys generated by the Go path or regenerated for a HelloRetryRequest carry no map and behave exactly as before. The hybrid key exchange, including the Firefox 148 classical-key reuse, is unchanged.The test runs the three Firefox parrots and a hand-written spec with X25519, P-256 and P-384 shares against a server preferring each of their classical groups in turn, and checks the handshake completes with that group and no retry. Before the change every share but the first failed.
Verified with Go 1.27.3:
go test ./...,go test -raceon the key share tests,go vet,gofmt -s,go fix -diff, staticcheck and golangci-lint report nothing.