x/crypto/x509roots: new module

[rolandshoemaker]

#43958 is the accepted proposal for introducing the x509.SetFallbackRoots API, and had a rough outline of the bundle module. We want to have a slightly different API for the bundle package than was originally outlined there, so instead of continuing the discussion in the previous issue, I'm opening a new proposal that is tightly focused on x/crypto/x509roots (the name was previously decided on, this proposal will not rehash that discussion).

x/crypto/x509roots will be a submodule of x/crypto, containing the root package which contains the NSS certdata.txt parser, and a package x/crypto/x509roots/fallback, which registers the fallbacks on import.

API for x/crypto/x509roots:

// Package x509roots provides functionality for parsing NSS certdata.txt
// formatted certificate lists and extracting serverAuth roots. The parser
// provided by this package is very opinionated, only returning roots that are
// currently trusted for serverAuth. As such roots returned by this package
// should only be used for making trust decisions about serverAuth certificates,
// as the trust status for other uses is not considered. Using the roots
// returned by this package for trust decisions should be done carefully.
//
// Some roots returned by the parser may include additional constraints
// (currently only DistrustAfter) which need to be considered when verifying
// certificates which chain to them.
package x509roots

// NSSConstraint is a constraint to be applied to a certificate or
// certificate chain.
type NSSConstraint any

// NSSDistrustAfter is a NSSConstraint that indicates a certificate has a
// CKA_NSS_SERVER_DISTRUST_AFTER constraint. This constraint defines a date
// after which any certificate issued which is rooted by the constrained
// certificate should be distrusted.
type NSSDistrustAfter time.Time

// NSSCert represents a single trusted serverAuth certificate in the NSS
// certdata.txt list and any constraints that should be applied to chains
// rooted by it.
type NSSCert struct {
	// Certificate is the parsed certificate
	Certificate   *x509.Certificate
	// Constraints contains a list of additional constraints that should be
	// applied to any certificates that chain to Certificate. If there are
	// any unknown constraints in the slice, Certificate should not be
	// trusted.
	Constraints []NSSConstraint
}

// ParseNSSCertData parses a NSS certdata.txt formatted file, returning only
// trusted serverAuth roots, as well as any additional constraints.
func ParseNSSCertData(r io.Reader) ([]NSSCert, error)

API for x/crypto/x509roots/fallback (taken verbatim from #43958, other than package name changes):

// Package fallback embeds a set of fallback X.509 trusted roots in the
// application by automatically invoking [x509.SetFallbackRoots]. This allows
// the application to work correctly even if the operating system does not
// provide a verifier or system roots pool.
//
// To use it, import the package like
//
//	import _ "golang.org/x/crypto/x509roots/fallback"
//
// It's recommended that only binaries, and not libraries, import this package.
//
// This package must be kept up to date for security and compatibility reasons.
// Use govulncheck to be notified of when new versions of the package are
// available.
package fallback

cc @FiloSottile

[FiloSottile]

LGTM. Only returning serverAuth makes sense per #26624.

One thing I'm worried about is that if we ever add a new constraint to NSSCert, code written against an old version will consider them unconstrained. Is it worth making the field a []NSSConstraint so code can panic on unknown values?

If we ever switch to a different root store, we can add new functions to x509roots and replace the one we use in fallback.

nit: do we usually use nil *time.Time or zero time.Time as the missing value sentinel?

Not necessary to ship the package, but we should soon after implement automation to notice changes to the pool, and mail CLs to update the submodule and file a govulncheck entry.

[rolandshoemaker]

One thing I'm worried about is that if we ever add a new constraint to NSSCert, code written against an old version will consider them unconstrained. Is it worth making the field a []NSSConstraint so code can panic on unknown values?

Oh hm, good point. Do you imagine the NSSConstraint would be something like type NSSConstraint any and then we'd have type NSSDistrustDate time.Time etc, with a comment along the lines of "callers should not trust any certificate which contains constraints which they do not understand".

[FiloSottile]

I couldn't come up with anything more elegant than that, so yeah :)

[ianlancetaylor]

Note that the 1.20 release notes mention this package, which does not yet exist, so it would be good to prioritize this. Or change the release notes.

[rolandshoemaker]

We have an inflight CL for this, I'd like to get this in ASAP rather than updating the release notes, but at some point that may be prudent.

[gopherbot]

Change https://go.dev/cl/462036 mentions this issue: x509roots: add new module

[rolandshoemaker]

(I realized I forgot the golang/go prefix for the Fixes line 🤦)