Skip to content

Commit

Permalink
crypto/x509: load all trusted certs on darwin
Browse files Browse the repository at this point in the history
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 golang#16532

Change-Id: I4786d6696b338c7e0e0c7806e5d0383f99d2db89
  • Loading branch information
mastercactapus committed Feb 11, 2017
1 parent 60d7d24 commit eae8c58
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 19 deletions.
18 changes: 0 additions & 18 deletions src/crypto/x509/root_cgo_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion src/crypto/x509/root_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit eae8c58

Please sign in to comment.