[release-branch.go1.26] crypto/tls: don't copy auto-rotated session ticket keys in...

Once a tls.Config is used, it is not safe to mutate. We provide the
Clone method in order to allow users to copy and modify a Config that
is in use.

If Config.SessionTicketKey is not populated, and if
Config.SetSessionTicketKeys has not been called, we automatically
populate and rotate session ticket keys. Clone was previously copying
these keys into the new Config, meaning that two Configs could share
the same auto-rotated session ticket keys. This could allow sessions to
be resumed across different Configs, which may have completely different
configurations.

This change updates Clone to not copy the auto-rotated session ticket
keys.

Additionally, when resuming a session, check that not just that the leaf
certificate is unexpired, but that the entire certificate chain is still
unexpired.

Fixes #77113
Fixes CVE-2025-68121

Change-Id: I011df7329de83068d11b3f0c793763692d018a98
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/3300
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/3340
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/736704
TryBot-Bypass: Michael Pratt <mpratt@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index 099a11ca63da758274f86e2354a2bd1a7b06fee0..093869ac8b7a874986402dea2cb041799ca0a143 100644 (file)
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -980,6 +980,10 @@ const maxSessionTicketLifetime = 7 * 24 * time.Hour
 
 // Clone returns a shallow clone of c or nil if c is nil. It is safe to clone a [Config] that is
 // being used concurrently by a TLS client or server.
+//
+// If Config.SessionTicketKey is unpopulated, and Config.SetSessionTicketKeys has not been
+// called, the clone will not share the same auto-rotated session ticket keys as the original
+// Config in order to prevent sessions from being resumed across Configs.
 func (c *Config) Clone() *Config {
        if c == nil {
                return nil
@@ -1020,7 +1024,8 @@ func (c *Config) Clone() *Config {
                EncryptedClientHelloRejectionVerify: c.EncryptedClientHelloRejectionVerify,
                EncryptedClientHelloKeys:            c.EncryptedClientHelloKeys,
                sessionTicketKeys:                   c.sessionTicketKeys,
-               autoSessionTicketKeys:               c.autoSessionTicketKeys,
+               // We explicitly do not copy autoSessionTicketKeys, so that Configs do
+               // not share the same auto-rotated keys.
        }
 }
 

diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go
index efdaeae6f7e39a23ae1f9b1cb6adbd7649d66935..06675a8ce9d2a10efd4c71b1e0f4cab675123a27 100644 (file)
--- a/src/crypto/tls/handshake_server.go
+++ b/src/crypto/tls/handshake_server.go
@@ -520,8 +520,13 @@ func (hs *serverHandshakeState) checkForResumption() error {
        if sessionHasClientCerts && c.config.ClientAuth == NoClientCert {
                return nil
        }
-       if sessionHasClientCerts && c.config.time().After(sessionState.peerCertificates[0].NotAfter) {
-               return nil
+       if sessionHasClientCerts {
+               now := c.config.time()
+               for _, c := range sessionState.peerCertificates {
+                       if now.After(c.NotAfter) {
+                               return nil
+                       }
+               }
        }
        if sessionHasClientCerts && c.config.ClientAuth >= VerifyClientCertIfGiven &&
                len(sessionState.verifiedChains) == 0 {

diff --git a/src/crypto/tls/handshake_server_test.go b/src/crypto/tls/handshake_server_test.go
index 7e35c252593658a3cb626daecb12bccea0189107..41ae87050eab6899638f7d8e35d3cd51d89b46bf 100644 (file)
--- a/src/crypto/tls/handshake_server_test.go
+++ b/src/crypto/tls/handshake_server_test.go
@@ -13,6 +13,7 @@ import (
        "crypto/rand"
        "crypto/tls/internal/fips140tls"
        "crypto/x509"
+       "crypto/x509/pkix"
        "encoding/pem"
        "errors"
        "fmt"
@@ -2153,3 +2154,103 @@ func TestHandshakeContextHierarchy(t *testing.T) {
                t.Errorf("Unexpected client error: %v", err)
        }
 }
+
+func TestHandshakeChainExpiryResumptionTLS12(t *testing.T) {
+       t.Run("TLS1.2", func(t *testing.T) {
+               testHandshakeChainExpiryResumption(t, VersionTLS12)
+       })
+       t.Run("TLS1.3", func(t *testing.T) {
+               testHandshakeChainExpiryResumption(t, VersionTLS13)
+       })
+}
+
+func testHandshakeChainExpiryResumption(t *testing.T, version uint16) {
+       now := time.Now()
+       createChain := func(leafNotAfter, rootNotAfter time.Time) (certDER []byte, root *x509.Certificate) {
+               tmpl := &x509.Certificate{
+                       Subject:               pkix.Name{CommonName: "root"},
+                       NotBefore:             rootNotAfter.Add(-time.Hour * 24),
+                       NotAfter:              rootNotAfter,
+                       IsCA:                  true,
+                       BasicConstraintsValid: true,
+               }
+               rootDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
+               if err != nil {
+                       t.Fatalf("CreateCertificate: %v", err)
+               }
+               root, err = x509.ParseCertificate(rootDER)
+               if err != nil {
+                       t.Fatalf("ParseCertificate: %v", err)
+               }
+
+               tmpl = &x509.Certificate{
+                       Subject:   pkix.Name{},
+                       DNSNames:  []string{"expired-resume.example.com"},
+                       NotBefore: leafNotAfter.Add(-time.Hour * 24),
+                       NotAfter:  leafNotAfter,
+                       KeyUsage:  x509.KeyUsageDigitalSignature,
+               }
+               certDER, err = x509.CreateCertificate(rand.Reader, tmpl, root, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
+               if err != nil {
+                       t.Fatalf("CreateCertificate: %v", err)
+               }
+
+               return certDER, root
+       }
+
+       initialLeafDER, initialRoot := createChain(now.Add(time.Hour), now.Add(2*time.Hour))
+
+       serverConfig := testConfig.Clone()
+       serverConfig.MaxVersion = version
+       serverConfig.Certificates = []Certificate{{
+               Certificate: [][]byte{initialLeafDER},
+               PrivateKey:  testECDSAPrivateKey,
+       }}
+       serverConfig.ClientCAs = x509.NewCertPool()
+       serverConfig.ClientCAs.AddCert(initialRoot)
+       serverConfig.ClientAuth = RequireAndVerifyClientCert
+       serverConfig.Time = func() time.Time {
+               return now
+       }
+
+       clientConfig := testConfig.Clone()
+       clientConfig.MaxVersion = version
+       clientConfig.Certificates = []Certificate{{
+               Certificate: [][]byte{initialLeafDER},
+               PrivateKey:  testECDSAPrivateKey,
+       }}
+       clientConfig.RootCAs = x509.NewCertPool()
+       clientConfig.RootCAs.AddCert(initialRoot)
+       clientConfig.ServerName = "expired-resume.example.com"
+       clientConfig.ClientSessionCache = NewLRUClientSessionCache(32)
+
+       testResume := func(t *testing.T, sc, cc *Config, expectResume bool) {
+               t.Helper()
+               ss, cs, err := testHandshake(t, cc, sc)
+               if err != nil {
+                       t.Fatalf("handshake: %v", err)
+               }
+               if cs.DidResume != expectResume {
+                       t.Fatalf("DidResume = %v; want %v", cs.DidResume, expectResume)
+               }
+               if ss.DidResume != expectResume {
+                       t.Fatalf("DidResume = %v; want %v", cs.DidResume, expectResume)
+               }
+       }
+
+       testResume(t, serverConfig, clientConfig, false)
+       testResume(t, serverConfig, clientConfig, true)
+
+       freshLeafDER, freshRoot := createChain(now.Add(2*time.Hour), now.Add(3*time.Hour))
+       clientConfig.Certificates = []Certificate{{
+               Certificate: [][]byte{freshLeafDER},
+               PrivateKey:  testECDSAPrivateKey,
+       }}
+       serverConfig.Time = func() time.Time {
+               return now.Add(1*time.Hour + 30*time.Minute)
+       }
+       serverConfig.ClientCAs = x509.NewCertPool()
+       serverConfig.ClientCAs.AddCert(freshRoot)
+
+       testResume(t, serverConfig, clientConfig, false)
+}

diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go
index b066924e29168648dabc53382d0febaf3b44de07..0033164f65da8c997e02550d31bb3e0287c5bc95 100644 (file)
--- a/src/crypto/tls/handshake_server_tls13.go
+++ b/src/crypto/tls/handshake_server_tls13.go
@@ -314,6 +314,7 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error {
                return nil
        }
 
+pskIdentityLoop:
        for i, identity := range hs.clientHello.pskIdentities {
                if i >= maxClientPSKIdentities {
                        break
@@ -366,8 +367,13 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error {
                if sessionHasClientCerts && c.config.ClientAuth == NoClientCert {
                        continue
                }
-               if sessionHasClientCerts && c.config.time().After(sessionState.peerCertificates[0].NotAfter) {
-                       continue
+               if sessionHasClientCerts {
+                       now := c.config.time()
+                       for _, c := range sessionState.peerCertificates {
+                               if now.After(c.NotAfter) {
+                                       continue pskIdentityLoop
+                               }
+                       }
                }
                if sessionHasClientCerts && c.config.ClientAuth >= VerifyClientCertIfGiven &&
                        len(sessionState.verifiedChains) == 0 {

diff --git a/src/crypto/tls/tls_test.go b/src/crypto/tls/tls_test.go
index 39ebb9d2f1edd39d30fc6035840b700019bcce82..48428e4cc916661218ed7273ccc129d975062d7c 100644 (file)
--- a/src/crypto/tls/tls_test.go
+++ b/src/crypto/tls/tls_test.go
@@ -935,8 +935,8 @@ func TestCloneNonFuncFields(t *testing.T) {
                }
        }
        // Set the unexported fields related to session ticket keys, which are copied with Clone().
-       c1.autoSessionTicketKeys = []ticketKey{c1.ticketKeyFromBytes(c1.SessionTicketKey)}
        c1.sessionTicketKeys = []ticketKey{c1.ticketKeyFromBytes(c1.SessionTicketKey)}
+       // We explicitly don't copy autoSessionTicketKeys in Clone, so don't set it.
 
        c2 := c1.Clone()
        if !reflect.DeepEqual(&c1, c2) {
@@ -2461,3 +2461,12 @@ func (s messageOnlySigner) SignMessage(rand io.Reader, msg []byte, opts crypto.S
        digest := h.Sum(nil)
        return s.Signer.Sign(rand, digest, opts)
 }
+
+func TestConfigCloneAutoSessionTicketKeys(t *testing.T) {
+       orig := &Config{}
+       orig.ticketKeys(nil)
+       clone := orig.Clone()
+       if slices.Equal(orig.autoSessionTicketKeys, clone.autoSessionTicketKeys) {
+               t.Fatal("autoSessionTicketKeys slice copied in Clone")
+       }
+}