crypto/x509: surface ReasonCode in RevocationList API

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 <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Roland Shoemaker <[email protected]> Reviewed-by: Emmanuel Odeke <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]>
cloudflare · Mar 13, 2023 · 82c713f · 82c713f
1 parent 7c019c6
commit 82c713f
Show file tree

Hide file tree

Showing 4 changed files with 228 additions and 29 deletions.
diff --git a/api/next/53573.txt b/api/next/53573.txt
@@ -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
diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go
@@ -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)
  }
  }
 

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
@@ -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 {

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
@@ -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)
  }
 }