crypto/x509: add ParseRevocationList, deprecate ParseCRL & ParseDERCRL

[rolandshoemaker]

The current implementation of CRLs in crypto/x509 and crypto/x509/pkix is somewhat confusing, easy to misuse, and does not match the design of the rest of the package. In particular it doesn't expose the necessary information to do safe issuer verification (see grpc/grpc-go#5130 for an example of how this can go wrong.)

We could try to provide the extra information required to do safe comparisons, but unfortunately due to the design of pkix.CertificateList, which is intended to be a direct ASN.1 analog, it is not possible to add new fields, since it will inherently change how encoding/asn1 encodes/decodes related data.

In go1.15 we introduced RevocationList, a type used as input to CreateRevocationList. I propose that we introduce ParseRevocationList, and convert RevocationList into a Go representation of a CRL, similar to how Certificate is used. This results in an API much more in line with the rest of the package, and would allow us more leeway to update the representative CRL structure without having to worry about its direct ASN.1 encoding.

For the sake of slowly moving away from reliance on encoding/asn1, the new ParseRevocationList function should employ a x/crypto/cryptobyte parser.

This would deprecate the pkix.CertificateList type (and associated types), the ParseCRL and ParseDERCRL functions, and the Certificate.CheckCRLSignature method (the latter being replaced with a method on RevocationList.)

// Added (crypto/x509)
func ParseRevocationList(der []byte) (*RevocationList, error)

func (rl *RevocationList) CheckSignatureFrom(issuer *Certificate) error

type RevocationList struct {
  ...
  // New fields
  RawIssuer      []byte
  Signature      []byte
  AuthorityKeyId []byte
  Extensions     []pkix.Extension
}

// Deprecated (crypto/x509)
func ParseCRL(crlBytes []byte) (*pkix.CertificateList, error)
func ParseDERCRL(derBytes []byte) (*pkix.CertificateList, error)
func (c *Certificate) CheckCRLSignature(crl *pkix.CertificateList) error

// Deprecated (crypto/x509/pkix)
type CertificateList ...
type TBSCertificateList ...

cc @golang/security

[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

[rsc]

Does anyone object to the API in the top comment?

[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/390834 mentions this issue: crypto/x509: add new CRL parser, deprecate old one

[rsc]

CL 390834 added three additional fields beyond what was in this proposal:

Raw                  []byte
RawTBSRevocationList []byte
Issuer         pkix.Name

Noting here in case anyone objects to them, although I doubt it given the vigor of the earlier discussion.

[gopherbot]

Change https://go.dev/cl/414635 mentions this issue: crypto/x509: improve RevocationList documentation