crypto/x509: add new CRL parser, deprecate old one

Adds a new, cryptobyte based, CRL parser, which returns a
x509.RevocaitonList, rather than a pkix.CertificateList. This allows us
to return much more detailed information, as well as leaving open the
option of adding further information since RevocationList is not a
direct ASN.1 representation like pkix.CertificateList. Additionally
a new method is added to RevocationList, CheckSignatureFrom, which is
analogous to the method with the same name on Certificate, which
properly checks that the signature is from an issuing certiifcate.

This change also deprecates a number of older CRL related functions and
types, which have been replaced with the new functionality introduced
in this change:
  * crypto/x509.ParseCRL
  * crypto/x509.ParseDERCRL
  * crypto/x509.CheckCRLSignature
  * crypto/x509/pkix.CertificateList
  * crypto/x509/pkix.TBSCertificateList

Fixes #50674

Change-Id: I27dc219e39bef09a396e666b4fccaa32578fd913
Reviewed-on: https://go-review.googlesource.com/c/go/+/390834
Reviewed-by: Damien Neil <dneil@google.com>
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
Roland Shoemaker 2022-03-08 11:18:44 -08:00 committed by Gopher Robot
parent dbb52cc9f3
commit 2de2f6df64
5 changed files with 366 additions and 55 deletions

9
api/next/50674.txt Normal file
View File

@ -0,0 +1,9 @@
pkg crypto/x509, func ParseRevocationList([]uint8) (*RevocationList, error) #50674
pkg crypto/x509, method (*RevocationList) CheckSignatureFrom(*Certificate) error #50674
pkg crypto/x509, type RevocationList struct, AuthorityKeyId []uint8 #50674
pkg crypto/x509, type RevocationList struct, Extensions []pkix.Extension #50674
pkg crypto/x509, type RevocationList struct, Issuer pkix.Name #50674
pkg crypto/x509, type RevocationList struct, Raw []uint8 #50674
pkg crypto/x509, type RevocationList struct, RawIssuer []uint8 #50674
pkg crypto/x509, type RevocationList struct, RawTBSRevocationList []uint8 #50674
pkg crypto/x509, type RevocationList struct, Signature []uint8 #50674

View File

