[go] crypto/x509: use platform verifier on darwin

198 views
Skip to first unread message

Roland Shoemaker (Gerrit)

unread,
Nov 5, 2021, 6:28:36 PM11/5/21
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go Bot, Filippo Valsorda, Julie Qiu, Katie Hockman, golang-co...@googlegroups.com

Roland Shoemaker submitted this change.

View Change


Approvals: Filippo Valsorda: Looks good to me, approved Roland Shoemaker: Trusted; Run TryBots Go Bot: TryBots succeeded
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 <rol...@golang.org>
Run-TryBot: Roland Shoemaker <rol...@golang.org>
Reviewed-by: Filippo Valsorda <fil...@golang.org>
TryBot-Result: Go Bot <go...@golang.org>
---
M src/crypto/x509/cert_pool.go
M src/crypto/x509/internal/macos/corefoundation.go
M src/crypto/x509/verify_test.go
M src/crypto/x509/root_darwin_test.go
M src/crypto/x509/internal/macos/security.go
M src/crypto/x509/internal/macos/corefoundation.s
M src/crypto/x509/x509_test.go
M src/crypto/x509/internal/macos/security.s
M src/crypto/x509/root_darwin.go
M src/crypto/x509/verify.go
10 files changed, 453 insertions(+), 227 deletions(-)

diff --git a/src/crypto/x509/cert_pool.go b/src/crypto/x509/cert_pool.go
index bcc5db3..1886825 100644
--- a/src/crypto/x509/cert_pool.go
+++ b/src/crypto/x509/cert_pool.go
@@ -106,6 +106,8 @@
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 a91131a..07db5c7 100644
--- a/src/crypto/x509/internal/macos/corefoundation.go
+++ b/src/crypto/x509/internal/macos/corefoundation.go
@@ -14,6 +14,7 @@
"internal/abi"
"reflect"
"runtime"
+ "time"
"unsafe"
)

@@ -35,11 +36,37 @@
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 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 cda2336..376099c 100644
--- a/src/crypto/x509/internal/macos/corefoundation.s
+++ b/src/crypto/x509/internal/macos/corefoundation.s
@@ -28,3 +28,15 @@
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 a560248..2805076 100644
--- a/src/crypto/x509/internal/macos/security.go
+++ b/src/crypto/x509/internal/macos/security.go
@@ -8,6 +8,7 @@

import (
"errors"
+ "fmt"
"internal/abi"
"strconv"
"unsafe"
@@ -29,6 +30,19 @@
SecTrustSettingsResultUnspecified
)

+type SecTrustResultType int32
+
+const (
+ SecTrustResultInvalid SecTrustResultType = iota
+ SecTrustResultProceed
+ SecTrustResultConfirm // deprecated
+ SecTrustResultDeny
+ SecTrustResultUnspecified
+ SecTrustResultRecoverableTrustFailure
+ SecTrustResultFatalTrustFailure
+ SecTrustResultOtherError
+)
+
type SecTrustSettingsDomain int32

const (
@@ -115,3 +129,107 @@
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 0038f25..9c1c133 100644
--- a/src/crypto/x509/internal/macos/security.s
+++ b/src/crypto/x509/internal/macos/security.s
@@ -18,3 +18,21 @@
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 ef051ef..eab0461 100644
--- a/src/crypto/x509/root_darwin.go
+++ b/src/crypto/x509/root_darwin.go
@@ -7,109 +7,93 @@
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
-}
+ 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 {
+ return nil, err
+ }
+ sc := macOS.SecCertificateCreateWithData(c.Raw)
+ macOS.CFArrayAppendValue(certs, sc)
+ }
+ }

