From 7ba996874b541aa13b6bf1d1174b97372e0de20d Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Fri, 16 May 2025 13:08:16 -0400 Subject: [PATCH] crypto/tls: verify server chooses advertised curve When a crypto/tls client using TLS < 1.3 sends supported elliptic_curves in a client hello message the server must limit itself to choosing one of the supported options from our message. If we process a server key exchange message that chooses an unadvertised curve, abort the handshake w/ an error. Previously we would not note that the server chose a curve we didn't include in the client hello message, and would proceed with the handshake as long as the chosen curve was one that we've implemented. However, RFC 8422 5.1 makes it clear this is a server acting out-of-spec, as it says: If a server does not understand the Supported Elliptic Curves Extension, does not understand the Supported Point Formats Extension, or is unable to complete the ECC handshake while restricting itself to the enumerated curves and point formats, it MUST NOT negotiate the use of an ECC cipher suite. Changing our behaviour to enforce this also allows enabling the UnsupportedCurve BoGo test. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5cc Reviewed-on: https://go-review.googlesource.com/c/go/+/673735 TryBot-Bypass: Daniel McCarney Reviewed-by: David Chase Auto-Submit: Daniel McCarney Reviewed-by: Filippo Valsorda Reviewed-by: Roland Shoemaker --- src/crypto/tls/bogo_config.json | 1 - src/crypto/tls/key_agreement.go | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/crypto/tls/bogo_config.json b/src/crypto/tls/bogo_config.json index 191f48fc02..ba1dce8761 100644 --- a/src/crypto/tls/bogo_config.json +++ b/src/crypto/tls/bogo_config.json @@ -62,7 +62,6 @@ "BadRSAClientKeyExchange-5": "crypto/tls doesn't check the version number in the premaster secret - see processClientKeyExchange comment", "CheckLeafCurve": "TODO: first pass, this should be fixed", "DisabledCurve-HelloRetryRequest-TLS13": "TODO: first pass, this should be fixed", - "UnsupportedCurve": "TODO: first pass, this should be fixed", "SupportTicketsWithSessionID": "TODO: first pass, this should be fixed", "NoNullCompression-TLS12": "TODO: first pass, this should be fixed", "KeyUpdate-RequestACK": "TODO: first pass, this should be fixed", diff --git a/src/crypto/tls/key_agreement.go b/src/crypto/tls/key_agreement.go index 3e96242b97..d41bf43591 100644 --- a/src/crypto/tls/key_agreement.go +++ b/src/crypto/tls/key_agreement.go @@ -14,6 +14,7 @@ import ( "errors" "fmt" "io" + "slices" ) // A keyAgreement implements the client and server side of a TLS 1.0–1.2 key @@ -293,6 +294,10 @@ func (ka *ecdheKeyAgreement) processServerKeyExchange(config *Config, clientHell return errServerKeyExchange } + if !slices.Contains(clientHello.supportedCurves, curveID) { + return errors.New("tls: server selected unoffered curve") + } + if _, ok := curveForCurveID(curveID); !ok { return errors.New("tls: server selected unsupported curve") } -- 2.50.0