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: http.Response.TLS.VerifiedChains behavior changed in go1.9 #24685

Open
nolith opened this issue Apr 4, 2018 · 7 comments

Comments

@nolith
Copy link

commented Apr 4, 2018

Hello, I've found a behavior change in go1.9.x

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

this is a comparison between go version go1.9.5 linux/amd64, go version go1.10.1 linux/amd64 and go version go1.8.7 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

It affects linux, it can be reproduced using golang docker images

What did you do?

I prepared a small POC with automated execution in CI.

https://gitlab.com/nolith-tests/go-lang-tls-connection-state/blob/master/main.go

The relevant part is in this function

func dumpConnectionState(url string) {
	fmt.Println("URL", url, "with", runtime.Version())
	r, err := http.Head(url)
	if err != nil {
		panic(err)
	}

	fmt.Println("VerifiedChains len", len(r.TLS.VerifiedChains))
	for i, verifiedChain := range r.TLS.VerifiedChains {
		fmt.Println("Chain #", i)
		for j, certificate := range verifiedChain {
			signature := hex.EncodeToString(certificate.Signature)
			fmt.Println("[", j, "] =>", certificate.Subject.CommonName, signature)
		}
	}
}

This function will perform HTTPS HEAD request and then dump the VerifiedChains,
in a vanilla Linux environment, this program behaves exactly the same if compiled with go1.8.7 or go1.9.5

But when the leaf certificate is added to /etc/ssl/certs/ go1.8 will still dump the whole chain,
but go1.9 will print only the leaf certificate skipping the rest of the chain.

The test has been automated with this .gitlab-ci.yml configuration

What did you expect to see?

I did expect to have the same content in http.Response.TLS.VerifiedChains regardless of go version.

What did you see instead?

go 1.8.7 output
go 1.9.5 output
go 1.10.1 output

compare 1.8.7 and 1.9.5 outputs

@FiloSottile FiloSottile changed the title http.Response.TLS.VerifiedChains behavior changed in go1.9 crypto/x509: http.Response.TLS.VerifiedChains behavior changed in go1.9 Apr 4, 2018

@FiloSottile FiloSottile added this to the Unplanned milestone Apr 4, 2018

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Apr 4, 2018

The more recent behavior is not wrong, as the certificate is both leaf and root. If anything, 1.8 was wrong not to catch that.

I am not sure if we try to make VerifiedChains (which is just the return value of x509.Certificate.Verify) include all possible chains, but we don't promise that in the docs.

Can you tell us more about why you need the full chain?

@nolith

This comment has been minimized.

Copy link
Author

commented Apr 4, 2018

Sure, that code example is extracted from gitlab-runner

We perform a full CA chain verification and extraction on the host performing API calls to GitLab server; then we dump such chain inside the CI executor environment, which may be a container, a shell, a virtual machine even on a different host.
Then we point GIT_SSL_CAINFO to that file and we have git cloning with SSL, also with a self-signed certificate.

When we upgraded to go 1.9 users started reporting the inability to clone
https://gitlab.com/gitlab-org/gitlab-runner/issues/3180
https://gitlab.com/gitlab-org/gitlab-runner/issues/3183

@tmaczukin

This comment has been minimized.

Copy link

commented May 15, 2018

The more recent behavior is not wrong, as the certificate is both leaf and root. If anything, 1.8 was wrong not to catch that.

@FiloSottile This is right, when the certificate found locally is a self-signed certificate. But @nolith mentioned a case, where user has a leaf certificate, that is not self-signed, stored in a system certificate directory. And this ends with a situation, where a VerifiedChains method returns not a chain from a leaf certificate to root CA, but only the leaf (not self-signed) - without any CA.

For Git for example this is not enough to verify the requested HTTPS server, since there is no CA (self-signed) certificate available in the chain, that would verify site's certificate.

Since I was already checking the internals of SSL verification in Go in context of other problem, I've decided to compare 1.8.x and 1.9.x to understand what caused this issue.

r.TLS.VerifiedChains is feed with the result of x509.Certificate.Verify(opts), where opts.RootCAs is nil. This forces a system certificates store to be loaded (this will be important in a while). The x509.Certificate.Verify(opts) is implemented in https://github.com/golang/go/blob/go1.8.7/src/crypto/x509/verify.go#L267. The comment of the method says that

If successful, it returns one or more chains where the first element of the chain is c and the last element is from opts.Roots

c here is of course the certificate that is being verified. Also the header of the method suggests, that the method will return a chain:

