From feb024f4153395e5bbb2a51bb3d1ddc4f5b0d2dc Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 29 Sep 2021 11:31:01 -0700 Subject: [PATCH] crypto/x509: use platform verifier on darwin When VerifyOptions.Roots is nil, default to using the platform X.509 certificate verification APIs on darwin, rather than using the Go verifier. Since our oldest supported version of macOS is 10.12, we are able to use the modern verification APIs, and don't need to resort to the complex chain building trickery employed by chromium et al. Unfortunately there is not a clean way to programmatically add test roots to the system trust store that the builders would tolerate. The most obvious solution, using 'security add-trusted-cert' requires human interaction for authorization. We could also manually add anchors to the constructed SecTrustRef, but that would require adding a whole bunch of plumbing for test functionality, and would mean we weren't really testing the actual non-test path. The path I've chosen here is to just utilize existing valid, and purposefully invalid, trusted chains, from google.com and the badssl.com test suite. This requires external network access, but most accurately reflects real world contexts. This change removes the x509.SystemCertPool() functionality, which will be ammended in a follow-up change which supports the suggested hybrid pool approach described in #46287. Updates #46287 Fixes #42414 Fixes #38888 Fixes #35631 Fixes #19561 Change-Id: I17f0d6c5cb3ef8a1f2731ce3296478b28d30df46 Reviewed-on: https://go-review.googlesource.com/c/go/+/353132 Trust: Roland Shoemaker Run-TryBot: Roland Shoemaker Reviewed-by: Filippo Valsorda TryBot-Result: Go Bot --- src/crypto/x509/cert_pool.go | 2 + .../x509/internal/macos/corefoundation.go | 77 +++++ .../x509/internal/macos/corefoundation.s | 12 + src/crypto/x509/internal/macos/security.go | 118 ++++++++ src/crypto/x509/internal/macos/security.s | 18 ++ src/crypto/x509/root_darwin.go | 268 +++++------------- src/crypto/x509/root_darwin_test.go | 127 +++++++-- src/crypto/x509/verify.go | 4 +- src/crypto/x509/verify_test.go | 4 +- src/crypto/x509/x509_test.go | 4 +- 10 files changed, 409 insertions(+), 225 deletions(-) diff --git a/src/crypto/x509/cert_pool.go b/src/crypto/x509/cert_pool.go index bcc5db3b703ae..1886825b179df 100644 --- a/src/crypto/x509/cert_pool.go +++ b/src/crypto/x509/cert_pool.go @@ -106,6 +106,8 @@ func SystemCertPool() (*CertPool, error) { if runtime.GOOS == "windows" { // Issue 16736, 18609: return nil, errors.New("crypto/x509: system root pool is not available on Windows") + } else if runtime.GOOS == "darwin" { + return nil, errors.New("crypto/x509: system root pool is not available on macOS") } if sysRoots := systemRootsPool(); sysRoots != nil { diff --git a/src/crypto/x509/internal/macos/corefoundation.go b/src/crypto/x509/internal/macos/corefoundation.go index a91131ac984ad..07db5c7527a9e 100644 --- a/src/crypto/x509/internal/macos/corefoundation.go +++ b/src/crypto/x509/internal/macos/corefoundation.go @@ -14,6 +14,7 @@ import ( "internal/abi" "reflect" "runtime" + "time" "unsafe" ) @@ -35,11 +36,37 @@ func CFDataToSlice(data CFRef) []byte { return out } +// CFStringToString returns a Go string representation of the passed +// in CFString. +func CFStringToString(ref CFRef) string { + data := CFStringCreateExternalRepresentation(ref) + b := CFDataToSlice(data) + CFRelease(data) + return string(b) +} + +// TimeToCFDateRef converts a time.Time into an apple CFDateRef +func TimeToCFDateRef(t time.Time) CFRef { + secs := t.Sub(time.Date(2001, 1, 1, 0, 0, 0, 0, time.UTC)).Seconds() + ref := CFDateCreate(int(secs)) + return ref +} + type CFString CFRef const kCFAllocatorDefault = 0 const kCFStringEncodingUTF8 = 0x08000100 +//go:cgo_import_dynamic x509_CFDataCreate CFDataCreate "/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation" + +func BytesToCFData(b []byte) CFRef { + p := unsafe.Pointer((*reflect.SliceHeader)(unsafe.Pointer(&b)).Data) + ret := syscall(abi.FuncPCABI0(x509_CFDataCreate_trampoline), kCFAllocatorDefault, uintptr(p), uintptr(len(b)), 0, 0, 0) + runtime.KeepAlive(p) + return CFRef(ret) +} +func x509_CFDataCreate_trampoline() + //go:cgo_import_dynamic x509_CFStringCreateWithBytes CFStringCreateWithBytes "/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation" // StringToCFString returns a copy of the UTF-8 contents of s as a new CFString. @@ -126,5 +153,55 @@ func CFRelease(ref CFRef) { } func x509_CFRelease_trampoline() +//go:cgo_import_dynamic x509_CFArrayCreateMutable CFArrayCreateMutable "/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation" + +func CFArrayCreateMutable() CFRef { + ret := syscall(abi.FuncPCABI0(x509_CFArrayCreateMutable_trampoline), kCFAllocatorDefault, 0, 0 /* kCFTypeArrayCallBacks */, 0, 0, 0) + return CFRef(ret) +} +func x509_CFArrayCreateMutable_trampoline() + +//go:cgo_import_dynamic x509_CFArrayAppendValue CFArrayAppendValue "/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation" + +func CFArrayAppendValue(array CFRef, val CFRef) { + syscall(abi.FuncPCABI0(x509_CFArrayAppendValue_trampoline), uintptr(array), uintptr(val), 0, 0, 0, 0) +} +func x509_CFArrayAppendValue_trampoline() + +//go:cgo_import_dynamic x509_CFDateCreate CFDateCreate "/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation" + +func CFDateCreate(seconds int) CFRef { + ret := syscall(abi.FuncPCABI0(x509_CFDateCreate_trampoline), kCFAllocatorDefault, uintptr(seconds), 0, 0, 0, 0) + return CFRef(ret) +} +func x509_CFDateCreate_trampoline() + +//go:cgo_import_dynamic x509_CFErrorCopyDescription CFErrorCopyDescription "/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation" + +func CFErrorCopyDescription(errRef CFRef) CFRef { + ret := syscall(abi.FuncPCABI0(x509_CFErrorCopyDescription_trampoline), uintptr(errRef), 0, 0, 0, 0, 0) + return CFRef(ret) +} +func x509_CFErrorCopyDescription_trampoline() + +//go:cgo_import_dynamic x509_CFStringCreateExternalRepresentation CFStringCreateExternalRepresentation "/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation" + +func CFStringCreateExternalRepresentation(strRef CFRef) CFRef { + ret := syscall(abi.FuncPCABI0(x509_CFStringCreateExternalRepresentation_trampoline), kCFAllocatorDefault, uintptr(strRef), kCFStringEncodingUTF8, 0, 0, 0) + return CFRef(ret) +} +func x509_CFStringCreateExternalRepresentation_trampoline() + // syscall is implemented in the runtime package (runtime/sys_darwin.go) func syscall(fn, a1, a2, a3, a4, a5, a6 uintptr) uintptr + +// ReleaseCFArray iterates through an array, releasing its contents, and then +// releases the array itself. This is necessary because we cannot, easily, set the +// CFArrayCallBacks argument when creating CFArrays. +func ReleaseCFArray(array CFRef) { + for i := 0; i < CFArrayGetCount(array); i++ { + ref := CFArrayGetValueAtIndex(array, i) + CFRelease(ref) + } + CFRelease(array) +} diff --git a/src/crypto/x509/internal/macos/corefoundation.s b/src/crypto/x509/internal/macos/corefoundation.s index cda2336c9d1b6..376099caa3438 100644 --- a/src/crypto/x509/internal/macos/corefoundation.s +++ b/src/crypto/x509/internal/macos/corefoundation.s @@ -28,3 +28,15 @@ TEXT ·x509_CFNumberGetValue_trampoline(SB),NOSPLIT,$0-0 JMP x509_CFNumberGetValue(SB) TEXT ·x509_CFEqual_trampoline(SB),NOSPLIT,$0-0 JMP x509_CFEqual(SB) +TEXT ·x509_CFArrayCreateMutable_trampoline(SB),NOSPLIT,$0-0 + JMP x509_CFArrayCreateMutable(SB) +TEXT ·x509_CFArrayAppendValue_trampoline(SB),NOSPLIT,$0-0 + JMP x509_CFArrayAppendValue(SB) +TEXT ·x509_CFDateCreate_trampoline(SB),NOSPLIT,$0-0 + JMP x509_CFDateCreate(SB) +TEXT ·x509_CFDataCreate_trampoline(SB),NOSPLIT,$0-0 + JMP x509_CFDataCreate(SB) +TEXT ·x509_CFErrorCopyDescription_trampoline(SB),NOSPLIT,$0-0 + JMP x509_CFErrorCopyDescription(SB) +TEXT ·x509_CFStringCreateExternalRepresentation_trampoline(SB),NOSPLIT,$0-0 + JMP x509_CFStringCreateExternalRepresentation(SB) diff --git a/src/crypto/x509/internal/macos/security.go b/src/crypto/x509/internal/macos/security.go index a560248e8b760..2805076ccdaea 100644 --- a/src/crypto/x509/internal/macos/security.go +++ b/src/crypto/x509/internal/macos/security.go @@ -8,6 +8,7 @@ package macOS import ( "errors" + "fmt" "internal/abi" "strconv" "unsafe" @@ -29,6 +30,19 @@ const ( SecTrustSettingsResultUnspecified ) +type SecTrustResultType int32 + +const ( + SecTrustResultInvalid SecTrustResultType = iota + SecTrustResultProceed + SecTrustResultConfirm // deprecated + SecTrustResultDeny + SecTrustResultUnspecified + SecTrustResultRecoverableTrustFailure + SecTrustResultFatalTrustFailure + SecTrustResultOtherError +) + type SecTrustSettingsDomain int32 const ( @@ -115,3 +129,107 @@ func SecPolicyCopyProperties(policy CFRef) CFRef { return CFRef(ret) } func x509_SecPolicyCopyProperties_trampoline() + +//go:cgo_import_dynamic x509_SecTrustCreateWithCertificates SecTrustCreateWithCertificates "/System/Library/Frameworks/Security.framework/Versions/A/Security" + +func SecTrustCreateWithCertificates(certs CFRef, policies CFRef) (CFRef, error) { + var trustObj CFRef + ret := syscall(abi.FuncPCABI0(x509_SecTrustCreateWithCertificates_trampoline), uintptr(certs), uintptr(policies), + uintptr(unsafe.Pointer(&trustObj)), 0, 0, 0) + if int32(ret) != 0 { + return 0, OSStatus{"SecTrustCreateWithCertificates", int32(ret)} + } + return trustObj, nil +} +func x509_SecTrustCreateWithCertificates_trampoline() + +//go:cgo_import_dynamic x509_SecCertificateCreateWithData SecCertificateCreateWithData "/System/Library/Frameworks/Security.framework/Versions/A/Security" + +func SecCertificateCreateWithData(b []byte) CFRef { + data := BytesToCFData(b) + ret := syscall(abi.FuncPCABI0(x509_SecCertificateCreateWithData_trampoline), kCFAllocatorDefault, uintptr(data), 0, 0, 0, 0) + CFRelease(data) + return CFRef(ret) +} +func x509_SecCertificateCreateWithData_trampoline() + +//go:cgo_import_dynamic x509_SecPolicyCreateSSL SecPolicyCreateSSL "/System/Library/Frameworks/Security.framework/Versions/A/Security" + +func SecPolicyCreateSSL(name string) CFRef { + var hostname CFString + if name != "" { + hostname = StringToCFString(name) + defer CFRelease(CFRef(hostname)) + } + ret := syscall(abi.FuncPCABI0(x509_SecPolicyCreateSSL_trampoline), 1 /* true */, uintptr(hostname), 0, 0, 0, 0) + return CFRef(ret) +} +func x509_SecPolicyCreateSSL_trampoline() + +//go:cgo_import_dynamic x509_SecTrustSetVerifyDate SecTrustSetVerifyDate "/System/Library/Frameworks/Security.framework/Versions/A/Security" + +func SecTrustSetVerifyDate(trustObj CFRef, dateRef CFRef) error { + ret := syscall(abi.FuncPCABI0(x509_SecTrustSetVerifyDate_trampoline), uintptr(trustObj), uintptr(dateRef), 0, 0, 0, 0) + if int32(ret) != 0 { + return OSStatus{"SecTrustSetVerifyDate", int32(ret)} + } + return nil +} +func x509_SecTrustSetVerifyDate_trampoline() + +//go:cgo_import_dynamic x509_SecTrustEvaluate SecTrustEvaluate "/System/Library/Frameworks/Security.framework/Versions/A/Security" + +func SecTrustEvaluate(trustObj CFRef) (CFRef, error) { + var result CFRef + ret := syscall(abi.FuncPCABI0(x509_SecTrustEvaluate_trampoline), uintptr(trustObj), uintptr(unsafe.Pointer(&result)), 0, 0, 0, 0) + if int32(ret) != 0 { + return 0, OSStatus{"SecTrustEvaluate", int32(ret)} + } + return CFRef(result), nil +} +func x509_SecTrustEvaluate_trampoline() + +//go:cgo_import_dynamic x509_SecTrustGetResult SecTrustGetResult "/System/Library/Frameworks/Security.framework/Versions/A/Security" + +func SecTrustGetResult(trustObj CFRef, result CFRef) (CFRef, CFRef, error) { + var chain, info CFRef + ret := syscall(abi.FuncPCABI0(x509_SecTrustGetResult_trampoline), uintptr(trustObj), uintptr(unsafe.Pointer(&result)), + uintptr(unsafe.Pointer(&chain)), uintptr(unsafe.Pointer(&info)), 0, 0) + if int32(ret) != 0 { + return 0, 0, OSStatus{"SecTrustGetResult", int32(ret)} + } + return chain, info, nil +} +func x509_SecTrustGetResult_trampoline() + +//go:cgo_import_dynamic x509_SecTrustEvaluateWithError SecTrustEvaluateWithError "/System/Library/Frameworks/Security.framework/Versions/A/Security" + +func SecTrustEvaluateWithError(trustObj CFRef) error { + var errRef CFRef + ret := syscall(abi.FuncPCABI0(x509_SecTrustEvaluateWithError_trampoline), uintptr(trustObj), uintptr(unsafe.Pointer(&errRef)), 0, 0, 0, 0) + if int32(ret) != 1 { + errStr := CFErrorCopyDescription(errRef) + err := fmt.Errorf("x509: %s", CFStringToString(errStr)) + CFRelease(errRef) + CFRelease(errStr) + return err + } + return nil +} +func x509_SecTrustEvaluateWithError_trampoline() + +//go:cgo_import_dynamic x509_SecTrustGetCertificateCount SecTrustGetCertificateCount "/System/Library/Frameworks/Security.framework/Versions/A/Security" + +func SecTrustGetCertificateCount(trustObj CFRef) int { + ret := syscall(abi.FuncPCABI0(x509_SecTrustGetCertificateCount_trampoline), uintptr(trustObj), 0, 0, 0, 0, 0) + return int(ret) +} +func x509_SecTrustGetCertificateCount_trampoline() + +//go:cgo_import_dynamic x509_SecTrustGetCertificateAtIndex SecTrustGetCertificateAtIndex "/System/Library/Frameworks/Security.framework/Versions/A/Security" + +func SecTrustGetCertificateAtIndex(trustObj CFRef, i int) CFRef { + ret := syscall(abi.FuncPCABI0(x509_SecTrustGetCertificateAtIndex_trampoline), uintptr(trustObj), uintptr(i), 0, 0, 0, 0) + return CFRef(ret) +} +func x509_SecTrustGetCertificateAtIndex_trampoline() diff --git a/src/crypto/x509/internal/macos/security.s b/src/crypto/x509/internal/macos/security.s index 0038f25b27701..9c1c1334898e0 100644 --- a/src/crypto/x509/internal/macos/security.s +++ b/src/crypto/x509/internal/macos/security.s @@ -18,3 +18,21 @@ TEXT ·x509_SecTrustSettingsCopyTrustSettings_trampoline(SB),NOSPLIT,$0-0 JMP x509_SecTrustSettingsCopyTrustSettings(SB) TEXT ·x509_SecPolicyCopyProperties_trampoline(SB),NOSPLIT,$0-0 JMP x509_SecPolicyCopyProperties(SB) +TEXT ·x509_SecTrustCreateWithCertificates_trampoline(SB),NOSPLIT,$0-0 + JMP x509_SecTrustCreateWithCertificates(SB) +TEXT ·x509_SecCertificateCreateWithData_trampoline(SB),NOSPLIT,$0-0 + JMP x509_SecCertificateCreateWithData(SB) +TEXT ·x509_SecPolicyCreateSSL_trampoline(SB),NOSPLIT,$0-0 + JMP x509_SecPolicyCreateSSL(SB) +TEXT ·x509_SecTrustSetVerifyDate_trampoline(SB),NOSPLIT,$0-0 + JMP x509_SecTrustSetVerifyDate(SB) +TEXT ·x509_SecTrustEvaluate_trampoline(SB),NOSPLIT,$0-0 + JMP x509_SecTrustEvaluate(SB) +TEXT ·x509_SecTrustGetResult_trampoline(SB),NOSPLIT,$0-0 + JMP x509_SecTrustGetResult(SB) +TEXT ·x509_SecTrustEvaluateWithError_trampoline(SB),NOSPLIT,$0-0 + JMP x509_SecTrustEvaluateWithError(SB) +TEXT ·x509_SecTrustGetCertificateCount_trampoline(SB),NOSPLIT,$0-0 + JMP x509_SecTrustGetCertificateCount(SB) +TEXT ·x509_SecTrustGetCertificateAtIndex_trampoline(SB),NOSPLIT,$0-0 + JMP x509_SecTrustGetCertificateAtIndex(SB) diff --git a/src/crypto/x509/root_darwin.go b/src/crypto/x509/root_darwin.go index ef051efd31556..eab046120fe29 100644 --- a/src/crypto/x509/root_darwin.go +++ b/src/crypto/x509/root_darwin.go @@ -7,233 +7,107 @@ package x509 import ( - "bytes" macOS "crypto/x509/internal/macos" - "fmt" - "internal/godebug" - "os" + "errors" ) -var debugDarwinRoots = godebug.Get("x509roots") == "1" - func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) { - return nil, nil -} - -func loadSystemRoots() (*CertPool, error) { - var trustedRoots []*Certificate - untrustedRoots := make(map[string]bool) - - // macOS has three trust domains: one for CAs added by users to their - // "login" keychain, one for CAs added by Admins to the "System" keychain, - // and one for the CAs that ship with the OS. - for _, domain := range []macOS.SecTrustSettingsDomain{ - macOS.SecTrustSettingsDomainUser, - macOS.SecTrustSettingsDomainAdmin, - macOS.SecTrustSettingsDomainSystem, - } { - certs, err := macOS.SecTrustSettingsCopyCertificates(domain) - if err == macOS.ErrNoTrustSettings { - continue - } else if err != nil { - return nil, err - } - defer macOS.CFRelease(certs) - - for i := 0; i < macOS.CFArrayGetCount(certs); i++ { - c := macOS.CFArrayGetValueAtIndex(certs, i) - cert, err := exportCertificate(c) + certs := macOS.CFArrayCreateMutable() + defer macOS.ReleaseCFArray(certs) + leaf := macOS.SecCertificateCreateWithData(c.Raw) + macOS.CFArrayAppendValue(certs, leaf) + if opts.Intermediates != nil { + for _, lc := range opts.Intermediates.lazyCerts { + c, err := lc.getCert() if err != nil { - if debugDarwinRoots { - fmt.Fprintf(os.Stderr, "crypto/x509: domain %d, certificate #%d: %v\n", domain, i, err) - } - continue - } - - var result macOS.SecTrustSettingsResult - if domain == macOS.SecTrustSettingsDomainSystem { - // 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 untrustedRoots. - result = macOS.SecTrustSettingsResultTrustRoot - } else { - result, err = sslTrustSettingsResult(c) - if err != nil { - if debugDarwinRoots { - fmt.Fprintf(os.Stderr, "crypto/x509: trust settings for %v: %v\n", cert.Subject, err) - } - continue - } - if debugDarwinRoots { - fmt.Fprintf(os.Stderr, "crypto/x509: trust settings for %v: %d\n", cert.Subject, result) - } - } - - switch result { - // "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." - case macOS.SecTrustSettingsResultTrustRoot: - if isRootCertificate(cert) { - trustedRoots = append(trustedRoots, cert) - } - case macOS.SecTrustSettingsResultTrustAsRoot: - if !isRootCertificate(cert) { - trustedRoots = append(trustedRoots, cert) - } - - case macOS.SecTrustSettingsResultDeny: - // Add this certificate to untrustedRoots, which are subtracted - // from trustedRoots, so that we don't have to evaluate policies - // for every root in the system domain, but still apply user and - // admin policies that override system roots. - untrustedRoots[string(cert.Raw)] = true - - case macOS.SecTrustSettingsResultUnspecified: - // Certificates with unspecified trust should be added to a pool - // of intermediates for chain building, but we don't support it - // at the moment. This is Issue 35631. - - default: - if debugDarwinRoots { - fmt.Fprintf(os.Stderr, "crypto/x509: unknown trust setting for %v: %d\n", cert.Subject, result) - } + return nil, err } + sc := macOS.SecCertificateCreateWithData(c.Raw) + macOS.CFArrayAppendValue(certs, sc) } } - pool := NewCertPool() - for _, cert := range trustedRoots { - if !untrustedRoots[string(cert.Raw)] { - pool.AddCert(cert) - } - } - return pool, nil -} + policies := macOS.CFArrayCreateMutable() + defer macOS.ReleaseCFArray(policies) + sslPolicy := macOS.SecPolicyCreateSSL(opts.DNSName) + macOS.CFArrayAppendValue(policies, sslPolicy) -// exportCertificate returns a *Certificate for a SecCertificateRef. -func exportCertificate(cert macOS.CFRef) (*Certificate, error) { - data, err := macOS.SecItemExport(cert) + trustObj, err := macOS.SecTrustCreateWithCertificates(certs, policies) if err != nil { return nil, err } - defer macOS.CFRelease(data) - der := macOS.CFDataToSlice(data) + defer macOS.CFRelease(trustObj) - return ParseCertificate(der) -} + if !opts.CurrentTime.IsZero() { + dateRef := macOS.TimeToCFDateRef(opts.CurrentTime) + defer macOS.CFRelease(dateRef) + if err := macOS.SecTrustSetVerifyDate(trustObj, dateRef); err != nil { + return nil, err + } + } -// isRootCertificate reports whether Subject and Issuer match. -func isRootCertificate(cert *Certificate) bool { - return bytes.Equal(cert.RawSubject, cert.RawIssuer) -} + // TODO(roland): we may want to allow passing in SCTs via VerifyOptions and + // set them via SecTrustSetSignedCertificateTimestamps, since Apple will + // always enforce its SCT requirements, and there are still _some_ people + // using TLS or OCSP for that. -// sslTrustSettingsResult obtains the final kSecTrustSettingsResult value for a -// certificate in the user or admin domain, combining usage constraints for the -// SSL SecTrustSettingsPolicy, -// -// It ignores SecTrustSettingsKeyUsage and kSecTrustSettingsAllowedError, and -// doesn't support kSecTrustSettingsDefaultRootCertSetting. -// -// https://developer.apple.com/documentation/security/1400261-sectrustsettingscopytrustsetting -func sslTrustSettingsResult(cert macOS.CFRef) (macOS.SecTrustSettingsResult, error) { - // In Apple's implementation user trust settings override admin trust settings - // (which themselves override system trust settings). If SecTrustSettingsCopyTrustSettings - // fails, or returns a NULL trust settings, when looking for the user trust - // settings then fallback to checking the admin trust settings. - // - // See Security-59306.41.2/trust/headers/SecTrustSettings.h for a description of - // the trust settings overrides, and SecLegacyAnchorSourceCopyUsageConstraints in - // Security-59306.41.2/trust/trustd/SecCertificateSource.c for a concrete example - // of how Apple applies the override in the case of NULL trust settings, or non - // success errors. - trustSettings, err := macOS.SecTrustSettingsCopyTrustSettings(cert, macOS.SecTrustSettingsDomainUser) - if err != nil || trustSettings == 0 { - if debugDarwinRoots && err != macOS.ErrNoTrustSettings { - fmt.Fprintf(os.Stderr, "crypto/x509: SecTrustSettingsCopyTrustSettings for SecTrustSettingsDomainUser failed: %s\n", err) - } - trustSettings, err = macOS.SecTrustSettingsCopyTrustSettings(cert, macOS.SecTrustSettingsDomainAdmin) + if err := macOS.SecTrustEvaluateWithError(trustObj); err != nil { + return nil, err } - if err != nil || trustSettings == 0 { - // If there are neither user nor admin trust settings for a certificate returned - // from SecTrustSettingsCopyCertificates Apple returns kSecTrustSettingsResultInvalid, - // as this method is intended to return certificates _which have trust settings_. - // The most likely case for this being triggered is that the existing trust settings - // are invalid and cannot be properly parsed. In this case SecTrustSettingsCopyTrustSettings - // returns errSecInvalidTrustSettings. The existing cgo implementation returns - // kSecTrustSettingsResultUnspecified in this case, which mostly matches the Apple - // implementation because we don't do anything with certificates marked with this - // result. - // - // See SecPVCGetTrustSettingsResult in Security-59306.41.2/trust/trustd/SecPolicyServer.c - if debugDarwinRoots && err != macOS.ErrNoTrustSettings { - fmt.Fprintf(os.Stderr, "crypto/x509: SecTrustSettingsCopyTrustSettings for SecTrustSettingsDomainAdmin failed: %s\n", err) + + chain := [][]*Certificate{{}} + numCerts := macOS.SecTrustGetCertificateCount(trustObj) + for i := 0; i < numCerts; i++ { + certRef := macOS.SecTrustGetCertificateAtIndex(trustObj, i) + cert, err := exportCertificate(certRef) + if err != nil { + return nil, err } - return macOS.SecTrustSettingsResultUnspecified, nil + chain[0] = append(chain[0], cert) } - defer macOS.CFRelease(trustSettings) - - // "An empty trust settings array means 'always trust this certificate' with an - // overall trust setting for the certificate of kSecTrustSettingsResultTrustRoot." - if macOS.CFArrayGetCount(trustSettings) == 0 { - return macOS.SecTrustSettingsResultTrustRoot, nil + if len(chain[0]) == 0 { + // This should _never_ happen, but to be safe + return nil, errors.New("x509: macOS certificate verification internal error") } - isSSLPolicy := func(policyRef macOS.CFRef) bool { - properties := macOS.SecPolicyCopyProperties(policyRef) - defer macOS.CFRelease(properties) - if v, ok := macOS.CFDictionaryGetValueIfPresent(properties, macOS.SecPolicyOid); ok { - return macOS.CFEqual(v, macOS.CFRef(macOS.SecPolicyAppleSSL)) + if opts.DNSName != "" { + // If we have a DNS name, apply our own name verification + if err := chain[0][0].VerifyHostname(opts.DNSName); err != nil { + return nil, err } - return false } - for i := 0; i < macOS.CFArrayGetCount(trustSettings); i++ { - tSetting := macOS.CFArrayGetValueAtIndex(trustSettings, i) + keyUsages := opts.KeyUsages + if len(keyUsages) == 0 { + keyUsages = []ExtKeyUsage{ExtKeyUsageServerAuth} + } - // First, check if this trust setting is constrained to a non-SSL policy. - if policyRef, ok := macOS.CFDictionaryGetValueIfPresent(tSetting, macOS.SecTrustSettingsPolicy); ok { - if !isSSLPolicy(policyRef) { - continue - } + // If any key usage is acceptable then we're done. + for _, usage := range keyUsages { + if usage == ExtKeyUsageAny { + return chain, nil } + } - // Then check if it is restricted to a hostname, so not a root. - if _, ok := macOS.CFDictionaryGetValueIfPresent(tSetting, macOS.SecTrustSettingsPolicyString); ok { - continue - } + if !checkChainForKeyUsage(chain[0], keyUsages) { + return nil, CertificateInvalidError{c, IncompatibleUsage, ""} + } - cfNum, ok := macOS.CFDictionaryGetValueIfPresent(tSetting, macOS.SecTrustSettingsResultKey) - // "If this key is not present, a default value of kSecTrustSettingsResultTrustRoot is assumed." - if !ok { - return macOS.SecTrustSettingsResultTrustRoot, nil - } - result, err := macOS.CFNumberGetValue(cfNum) - if err != nil { - return 0, err - } + return chain, nil +} - // 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. - switch r := macOS.SecTrustSettingsResult(result); r { - case macOS.SecTrustSettingsResultTrustRoot, - macOS.SecTrustSettingsResultTrustAsRoot, - macOS.SecTrustSettingsResultDeny: - return r, nil - } +// exportCertificate returns a *Certificate for a SecCertificateRef. +func exportCertificate(cert macOS.CFRef) (*Certificate, error) { + data, err := macOS.SecItemExport(cert) + if err != nil { + return nil, err } + defer macOS.CFRelease(data) + der := macOS.CFDataToSlice(data) - // 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 maybe - // fallback to the admin trust settings? TODO(golang.org/issue/38888). + return ParseCertificate(der) +} - return macOS.SecTrustSettingsResultUnspecified, nil +func loadSystemRoots() (*CertPool, error) { + return nil, nil } diff --git a/src/crypto/x509/root_darwin_test.go b/src/crypto/x509/root_darwin_test.go index ae2bd02bf85df..90a464f624960 100644 --- a/src/crypto/x509/root_darwin_test.go +++ b/src/crypto/x509/root_darwin_test.go @@ -2,38 +2,121 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package x509 +package x509_test import ( - "os" - "os/exec" + "crypto/tls" + "crypto/x509" + "internal/testenv" "testing" "time" ) -func TestSystemRoots(t *testing.T) { - t0 := time.Now() - sysRoots, err := loadSystemRoots() // actual system roots - sysRootsDuration := time.Since(t0) - - if err != nil { - t.Fatalf("failed to read system roots: %v", err) +func TestPlatformVerifier(t *testing.T) { + if !testenv.HasExternalNetwork() { + t.Skip() } - t.Logf("loadSystemRoots: %v", sysRootsDuration) + getChain := func(host string) []*x509.Certificate { + t.Helper() + c, err := tls.Dial("tcp", host+":443", &tls.Config{InsecureSkipVerify: true}) + if err != nil { + t.Fatalf("tls connection failed: %s", err) + } + return c.ConnectionState().PeerCertificates + } - // There are 174 system roots on Catalina, and 163 on iOS right now, require - // at least 100 to make sure this is not completely broken. - if want, have := 100, sysRoots.len(); have < want { - t.Errorf("want at least %d system roots, have %d", want, have) + tests := []struct { + name string + host string + verifyName string + verifyTime time.Time + verifyEKU []x509.ExtKeyUsage + expectedErr string + }{ + { + // whatever google.com serves should, hopefully, be trusted + name: "valid chain", + host: "google.com", + }, + { + name: "expired leaf", + host: "expired.badssl.com", + expectedErr: "x509: “*.badssl.com” certificate is expired", + }, + { + name: "wrong host for leaf", + host: "wrong.host.badssl.com", + verifyName: "wrong.host.badssl.com", + expectedErr: "x509: “*.badssl.com” certificate name does not match input", + }, + { + name: "self-signed leaf", + host: "self-signed.badssl.com", + expectedErr: "x509: “*.badssl.com” certificate is not trusted", + }, + { + name: "untrusted root", + host: "untrusted-root.badssl.com", + expectedErr: "x509: “BadSSL Untrusted Root Certificate Authority” certificate is not trusted", + }, + { + name: "revoked leaf", + host: "revoked.badssl.com", + expectedErr: "x509: “revoked.badssl.com” certificate is revoked", + }, + { + name: "leaf missing SCTs", + host: "no-sct.badssl.com", + expectedErr: "x509: “no-sct.badssl.com” certificate is not standards compliant", + }, + { + name: "expired leaf (custom time)", + host: "google.com", + verifyTime: time.Time{}.Add(time.Hour), + expectedErr: "x509: “*.google.com” certificate is expired", + }, + { + name: "valid chain (custom time)", + host: "google.com", + verifyTime: time.Now(), + }, + { + name: "leaf doesn't have acceptable ExtKeyUsage", + host: "google.com", + expectedErr: "x509: certificate specifies an incompatible key usage", + verifyEKU: []x509.ExtKeyUsage{x509.ExtKeyUsageEmailProtection}, + }, } - if t.Failed() { - cmd := exec.Command("security", "dump-trust-settings") - cmd.Stdout, cmd.Stderr = os.Stderr, os.Stderr - cmd.Run() - cmd = exec.Command("security", "dump-trust-settings", "-d") - cmd.Stdout, cmd.Stderr = os.Stderr, os.Stderr - cmd.Run() + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + chain := getChain(tc.host) + var opts x509.VerifyOptions + if len(chain) > 1 { + opts.Intermediates = x509.NewCertPool() + for _, c := range chain[1:] { + opts.Intermediates.AddCert(c) + } + } + if tc.verifyName != "" { + opts.DNSName = tc.verifyName + } + if !tc.verifyTime.IsZero() { + opts.CurrentTime = tc.verifyTime + } + if len(tc.verifyEKU) > 0 { + opts.KeyUsages = tc.verifyEKU + } + + _, err := chain[0].Verify(opts) + if err != nil && tc.expectedErr == "" { + t.Errorf("unexpected verification error: %s", err) + } else if err != nil && err.Error() != tc.expectedErr { + t.Errorf("unexpected verification error: got %q, want %q", err.Error(), tc.expectedErr) + } else if err == nil && tc.expectedErr != "" { + t.Errorf("unexpected verification success: want %q", tc.expectedErr) + } + }) } } diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go index 8aff53afa1ca8..1822a609da4d1 100644 --- a/src/crypto/x509/verify.go +++ b/src/crypto/x509/verify.go @@ -741,8 +741,8 @@ func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err e } } - // Use Windows's own verification and chain building. - if opts.Roots == nil && runtime.GOOS == "windows" { + // Use platform verifiers, where available + if opts.Roots == nil && (runtime.GOOS == "windows" || runtime.GOOS == "darwin") { return c.systemVerify(&opts) } diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go index b9b71f4c1e52d..5b3bf9340a5dc 100644 --- a/src/crypto/x509/verify_test.go +++ b/src/crypto/x509/verify_test.go @@ -1836,8 +1836,8 @@ func TestLongChain(t *testing.T) { } func TestSystemRootsError(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("Windows does not use (or support) systemRoots") + if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { + t.Skip("Windows and darwin do not use (or support) systemRoots") } defer func(oldSystemRoots *CertPool) { systemRoots = oldSystemRoots }(systemRootsPool()) diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go index affab3789d5f9..949bd7f08bfb5 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go @@ -1975,8 +1975,8 @@ func TestMultipleRDN(t *testing.T) { } func TestSystemCertPool(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("not implemented on Windows; Issue 16736, 18609") + if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { + t.Skip("not implemented on Windows (Issue 16736, 18609) or darwin (Issue 46287)") } a, err := SystemCertPool() if err != nil {