Skip to content

Commit

Permalink
crypto/tls: implement X25519MLKEM768
Browse files Browse the repository at this point in the history
This makes three related changes that work particularly well together
and would require significant extra work to do separately: it replaces
X25519Kyber768Draft00 with X25519MLKEM768, it makes CurvePreferences
ordering crypto/tls-selected, and applies a preference to PQ key
exchange methods over key shares (to mitigate downgrades).

TestHandshakeServerUnsupportedKeyShare was removed because we are not
rejecting unsupported key shares anymore (nor do we select them, and
rejecting them actively is a MAY). It would have been nice to keep the
test to check we still continue successfully, but testClientHelloFailure
is broken in the face of any server-side behavior which requires writing
any other messages back to the client, or reading them.

Updates #69985
Fixes #69393

Change-Id: I58de76f5b8742a9bd4543fd7907c48e038507b19
Reviewed-on: https://go-review.googlesource.com/c/go/+/630775
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
FiloSottile authored and gopherbot committed Nov 22, 2024
1 parent dbfd003 commit 4b7f7cd
Show file tree
Hide file tree
Showing 18 changed files with 186 additions and 206 deletions.
2 changes: 2 additions & 0 deletions api/next/69985.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pkg crypto/tls, const X25519MLKEM768 = 4588 #69985
pkg crypto/tls, const X25519MLKEM768 CurveID #69985
5 changes: 5 additions & 0 deletions doc/godebug.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ than the
[`Certificate.PolicyIdentifiers`](/pkg/crypto/x509/#Certificate.PolicyIdentifiers)
field by default.

Go 1.24 enabled the post-quantum key exchange mechanism
X25519MLKEM768 by default. The default can be reverted using the
[`tlsmlkem` setting](/pkg/crypto/tls/#Config.CurvePreferences).
Go 1.24 also removed X25519Kyber768Draft00 and the Go 1.23 `tlskyber` setting.

### Go 1.23

Go 1.23 changed the channels created by package time to be unbuffered
Expand Down
2 changes: 2 additions & 0 deletions doc/next/6-stdlib/99-minor/crypto/tls/69985.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
`crypto/tls` now supports the post-quantum [X25519MLKEM768] key exchange. Support
for the experimental X25519Kyber768Draft00 key exchange has been removed.
13 changes: 10 additions & 3 deletions src/crypto/tls/bogo_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,14 @@
"TLS-ECH-Server-EarlyData": "Go does not support early (0-RTT) data",
"TLS-ECH-Server-EarlyDataRejected": "Go does not support early (0-RTT) data",

"CurveTest-Client-Kyber-TLS13": "Temporarily disabled since the curve ID is not exposed and it cannot be correctly configured",
"CurveTest-Server-Kyber-TLS13": "Temporarily disabled since the curve ID is not exposed and it cannot be correctly configured",
"MLKEMKeyShareIncludedSecond": "BoGo wants us to order the key shares based on its preference, but we don't support that",
"MLKEMKeyShareIncludedThird": "BoGo wants us to order the key shares based on its preference, but we don't support that",
"PostQuantumNotEnabledByDefaultInClients": "We do enable it by default!",
"*-Kyber-TLS13": "We don't support Kyber, only ML-KEM (BoGo bug ignoring AllCurves?)",

"SendEmptySessionTicket-TLS13": "https://github.com/golang/go/issues/70513",

"*-SignDefault-*": "TODO, partially it encodes BoringSSL defaults, partially we might be missing some implicit behavior of a missing flag",

"SendV2ClientHello*": "We don't support SSLv2",
"*QUIC*": "No QUIC support",
Expand Down Expand Up @@ -238,6 +244,7 @@
23,
24,
25,
29
29,
4588
]
}
9 changes: 3 additions & 6 deletions src/crypto/tls/bogo_shim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ func TestBogoSuite(t *testing.T) {
if *bogoLocalDir != "" {
bogoDir = *bogoLocalDir
} else {
const boringsslModVer = "v0.0.0-20240523173554-273a920f84e8"
const boringsslModVer = "v0.0.0-20241120195446-5cce3fbd23e1"
bogoDir = cryptotest.FetchModule(t, "boringssl.googlesource.com/boringssl.git", boringsslModVer)
}

Expand Down Expand Up @@ -473,11 +473,8 @@ func TestBogoSuite(t *testing.T) {
// are present in the output. They are only checked if -bogo-filter
// was not passed.
assertResults := map[string]string{
// TODO: these tests are temporarily disabled, since we don't expose the
// necessary curve ID, and it's currently not possible to correctly
// configure it.
// "CurveTest-Client-Kyber-TLS13": "PASS",
// "CurveTest-Server-Kyber-TLS13": "PASS",
"CurveTest-Client-MLKEM-TLS13": "PASS",
"CurveTest-Server-MLKEM-TLS13": "PASS",
}

for name, result := range results.Tests {
Expand Down
62 changes: 33 additions & 29 deletions src/crypto/tls/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,17 +145,21 @@ const (
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
CurveP256 CurveID = 23
CurveP384 CurveID = 24
CurveP521 CurveID = 25
X25519 CurveID = 29
X25519MLKEM768 CurveID = 4588
)

func isTLS13OnlyKeyExchange(curve CurveID) bool {
return curve == X25519MLKEM768
}

func isPQKeyExchange(curve CurveID) bool {
return curve == X25519MLKEM768
}

// TLS 1.3 Key Share. See RFC 8446, Section 4.2.8.
type keyShare struct {
group CurveID
Expand Down Expand Up @@ -419,9 +423,12 @@ type ClientHelloInfo struct {
// client is using SNI (see RFC 4366, Section 3.1).
ServerName string

// SupportedCurves lists the elliptic curves supported by the client.
// SupportedCurves is set only if the Supported Elliptic Curves
// Extension is being used (see RFC 4492, Section 5.1.1).
// SupportedCurves lists the key exchange mechanisms supported by the
// client. It was renamed to "supported groups" in TLS 1.3, see RFC 8446,
// Section 4.2.7 and [CurveID].
//
// SupportedCurves may be nil in TLS 1.2 and lower if the Supported Elliptic
// Curves Extension is not being used (see RFC 4492, Section 5.1.1).
SupportedCurves []CurveID

// SupportedPoints lists the point formats supported by the client.
Expand Down Expand Up @@ -761,14 +768,15 @@ type Config struct {
// which is currently TLS 1.3.
MaxVersion uint16

// CurvePreferences contains the elliptic curves that will be used in
// 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.
// CurvePreferences contains a set of supported key exchange mechanisms.
// The name refers to elliptic curves for legacy reasons, see [CurveID].
// The order of the list is ignored, and key exchange mechanisms are chosen
// from this list using an internal preference order. If empty, the default
// will be used.
//
// From Go 1.23, the default includes the X25519Kyber768Draft00 hybrid
// From Go 1.24, the default includes the [X25519MLKEM768] hybrid
// post-quantum key exchange. To disable it, set CurvePreferences explicitly
// or use the GODEBUG=tlskyber=0 environment variable.
// or use the GODEBUG=tlsmlkem=0 environment variable.
CurvePreferences []CurveID

// DynamicRecordSizingDisabled disables adaptive sizing of TLS records.
Expand Down Expand Up @@ -1176,23 +1184,19 @@ func supportedVersionsFromMax(maxVersion uint16) []uint16 {

func (c *Config) curvePreferences(version uint16) []CurveID {
var curvePreferences []CurveID
if c != nil && len(c.CurvePreferences) != 0 {
curvePreferences = slices.Clone(c.CurvePreferences)
if fips140tls.Required() {
return slices.DeleteFunc(curvePreferences, func(c CurveID) bool {
return !slices.Contains(defaultCurvePreferencesFIPS, c)
})
}
} else if fips140tls.Required() {
if fips140tls.Required() {
curvePreferences = slices.Clone(defaultCurvePreferencesFIPS)
} else {
curvePreferences = defaultCurvePreferences()
}
if version < VersionTLS13 {
return slices.DeleteFunc(curvePreferences, func(c CurveID) bool {
return c == x25519Kyber768Draft00
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
}

Expand Down
6 changes: 3 additions & 3 deletions src/crypto/tls/common_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions src/crypto/tls/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ import (
// Defaults are collected in this file to allow distributions to more easily patch
// them to apply local policies.

var tlskyber = godebug.New("tlskyber")
var tlsmlkem = godebug.New("tlsmlkem")

// defaultCurvePreferences is the default set of supported key exchanges, as
// well as the preference order.
func defaultCurvePreferences() []CurveID {
if tlskyber.Value() == "0" {
if tlsmlkem.Value() == "0" {
return []CurveID{X25519, CurveP256, CurveP384, CurveP521}
}
// For now, x25519Kyber768Draft00 must always be followed by X25519.
return []CurveID{x25519Kyber768Draft00, X25519, CurveP256, CurveP384, CurveP521}
return []CurveID{X25519MLKEM768, X25519, CurveP256, CurveP384, CurveP521}
}

// defaultSupportedSignatureAlgorithms contains the signature and hash algorithms that
Expand Down
5 changes: 1 addition & 4 deletions src/crypto/tls/fips_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,16 +184,13 @@ func TestFIPSServerCipherSuites(t *testing.T) {

func TestFIPSServerCurves(t *testing.T) {
serverConfig := testConfig.Clone()
serverConfig.CurvePreferences = nil
serverConfig.BuildNameToCertificate()

for _, curveid := range defaultCurvePreferences() {
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)
}

runWithFIPSDisabled(t, func(t *testing.T) {
if _, _, err := testHandshake(t, clientConfig, serverConfig); err != nil {
Expand Down
25 changes: 15 additions & 10 deletions src/crypto/tls/handshake_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"internal/godebug"
"io"
"net"
"slices"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -156,7 +157,9 @@ func (c *Conn) makeClientHello() (*clientHelloMsg, *keySharePrivateKeys, *echCli
}
curveID := hello.supportedCurves[0]
keyShareKeys = &keySharePrivateKeys{curveID: curveID}
if curveID == x25519Kyber768Draft00 {
// Note that if X25519MLKEM768 is supported, it will be first because
// the preference order is fixed.
if curveID == X25519MLKEM768 {
keyShareKeys.ecdhe, err = generateECDHEKey(config.rand(), X25519)
if err != nil {
return nil, nil, nil, err
Expand All @@ -165,18 +168,20 @@ func (c *Conn) makeClientHello() (*clientHelloMsg, *keySharePrivateKeys, *echCli
if _, err := io.ReadFull(config.rand(), seed); err != nil {
return nil, nil, nil, err
}
keyShareKeys.kyber, err = mlkem.NewDecapsulationKey768(seed)
keyShareKeys.mlkem, err = mlkem.NewDecapsulationKey768(seed)
if err != nil {
return nil, nil, nil, err
}
// For draft-tls-westerbaan-xyber768d00-03, we send both a hybrid
// and a standard X25519 key share, since most servers will only
// support the latter. We reuse the same X25519 ephemeral key for
// both, as allowed by draft-ietf-tls-hybrid-design-09, Section 3.2.
mlkemEncapsulationKey := keyShareKeys.mlkem.EncapsulationKey().Bytes()
x25519EphemeralKey := keyShareKeys.ecdhe.PublicKey().Bytes()
hello.keyShares = []keyShare{
{group: x25519Kyber768Draft00, data: append(keyShareKeys.ecdhe.PublicKey().Bytes(),
keyShareKeys.kyber.EncapsulationKey().Bytes()...)},
{group: X25519, data: keyShareKeys.ecdhe.PublicKey().Bytes()},
{group: X25519MLKEM768, data: append(mlkemEncapsulationKey, x25519EphemeralKey...)},
}
// If both X25519MLKEM768 and X25519 are supported, we send both key
// shares (as a fallback) and we reuse the same X25519 ephemeral
// key, as allowed by draft-ietf-tls-hybrid-design-09, Section 3.2.
if slices.Contains(hello.supportedCurves, X25519) {
hello.keyShares = append(hello.keyShares, keyShare{group: X25519, data: x25519EphemeralKey})
}
} else {
if _, ok := curveForCurveID(curveID); !ok {
Expand Down Expand Up @@ -711,7 +716,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
if ok {
err = keyAgreement.processServerKeyExchange(c.config, hs.hello, hs.serverHello, c.peerCertificates[0], skx)
if err != nil {
c.sendAlert(alertUnexpectedMessage)
c.sendAlert(alertIllegalParameter)
return err
}
if len(skx.key) >= 3 && skx.key[0] == 3 /* named curve */ {
Expand Down
29 changes: 14 additions & 15 deletions src/crypto/tls/handshake_client_tls13.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,12 +322,11 @@ func (hs *clientHandshakeStateTLS13) processHelloRetryRequest() error {
c.sendAlert(alertIllegalParameter)
return errors.New("tls: server sent an unnecessary HelloRetryRequest key_share")
}
// Note: we don't support selecting X25519Kyber768Draft00 in a HRR,
// because we currently only support it at all when CurvePreferences is
// empty, which will cause us to also send a key share for it.
// Note: we don't support selecting X25519MLKEM768 in a HRR, because it
// is currently first in preference order, so if it's enabled we'll
// always send a key share for it.
//
// This will have to change once we support selecting hybrid KEMs
// without sending key shares for them.
// This will have to change once we support multiple hybrid KEMs.
if _, ok := curveForCurveID(curveID); !ok {
c.sendAlert(alertInternalError)
return errors.New("tls: CurvePreferences includes unsupported curve")
Expand Down Expand Up @@ -480,12 +479,12 @@ func (hs *clientHandshakeStateTLS13) establishHandshakeKeys() error {
c := hs.c

ecdhePeerData := hs.serverHello.serverShare.data
if hs.serverHello.serverShare.group == x25519Kyber768Draft00 {
if len(ecdhePeerData) != x25519PublicKeySize+mlkem.CiphertextSize768 {
if hs.serverHello.serverShare.group == X25519MLKEM768 {
if len(ecdhePeerData) != mlkem.CiphertextSize768+x25519PublicKeySize {
c.sendAlert(alertIllegalParameter)
return errors.New("tls: invalid server key share")
return errors.New("tls: invalid server X25519MLKEM768 key share")
}
ecdhePeerData = hs.serverHello.serverShare.data[:x25519PublicKeySize]
ecdhePeerData = hs.serverHello.serverShare.data[mlkem.CiphertextSize768:]
}
peerKey, err := hs.keyShareKeys.ecdhe.Curve().NewPublicKey(ecdhePeerData)
if err != nil {
Expand All @@ -497,17 +496,17 @@ func (hs *clientHandshakeStateTLS13) establishHandshakeKeys() error {
c.sendAlert(alertIllegalParameter)
return errors.New("tls: invalid server key share")
}
if hs.serverHello.serverShare.group == x25519Kyber768Draft00 {
if hs.keyShareKeys.kyber == nil {
if hs.serverHello.serverShare.group == X25519MLKEM768 {
if hs.keyShareKeys.mlkem == nil {
return c.sendAlert(alertInternalError)
}
ciphertext := hs.serverHello.serverShare.data[x25519PublicKeySize:]
kyberShared, err := kyberDecapsulate(hs.keyShareKeys.kyber, ciphertext)
ciphertext := hs.serverHello.serverShare.data[:mlkem.CiphertextSize768]
mlkemShared, err := hs.keyShareKeys.mlkem.Decapsulate(ciphertext)
if err != nil {
c.sendAlert(alertIllegalParameter)
return errors.New("tls: invalid Kyber server key share")
return errors.New("tls: invalid X25519MLKEM768 server key share")
}
sharedKey = append(sharedKey, kyberShared...)
sharedKey = append(mlkemShared, sharedKey...)
}
c.curveID = hs.serverHello.serverShare.group

Expand Down
2 changes: 1 addition & 1 deletion src/crypto/tls/handshake_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ func (hs *serverHandshakeState) doFullHandshake() error {

preMasterSecret, err := keyAgreement.processClientKeyExchange(c.config, hs.cert, ckx, c.vers)
if err != nil {
c.sendAlert(alertHandshakeFailure)
c.sendAlert(alertIllegalParameter)
return err
}
if hs.hello.extendedMasterSecret {
Expand Down
16 changes: 0 additions & 16 deletions src/crypto/tls/handshake_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -939,22 +939,6 @@ func TestHandshakeServerKeySharePreference(t *testing.T) {
runServerTestTLS13(t, test)
}

// TestHandshakeServerUnsupportedKeyShare tests a client that sends a key share
// that's not in the supported groups list.
func TestHandshakeServerUnsupportedKeyShare(t *testing.T) {
pk, _ := ecdh.P384().GenerateKey(rand.Reader)
clientHello := &clientHelloMsg{
vers: VersionTLS12,
random: make([]byte, 32),
supportedVersions: []uint16{VersionTLS13},
cipherSuites: []uint16{TLS_AES_128_GCM_SHA256},
compressionMethods: []uint8{compressionNone},
keyShares: []keyShare{{group: CurveP384, data: pk.PublicKey().Bytes()}},
supportedCurves: []CurveID{CurveP256},
}
testClientHelloFailure(t, testConfig, clientHello, "client sent key share for group it does not support")
}

func TestHandshakeServerALPN(t *testing.T) {
config := testConfig.Clone()
config.NextProtos = []string{"proto1", "proto2"}
Expand Down
Loading

0 comments on commit 4b7f7cd

Please sign in to comment.