Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto/x509: possible crash on macOS 10.13 (in SecTrustEvaluateWithError) #52112

Closed
tmm1 opened this issue Apr 1, 2022 · 5 comments
Closed
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@tmm1
Copy link
Contributor

tmm1 commented Apr 1, 2022

What version of Go are you using (go version)?

$ go version
go version go1.18 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

Tested on macOS 10.11 (which I know is not supported).

But I believe it will also trigger on macOS 10.13, but it would be helpful if someone could confirm one way or the other.

What did you do?

Make a https request

What did you see instead?

SIGTRAP: trace trap
PC=0x7fff6590d075 m=12 sigcode=1

goroutine 0 [idle]:
crypto/x509/internal/macos.syscall(0x0?, 0x0?, 0xc000883360?, 0x4202738?, 0x45f5e40a9d?, 0x67acb40?, 0x0?)
	runtime/sys_darwin.go:99 +0x58 fp=0xc0008832f0 sp=0xc000883290 pc=0x4066038
crypto/x509/internal/macos.SecTrustEvaluateWithError(0xc000942140?)
	crypto/x509/internal/macos/security.go:195 +0x48 fp=0xc000883370 sp=0xc0008832f0 pc=0x4202808
crypto/x509.(*Certificate).systemVerify(0xc0000b0580, 0xc000883718)
	crypto/x509/root_darwin.go:52 +0x2de fp=0xc0008835c8 sp=0xc000883370 pc=0x420c09e
crypto/x509.(*Certificate).Verify(0xc0000b0580, {{0xc000942140, 0x19}, 0xc000abce70, 0x0, {0xc089fbd45c8a4e10, 0x45f5e40a9d, 0x67acb40}, {0x0, 0x0, ...}, ...})
	crypto/x509/verify.go:747 +0x4c7 fp=0xc000883718 sp=0xc0008835c8 pc=0x4210be7
crypto/tls.(*Conn).verifyServerCertificate(0xc000628a80, {0xc000abc8d0, 0x2, 0x2})
	crypto/tls/handshake_client.go:868 +0x658 fp=0xc0008839a0 sp=0xc000883718 pc=0x4234ed8
crypto/tls.(*clientHandshakeStateTLS13).readServerCertificate(0xc000883d98)
	crypto/tls/handshake_client_tls13.go:457 +0x2d1 fp=0xc000883bb0 sp=0xc0008839a0 pc=0x4237751
crypto/tls.(*clientHandshakeStateTLS13).handshake(0xc000883d98)
	crypto/tls/handshake_client_tls13.go:87 +0x1d9 fp=0xc000883be8 sp=0xc000883bb0 pc=0x4235899
crypto/tls.(*Conn).clientHandshake(0xc000628a80, {0x5665c68, 0xc00050cb00})
	crypto/tls/handshake_client.go:219 +0x578 fp=0xc000883e78 sp=0xc000883be8 pc=0x4230878
crypto/tls.(*Conn).clientHandshake-fm({0x5665c68?, 0xc00050cb00?})
	<autogenerated>:1 +0x39 fp=0xc000883ea0 sp=0xc000883e78 pc=0x4261859
crypto/tls.(*Conn).handshakeContext(0xc000628a80, {0x5665ca0, 0xc0001b2010})
	crypto/tls/conn.go:1452 +0x3d1 fp=0xc000883f70 sp=0xc000883ea0 pc=0x422e831
crypto/tls.(*Conn).HandshakeContext(...)
	crypto/tls/conn.go:1402
net/http.(*persistConn).addTLS.func2()
	net/http/transport.go:1537 +0x71 fp=0xc000883fe0 sp=0xc000883f70 pc=0x430e1b1
runtime.goexit()
	runtime/asm_amd64.s:1571 +0x1 fp=0xc000883fe8 sp=0xc000883fe0 pc=0x4069401
created by net/http.(*persistConn).addTLS
	net/http/transport.go:1533 +0x345

goroutine 31 [syscall]:
crypto/x509/internal/macos.syscall(0x0?, 0x0?, 0xc000883360?, 0x4202738?, 0x45f5e40a9d?, 0x67acb40?, 0x0?)
	runtime/sys_darwin.go:99 +0x58 fp=0xc0008832f0 sp=0xc000883290 pc=0x4066038
crypto/x509/internal/macos.SecTrustEvaluateWithError(0xc000942140?)
	crypto/x509/internal/macos/security.go:195 +0x48 fp=0xc000883370 sp=0xc0008832f0 pc=0x4202808
crypto/x509.(*Certificate).systemVerify(0xc0000b0580, 0xc000883718)
	crypto/x509/root_darwin.go:52 +0x2de fp=0xc0008835c8 sp=0xc000883370 pc=0x420c09e
crypto/x509.(*Certificate).Verify(0xc0000b0580, {{0xc000942140, 0x19}, 0xc000abce70, 0x0, {0xc089fbd45c8a4e10, 0x45f5e40a9d, 0x67acb40}, {0x0, 0x0, ...}, ...})
	crypto/x509/verify.go:747 +0x4c7 fp=0xc000883718 sp=0xc0008835c8 pc=0x4210be7
crypto/tls.(*Conn).verifyServerCertificate(0xc000628a80, {0xc000abc8d0, 0x2, 0x2})
	crypto/tls/handshake_client.go:868 +0x658 fp=0xc0008839a0 sp=0xc000883718 pc=0x4234ed8
crypto/tls.(*clientHandshakeStateTLS13).readServerCertificate(0xc000883d98)
	crypto/tls/handshake_client_tls13.go:457 +0x2d1 fp=0xc000883bb0 sp=0xc0008839a0 pc=0x4237751
crypto/tls.(*clientHandshakeStateTLS13).handshake(0xc000883d98)
	crypto/tls/handshake_client_tls13.go:87 +0x1d9 fp=0xc000883be8 sp=0xc000883bb0 pc=0x4235899
crypto/tls.(*Conn).clientHandshake(0xc000628a80, {0x5665c68, 0xc00050cb00})
	crypto/tls/handshake_client.go:219 +0x578 fp=0xc000883e78 sp=0xc000883be8 pc=0x4230878
crypto/tls.(*Conn).clientHandshake-fm({0x5665c68?, 0xc00050cb00?})
	<autogenerated>:1 +0x39 fp=0xc000883ea0 sp=0xc000883e78 pc=0x4261859
crypto/tls.(*Conn).handshakeContext(0xc000628a80, {0x5665ca0, 0xc0001b2010})
	crypto/tls/conn.go:1452 +0x3d1 fp=0xc000883f70 sp=0xc000883ea0 pc=0x422e831
crypto/tls.(*Conn).HandshakeContext(...)
	crypto/tls/conn.go:1402
net/http.(*persistConn).addTLS.func2()
	net/http/transport.go:1537 +0x71 fp=0xc000883fe0 sp=0xc000883f70 pc=0x430e1b1
runtime.goexit()
	runtime/asm_amd64.s:1571 +0x1 fp=0xc000883fe8 sp=0xc000883fe0 pc=0x4069401
created by net/http.(*persistConn).addTLS
	net/http/transport.go:1533 +0x345

