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: root_darwin.go does not include trusted root certificates from System/Login keychains #14514

Closed
mwielgoszewski opened this issue Feb 25, 2016 · 11 comments
Milestone

Comments

@mwielgoszewski
Copy link

  1. Tested on go version go1.6 darwin/amd64 and go version go1.5.3 darwin/amd64.
  2. OS X Yosemite, OS X El Capitan
  3. I attempted to connect to a TLS service with a certificate signed by an internal, enterprise certificate authority. The CA certificate was added to the System keychain.
  4. I expected to be able to connect to the TLS service without any issue as the certificate is signed by a CA I trust.
  5. The program failed to connect to the TLS service, because the CA certificate that issued the certificate is not installed in the SystemRootCertificates.keychain that Go loads trusted roots from.

On OS X Yosemite, this issue can be mitigated by installing the certificate into SystemRootCertificates.keychain via /usr/bin/security from the terminal:

sudo security add-trusted-cert -k /System/Library/Keychains/SystemRootCertificates.keychain <certificate>

However, on El Capitan this is no longer possible without turning off security protections enabled by Apple.

execSecurityRoots should try to load additional certificates from the System keychain located at /Library/Keychains/System.keychain, and the Login keychain (~/Library/Keychains/login.keychain).

This bug makes distributing Go clients (especially 3rd-party developed) difficult in organizations with an internal PKI.

@ianlancetaylor ianlancetaylor added this to the Go1.7 milestone Feb 26, 2016
@dndx
Copy link
Contributor

dndx commented Feb 26, 2016

Currently Golang uses security find-certificate command to get a dump of everything that is inside /System/Library/Keychains/SystemRootCertificates.keychain which contains all system-wide root certificates.

Obviously we may simply run security find-certificate multiple times and grab certificates from all three (System Roots, System and login) chains but the last chain login does not actually require root privilege to modify and only applies to the user that is currently logged in.

Look at what we are doing with Linux and BSD, in those OSes we have been importing certificates from only trusted locations (means you need root privilege to change them) we might want to include only /Library/Keychains/System.keychain but not ~/Library/Keychains/login.keychain since the previous one needs root privilege to change and is not affected by SIP.

Just my thought.

@mwielgoszewski
Copy link
Author

That sounds pretty reasonable, though other suggestions (i.e., #14022, #3905) indicate the Golang project is considering following OpenSSL's conventions by looking up SSL_CA_CERT_FILE or SSL_CA_CERT_PATH environment variables pointing to to a stack of certificates or directory of certificates respectively. Unless the same considerations are made when loading certificates (e.g., ensure only root can write to that path, and all its parent directories), I don't see a reason to single out OS X.

@dndx
Copy link
Contributor

dndx commented Feb 26, 2016

Yeah I can see your point, it seems unnecessary to limit the current user from using his/her own keychain as a trusted storage.

Also note that if the target machine has cgo available, root_cgo_darwin.go will be used instead which in turn uses the SecTrustCopyAnchorCertificates() function to obtain a list of all trusted roots. On my machine that function returned 192 certs and does not appears to be including those ones from login or System. The returned certs seems to came from /System/Library/Keychains/X509Anchors as documented by Apple and is not necessarily the same as what's inside /System/Library/Keychains/SystemRootCertificates.keychain.

@hinman
Copy link
Contributor

hinman commented Mar 2, 2016

I have a potential fix here:
https://github.com/hinman/go/commit/e3e9ebf21b8f949d9b3eaed95e61640e3430ad27

I'll work on https://golang.org/doc/contribute.html if no one else has a proposed solution.

Basically I'm copying all the trusted certificates from the System, Admin & User domains and then filtering out all the ones where the subject and issuer names don't match (not a root CA or self signed cert).

@gopherbot
Copy link

CL https://golang.org/cl/20351 mentions this issue.

phinze added a commit to hashicorp/atlas-go that referenced this issue May 3, 2016
Allows configuration of the CA certs for the Atlas connection via
environment variables `ATLAS_CAFILE` or `ATLAS_CAPATH`.

Also catches the workaround for
golang/go#14514 in go-rootcerts so that OS X
clients behave properly.
phinze added a commit to hashicorp/atlas-go that referenced this issue May 3, 2016
Allows configuration of the CA certs for the Atlas connection via
environment variables `ATLAS_CAFILE` or `ATLAS_CAPATH`.

Also catches the workaround for
golang/go#14514 in go-rootcerts so that OS X
clients behave properly.
phinze added a commit to hashicorp/atlas-go that referenced this issue May 3, 2016
Allows configuration of the CA certs for the Atlas connection via
environment variables `ATLAS_CAFILE` or `ATLAS_CAPATH`.

Also catches the workaround for
golang/go#14514 in go-rootcerts so that OS X
clients behave properly.
phinze added a commit to hashicorp/terraform that referenced this issue May 3, 2016
Allows CA certs to be configured via `ATLAS_CAFILE` and `ATLAS_CAPATH`
env vars, and works around golang/go#14514 on
OS X.
phinze added a commit to hashicorp/terraform that referenced this issue May 3, 2016
Allows CA certs to be configured via `ATLAS_CAFILE` and `ATLAS_CAPATH`
env vars, and works around golang/go#14514 on
OS X.
@phinze
Copy link

phinze commented May 3, 2016

Hello friends! As you might have guessed from the above linked-issue noise - I made a package today to work around this issue in Packer and Terraform.

I'm hoping to convert the basic strategy into a Go patch soon, but in the meantime you can check out the package here: https://github.com/hashicorp/go-rootcerts

bigkraig pushed a commit to ticketmaster/terraform that referenced this issue May 5, 2016
Allows CA certs to be configured via `ATLAS_CAFILE` and `ATLAS_CAPATH`
env vars, and works around golang/go#14514 on
OS X.
bigkraig pushed a commit to ticketmaster/terraform that referenced this issue May 5, 2016
Allows CA certs to be configured via `ATLAS_CAFILE` and `ATLAS_CAPATH`
env vars, and works around golang/go#14514 on
OS X.
xsellier pushed a commit to xsellier/terraform that referenced this issue May 17, 2016
Allows CA certs to be configured via `ATLAS_CAFILE` and `ATLAS_CAPATH`
env vars, and works around golang/go#14514 on
OS X.
@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016
@phinze
Copy link

phinze commented May 18, 2016

Hi folks - great to see @hinman's patch landing, thank you for that work!

Because it's a cgo-only solution, this does not solve the problem for our use case. I'm not sure what the intent of this particular issue is - should a cgo-free story here be a separate issue or should this one be reopened?

@bradfitz
Copy link
Contributor

New bug.

cristicalin pushed a commit to cristicalin/terraform that referenced this issue May 24, 2016
Allows CA certs to be configured via `ATLAS_CAFILE` and `ATLAS_CAPATH`
env vars, and works around golang/go#14514 on
OS X.
@wmarbut
Copy link

wmarbut commented Jun 16, 2016

@phinze Yeah, I'm in the same boat. The cgo only solution doesn't work for me.

@ianlancetaylor
Copy link
Contributor

@grep-awesome This issue is closed. Please open a new issue or discuss this in a forum (https://golang.org/wiki/Questions).

@golang golang locked and limited conversation to collaborators Jun 23, 2016
@gopherbot
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants