]> Cypherpunks.ru repositories - gostls13.git/commitdiff
crypto/x509: surface ReasonCode in RevocationList API
authorAaron Gable <aaron@letsencrypt.org>
Wed, 15 Feb 2023 22:25:34 +0000 (14:25 -0800)
committerRoland Shoemaker <roland@golang.org>
Mon, 13 Mar 2023 20:25:37 +0000 (20:25 +0000)
Creates x509.RevocationListEntry, a new type representing a single
revoked certificate entry in a CRL. Like the existing Certificate and
RevocationList types, this new type has a field for its Raw bytes, and
exposes its mostly-commonly-used extension (ReasonCode) as a top-level
field. This provides more functionality to the user than the existing
pkix.RevokedCertificate type.

Adds a RevokedCertificateEntries field which is a []RevocationListEntry
to RevocationList. This field deprecates the RevokedCertificates field.
When the RevokedCertificates field is removed in a future release, this
will remove one of the last places where a pkix type is directly exposed
in the x509 package API.

Updates the ParseRevocationList function to populate both fields for
now, and updates the CreateRevocationList function to prefer the new
field if it is populated, but use the deprecated field if not. Finally,
also updates the x509 unit tests to use the new .ReasonCode field in
most cases.

Fixes #53573

Change-Id: Ia6de171802a5bd251938366508532e806772d7d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/468875
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
api/next/53573.txt [new file with mode: 0644]
src/crypto/x509/parser.go
src/crypto/x509/x509.go
src/crypto/x509/x509_test.go

