Stowage/go
2
0
Fork 0
mirror of https://github.com/golang/go.git synced 2026-02-07 02:09:55 +00:00
Code Issues Projects Releases Packages Wiki Activity

crypto/tls: revalidate whole chain on resumption on Windows and macOS

Browse source 
TestHandshakeChangeRootCAsResumption and TestHandshakeGetConfigForClientDifferentClientCAs 
changed because previously rootA and rootB shared Subject and SPKI,
which made the new full-chain revalidation check succeed, as the
same leaf would verify against both roots.

Fixes #77376

Cq-Include-Trybots: luci.golang.try:gotip-darwin-arm64-longtest
Change-Id: I60bed694bdc621c9e83f1bd8a8224c016a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/741361
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
This commit is contained in:
Filippo Valsorda 2026-01-30 18:07:23 +01:00 • committed by Gopher Robot
parent 31c9bcb103
commit b691a2edc7
3 changed files with 77 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

31
src/crypto/tls/common.go
View file

@ -22,6 +22,7 @@ import (
"internal/godebug"
"io"
"net"
"runtime"
"slices"
"strings"
"sync"
@ -1873,15 +1874,27 @@ func anyValidVerifiedChain(verifiedChains [][]*x509.Certificate, opts x509.Verif
}) {
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
// Since we already validated the chain, we only care that it is rooted
// in a CA in opts.Roots. On platforms where we control chain validation
// (e.g. not Windows or macOS) this is a simple lookup in the CertPool
// internal hash map, which we can simulate by running Verify on the
// root. On other platforms, we have to do full verification again,
// because EKU handling might differ. We will want to replace this with
// CertPool.Contains if/once that is available. See go.dev/issue/77376.
if runtime.GOOS == "windows" || runtime.GOOS == "darwin" || runtime.GOOS == "ios" {
opts.Intermediates = x509.NewCertPool()
for _, cert := range chain[1:max(1, len(chain)-1)] {
opts.Intermediates.AddCert(cert)
}
leaf := chain[0]
if _, err := leaf.Verify(opts); err == nil {
return true
}
} else {
root := chain[len(chain)-1]
if _, err := root.Verify(opts); err == nil {
return true
}
}
}
return false

28
src/crypto/tls/handshake_server_test.go
View file

