From 3d4dd002a42266b046dd894a868135db08fe7258 Mon Sep 17 00:00:00 2001 From: Sun Yimin Date: Thu, 27 Feb 2025 10:22:19 +0800 Subject: [PATCH] smx509: add new CRL parser, deprecate old one #40 --- smx509/parser.go | 232 +++++++++++++++++++++++++++++++++- smx509/x509.go | 155 +++++++++++++---------- smx509/x509_test.go | 298 +++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 600 insertions(+), 85 deletions(-) diff --git a/smx509/parser.go b/smx509/parser.go index 42d4f8d..0e9e9f0 100644 --- a/smx509/parser.go +++ b/smx509/parser.go @@ -7,6 +7,7 @@ import ( "crypto/ed25519" "crypto/elliptic" "crypto/rsa" + "crypto/x509" "crypto/x509/pkix" "encoding/asn1" "encoding/pem" @@ -162,6 +163,23 @@ func parseAI(der cryptobyte.String) (pkix.AlgorithmIdentifier, error) { return ai, nil } +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") + } + 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 +} + func parseValidity(der cryptobyte.String) (time.Time, time.Time, error) { extract := func() (time.Time, error) { var t time.Time @@ -457,6 +475,26 @@ func parseSANExtension(der cryptobyte.String) (dnsNames, emailAddresses []string return } +func parseAuthorityKeyIdentifier(e pkix.Extension) ([]byte, error) { + // RFC 5280, Section 4.2.1.1 + if e.Critical { + // Conforming CAs MUST mark this extension as non-critical + return nil, errors.New("x509: authority key identifier incorrectly marked critical") + } + val := cryptobyte.String(e.Value) + var akid cryptobyte.String + if !val.ReadASN1(&akid, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: invalid authority key identifier") + } + if akid.PeekASN1Tag(cryptobyte_asn1.Tag(0).ContextSpecific()) { + if !akid.ReadASN1(&akid, cryptobyte_asn1.Tag(0).ContextSpecific()) { + return nil, errors.New("x509: invalid authority key identifier") + } + return akid, nil + } + return nil, nil +} + func parseExtKeyUsageExtension(der cryptobyte.String) ([]ExtKeyUsage, []asn1.ObjectIdentifier, error) { var extKeyUsages []ExtKeyUsage var unknownUsages []asn1.ObjectIdentifier @@ -768,7 +806,7 @@ func processExtensions(out *Certificate) error { if e.Critical { // Conforming CAs MUST mark this extension as non-critical return errors.New("x509: authority key identifier incorrectly marked critical") - } + } val := cryptobyte.String(e.Value) var akid cryptobyte.String if !val.ReadASN1(&akid, cryptobyte_asn1.SEQUENCE) { @@ -790,7 +828,7 @@ func processExtensions(out *Certificate) error { if e.Critical { // Conforming CAs MUST mark this extension as non-critical return errors.New("x509: subject key identifier incorrectly marked critical") - } + } val := cryptobyte.String(e.Value) var skid cryptobyte.String if !val.ReadASN1(&skid, cryptobyte_asn1.OCTET_STRING) { @@ -811,7 +849,7 @@ func processExtensions(out *Certificate) error { if e.Critical { // Conforming CAs MUST mark this extension as non-critical return errors.New("x509: authority info access incorrectly marked critical") - } + } val := cryptobyte.String(e.Value) if !val.ReadASN1(&val, cryptobyte_asn1.SEQUENCE) { return errors.New("x509: invalid authority info access") @@ -1068,3 +1106,191 @@ func ParseCertificatePEM(data []byte) (*Certificate, error) { } return ParseCertificate(block.Bytes) } + +// 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 crl") + } + rl.Raw = input + if !input.ReadASN1(&input, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed crl") + } + + 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 crl") + } + rl.RawTBSRevocationList = tbs + if !tbs.ReadASN1(&tbs, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed tbs crl") + } + + 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() { + rce := x509.RevocationListEntry{} + + var certSeq cryptobyte.String + if !revokedSeq.ReadASN1Element(&certSeq, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed crl") + } + rce.Raw = certSeq + if !certSeq.ReadASN1(&certSeq, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed crl") + } + + rce.SerialNumber = new(big.Int) + if !certSeq.ReadASN1Integer(rce.SerialNumber) { + return nil, errors.New("x509: malformed serial number") + } + rce.RevocationTime, err = parseTime(&certSeq) + if err != nil { + return nil, err + } + var extensions cryptobyte.String + var present bool + if !certSeq.ReadOptionalASN1(&extensions, &present, cryptobyte_asn1.SEQUENCE) { + return nil, errors.New("x509: malformed extensions") + } + if present { + 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 + } + if ext.Id.Equal(oidExtensionReasonCode) { + val := cryptobyte.String(ext.Value) + if !val.ReadASN1Enum(&rce.ReasonCode) { + return nil, fmt.Errorf("x509: malformed reasonCode extension") + } + } + rce.Extensions = append(rce.Extensions, ext) + } + } + + rl.RevokedCertificateEntries = append(rl.RevokedCertificateEntries, rce) + rcDeprecated := pkix.RevokedCertificate{ + SerialNumber: rce.SerialNumber, + RevocationTime: rce.RevocationTime, + Extensions: rce.Extensions, + } + rl.RevokedCertificates = append(rl.RevokedCertificates, rcDeprecated) + } + } + + 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 + } + if ext.Id.Equal(oidExtensionAuthorityKeyId) { + rl.AuthorityKeyId, err = parseAuthorityKeyIdentifier(ext) + if err != nil { + return nil, err + } + } else if ext.Id.Equal(oidExtensionCRLNumber) { + value := cryptobyte.String(ext.Value) + rl.Number = new(big.Int) + if !value.ReadASN1Integer(rl.Number) { + return nil, errors.New("x509: malformed crl number") + } + } + rl.Extensions = append(rl.Extensions, ext) + } + } + + return rl, nil +} diff --git a/smx509/x509.go b/smx509/x509.go index d490a4b..cc09bbe 100644 --- a/smx509/x509.go +++ b/smx509/x509.go @@ -885,6 +885,7 @@ 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()) @@ -1641,11 +1642,15 @@ func toCertificate(in any) (*x509.Certificate, error) { // 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) { return x509.ParseCRL(crlBytes) } // ParseDERCRL parses a DER encoded CRL from the given bytes. +// +// Deprecated: Use [ParseRevocationList] instead. func ParseDERCRL(derBytes []byte) (*pkix.CertificateList, error) { return x509.ParseDERCRL(derBytes) } @@ -1653,8 +1658,8 @@ 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. -// To generate a standards compliant CRL, use CreateRevocationList instead. +// 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) if !ok { @@ -2064,6 +2069,16 @@ func (c *CertificateRequest) CheckSignature() error { return checkSignature(c.SignatureAlgorithm, c.RawTBSCertificateRequest, c.Signature, c.PublicKey, true) } +type RevocationList x509.RevocationList + +func (c *RevocationList) asX509() *x509.RevocationList { + return (*x509.RevocationList)(c) +} + +func (c *RevocationList) ToX509() *x509.RevocationList { + return c.asX509() +} + // These structures reflect the ASN.1 structure of X.509 CRLs better than // the existing crypto/x509/pkix variants do. These mirror the existing // certificate structs in this file. @@ -2125,69 +2140,62 @@ func CreateRevocationList(rand io.Reader, template *x509.RevocationList, issuer } var revokedCerts []pkix.RevokedCertificate - revokedCerts = make([]pkix.RevokedCertificate, len(template.RevokedCertificates)) - for i, rc := range template.RevokedCertificates { - rc.RevocationTime = rc.RevocationTime.UTC() - revokedCerts[i] = rc - } - /* - // Only process the deprecated RevokedCertificates field if it is populated - // and the new RevokedCertificateEntries field is not populated. - if len(template.RevokedCertificates) > 0 && len(template.RevokedCertificateEntries) == 0 { - // Force revocation times to UTC per RFC 5280. - revokedCerts = make([]pkix.RevokedCertificate, len(template.RevokedCertificates)) - for i, rc := range template.RevokedCertificates { - rc.RevocationTime = rc.RevocationTime.UTC() - revokedCerts[i] = rc - } - } else { - // Convert the ReasonCode field to a proper extension, and force revocation - // times to UTC per RFC 5280. - revokedCerts = make([]pkix.RevokedCertificate, len(template.RevokedCertificateEntries)) - for i, rce := range template.RevokedCertificateEntries { - if rce.SerialNumber == nil { - return nil, errors.New("x509: template contains entry with nil SerialNumber field") - } - if rce.RevocationTime.IsZero() { - return nil, errors.New("x509: template contains entry with zero RevocationTime field") - } - - rc := pkix.RevokedCertificate{ - SerialNumber: rce.SerialNumber, - RevocationTime: rce.RevocationTime.UTC(), - } - - // Copy over any extra extensions, except for a Reason Code extension, - // because we'll synthesize that ourselves to ensure it is correct. - exts := make([]pkix.Extension, 0, len(rce.ExtraExtensions)) - for _, ext := range rce.ExtraExtensions { - if ext.Id.Equal(oidExtensionReasonCode) { - return nil, errors.New("x509: template contains entry with ReasonCode ExtraExtension; use ReasonCode field instead") - } - exts = append(exts, ext) - } - - // Only add a reasonCode extension if the reason is non-zero, as per - // RFC 5280 Section 5.3.1. - if rce.ReasonCode != 0 { - reasonBytes, err := asn1.Marshal(asn1.Enumerated(rce.ReasonCode)) - if err != nil { - return nil, err - } - - exts = append(exts, pkix.Extension{ - Id: oidExtensionReasonCode, - Value: reasonBytes, - }) - } - - if len(exts) > 0 { - rc.Extensions = exts - } - revokedCerts[i] = rc - } + // Only process the deprecated RevokedCertificates field if it is populated + // and the new RevokedCertificateEntries field is not populated. + if len(template.RevokedCertificates) > 0 && len(template.RevokedCertificateEntries) == 0 { + // Force revocation times to UTC per RFC 5280. + revokedCerts = make([]pkix.RevokedCertificate, len(template.RevokedCertificates)) + for i, rc := range template.RevokedCertificates { + rc.RevocationTime = rc.RevocationTime.UTC() + revokedCerts[i] = rc } - */ + } else { + // Convert the ReasonCode field to a proper extension, and force revocation + // times to UTC per RFC 5280. + revokedCerts = make([]pkix.RevokedCertificate, len(template.RevokedCertificateEntries)) + for i, rce := range template.RevokedCertificateEntries { + if rce.SerialNumber == nil { + return nil, errors.New("x509: template contains entry with nil SerialNumber field") + } + if rce.RevocationTime.IsZero() { + return nil, errors.New("x509: template contains entry with zero RevocationTime field") + } + + rc := pkix.RevokedCertificate{ + SerialNumber: rce.SerialNumber, + RevocationTime: rce.RevocationTime.UTC(), + } + + // Copy over any extra extensions, except for a Reason Code extension, + // because we'll synthesize that ourselves to ensure it is correct. + exts := make([]pkix.Extension, 0, len(rce.ExtraExtensions)) + for _, ext := range rce.ExtraExtensions { + if ext.Id.Equal(oidExtensionReasonCode) { + return nil, errors.New("x509: template contains entry with ReasonCode ExtraExtension; use ReasonCode field instead") + } + exts = append(exts, ext) + } + + // Only add a reasonCode extension if the reason is non-zero, as per + // RFC 5280 Section 5.3.1. + if rce.ReasonCode != 0 { + reasonBytes, err := asn1.Marshal(asn1.Enumerated(rce.ReasonCode)) + if err != nil { + return nil, err + } + + exts = append(exts, pkix.Extension{ + Id: oidExtensionReasonCode, + Value: reasonBytes, + }) + } + + if len(exts) > 0 { + rc.Extensions = exts + } + revokedCerts[i] = rc + } + } aki, err := asn1.Marshal(authKeyId{Id: issuer.SubjectKeyId}) if err != nil { @@ -2252,3 +2260,22 @@ func CreateRevocationList(rand io.Reader, template *x509.RevocationList, issuer 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 x509.ConstraintViolationError{} + } + + if parent.KeyUsage != 0 && parent.KeyUsage&KeyUsageCRLSign == 0 { + return x509.ConstraintViolationError{} + } + + if parent.PublicKeyAlgorithm == UnknownPublicKeyAlgorithm { + return x509.ErrUnsupportedAlgorithm + } + + return parent.CheckSignature(rl.SignatureAlgorithm, rl.RawTBSRevocationList, rl.Signature) +} diff --git a/smx509/x509_test.go b/smx509/x509_test.go index 444dde0..db12552 100644 --- a/smx509/x509_test.go +++ b/smx509/x509_test.go @@ -2414,6 +2414,57 @@ func TestCreateRevocationList(t *testing.T) { NextUpdate: time.Time{}.Add(time.Hour * 48), }, }, + { + name: "valid, reason code", + key: sm2Priv, + issuer: &x509.Certificate{ + KeyUsage: KeyUsageCRLSign, + Subject: pkix.Name{ + CommonName: "testing", + }, + SubjectKeyId: []byte{1, 2, 3}, + }, + template: &x509.RevocationList{ + RevokedCertificateEntries: []x509.RevocationListEntry{ + { + SerialNumber: big.NewInt(2), + RevocationTime: time.Time{}.Add(time.Hour), + ReasonCode: 1, + }, + }, + Number: big.NewInt(5), + ThisUpdate: time.Time{}.Add(time.Hour * 24), + NextUpdate: time.Time{}.Add(time.Hour * 48), + }, + }, + { + name: "valid, extra entry extension", + key: sm2Priv, + issuer: &x509.Certificate{ + KeyUsage: KeyUsageCRLSign, + Subject: pkix.Name{ + CommonName: "testing", + }, + SubjectKeyId: []byte{1, 2, 3}, + }, + template: &x509.RevocationList{ + RevokedCertificateEntries: []x509.RevocationListEntry{ + { + SerialNumber: big.NewInt(2), + RevocationTime: time.Time{}.Add(time.Hour), + ExtraExtensions: []pkix.Extension{ + { + Id: []int{2, 5, 29, 99}, + Value: []byte{5, 0}, + }, + }, + }, + }, + Number: big.NewInt(5), + ThisUpdate: time.Time{}.Add(time.Hour * 24), + NextUpdate: time.Time{}.Add(time.Hour * 48), + }, + }, { name: "valid, Ed25519 key", key: ed25519Priv, @@ -2487,6 +2538,34 @@ func TestCreateRevocationList(t *testing.T) { }, }, }, + { + name: "valid, deprecated entries with extension", + key: sm2Priv, + issuer: &x509.Certificate{ + KeyUsage: KeyUsageCRLSign, + Subject: pkix.Name{ + CommonName: "testing", + }, + SubjectKeyId: []byte{1, 2, 3}, + }, + template: &x509.RevocationList{ + RevokedCertificates: []pkix.RevokedCertificate{ + { + SerialNumber: big.NewInt(2), + RevocationTime: time.Time{}.Add(time.Hour), + Extensions: []pkix.Extension{ + { + Id: []int{2, 5, 29, 99}, + Value: []byte{5, 0}, + }, + }, + }, + }, + Number: big.NewInt(5), + ThisUpdate: time.Time{}.Add(time.Hour * 24), + NextUpdate: time.Time{}.Add(time.Hour * 48), + }, + }, { name: "valid, empty list", key: sm2Priv, @@ -2533,24 +2612,45 @@ 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) { - t.Fatalf("RevokedCertificates mismatch: got %v; want %v.", - parsedCRL.TBSCertList.RevokedCertificates, tc.template.RevokedCertificates) + if len(tc.template.RevokedCertificates) > 0 { + if !reflect.DeepEqual(parsedCRL.RevokedCertificates, tc.template.RevokedCertificates) { + t.Fatalf("RevokedCertificates mismatch: got %v; want %v.", + parsedCRL.RevokedCertificates, tc.template.RevokedCertificates) + } + } else { + if len(parsedCRL.RevokedCertificateEntries) != len(tc.template.RevokedCertificateEntries) { + t.Fatalf("RevokedCertificateEntries length mismatch: got %d; want %d.", + len(parsedCRL.RevokedCertificateEntries), + len(tc.template.RevokedCertificateEntries)) + } + for i, rce := range parsedCRL.RevokedCertificateEntries { + expected := tc.template.RevokedCertificateEntries[i] + if rce.SerialNumber.Cmp(expected.SerialNumber) != 0 { + t.Fatalf("RevocationListEntry serial mismatch: got %d; want %d.", + rce.SerialNumber, expected.SerialNumber) + } + if !rce.RevocationTime.Equal(expected.RevocationTime) { + t.Fatalf("RevocationListEntry revocation time mismatch: got %v; want %v.", + rce.RevocationTime, expected.RevocationTime) + } + if rce.ReasonCode != expected.ReasonCode { + t.Fatalf("RevocationListEntry reason code mismatch: got %d; want %d.", + rce.ReasonCode, expected.ReasonCode) + } + } } - - 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 { @@ -2560,9 +2660,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 { @@ -2572,18 +2672,57 @@ 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 { + // With Go 1.19's updated RevocationList, we can now directly compare + // the RawSubject of the certificate to RawIssuer on the parsed CRL. + // However, this doesn't work with our hacked issuers above (that + // aren't parsed from a proper DER bundle but are instead manually + // constructed). Prefer RawSubject when it is set. + if len(tc.issuer.RawSubject) > 0 { + issuerSubj, err := subjectBytes(tc.issuer) + if err != nil { + t.Fatalf("failed to get issuer subject: %s", err) + } + if !bytes.Equal(issuerSubj, parsedCRL.RawIssuer) { + t.Fatalf("Unexpected issuer subject; wanted: %v, got: %v", hex.EncodeToString(issuerSubj), hex.EncodeToString(parsedCRL.RawIssuer)) + } + } else { + // When we hack our custom Subject in the test cases above, + // we don't set the additional fields (such as Names) in the + // hacked issuer. Round-trip a parsing of pkix.Name so that + // we add these missing fields for the comparison. + issuerRDN := tc.issuer.Subject.ToRDNSequence() + var caIssuer pkix.Name + caIssuer.FillFromRDNSequence(&issuerRDN) + if !reflect.DeepEqual(caIssuer, parsedCRL.Issuer) { + t.Fatalf("Expected issuer.Subject, parsedCRL.Issuer to be the same; wanted: %#v, got: %#v", caIssuer, parsedCRL.Issuer) + } + } + + 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) + } + + if tc.template.Number != nil && parsedCRL.Number == nil { + t.Fatalf("Generated CRL missing Number: got nil, want %s", + tc.template.Number.String()) + } + if tc.template.Number != nil && tc.template.Number.Cmp(parsedCRL.Number) != 0 { + t.Fatalf("Generated CRL has wrong Number: got %s, want %s", + parsedCRL.Number.String(), tc.template.Number.String()) + } + if !bytes.Equal(parsedCRL.AuthorityKeyId, tc.issuer.SubjectKeyId) { + t.Fatalf("Generated CRL has wrong AuthorityKeyId: got %x, want %x", + parsedCRL.AuthorityKeyId, tc.issuer.SubjectKeyId) } }) } @@ -3150,12 +3289,12 @@ func TestDisableSHA1ForCertOnly(t *testing.T) { t.Fatalf("failed to generate test CRL: %s", err) } // TODO(rolandshoemaker): this should be ParseRevocationList once it lands - crl, err := ParseCRL(crlDER) + crl, err := ParseRevocationList(crlDER) if err != nil { t.Fatalf("failed to parse test CRL: %s", err) } - if err = cert.CheckCRLSignature(crl); err != nil { + if err = crl.CheckSignatureFrom(cert); err != nil { t.Errorf("unexpected error: %s", err) } @@ -3174,6 +3313,129 @@ func TestDisableSHA1ForCertOnly(t *testing.T) { } } +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.RevokedCertificateEntries) + numCertsDeprecated := len(certList.RevokedCertificateEntries) + expected := 88 + if numCerts != expected || numCertsDeprecated != expected { + t.Errorf("bad number of revoked certificates. got: %d want: %d", numCerts, expected) + } +} + +func TestRevocationListCheckSignatureFrom(t *testing.T) { + goodKey, err := sm2.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("failed to generate test key: %s", err) + } + badKey, err := sm2.GenerateKey(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: SM2 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, &x509.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) + } + }) + } +} + func TestOmitEmptyExtensions(t *testing.T) { k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil {