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: fec88bb
GitHub-Pull-Request: #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>

Author: mateusz834

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)

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)
 			}
 		})
 	}