crypto/x509: TestSystemRoots failing when keychain contains expired or untrusted certificates

[akamensky]

What version of Go are you using (go version)?

go version devel +204a8f5 Tue Jan 1 20:15:48 2019 +0000 darwin/amd64

Does this issue reproduce with the latest release?

reproduced using git tip

What operating system and processor architecture are you using (go env)?

macos 10.14.2
with expired certificate in keychain (certificate comes from corporate wi-fi network that requires their cert deployed to machine

What did you do?

1. Get source code $ git clone https://github.com/golang/go
2. Build and test code $ cd go/src && ./all.bash

What did you expect to see?

All tests passed

What did you see instead?

crypto/x509 test fails, although those certificates should not be affecting tests... At least I would imagine they should not affect that.

--- FAIL: TestSystemRoots (1.31s)
    root_darwin_test.go:34:     cgo sys roots: 680.554535ms
    root_darwin_test.go:35: non-cgo sys roots: 563.203215ms
    root_darwin_test.go:74: certificate only present in non-cgo pool: CN=wifi.vanke.com,OU=IT Center,O=China Vanke,L=Shen Zhen,ST=Guang Dong,C=CN (verify error: x509: certificate has expired or is not yet valid)
    root_darwin_test.go:76: signed certificate only present in non-cgo pool (acceptable): CN=Developer ID Certification Authority,OU=Apple Certification Authority,O=Apple Inc.,C=US
FAIL
FAIL	crypto/x509	3.412s

Deleting failed certificates from keychain passes the test.

[FiloSottile]

Thanks for reporting this.

The problem here is a compound issue: the no-cgo path lets in certificates that are in the root store, not marked as roots themselves, and signed by a root (because the hack we use to validate roots does not allow us to distinguish them from real roots); the cgo path correctly excludes them. The test which compares cgo and no-cgo results tries to ignore them by ignoring certificates which pass validation, but being expired breaks that.

I will try again to think about how to fix the underlying issue, and otherwise add hacks to the test.

[gopherbot]

Change https://golang.org/cl/156330 mentions this issue: crypto/x509: ignore harmless edge case in TestSystemRoots