Skip to content

Commit d0edd9a

Browse files
FiloSottilegopherbot
authored andcommitted
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 <filippo@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent 7c52c06 commit d0edd9a

18 files changed

+498
-102
lines changed

doc/godebug.md

+4
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,10 @@ Previous versions default to `winreadlinkvolume=0`.
179179
Go 1.23 corrected the semantics of contention reports for runtime-internal locks,
180180
and so removed the [`runtimecontentionstacks` setting](/pkg/runtime#hdr-Environment_Variable).
181181

182+
Go 1.23 enabled the experimental post-quantum key exchange mechanism
183+
X25519Kyber768Draft00 by default. The default can be reverted using the
184+
[`tlskyber` setting](/pkg/crypto/tls/#Config.CurvePreferences).
185+
182186
### Go 1.22
183187

184188
Go 1.22 adds a configurable limit to control the maximum acceptable RSA key size

src/crypto/tls/bogo_config.json

+11-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
"SendEmptyRecords*": "crypto/tls doesn't implement spam protections",
77
"SendWarningAlerts*": "crypto/tls doesn't implement spam protections",
88
"TooManyKeyUpdates": "crypto/tls doesn't implement spam protections (TODO: I think?)",
9+
"KyberNotEnabledByDefaultInClients": "crypto/tls intentionally enables it",
10+
"JustConfiguringKyberWorks": "we always send a X25519 key share with Kyber",
11+
"KyberKeyShareIncludedSecond": "we always send the Kyber key share first",
12+
"KyberKeyShareIncludedThird": "we always send the Kyber key share first",
913
"SkipNewSessionTicket": "TODO confusing? maybe bug",
1014
"SendUserCanceledAlerts*": "TODO may be a real bug?",
1115
"GREASE-Server-TLS13": "TODO ???",
@@ -14,6 +18,12 @@
1418
"EchoTLS13CompatibilitySessionID": "TODO reject compat session ID",
1519
"*ECH-Server*": "no ECH server support",
1620
"TLS-ECH-Client-UnsolictedHRRExtension": "TODO",
21+
"*Client-P-224*": "no P-224 support",
22+
"*Server-P-224*": "no P-224 support",
23+
"CurveID-Resume*": "unexposed curveID is not stored in the ticket yet",
24+
"CheckLeafCurve": "TODO: first pass, this should be fixed",
25+
"DisabledCurve-HelloRetryRequest-TLS13": "TODO: first pass, this should be fixed",
26+
"UnsupportedCurve": "TODO: first pass, this should be fixed",
1727
"SupportTicketsWithSessionID": "TODO: first pass, this should be fixed",
1828
"NoNullCompression-TLS12": "TODO: first pass, this should be fixed",
1929
"KeyUpdate-RequestACK": "TODO: first pass, this should be fixed",
@@ -172,4 +182,4 @@
172182
"CheckClientCertificateTypes": "TODO: first pass, this should be fixed",
173183
"CheckECDSACurve-TLS12": "TODO: first pass, this should be fixed"
174184
}
175-
}
185+
}

src/crypto/tls/bogo_shim_test.go

+58-2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import (
1515
"os/exec"
1616
"path/filepath"
1717
"runtime"
18+
"strconv"
19+
"strings"
1820
"testing"
1921
)
2022

@@ -40,6 +42,9 @@ var (
4042

4143
resumeCount = flag.Int("resume-count", 0, "")
4244

45+
curves = flagStringSlice("curves", "")
46+
expectedCurve = flag.String("expect-curve-id", "", "")
47+
4348
shimID = flag.Uint64("shim-id", 0, "")
4449
_ = flag.Bool("ipv6", false, "")
4550

@@ -132,6 +137,23 @@ var (
132137
// -verify-prefs
133138
)
134139

140+
type stringSlice []string
141+
142+
func flagStringSlice(name, usage string) *stringSlice {
143+
f := &stringSlice{}
144+
flag.Var(f, name, usage)
145+
return f
146+
}
147+
148+
func (saf stringSlice) String() string {
149+
return strings.Join(saf, ",")
150+
}
151+
152+
func (saf stringSlice) Set(s string) error {
153+
saf = append(saf, s)
154+
return nil
155+
}
156+
135157
func bogoShim() {
136158
if *isHandshakerSupported {
137159
fmt.Println("No")
@@ -177,6 +199,16 @@ func bogoShim() {
177199
cfg.ClientAuth = RequireAnyClientCert
178200
}
179201

202+
if len(*curves) != 0 {
203+
for _, curveStr := range *curves {
204+
id, err := strconv.Atoi(curveStr)
205+
if err != nil {
206+
log.Fatalf("failed to parse curve id %q: %s", curveStr, err)
207+
}
208+
cfg.CurvePreferences = append(cfg.CurvePreferences, CurveID(id))
209+
}
210+
}
211+
180212
for i := 0; i < *resumeCount+1; i++ {
181213
conn, err := net.Dial("tcp", net.JoinHostPort("localhost", *port))
182214
if err != nil {
@@ -221,6 +253,16 @@ func bogoShim() {
221253
log.Fatalf("write err: %s", err)
222254
}
223255
}
256+
257+
if *expectedCurve != "" {
258+
expectedCurveID, err := strconv.Atoi(*expectedCurve)
259+
if err != nil {
260+
log.Fatalf("failed to parse -expect-curve-id: %s", err)
261+
}
262+
if tlsConn.curveID != CurveID(expectedCurveID) {
263+
log.Fatalf("unexpected curve id: want %d, got %d", expectedCurveID, tlsConn.curveID)
264+
}
265+
}
224266
}
225267
}
226268

@@ -238,7 +280,7 @@ func TestBogoSuite(t *testing.T) {
238280
t.Skip("#66913: windows network connections are flakey on builders")
239281
}
240282

241-
const boringsslModVer = "v0.0.0-20240412155355-1c6e10495e4f"
283+
const boringsslModVer = "v0.0.0-20240517213134-ba62c812f01f"
242284
output, err := exec.Command("go", "mod", "download", "-json", "github.com/google/boringssl@"+boringsslModVer).CombinedOutput()
243285
if err != nil {
244286
t.Fatalf("failed to download boringssl: %s", err)
@@ -263,6 +305,8 @@ func TestBogoSuite(t *testing.T) {
263305
"-shim-extra-flags=-bogo-mode",
264306
"-allow-unimplemented",
265307
"-loose-errors", // TODO(roland): this should be removed eventually
308+
"-pipe",
309+
"-v",
266310
}
267311
if *bogoFilter != "" {
268312
args = append(args, fmt.Sprintf("-test=%s", *bogoFilter))
@@ -273,10 +317,22 @@ func TestBogoSuite(t *testing.T) {
273317
t.Fatal(err)
274318
}
275319
cmd := exec.Command(goCmd, args...)
276-
cmd.Stdout, cmd.Stderr = os.Stdout, os.Stderr
320+
out := &strings.Builder{}
321+
cmd.Stdout, cmd.Stderr = io.MultiWriter(os.Stdout, out), os.Stderr
277322
cmd.Dir = filepath.Join(j.Dir, "ssl/test/runner")
278323
err = cmd.Run()
279324
if err != nil {
280325
t.Fatalf("bogo failed: %s", err)
281326
}
327+
328+
if *bogoFilter == "" {
329+
assertPass := func(t *testing.T, name string) {
330+
t.Helper()
331+
if !strings.Contains(out.String(), "PASSED ("+name+")\n") {
332+
t.Errorf("Expected test %s did not run", name)
333+
}
334+
}
335+
assertPass(t, "CurveTest-Client-Kyber-TLS13")
336+
assertPass(t, "CurveTest-Server-Kyber-TLS13")
337+
}
282338
}

src/crypto/tls/boring_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,10 @@ func TestBoringServerCurves(t *testing.T) {
165165
t.Run(fmt.Sprintf("curve=%d", curveid), func(t *testing.T) {
166166
clientConfig := testConfig.Clone()
167167
clientConfig.CurvePreferences = []CurveID{curveid}
168+
if curveid == x25519Kyber768Draft00 {
169+
// x25519Kyber768Draft00 is not supported standalone.
170+
clientConfig.CurvePreferences = append(clientConfig.CurvePreferences, X25519)
171+
}
168172
if _, _, err := testHandshake(t, clientConfig, serverConfig); err != nil {
169173
t.Fatalf("got error: %v, expected success", err)
170174
}

src/crypto/tls/common.go

+32-10
Original file line numberDiff line numberDiff line change
@@ -130,18 +130,25 @@ const (
130130
scsvRenegotiation uint16 = 0x00ff
131131
)
132132

133-
// CurveID is the type of a TLS identifier for an elliptic curve. See
133+
// CurveID is the type of a TLS identifier for a key exchange mechanism. See
134134
// https://www.iana.org/assignments/tls-parameters/tls-parameters.xml#tls-parameters-8.
135135
//
136-
// In TLS 1.3, this type is called NamedGroup, but at this time this library
137-
// only supports Elliptic Curve based groups. See RFC 8446, Section 4.2.7.
136+
// In TLS 1.2, this registry used to support only elliptic curves. In TLS 1.3,
137+
// it was extended to other groups and renamed NamedGroup. See RFC 8446, Section
138+
// 4.2.7. It was then also extended to other mechanisms, such as hybrid
139+
// post-quantum KEMs.
138140
type CurveID uint16
139141

140142
const (
141143
CurveP256 CurveID = 23
142144
CurveP384 CurveID = 24
143145
CurveP521 CurveID = 25
144146
X25519 CurveID = 29
147+
148+
// Experimental codepoint for X25519Kyber768Draft00, specified in
149+
// draft-tls-westerbaan-xyber768d00-03. Not exported, as support might be
150+
// removed in the future.
151+
x25519Kyber768Draft00 CurveID = 0x6399 // X25519Kyber768Draft00
145152
)
146153

147154
// TLS 1.3 Key Share. See RFC 8446, Section 4.2.8.
@@ -302,6 +309,10 @@ type ConnectionState struct {
302309

303310
// testingOnlyDidHRR is true if a HelloRetryRequest was sent/received.
304311
testingOnlyDidHRR bool
312+
313+
// testingOnlyCurveID is the selected CurveID, or zero if an RSA exchanges
314+
// is performed.
315+
testingOnlyCurveID CurveID
305316
}
306317

307318
// ExportKeyingMaterial returns length bytes of exported key material in a new
@@ -375,7 +386,7 @@ type ClientSessionCache interface {
375386
Put(sessionKey string, cs *ClientSessionState)
376387
}
377388

378-
//go:generate stringer -type=SignatureScheme,CurveID,ClientAuthType -output=common_string.go
389+
//go:generate stringer -linecomment -type=SignatureScheme,CurveID,ClientAuthType -output=common_string.go
379390

380391
// SignatureScheme identifies a signature algorithm supported by TLS. See
381392
// RFC 8446, Section 4.2.3.
@@ -757,6 +768,10 @@ type Config struct {
757768
// an ECDHE handshake, in preference order. If empty, the default will
758769
// be used. The client will use the first preference as the type for
759770
// its key share in TLS 1.3. This may change in the future.
771+
//
772+
// From Go 1.23, the default includes the X25519Kyber768Draft00 hybrid
773+
// post-quantum key exchange. To disable it, set CurvePreferences explicitly
774+
// or use the GODEBUG=tlskyber=0 environment variable.
760775
CurvePreferences []CurveID
761776

762777
// DynamicRecordSizingDisabled disables adaptive sizing of TLS records.
@@ -1084,20 +1099,27 @@ func supportedVersionsFromMax(maxVersion uint16) []uint16 {
10841099
return versions
10851100
}
10861101

1087-
var defaultCurvePreferences = []CurveID{X25519, CurveP256, CurveP384, CurveP521}
1102+
var tlskyber = godebug.New("tlskyber")
10881103

1089-
func (c *Config) curvePreferences() []CurveID {
1104+
var defaultCurvePreferences = []CurveID{x25519Kyber768Draft00, X25519, CurveP256, CurveP384, CurveP521}
1105+
1106+
var defaultCurvePreferencesWithoutKyber = []CurveID{X25519, CurveP256, CurveP384, CurveP521}
1107+
1108+
func (c *Config) curvePreferences(version uint16) []CurveID {
10901109
if needFIPS() {
10911110
return fipsCurvePreferences(c)
10921111
}
10931112
if c == nil || len(c.CurvePreferences) == 0 {
1113+
if version < VersionTLS13 || tlskyber.Value() == "0" {
1114+
return defaultCurvePreferencesWithoutKyber
1115+
}
10941116
return defaultCurvePreferences
10951117
}
10961118
return c.CurvePreferences
10971119
}
10981120

1099-
func (c *Config) supportsCurve(curve CurveID) bool {
1100-
for _, cc := range c.curvePreferences() {
1121+
func (c *Config) supportsCurve(version uint16, curve CurveID) bool {
1122+
for _, cc := range c.curvePreferences(version) {
11011123
if cc == curve {
11021124
return true
11031125
}
@@ -1256,7 +1278,7 @@ func (chi *ClientHelloInfo) SupportsCertificate(c *Certificate) error {
12561278
}
12571279

12581280
// The only signed key exchange we support is ECDHE.
1259-
if !supportsECDHE(config, chi.SupportedCurves, chi.SupportedPoints) {
1281+
if !supportsECDHE(config, vers, chi.SupportedCurves, chi.SupportedPoints) {
12601282
return supportsRSAFallback(errors.New("client doesn't support ECDHE, can only use legacy RSA key exchange"))
12611283
}
12621284

@@ -1277,7 +1299,7 @@ func (chi *ClientHelloInfo) SupportsCertificate(c *Certificate) error {
12771299
}
12781300
var curveOk bool
12791301
for _, c := range chi.SupportedCurves {
1280-
if c == curve && config.supportsCurve(c) {
1302+
if c == curve && config.supportsCurve(vers, c) {
12811303
curveOk = true
12821304
break
12831305
}

src/crypto/tls/common_string.go

+5-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/crypto/tls/conn.go

+3
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ type Conn struct {
5050
didResume bool // whether this connection was a session resumption
5151
didHRR bool // whether a HelloRetryRequest was sent/received
5252
cipherSuite uint16
53+
curveID CurveID
5354
ocspResponse []byte // stapled OCSP response
5455
scts [][]byte // signed certificate timestamps from server
5556
peerCertificates []*x509.Certificate
@@ -1610,6 +1611,8 @@ func (c *Conn) connectionStateLocked() ConnectionState {
16101611
state.NegotiatedProtocol = c.clientProtocol
16111612
state.DidResume = c.didResume
16121613
state.testingOnlyDidHRR = c.didHRR
1614+
// c.curveID is not set on TLS 1.0–1.2 resumptions. Fix that before exposing it.
1615+
state.testingOnlyCurveID = c.curveID
16131616
state.NegotiatedProtocolIsMutual = true
16141617
state.ServerName = c.serverName
16151618
state.CipherSuite = c.cipherSuite

0 commit comments

Comments
 (0)