Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto/tls: TestVerifyConnection/TLSv12 failures #71077

Closed
gopherbot opened this issue Jan 1, 2025 · 13 comments
Closed

crypto/tls: TestVerifyConnection/TLSv12 failures #71077

gopherbot opened this issue Jan 1, 2025 · 13 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@gopherbot
Copy link
Contributor

#!watchflakes
default <- pkg == "crypto/tls" && test == "TestVerifyConnection/TLSv12"

Issue created automatically to collect these failures.

Example (log):

=== RUN   TestVerifyConnection/TLSv12
    handshake_client_test.go:1766: RequireAndVerifyClientCert-FullHandshake: handshake failed: server: remote error: tls: bad certificate
        client: tls: failed to verify certificate: x509: certificate has expired or is not yet valid: current time 2025-01-01T02:46:41Z is after 2025-01-01T00:00:00Z
--- FAIL: TestVerifyConnection/TLSv12 (0.08s)

watchflakes

@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
default <- pkg == "crypto/tls" && test == "TestVerifyConnection/TLSv12"
2024-12-30 20:23 gotip-netbsd-arm go@b702a26c crypto/tls.TestVerifyConnection/TLSv12 (log)
=== RUN   TestVerifyConnection/TLSv12
    handshake_client_test.go:1766: RequireAndVerifyClientCert-FullHandshake: handshake failed: server: remote error: tls: bad certificate
        client: tls: failed to verify certificate: x509: certificate has expired or is not yet valid: current time 2025-01-01T02:46:41Z is after 2025-01-01T00:00:00Z
--- FAIL: TestVerifyConnection/TLSv12 (0.08s)

watchflakes

@gopherbot gopherbot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 1, 2025
@4a6f656c
Copy link
Contributor

4a6f656c commented Jan 1, 2025

Most of the test certificates (e.g. testRSACertificate, testRSACertificateIssuer, testRSA2048CertificateIssuer) have a not after of Jan 1 00:00:00 2025 GMT.

@dmitshur dmitshur added the Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) label Jan 1, 2025
@dmitshur dmitshur added this to the Go1.24 milestone Jan 1, 2025
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/639595 mentions this issue: crypto/tls: update the expiration time of the certificate in the test case

@ianlancetaylor
Copy link
Member

As far as I can tell one of the failing certificates is testRSA2048CertificateIssuer in crypto/tls/handshake_test.go. This certificate was just added in https://go.dev/cl/629736 committed in November, 2024, so this test was only going to work for a few weeks. Unfortunately I don't know how to correctly recreate this certificate with a more useful expiration date.

CC @golang/security @rolandshoemaker @FiloSottile @cpu

@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/639655 mentions this issue: crypto/tls: fix Config.Time in tests using expired certificates

@cuishuang
Copy link
Contributor

cuishuang commented Jan 2, 2025

Change https://go.dev/cl/639595 mentions this issue: crypto/tls: update the expiration time of the certificate in the test case

I tried to generate a certificate with an expiration date of 2026-01-01, which passed the verification of func (c *Certificate) isValid(certType int, currentChain []*Certificate, opts *VerifyOptions) error, but the complete test still reported an error

The maintainer is fixing the problem here in CL 639655, and I will abandon this CL.

@abner-chenc
Copy link
Contributor

abner-chenc commented Jan 3, 2025

I also get the same problem when executing all.bash in Go1.22 and Go1.23 branches. I think CL 639655 also needs to be backported to Go1.23 and Go1.22 release branches.

@FiloSottile
Copy link
Contributor

FiloSottile commented Jan 3, 2025

@gopherbot please open backport issues for CL 639655. Tests are broken across releases.

@gopherbot
Copy link
Contributor Author

Backport issue(s) opened: #71103 (for 1.22), #71104 (for 1.23).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@dmitshur dmitshur added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 3, 2025
sorend pushed a commit to sorend/guix-mirror that referenced this issue Jan 3, 2025
As seen in <https://ci.guix.gnu.org/build/7713190/log/raw>.
Certificates which are used in tests are not valid after Jan 1 00:00:00
2025 GMT, Happy New Year!.
See <golang/go#71077>.