@ -2302,7 +2302,7 @@ func testHandshakeGetConfigForClientDifferentClientCAs(t *testing.T, version uin
if err != nil {
t.Fatalf("ParseCertificate: %v", err)
}
rootDER, err = x509.CreateCertificate(rand.Reader, tmpl, tmpl, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
rootDER, err = x509.CreateCertificate(rand.Reader, tmpl, tmpl, &testRSA2048PrivateKey.PublicKey, testRSA2048PrivateKey)
if err != nil {
t.Fatalf("CreateCertificate: %v", err)
}
@ -2318,7 +2318,11 @@ func testHandshakeGetConfigForClientDifferentClientCAs(t *testing.T, version uin
NotAfter: now.Add(time.Hour * 24),
KeyUsage: x509.KeyUsageDigitalSignature,
}
certDER, err := x509.CreateCertificate(rand.Reader, tmpl, rootA, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
certA, err := x509.CreateCertificate(rand.Reader, tmpl, rootA, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
if err != nil {
t.Fatalf("CreateCertificate: %v", err)
}
certB, err := x509.CreateCertificate(rand.Reader, tmpl, rootB, &testECDSAPrivateKey.PublicKey, testRSA2048PrivateKey)
if err != nil {
t.Fatalf("CreateCertificate: %v", err)
}
@ -2326,7 +2330,7 @@ func testHandshakeGetConfigForClientDifferentClientCAs(t *testing.T, version uin
serverConfig := testConfig.Clone()
serverConfig.MaxVersion = version
serverConfig.Certificates = []Certificate{{
Certificate: [][]byte{certDER},
Certificate: [][]byte{certA},
PrivateKey: testECDSAPrivateKey,
}}
serverConfig.Time = func() time.Time {
@ -2351,7 +2355,7 @@ func testHandshakeGetConfigForClientDifferentClientCAs(t *testing.T, version uin
clientConfig := testConfig.Clone()
clientConfig.MaxVersion = version
clientConfig.Certificates = []Certificate{{
Certificate: [][]byte{certDER},
Certificate: [][]byte{certA},
PrivateKey: testECDSAPrivateKey,
}}
clientConfig.ClientSessionCache = NewLRUClientSessionCache(32)
@ -2380,6 +2384,8 @@ func testHandshakeGetConfigForClientDifferentClientCAs(t *testing.T, version uin
testResume(t, serverConfig, clientConfig, false)
testResume(t, serverConfig, clientConfig, true)
clientConfig.Certificates[0].Certificate = [][]byte{certB}
// 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
@ -2414,7 +2420,7 @@ func testHandshakeChangeRootCAsResumption(t *testing.T, version uint16) {
if err != nil {
t.Fatalf("ParseCertificate: %v", err)
}
rootDER, err = x509.CreateCertificate(rand.Reader, tmpl, tmpl, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
rootDER, err = x509.CreateCertificate(rand.Reader, tmpl, tmpl, &testRSA2048PrivateKey.PublicKey, testRSA2048PrivateKey)
if err != nil {
t.Fatalf("CreateCertificate: %v", err)
}
@ -2430,7 +2436,11 @@ func testHandshakeChangeRootCAsResumption(t *testing.T, version uint16) {
NotAfter: now.Add(time.Hour * 24),
KeyUsage: x509.KeyUsageDigitalSignature,
}
certDER, err := x509.CreateCertificate(rand.Reader, tmpl, rootA, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
certA, err := x509.CreateCertificate(rand.Reader, tmpl, rootA, &testECDSAPrivateKey.PublicKey, testECDSAPrivateKey)
if err != nil {
t.Fatalf("CreateCertificate: %v", err)
}
certB, err := x509.CreateCertificate(rand.Reader, tmpl, rootB, &testECDSAPrivateKey.PublicKey, testRSA2048PrivateKey)
if err != nil {
t.Fatalf("CreateCertificate: %v", err)
}
@ -2438,7 +2448,7 @@ func testHandshakeChangeRootCAsResumption(t *testing.T, version uint16) {
serverConfig := testConfig.Clone()
serverConfig.MaxVersion = version
serverConfig.Certificates = []Certificate{{
Certificate: [][]byte{certDER},
Certificate: [][]byte{certA},
PrivateKey: testECDSAPrivateKey,
}}
serverConfig.Time = func() time.Time {
@ -2453,7 +2463,7 @@ func testHandshakeChangeRootCAsResumption(t *testing.T, version uint16) {
clientConfig := testConfig.Clone()
clientConfig.MaxVersion = version
clientConfig.Certificates = []Certificate{{
Certificate: [][]byte{certDER},
Certificate: [][]byte{certA},
PrivateKey: testECDSAPrivateKey,
}}
clientConfig.ClientSessionCache = NewLRUClientSessionCache(32)
@ -2486,6 +2496,8 @@ func testHandshakeChangeRootCAsResumption(t *testing.T, version uint16) {
clientConfig.RootCAs = x509.NewCertPool()
clientConfig.RootCAs.AddCert(rootB)
serverConfig.Certificates[0].Certificate = [][]byte{certB}
testResume(t, serverConfig, clientConfig, false)
testResume(t, serverConfig, clientConfig, true)
}

35
src/crypto/tls/tls_test.go
View file

@ -572,6 +572,41 @@ func TestVerifyHostname(t *testing.T) {
}
}
func TestRealResumption(t *testing.T) {
testenv.MustHaveExternalNetwork(t)
config := &Config{
ServerName: "yahoo.com",
ClientSessionCache: NewLRUClientSessionCache(0),
}
for range 10 {
conn, err := Dial("tcp", "yahoo.com:443", config)
if err != nil {
t.Log("Dial error:", err)
continue
}
// Do a read to consume the NewSessionTicket messages.
fmt.Fprintf(conn, "GET / HTTP/1.1\r\nHost: yahoo.com\r\nConnection: close\r\n\r\n")
conn.Read(make([]byte, 4096))
conn.Close()
conn, err = Dial("tcp", "yahoo.com:443", config)
if err != nil {
t.Log("second Dial error:", err)
continue
}
state := conn.ConnectionState()
conn.Close()
if state.DidResume {
return
}
}
t.Fatal("no connection used session resumption")
}
func TestConnCloseBreakingWrite(t *testing.T) {
ln := newLocalListener(t)
defer ln.Close()