crypto/x509: properly pouplate the RevocationList.AuthorityKeyId field

This looks like a oversight in CL 416354.

Fixes #67571
Fixes #57461

Change-Id: I564c008989fecf84b437e123d27121ac907642fa
GitHub-Last-Rev: fec88bbf39a397cc43ff650db9bf0b7ad28e42a0
GitHub-Pull-Request: golang/go#67576
Reviewed-on: https://go-review.googlesource.com/c/go/+/587455
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>

diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go
index cbc5836b32fded6ed3487f084678315baabfaf68..5cc0c7742e44dfdad2b5c69fa25ff3df04fc8ddc 100644 (file)
--- a/src/crypto/x509/parser.go
+++ b/src/crypto/x509/parser.go
@@ -416,6 +416,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
@@ -723,21 +743,9 @@ func processExtensions(out *Certificate) error {
                                }
 
                        case 35:
-                               // RFC 5280, 4.2.1.1
-                               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) {
-                                       return 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 errors.New("x509: invalid authority key identifier")
-                                       }
-                                       out.AuthorityKeyId = akid
+                               out.AuthorityKeyId, err = parseAuthorityKeyIdentifier(e)
+                               if err != nil {
+                                       return err
                                }
                        case 37:
                                out.ExtKeyUsage, out.UnknownExtKeyUsage, err = parseExtKeyUsageExtension(e.Value)
@@ -1226,7 +1234,10 @@ func ParseRevocationList(der []byte) (*RevocationList, error) {
                                return nil, err
                        }
                        if ext.Id.Equal(oidExtensionAuthorityKeyId) {
-                               rl.AuthorityKeyId = ext.Value
+                               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)

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index 954a839fa1a55fc8503f7c26021ba920cf1a963d..d40fd836e019604bb33f106aa7675b0c1d0ad477 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -2908,9 +2908,9 @@ func TestCreateRevocationList(t *testing.T) {
                                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)
+                       if !bytes.Equal(parsedCRL.AuthorityKeyId, tc.issuer.SubjectKeyId) {
+                               t.Fatalf("Generated CRL has wrong AuthorityKeyId: got %x, want %x",
+                                       parsedCRL.AuthorityKeyId, tc.issuer.SubjectKeyId)
                        }
                })
        }