crypto/x509: support code-constrained roots

[rolandshoemaker]

CertPool and Certificate.Verify currently only support roots constrained using X509 semantics. For most cases this is fine, but with the introduction of the golang.org/x/crypto/x509roots/fallback package (#43958), which plans to use the Mozilla NSS root list, there is a need to implement additional constraints.

The Mozilla NSS list encodes additional trust constraints for specific root certificates, such as only trusting certificates issued before a specific date (e.g. what is happening with TrustCor, or what previous happened with Symantec), etc. In order to support these constraints we need a new API which allows for attaching additional checks to certificates which chain to particular roots.

The API suggested below, CertPool.AddCertWithConstraint, would allow attaching a callback to a root, which Certificate.Verify would call during chain building. There would be no other externally visible API changes, as the root->constraint mapping would be a private structure in the CertPool, accessible only by Certificate.Verify.

As always the name is up for debate. The meaning of 'Constraint' in X509 is perhaps overloaded, AddCertWithAdditionalChecks could also work, but is possibly a little verbose. Another question is if the function should accept a single callback, or a list of callbacks (i.e. should the caller compose all of their constraints into a single callback, or should we allow passing multiple callbacks that we check ourselves serially).

// AddCertWithConstraint adds a certificate to the pool with the additional constraint.
// When Certificate.Verify builds a chain which is rooted by cert, it will additionally
// pass the chain to constraint to determine its validity. If constraint returns a non-nil
// error, the chain will be discarded.
func (*CertPool) AddCertWithConstraint(cert *Certificate, constraint func([]*Certificate) error)

cc @FiloSottile @golang/security

[seankhliao]

This allows adding constraints to individual certs as they're added to the pool, but what if you're starting with the system pool and want to layer additional checks on top of existing certs/system verifier checks?

[rolandshoemaker]

I don't think we want to support adding additional arbitrary constraints to platform verifiers. Platforms have their own ways of applying constraints to trust stores (i.e. both Windows and macOS allow you to constrain certificates in complicated ways), and I think we'd prefer that the platform verifier, that can more properly understand those constraints, apply them themselves.

If users really want to apply their own constraints purely in code, that can be done by wrapping calls to Verify with the logic that excludes specific chains.

For platforms that don't allow applying additional constraints (namely Linux), the root pool can be loaded manually, and constraints can be added at that point (we've considered exposing the logic used to load Linux trust stores, which perhaps we should do to make this simpler).

[rsc]

What is the motivation for this, and how does it affect the feasibility of the algorithms for building the chain? Do we have to build all possible chains ending at a given root to try to make the constraint happy?

And why is it only about roots? That makes the combinatorics better (but not great), but nothing in the API suggests it would only be about roots.

Is there something simpler we can do, like allowing a parent cert to disallow a direct child?

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rolandshoemaker]

The motivation as described in the parent comment is that there are trust stores that encode additional trust constraints outside of those that can be defined in X509. In particular with the introduction of the golang.org/x/crypto/x509roots package, there are roots in the NSS list that contain additional constraints around trust based on issuance dates. Currently we just ignore roots with these extra constraints, but it'd be more valuable for users to attempt to match behavior of NSS itself, and apply the additional constraints consistently.

This has no meaningful impact on the chain building algorithm, it can be implemented in a similar way to how EKU chaining checks are applied. We already build all chains from a leaf.

It is unlikely you'd want to apply constraints to intermediates (the only other anchor type feasibly in a CertPool), but there is not really any extra complexity to check both roots and intermediates (either during the path building process, or once it is completed).

[rsc]

Spoke to @rolandshoemaker about this. The code already generates all possible chains (paths from the leaf back to a known root), and while that is potentially exponential in the number of intermediate certs, you can't get an signed, trusted intermediate cert without essentially being a CA, so the opportunity does not really exist for a malicious peer to create a large enough graph problem to cause issues. Things are also helped by the relevant TLS message being fairly limited in size.

This new API locks us in to that decision, but some other semantics of certficate chain validation arguably do too, and since the attack surface seems not to exist, using this API seems okay to me.

[rsc]

Have this all concerns about this proposal been addressed?

[rolandshoemaker]

I believe so.

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

[gopherbot]

Change https://go.dev/cl/519315 mentions this issue: crypto/x509: implement AddCertWithConstraint