* gnu/packages/golang.scm (go-1.21) [arguments] <phases>: Add
'skip-crypto-tls-tests.

Change-Id: Id9f8ad93201aedae4f4451ee8b7b9cf40cd33cdb
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/640237 mentions this issue: [release-branch.go1.22] crypto/tls: fix Config.Time in tests using expired certificates

@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/640315 mentions this issue: [release-branch.go1.23] crypto/tls: fix Config.Time in tests using expired certificates

copybara-service bot pushed a commit to google/gvisor that referenced this issue Jan 5, 2025
This test is broken in Go 1.22 as per golang/go#71077.

Fixes #11338

PiperOrigin-RevId: 712237566
copybara-service bot pushed a commit to google/gvisor that referenced this issue Jan 6, 2025
This test is broken in Go 1.22 as per golang/go#71077.

Updates #11338

PiperOrigin-RevId: 712237566
copybara-service bot pushed a commit to google/gvisor that referenced this issue Jan 6, 2025
This test is broken in Go 1.22 as per golang/go#71077.

Updates #11338

PiperOrigin-RevId: 712237566
copybara-service bot pushed a commit to google/gvisor that referenced this issue Jan 6, 2025
This test is broken in Go 1.22 as per golang/go#71077.

Updates #11338

PiperOrigin-RevId: 712398784
@bwesterb
Copy link

bwesterb commented Jan 6, 2025

Presently crypto/tls doesn't expect config.Time to increment. In case that changes you can do something like

diff --git a/src/crypto/tls/handshake_test.go b/src/crypto/tls/handshake_test.go
index baf54c9f25..fff08e3045 100644
--- a/src/crypto/tls/handshake_test.go
+++ b/src/crypto/tls/handshake_test.go
@@ -20,6 +20,7 @@ import (
        "strconv"
        "strings"
        "sync"
+       "sync/atomic"
        "testing"
        "time"
 )
@@ -438,10 +439,17 @@ func fromHex(s string) []byte {
        return b
 }
 
-// testTime is 2016-10-20T17:32:09.000Z, which is within the validity period of
-// [testRSACertificate], [testRSACertificateIssuer], [testRSA2048Certificate],
-// [testRSA2048CertificateIssuer], and [testECDSACertificate].
-var testTime = func() time.Time { return time.Unix(1476984729, 0) }
+// Returns currentTestTime and then increments by a second.
+var testTime = func() time.Time { return time.Unix(currentTestTime.Add(1), 0) }
+
+// currentTestTime is initially 2016-10-20T17:32:09.000Z, which is well within
+// the validity period of [testRSACertificate], [testRSACertificateIssuer],
+// [testRSA2048Certificate], [testRSA2048CertificateIssuer], and [testECDSACertificate].
+var currentTestTime atomic.Int64
+
+func init() {
+       currentTestTime.Store(1476984729)
+}
 
 var testRSACertificate = fromHex("3082024b308201b4a003020102020900e8f09d3fe25beaa6300d06092a864886f70d01010b0500301f310b3009060355040a1302476f3110300e06035504031307476f20526f6f74301e170d3136303130313030303030305a170d3235303130313030303030305a301a310b3009060355040a1302476f310b300906035504031302476f30819f300d06092a864886f70d010101050003818d0030818902818100db467d932e12270648bc062821ab7ec4b6a25dfe1e5245887a3647a5080d92425bc281c0be97799840fb4f6d14fd2b138bc2a52e67d8d4099ed62238b74a0b74732bc234f1d193e596d9747bf3589f6c613cc0b041d4d92b2b2423775b1c3bbd755dce2054cfa163871d1e24c4f31d1a508baab61443ed97a77562f414c852d70203010001a38193308190300e0603551d0f0101ff0404030205a0301d0603551d250416301406082b0601050507030106082b06010505070302300c0603551d130101ff0402300030190603551d0e041204109f91161f43433e49a6de6db680d79f60301b0603551d230414301280104813494d137e1631bba301d5acab6e7b30190603551d1104123010820e6578616d706c652e676f6c616e67300d06092a864886f70d01010b0500038181009d30cc402b5b50a061cbbae55358e1ed8328a9581aa938a495a1ac315a1a84663d43d32dd90bf297dfd320643892243a00bccf9c7db74020015faad3166109a276fd13c3cce10c5ceeb18782f16c04ed73bbb343778d0c1cf10fa1d8408361c94c722b9daedb4606064df4c1b33ec0d1bd42d4dbfe3d1360845c21d33be9fae7")

