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/x509: link Security.framework symbols without cgo #32604

Closed
FiloSottile opened this issue Jun 13, 2019 · 17 comments
Closed

crypto/x509: link Security.framework symbols without cgo #32604

FiloSottile opened this issue Jun 13, 2019 · 17 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin
Milestone

Comments

@FiloSottile
Copy link
Contributor

Just like we link libSystem when CGO_ENABLED=0, we can probably do the same with Security.framework for obtaining the root CAs, and drop the horrible no-cgo fallback path that shells out to security. The latter is slow and makes some dangerous approximations due to not having access to the actual trust policies.

Suggested by @zx2c4.

@FiloSottile FiloSottile added OS-Darwin NeedsFix The path to resolution is known, but the work has not been done. labels Jun 13, 2019
@FiloSottile FiloSottile added this to the Go1.14 milestone Jun 13, 2019
@zx2c4
Copy link
Contributor

zx2c4 commented Jun 13, 2019

and drop the horrible no-cgo fallback path that shells out to security.

And drop the cgo one too. There's not much of a strong reason for keeping cgo around when you can efficiently implement the same exact code in Go.

@gopherbot
Copy link

Change https://golang.org/cl/227037 mentions this issue: crypto/x509: use Security.framework without cgo for roots on macOS

@deitch
Copy link

deitch commented Apr 3, 2020

Wow that was quick @FiloSottile.

Is there an easy way to test it out locally? I’d like to be helpful.

@FiloSottile
Copy link
Contributor Author

Not yet, it's just a stub making the linker parts work, I haven't ported the logic to Go yet. I'll ping this issue when it's ready to test, and I'll definitely appreciate testing in as many settings as possible.

@JohnStarich
Copy link

Hey @FiloSottile, happy to help here too. We're seeing CGO-disabled CLI binaries are failing with cert verify errors, which I think could be fixed by this change.

@FiloSottile
Copy link
Contributor Author

Could you test the CL on those machines?

GO111MODULE=on go get golang.org/dl/gotip@latest 
gotip download 227037    
GODEBUG=x509roots=1 gotip test crypto/x509 -v -run TestSystemRoots

You can also use gotip to build the binaries and see if it fixes it.

@JohnStarich
Copy link

Thank you for the quick reply! I'll need to poke the affected users and have them run it. (My machine doesn't have a problem with CGO/no-CGO.)

@JohnStarich
Copy link

JohnStarich commented Apr 21, 2020

Here's their results:

GODEBUG=x509roots=1 gotip test crypto/x509 -v -run TestSystemRoots
=== RUN   TestSystemRoots
crypto/x509: trust settings for CN=IBM VPN CA,O=International Business Machines Corporation,C=US: 1
crypto/x509: trust settings for CN=GeoTrust SSL CA,O=GeoTrust\, Inc.,C=US: 2
crypto/x509: trust settings for CN=IBM INTERNAL INTERMEDIATE CA,O=International Business Machines Corporation,C=US: 2
crypto/x509: trust settings for CN=DigiCert Global Root CA,OU=www.digicert.com,O=DigiCert Inc,C=US: 1
crypto/x509: trust settings for CN=com.apple.systemdefault,O=System Identity: 1
crypto/x509: trust settings for CN=com.apple.kerberos.kdc,O=System Identity: 1
crypto/x509: trust settings for CN=dlv-cert: 1
crypto/x509: trust settings for CN=IBM JSS Built-in Certificate Authority: 1
crypto/x509: trust settings for CN=IBM Internal Root CA,O=International Business Machines Corporation,C=US: 1
    TestSystemRoots: root_darwin_test.go:23: loadSystemRoots: 180.095636ms
crypto/x509: kSecTrustSettingsResultInvalid = 0
crypto/x509: kSecTrustSettingsResultTrustRoot = 1
crypto/x509: kSecTrustSettingsResultTrustAsRoot = 2
crypto/x509: kSecTrustSettingsResultDeny = 3
crypto/x509: kSecTrustSettingsResultUnspecified = 4
crypto/x509: com.apple.systemdefault returned 1
crypto/x509: com.apple.kerberos.kdc returned 1
crypto/x509: dlv-cert returned 1
crypto/x509: IBM JSS Built-in Certificate Authority returned 1
crypto/x509: IBM Internal Root CA returned 1
crypto/x509: IBM VPN CA returned 1
crypto/x509: GeoTrust SSL CA returned 2
crypto/x509: IBM INTERNAL INTERMEDIATE CA returned 2
crypto/x509: DigiCert Global Root CA returned 1
    TestSystemRoots: root_darwin_test.go:43: loadSystemRootsWithCgo: 165.60705ms
