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

Closed
nolith opened this issue Apr 4, 2018 · 9 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@nolith
Copy link

nolith 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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 4, 2018
@FiloSottile FiloSottile added this to the Unplanned milestone Apr 4, 2018
@FiloSottile
Copy link
Contributor

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
Copy link
Author

nolith 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
Copy link

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
Copy link
Contributor

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
Copy link

@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
Copy link

stanhu 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
Copy link

@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.

@saracen
Copy link
Contributor

saracen commented Dec 21, 2021

I recently revisited this problem and there's been some changes since this was last discussed.

curl not treating intermediate certificates in the trust store as trust-anchors is only true if the ssl backend is openssl.

libcurl/7.68.0 (release Jan 2020) rectified this inconsistency using openssl's X509_V_FLAG_PARTIAL_CHAIN flag: curl/curl@94f1f77.

The change in go1.9 made the behaviour similar to the majority of ssl backends (though openssl has yet to make X509_V_FLAG_PARTIAL_CHAIN the default: openssl/openssl#7871).

git requires at least libcurl/7.19.4 so it will take a while for git built with openssl to work consistently in this respect, but debian/ubuntu use gnutls (so unaffected) and alpine upgraded to a libcurl that supports this from v3.11 (release 2019), so it's likely less of an issue today than it previously was.

It would be nice if an option passed to x509.Certificate.Verify could disable partial chains so that it could be used to return a chain whose root is a self-signed trust anchor and not a leaf or intermediate.

@FiloSottile
Copy link
Contributor

The nature of trust anchors is subtle: at their core, they are just a Subject and a public key. Then, you might want to attach metadata to them, or even custom policies, such as "can only issue for these names", "can only issue for these purposes", and "can only issue until this date". The temptation to encode trust anchors as certificates is strong, and most libraries (Go and OpenSSL included) do that, but there is no consensus on how to do that. Proper root stores (Windows, macOS, ...) gave up on separating the trust anchor metadata and the verifier code, which is why we are moving to invoking the platform verifiers. Go has only one bit of metadata: trusted (opts.Roots) or untrusted (opts.Intermediates). If something is in Roots, it's trusted. Regrettably, Linux doesn't really have a place for untrusted intermediates, or any metadata, so all certificates go in Roots. OpenSSL uniquely encodes the trust bit as self-signing, unless X509_V_FLAG_PARTIAL_CHAIN is set, in which case it behaves somewhat like Go. Go and OpenSSL with X509_V_FLAG_PARTIAL_CHAIN both lack a way to bring in untrusted intermediates from the system (which again we are handling by delegating to the platform verifier where a system store of untrusted intermediates exists). Indeed, the discussion on making X509_V_FLAG_PARTIAL_CHAIN the default devolved into a discussion of what I am describing above: openssl/openssl#7871. In Go, this is really part of #39540. The specific issue though looks like is solving itself, because most of the ecosystem is moving towards enabling X509_V_FLAG_PARTIAL_CHAIN and behaving like Go does. I don't think we should add complexity to be compatible with OpenSSL here.

/cc @golang/security

gitlab-runner-bot pushed a commit to gitlabhq/gitlab-runner that referenced this issue Nov 7, 2022
In the past, the runner needed to resolve a full TLS certificate
chain, including the self-signed root, in order for Git clones to work
over HTTPS. Go 1.9 changed the behavior to present a partial
certificate chain if a trusted intermediate certificate were placed in
the system certificate directory
(golang/go#24685).
https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/1581
worked around that change by restoring the Go 1.8 behavior of
presenting the full chain in `CI_SERVER_TLS_CA_FILE`.

libcurl v7.68 has since fixed the behavior to trust a certificate
authority that is not self-signed
(curl/curl@94f1f77).
As a result, the need to resolve the full chain is no longer
necessary. As long as there is a trusted certificate authority in the
chain, the TLS connection can proceed.

Go 1.18 modified `Certificate.Verify` to use the macOS and
Windows-specific platform APIs. As a result, a root certificate signed
with a SHA-1 certificate will be rejected, which prevents the runner
from generating `CI_SERVER_TLS_CA_FILE`. This may cause Git clones to
fail.

This commit adds a feature flag, `FF_RESOLVE_FULL_TLS_CHAIN`, that is
enabled by default. This flag makes it possible to disable this
resolving of the full certificate chain. On most platforms, this can
be disabled safely, assuming Git and other clients are compiled with
an updated libcurl version.

Relates to https://gitlab.com/gitlab-org/gitlab-runner/-/issues/29373
@golang golang locked and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants