]> Cypherpunks repositories - gostls13.git/commitdiff
crypto/x509: add new CRL parser, deprecate old one
authorRoland Shoemaker <roland@golang.org>
Tue, 8 Mar 2022 19:18:44 +0000 (11:18 -0800)
committerGopher Robot <gobot@golang.org>
Tue, 5 Apr 2022 23:32:55 +0000 (23:32 +0000)
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>

api/next/50674.txt [new file with mode: 0644]
src/crypto/x509/parser.go
src/crypto/x509/pkix/pkix.go
src/crypto/x509/x509.go
src/crypto/x509/x509_test.go

diff --git a/api/next/50674.txt b/api/next/50674.txt
new file mode 100644 (file)
index 0000000..6b5bca3
--- /dev/null
@@ -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
index 333991bf14d97d67c6f5ed6d466defd89ca160b1..9dd7c2564e12b7cf37085263a401e1e9145d6afa 100644 (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
+}
index e9179ed0679be5dfc8570e04e3de66f6955d99a2..da57b66831b87c1a591770e7d325393a9dc2c100 100644 (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
index cb43079a9c75e69e8f0278270658ed6611f11ffd..41d85c458d04730ca03a1dbba0831bbe4cfb3437 100644 (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)
+}
index d2889fc1d7bd2f368d63592bb20b21bf5ec5a8b4..b26ace89e72e2f2c501832a87a07885f65e00b1f 100644 (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)
+                       }
+               })
+       }
+}