--- PASS: TestSystemRoots (0.35s)
PASS
ok  	crypto/x509	0.547s
And my machine's results, for comparison:
➜ testing GODEBUG=x509roots=1 gotip test crypto/x509 -v -run TestSystemRoots
=== RUN   TestSystemRoots
crypto/x509: trust settings for CN=IBM VPN CA,O=International Business Machines Corporation,C=US: 1
crypto/x509: trust settings for CN=*.ibm.com,O=International Business Machines Corporation,L=Armonk,ST=New York,C=US: 4
crypto/x509: trust settings for CN=*.pandora.com,OU=operations,O=Pandora Media\, Inc.,L=Oakland,ST=California,C=US: 4
crypto/x509: trust settings for CN=<redacted>: 4
crypto/x509: trust settings for CN=k8s-apiserver,L=CA,ST=San Francisco,C=US: 2
crypto/x509: trust settings for CN=k8s-apiserver,L=CA,ST=San Francisco,C=US: 4
crypto/x509: trust settings for CN=k8s-apiserver,L=CA,ST=San Francisco,C=US: 2
crypto/x509: trust settings for CN=IBM Internal Root CA,O=International Business Machines Corporation,C=US: 1
crypto/x509: trust settings for CN=IBM JSS Built-in Certificate Authority: 1
crypto/x509: trust settings for CN=IBM CA,O=International Business Machines Corporation,C=US: 1
crypto/x509: trust settings for CN=International Business Machines Corporation CA,O=International Business Machines Corporation: 1
    TestSystemRoots: root_darwin_test.go:23: loadSystemRoots: 184.048064ms
crypto/x509: kSecTrustSettingsResultInvalid = 0
crypto/x509: kSecTrustSettingsResultTrustRoot = 1
crypto/x509: kSecTrustSettingsResultTrustAsRoot = 2
crypto/x509: kSecTrustSettingsResultDeny = 3
crypto/x509: kSecTrustSettingsResultUnspecified = 4
crypto/x509: IBM Internal Root CA returned 1
crypto/x509: IBM JSS Built-in Certificate Authority returned 1
crypto/x509: IBM CA returned 1
crypto/x509: International Business Machines Corporation CA returned 1
crypto/x509: IBM VPN CA returned 1
crypto/x509: *.ibm.com returned 4
crypto/x509: *.pandora.com returned 4
crypto/x509: <redacted> returned 4
crypto/x509: k8s-apiserver returned 2
crypto/x509: k8s-apiserver returned 4
crypto/x509: k8s-apiserver returned 2
    TestSystemRoots: root_darwin_test.go:43: loadSystemRootsWithCgo: 133.480018ms
--- PASS: TestSystemRoots (0.32s)
PASS
ok      crypto/x509     0.690s

@JohnStarich
Copy link

Follow up: the new binaries I built also work great for them. Seems like this is the fix we need! 🚀

@JohnStarich
Copy link

Hey @FiloSottile, thanks again for your great work here. Is there anything I can do now to help nudge this one along?

@JohnStarich
Copy link

Glad to see it merged! Thanks again, @FiloSottile 🎉 🚀

@deitch
Copy link

deitch commented May 12, 2020

Great @FiloSottile ! Thank you.

@deitch
Copy link

deitch commented May 12, 2020

What go version(s) is this rolling into?

@odeke-em
Copy link
Member

odeke-em commented May 12, 2020 via email

@JohnStarich
Copy link

Oof. That’s a while off. Are there any compatibility concerns with cherry-picking into the next patch of 1.14?

@FiloSottile
Copy link
Contributor Author

We only cherry-pick critical and safe fixes, and this is definitely too large and risky for a minor release, sorry.

@JohnStarich
Copy link

No worries, I understand 👍

Thanks again!

@golang golang locked and limited conversation to collaborators May 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin
Projects
None yet
Development

No branches or pull requests

7 participants