NexediGitlab pushed a commit to SlapOS/slapos that referenced this issue Jan 7, 2025
golang tests fail because of golang/go#71077.
This patch backports golang/go@d1d9312.
go1.22 and go1.23 include this fix already [1].

/reported-by @Romain

[1] https://go-review.googlesource.com/c/go/+/640315
github-actions bot pushed a commit to guix-ru/guix that referenced this issue Jan 7, 2025
As seen in <https://ci.guix.gnu.org/build/7713190/log/raw>.
Certificates which are used in tests are not valid after Jan 1 00:00:00
2025 GMT, Happy New Year!.
See <golang/go#71077>.

* gnu/packages/golang.scm (go-1.21) [arguments] <phases>: Add
'skip-crypto-tls-tests.

Change-Id: Id9f8ad93201aedae4f4451ee8b7b9cf40cd33cdb
NexediGitlab pushed a commit to SlapOS/slapos that referenced this issue Jan 8, 2025
golang tests fail because of golang/go#71077.
This patch backports golang/go@d1d9312.
go1.22 and go1.23 include this fix already [1].

/reported-by @Romain

[1] https://go-review.googlesource.com/c/go/+/640315
gopherbot pushed a commit that referenced this issue Jan 8, 2025
…pired certificates

Updates #71077
Fixes #71104

Change-Id: I6a6a465685f3bd50a5bb35a160f87b59b74fa6af
Reviewed-on: https://go-review.googlesource.com/c/go/+/639655
Auto-Submit: Ian Lance Taylor <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Filippo Valsorda <[email protected]>
Auto-Submit: Damien Neil <[email protected]>
Reviewed-by: Joel Sing <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/640315
Reviewed-by: Filippo Valsorda <[email protected]>
gopherbot pushed a commit that referenced this issue Jan 8, 2025
…pired certificates

Updates #71077
Fixes #71103

Change-Id: I6a6a465685f3bd50a5bb35a160f87b59b74fa6af
Reviewed-on: https://go-review.googlesource.com/c/go/+/639655
Auto-Submit: Ian Lance Taylor <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Filippo Valsorda <[email protected]>
Auto-Submit: Damien Neil <[email protected]>
Reviewed-by: Joel Sing <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/640237
@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
default <- pkg == "crypto/tls" && test == "TestVerifyConnection/TLSv12"
2024-12-03 18:00 go1.22-linux-amd64 release-branch.go1.22@8f3f22ee crypto/tls.TestVerifyConnection/TLSv12 (log)
=== RUN   TestVerifyConnection/TLSv12
    handshake_client_test.go:1784: RequireAndVerifyClientCert-FullHandshake: handshake failed: remote error: tls: bad certificate
--- FAIL: TestVerifyConnection/TLSv12 (0.00s)
2024-12-18 17:42 go1.23-linux-amd64 release-branch.go1.23@1576793c crypto/tls.TestVerifyConnection/TLSv12 (log)
=== RUN   TestVerifyConnection/TLSv12
    handshake_client_test.go:1759: RequireAndVerifyClientCert-FullHandshake: handshake failed: server: remote error: tls: bad certificate
        client: tls: failed to verify certificate: x509: certificate has expired or is not yet valid: current time 2025-01-03T23:03:32Z is after 2025-01-01T00:00:00Z
