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 <daniel@binaryparadox.net>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>

diff --git a/src/crypto/tls/bogo_config.json b/src/crypto/tls/bogo_config.json
index 191f48fc02b9244e55285db31e5a09410a9b214e..ba1dce87610fc6dacf771ac978c75abb629b4eba 100644 (file)
--- 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 3e96242b9798b8cdab5c78cfe62372b6ff387e85..d41bf435910f9737097f5bcaf4989a60d64b9a6a 100644 (file)
--- 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")
        }