crypto/x509: TestHybridPool failures on Windows with `x509: certificate signed by unknown authority` #51599

bcmills · 2022-03-10T18:49:08Z

--- FAIL: TestHybridPool (15.02s)
    hybrid_pool_test.go:66: verification failed for google.com chain (empty pool): x509: certificate signed by unknown authority
FAIL
FAIL	crypto/x509	31.546s

greplogs --dashboard -md -l -e 'FAIL: TestHybridPool'

2022-03-10T16:06:29-1cf6770/windows-amd64-longtest

(attn @golang/security for triage)

The text was updated successfully, but these errors were encountered:

bcmills · 2022-03-30T20:17:54Z

greplogs --dashboard -md -l -e 'FAIL: TestHybridPool' --since=2022-03-11

2022-03-24T17:50:47-b95d332/windows-amd64-longtest

gopherbot · 2022-04-02T01:45:19Z

Change https://go.dev/cl/397694 mentions this issue: crypto/x509: local platform verifier tests on trybots

bcmills · 2022-05-12T12:43:12Z

greplogs --dashboard -md -l -e 'FAIL: TestHybridPool' --since=2022-03-25
2022-05-12T04:47:29-2bea43b/windows-amd64-longtest
2022-05-12T04:08:07-900ae5b/windows-amd64-longtest
2022-05-11T23:39:27-ec5bdef/windows-amd64-longtest
2022-05-11T20:11:34-c1105cf/windows-amd64-longtest
2022-05-11T15:00:05-d339d08/windows-amd64-longtest
2022-05-11T13:55:35-de9805c/windows-amd64-longtest
2022-05-11T08:12:15-7b314d2/windows-amd64-longtest
2022-05-10T22:12:11-6fd0520/windows-amd64-longtest

gopherbot · 2022-05-12T17:30:46Z

Change https://go.dev/cl/405914 mentions this issue: crypto/x509: attempt to prime windows root pool before hybrid test

In TestHybridPool attempt to prime to the windows root pool before the real test actually happens. This is a bit of a band-aid, with a better long term solution discussed in #52108. Updates #51599 Change-Id: I406add8d9cd9e3fae37bfc20b97f5479c10a52c2 Reviewed-on: https://go-review.googlesource.com/c/go/+/405914 Reviewed-by: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Roland Shoemaker <[email protected]>

bcmills · 2022-05-13T14:56:26Z

Due to #52591 the windows-amd64-longtest builders have only completed one run since the above CL went in, but so far they haven't failed with this mode again.

Marking WaitingForInfo to remind us to recheck the fix after more test runs have completed.

bcmills · 2022-05-18T13:55:41Z

There was a rash of failures yesterday — again all on Windows — but the failure mode is slightly different. 🤔

--- FAIL: TestPlatformVerifier (0.08s)
    --- FAIL: TestPlatformVerifier/wrong_host_for_leaf (0.02s)
        root_windows_test.go:109: unexpected verification error: got "x509: certificate has expired or is not yet valid: ", want "x509: certificate is valid for *.badssl.com, badssl.com, not wrong.host.badssl.com"
FAIL
FAIL	crypto/x509	1.942s

greplogs -l -e 'FAIL: TestPlatformVerifier .*(?:\n[ ]{4}.*)*badssl\.com' --since=2022-05-13
2022-05-17T18:55:46-668041e/windows-amd64-longtest
2022-05-17T18:19:34-076039a/windows-amd64-longtest
2022-05-17T18:13:13-afd181c/windows-amd64-longtest
2022-05-17T17:54:33-cc4957a/windows-amd64-longtest
2022-05-17T17:29:34-cf5fa2b/windows-amd64-longtest
2022-05-17T16:09:37-770e0e5/windows-amd64-longtest
2022-05-17T14:39:18-cf7ec0f/windows-amd64-longtest

bcmills · 2022-05-18T13:58:25Z

I think the above failures may have been due to a misconfiguration of the badssl.com service itself, although it does seem strange to me that it only failed on Windows.

I wonder if the Windows root store ended up pulling in one of the badssl.com certificates but not the others, causing the error to be reported from the wrong certificate? Or maybe the badssl.com service itself was misconfigured for a while...

dmitshur · 2022-06-01T16:17:08Z

@rolandshoemaker This is currently blocking beta 1. Any updates and do you expect it's safe to resolve after beta 1?

rolandshoemaker · 2022-06-03T23:35:31Z

Sorry for the lag. I think this is reasonable to resolve after beta 1. This failure isn't due to new behavior, it's simply surfacing behavior that always existed but was never tested. Any resolution (likely) requires changes to builders, which seems unlikely to happen before the beta. If we just want to remove this flake until we re-open the tree for 1.20, it would probably be reasonable to just skip this test until the tree re-opens.

