From e7f95b3ccabef26cc662d9dbfaf5e38a866a86f9 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Mon, 13 Feb 2017 21:15:40 -0600 Subject: [PATCH] crypto/x509: load all trusted certs on darwin (cgo) The current implementation ignores certs wherein the Subject does not match the Issuer. An example of where this causes issue is an enterprise environment with intermediate CAs. In this case, the issuer is separate (and may be loaded) but the intermediate is ignored. A TLS handshake that does not include the intermediate cert would then fail with an untrusted error in Go. On other platforms (darwin-nocgo included), all trusted certs are loaded and accepted reguardless of Subject/Issuer names. This change removes the Subject/Issuer name-matching restriction of certificates when trustAsRoot is set, allowing all trusted certs to be loaded on darwin (cgo). Refs #16532 Change-Id: I451e929588f8911892be6bdc2143d0799363c5f8 Reviewed-on: https://go-review.googlesource.com/36942 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/crypto/x509/root_cgo_darwin.go | 50 +++++++++++++++++++----------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/src/crypto/x509/root_cgo_darwin.go b/src/crypto/x509/root_cgo_darwin.go index 5f0e5e1888a..80cd2501c43 100644 --- a/src/crypto/x509/root_cgo_darwin.go +++ b/src/crypto/x509/root_cgo_darwin.go @@ -119,7 +119,11 @@ int FetchPEMRoots(CFDataRef *pemRoots, CFDataRef *untrustedPemRoots) { } // We only want trusted certs. int untrusted = 0; - if (i != 0) { + int trustAsRoot = 0; + int trustRoot = 0; + if (i == 0) { + trustAsRoot = 1; + } else { // Certs found in the system domain are always trusted. If the user // configures "Never Trust" on such a cert, it will also be found in the // admin or user domain, causing it to be added to untrustedPemRoots. The @@ -129,7 +133,7 @@ int FetchPEMRoots(CFDataRef *pemRoots, CFDataRef *untrustedPemRoots) { // SecTrustServer.c, "user trust settings overrule admin trust settings", // so take the last trust settings array we find. // Skip the system domain since it is always trusted. - for (int k = 1; k < numDomains; k++) { + for (int k = i; k < numDomains; k++) { CFArrayRef domainTrustSettings = NULL; err = SecTrustSettingsCopyTrustSettings(cert, domains[k], &domainTrustSettings); if (err == errSecSuccess && domainTrustSettings != NULL) { @@ -152,28 +156,35 @@ int FetchPEMRoots(CFDataRef *pemRoots, CFDataRef *untrustedPemRoots) { // TODO: The rest of the dictionary specifies conditions for evaluation. if (result == kSecTrustSettingsResultDeny) { untrusted = 1; + } else if (result == kSecTrustSettingsResultTrustAsRoot) { + trustAsRoot = 1; + } else if (result == kSecTrustSettingsResultTrustRoot) { + trustRoot = 1; } } } 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) { + + if (trustRoot) { + // 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(errRef); - continue; - } - Boolean equal = CFEqual(subjectName, issuerName); - CFRelease(subjectName); - CFRelease(issuerName); - if (!equal) { - continue; + CFRelease(issuerName); + if (!equal) { + continue; + } } // Note: SecKeychainItemExport is deprecated as of 10.7 in favor of SecItemExport. @@ -185,6 +196,9 @@ int FetchPEMRoots(CFDataRef *pemRoots, CFDataRef *untrustedPemRoots) { } if (data != NULL) { + if (!trustRoot && !trustAsRoot) { + untrusted = 1; + } CFMutableDataRef appendTo = untrusted ? combinedUntrustedData : combinedData; CFDataAppendBytes(appendTo, CFDataGetBytePtr(data), CFDataGetLength(data)); CFRelease(data);