This seems to be related to CL353132 (feb024f, #46287) which added calls to SecTrustEvaluateWithError

According to https://developer.apple.com/documentation/security/2980705-sectrustevaluatewitherror, SecTrustEvaluateWithError is only available in macOS 10.14+

cc #23011

@tmm1
Copy link
Contributor Author

tmm1 commented Apr 1, 2022

I started on a patch to fallback to SecTrustEvaluate but it is not clear to me how to finish it.

cc @rolandshoemaker

diff --git a/src/crypto/x509/internal/macos/security.go b/src/crypto/x509/internal/macos/security.go
index ef64bda49f..0523c77ab8 100644
--- a/src/crypto/x509/internal/macos/security.go
+++ b/src/crypto/x509/internal/macos/security.go
@@ -75,6 +75,7 @@ var SecPolicyOid = StringToCFString("SecPolicyOid")
 var SecPolicyAppleSSL = StringToCFString("1.2.840.113635.100.1.3") // defined by POLICYMACRO
 
 var ErrNoTrustSettings = errors.New("no trust settings found")
+var ErrAPIUnavailable = errors.New("api not available")
 
 const errSecNoTrustSettings = -25263
 
@@ -165,13 +166,13 @@ 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
+func SecTrustEvaluate(trustObj CFRef) (SecTrustResultType, error) {
+	var result SecTrustResultType = SecTrustResultInvalid
 	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
+	return result, nil
 }
 func x509_SecTrustEvaluate_trampoline()
 
@@ -191,6 +192,9 @@ func x509_SecTrustGetResult_trampoline()
 //go:cgo_import_dynamic x509_SecTrustEvaluateWithError SecTrustEvaluateWithError "/System/Library/Frameworks/Security.framework/Versions/A/Security"
 
 func SecTrustEvaluateWithError(trustObj CFRef) error {
+	if true /* xxx: detect macOS 10.14+ */ {
+		return ErrAPIUnavailable
+	}
 	var errRef CFRef
 	ret := syscall(abi.FuncPCABI0(x509_SecTrustEvaluateWithError_trampoline), uintptr(trustObj), uintptr(unsafe.Pointer(&errRef)), 0, 0, 0, 0)
 	if int32(ret) != 1 {
diff --git a/src/crypto/x509/root_darwin.go b/src/crypto/x509/root_darwin.go
index 1ef9c0f71e..7168c28cf0 100644
--- a/src/crypto/x509/root_darwin.go
+++ b/src/crypto/x509/root_darwin.go
@@ -50,7 +50,14 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate
 	// using TLS or OCSP for that.
 
 	if err := macOS.SecTrustEvaluateWithError(trustObj); err != nil {
-		return nil, err
+		if err != macOS.ErrAPIUnavailable {
+			return nil, err
+		}
+		if result, err := macOS.SecTrustEvaluate(trustObj); err != nil {
+			return nil, err
+		} else if result != macOS.SecTrustResultProceed && result != macOS.SecTrustResultUnspecified {
+			return nil, errors.New("verification failed")
+		}
 	}
 
 	chain := [][]*Certificate{{}}

@cherrymui cherrymui added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Apr 5, 2022
@cherrymui
Copy link
Member

Could you (or find someone) confirm if this fails on macOS 10.13? Thanks.

@tmm1
Copy link
Contributor Author

tmm1 commented Apr 5, 2022

I guess the docs are wrong.

$ sw_vers
ProductName:	Mac OS X
ProductVersion:	10.11.6
BuildVersion:	15G22010

$ nm /System/Library/Frameworks/Security.framework/Versions/Current/Security | grep EvaluateWithError
$ sw_vers
ProductName:	Mac OS X
ProductVersion:	10.13.6
BuildVersion:	17G14042

$ nm /System/Library/Frameworks/Security.framework/Versions/Current/Security | grep EvaluateWithError
00000000000bf8ae T _SecTrustEvaluateWithError

@cherrymui
Copy link
Member

So it sounds like the program should work without failure? If so, we can close the issue. Thanks.

@tmm1 tmm1 closed this as completed Apr 5, 2022
@tmm1
Copy link
Contributor Author

tmm1 commented Apr 5, 2022

Thanks.

I am maintaining a fork of golang that adds back support for older macOS versions.

If anyone has advice on how best to detect the availability of this function, or simply to detect macOS version inside golang, I would appreciate it.

EDIT: for posterity fancybits/go@6432f14...989d1b1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

3 participants