From 6db272d2dd65202e97058cb0c4909151eed05ae6 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Wed, 22 May 2024 18:38:37 +0000 Subject: [PATCH] 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 LUCI-TryBot-Result: Go LUCI Reviewed-by: Roland Shoemaker --- src/crypto/x509/parser.go | 43 ++++++++++++++++++++++-------------- src/crypto/x509/x509_test.go | 6 ++--- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go index cbc5836b32..5cc0c7742e 100644 --- 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 954a839fa1..d40fd836e0 100644 --- 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) } }) } -- 2.48.1