crypto/x509: SSL_CERT_DIR should support multiple directories separated by a colon like OpenSSL and BoringSSL do

[freedge]

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

$ go version
go version go1.13 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output

$ go env
GO111MODULE="auto"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/travis/.cache/go-build"
GOENV="/home/travis/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/travis/gopath"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/travis/.gimme/versions/go1.13.linux.amd64"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/travis/.gimme/versions/go1.13.linux.amd64/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build174585789=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Set:

export SSL_CERT_DIR=~/certs:/etc/ssl/certs

Then perform a call to a https service using http.Client.

This piece of code https://golang.org/src/crypto/x509/root_unix.go#L60

reads the SSL_CERT_DIR as a single path.

What did you expect to see?

as per doc from https://www.openssl.org/docs/man1.0.2/man1/c_rehash.html

the SSL_CERT_DIR environment variable is consulted; this shold be a colon-separated list of directories, like the Unix PATH variable.

This works for curl as well, and is convenient to provide some extract certificates on top of the default ones.

What did you see instead?

x509: certificate is not authorized to sign other certificates

[odeke-em]

Thank you for filing this issue @freedge and welcome to the Go project!

Great catch! You are right, OpenSSL and BoringSSL both recognize the colon separator as per

OpenSSL

https://github.com/openssl/openssl/blob/12a765a5235f181c2f4992b615eb5f892c368e88/crypto/x509/by_dir.c#L153-L209

BoringSSL

https://github.com/google/boringssl/blob/3ba9586bc081f67903c89917f23e74a0662ba953/crypto/x509/by_dir.c#L194-L247

Proposed fix

The fix would seem pretty simple IMHO, we can change

go/src/crypto/x509/root_unix.go

Lines 60 to 62 in a8fc82f

	if d := os.Getenv(certDirEnv); d != "" {
	dirs = []string{d}
	}

to

if d := os.Getenv(certDirEnv); d != "" {
        // OpenSSL and BoringSSL both use ":" as the SSL_CERT_DIR separator.
        // See:
        //  * https://golang.org/issue/35325
        //  * https://www.openssl.org/docs/man1.0.2/man1/c_rehash.html
 	dirs = strings.Split(d, ":")
 } 

[gopherbot]

Change https://golang.org/cl/205237 mentions this issue: crypto/x509: load roots from colon separated SSL_CERT_DIR in loadSystemRoots

[odeke-em]

@freedge I sent a CL but we are currently in a code freeze, so this will have to wait until February 2020.

[freedge]

super cool, thanks for your quick work!
I will try to validate your fix one way or another :)

Also, nobody had complained about this for like 10 years, so it can't be super important.

I think having an InsecureSkipVerify option in the tls.Config (or a -k option to curl) is a mistake. The doc in tls.Config says it "should be used only for testing" but it does not describe any alternative, and there is nothing that validates that this parameter is not used on a production system. Practically, it is just too convenient to set up a simple boolean to make your stuff work (solving your immediate problem: connecting to a server, no matter its certificate), that people won't look for alternatives.

This is what govc (cli to target the vmware api) does:

• provide an "insecure" mode that sets the InsecureSkipVerifyMode, through an environment variable
• for motivated users, provide a -tls-ca-certs command line parameter
• since openssl commands to retrieve a cert are not well know (and pretty awkward), also provide an about.certs command to dump the certificate easily

SSL_CERT_DIR is a pretty unknown environment variable. But if you think about having every single tool written in go getting rid of the InsecureSkipVerifyMode config parameter, while minimizing the development cost, and the burden put on the user, then SSL_CERT_DIR is a good choice. 0 development needed, and users can just put it into their environment (condition being: it does not break other tools! which can now be achieved thanks to your work) - much better than coding a tls-ca-certs parameter yourself, that users need to specify at every call. Next step could be to ease the retrieval of that certificate (eg: if certificate is untrusted, provide it into the Error message thrown by httpclient?) and cover extra usecases (for example certificate is for a subject that is different from the hostname in the URL)...

To sum up, nobody complained because SSL_CERT_DIR is not well known and security not that important.

In any case, thanks for your work, I'm just reporting it as it would save me 15 lines of code for my own little project - for which I am the only user :)