crypto/tls: implement X25519Kyber768Draft00

Forced the testConfig CurvePreferences to exclude X25519Kyber768Draft00
to avoid bloating the transcripts, but I manually tested it and the
tests all update and pass successfully, causing 7436 insertions(+), 3251
deletions(-).

Fixes #67061

Change-Id: If6f13bca561835777ab0889a490487b7c2366c3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/586656
Auto-Submit: Filippo Valsorda <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: FiloSottile

doc/godebug.md

@@ -179,6 +179,10 @@ Previous versions default to `winreadlinkvolume=0`.
 Go 1.23 corrected the semantics of contention reports for runtime-internal locks,
 and so removed the [`runtimecontentionstacks` setting](/pkg/runtime#hdr-Environment_Variable).
 
+Go 1.23 enabled the experimental post-quantum key exchange mechanism
+X25519Kyber768Draft00 by default. The default can be reverted using the
+[`tlskyber` setting](/pkg/crypto/tls/#Config.CurvePreferences).
+
 ### Go 1.22
 
 Go 1.22 adds a configurable limit to control the maximum acceptable RSA key size

src/crypto/tls/bogo_config.json

@@ -6,6 +6,10 @@
         "SendEmptyRecords*": "crypto/tls doesn't implement spam protections",
         "SendWarningAlerts*": "crypto/tls doesn't implement spam protections",
         "TooManyKeyUpdates": "crypto/tls doesn't implement spam protections (TODO: I think?)",
+        "KyberNotEnabledByDefaultInClients": "crypto/tls intentionally enables it",
+        "JustConfiguringKyberWorks": "we always send a X25519 key share with Kyber",
+        "KyberKeyShareIncludedSecond": "we always send the Kyber key share first",
+        "KyberKeyShareIncludedThird": "we always send the Kyber key share first",
         "SkipNewSessionTicket": "TODO confusing? maybe bug",
         "SendUserCanceledAlerts*": "TODO may be a real bug?",
         "GREASE-Server-TLS13": "TODO ???",
@@ -14,6 +18,12 @@
         "EchoTLS13CompatibilitySessionID": "TODO reject compat session ID",
         "*ECH-Server*": "no ECH server support",
         "TLS-ECH-Client-UnsolictedHRRExtension": "TODO",
+        "*Client-P-224*": "no P-224 support",
+        "*Server-P-224*": "no P-224 support",
+        "CurveID-Resume*": "unexposed curveID is not stored in the ticket yet",
+        "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",
@@ -172,4 +182,4 @@
         "CheckClientCertificateTypes": "TODO: first pass, this should be fixed",
         "CheckECDSACurve-TLS12": "TODO: first pass, this should be fixed"
     }
-}
+}

src/crypto/tls/bogo_shim_test.go

@@ -15,6 +15,8 @@ import (
 	"os/exec"
 	"path/filepath"
 	"runtime"
+	"strconv"
+	"strings"
 	"testing"
 )
 