--- FAIL: TestVerifyConnection/TLSv12 (0.00s)

watchflakes

NexediGitlab pushed a commit to SlapOS/slapos that referenced this issue Jan 8, 2025
golang tests fail because of golang/go#71077.
This patch backports golang/go@d1d9312.
go1.22 and go1.23 include this fix already [1].

[1] https://go-review.googlesource.com/c/go/+/640315

/reported-by @Romain
/reviewed-by @jerome @kirr @tomo
/reviewed-on https://lab.nexedi.com/nexedi/slapos/-/merge_requests/1713
github-actions bot pushed a commit to guix-ru/guix that referenced this issue Jan 9, 2025
As seen in <https://ci.guix.gnu.org/build/7713190/log/raw>.
Certificates which are used in tests are not valid after Jan 1 00:00:00
2025 GMT, Happy New Year!.
See <golang/go#71077>.

* gnu/packages/golang.scm (go-1.21) [arguments] <phases>: Add
'skip-crypto-tls-tests.

Change-Id: Id9f8ad93201aedae4f4451ee8b7b9cf40cd33cdb
sorend pushed a commit to sorend/guix-mirror that referenced this issue Jan 10, 2025
As seen in <https://ci.guix.gnu.org/build/7713190/log/raw>.
Certificates which are used in tests are not valid after Jan 1 00:00:00
2025 GMT, Happy New Year!.
See <golang/go#71077>.

* gnu/packages/golang.scm (go-1.21) [arguments] <phases>: Add
'skip-crypto-tls-tests.

Change-Id: Id9f8ad93201aedae4f4451ee8b7b9cf40cd33cdb
github-actions bot pushed a commit to guix-ru/guix that referenced this issue Jan 12, 2025
As seen in <https://ci.guix.gnu.org/build/7713190/log/raw>.
Certificates which are used in tests are not valid after Jan 1 00:00:00
2025 GMT, Happy New Year!.
See <golang/go#71077>.

* gnu/packages/golang.scm (go-1.21) [arguments] <phases>: Add
'skip-crypto-tls-tests.

Change-Id: Id9f8ad93201aedae4f4451ee8b7b9cf40cd33cdb
wyf9661 pushed a commit to wyf9661/go that referenced this issue Jan 21, 2025
Fixes golang#71077

Change-Id: I6a6a465685f3bd50a5bb35a160f87b59b74fa6af
Reviewed-on: https://go-review.googlesource.com/c/go/+/639655
Auto-Submit: Ian Lance Taylor <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Filippo Valsorda <[email protected]>
Auto-Submit: Damien Neil <[email protected]>
Reviewed-by: Joel Sing <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
sorend pushed a commit to sorend/guix-mirror that referenced this issue Jan 22, 2025
As seen in <https://ci.guix.gnu.org/build/7713190/log/raw>.
Certificates which are used in tests are not valid after Jan 1 00:00:00
2025 GMT, Happy New Year!.
See <golang/go#71077>.

* gnu/packages/golang.scm (go-1.21) [arguments] <phases>: Add
'skip-crypto-tls-tests.

Change-Id: Id9f8ad93201aedae4f4451ee8b7b9cf40cd33cdb
mpminardi pushed a commit to tailscale/go that referenced this issue Jan 28, 2025
…pired certificates

Updates golang#71077
Fixes golang#71104

Change-Id: I6a6a465685f3bd50a5bb35a160f87b59b74fa6af
Reviewed-on: https://go-review.googlesource.com/c/go/+/639655
Auto-Submit: Ian Lance Taylor <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Filippo Valsorda <[email protected]>
Auto-Submit: Damien Neil <[email protected]>
Reviewed-by: Joel Sing <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/640315
Reviewed-by: Filippo Valsorda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
Status: Done
Development

No branches or pull requests

8 participants