crypto/x509: darwin only loads system.root keychain should also load system keychain

[jostockley]

Please answer these questions before submitting your issue. Thanks!

1. What version of Go are you using (go version)?
   1.6
2. What operating system and processor architecture are you using (go env)?
   amd64 darwin El Capitan
3. What did you do?
   I am trying to run minikube start on a mac running el capitan. I am inside my corporate network and they have a TLS man-in-the-middle box between the internal network and the internet so that when a TLS connection is made to an internet site, it generates an SSL certificate signed by the corporate root CA. This is installed in my Mac in the system keychain since it is not possible to install trusted CA root certs in the system.root keychain. However, in src/crypto/x509/root_darwin.go I see this:

func execSecurityRoots() (*CertPool, error) {
        cmd := exec.Command("/usr/bin/security", "find-certificate", "-a", "-p", "/System/Library/Keychains/SystemRootCertificates.keychain")
        data, err := cmd.Output()
        if err != nil {
                return nil, err
        }

        roots := NewCertPool()
        roots.AppendCertsFromPEM(data)
        return roots, nil
}

I believe the code should also load certificates from /System/Library/Keychains/SystemCACertificates.keychain

so as to pick up any user installed root certificates.

[bradfitz]

I suspect this is a dup of #14514 which was fixed for Go 1.7. Can you verify?

[jostockley]

I don't see any difference using 1.7rc3, here's my example:

package main

import (
    "log"
    "net/http"
)

func main() {
    _, err := http.Get("https://storage.googleapis.com/minikube/releases.json")
    if err != nil {
        log.Fatal(err)
    }
}

The certificate sent by googleapis is intercepted by our corporate mitm server and a substitute certificate is generated and signed by our internal enterprise intermediate CA SSL which is in turn signed by the internal enterprise CA Root. The program fails with this error:

2016/07/29 11:49:35 Get https://storage.googleapis.com/minikube/releases.json: x509: certificate signed by unknown authority

If I connect my mac to an outside network (such as Starbucks WIFI) the program runs successfully.

[josharian]

The code quoted is for toolchains without cgo. Do you have cgo?

It could well be that the fix you suggest is correct, which would be great. I didn't find that file when I was working on https://codereview.appspot.com/22020045, and I experienced the exact same problem at the time, with a corp cert.

[bradfitz]

@jostockley, could you answer Josh's question? Are you using cgo?

[jostockley]

This was a while ago now, so I tried my test again. I'm using go installed on my Mac using homebrew.
How do I tell if its cgo or regular go??

$ more https.go
package main

import (
    "log"
    "net/http"
)

func main() {
    _, err := http.Get("https://storage.googleapis.com/minikube/releases.json")
    if err != nil {
        log.Fatal(err)
    }
}

$ go version
go version go1.7.4 darwin/amd64
$ go run https.go
2017/01/06 17:10:17 Get https://storage.googleapis.com/minikube/releases.json: x509: certificate signed by unknown authority
exit status 1

[bradfitz]

When you filed a bug, the issue template requested that you paste the output of go env. That would contain the answer.

[jostockley]

So that is what the (go env) meant. It might be better if it said explicity to run the go env command.
Anyhow, here's the output from the go I have from homebrew:

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/stockj3/work"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.7.4_1/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.7.4_1/libexec/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/n2/9tk23c5d1bbb8tkpwz_w9pfmm97_88/T/go-build030846639=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

[bradfitz]

Okay, so you have cgo.

I guess the next question is whether the changes during Go 1.8 helped this or not.

Could you try Go 1.8beta2? https://golang.org/dl/#go1.8beta2

$ go get golang.org/x/build/version/go1.8beta2
$ go1.8beta2 download
$ go1.8beta2 run https.go

[jostockley]

Problem still exists:

$ go1.8beta2 env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/stockj3/work"
GORACE=""
GOROOT="/Users/stockj3/sdk/go1.8beta2"
GOTOOLDIR="/Users/stockj3/sdk/go1.8beta2/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/n2/9tk23c5d1bbb8tkpwz_w9pfmm97_88/T/go-build809517524=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
$ go1.8beta2 run https.go
2017/01/06 17:42:26 Get https://storage.googleapis.com/minikube/releases.json: x509: certificate signed by unknown authority
exit status 1

[josharian]

I don't know much about this stuff, but if you wanted to experiment, the relevant code is in crypto/x509/root_cgo_darwin.go. One place to go sniffing might be the list of security domains we respect.

[mastercactapus]

Correct me if I'm wrong, but if you're not building against C, cgo won't be used (even if CGO_ENABLED=1) -- on darwin at least. A local test with CGO_ENABLED=0 and CGO_ENABLED=1 yields an identical binary.

The non-cgo cert code only looks at SystemRoots on darwin. It's missing the System and login keychains.

