From de0813ca8889aa98638ed851605bdc0aef3e2687 Mon Sep 17 00:00:00 2001 From: yuhan6665 <1588741+yuhan6665@users.noreply.github.com> Date: Thu, 10 Sep 2026 21:54:16 -0400 Subject: [PATCH] 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 Reviewed-by: Roland Shoemaker LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: David Chase --- common.go | 41 +++++++++++++++++++++++---------------- defaults.go | 37 ++++++++++++++++++++++------------- handshake_client.go | 2 +- handshake_client_tls13.go | 2 +- handshake_server_tls13.go | 2 +- key_agreement.go | 2 +- 6 files changed, 51 insertions(+), 35 deletions(-) diff --git a/common.go b/common.go index 67c6994..dc4adae 100644 --- a/common.go +++ b/common.go @@ -1310,25 +1310,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 diff --git a/defaults.go b/defaults.go index 5c03801..17ab1d6 100644 --- a/defaults.go +++ b/defaults.go @@ -12,24 +12,33 @@ 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 true//tlsmlkem.Value() != "0" + case SecP256r1MLKEM768, SecP384r1MLKEM1024: + return true//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, } } diff --git a/handshake_client.go b/handshake_client.go index 22aed23..94cd56a 100644 --- a/handshake_client.go +++ b/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 { diff --git a/handshake_client_tls13.go b/handshake_client_tls13.go index 0084dc7..e6db8e9 100644 --- a/handshake_client_tls13.go +++ b/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 { diff --git a/handshake_server_tls13.go b/handshake_server_tls13.go index 673843f..7ae0f6b 100644 --- a/handshake_server_tls13.go +++ b/handshake_server_tls13.go @@ -343,7 +343,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 { diff --git a/key_agreement.go b/key_agreement.go index 220df9a..099cf88 100644 --- a/key_agreement.go +++ b/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)