crypto/x509: populate Number and AKI of parsed CRLs

The x509.RevocationList type has two fields which correspond to
extensions, rather than native fields, of the underlying ASN.1 CRL:
the .Number field corresponds to the crlNumber extension, and
the .AuthorityKeyId field corresponds to the authorityKeyIdentifier
extension.

The x509.CreateRevocationList() function uses these fields to populate
their respective extensions in the resulting CRL. However, the
x509.ParseRevocationList() function does not perform the reverse
operation: the fields retain their zero-values even after parsing a CRL
which contains the relevant extensions.

Add code which populates these fields when parsing their extensions.
Add assertions to the existing tests to confirm that the values are
populated appropriately.

Fixes #53726

Change-Id: Ie5b71081e53034e0b5b9ff3c122065c62f15cf23
Reviewed-on: https://go-review.googlesource.com/c/go/+/416354
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go
index e0e8f6125fdca577aa29844444f8b923343280f5..cd87044d17d4ff7af82730857d4a0bec69435341 100644 (file)
--- a/src/crypto/x509/parser.go
+++ b/src/crypto/x509/parser.go
@@ -1008,22 +1008,22 @@ func ParseRevocationList(der []byte) (*RevocationList, error) {
        // 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")
+               return nil, errors.New("x509: malformed crl")
        }
        rl.Raw = input
        if !input.ReadASN1(&input, cryptobyte_asn1.SEQUENCE) {
-               return nil, errors.New("x509: malformed certificate")
+               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 certificate")
+               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 certificate")
+               return nil, errors.New("x509: malformed tbs crl")
        }
 
        var version int
@@ -1148,6 +1148,15 @@ func ParseRevocationList(der []byte) (*RevocationList, error) {
                        if err != nil {
                                return nil, err
                        }
+                       if ext.Id.Equal(oidExtensionAuthorityKeyId) {
+                               rl.AuthorityKeyId = ext.Value
+                       } 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)
                }
        }

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 87eb1f7720b69dca653e8504914389e8f0f2342d..7dcebfa5f10313681dd339bf6ca6e876bb16d303 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -2109,7 +2109,9 @@ type RevocationList struct {
        // Issuer contains the DN of the issuing certificate.
        Issuer pkix.Name
        // AuthorityKeyId is used to identify the public key associated with the
-       // issuing certificate.
+       // issuing certificate. It is populated from the authorityKeyIdentifier
+       // extension when parsing a CRL. It is ignored when creating a CRL; the
+       // extension is populated from the issuing certificate itself.
        AuthorityKeyId []byte
 
        Signature []byte
@@ -2125,7 +2127,8 @@ type RevocationList struct {
 
        // Number is used to populate the X.509 v2 cRLNumber extension in the CRL,
        // which should be a monotonically increasing sequence number for a given
-       // CRL scope and CRL issuer.
+       // CRL scope and CRL issuer. It is also populated from the cRLNumber
+       // extension when parsing a CRL.
        Number *big.Int
 
        // ThisUpdate is used to populate the thisUpdate field in the CRL, which

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index 8ef6115df4c02d144eed9d20abf06d3eded7ae13..594ee1dceb0b7f1d6eba78c0d9efecb91b05056e 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -2681,6 +2681,19 @@ func TestCreateRevocationList(t *testing.T) {
                                t.Fatalf("Extensions mismatch: got %v; want %v.",
                                        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, expectedAKI) {
+                               t.Fatalf("Generated CRL has wrong Number: got %x, want %x",
+                                       parsedCRL.AuthorityKeyId, expectedAKI)
+                       }
                })
        }
 }