crypto/tls: let Config.CurvePreferences override GODEBUG options

tlsmlkem=0 and tlssecpmlkem=0 were never meant to forcibly disable PQ
KEMs, they were only meant to restore the Go 1.24 and Go 1.26 defaults
when Config.CurvePreferences is nil.

I noticed this while struggling to add a non-default key exchange.

While at it, make our behavior on unimplemented Config.CurvePreferences
entries more consistent by ignoring them regardless of role.

Udpates #69985
Updates #71206

Change-Id: I7d977282153b1d95fdb549efa92353e86a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/777220
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>

doc/next/6-stdlib/99-minor/crypto/tls/tlsmlkem.md

@@ -0,0 +1,4 @@
+Post-quantum hybrid key exchanges can now be explicitly enabled in
+[Config.CurvePreferences] even if the `tlsmlkem=0` or `tlssecpmlkem=0` GODEBUG
+options are used. Those options were always meant to only apply to the default
+set used when [Config.CurvePreferences] is nil.

src/crypto/tls/common.go

@@ -1276,25 +1276,32 @@ func supportedVersionsFromMax(maxVersion uint16) []uint16 {
 }
 
 func (c *Config) curvePreferences(version uint16) []CurveID {
-	curvePreferences := defaultCurvePreferences()
-	if fips140tls.Required() {
-		curvePreferences = slices.DeleteFunc(curvePreferences, func(x CurveID) bool {
-			return !slices.Contains(allowedCurvePreferencesFIPS, x)
-		})
-	}
-	if c != nil && len(c.CurvePreferences) != 0 {
-		curvePreferences = slices.DeleteFunc(curvePreferences, func(x CurveID) bool {
-			return !slices.Contains(c.CurvePreferences, x)
-		})
-	}
-	if version < VersionTLS13 {
-		curvePreferences = slices.DeleteFunc(curvePreferences, isTLS13OnlyKeyExchange)
-	}
-	return curvePreferences
+	return slices.DeleteFunc(curvePreferenceOrder(), func(x CurveID) bool {
+		return !c.supportsCurve(version, x)
+	})
 }
 
