From 088ba94439775e025b90790f1c8db49ee2e7017f Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Thu, 8 Jan 2026 02:23:02 +0100 Subject: crypto/rsa: log key on test failure For #74326 Change-Id: If1e61db22c9e7192e5dd56cd36141e5b6a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/734640 LUCI-TryBot-Result: Go LUCI Auto-Submit: Filippo Valsorda Reviewed-by: Daniel McCarney Reviewed-by: Roland Shoemaker Reviewed-by: Junyang Shao --- src/crypto/rsa/rsa_test.go | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/crypto') diff --git a/src/crypto/rsa/rsa_test.go b/src/crypto/rsa/rsa_test.go index 5ae4c1dd20..124fba1e8a 100644 --- a/src/crypto/rsa/rsa_test.go +++ b/src/crypto/rsa/rsa_test.go @@ -145,6 +145,12 @@ d8Y7 } func testKeyBasics(t *testing.T, priv *PrivateKey) { + defer func() { + if t.Failed() { + t.Logf("failed key: %#v", priv) + } + }() + if err := priv.Validate(); err != nil { t.Errorf("Validate() failed: %s", err) } -- cgit v1.3 From df6c351aa4bbc8805406bfef979e62f59fc76da9 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Fri, 9 Jan 2026 19:01:50 +0100 Subject: crypto: use testenv.Executable(t) instead of os.Args[0] in tests Change-Id: Ib0ec1f05e51a4295a9369d6e8c6b61976a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/735260 LUCI-TryBot-Result: Go LUCI Reviewed-by: Junyang Shao Reviewed-by: Daniel McCarney Auto-Submit: Filippo Valsorda Reviewed-by: Michael Pratt --- src/crypto/internal/fips140test/acvp_test.go | 4 ++-- src/crypto/internal/fips140test/check_test.go | 2 +- src/crypto/tls/bogo_shim_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'src/crypto') diff --git a/src/crypto/internal/fips140test/acvp_test.go b/src/crypto/internal/fips140test/acvp_test.go index e94bab74fd..6a0b46af2b 100644 --- a/src/crypto/internal/fips140test/acvp_test.go +++ b/src/crypto/internal/fips140test/acvp_test.go @@ -2146,7 +2146,7 @@ func TestACVP(t *testing.T) { } configPath := filepath.Join(cwd, testConfigFile) t.Logf("running check_expected.go\ncwd: %q\ndata_dir: %q\nconfig: %q\ntool: %q\nmodule-wrapper: %q\n", - cwd, dataDir, configPath, toolPath, os.Args[0]) + cwd, dataDir, configPath, toolPath, testenv.Executable(t)) // Run the check_expected test driver using the acvptool we built, and this test binary as the // module wrapper. The file paths in the config file are specified relative to the dataDir root @@ -2157,7 +2157,7 @@ func TestACVP(t *testing.T) { "-tool", toolPath, // Note: module prefix must match Wrapper value in testConfigFile. - "-module-wrappers", "go:" + os.Args[0], + "-module-wrappers", "go:" + testenv.Executable(t), "-tests", configPath, } cmd = testenv.Command(t, testenv.GoToolPath(t), args...) diff --git a/src/crypto/internal/fips140test/check_test.go b/src/crypto/internal/fips140test/check_test.go index 8aef1f9b9b..d70ffbb77f 100644 --- a/src/crypto/internal/fips140test/check_test.go +++ b/src/crypto/internal/fips140test/check_test.go @@ -46,7 +46,7 @@ func TestIntegrityCheckFailure(t *testing.T) { moduleStatus(t) cryptotest.MustSupportFIPS140(t) - bin, err := os.ReadFile(os.Args[0]) + bin, err := os.ReadFile(testenv.Executable(t)) if err != nil { t.Fatal(err) } diff --git a/src/crypto/tls/bogo_shim_test.go b/src/crypto/tls/bogo_shim_test.go index 1b5fc49c4f..ccac47c271 100644 --- a/src/crypto/tls/bogo_shim_test.go +++ b/src/crypto/tls/bogo_shim_test.go @@ -577,7 +577,7 @@ func TestBogoSuite(t *testing.T) { "test", ".", fmt.Sprintf("-shim-config=%s", filepath.Join(cwd, "bogo_config.json")), - fmt.Sprintf("-shim-path=%s", os.Args[0]), + fmt.Sprintf("-shim-path=%s", testenv.Executable(t)), "-shim-extra-flags=-bogo-mode", "-allow-unimplemented", "-loose-errors", // TODO(roland): this should be removed eventually -- cgit v1.3 From 2bc4315d92a70d9a5e895d60defba4f799798806 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Fri, 19 Dec 2025 23:14:05 +0100 Subject: crypto/internal/fips140test: add ML-DSA to FIPS 140-3 functional tests Change-Id: I568d28d27d2bc55bbadcc678a2fcf9d36a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/731540 Reviewed-by: Roland Shoemaker Reviewed-by: Junyang Shao Auto-Submit: Filippo Valsorda LUCI-TryBot-Result: Go LUCI --- .../internal/fips140test/cast_fips140v1.0_test.go | 9 ------ .../internal/fips140test/cast_fips140v1.26_test.go | 16 ----------- .../internal/fips140test/fips140v1.0_test.go | 13 +++++++++ .../internal/fips140test/fips140v1.26_test.go | 33 ++++++++++++++++++++++ src/crypto/internal/fips140test/fips_test.go | 2 ++ 5 files changed, 48 insertions(+), 25 deletions(-) delete mode 100644 src/crypto/internal/fips140test/cast_fips140v1.0_test.go delete mode 100644 src/crypto/internal/fips140test/cast_fips140v1.26_test.go create mode 100644 src/crypto/internal/fips140test/fips140v1.0_test.go create mode 100644 src/crypto/internal/fips140test/fips140v1.26_test.go (limited to 'src/crypto') diff --git a/src/crypto/internal/fips140test/cast_fips140v1.0_test.go b/src/crypto/internal/fips140test/cast_fips140v1.0_test.go deleted file mode 100644 index b9ddfe4d8b..0000000000 --- a/src/crypto/internal/fips140test/cast_fips140v1.0_test.go +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright 2024 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build fips140v1.0 - -package fipstest - -func fips140v126Conditionals() {} diff --git a/src/crypto/internal/fips140test/cast_fips140v1.26_test.go b/src/crypto/internal/fips140test/cast_fips140v1.26_test.go deleted file mode 100644 index ef79068c38..0000000000 --- a/src/crypto/internal/fips140test/cast_fips140v1.26_test.go +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright 2024 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build !fips140v1.0 - -package fipstest - -import "crypto/internal/fips140/mldsa" - -func fips140v126Conditionals() { - // ML-DSA sign and verify PCT - kMLDSA := mldsa.GenerateKey44() - // ML-DSA-44 - mldsa.SignDeterministic(kMLDSA, make([]byte, 32), "") -} diff --git a/src/crypto/internal/fips140test/fips140v1.0_test.go b/src/crypto/internal/fips140test/fips140v1.0_test.go new file mode 100644 index 0000000000..262ef61d5c --- /dev/null +++ b/src/crypto/internal/fips140test/fips140v1.0_test.go @@ -0,0 +1,13 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build fips140v1.0 + +package fipstest + +import "testing" + +func fips140v126Conditionals() {} + +func testFIPS140v126(t *testing.T, plaintext []byte) {} diff --git a/src/crypto/internal/fips140test/fips140v1.26_test.go b/src/crypto/internal/fips140test/fips140v1.26_test.go new file mode 100644 index 0000000000..6cd9f4fe40 --- /dev/null +++ b/src/crypto/internal/fips140test/fips140v1.26_test.go @@ -0,0 +1,33 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build !fips140v1.0 + +package fipstest + +import ( + "crypto/internal/fips140/mldsa" + "testing" +) + +func fips140v126Conditionals() { + // ML-DSA sign and verify PCT + kMLDSA := mldsa.GenerateKey44() + // ML-DSA-44 + mldsa.SignDeterministic(kMLDSA, make([]byte, 32), "") +} + +func testFIPS140v126(t *testing.T, plaintext []byte) { + t.Run("ML-DSA KeyGen, SigGen, SigVer", func(t *testing.T) { + ensureServiceIndicator(t) + k := mldsa.GenerateKey44() + + sig, err := mldsa.SignDeterministic(k, plaintext, "") + fatalIfErr(t, err) + t.Logf("ML-DSA signature: %x", sig) + + err = mldsa.Verify(k.PublicKey(), plaintext, sig, "") + fatalIfErr(t, err) + }) +} diff --git a/src/crypto/internal/fips140test/fips_test.go b/src/crypto/internal/fips140test/fips_test.go index 52fc9d3488..7f2824ca9a 100644 --- a/src/crypto/internal/fips140test/fips_test.go +++ b/src/crypto/internal/fips140test/fips_test.go @@ -101,6 +101,8 @@ func TestFIPS140(t *testing.T) { aesBlock, err := aes.New(aesKey) fatalIfErr(t, err) + testFIPS140v126(t, plaintext) + t.Run("AES-CTR", func(t *testing.T) { ensureServiceIndicator(t) ctr := aes.NewCTR(aesBlock, aesIV) -- cgit v1.3 From bba24719a4cad5cc8d771fc9cfff5a38019d554a Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 6 Jan 2026 14:36:01 -0800 Subject: crypto/tls: don't copy auto-rotated session ticket keys in Config.Clone Once a tls.Config is used, it is not safe to mutate. We provide the Clone method in order to allow users to copy and modify a Config that is in use. If Config.SessionTicketKey is not populated, and if Config.SetSessionTicketKeys has not been called, we automatically populate and rotate session ticket keys. Clone was previously copying these keys into the new Config, meaning that two Configs could share the same auto-rotated session ticket keys. This could allow sessions to be resumed across different Configs, which may have completely different configurations. This change updates Clone to not copy the auto-rotated session ticket keys. Additionally, when resuming a session, check that not just that the leaf certificate is unexpired, but that the entire certificate chain is still unexpired. Fixes #77113 Fixes CVE-2025-68121 Change-Id: I011df7329de83068d11b3f0c793763692d018a98 Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/3300 Reviewed-by: Damien Neil Reviewed-by: Nicholas Husin Reviewed-on: https://go-review.googlesource.com/c/go/+/736709 Auto-Submit: Michael Pratt Reviewed-by: Junyang Shao LUCI-TryBot-Result: Go LUCI --- src/crypto/tls/common.go | 7 ++- src/crypto/tls/handshake_server.go | 9 ++- src/crypto/tls/handshake_server_test.go | 101 +++++++++++++++++++++++++++++++ src/crypto/tls/handshake_server_tls13.go | 10 ++- src/crypto/tls/tls_test.go | 11 +++- 5 files changed, 132 insertions(+), 6 deletions(-) (limited to 'src/crypto') diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go index 099a11ca63..093869ac8b 100644 --- a/src/crypto/tls/common.go +++ b/src/crypto/tls/common.go @@ -980,6 +980,10 @@ 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 @@ -1020,7 +1024,8 @@ func (c *Config) Clone() *Config { EncryptedClientHelloRejectionVerify: c.EncryptedClientHelloRejectionVerify, EncryptedClientHelloKeys: c.EncryptedClientHelloKeys, sessionTicketKeys: c.sessionTicketKeys, - autoSessionTicketKeys: c.autoSessionTicketKeys, + // We explicitly do not copy autoSessionTicketKeys, so that Configs do + // not share the same auto-rotated keys. } } diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go index efdaeae6f7..06675a8ce9 100644 --- a/src/crypto/tls/handshake_server.go +++ b/src/crypto/tls/handshake_server.go @@ -520,8 +520,13 @@ func (hs *serverHandshakeState) checkForResumption() error { if sessionHasClientCerts && c.config.ClientAuth == NoClientCert { return nil } - if sessionHasClientCerts && c.config.time().After(sessionState.peerCertificates[0].NotAfter) { - return nil + if sessionHasClientCerts { + now := c.config.time() + for _, c := range sessionState.peerCertificates { + if now.After(c.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 7e35c25259..41ae87050e 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" @@ -2153,3 +2154,103 @@ 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 b066924e29..0033164f65 100644 --- a/src/crypto/tls/handshake_server_tls13.go +++ b/src/crypto/tls/handshake_server_tls13.go @@ -314,6 +314,7 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error { return nil } +pskIdentityLoop: for i, identity := range hs.clientHello.pskIdentities { if i >= maxClientPSKIdentities { break @@ -366,8 +367,13 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error { if sessionHasClientCerts && c.config.ClientAuth == NoClientCert { continue } - if sessionHasClientCerts && c.config.time().After(sessionState.peerCertificates[0].NotAfter) { - continue + if sessionHasClientCerts { + now := c.config.time() + for _, c := range sessionState.peerCertificates { + if now.After(c.NotAfter) { + continue pskIdentityLoop + } + } } 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 39ebb9d2f1..48428e4cc9 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) { @@ -2461,3 +2461,12 @@ func (s messageOnlySigner) SignMessage(rand io.Reader, msg []byte, opts crypto.S digest := h.Sum(nil) return s.Signer.Sign(rand, digest, opts) } + +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") + } +} -- cgit v1.3