I dealt with this issue a lot at a previous employer (they did the same MITM-thing). For awhile we got away with adding the certs to the System Roots keychain, but newer versions of macOS prevent this. After that we just started using our own tls.Config.

Having said that, I honestly think this may be as simple as appending the missing paths to the /usr/bin/security command.
https://github.com/golang/go/blob/master/src/crypto/x509/root_darwin.go#L64

I'd be happy to make a CL if this seems sane to everyone.

[anoopbhat]

I too just ran into this issue in mac os el capitan. The only place I can add a self signed cert is in

~/Library/Keychains/login.keychain

and

/Library/Keychains/System.keychain

Adding a cert using

sudo security add-trusted-cert -d -r trustRoot -k "/System/Library/Keychains/SystemRootCertificates.keychain" ~/cert.cer

results in SecCertificateAddToKeychain: UNIX[Operation not permitted]

So i think the above two keychains also need to be included. Or at the very least, the users login keychain but i don't know how difficult that would be. I'm not a go expert or a c expert so i'm just guessing here.

[mastercactapus]

I did some more testing this morning. The non-cgo solution is pretty simple and I can confirm it works, I can add a cert to the login.keychain on Sierra, trust it, and have it validate on an https request. Un-trust it and it doesn't, etc.. The same for the System keychain.

With cgo enabled, however, I'm getting ...signed by unknown authority (possibly because of "crypto/rsa: verification error" while trying to verify candidate authority certificate... -- so maybe there is a couple issues going on here.

[anoopbhat]

Hmm. I can't seem to recreate this.

I added the cert to my login keychain and System keychain and trusted both. Curl works as expected and so does Safari.

then i built my go binary with

export CGO_ENABLED=0; go build -v -o $GOPATH/bin/session ./cmd/session

This is my go env

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/anoopbhat/GO/gogo-cds-cli/"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/04/3g8k53491bj5qr4f_r_q0x8w0000gp/T/go-build322740831=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="0"

The build is successful but here's the result

./session -a 172.24.50.44 -u username
Password: 
Get https://172.24.50.44/api/...: x509: cannot validate certificate for 172.24.50.44 because it doesn't contain any IP SANs

Maybe one of the packages i'm importing is using cgo and that could cause it?

i'll try just a plain old get of a page on that site.

edit: ok. figured out that certs with IP addresses are only validated using IP SAN certs. In my case, I'm screwed with getting this to work because I can't change the certificate but at least it's a problem on my end :)

[mastercactapus]

Just an update, I'm pretty confident that I have a fix ready for both cgo and nocgo. I'm attempting to validate that it fixes the issue in these environments. I'll make a CL once I can validate it.

@bradfitz in the cgo code path, it looks to be that the Issuer and Subject-match check doesn't account for environments with intermediate CAs that need to be on the chain. That's the general idea anyway; the CL commit will have a more detailed explanation/example once I "mail" it.

As for the nocgo: that code simply doesn't load the System and login keychains currently.

@jostockley would you be able to help validate the fix?

If anyone behind a MITM-type proxy would like to try validating, I made a gist to try to make validation simple as possible:
https://gist.github.com/mastercactapus/14554bc8379016e766bb0229cf386bf8

If you follow the script, it gets go 1.8, clones the fix, builds it, and runs an http.Get in go, printing the results of each.

When it's done, if all goes well, you should see:

CGO   1.8: 0
NOCGO 1.8: 0
CGO   fix: 1
NOCGO fix: 1

Meaning with 1.8 it should get a cert issue with both cgo/nocgo and with the fix, a successful https request for both.

[josharian]

@mastercactapus thanks for working on this! When you mail the CLs, please mail separate CLs for the cgo and non-cgo paths. A good reviewer would be [email protected], and please CC me.

[jostockley]

@mastercactapus I ran your test script and here's the result:

CGO   1.8: 1
NOCGO 1.8: 1
CGO   fix: 1
NOCGO fix: 1

Not what I expected. I also tried my original test case and with your fix my test passes, without you fix my test fails. So that indicates that you fix works at least for my test case.

[gopherbot]

CL https://golang.org/cl/36941 mentions this issue.

[gopherbot]

CL https://golang.org/cl/36942 mentions this issue.

[goraxe]

Hi, any news on this issue?

[josharian]

I think @quentinmit has reviewed some related code in the past. Quentin?

[gopherbot]

Change https://golang.org/cl/57830 mentions this issue: crypto/x509: skip TestSystemRoots

[gopherbot]

Change https://golang.org/cl/64851 mentions this issue: crypto/x509: add cgo for system keychain loading

[gopherbot]

Change https://golang.org/cl/70847 mentions this issue: [release-branch.go1.9] crypto/x509: skip TestSystemRoots

[bradfitz]

I believe this is fixed by https://go-review.googlesource.com/c/go/+/36942 which lacked the magic "Fixes" word.