mirror of
https://github.com/golang/go.git
synced 2026-01-29 07:02:05 +03:00
[release-branch.go1.25] Revert "crypto/tls: don't copy auto-rotated session ticket keys in Config.Clone"
This reverts CL 736709 (commit bba24719a4).
Updates #77113
Updates #77356
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/+/740063
Reviewed-by: Nicholas Husin <nsh@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
This commit is contained in:
committed by
Gopher Robot
parent
0765a9d624
commit
6b1110a40f
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user