crypto/x509: VerifyOptions.Roots does not enforce that certificates are root certificates

[Hayden-IO]

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

$ go version

1.18

What did you do?

https://go.dev/play/p/InpOJXUXnUl

A leaf and intermediate certificate can be provided to the root pool.

What did you expect to see?

I expected that verification would fail due to the lack of root certificates.

What did you see instead?

Successful verification.

It's unclear to me if this is an intentional feature or a bug.

Should a certificate in a root pool always be a CA certificate (related to RFC5280 4.2.1.9)?
The check that a parent is a CA certificate occurs in CheckSignatureFrom, called in buildChains, which is bypassed because the leaf certificate is in the root pool (https://go.dev/src/crypto/x509/verify.go#L781). Allowing a non-CA certificate to be verified successfully against itself seems prone to bugs.

Is a root certificate meant to be defined as a self-signed certificate, or any certificate that forms the trust anchor?
I expected that it must be a self-signed certificate, but an intermediate was permitted. This also seems prone to bugs, for example if a client supports a user-provided certificate chain and the client isn't enforcing that a chain is formed from a root.

[rittneje]

I'm confused. Your application is responsible for putting the trusted "roots" in the Roots field. Whether these are self-signed or not is ultimately a configuration decision on your part. (This is analogous to OpenSSL's -partial_chain flag.)

This also seems prone to bugs, for example if a client supports a user-provided certificate chain and the client isn't enforcing that a chain is formed from a root.

A client-provided chain is by definition untrusted. Those certificates belong in Intermediates not Roots. Otherwise, you have a bug anyway where the client can just present their own crafted chain back to their own self-signed root.

[Hayden-IO]

This seems like an intentional feature then. If there’s a separate flag in OpenSSL for this, does it seem reasonable to also have a separate configuration option to explicitly say that you want to verify with a partial chain? This is focused on reducing the chance of a client's misconfiguration leading to an issue.

[rittneje]

I'm not sure what kind of misconfiguration you are intending to protect against. Applications are responsible for putting only the trusted root anchors in the Roots field (or equivalently in tls.Config.RootCAs). If you are putting untrusted certificates there, then you will always have a security issue, regardless of whether Go requires self-signed roots.

If I want to use an intermediate (as in not self-signed) certificate as a trust anchor, it seems pretty weird to have to pass another flag to tell it that I really mean what I already told it by including said certificate in the pool.

I will also say that, from experience, OpenSSL's default behavior of not properly honoring the configured trust pool when the certificate is not self-signed is extremely surprising and confusing behavior. Personally, I think it is a bug. I'm not sure what motivated the introduction of that flag - perhaps this functionality was added later, and they wanted to be very careful about backwards compatibility.

[Hayden-IO]

Thanks for your perspective. To me, it seems unexpected that partial chains are supported by default. I would assume users are more likely to verify a certificate against a full chain, so needing to explicitly specify the preference to verify a partial chain seems reasonable to me.

With respect to the misconfigurations, I'm just referring to any type of bug where the application parses a chain. For example say there's an off-by-one error when reading a chain, and part of the chain is dropped off. If a partial chain wasn't verified by default, those bugs would be caught.

[rittneje]

In order for a "partial" chain to verify, you need to have put one of the intermediates into the Roots CA pool. If you are missing one of the links between the leaf certificate and the trusted root (which belongs in Intermediates), then it will not verify, regardless of whether your root is self-signed. Consequently, the off-by-one issue you are describing is not relevant.

Do you have a real world example of how someone would put an intermediate certificate in Roots, but not intend for it to be used as a root? From what you are describing, this is only possible if you take the chain presented by the client and put it in Roots. But this is clear API misuse, and a major security hole, as the chain presented by the client belongs in Intermediates not Roots.

[seankhliao]

cc @golang/security

[Hayden-IO]