func (c *Certificate) Verify(opts VerifyOptions) (chains [][]*Certificate, err error) {

The part of the code that forces the leaf certificate to be returned as the chain (no matter if it's self-signed or not) is this if statement: https://github.com/golang/go/blob/go1.8.7/src/crypto/x509/verify.go#L306. Which is exactly the same in 1.9.6: https://github.com/golang/go/blob/go1.9.6/src/crypto/x509/verify.go#L312! "If the certificate itself is found in the RootCAs list, then just use it as the chain". The rest of the method is not important in our case, since it only filters the already created chain against a configured key usage.

This code was added with 8ad70a5 and it exists since August 2016.

If this exists in that form for so long, but we've been hit by this after migrating to 1.9.x, I started to search what caused it. And I found it: e83bcd9#diff-7d6780a874c9e5d3777b2e560f75ec5dL32. Before this change, if a system CA file was found, then the return was raised and the system certificates pool contained only the main store. In case of Linux (which is the case of our problem) it means the content of e.g. /etc/ssl/certs/ca-certificates.crt file. If the leaf certificate was stored under /etc/ssl/certs/any-file.crt, it was not loaded to the pool.

e83bcd9 changed it, so both - system default CA file, and system default CAs directory - are loaded. It was done because:

Certificates in the first valid location found for both file and directory are added, instead of only the first file location if a valid one was found, which is consistent with OpenSSL.

Which seems to be a bug fix, so definitely something needed.

I've tested a quick hack (bringing back the return roots, nil in 1.9.6 codebase) against @nolith's test project, and it helped. So this is the change that generated the problem.

Conclusions:

Loading both CA file and CAs directory is something that is expected. So change between 1.8 and 1.9 is valid.

Also probably expected is that the certificate is used directly, if it's already loaded in roots pool.

What I would propose here is to use the certificate directly only if it's self-signed. If not by default, then at least to make it configurable. For some cases a leaf, non-self-signed certificate is just unusable.

@FiloSottile

This comment has been minimized.

Copy link
Member

commented May 15, 2018

Yes, that’s correct, but there is no rule saying roots need to be self-signed. The WebPKI is complex and it involves many cross-signatures, so it’s not uncommon to have what would be an intermediate elsewhere in a root CA pool.

So if I understand correctly, the issue is that git does not work if the leaf is passed as a root? That sounds like a git bug to me.

@tmaczukin

This comment has been minimized.

Copy link

commented May 15, 2018

@FiloSottile Git uses libcurl for handling HTTP(s) connections. The file with certificates chain that we're writing about is pointed to CURLOPT_CAINFO setting.

According to libcurl's documentation:

Curl verifies whether the certificate is authentic, i.e. that you can trust that the server is who the certificate says it is. This trust is based on a chain of digital signatures, rooted in certification authority (CA) certificates you supply. curl uses a default bundle of CA certificates (the path for that is determined at build time) and you can specify alternate certificates with the CURLOPT_CAINFO option or the CURLOPT_CAPATH option.

If the leaf certificate is not a root for itself (which is a case only of self-signed certificates), the above is not fulfilled. If I understand the documentation correctly, the chain configured by CURLOPT_CAINFO or CURLOPT_CAPATH need to contain at least the issuer of the leaf certificate that is being verified. And if we would consider this as a bug, then... well... quite significant number of programs and libraries in the world, that are relying on libcurl, would be buggy.

The general idea of PKI is that we don't need to know each one certificate - we only need to know and trust to a CA certificates that will sign the leaf ones. If we can't provide a full chain, then sure, let's provide at least the leaf certificate itself if it's in systems CA store. It will work for at least some software. But if we can, why to force a one-element chain with the leaf certificate instead of the full chain that really verifies the validity of it?

8ad70a5 pointed that In other systems, putting a leaf certificate in the root store works to express that exactly that certificate is acceptable. As you can see, not all systems (e.g. libcurl) agree with such statement. And since x509/tls/https support is quite low-level and a general purpose mechanism (that may be used in many different cases) why not make it at least configurable?

I have a PoC of such change. I can open a PR and share it if there is a will of going in that direction.

@stanhu

This comment has been minimized.

Copy link

commented Dec 31, 2018

I've just had a chance to catch up with this discussion and examine the behavior with libcurl with OpenSSL. As @tmaczukin mentioned, curl will fail TLS verification if you provide the output of r.TLS.VerifiedChains from a leaf certificate that happens to be added to the system cert directory (e.g. /etc/ssl/certs in Linux).

Is the Go implementation assuming that all certificates in /etc/ssl/certs (for Linux) are root certificates and therefore a chain with just that leaf is sufficient? That does seem fundamentally at odds with OpenSSL's implementation, which does appear to continue verifying the trust chain and treat self-signed certificates as a trust anchor: https://github.com/openssl/openssl/blob/0b4233f5a4a181a6dcb7c511cd2663e500e659a4/crypto/x509/x509_vfy.c#L3073-L3076

If treating self-signed certificates as anchors isn't the right behavior, could we do as @tmaczukin suggests and make it possible for Go to continue verifying the chain if it finds a leaf certificate that happens to be in the root store?

@redbaron

This comment has been minimized.

Copy link

commented May 25, 2019

@FiloSottile , whats your view on provided arguments above? It is a bug, that Go doesn't always produce chain which can be verified by OpenSSL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.