From: Filippo Valsorda Date: Mon, 9 Nov 2020 13:54:55 +0000 (+0100) Subject: crypto/x509: drop the cgo implementation of root_darwin_amd64.go X-Git-Tag: go1.16beta1~272 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=5e181357c7dd5cde11b28b2db0b4fb02159ddacf;p=gostls13.git crypto/x509: drop the cgo implementation of root_darwin_amd64.go This code was preserved just to do side-by-side testing while transitioning to the Go implementation. There haven't been mismatch issues, so drop the cgo code, which was making it hard to improve the Go code without diverging. Change-Id: I2a23039c31a46e88b94250aafbc98d4ea8daf22f Reviewed-on: https://go-review.googlesource.com/c/go/+/232397 Run-TryBot: Filippo Valsorda TryBot-Result: Go Bot Trust: Filippo Valsorda Reviewed-by: Brad Fitzpatrick --- diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index 5e3711b0c8..2a17ab9cae 100644 --- a/src/cmd/dist/test.go +++ b/src/cmd/dist/test.go @@ -1444,7 +1444,6 @@ func (t *tester) testDirTest(dt *distTest, shard, shards int) error { // cgoPackages is the standard packages that use cgo. var cgoPackages = []string{ - "crypto/x509", "net", "os/user", } diff --git a/src/crypto/x509/internal/macos/corefoundation.go b/src/crypto/x509/internal/macos/corefoundation.go index a248ee3292..9b776d4b85 100644 --- a/src/crypto/x509/internal/macos/corefoundation.go +++ b/src/crypto/x509/internal/macos/corefoundation.go @@ -16,6 +16,10 @@ import ( "unsafe" ) +// Core Foundation linker flags for the external linker. See Issue 42459. +//go:cgo_ldflag "-framework" +//go:cgo_ldflag "CoreFoundation" + // CFRef is an opaque reference to a Core Foundation object. It is a pointer, // but to memory not owned by Go, so not an unsafe.Pointer. type CFRef uintptr diff --git a/src/crypto/x509/internal/macos/security.go b/src/crypto/x509/internal/macos/security.go index 59cc19c587..5e39e93666 100644 --- a/src/crypto/x509/internal/macos/security.go +++ b/src/crypto/x509/internal/macos/security.go @@ -12,6 +12,10 @@ import ( "unsafe" ) +// Security.framework linker flags for the external linker. See Issue 42459. +//go:cgo_ldflag "-framework" +//go:cgo_ldflag "Security" + // Based on https://opensource.apple.com/source/Security/Security-59306.41.2/base/Security.h type SecTrustSettingsResult int32 diff --git a/src/crypto/x509/root_cgo_darwin.go b/src/crypto/x509/root_cgo_darwin.go deleted file mode 100644 index 825e8d4812..0000000000 --- a/src/crypto/x509/root_cgo_darwin.go +++ /dev/null @@ -1,326 +0,0 @@ -// Copyright 2011 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// +build !ios - -package x509 - -// This cgo implementation exists only to support side-by-side testing by -// TestSystemRoots. It can be removed once we are confident in the no-cgo -// implementation. - -/* -#cgo CFLAGS: -mmacosx-version-min=10.11 -#cgo LDFLAGS: -framework CoreFoundation -framework Security - -#include -#include - -#include -#include - -static Boolean isSSLPolicy(SecPolicyRef policyRef) { - if (!policyRef) { - return false; - } - CFDictionaryRef properties = SecPolicyCopyProperties(policyRef); - if (properties == NULL) { - return false; - } - Boolean isSSL = false; - CFTypeRef value = NULL; - if (CFDictionaryGetValueIfPresent(properties, kSecPolicyOid, (const void **)&value)) { - isSSL = CFEqual(value, kSecPolicyAppleSSL); - } - CFRelease(properties); - return isSSL; -} - -// sslTrustSettingsResult obtains the final kSecTrustSettingsResult value -// for a certificate in the user or admin domain, combining usage constraints -// for the SSL SecTrustSettingsPolicy, ignoring SecTrustSettingsKeyUsage and -// kSecTrustSettingsAllowedError. -// https://developer.apple.com/documentation/security/1400261-sectrustsettingscopytrustsetting -static SInt32 sslTrustSettingsResult(SecCertificateRef cert) { - CFArrayRef trustSettings = NULL; - OSStatus err = SecTrustSettingsCopyTrustSettings(cert, kSecTrustSettingsDomainUser, &trustSettings); - - // According to Apple's SecTrustServer.c, "user trust settings overrule admin trust settings", - // but the rules of the override are unclear. Let's assume admin trust settings are applicable - // if and only if user trust settings fail to load or are NULL. - if (err != errSecSuccess || trustSettings == NULL) { - if (trustSettings != NULL) CFRelease(trustSettings); - err = SecTrustSettingsCopyTrustSettings(cert, kSecTrustSettingsDomainAdmin, &trustSettings); - } - - // > no trust settings [...] means "this certificate must be verified to a known trusted certificate” - // (Should this cause a fallback from user to admin domain? It's unclear.) - if (err != errSecSuccess || trustSettings == NULL) { - if (trustSettings != NULL) CFRelease(trustSettings); - return kSecTrustSettingsResultUnspecified; - } - - // > An empty trust settings array means "always trust this certificate” with an - // > overall trust setting for the certificate of kSecTrustSettingsResultTrustRoot. - if (CFArrayGetCount(trustSettings) == 0) { - CFRelease(trustSettings); - return kSecTrustSettingsResultTrustRoot; - } - - // kSecTrustSettingsResult is defined as CFSTR("kSecTrustSettingsResult"), - // but the Go linker's internal linking mode can't handle CFSTR relocations. - // Create our own dynamic string instead and release it below. - CFStringRef _kSecTrustSettingsResult = CFStringCreateWithCString( - NULL, "kSecTrustSettingsResult", kCFStringEncodingUTF8); - CFStringRef _kSecTrustSettingsPolicy = CFStringCreateWithCString( - NULL, "kSecTrustSettingsPolicy", kCFStringEncodingUTF8); - CFStringRef _kSecTrustSettingsPolicyString = CFStringCreateWithCString( - NULL, "kSecTrustSettingsPolicyString", kCFStringEncodingUTF8); - - CFIndex m; SInt32 result = 0; - for (m = 0; m < CFArrayGetCount(trustSettings); m++) { - CFDictionaryRef tSetting = (CFDictionaryRef)CFArrayGetValueAtIndex(trustSettings, m); - - // First, check if this trust setting is constrained to a non-SSL policy. - SecPolicyRef policyRef; - if (CFDictionaryGetValueIfPresent(tSetting, _kSecTrustSettingsPolicy, (const void**)&policyRef)) { - if (!isSSLPolicy(policyRef)) { - continue; - } - } - - if (CFDictionaryContainsKey(tSetting, _kSecTrustSettingsPolicyString)) { - // Restricted to a hostname, not a root. - continue; - } - - CFNumberRef cfNum; - if (CFDictionaryGetValueIfPresent(tSetting, _kSecTrustSettingsResult, (const void**)&cfNum)) { - CFNumberGetValue(cfNum, kCFNumberSInt32Type, &result); - } else { - // > If this key is not present, a default value of - // > kSecTrustSettingsResultTrustRoot is assumed. - result = kSecTrustSettingsResultTrustRoot; - } - - // If multiple dictionaries match, we are supposed to "OR" them, - // the semantics of which are not clear. Since TrustRoot and TrustAsRoot - // are mutually exclusive, Deny should probably override, and Invalid and - // Unspecified be overridden, approximate this by stopping at the first - // TrustRoot, TrustAsRoot or Deny. - if (result == kSecTrustSettingsResultTrustRoot) { - break; - } else if (result == kSecTrustSettingsResultTrustAsRoot) { - break; - } else if (result == kSecTrustSettingsResultDeny) { - break; - } - } - - // If trust settings are present, but none of them match the policy... - // the docs don't tell us what to do. - // - // "Trust settings for a given use apply if any of the dictionaries in the - // certificate’s trust settings array satisfies the specified use." suggests - // that it's as if there were no trust settings at all, so we should probably - // fallback to the admin trust settings. TODO. - if (result == 0) { - result = kSecTrustSettingsResultUnspecified; - } - - CFRelease(_kSecTrustSettingsPolicy); - CFRelease(_kSecTrustSettingsPolicyString); - CFRelease(_kSecTrustSettingsResult); - CFRelease(trustSettings); - - return result; -} - -// isRootCertificate reports whether Subject and Issuer match. -static Boolean isRootCertificate(SecCertificateRef cert, CFErrorRef *errRef) { - CFDataRef subjectName = SecCertificateCopyNormalizedSubjectContent(cert, errRef); - if (*errRef != NULL) { - return false; - } - CFDataRef issuerName = SecCertificateCopyNormalizedIssuerContent(cert, errRef); - if (*errRef != NULL) { - CFRelease(subjectName); - return false; - } - Boolean equal = CFEqual(subjectName, issuerName); - CFRelease(subjectName); - CFRelease(issuerName); - return equal; -} - -// CopyPEMRoots fetches the system's list of trusted X.509 root certificates -// for the kSecTrustSettingsPolicy SSL. -// -// On success it returns 0 and fills pemRoots with a CFDataRef that contains the extracted root -// certificates of the system. On failure, the function returns -1. -// Additionally, it fills untrustedPemRoots with certs that must be removed from pemRoots. -// -// Note: The CFDataRef returned in pemRoots and untrustedPemRoots must -// be released (using CFRelease) after we've consumed its content. -static int CopyPEMRoots(CFDataRef *pemRoots, CFDataRef *untrustedPemRoots, bool debugDarwinRoots) { - int i; - - if (debugDarwinRoots) { - fprintf(stderr, "crypto/x509: kSecTrustSettingsResultInvalid = %d\n", kSecTrustSettingsResultInvalid); - fprintf(stderr, "crypto/x509: kSecTrustSettingsResultTrustRoot = %d\n", kSecTrustSettingsResultTrustRoot); - fprintf(stderr, "crypto/x509: kSecTrustSettingsResultTrustAsRoot = %d\n", kSecTrustSettingsResultTrustAsRoot); - fprintf(stderr, "crypto/x509: kSecTrustSettingsResultDeny = %d\n", kSecTrustSettingsResultDeny); - fprintf(stderr, "crypto/x509: kSecTrustSettingsResultUnspecified = %d\n", kSecTrustSettingsResultUnspecified); - } - - // Get certificates from all domains, not just System, this lets - // the user add CAs to their "login" keychain, and Admins to add - // to the "System" keychain - SecTrustSettingsDomain domains[] = { kSecTrustSettingsDomainSystem, - kSecTrustSettingsDomainAdmin, kSecTrustSettingsDomainUser }; - - int numDomains = sizeof(domains)/sizeof(SecTrustSettingsDomain); - if (pemRoots == NULL || untrustedPemRoots == NULL) { - return -1; - } - - CFMutableDataRef combinedData = CFDataCreateMutable(kCFAllocatorDefault, 0); - CFMutableDataRef combinedUntrustedData = CFDataCreateMutable(kCFAllocatorDefault, 0); - for (i = 0; i < numDomains; i++) { - int j; - CFArrayRef certs = NULL; - OSStatus err = SecTrustSettingsCopyCertificates(domains[i], &certs); - if (err != noErr) { - continue; - } - - CFIndex numCerts = CFArrayGetCount(certs); - for (j = 0; j < numCerts; j++) { - SecCertificateRef cert = (SecCertificateRef)CFArrayGetValueAtIndex(certs, j); - if (cert == NULL) { - continue; - } - - SInt32 result; - if (domains[i] == kSecTrustSettingsDomainSystem) { - // 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 - // Go code will then clean this up. - result = kSecTrustSettingsResultTrustRoot; - } else { - result = sslTrustSettingsResult(cert); - if (debugDarwinRoots) { - CFErrorRef errRef = NULL; - CFStringRef summary = SecCertificateCopyShortDescription(NULL, cert, &errRef); - if (errRef != NULL) { - fprintf(stderr, "crypto/x509: SecCertificateCopyShortDescription failed\n"); - CFRelease(errRef); - continue; - } - - CFIndex length = CFStringGetLength(summary); - CFIndex maxSize = CFStringGetMaximumSizeForEncoding(length, kCFStringEncodingUTF8) + 1; - char *buffer = malloc(maxSize); - if (CFStringGetCString(summary, buffer, maxSize, kCFStringEncodingUTF8)) { - fprintf(stderr, "crypto/x509: %s returned %d\n", buffer, (int)result); - } - free(buffer); - CFRelease(summary); - } - } - - CFMutableDataRef appendTo; - // > Note the distinction between the results kSecTrustSettingsResultTrustRoot - // > and kSecTrustSettingsResultTrustAsRoot: The former can only be applied to - // > root (self-signed) certificates; the latter can only be applied to - // > non-root certificates. - if (result == kSecTrustSettingsResultTrustRoot) { - CFErrorRef errRef = NULL; - if (!isRootCertificate(cert, &errRef) || errRef != NULL) { - if (errRef != NULL) CFRelease(errRef); - continue; - } - - appendTo = combinedData; - } else if (result == kSecTrustSettingsResultTrustAsRoot) { - CFErrorRef errRef = NULL; - if (isRootCertificate(cert, &errRef) || errRef != NULL) { - if (errRef != NULL) CFRelease(errRef); - continue; - } - - appendTo = combinedData; - } else if (result == kSecTrustSettingsResultDeny) { - appendTo = combinedUntrustedData; - } else if (result == kSecTrustSettingsResultUnspecified) { - // Certificates with unspecified trust should probably be added to a pool of - // intermediates for chain building, or checked for transitive trust and - // added to the root pool (which is an imprecise approximation because it - // cuts chains short) but we don't support either at the moment. TODO. - continue; - } else { - continue; - } - - CFDataRef data = NULL; - err = SecItemExport(cert, kSecFormatX509Cert, kSecItemPemArmour, NULL, &data); - if (err != noErr) { - continue; - } - if (data != NULL) { - CFDataAppendBytes(appendTo, CFDataGetBytePtr(data), CFDataGetLength(data)); - CFRelease(data); - } - } - CFRelease(certs); - } - *pemRoots = combinedData; - *untrustedPemRoots = combinedUntrustedData; - return 0; -} -*/ -import "C" -import ( - "errors" - "unsafe" -) - -func init() { - loadSystemRootsWithCgo = _loadSystemRootsWithCgo -} - -func _loadSystemRootsWithCgo() (*CertPool, error) { - var data, untrustedData C.CFDataRef - err := C.CopyPEMRoots(&data, &untrustedData, C.bool(debugDarwinRoots)) - if err == -1 { - return nil, errors.New("crypto/x509: failed to load darwin system roots with cgo") - } - defer C.CFRelease(C.CFTypeRef(data)) - defer C.CFRelease(C.CFTypeRef(untrustedData)) - - buf := C.GoBytes(unsafe.Pointer(C.CFDataGetBytePtr(data)), C.int(C.CFDataGetLength(data))) - roots := NewCertPool() - roots.AppendCertsFromPEM(buf) - - if C.CFDataGetLength(untrustedData) == 0 { - return roots, nil - } - - buf = C.GoBytes(unsafe.Pointer(C.CFDataGetBytePtr(untrustedData)), C.int(C.CFDataGetLength(untrustedData))) - untrustedRoots := NewCertPool() - untrustedRoots.AppendCertsFromPEM(buf) - - trustedRoots := NewCertPool() - for _, lc := range roots.lazyCerts { - c, err := lc.getCert() - if err != nil { - return nil, err - } - if !untrustedRoots.contains(c) { - trustedRoots.AddCert(c) - } - } - return trustedRoots, nil -} diff --git a/src/crypto/x509/root_darwin.go b/src/crypto/x509/root_darwin.go index ce88de025e..c9ea7e80f3 100644 --- a/src/crypto/x509/root_darwin.go +++ b/src/crypto/x509/root_darwin.go @@ -20,10 +20,6 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate return nil, nil } -// loadSystemRootsWithCgo is set in root_cgo_darwin_amd64.go when cgo is -// available, and is only used for testing. -var loadSystemRootsWithCgo func() (*CertPool, error) - func loadSystemRoots() (*CertPool, error) { var trustedRoots []*Certificate untrustedRoots := make(map[string]bool) diff --git a/src/crypto/x509/root_darwin_test.go b/src/crypto/x509/root_darwin_test.go index 69f181c2d4..ae2bd02bf8 100644 --- a/src/crypto/x509/root_darwin_test.go +++ b/src/crypto/x509/root_darwin_test.go @@ -28,39 +28,6 @@ func TestSystemRoots(t *testing.T) { t.Errorf("want at least %d system roots, have %d", want, have) } - if loadSystemRootsWithCgo == nil { - t.Skip("cgo not available, can't compare pool") - } - - t1 := time.Now() - cgoRoots, err := loadSystemRootsWithCgo() // cgo roots - cgoSysRootsDuration := time.Since(t1) - - if err != nil { - t.Fatalf("failed to read cgo roots: %v", err) - } - - t.Logf("loadSystemRootsWithCgo: %v", cgoSysRootsDuration) - - // Check that the two cert pools are the same. - sysPool := make(map[string]*Certificate, sysRoots.len()) - for i := 0; i < sysRoots.len(); i++ { - c := sysRoots.mustCert(t, i) - sysPool[string(c.Raw)] = c - } - for i := 0; i < cgoRoots.len(); i++ { - c := cgoRoots.mustCert(t, i) - - if _, ok := sysPool[string(c.Raw)]; ok { - delete(sysPool, string(c.Raw)) - } else { - t.Errorf("certificate only present in cgo pool: %v", c.Subject) - } - } - for _, c := range sysPool { - t.Errorf("certificate only present in real pool: %v", c.Subject) - } - if t.Failed() { cmd := exec.Command("security", "dump-trust-settings") cmd.Stdout, cmd.Stderr = os.Stderr, os.Stderr diff --git a/src/crypto/x509/root_ios.go b/src/crypto/x509/root_ios.go index bb4a5f75ba..cb3529d6d5 100644 --- a/src/crypto/x509/root_ios.go +++ b/src/crypto/x509/root_ios.go @@ -10,9 +10,6 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate return nil, nil } -// loadSystemRootsWithCgo is not available on iOS. -var loadSystemRootsWithCgo func() (*CertPool, error) - func loadSystemRoots() (*CertPool, error) { p := NewCertPool() p.AppendCertsFromPEM([]byte(systemRootsPEM)) diff --git a/src/crypto/x509/root_ios_gen.go b/src/crypto/x509/root_ios_gen.go index f7eecb576d..2bcdab1a77 100644 --- a/src/crypto/x509/root_ios_gen.go +++ b/src/crypto/x509/root_ios_gen.go @@ -172,9 +172,6 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate return nil, nil } -// loadSystemRootsWithCgo is not available on iOS. -var loadSystemRootsWithCgo func() (*CertPool, error) - func loadSystemRoots() (*CertPool, error) { p := NewCertPool() p.AppendCertsFromPEM([]byte(systemRootsPEM)) diff --git a/src/crypto/x509/root_omit.go b/src/crypto/x509/root_omit.go index 175d71643b..0055b3b862 100644 --- a/src/crypto/x509/root_omit.go +++ b/src/crypto/x509/root_omit.go @@ -24,6 +24,3 @@ func loadSystemRoots() (*CertPool, error) { func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) { return nil, nil } - -// loadSystemRootsWithCgo is not available on iOS. -var loadSystemRootsWithCgo func() (*CertPool, error) diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index b26b2bd199..bf1367355d 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -395,7 +395,7 @@ var depsRules = ` CGO, net !< CRYPTO-MATH; # TLS, Prince of Dependencies. - CGO, CRYPTO-MATH, NET, container/list, encoding/hex, encoding/pem + CRYPTO-MATH, NET, container/list, encoding/hex, encoding/pem < golang.org/x/crypto/internal/subtle < golang.org/x/crypto/chacha20 < golang.org/x/crypto/poly1305