[release-branch.go1.26] crypto/tls: revalidate whole chain on resumption on Windows and macOS

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.

Updates #77376
Fixes #77426

Cq-Include-Trybots: luci.golang.try:go1.26-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>
(cherry picked from commit b691a2edc7f5863f61a07c4a4f087eef1a15a704)
Reviewed-on: https://go-review.googlesource.com/c/go/+/741680
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>

3 files changed, 77 insertions, 17 deletions

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index dfd24d70b0..85a2a9e811 100644
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -22,6 +22,7 @@ import (
 	"internal/godebug"
 	"io"
 	"net"
+	"runtime"
 	"slices"
 	"strings"
 	"sync"
@@ -1861,15 +1862,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
diff --git a/src/crypto/tls/handshake_server_test.go b/src/crypto/tls/handshake_server_test.go
index cbcf012974..736bb831ab 100644
--- a/src/crypto/tls/handshake_server_test.go
+++ b/src/crypto/tls/handshake_server_test.go
@@ -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)
 }
diff --git a/src/crypto/tls/tls_test.go b/src/crypto/tls/tls_test.go
index 39ebb9d2f1..4513008a5f 100644
--- a/src/crypto/tls/tls_test.go
+++ b/src/crypto/tls/tls_test.go
@@ -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()