@@ -40,6 +42,9 @@ var (
 
 	resumeCount = flag.Int("resume-count", 0, "")
 
+	curves        = flagStringSlice("curves", "")
+	expectedCurve = flag.String("expect-curve-id", "", "")
+
 	shimID = flag.Uint64("shim-id", 0, "")
 	_      = flag.Bool("ipv6", false, "")
 
@@ -132,6 +137,23 @@ var (
 	// -verify-prefs
 )
 
+type stringSlice []string
+
+func flagStringSlice(name, usage string) *stringSlice {
+	f := &stringSlice{}
+	flag.Var(f, name, usage)
+	return f
+}
+
+func (saf stringSlice) String() string {
+	return strings.Join(saf, ",")
+}
+
+func (saf stringSlice) Set(s string) error {
+	saf = append(saf, s)
+	return nil
+}
+
 func bogoShim() {
 	if *isHandshakerSupported {
 		fmt.Println("No")
@@ -177,6 +199,16 @@ func bogoShim() {
 		cfg.ClientAuth = RequireAnyClientCert
 	}
 
+	if len(*curves) != 0 {
+		for _, curveStr := range *curves {
+			id, err := strconv.Atoi(curveStr)
+			if err != nil {
+				log.Fatalf("failed to parse curve id %q: %s", curveStr, err)
+			}
+			cfg.CurvePreferences = append(cfg.CurvePreferences, CurveID(id))
+		}
+	}
+
 	for i := 0; i < *resumeCount+1; i++ {
 		conn, err := net.Dial("tcp", net.JoinHostPort("localhost", *port))
 		if err != nil {
@@ -221,6 +253,16 @@ func bogoShim() {
 				log.Fatalf("write err: %s", err)
 			}
 		}
+
+		if *expectedCurve != "" {
+			expectedCurveID, err := strconv.Atoi(*expectedCurve)
+			if err != nil {
+				log.Fatalf("failed to parse -expect-curve-id: %s", err)
+			}
+			if tlsConn.curveID != CurveID(expectedCurveID) {
+				log.Fatalf("unexpected curve id: want %d, got %d", expectedCurveID, tlsConn.curveID)
+			}
+		}
 	}
 }
 
@@ -238,7 +280,7 @@ func TestBogoSuite(t *testing.T) {
 		t.Skip("#66913: windows network connections are flakey on builders")
 	}
 
-	const boringsslModVer = "v0.0.0-20240412155355-1c6e10495e4f"
+	const boringsslModVer = "v0.0.0-20240517213134-ba62c812f01f"
 	output, err := exec.Command("go", "mod", "download", "-json", "github.com/google/boringssl@"+boringsslModVer).CombinedOutput()
 	if err != nil {
 		t.Fatalf("failed to download boringssl: %s", err)
@@ -263,6 +305,8 @@ func TestBogoSuite(t *testing.T) {
 		"-shim-extra-flags=-bogo-mode",
 		"-allow-unimplemented",
 		"-loose-errors", // TODO(roland): this should be removed eventually
+		"-pipe",
+		"-v",
 	}
 	if *bogoFilter != "" {
 		args = append(args, fmt.Sprintf("-test=%s", *bogoFilter))
@@ -273,10 +317,22 @@ func TestBogoSuite(t *testing.T) {
 		t.Fatal(err)
 	}
 	cmd := exec.Command(goCmd, args...)
-	cmd.Stdout, cmd.Stderr = os.Stdout, os.Stderr
+	out := &strings.Builder{}
+	cmd.Stdout, cmd.Stderr = io.MultiWriter(os.Stdout, out), os.Stderr
 	cmd.Dir = filepath.Join(j.Dir, "ssl/test/runner")
 	err = cmd.Run()
 	if err != nil {
 		t.Fatalf("bogo failed: %s", err)
 	}
+
+	if *bogoFilter == "" {
+		assertPass := func(t *testing.T, name string) {
+			t.Helper()
+			if !strings.Contains(out.String(), "PASSED ("+name+")\n") {
+				t.Errorf("Expected test %s did not run", name)
+			}
+		}
+		assertPass(t, "CurveTest-Client-Kyber-TLS13")
+		assertPass(t, "CurveTest-Server-Kyber-TLS13")
+	}
 }

src/crypto/tls/boring_test.go

@@ -165,6 +165,10 @@ func TestBoringServerCurves(t *testing.T) {
 		t.Run(fmt.Sprintf("curve=%d", curveid), func(t *testing.T) {
 			clientConfig := testConfig.Clone()
 			clientConfig.CurvePreferences = []CurveID{curveid}
+			if curveid == x25519Kyber768Draft00 {
+				// x25519Kyber768Draft00 is not supported standalone.
+				clientConfig.CurvePreferences = append(clientConfig.CurvePreferences, X25519)
+			}
 			if _, _, err := testHandshake(t, clientConfig, serverConfig); err != nil {
 				t.Fatalf("got error: %v, expected success", err)
 			}

src/crypto/tls/common.go

@@ -130,18 +130,25 @@ const (
 	scsvRenegotiation uint16 = 0x00ff
 )
 
-// CurveID is the type of a TLS identifier for an elliptic curve. See
+// CurveID is the type of a TLS identifier for a key exchange mechanism. See
 // https://www.iana.org/assignments/tls-parameters/tls-parameters.xml#tls-parameters-8.
 //
-// In TLS 1.3, this type is called NamedGroup, but at this time this library
-// only supports Elliptic Curve based groups. See RFC 8446, Section 4.2.7.
+// In TLS 1.2, this registry used to support only elliptic curves. In TLS 1.3,
+// it was extended to other groups and renamed NamedGroup. See RFC 8446, Section
+// 4.2.7. It was then also extended to other mechanisms, such as hybrid
+// post-quantum KEMs.
 type CurveID uint16
 
 const (
 	CurveP256 CurveID = 23
 	CurveP384 CurveID = 24
 	CurveP521 CurveID = 25
 	X25519    CurveID = 29
+
+	// Experimental codepoint for X25519Kyber768Draft00, specified in
+	// draft-tls-westerbaan-xyber768d00-03. Not exported, as support might be
+	// removed in the future.
+	x25519Kyber768Draft00 CurveID = 0x6399 // X25519Kyber768Draft00
 )
 
 // TLS 1.3 Key Share. See RFC 8446, Section 4.2.8.
@@ -302,6 +309,10 @@ type ConnectionState struct {
 
 	// testingOnlyDidHRR is true if a HelloRetryRequest was sent/received.
 	testingOnlyDidHRR bool
+
+	// testingOnlyCurveID is the selected CurveID, or zero if an RSA exchanges
+	// is performed.
+	testingOnlyCurveID CurveID
 }
 
 // ExportKeyingMaterial returns length bytes of exported key material in a new
@@ -375,7 +386,7 @@ type ClientSessionCache interface {
 	Put(sessionKey string, cs *ClientSessionState)
 }
 
-//go:generate stringer -type=SignatureScheme,CurveID,ClientAuthType -output=common_string.go
+//go:generate stringer -linecomment -type=SignatureScheme,CurveID,ClientAuthType -output=common_string.go
 
 // SignatureScheme identifies a signature algorithm supported by TLS. See
 // RFC 8446, Section 4.2.3.
@@ -757,6 +768,10 @@ type Config struct {
 	// an ECDHE handshake, in preference order. If empty, the default will
 	// be used. The client will use the first preference as the type for
 	// its key share in TLS 1.3. This may change in the future.
+	//
+	// From Go 1.23, the default includes the X25519Kyber768Draft00 hybrid
+	// post-quantum key exchange. To disable it, set CurvePreferences explicitly
+	// or use the GODEBUG=tlskyber=0 environment variable.
 	CurvePreferences []CurveID
 
 	// DynamicRecordSizingDisabled disables adaptive sizing of TLS records.
@@ -1084,20 +1099,27 @@ func supportedVersionsFromMax(maxVersion uint16) []uint16 {
 	return versions
 }
 
-var defaultCurvePreferences = []CurveID{X25519, CurveP256, CurveP384, CurveP521}
+var tlskyber = godebug.New("tlskyber")
 
-func (c *Config) curvePreferences() []CurveID {
+var defaultCurvePreferences = []CurveID{x25519Kyber768Draft00, X25519, CurveP256, CurveP384, CurveP521}
+
+var defaultCurvePreferencesWithoutKyber = []CurveID{X25519, CurveP256, CurveP384, CurveP521}
+
+func (c *Config) curvePreferences(version uint16) []CurveID {
 	if needFIPS() {
 		return fipsCurvePreferences(c)
 	}
 	if c == nil || len(c.CurvePreferences) == 0 {
+		if version < VersionTLS13 || tlskyber.Value() == "0" {
+			return defaultCurvePreferencesWithoutKyber
+		}
 		return defaultCurvePreferences
 	}
 	return c.CurvePreferences
 }
 
-func (c *Config) supportsCurve(curve CurveID) bool {
-	for _, cc := range c.curvePreferences() {
+func (c *Config) supportsCurve(version uint16, curve CurveID) bool {
+	for _, cc := range c.curvePreferences(version) {
 		if cc == curve {
 			return true
 		}
@@ -1256,7 +1278,7 @@ func (chi *ClientHelloInfo) SupportsCertificate(c *Certificate) error {
 	}
 
 	// The only signed key exchange we support is ECDHE.
-	if !supportsECDHE(config, chi.SupportedCurves, chi.SupportedPoints) {
+	if !supportsECDHE(config, vers, chi.SupportedCurves, chi.SupportedPoints) {
 		return supportsRSAFallback(errors.New("client doesn't support ECDHE, can only use legacy RSA key exchange"))
 	}
 
@@ -1277,7 +1299,7 @@ func (chi *ClientHelloInfo) SupportsCertificate(c *Certificate) error {
 			}
 			var curveOk bool
 			for _, c := range chi.SupportedCurves {
-				if c == curve && config.supportsCurve(c) {
+				if c == curve && config.supportsCurve(vers, c) {
 					curveOk = true
 					break
 				}

src/crypto/tls/common_string.go

@@ -50,6 +50,7 @@ type Conn struct {
 	didResume        bool // whether this connection was a session resumption
 	didHRR           bool // whether a HelloRetryRequest was sent/received
 	cipherSuite      uint16
+	curveID          CurveID
 	ocspResponse     []byte   // stapled OCSP response
 	scts             [][]byte // signed certificate timestamps from server
 	peerCertificates []*x509.Certificate
@@ -1610,6 +1611,8 @@ func (c *Conn) connectionStateLocked() ConnectionState {
 	state.NegotiatedProtocol = c.clientProtocol
 	state.DidResume = c.didResume
 	state.testingOnlyDidHRR = c.didHRR
+	// c.curveID is not set on TLS 1.0–1.2 resumptions. Fix that before exposing it.
+	state.testingOnlyCurveID = c.curveID
 	state.NegotiatedProtocolIsMutual = true
 	state.ServerName = c.serverName
 	state.CipherSuite = c.cipherSuite