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.

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.

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.

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.

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.

cc @golang/security

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.

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.

"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?

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.

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

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.

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

🤷

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

@FiloSottile I'm confused. As far as I can tell it already enforces that the signer have the CA bit set, regardless of whether it is a root or even a self-signed root.

Adapted from the original example: https://go.dev/play/p/Fbj0U94ep9G

Oh right, we enforce this in CheckSignatureFrom. So it seems like the question here is: should you be able to add certificates to the pool which do not have the cA bit set. They will never be used when building a proper chain, but will for direct matches.

Since we can't break direct matches, seems like there is nothing we can really change here? (although possibly the check from CheckSignatureFrom should be duplicated in isValid.)

Oh I did not realize we have the check in CheckSignatureFrom. Then yeah, I think there is no behavior change to make here, sorry. We should fix isValid to be consistent, though.