diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go index d6d4c917402..6fe6f34cd21 100644 --- a/src/crypto/tls/common.go +++ b/src/crypto/tls/common.go @@ -942,10 +942,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 @@ -986,8 +982,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 f244fc176b4..088c66fadb2 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 382d3f72809..a6d64a506a0 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" @@ -2122,103 +2121,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 6a30855dce7..71f6c1b5a4c 100644 --- a/src/crypto/tls/handshake_server_tls13.go +++ b/src/crypto/tls/handshake_server_tls13.go @@ -354,7 +354,6 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error { return nil } -pskIdentityLoop: for i, identity := range hs.clientHello.pskIdentities { if i >= maxClientPSKIdentities { break @@ -407,13 +406,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 0a791bd7b70..bfcc62ccfb8 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) { @@ -2347,12 +2347,3 @@ 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") - } -}