diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go index 6fe6f34cd21..7208e69334a 100644 --- a/src/crypto/tls/common.go +++ b/src/crypto/tls/common.go @@ -1803,3 +1803,16 @@ func fipsAllowChain(chain []*x509.Certificate) bool { return true } + +// 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 { + 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 + }) { + return true + } + } + return false +} diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go index 8d09d186529..3752df09b6d 100644 --- a/src/crypto/tls/handshake_client.go +++ b/src/crypto/tls/handshake_client.go @@ -429,9 +429,6 @@ func (c *Conn) loadSession(hello *clientHelloMsg) ( return nil, nil, nil, nil } - // Check that the cached server certificate is not expired, and that it's - // valid for the ServerName. This should be ensured by the cache key, but - // protect the application from a faulty ClientSessionCache implementation. if c.config.time().After(session.peerCertificates[0].NotAfter) { // Expired certificate, delete the entry. c.config.ClientSessionCache.Put(cacheKey, nil) @@ -443,6 +440,13 @@ func (c *Conn) loadSession(hello *clientHelloMsg) ( return nil, nil, nil, nil } if err := session.peerCertificates[0].VerifyHostname(c.config.ServerName); err != nil { + // This should be ensured by the cache key, but protect the + // application from a faulty ClientSessionCache implementation. + return nil, nil, nil, nil + } + if !anyUnexpiredChain(session.verifiedChains, c.config.time()) { + // 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 088c66fadb2..d4d05e54629 100644 --- a/src/crypto/tls/handshake_server.go +++ b/src/crypto/tls/handshake_server.go @@ -524,7 +524,7 @@ func (hs *serverHandshakeState) checkForResumption() error { return nil } if sessionHasClientCerts && c.config.ClientAuth >= VerifyClientCertIfGiven && - len(sessionState.verifiedChains) == 0 { + !anyUnexpiredChain(sessionState.verifiedChains, c.config.time()) { return nil } diff --git a/src/crypto/tls/handshake_server_test.go b/src/crypto/tls/handshake_server_test.go index a6d64a506a0..406dab48eef 100644 --- 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,124 @@ func TestHandshakeContextHierarchy(t *testing.T) { t.Errorf("Unexpected client error: %v", err) } } + +func TestHandshakeChainExpiryResumption(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) (leafDER, expiredLeafDER []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, + } + leafCertDER, err := x509.CreateCertificate(rand.Reader, tmpl, root, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey) + if err != nil { + t.Fatalf("CreateCertificate: %v", err) + } + tmpl.NotBefore, tmpl.NotAfter = leafNotAfter.Add(-time.Hour*24*365), leafNotAfter.Add(-time.Hour*24*364) + expiredLeafDERCertDER, err := x509.CreateCertificate(rand.Reader, tmpl, root, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey) + if err != nil { + t.Fatalf("CreateCertificate: %v", err) + } + + return leafCertDER, expiredLeafDERCertDER, root + } + testExpiration := func(name string, leafNotAfter, rootNotAfter time.Time) { + t.Run(name, func(t *testing.T) { + initialLeafDER, expiredLeafDER, initialRoot := createChain(leafNotAfter, rootNotAfter) + + serverConfig := testConfig.Clone() + serverConfig.MaxVersion = version + serverConfig.Certificates = []Certificate{{ + Certificate: [][]byte{initialLeafDER, expiredLeafDER}, + PrivateKey: testECDSAPrivateKey, + }} + serverConfig.ClientCAs = x509.NewCertPool() + serverConfig.ClientCAs.AddCert(initialRoot) + serverConfig.ClientAuth = RequireAndVerifyClientCert + serverConfig.Time = func() time.Time { + return now + } + serverConfig.InsecureSkipVerify = false + serverConfig.ServerName = "expired-resume.example.com" + + clientConfig := testConfig.Clone() + clientConfig.MaxVersion = version + clientConfig.Certificates = []Certificate{{ + Certificate: [][]byte{initialLeafDER, expiredLeafDER}, + PrivateKey: testECDSAPrivateKey, + }} + clientConfig.RootCAs = x509.NewCertPool() + clientConfig.RootCAs.AddCert(initialRoot) + clientConfig.ServerName = "expired-resume.example.com" + clientConfig.ClientSessionCache = NewLRUClientSessionCache(32) + clientConfig.InsecureSkipVerify = false + clientConfig.ServerName = "expired-resume.example.com" + clientConfig.Time = func() time.Time { + return now + } + + 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) + + expiredNow := time.Unix(0, min(leafNotAfter.UnixNano(), rootNotAfter.UnixNano())).Add(time.Minute) + + freshLeafDER, expiredLeafDER, freshRoot := createChain(expiredNow.Add(time.Hour), expiredNow.Add(time.Hour)) + clientConfig.Certificates = []Certificate{{ + Certificate: [][]byte{freshLeafDER, expiredLeafDER}, + PrivateKey: testECDSAPrivateKey, + }} + serverConfig.Time = func() time.Time { + return expiredNow + } + serverConfig.ClientCAs = x509.NewCertPool() + serverConfig.ClientCAs.AddCert(freshRoot) + + testResume(t, serverConfig, clientConfig, false) + }) + } + + testExpiration("LeafExpiresBeforeRoot", now.Add(2*time.Hour), now.Add(3*time.Hour)) + testExpiration("LeafExpiresAfterRoot", now.Add(2*time.Hour), now.Add(time.Hour)) +} diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go index 71f6c1b5a4c..3dba595331d 100644 --- a/src/crypto/tls/handshake_server_tls13.go +++ b/src/crypto/tls/handshake_server_tls13.go @@ -410,7 +410,7 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error { continue } if sessionHasClientCerts && c.config.ClientAuth >= VerifyClientCertIfGiven && - len(sessionState.verifiedChains) == 0 { + !anyUnexpiredChain(sessionState.verifiedChains, c.config.time()) { continue }