diff --git a/api/next/53573.txt b/api/next/53573.txt
new file mode 100644 (file)
index 0000000..a371921
--- /dev/null
@@ -0,0 +1,9 @@
+pkg crypto/x509, type RevocationList struct, RevokedCertificateEntries []RevocationListEntry #53573
+pkg crypto/x509, type RevocationList struct, RevokedCertificates //deprecated #53573
+pkg crypto/x509, type RevocationListEntry struct #53573
+pkg crypto/x509, type RevocationListEntry struct, Extensions []pkix.Extension #53573
+pkg crypto/x509, type RevocationListEntry struct, ExtraExtensions []pkix.Extension #53573
+pkg crypto/x509, type RevocationListEntry struct, Raw []uint8 #53573
+pkg crypto/x509, type RevocationListEntry struct, ReasonCode int #53573
+pkg crypto/x509, type RevocationListEntry struct, RevocationTime time.Time #53573
+pkg crypto/x509, type RevocationListEntry struct, SerialNumber *big.Int #53573
index 6ea30178d95dbf976a75d38c30bd289c32c41a6a..735a27607ea71df43b733c6e032de04f7869e64d 100644 (file)
@@ -1104,16 +1104,22 @@ func ParseRevocationList(der []byte) (*RevocationList, error) {
                        return nil, errors.New("x509: malformed crl")
                }
                for !revokedSeq.Empty() {
+                       rce := RevocationListEntry{}
+
                        var certSeq cryptobyte.String
-                       if !revokedSeq.ReadASN1(&certSeq, cryptobyte_asn1.SEQUENCE) {
+                       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")
                        }
-                       rc := pkix.RevokedCertificate{}
-                       rc.SerialNumber = new(big.Int)
-                       if !certSeq.ReadASN1Integer(rc.SerialNumber) {
+
+                       rce.SerialNumber = new(big.Int)
+                       if !certSeq.ReadASN1Integer(rce.SerialNumber) {
                                return nil, errors.New("x509: malformed serial number")
                        }
-                       rc.RevocationTime, err = parseTime(&certSeq)
+                       rce.RevocationTime, err = parseTime(&certSeq)
                        if err != nil {
                                return nil, err
                        }
@@ -1132,11 +1138,23 @@ func ParseRevocationList(der []byte) (*RevocationList, error) {
                                        if err != nil {
                                                return nil, err
                                        }
-                                       rc.Extensions = append(rc.Extensions, ext)
+                                       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.RevokedCertificates = append(rl.RevokedCertificates, rc)
+                       rl.RevokedCertificateEntries = append(rl.RevokedCertificateEntries, rce)
+                       rcDeprecated := pkix.RevokedCertificate{
+                               SerialNumber:   rce.SerialNumber,
+                               RevocationTime: rce.RevocationTime,
+                               Extensions:     rce.Extensions,
+                       }
+                       rl.RevokedCertificates = append(rl.RevokedCertificates, rcDeprecated)
                }
        }
 
index a4713beb1c471e97641dd141d16ee948a643753c..9d80b1d8ba3486a3255c5501c6a6be3137c2b3c1 100644 (file)
@@ -1028,6 +1028,7 @@ var (
        oidExtensionCRLDistributionPoints = []int{2, 5, 29, 31}
        oidExtensionAuthorityInfoAccess   = []int{1, 3, 6, 1, 5, 5, 7, 1, 1}
        oidExtensionCRLNumber             = []int{2, 5, 29, 20}
+       oidExtensionReasonCode            = []int{2, 5, 29, 21}
 )
 
 var (
@@ -2154,8 +2155,45 @@ func (c *CertificateRequest) CheckSignature() error {
        return checkSignature(c.SignatureAlgorithm, c.RawTBSCertificateRequest, c.Signature, c.PublicKey, true)
 }
 
-// RevocationList contains the fields used to create an X.509 v2 Certificate
-// Revocation list with CreateRevocationList.
+// RevocationListEntry represents an entry in the revokedCertificates
+// sequence of a CRL.
+type RevocationListEntry struct {
+       // Raw contains the raw bytes of the revokedCertificates entry. It is set when
+       // parsing a CRL; it is ignored when generating a CRL.
+       Raw []byte
+
+       // SerialNumber represents the serial number of a revoked certificate. It is
+       // both used when creating a CRL and populated when parsing a CRL. It must not
+       // be nil.
+       SerialNumber *big.Int
+       // RevocationTime represents the time at which the certificate was revoked. It
+       // is both used when creating a CRL and populated when parsing a CRL. It must
+       // not be the zero time.
+       RevocationTime time.Time
+       // ReasonCode represents the reason for revocation, using the integer enum
+       // values specified in RFC 5280 Section 5.3.1. When creating a CRL, the zero
+       // value will result in the reasonCode extension being omitted. When parsing a
+       // CRL, the zero value may represent either the reasonCode extension being
+       // absent (which implies the default revocation reason of 0/Unspecified), or
+       // it may represent the reasonCode extension being present and explicitly
+       // containing a value of 0/Unspecified (which should not happen according to
+       // the DER encoding rules, but can and does happen anyway).
+       ReasonCode int
+
+       // Extensions contains raw X.509 extensions. When parsing CRL entries,
+       // this can be used to extract non-critical extensions that are not
+       // parsed by this package. When marshaling CRL entries, the Extensions
+       // field is ignored, see ExtraExtensions.
+       Extensions []pkix.Extension
+       // ExtraExtensions contains extensions to be copied, raw, into any
+       // marshaled CRL entries. Values override any extensions that would
+       // otherwise be produced based on the other fields. The ExtraExtensions
+       // field is not populated when parsing CRL entries, see Extensions.
+       ExtraExtensions []pkix.Extension
+}
+
+// RevocationList represents a Certificate Revocation List (CRL) as specified
+// by RFC 5280.
 type RevocationList struct {
        // Raw contains the complete ASN.1 DER content of the CRL (tbsCertList,
        // signatureAlgorithm, and signatureValue.)
@@ -2180,9 +2218,17 @@ type RevocationList struct {
        // key will be used.
        SignatureAlgorithm SignatureAlgorithm
 
+       // RevokedCertificateEntries represents the revokedCertificates sequence in
+       // the CRL. It is used when creating a CRL and also populated when parsing a
+       // CRL. When creating a CRL, it may be empty or nil, in which case the
+       // revokedCertificates ASN.1 sequence will be omitted from the CRL entirely.
+       RevokedCertificateEntries []RevocationListEntry
+
        // RevokedCertificates is used to populate the revokedCertificates
-       // sequence in the CRL, it may be empty. RevokedCertificates may be nil,
-       // in which case an empty CRL will be created.
+       // sequence in the CRL if RevokedCertificateEntries is empty. It may be empty
+       // or nil, in which case an empty CRL will be created.
+       //
+       // Deprecated: Use RevokedCertificateEntries instead.
        RevokedCertificates []pkix.RevokedCertificate
 
        // Number is used to populate the X.509 v2 cRLNumber extension in the CRL,
@@ -2268,11 +2314,62 @@ func CreateRevocationList(rand io.Reader, template *RevocationList, issuer *Cert
                return nil, err
        }
 
-       // Force revocation times to UTC per RFC 5280.
-       revokedCertsUTC := make([]pkix.RevokedCertificate, len(template.RevokedCertificates))
-       for i, rc := range template.RevokedCertificates {
-               rc.RevocationTime = rc.RevocationTime.UTC()
-               revokedCertsUTC[i] = rc
+       var revokedCerts []pkix.RevokedCertificate
+       // 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})
@@ -2311,8 +2408,8 @@ func CreateRevocationList(rand io.Reader, template *RevocationList, issuer *Cert
                        },
                },
        }
-       if len(revokedCertsUTC) > 0 {
-               tbsCertList.RevokedCertificates = revokedCertsUTC
+       if len(revokedCerts) > 0 {
+               tbsCertList.RevokedCertificates = revokedCerts
        }
 
        if len(template.ExtraExtensions) > 0 {
index 8846b003123274fe276db25dfca85b7077bba37c..32298a2b0c5ecbcc9d146a13ce9a40423c2acac9 100644 (file)
@@ -2586,7 +2586,7 @@ func TestCreateRevocationList(t *testing.T) {
                                SubjectKeyId: []byte{1, 2, 3},
                        },
                        template: &RevocationList{
-                               RevokedCertificates: []pkix.RevokedCertificate{
+                               RevokedCertificateEntries: []RevocationListEntry{
                                        {
                                                SerialNumber:   big.NewInt(2),
                                                RevocationTime: time.Time{}.Add(time.Hour),
@@ -2597,6 +2597,29 @@ func TestCreateRevocationList(t *testing.T) {
                                NextUpdate: time.Time{}.Add(time.Hour * 48),
                        },
                },
+               {
+                       name: "valid, reason code",
+                       key:  ec256Priv,
+                       issuer: &Certificate{
+                               KeyUsage: KeyUsageCRLSign,
+                               Subject: pkix.Name{
+                                       CommonName: "testing",
+                               },
+                               SubjectKeyId: []byte{1, 2, 3},
+                       },
+                       template: &RevocationList{
+                               RevokedCertificateEntries: []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:  ec256Priv,
@@ -2608,11 +2631,11 @@ func TestCreateRevocationList(t *testing.T) {
                                SubjectKeyId: []byte{1, 2, 3},
                        },
                        template: &RevocationList{
-                               RevokedCertificates: []pkix.RevokedCertificate{
+                               RevokedCertificateEntries: []RevocationListEntry{
                                        {
                                                SerialNumber:   big.NewInt(2),
                                                RevocationTime: time.Time{}.Add(time.Hour),
-                                               Extensions: []pkix.Extension{
+                                               ExtraExtensions: []pkix.Extension{
                                                        {
                                                                Id:    []int{2, 5, 29, 99},
                                                                Value: []byte{5, 0},
@@ -2636,7 +2659,7 @@ func TestCreateRevocationList(t *testing.T) {
                                SubjectKeyId: []byte{1, 2, 3},
                        },
                        template: &RevocationList{
-                               RevokedCertificates: []pkix.RevokedCertificate{
+                               RevokedCertificateEntries: []RevocationListEntry{
                                        {
                                                SerialNumber:   big.NewInt(2),
                                                RevocationTime: time.Time{}.Add(time.Hour),
@@ -2659,7 +2682,7 @@ func TestCreateRevocationList(t *testing.T) {
                        },
                        template: &RevocationList{
                                SignatureAlgorithm: ECDSAWithSHA512,
-                               RevokedCertificates: []pkix.RevokedCertificate{
+                               RevokedCertificateEntries: []RevocationListEntry{
                                        {
                                                SerialNumber:   big.NewInt(2),
                                                RevocationTime: time.Time{}.Add(time.Hour),
@@ -2681,7 +2704,7 @@ func TestCreateRevocationList(t *testing.T) {
                                SubjectKeyId: []byte{1, 2, 3},
                        },
                        template: &RevocationList{
-                               RevokedCertificates: []pkix.RevokedCertificate{
+                               RevokedCertificateEntries: []RevocationListEntry{
                                        {
                                                SerialNumber:   big.NewInt(2),
                                                RevocationTime: time.Time{}.Add(time.Hour),
@@ -2698,6 +2721,34 @@ func TestCreateRevocationList(t *testing.T) {
                                },
                        },
                },
+               {
+                       name: "valid, deprecated entries with extension",
+                       key:  ec256Priv,
+                       issuer: &Certificate{
+                               KeyUsage: KeyUsageCRLSign,
+                               Subject: pkix.Name{
+                                       CommonName: "testing",
+                               },
+                               SubjectKeyId: []byte{1, 2, 3},
+                       },
+                       template: &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:  ec256Priv,
@@ -2751,9 +2802,32 @@ func TestCreateRevocationList(t *testing.T) {
                                        tc.template.SignatureAlgorithm)
                        }
 
-                       if !reflect.DeepEqual(parsedCRL.RevokedCertificates, tc.template.RevokedCertificates) {
-                               t.Fatalf("RevokedCertificates mismatch: got %v; want %v.",
-                                       parsedCRL.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.Extensions) != 2+len(tc.template.ExtraExtensions) {
@@ -3599,9 +3673,10 @@ func TestParseRevocationList(t *testing.T) {
                t.Errorf("error parsing: %s", err)
                return
        }
-       numCerts := len(certList.RevokedCertificates)
+       numCerts := len(certList.RevokedCertificateEntries)
+       numCertsDeprecated := len(certList.RevokedCertificateEntries)
        expected := 88
-       if numCerts != expected {
+       if numCerts != expected || numCertsDeprecated != expected {
                t.Errorf("bad number of revoked certificates. got: %d want: %d", numCerts, expected)
        }
 }