@ -164,53 +164,29 @@ func parseAI(der cryptobyte.String) (pkix.AlgorithmIdentifier, error) {
return ai, nil
}
func parseValidity(der cryptobyte.String) (time.Time, time.Time, error) {
extract := func() (time.Time, error) {
var t time.Time
switch {
case der.PeekASN1Tag(cryptobyte_asn1.UTCTime):
// TODO(rolandshoemaker): once #45411 is fixed, the following code
// should be replaced with a call to der.ReadASN1UTCTime.
var utc cryptobyte.String
if !der.ReadASN1(&utc, cryptobyte_asn1.UTCTime) {
return t, errors.New("x509: malformed UTCTime")
}
s := string(utc)
formatStr := "0601021504Z0700"
var err error
t, err = time.Parse(formatStr, s)
if err != nil {
formatStr = "060102150405Z0700"
t, err = time.Parse(formatStr, s)
}
if err != nil {
return t, err
}
if serialized := t.Format(formatStr); serialized != s {
return t, errors.New("x509: malformed UTCTime")
}
if t.Year() >= 2050 {
// UTCTime only encodes times prior to 2050. See https://tools.ietf.org/html/rfc5280#section-4.1.2.5.1
t = t.AddDate(-100, 0, 0)
}
case der.PeekASN1Tag(cryptobyte_asn1.GeneralizedTime):
if !der.ReadASN1GeneralizedTime(&t) {
return t, errors.New("x509: malformed GeneralizedTime")
}
default:
return t, errors.New("x509: unsupported time format")
func parseTime(der *cryptobyte.String) (time.Time, error) {
var t time.Time
switch {
case der.PeekASN1Tag(cryptobyte_asn1.UTCTime):
if !der.ReadASN1UTCTime(&t) {
return t, errors.New("x509: malformed UTCTime")
}
return t, nil
case der.PeekASN1Tag(cryptobyte_asn1.GeneralizedTime):
if !der.ReadASN1GeneralizedTime(&t) {
return t, errors.New("x509: malformed GeneralizedTime")
}
default:
return t, errors.New("x509: unsupported time format")
}
return t, nil
}
notBefore, err := extract()
func parseValidity(der cryptobyte.String) (time.Time, time.Time, error) {
notBefore, err := parseTime(&der)
if err != nil {
return time.Time{}, time.Time{}, err
}
notAfter, err := extract()
notAfter, err := parseTime(&der)
if err != nil {
return time.Time{}, time.Time{}, err
}
@ -1011,3 +987,164 @@ func ParseCertificates(der []byte) ([]*Certificate, error) {
}
return certs, nil
}
// The X.509 standards confusingly 1-indexed the version names, but 0-indexed
// the actual encoded version, so the version for X.509v2 is 1.
const x509v2Version = 1
// ParseRevocationList parses a X509 v2 Certificate Revocation List from the given
// ASN.1 DER data.
func ParseRevocationList(der []byte) (*RevocationList, error) {
rl := &RevocationList{}
input := cryptobyte.String(der)
// we read the SEQUENCE including length and tag bytes so that
// we can populate RevocationList.Raw, before unwrapping the
// SEQUENCE so it can be operated on
if !input.ReadASN1Element(&input, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed certificate")
}
rl.Raw = input
if !input.ReadASN1(&input, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed certificate")
}
var tbs cryptobyte.String
// do the same trick again as above to extract the raw
// bytes for Certificate.RawTBSCertificate
if !input.ReadASN1Element(&tbs, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed tbs certificate")
}
rl.RawTBSRevocationList = tbs
if !tbs.ReadASN1(&tbs, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed tbs certificate")
}
var version int
if !tbs.PeekASN1Tag(cryptobyte_asn1.INTEGER) {
return nil, errors.New("x509: unsupported crl version")
}
if !tbs.ReadASN1Integer(&version) {
return nil, errors.New("x509: malformed crl")
}
if version != x509v2Version {
return nil, fmt.Errorf("x509: unsupported crl version: %d", version)
}
var sigAISeq cryptobyte.String
if !tbs.ReadASN1(&sigAISeq, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed signature algorithm identifier")
}
// Before parsing the inner algorithm identifier, extract
// the outer algorithm identifier and make sure that they
// match.
var outerSigAISeq cryptobyte.String
if !input.ReadASN1(&outerSigAISeq, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed algorithm identifier")
}
if !bytes.Equal(outerSigAISeq, sigAISeq) {
return nil, errors.New("x509: inner and outer signature algorithm identifiers don't match")
}
sigAI, err := parseAI(sigAISeq)
if err != nil {
return nil, err
}
rl.SignatureAlgorithm = getSignatureAlgorithmFromAI(sigAI)
var signature asn1.BitString
if !input.ReadASN1BitString(&signature) {
return nil, errors.New("x509: malformed signature")
}
rl.Signature = signature.RightAlign()
var issuerSeq cryptobyte.String
if !tbs.ReadASN1Element(&issuerSeq, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed issuer")
}
rl.RawIssuer = issuerSeq
issuerRDNs, err := parseName(issuerSeq)
if err != nil {
return nil, err
}
rl.Issuer.FillFromRDNSequence(issuerRDNs)
rl.ThisUpdate, err = parseTime(&tbs)
if err != nil {
return nil, err
}
if tbs.PeekASN1Tag(cryptobyte_asn1.GeneralizedTime) || tbs.PeekASN1Tag(cryptobyte_asn1.UTCTime) {
rl.NextUpdate, err = parseTime(&tbs)
if err != nil {
return nil, err
}
}
if tbs.PeekASN1Tag(cryptobyte_asn1.SEQUENCE) {
var revokedSeq cryptobyte.String
if !tbs.ReadASN1(&revokedSeq, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed crl")
}
for !revokedSeq.Empty() {
var certSeq cryptobyte.String
if !revokedSeq.ReadASN1(&certSeq, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed crl")
}
rc := pkix.RevokedCertificate{}
rc.SerialNumber = new(big.Int)
if !certSeq.ReadASN1Integer(rc.SerialNumber) {
return nil, errors.New("x509: malformed serial number")
}
rc.RevocationTime, err = parseTime(&certSeq)
if err != nil {
return nil, err
}
var extensions cryptobyte.String
var present bool
if !tbs.ReadOptionalASN1(&extensions, &present, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed extensions")
}
if present {
if !extensions.ReadASN1(&extensions, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed extensions")
}
for !extensions.Empty() {
var extension cryptobyte.String
if !extensions.ReadASN1(&extension, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed extension")
}
ext, err := parseExtension(extension)
if err != nil {
return nil, err
}
rc.Extensions = append(rc.Extensions, ext)
}
}
rl.RevokedCertificates = append(rl.RevokedCertificates, rc)
}
}
var extensions cryptobyte.String
var present bool
if !tbs.ReadOptionalASN1(&extensions, &present, cryptobyte_asn1.Tag(0).Constructed().ContextSpecific()) {
return nil, errors.New("x509: malformed extensions")
}
if present {
if !extensions.ReadASN1(&extensions, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed extensions")
}
for !extensions.Empty() {
var extension cryptobyte.String
if !extensions.ReadASN1(&extension, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed extension")
}
ext, err := parseExtension(extension)
if err != nil {
return nil, err
}
rl.Extensions = append(rl.Extensions, ext)
}
}
return rl, nil
}

View File

@ -296,6 +296,8 @@ func (certList *CertificateList) HasExpired(now time.Time) bool {
// TBSCertificateList represents the ASN.1 structure of the same name. See RFC
// 5280, section 5.1.
//
// Deprecated: x509.RevocationList should be used instead.
type TBSCertificateList struct {
Raw asn1.RawContent
Version int `asn1:"optional,default:0"`
@ -309,6 +311,8 @@ type TBSCertificateList struct {
// RevokedCertificate represents the ASN.1 structure of the same name. See RFC
// 5280, section 5.1.
//
// Deprecated: x509.RevocationList should be used instead.
type RevokedCertificate struct {
SerialNumber *big.Int
RevocationTime time.Time

View File

@ -880,6 +880,8 @@ func checkSignature(algo SignatureAlgorithm, signed, signature []byte, publicKey
}
// CheckCRLSignature checks that the signature in crl is from c.
//
// Deprecated: Use RevocationList.CheckSignatureFrom instead.
func (c *Certificate) CheckCRLSignature(crl *pkix.CertificateList) error {
algo := getSignatureAlgorithmFromAI(crl.SignatureAlgorithm)
return c.CheckSignature(algo, crl.TBSCertList.Raw, crl.SignatureValue.RightAlign())
@ -1607,6 +1609,8 @@ var pemType = "X509 CRL"
// encoded CRLs will appear where they should be DER encoded, so this function
// will transparently handle PEM encoding as long as there isn't any leading
// garbage.
//
// Deprecated: Use ParseRevocationList instead.
func ParseCRL(crlBytes []byte) (*pkix.CertificateList, error) {
if bytes.HasPrefix(crlBytes, pemCRLPrefix) {
block, _ := pem.Decode(crlBytes)
@ -1618,6 +1622,8 @@ func ParseCRL(crlBytes []byte) (*pkix.CertificateList, error) {
}
// ParseDERCRL parses a DER encoded CRL from the given bytes.
//
// Deprecated: Use ParseRevocationList instead.
func ParseDERCRL(derBytes []byte) (*pkix.CertificateList, error) {
certList := new(pkix.CertificateList)
if rest, err := asn1.Unmarshal(derBytes, certList); err != nil {
@ -1631,7 +1637,7 @@ func ParseDERCRL(derBytes []byte) (*pkix.CertificateList, error) {
// CreateCRL returns a DER encoded CRL, signed by this Certificate, that
// contains the given list of revoked certificates.
//
// Note: this method does not generate an RFC 5280 conformant X.509 v2 CRL.
// Deprecated: this method does not generate an RFC 5280 conformant X.509 v2 CRL.
// To generate a standards compliant CRL, use CreateRevocationList instead.
func (c *Certificate) CreateCRL(rand io.Reader, priv any, revokedCerts []pkix.RevokedCertificate, now, expiry time.Time) (crlBytes []byte, err error) {
key, ok := priv.(crypto.Signer)
@ -2073,6 +2079,14 @@ func (c *CertificateRequest) CheckSignature() error {
// RevocationList contains the fields used to create an X.509 v2 Certificate
// Revocation list with CreateRevocationList.
type RevocationList struct {
Raw []byte
RawTBSRevocationList []byte
RawIssuer []byte
Issuer pkix.Name
AuthorityKeyId []byte
Signature []byte
// SignatureAlgorithm is used to determine the signature algorithm to be
// used when signing the CRL. If 0 the default algorithm for the signing
// key will be used.
@ -2087,6 +2101,7 @@ type RevocationList struct {
// which should be a monotonically increasing sequence number for a given
// CRL scope and CRL issuer.
Number *big.Int
// ThisUpdate is used to populate the thisUpdate field in the CRL, which
// indicates the issuance date of the CRL.
ThisUpdate time.Time
@ -2094,6 +2109,11 @@ type RevocationList struct {
// indicates the date by which the next CRL will be issued. NextUpdate
// must be greater than ThisUpdate.
NextUpdate time.Time
// Extensions contains raw X.509 extensions. When creating a CRL,
// the Extensions field is ignored, see ExtraExtensions.
Extensions []pkix.Extension
// ExtraExtensions contains any additional extensions to add directly to
// the CRL.
ExtraExtensions []pkix.Extension
@ -2207,3 +2227,22 @@ func CreateRevocationList(rand io.Reader, template *RevocationList, issuer *Cert
SignatureValue: asn1.BitString{Bytes: signature, BitLength: len(signature) * 8},
})
}
// CheckSignatureFrom verifies that the signature on rl is a valid signature
// from issuer.
func (rl *RevocationList) CheckSignatureFrom(parent *Certificate) error {
if parent.Version == 3 && !parent.BasicConstraintsValid ||
parent.BasicConstraintsValid && !parent.IsCA {
return ConstraintViolationError{}
}
if parent.KeyUsage != 0 && parent.KeyUsage&KeyUsageCRLSign == 0 {
return ConstraintViolationError{}
}
if parent.PublicKeyAlgorithm == UnknownPublicKeyAlgorithm {
return ErrUnsupportedAlgorithm
}
return parent.CheckSignature(rl.SignatureAlgorithm, rl.RawTBSRevocationList, rl.Signature)
}

View File

@ -2631,24 +2631,24 @@ func TestCreateRevocationList(t *testing.T) {
return
}
parsedCRL, err := ParseDERCRL(crl)
parsedCRL, err := ParseRevocationList(crl)
if err != nil {
t.Fatalf("Failed to parse generated CRL: %s", err)
}
if tc.template.SignatureAlgorithm != UnknownSignatureAlgorithm &&
parsedCRL.SignatureAlgorithm.Algorithm.Equal(signatureAlgorithmDetails[tc.template.SignatureAlgorithm].oid) {
parsedCRL.SignatureAlgorithm != tc.template.SignatureAlgorithm {
t.Fatalf("SignatureAlgorithm mismatch: got %v; want %v.", parsedCRL.SignatureAlgorithm,
tc.template.SignatureAlgorithm)
}
if !reflect.DeepEqual(parsedCRL.TBSCertList.RevokedCertificates, tc.template.RevokedCertificates) {
if !reflect.DeepEqual(parsedCRL.RevokedCertificates, tc.template.RevokedCertificates) {
t.Fatalf("RevokedCertificates mismatch: got %v; want %v.",
parsedCRL.TBSCertList.RevokedCertificates, tc.template.RevokedCertificates)
parsedCRL.RevokedCertificates, tc.template.RevokedCertificates)
}
if len(parsedCRL.TBSCertList.Extensions) != 2+len(tc.template.ExtraExtensions) {
t.Fatalf("Generated CRL has wrong number of extensions, wanted: %d, got: %d", 2+len(tc.template.ExtraExtensions), len(parsedCRL.TBSCertList.Extensions))
if len(parsedCRL.Extensions) != 2+len(tc.template.ExtraExtensions) {
t.Fatalf("Generated CRL has wrong number of extensions, wanted: %d, got: %d", 2+len(tc.template.ExtraExtensions), len(parsedCRL.Extensions))
}
expectedAKI, err := asn1.Marshal(authKeyId{Id: tc.issuer.SubjectKeyId})
if err != nil {
@ -2658,9 +2658,9 @@ func TestCreateRevocationList(t *testing.T) {
Id: oidExtensionAuthorityKeyId,
Value: expectedAKI,
}
if !reflect.DeepEqual(parsedCRL.TBSCertList.Extensions[0], akiExt) {
if !reflect.DeepEqual(parsedCRL.Extensions[0], akiExt) {
t.Fatalf("Unexpected first extension: got %v, want %v",
parsedCRL.TBSCertList.Extensions[0], akiExt)
parsedCRL.Extensions[0], akiExt)
}
expectedNum, err := asn1.Marshal(tc.template.Number)
if err != nil {
@ -2670,18 +2670,18 @@ func TestCreateRevocationList(t *testing.T) {
Id: oidExtensionCRLNumber,
Value: expectedNum,
}
if !reflect.DeepEqual(parsedCRL.TBSCertList.Extensions[1], crlExt) {
if !reflect.DeepEqual(parsedCRL.Extensions[1], crlExt) {
t.Fatalf("Unexpected second extension: got %v, want %v",
parsedCRL.TBSCertList.Extensions[1], crlExt)
parsedCRL.Extensions[1], crlExt)
}
if len(parsedCRL.TBSCertList.Extensions[2:]) == 0 && len(tc.template.ExtraExtensions) == 0 {
if len(parsedCRL.Extensions[2:]) == 0 && len(tc.template.ExtraExtensions) == 0 {
// If we don't have anything to check return early so we don't
// hit a [] != nil false positive below.
return
}
if !reflect.DeepEqual(parsedCRL.TBSCertList.Extensions[2:], tc.template.ExtraExtensions) {
if !reflect.DeepEqual(parsedCRL.Extensions[2:], tc.template.ExtraExtensions) {
t.Fatalf("Extensions mismatch: got %v; want %v.",
parsedCRL.TBSCertList.Extensions[2:], tc.template.ExtraExtensions)
parsedCRL.Extensions[2:], tc.template.ExtraExtensions)
}
})
}
@ -3444,3 +3444,125 @@ func TestDisableSHA1ForCertOnly(t *testing.T) {
t.Errorf("unexpected error: %s", err)
}
}
func TestParseRevocationList(t *testing.T) {
derBytes := fromBase64(derCRLBase64)
certList, err := ParseRevocationList(derBytes)
if err != nil {
t.Errorf("error parsing: %s", err)
return
}
numCerts := len(certList.RevokedCertificates)
expected := 88
if numCerts != expected {
t.Errorf("bad number of revoked certificates. got: %d want: %d", numCerts, expected)
}
}
func TestRevocationListCheckSignatureFrom(t *testing.T) {
goodKey, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader)
if err != nil {
t.Fatalf("failed to generate test key: %s", err)
}
badKey, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader)
if err != nil {
t.Fatalf("failed to generate test key: %s", err)
}
tests := []struct {
name string
issuer *Certificate
err string
}{
{
name: "valid",
issuer: &Certificate{
Version: 3,
BasicConstraintsValid: true,
IsCA: true,
PublicKeyAlgorithm: ECDSA,
PublicKey: goodKey.Public(),
},
},
{
name: "valid, key usage set",
issuer: &Certificate{
Version: 3,
BasicConstraintsValid: true,
IsCA: true,
PublicKeyAlgorithm: ECDSA,
PublicKey: goodKey.Public(),
KeyUsage: KeyUsageCRLSign,
},
},
{
name: "invalid issuer, wrong key usage",
issuer: &Certificate{
Version: 3,
BasicConstraintsValid: true,
IsCA: true,
PublicKeyAlgorithm: ECDSA,
PublicKey: goodKey.Public(),
KeyUsage: KeyUsageCertSign,
},
err: "x509: invalid signature: parent certificate cannot sign this kind of certificate",
},
{
name: "invalid issuer, no basic constraints/ca",
issuer: &Certificate{
Version: 3,
PublicKeyAlgorithm: ECDSA,
PublicKey: goodKey.Public(),
},
err: "x509: invalid signature: parent certificate cannot sign this kind of certificate",
},
{
name: "invalid issuer, unsupported public key type",
issuer: &Certificate{
Version: 3,
BasicConstraintsValid: true,
IsCA: true,
PublicKeyAlgorithm: UnknownPublicKeyAlgorithm,
PublicKey: goodKey.Public(),
},
err: "x509: cannot verify signature: algorithm unimplemented",
},
{
name: "wrong key",
issuer: &Certificate{
Version: 3,
BasicConstraintsValid: true,
IsCA: true,
PublicKeyAlgorithm: ECDSA,
PublicKey: badKey.Public(),
},
err: "x509: ECDSA verification failure",
},
}
crlIssuer := &Certificate{
BasicConstraintsValid: true,
IsCA: true,
PublicKeyAlgorithm: ECDSA,
PublicKey: goodKey.Public(),
KeyUsage: KeyUsageCRLSign,
SubjectKeyId: []byte{1, 2, 3},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
crlDER, err := CreateRevocationList(rand.Reader, &RevocationList{Number: big.NewInt(1)}, crlIssuer, goodKey)
if err != nil {
t.Fatalf("failed to generate CRL: %s", err)
}
crl, err := ParseRevocationList(crlDER)
if err != nil {
t.Fatalf("failed to parse test CRL: %s", err)
}
err = crl.CheckSignatureFrom(tc.issuer)
if err != nil && err.Error() != tc.err {
t.Errorf("unexpected error: got %s, want %s", err, tc.err)
} else if err == nil && tc.err != "" {
t.Errorf("CheckSignatureFrom did not fail: want %s", tc.err)
}
})
}
}