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.

Sign up for GitHub

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

Jump to bottom

crypto/x509: Trust relationship of certs returned by 'x509.SystemCertPool()' is ambiguous #39540

Open
stephen-fox opened this issue Jun 12, 2020 · 5 comments
Open

crypto/x509: Trust relationship of certs returned by 'x509.SystemCertPool()' is ambiguous #39540

stephen-fox opened this issue Jun 12, 2020 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@stephen-fox
Copy link

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

$ go version
go version go1.13.7 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

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

CentOS 7.8.

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/usr/lib/golang"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/golang/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-build466747349=/tmp/go-build -gno-record-gcc-switches"

What did you do?

On a CentOS 7.8 system, the x509.SystemCertPool() function returns a CertPool containing all X.509 certificates stored in the directory /etc/pki/tls/certs/. While I cannot find a clear source describing the purpose of this directory, there is RedHat documentation that indicates Apache httpd end entity (server) certificates can be stored there (I believe this is also the default directory for end entity certificates in the httpd configs as well). This can be seen in the following snippets from https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/5/html/deployment_guide/s1-httpd-secure-server:

25.8.3. Using Pre-Existing Keys and Certificates
[...]
Move your existing certificate file to: 
/etc/pki/tls/certs/server.crt

25.8.6. How to configure the server to use the new key
[...]
Edit /etc/httpd/conf.d/ssl.conf. Change the SSLCertificateFile and SSLCertificateKey lines to be.
SSLCertificateFile /etc/pki/tls/certs/www.example.com.crt
SSLCertificateKeyFile /etc/pki/tls/private/www.example.com.key

I would never implicitly trust these certificates, even if I am serving them. However, the CertPool returned by SystemCertPool() includes them via loadSystemRoots() in 'root_unix.go'.

Taking a look at the function, its current documentation states:

// SystemCertPool returns a copy of the system cert pool.
//
// Any mutations to the returned pool are not written to disk and do
// not affect any other pool returned by SystemCertPool.
//
// New changes in the system cert pool might not be reflected
// in subsequent calls.

I realize that there is no mention of trust relationship here - however, in a few spots in the code, there are hints about "roots", which to me indicates CA certificates (or at least non-server certificates). For example the error message in 'cert_pool.go', line 50:

return nil, errors.New("crypto/x509: system root pool is not available on Windows")

And the function name loadSystemRoots(), and the local variable name in 'root_unix.go', line 39:

func loadSystemRoots() (*CertPool, error) {
	roots := NewCertPool()

The change to search in /etc/pki/tls/certs for certificates in loadSystemRoots() occurred in:
e83bcd9

I do not see any obvious reason why any random certificate file in this directory would be trusted, barring ca-bundle.crt. As noted in the RedHat documentation, there could be end entity certificates stored in /etc/pki/tls/certs... So, Go's behavior feels incorrect.

At the very least, the purpose of SystemCertPool() feels ambiguous.

What did you expect to see?

I expected x509.SystemCertPool() to return only CA certificates trusted by the operating system.

What did you see instead?

The function returns non-CA, or end entity certificates that are not trusted by the operating system (specifically, on CentOS 7.8). This can result in unexpected validation paths ("verified chains" as referenced by the Go library) when connecting to TLS servers.
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 12, 2020
@dmitshur
Copy link
Contributor

/cc @FiloSottile @katiehockman

@dmitshur dmitshur added this to the Backlog milestone Jun 12, 2020
@FiloSottile FiloSottile modified the milestones: Backlog, Go1.17 Oct 5, 2020
@FiloSottile FiloSottile self-assigned this Oct 5, 2020
@FiloSottile
Copy link
Contributor

Yeah, SystemCertPool needs to be cleaned up and documented, but given we had major changes to crypto/x509 in Go 1.15 I'd like to give it a rest in Go 1.16, so targeting Go 1.17.

@FiloSottile FiloSottile mentioned this issue Oct 5, 2020
Open
@FiloSottile FiloSottile removed their assignment Mar 17, 2021
@ianlancetaylor
Copy link
Member

Do we still expect to address this for 1.17? Thanks.

@knweiss
Copy link

knweiss commented May 13, 2021

FWIW: Red Hat's documentation describes the Shared System Certificates mechanism like this:

The Shared System Certificates storage allows NSS, GnuTLS, OpenSSL, and Java to share a default source for retrieving system certificate anchors and black list information. By default, the trust store contains the Mozilla CA list, including positive and negative trust. The system allows updating of the core Mozilla CA list or choosing another certificate list.

I.e. this is what I would consider the "SystemCertPool" of Red Hat/CentOS. (btw: I agreed this name is not optimal.)

It works like this: You can place additional CA certs in anchor/blacklist cert directories (see the link for details) and then execute update-ca-trust which will generate (among others) the /etc/pki/tls/certs/ca-bundle.crt file (actually a symlink).

To quote the update-ca-trust(8) man page:

/etc/pki/tls/certs/ca-bundle.crt - Classic filename, file contains a list of CA certificates trusted for TLS server authentication usage, in the simple BEGIN/END CERTIFICATE file format, without distrust information. This file is a symbolic link that refers to the consolidated output created by the update-ca-trust command.

Thus, from my understanding, if x509.SystemCertPool() parses only(^1) the file /etc/pki/tls/certs/ca-bundle.crt from the /etc/pki/tls/certs/ directory it would result in the expected behaviour and Go could even be added to the list of libraries/languages in the first quote above that respect the Shared System Certificates storage.

Actually, Go already does this via the certFiles list in https://go.googlesource.com/go/+/go1.16.4/src/crypto/x509/root_linux.go.
However, Go then continues to also iterate over thecertDirectories list that includes /etc/pki/tls/certs.

^1: Of course, the SSL_CERT_FILE and SSL_CERT_DIR overrides should always be supported.

(Note: There's also the generated file /etc/pki/tls/certs/ca-bundle.trust.crt that also includes distrust information.)

@FiloSottile FiloSottile mentioned this issue May 20, 2021
Closed
@ianlancetaylor ianlancetaylor changed the title crypto/x509: Trust releationship of certs returned by 'x509.SystemCertPool()' is ambiguous crypto/x509: Trust relationship of certs returned by 'x509.SystemCertPool()' is ambiguous May 20, 2021
@dmitshur
Copy link
Contributor

It seems this is a bigger issue (as mentioned in proposal #46287) that won't happen in time for Go 1.17 given we're 3 weeks into the release freeze, so moving to Backlog. Please update it as needed.

@dmitshur dmitshur modified the milestones: Go1.17, Backlog May 21, 2021
@FiloSottile FiloSottile mentioned this issue Mar 3, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@knweiss @FiloSottile @dmitshur @ianlancetaylor @stephen-fox