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

This reverts CL 736709 (commit bba24719a4cad5cc8d771fc9cfff5a38019d554a).

Updates #77113
Updates #77357
Updates CVE-2025-68121

Change-Id: I0261cb75e9adf9d0ac9890dc91ae8476b8988ba0
Reviewed-on: https://go-review.googlesource.com/c/go/+/739320
Reviewed-by: Coia Prant <coiaprant@gmail.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/740002
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index 093869ac8b7a874986402dea2cb041799ca0a143..099a11ca63da758274f86e2354a2bd1a7b06fee0 100644 (file)
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -980,10 +980,6 @@ 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
@@ -1024,8 +1020,7 @@ func (c *Config) Clone() *Config {
                EncryptedClientHelloRejectionVerify: c.EncryptedClientHelloRejectionVerify,
                EncryptedClientHelloKeys:            c.EncryptedClientHelloKeys,
                sessionTicketKeys:                   c.sessionTicketKeys,
-               // We explicitly do not copy autoSessionTicketKeys, so that Configs do
-               // not share the same auto-rotated keys.
+               autoSessionTicketKeys:               c.autoSessionTicketKeys,
        }
 }
 

diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go
index 06675a8ce9d2a10efd4c71b1e0f4cab675123a27..efdaeae6f7e39a23ae1f9b1cb6adbd7649d66935 100644 (file)
--- a/src/crypto/tls/handshake_server.go
+++ b/src/crypto/tls/handshake_server.go
@@ -520,13 +520,8 @@ func (hs *serverHandshakeState) checkForResumption() error {
        if sessionHasClientCerts && c.config.ClientAuth == NoClientCert {
                return nil
        }
-       if sessionHasClientCerts {
-               now := c.config.time()
-               for _, c := range sessionState.peerCertificates {
-                       if now.After(c.NotAfter) {
-                               return nil
-                       }
-               }
+       if sessionHasClientCerts && c.config.time().After(sessionState.peerCertificates[0].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 41ae87050eab6899638f7d8e35d3cd51d89b46bf..7e35c252593658a3cb626daecb12bccea0189107 100644 (file)
--- a/src/crypto/tls/handshake_server_test.go
+++ b/src/crypto/tls/handshake_server_test.go
@@ -13,7 +13,6 @@ import (
        "crypto/rand"
        "crypto/tls/internal/fips140tls"
        "crypto/x509"
-       "crypto/x509/pkix"
        "encoding/pem"
        "errors"
        "fmt"
@@ -2154,103 +2153,3 @@ 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 0033164f65da8c997e02550d31bb3e0287c5bc95..b066924e29168648dabc53382d0febaf3b44de07 100644 (file)
--- a/src/crypto/tls/handshake_server_tls13.go
+++ b/src/crypto/tls/handshake_server_tls13.go
@@ -314,7 +314,6 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error {
                return nil
        }
 
-pskIdentityLoop:
        for i, identity := range hs.clientHello.pskIdentities {
                if i >= maxClientPSKIdentities {
                        break
@@ -367,13 +366,8 @@ pskIdentityLoop:
                if sessionHasClientCerts && c.config.ClientAuth == NoClientCert {
                        continue
                }
-               if sessionHasClientCerts {
-                       now := c.config.time()
-                       for _, c := range sessionState.peerCertificates {
-                               if now.After(c.NotAfter) {
-                                       continue pskIdentityLoop
-                               }
-                       }
+               if sessionHasClientCerts && c.config.time().After(sessionState.peerCertificates[0].NotAfter) {
+                       continue
                }
                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 48428e4cc916661218ed7273ccc129d975062d7c..39ebb9d2f1edd39d30fc6035840b700019bcce82 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,12 +2461,3 @@ 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")
-       }
-}