From a1d9a25ddc3f3446a015f74c998635ff134409ca Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 26 Jan 2026 10:49:30 -0800 Subject: [PATCH] [release-branch.go1.26] Revert "crypto/tls: don't copy auto-rotated session ticket keys in Config.Clone" 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 Reviewed-by: Filippo Valsorda LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov Reviewed-on: https://go-review.googlesource.com/c/go/+/740002 Reviewed-by: Damien Neil Reviewed-by: Nicholas Husin Auto-Submit: Dmitri Shuralyov Reviewed-by: Nicholas Husin --- src/crypto/tls/common.go | 7 +- src/crypto/tls/handshake_server.go | 9 +- src/crypto/tls/handshake_server_test.go | 101 ----------------------- src/crypto/tls/handshake_server_tls13.go | 10 +-- src/crypto/tls/tls_test.go | 11 +-- 5 files changed, 6 insertions(+), 132 deletions(-) diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go index 093869ac8b7..099a11ca63d 100644 --- 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 06675a8ce9d..efdaeae6f7e 100644 --- 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 41ae87050ea..7e35c252593 100644 --- 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 0033164f65d..b066924e291 100644 --- 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 48428e4cc91..39ebb9d2f1e 100644 --- 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") - } -}