-func loadSystemRoots() (*CertPool, error) {
- var trustedRoots []*Certificate
- untrustedRoots := make(map[string]bool)
+ policies := macOS.CFArrayCreateMutable()
+ defer macOS.ReleaseCFArray(policies)
+ sslPolicy := macOS.SecPolicyCreateSSL(opts.DNSName)
+ macOS.CFArrayAppendValue(policies, sslPolicy)

- // 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 {
+ trustObj, err := macOS.SecTrustCreateWithCertificates(certs, policies)
+ if err != nil {
+ return nil, err
+ }
+ defer macOS.CFRelease(trustObj)
+
+ if !opts.CurrentTime.IsZero() {
+ dateRef := macOS.TimeToCFDateRef(opts.CurrentTime)
+ defer macOS.CFRelease(dateRef)
+ if err := macOS.SecTrustSetVerifyDate(trustObj, dateRef); 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)
- if err != nil {
- if debugDarwinRoots {
- fmt.Fprintf(os.Stderr, "crypto/x509: domain %d, certificate #%d: %v\n", domain, i, err)
- }
- continue
- }
+ // 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.

- 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)
- }
- }
+ if err := macOS.SecTrustEvaluateWithError(trustObj); err != nil {
+ return nil, err
+ }

- 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)
- }
+ 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
+ }
+ chain[0] = append(chain[0], cert)
+ }
+ if len(chain[0]) == 0 {
+ // This should _never_ happen, but to be safe
+ return nil, errors.New("x509: macOS certificate verification internal error")
+ }

- 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)
- }
- }
+ 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
}
}

- pool := NewCertPool()
- for _, cert := range trustedRoots {
- if !untrustedRoots[string(cert.Raw)] {
- pool.AddCert(cert)
+ keyUsages := opts.KeyUsages
+ if len(keyUsages) == 0 {
+ keyUsages = []ExtKeyUsage{ExtKeyUsageServerAuth}
+ }
+
+ // If any key usage is acceptable then we're done.
+ for _, usage := range keyUsages {
+ if usage == ExtKeyUsageAny {
+ return chain, nil
}
}
- return pool, nil
+
+ if !checkChainForKeyUsage(chain[0], keyUsages) {
+ return nil, CertificateInvalidError{c, IncompatibleUsage, ""}
+ }
+
+ return chain, nil
}

// exportCertificate returns a *Certificate for a SecCertificateRef.
@@ -124,116 +108,6 @@
return ParseCertificate(der)
}

-// isRootCertificate reports whether Subject and Issuer match.
-func isRootCertificate(cert *Certificate) bool {
- return bytes.Equal(cert.RawSubject, cert.RawIssuer)
-}
-
-// 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 != 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)
- }
- return macOS.SecTrustSettingsResultUnspecified, nil
- }
- 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
- }
-
- 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))
- }
- return false
- }
-
- for i := 0; i < macOS.CFArrayGetCount(trustSettings); i++ {
- tSetting := macOS.CFArrayGetValueAtIndex(trustSettings, i)
-
- // 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
- }
- }
-
- // Then check if it is restricted to a hostname, so not a root.
- if _, ok := macOS.CFDictionaryGetValueIfPresent(tSetting, macOS.SecTrustSettingsPolicyString); ok {
- continue
- }
-
- 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
- }
-
- // 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
- }
- }
-
- // 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 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 ae2bd02..90a464f 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)
-
- // 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)
+ 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
}

- 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()
+ 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},
+ },
+ }
+
+ 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 8aff53a..1822a60 100644
--- a/src/crypto/x509/verify.go
+++ b/src/crypto/x509/verify.go
@@ -741,8 +741,8 @@
}
}

- // 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 b9b71f4..5b3bf93 100644
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -1836,8 +1836,8 @@
}

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 affab37..949bd7f 100644
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -1975,8 +1975,8 @@
}

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 {

To view, visit change 353132. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I17f0d6c5cb3ef8a1f2731ce3296478b28d30df46
Gerrit-Change-Number: 353132
Gerrit-PatchSet: 8
Gerrit-Owner: Roland Shoemaker <rol...@golang.org>
Gerrit-Reviewer: Filippo Valsorda <fil...@golang.org>
Gerrit-Reviewer: Go Bot <go...@golang.org>
Gerrit-Reviewer: Julie Qiu <ju...@golang.org>
Gerrit-Reviewer: Katie Hockman <ka...@golang.org>
Gerrit-Reviewer: Roland Shoemaker <rol...@golang.org>
Gerrit-MessageType: merged
Reply all
Reply to author
Forward
0 new messages