From eae8c58e36843cb1a6554f8393066507efdd01e3 Mon Sep 17 00:00:00 2001 From: Nathan Caza Date: Sat, 11 Feb 2017 12:37:58 -0600 Subject: [PATCH] crypto/x509: load all trusted certs on darwin The current implementation of loadSystemRoots does not load all required (trusted) certificates in both the cgo and nocgo paths. In the nocgo path, certificates are simply not loaded from the login or System keychains. In the cgo path, certificates whos Subject doesn't match the Issuer, are ignored. This is problematic in the case of a enterprise environment with their own intermediate CAs. In this case: the issuer is a separate root, which may be loaded, but the intermediate is ignored. A TLS handshake may not include the intermediate cert, leading to an error. This change adds the System and login keychain files to the nocgo path, and removes the restriction on Issuer and Subject name matching in the cgo path. Fixes #16532 Change-Id: I4786d6696b338c7e0e0c7806e5d0383f99d2db89 --- src/crypto/x509/root_cgo_darwin.go | 18 ------------------ src/crypto/x509/root_darwin.go | 9 ++++++++- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/crypto/x509/root_cgo_darwin.go b/src/crypto/x509/root_cgo_darwin.go index 8e80533590478c..f43cbc5c704ba1 100644 --- a/src/crypto/x509/root_cgo_darwin.go +++ b/src/crypto/x509/root_cgo_darwin.go @@ -157,24 +157,6 @@ int FetchPEMRoots(CFDataRef *pemRoots, CFDataRef *untrustedPemRoots) { } CFRelease(trustSettings); } - // We only want to add Root CAs, so make sure Subject and Issuer Name match - CFDataRef subjectName = SecCertificateCopyNormalizedSubjectContent(cert, &errRef); - if (errRef != NULL) { - CFRelease(errRef); - continue; - } - CFDataRef issuerName = SecCertificateCopyNormalizedIssuerContent(cert, &errRef); - if (errRef != NULL) { - CFRelease(subjectName); - CFRelease(errRef); - continue; - } - Boolean equal = CFEqual(subjectName, issuerName); - CFRelease(subjectName); - CFRelease(issuerName); - if (!equal) { - continue; - } // Note: SecKeychainItemExport is deprecated as of 10.7 in favor of SecItemExport. // Once we support weak imports via cgo we should prefer that, and fall back to this diff --git a/src/crypto/x509/root_darwin.go b/src/crypto/x509/root_darwin.go index 66cdb5ea261f0d..2f858ae9d43a62 100644 --- a/src/crypto/x509/root_darwin.go +++ b/src/crypto/x509/root_darwin.go @@ -61,7 +61,14 @@ func execSecurityRoots() (*CertPool, error) { println(fmt.Sprintf("crypto/x509: %d certs have a trust policy", len(hasPolicy))) } - cmd := exec.Command("/usr/bin/security", "find-certificate", "-a", "-p", "/System/Library/Keychains/SystemRootCertificates.keychain") + cmd := exec.Command("/usr/bin/security", "find-certificate", "-a", "-p", + "/System/Library/Keychains/SystemRootCertificates.keychain", + "/Library/Keychains/System.keychain", + os.ExpandEnv("$HOME/Library/Keychains/login.keychain"), + + // Fresh installs of Sierra use a slightly different path for the login keychain + os.ExpandEnv("$HOME/Library/Keychains/login.keychain-db"), + ) data, err := cmd.Output() if err != nil { return nil, err