[release-branch.go1.25] 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/+/3321
Reviewed-on: https://go-review.googlesource.com/c/go/+/736720
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Bypass: Michael Pratt <mpratt@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index 6fe6f34cd210d8bdffb6b8e32e0c8051a08d4256..d6d4c91740204daf8d120d53fe0cb16403265c18 100644 (file)
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -942,6 +942,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
@@ -982,7 +986,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 088c66fadb2a4446453b6b8852966ad3006e47e1..f244fc176b4668ccf527c27e480750b971b6234a 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 a6d64a506a0542f1498764cbf1da1b5962ba6c26..382d3f72809e1446f30e3d3e62a57e4b3721988b 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"
@@ -2121,3 +2122,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 71f6c1b5a4c2d986ad12c5a147814c32844ce409..6a30855dce7a134170b6913e5d41a942b350d0c4 100644 (file)
--- a/src/crypto/tls/handshake_server_tls13.go
+++ b/src/crypto/tls/handshake_server_tls13.go
@@ -354,6 +354,7 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error {
                return nil
        }
 
+pskIdentityLoop:
        for i, identity := range hs.clientHello.pskIdentities {
                if i >= maxClientPSKIdentities {
                        break
@@ -406,8 +407,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 bfcc62ccfb8ba0698eaefff628a4c1936698bb34..0a791bd7b709b604b3d31976c635dcc2cfc1104b 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) {
@@ -2347,3 +2347,12 @@ func TestECH(t *testing.T) {
 
        check()
 }
+
+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")
+       }
+}