heschi · 2022-06-08T16:15:09Z

@rolandshoemaker It seems like there's some confusion about whether this is a bug in the test, or a bug in badssl.com. If the former, is it feasible to fix before the final release? If the latter, is this test valuable enough to risk having a release get stuck due to it being misconfigured again?

rolandshoemaker · 2022-06-09T18:37:30Z

There are two different failure modes described here, the first is the failures in TestHybridPool, which are an inherent problem in how the test and the Windows root store interact (sometimes priming the pool fails/times out before our call to CertGetCertificateChain returns), and the second, the failures in TestPlatformVerifier , was that badssl.com was (for some short period of time) serving an expired certificate.

For the first issue, after landing CL 405914 I don't think we've seen these failures again, but they could still plausibly happen (if the builders are, for whatever reason, being incredibly slow).

For the second, you are right that we are entirely reliant on badssl.com being properly configured, and they could realistically break us by doing something bad.

I opened #52108 to solve both of these issues, but the solution is... complicated, and seems unlikely to happen before the final release. If we want to make sure these tests don't block the release, I think it's reasonable to just add skips until the release and then fix #52108 in the 1.20 cycle (this behavior has been included since 1.18, so we're relatively confident it's not horribly broken.)

heschi · 2022-06-09T19:11:05Z

IMO it's up to you if you want to add the skips; you're the one who has perspective on how valuable the tests are. Unless we have reason to expect badssl.com to break again it doesn't seem urgent to me and in that case I'd vote to close this issue.

rolandshoemaker · 2022-06-09T20:48:58Z

I think it's fine to close this out, if it pops back up we can revisit.

bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Mar 10, 2022

bcmills added this to the Go1.19 milestone Mar 10, 2022

rolandshoemaker self-assigned this Mar 18, 2022

rolandshoemaker mentioned this issue Apr 1, 2022

crypto/x509: come up with better solution for testing platform verifiers #52108

Open

bcmills changed the title ~~crypto/x509: TestHybridPool failure with x509: certificate signed by unknown authority~~ crypto/x509: TestHybridPool failures on Windows with x509: certificate signed by unknown authority May 12, 2022

bcmills added the OS-Windows label May 12, 2022

bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label May 13, 2022

bcmills removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label May 18, 2022

heschi added the okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 label Jun 6, 2022

rolandshoemaker closed this as completed Jun 9, 2022

rsc unassigned rolandshoemaker Jun 22, 2022

golang locked and limited conversation to collaborators Jun 22, 2023

gopherbot added the FrozenDueToAge label Jun 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

crypto/x509: TestHybridPool failures on Windows with `x509: certificate signed by unknown authority` #51599

crypto/x509: TestHybridPool failures on Windows with `x509: certificate signed by unknown authority` #51599

bcmills commented Mar 10, 2022

bcmills commented Mar 30, 2022

Uh oh!

gopherbot commented Apr 2, 2022

Uh oh!

bcmills commented May 12, 2022

Uh oh!

gopherbot commented May 12, 2022

Uh oh!

bcmills commented May 13, 2022

Uh oh!

bcmills commented May 18, 2022

Uh oh!

bcmills commented May 18, 2022 •

edited

Loading

Uh oh!

dmitshur commented Jun 1, 2022

Uh oh!

rolandshoemaker commented Jun 3, 2022

Uh oh!

heschi commented Jun 8, 2022

Uh oh!

rolandshoemaker commented Jun 9, 2022

Uh oh!

heschi commented Jun 9, 2022

Uh oh!

rolandshoemaker commented Jun 9, 2022

Uh oh!

crypto/x509: TestHybridPool failures on Windows with x509: certificate signed by unknown authority #51599

crypto/x509: TestHybridPool failures on Windows with x509: certificate signed by unknown authority #51599

Comments

bcmills commented Mar 10, 2022

bcmills commented Mar 30, 2022

Uh oh!

gopherbot commented Apr 2, 2022

Uh oh!

bcmills commented May 12, 2022

Uh oh!

gopherbot commented May 12, 2022

Uh oh!

bcmills commented May 13, 2022

Uh oh!

bcmills commented May 18, 2022

Uh oh!

bcmills commented May 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmitshur commented Jun 1, 2022

Uh oh!

rolandshoemaker commented Jun 3, 2022

Uh oh!

heschi commented Jun 8, 2022

Uh oh!

rolandshoemaker commented Jun 9, 2022

Uh oh!

heschi commented Jun 9, 2022

Uh oh!

rolandshoemaker commented Jun 9, 2022

Uh oh!

crypto/x509: TestHybridPool failures on Windows with `x509: certificate signed by unknown authority` #51599

crypto/x509: TestHybridPool failures on Windows with `x509: certificate signed by unknown authority` #51599

bcmills commented May 18, 2022 •

edited

Loading