[release-branch.go1.24] crypto/tls: check verifiedChains roots when resuming sessions

When resuming TLS sessions, on the server and client verify that the
chains stored in the session state (verifiedChains) are still acceptable
with regards to the Config by checking for the inclusion of the root in
either ClientCAs (server) or RootCAs (client). This prevents resuming
a session with a certificate chain that would be rejected during a full
handshake due to an untrusted root.

Updates #77113
Updates #77355
Updates CVE-2025-68121

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

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index e93eb560404e41955242ad5ed8a5a427a667b92d..623edcd2583c457fc986fe77bc2636fa1cc57699 100644 (file)
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -1742,13 +1742,28 @@ func fipsAllowCert(c *x509.Certificate) bool {
        return false
 }
 
-// anyUnexpiredChain reports if at least one of verifiedChains is still
-// unexpired. If verifiedChains is empty, it returns false.
-func anyUnexpiredChain(verifiedChains [][]*x509.Certificate, now time.Time) bool {
+// anyValidVerifiedChain reports if at least one of the chains in verifiedChains
+// is valid, as indicated by none of the certificates being expired and the root
+// being in opts.Roots (or in the system root pool if opts.Roots is nil). If
+// verifiedChains is empty, it returns false.
+func anyValidVerifiedChain(verifiedChains [][]*x509.Certificate, opts x509.VerifyOptions) bool {
        for _, chain := range verifiedChains {
-               if len(chain) != 0 && !slices.ContainsFunc(chain, func(cert *x509.Certificate) bool {
-                       return now.Before(cert.NotBefore) || now.After(cert.NotAfter) // cert is expired
+               if len(chain) == 0 {
+                       continue
+               }
+               if slices.ContainsFunc(chain, func(cert *x509.Certificate) bool {
+                       return opts.CurrentTime.Before(cert.NotBefore) || opts.CurrentTime.After(cert.NotAfter)
                }) {
+                       continue
+               }
+               // Since we already validated the chain, we only care that it is
+               // rooted in a CA in CAs, or in the system pool. On platforms where
+               // we control chain validation (e.g. not Windows or macOS) this is a
+               // simple lookup in the CertPool internal hash map. On other
+               // platforms, this may be more expensive, depending on how they
+               // implement verification of just root certificates.
+               root := chain[len(chain)-1]
+               if _, err := root.Verify(opts); err == nil {
                        return true
                }
        }

diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go
index dace71975aa1a284c19506190473f8e9e9b6822b..33d9a5be5487bc5d099e20b5820aab6588ab5f3a 100644 (file)
--- a/src/crypto/tls/handshake_client.go
+++ b/src/crypto/tls/handshake_client.go
@@ -456,7 +456,12 @@ func (c *Conn) loadSession(hello *clientHelloMsg) (
                        // application from a faulty ClientSessionCache implementation.
                        return nil, nil, nil, nil
                }
-               if !anyUnexpiredChain(session.verifiedChains, c.config.time()) {
+               opts := x509.VerifyOptions{
+                       CurrentTime: c.config.time(),
+                       Roots:       c.config.RootCAs,
+                       KeyUsages:   []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+               }
+               if !anyValidVerifiedChain(session.verifiedChains, opts) {
                        // No valid chains, delete the entry.
                        c.config.ClientSessionCache.Put(cacheKey, nil)
                        return nil, nil, nil, nil

diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go
index d0b75d75c5a8f62b0e76441cfd8aa71e9fb9b5dc..69307e95b6b1011dfed8144df4d7fa14111ae075 100644 (file)
--- a/src/crypto/tls/handshake_server.go
+++ b/src/crypto/tls/handshake_server.go
@@ -513,8 +513,13 @@ func (hs *serverHandshakeState) checkForResumption() error {
        if sessionHasClientCerts && c.config.time().After(sessionState.peerCertificates[0].NotAfter) {
                return nil
        }
+       opts := x509.VerifyOptions{
+               CurrentTime: c.config.time(),
+               Roots:       c.config.ClientCAs,
+               KeyUsages:   []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
+       }
        if sessionHasClientCerts && c.config.ClientAuth >= VerifyClientCertIfGiven &&
-               !anyUnexpiredChain(sessionState.verifiedChains, c.config.time()) {
+               !anyValidVerifiedChain(sessionState.verifiedChains, opts) {
                return nil
        }
 

diff --git a/src/crypto/tls/handshake_server_test.go b/src/crypto/tls/handshake_server_test.go
index 30c0df9be3eda01c48423ad871aca3db295c923a..2b6eb2130194573da9d5bd0f67f8e1c52b6842f9 100644 (file)
--- a/src/crypto/tls/handshake_server_test.go
+++ b/src/crypto/tls/handshake_server_test.go
@@ -2244,3 +2244,217 @@ func testHandshakeChainExpiryResumption(t *testing.T, version uint16) {
        testExpiration("LeafExpiresBeforeRoot", now.Add(2*time.Hour), now.Add(3*time.Hour))
        testExpiration("LeafExpiresAfterRoot", now.Add(2*time.Hour), now.Add(time.Hour))
 }
+
+func TestHandshakeGetConfigForClientDifferentClientCAs(t *testing.T) {
+       t.Run("TLS1.2", func(t *testing.T) {
+               testHandshakeGetConfigForClientDifferentClientCAs(t, VersionTLS12)
+       })
+       t.Run("TLS1.3", func(t *testing.T) {
+               testHandshakeGetConfigForClientDifferentClientCAs(t, VersionTLS13)
+       })
+}
+
+func testHandshakeGetConfigForClientDifferentClientCAs(t *testing.T, version uint16) {
+       now := time.Now()
+       tmpl := &x509.Certificate{
+               Subject:               pkix.Name{CommonName: "root"},
+               NotBefore:             now.Add(-time.Hour * 24),
+               NotAfter:              now.Add(time.Hour * 24),
+               IsCA:                  true,
+               BasicConstraintsValid: true,
+       }
+       rootDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
+       if err != nil {
+               t.Fatalf("CreateCertificate: %v", err)
+       }
+       rootA, err := x509.ParseCertificate(rootDER)
+       if err != nil {
+               t.Fatalf("ParseCertificate: %v", err)
+       }
+       rootDER, err = x509.CreateCertificate(rand.Reader, tmpl, tmpl, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
+       if err != nil {
+               t.Fatalf("CreateCertificate: %v", err)
+       }
+       rootB, err := x509.ParseCertificate(rootDER)
+       if err != nil {
+               t.Fatalf("ParseCertificate: %v", err)
+       }
+
+       tmpl = &x509.Certificate{
+               Subject:   pkix.Name{},
+               DNSNames:  []string{"example.com"},
+               NotBefore: now.Add(-time.Hour * 24),
+               NotAfter:  now.Add(time.Hour * 24),
+               KeyUsage:  x509.KeyUsageDigitalSignature,
+       }
+       certDER, err := x509.CreateCertificate(rand.Reader, tmpl, rootA, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
+       if err != nil {
+               t.Fatalf("CreateCertificate: %v", err)
+       }
+
+       serverConfig := testConfig.Clone()
+       serverConfig.MaxVersion = version
+       serverConfig.Certificates = []Certificate{{
+               Certificate: [][]byte{certDER},
+               PrivateKey:  testECDSAPrivateKey,
+       }}
+       serverConfig.Time = func() time.Time {
+               return now
+       }
+       serverConfig.ClientCAs = x509.NewCertPool()
+       serverConfig.ClientCAs.AddCert(rootA)
+       serverConfig.ClientAuth = RequireAndVerifyClientCert
+       switchConfig := false
+       serverConfig.GetConfigForClient = func(clientHello *ClientHelloInfo) (*Config, error) {
+               if !switchConfig {
+                       return nil, nil
+               }
+               cfg := serverConfig.Clone()
+               cfg.ClientCAs = x509.NewCertPool()
+               cfg.ClientCAs.AddCert(rootB)
+               return cfg, nil
+       }
+       serverConfig.InsecureSkipVerify = false
+       serverConfig.ServerName = "example.com"
+
+       clientConfig := testConfig.Clone()
+       clientConfig.MaxVersion = version
+       clientConfig.Certificates = []Certificate{{
+               Certificate: [][]byte{certDER},
+               PrivateKey:  testECDSAPrivateKey,
+       }}
+       clientConfig.ClientSessionCache = NewLRUClientSessionCache(32)
+       clientConfig.RootCAs = x509.NewCertPool()
+       clientConfig.RootCAs.AddCert(rootA)
+       clientConfig.Time = func() time.Time {
+               return now
+       }
+       clientConfig.InsecureSkipVerify = false
+       clientConfig.ServerName = "example.com"
+
+       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)
+
+       // Cause GetConfigForClient to return a config cloned from the base config,
+       // but with a different ClientCAs pool. This should cause resumption to fail.
+       switchConfig = true
+
+       testResume(t, serverConfig, clientConfig, false)
+       testResume(t, serverConfig, clientConfig, true)
+}
+
+func TestHandshakeChangeRootCAsResumption(t *testing.T) {
+       t.Run("TLS1.2", func(t *testing.T) {
+               testHandshakeChangeRootCAsResumption(t, VersionTLS12)
+       })
+       t.Run("TLS1.3", func(t *testing.T) {
+               testHandshakeChangeRootCAsResumption(t, VersionTLS13)
+       })
+}
+
+func testHandshakeChangeRootCAsResumption(t *testing.T, version uint16) {
+       now := time.Now()
+       tmpl := &x509.Certificate{
+               Subject:               pkix.Name{CommonName: "root"},
+               NotBefore:             now.Add(-time.Hour * 24),
+               NotAfter:              now.Add(time.Hour * 24),
+               IsCA:                  true,
+               BasicConstraintsValid: true,
+       }
+       rootDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
+       if err != nil {
+               t.Fatalf("CreateCertificate: %v", err)
+       }
+       rootA, err := x509.ParseCertificate(rootDER)
+       if err != nil {
+               t.Fatalf("ParseCertificate: %v", err)
+       }
+       rootDER, err = x509.CreateCertificate(rand.Reader, tmpl, tmpl, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
+       if err != nil {
+               t.Fatalf("CreateCertificate: %v", err)
+       }
+       rootB, err := x509.ParseCertificate(rootDER)
+       if err != nil {
+               t.Fatalf("ParseCertificate: %v", err)
+       }
+
+       tmpl = &x509.Certificate{
+               Subject:   pkix.Name{},
+               DNSNames:  []string{"example.com"},
+               NotBefore: now.Add(-time.Hour * 24),
+               NotAfter:  now.Add(time.Hour * 24),
+               KeyUsage:  x509.KeyUsageDigitalSignature,
+       }
+       certDER, err := x509.CreateCertificate(rand.Reader, tmpl, rootA, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
+       if err != nil {
+               t.Fatalf("CreateCertificate: %v", err)
+       }
+
+       serverConfig := testConfig.Clone()
+       serverConfig.MaxVersion = version
+       serverConfig.Certificates = []Certificate{{
+               Certificate: [][]byte{certDER},
+               PrivateKey:  testECDSAPrivateKey,
+       }}
+       serverConfig.Time = func() time.Time {
+               return now
+       }
+       serverConfig.ClientCAs = x509.NewCertPool()
+       serverConfig.ClientCAs.AddCert(rootA)
+       serverConfig.ClientAuth = RequireAndVerifyClientCert
+       serverConfig.InsecureSkipVerify = false
+       serverConfig.ServerName = "example.com"
+
+       clientConfig := testConfig.Clone()
+       clientConfig.MaxVersion = version
+       clientConfig.Certificates = []Certificate{{
+               Certificate: [][]byte{certDER},
+               PrivateKey:  testECDSAPrivateKey,
+       }}
+       clientConfig.ClientSessionCache = NewLRUClientSessionCache(32)
+       clientConfig.RootCAs = x509.NewCertPool()
+       clientConfig.RootCAs.AddCert(rootA)
+       clientConfig.Time = func() time.Time {
+               return now
+       }
+       clientConfig.InsecureSkipVerify = false
+       clientConfig.ServerName = "example.com"
+
+       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)
+
+       clientConfig = clientConfig.Clone()
+       clientConfig.RootCAs = x509.NewCertPool()
+       clientConfig.RootCAs.AddCert(rootB)
+
+       testResume(t, serverConfig, clientConfig, false)
+       testResume(t, serverConfig, clientConfig, true)
+}

diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go
index cca26e8ae731f20821b1b57ba3e1a5a10a82f3ff..6d253408c1e160b0a6af440040e093c7d1d999b5 100644 (file)
--- a/src/crypto/tls/handshake_server_tls13.go
+++ b/src/crypto/tls/handshake_server_tls13.go
@@ -15,6 +15,7 @@ import (
        "crypto/internal/hpke"
        "crypto/rsa"
        "crypto/tls/internal/fips140tls"
+       "crypto/x509"
        "errors"
        "hash"
        "internal/byteorder"
@@ -407,8 +408,13 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error {
                if sessionHasClientCerts && c.config.time().After(sessionState.peerCertificates[0].NotAfter) {
                        continue
                }
+               opts := x509.VerifyOptions{
+                       CurrentTime: c.config.time(),
+                       Roots:       c.config.ClientCAs,
+                       KeyUsages:   []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
+               }
                if sessionHasClientCerts && c.config.ClientAuth >= VerifyClientCertIfGiven &&
-                       !anyUnexpiredChain(sessionState.verifiedChains, c.config.time()) {
+                       !anyValidVerifiedChain(sessionState.verifiedChains, opts) {
                        continue
                }