A command line application that accepts a user-provided chain would be an example. This is how I came upon this, working on a CLI app and testing out various chains, full and partial. The CLI app assumes that a chain starts with an intermediate and ends with a root. I had assumed that the library would handle verification that the root certificate is a self-signed root, but if I provided a truncated chain without a root, it was accepted since it's a partial chain.

[rittneje]

But what is the purpose of your command line application? When verifying a certificate chain, you need a trust pool, which is completely distinct from the chain presented by the client. For example, openssl verify has -untrusted for the chain presented by the client, and -trusted for the root of trust chosen by your application. Without this distinction, it is unclear what statement your application is attempting to prove, as a malicious client could choose to include any arbitrary self-signed certificate in the chain.

[FiloSottile]

"What's a trust anchor" is somewhat of a grey area of certificate verification APIs. Some implementations—Go included—use certificates to express trust anchors, which really just need to be a Subject, a Subject Key Identifier, a Subject Public Key Info, and an associated policy. When using certificates, it comes natural to also use the other certificate fields to encode policy, so there are implementations that will look at one or more of Not After, Extended Key Usage, Basic Constraints, Name Constraints, and Issuer (where if it doesn't match the Subject it's considered an untrusted intermediate, what this issue is about). That's somewhat suboptimal, because it should be possible to express policy choices without collaboration from the root operator: for example, you might want to restrict a root to only issuing for *.it without re-issuing it with a Name Constraints extension. Moreover, you might want to express policies for which there is no X.509 marker, like "all certificates must be logged in CT". Modern verifiers gave up on encoding the policies and made them part of the verifier code, which is why we are moving away from extracting system roots and towards using platform verifiers where available. Now, back to Go. crypto/x509 doesn't have a rich policy engine, but can express trusted vs untrusted with the separate RootCAs and Intermediates pools in VerifyOptions, so it doesn't use the Issuer to encode that bit of policy. It will check Not Before, Not After, Extended Key Usage, and Name Constraints though. It will not check that Basic Constraints has the CA bit, but it will enforce pathlen if present.

Accepting not self-signed certificates is not going to change, both because it would be a significant backwards compatibility break, and because there is already a way to express that policy distinction: RootCAs vs Intermediates. It's also useful sometimes to express trust in an intermediate without trusting its parent.

Not sure why we don't enforce the CA bit though, and that might be worth fixing, as a certificate without the CA bit should never issue anything at all. @rolandshoemaker, WDYT?

[rolandshoemaker]

It's probably fine to restrict things being added to the pools to having the isCA bit set, since we already check that roots and intermediates have that bit set during Verify.

This would break one use case though: when the certificate being verified is directly added to the root pool (in which case we short circuit all chain building logic in Verify and simply return the certificate being verified as the only certificate in the chain.) Not really sure how common that is, likely only happens when people are self-signing and then shoving a single cert into the root pool.

[ericlagergren]

Requiring the isCA bit would would break cert pinning, which is normal (perhaps unfortunately so) for some applications.

[FiloSottile]

I'm not suggesting restricting adding to the pool, I am suggesting rejecting in Verify chains where the signer doesn't have the isCA bit, even if it's a root certificate. Direct match of the leaf with the roots pool would still work, because it doesn't involve a signature check.

In other words, I am suggesting changing this line to also apply the check for root certificates.

go/src/crypto/x509/verify.go

Lines 687 to 689 in 2580d0e

	if certType == intermediateCertificate && (!c.BasicConstraintsValid || !c.IsCA) {
	return CertificateInvalidError{c, NotAuthorizedToSign, ""}
	}

[rolandshoemaker]

Seems reasonable on its face, although I'm somewhat worried how this will affect non root program enterprise-y type roots, where the adherence to standards is not... great. Perhaps we're fine breaking them, but the main problem is it's basically impossible to know how big of an effect it'll have (and often the developers are going to have little recourse to fix the problem themselves.)

🤷

[ericlagergren]

You can always put the current behavior behind a GODEBUG flag for Go 1.19 then remove the flag in Go 1.20.