-func (c *Config) supportsCurve(version uint16, curve CurveID) bool {
-	return slices.Contains(c.curvePreferences(version), curve)
+func (c *Config) supportsCurve(version uint16, x CurveID) bool {
+	if c != nil && len(c.CurvePreferences) != 0 {
+		if !slices.Contains(c.CurvePreferences, x) {
+			return false
+		}
+		// Ignore unimplemented entries in c.CurvePreferences.
+		if !slices.Contains(curvePreferenceOrder(), x) {
+			return false
+		}
+	} else {
+		if !defaultCurveEnabled(x) {
+			return false
+		}
+	}
+	if fips140tls.Required() && !slices.Contains(allowedCurvePreferencesFIPS, x) {
+		return false
+	}
+	if version < VersionTLS13 && isTLS13OnlyKeyExchange(x) {
+		return false
+	}
+	return true
 }
 
 // mutualVersion returns the protocol version to use given the advertised

src/crypto/tls/defaults.go

@@ -13,24 +13,32 @@ import (
 // Defaults are collected in this file to allow distributions to more easily patch
 // them to apply local policies.
 
+// tlsmlkem=0 restores the pre-Go 1.24 default key exchanges.
 var tlsmlkem = godebug.New("tlsmlkem")
+
+// tlssecpmlkem=0 restores the pre-Go 1.26 default key exchanges.
 var tlssecpmlkem = godebug.New("tlssecpmlkem")
 
-// defaultCurvePreferences is the default set of supported key exchanges, as
-// well as the preference order.
-func defaultCurvePreferences() []CurveID {
-	switch {
-	// tlsmlkem=0 restores the pre-Go 1.24 default.
-	case tlsmlkem.Value() == "0":
-		return []CurveID{X25519, CurveP256, CurveP384, CurveP521}
-	// tlssecpmlkem=0 restores the pre-Go 1.26 default.
-	case tlssecpmlkem.Value() == "0":
-		return []CurveID{X25519MLKEM768, X25519, CurveP256, CurveP384, CurveP521}
+// defaultCurveEnabled returns whether the key exchange c is enabled by default.
+func defaultCurveEnabled(c CurveID) bool {
+	switch c {
+	case X25519, CurveP256, CurveP384, CurveP521:
+		return true
+	case X25519MLKEM768:
+		return tlsmlkem.Value() != "0"
+	case SecP256r1MLKEM768, SecP384r1MLKEM1024:
+		return tlsmlkem.Value() != "0" && tlssecpmlkem.Value() != "0"
 	default:
-		return []CurveID{
-			X25519MLKEM768, SecP256r1MLKEM768, SecP384r1MLKEM1024,
-			X25519, CurveP256, CurveP384, CurveP521,
-		}
+		return false
+	}
+}
+
+// curvePreferenceOrder is the fixed preference order of key exchanges. It must
+// include every supported key exchange.
+func curvePreferenceOrder() []CurveID {
+	return []CurveID{
+		X25519MLKEM768, SecP256r1MLKEM768, SecP384r1MLKEM1024,
+		X25519, CurveP256, CurveP384, CurveP521,
 	}
 }
 

src/crypto/tls/fips140_test.go

@@ -233,7 +233,7 @@ func TestFIPSServerCipherSuites(t *testing.T) {
 }
 
 func TestFIPSServerCurves(t *testing.T) {
-	for _, curveid := range defaultCurvePreferences() {
+	for _, curveid := range curvePreferenceOrder() {
 		t.Run(fmt.Sprintf("curve=%v", curveid), func(t *testing.T) {
 			testConfig := testConfigFIPS140.Clone()
 			testConfig.CurvePreferences = []CurveID{curveid}
@@ -342,7 +342,7 @@ func testFIPSClientHello(t *testing.T) {
 	clientConfig.MinVersion = VersionSSL30
 	clientConfig.MaxVersion = VersionTLS13
 	clientConfig.CipherSuites = allCipherSuitesIncludingTLS13()
-	clientConfig.CurvePreferences = defaultCurvePreferences()
+	clientConfig.CurvePreferences = curvePreferenceOrder()
 
 	go Client(c, clientConfig).Handshake()
 	srv := Server(s, testConfigFIPS140)

src/crypto/tls/handshake_client.go

@@ -148,7 +148,7 @@ func (c *Conn) makeClientHello() (*clientHelloMsg, *keySharePrivateKeys, *echCli
 		curveID := hello.supportedCurves[0]
 		ke, err := keyExchangeForCurveID(curveID)
 		if err != nil {
-			return nil, nil, nil, errors.New("tls: CurvePreferences includes unsupported curve")
+			return nil, nil, nil, errors.New("tls: internal error: supportsCurve accepted unimplemented curve")
 		}
 		keyShareKeys, hello.keyShares, err = ke.keyShares(config.rand())
 		if err != nil {

src/crypto/tls/handshake_client_tls13.go

@@ -322,7 +322,7 @@ func (hs *clientHandshakeStateTLS13) processHelloRetryRequest() error {
 		ke, err := keyExchangeForCurveID(curveID)
 		if err != nil {
 			c.sendAlert(alertInternalError)
-			return errors.New("tls: CurvePreferences includes unsupported curve")
+			return errors.New("tls: internal error: supportsCurve accepted unimplemented curve")
 		}
 		hs.keyShareKeys, hello.keyShares, err = ke.keyShares(c.config.rand())
 		if err != nil {

src/crypto/tls/handshake_server_tls13.go

@@ -249,7 +249,7 @@ func (hs *serverHandshakeStateTLS13) processClientHello() error {
 	ke, err := keyExchangeForCurveID(selectedGroup)
 	if err != nil {
 		c.sendAlert(alertInternalError)
-		return errors.New("tls: CurvePreferences includes unsupported curve")
+		return errors.New("tls: internal error: supportsCurve accepted unimplemented curve")
 	}
 	hs.sharedKey, hs.hello.serverShare, err = ke.serverSharedSecret(c.config.rand(), clientKeyShare.data)
 	if err != nil {

src/crypto/tls/key_agreement.go

@@ -167,7 +167,7 @@ func (ka *ecdheKeyAgreement) generateServerKeyExchange(config *Config, cert *Cer
 		return nil, errors.New("tls: no supported elliptic curves offered")
 	}
 	if _, ok := curveForCurveID(ka.curveID); !ok {
-		return nil, errors.New("tls: CurvePreferences includes unsupported curve")
+		return nil, errors.New("tls: internal error: supportsCurve accepted unimplemented curve")
 	}
 
 	key, err := generateECDHEKey(config.rand(), ka.curveID)

src/crypto/tls/tls_test.go

@@ -2156,6 +2156,21 @@ func TestHandshakeMLKEM(t *testing.T) {
 			expectClient:   []CurveID{X25519MLKEM768, X25519, CurveP256, CurveP384, CurveP521},
 			expectSelected: X25519MLKEM768,
 		},
+		{
+			name: "CurvePreferences override GODEBUG",
+			preparation: func(t *testing.T) {
+				testenv.SetGODEBUG(t, "tlsmlkem=0")
+				testenv.SetGODEBUG(t, "tlssecpmlkem=0")
+			},
+			clientConfig: func(config *Config) {
+				config.CurvePreferences = []CurveID{CurveP256, SecP256r1MLKEM768}
+			},
+			serverConfig: func(config *Config) {
+				config.CurvePreferences = []CurveID{CurveP256, SecP256r1MLKEM768}
+			},
+			expectClient:   []CurveID{SecP256r1MLKEM768, CurveP256},
+			expectSelected: SecP256r1MLKEM768,
+		},
 	}
 
 	baseServerConfig := testConfigServer.Clone()