From e0aab32c7f90882ad2f6e52c03de0e22e2af5b31 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 7 Feb 2024 13:05:59 -0800 Subject: [PATCH] crypto/x509: reject critical SKI extensions Updates #65085 Change-Id: I8a00fff6b2af4e55bcb88456813b5ee1f7b1c01d Reviewed-on: https://go-review.googlesource.com/c/go/+/562344 Reviewed-by: Filippo Valsorda Reviewed-by: Cherry Mui LUCI-TryBot-Result: Go LUCI --- src/crypto/x509/parser.go | 4 ++++ src/crypto/x509/x509_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go index 726409e988..001b001775 100644 --- a/src/crypto/x509/parser.go +++ b/src/crypto/x509/parser.go @@ -741,6 +741,10 @@ func processExtensions(out *Certificate) error { } case 14: // RFC 5280, 4.2.1.2 + if e.Critical { + // Conforming CAs MUST mark this extension as non-critical + return errors.New("x509: subject key identifier incorrectly marked critical") + } val := cryptobyte.String(e.Value) var skid cryptobyte.String if !val.ReadASN1(&skid, cryptobyte_asn1.OCTET_STRING) { diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go index 0f528d4cc2..a29f914c8e 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go @@ -4035,3 +4035,28 @@ func TestRejectCriticalAIA(t *testing.T) { t.Fatalf("ParseCertificate() unexpected error: %v, want: %s", err, expectedErr) } } + +func TestRejectCriticalSKI(t *testing.T) { + template := Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "Cert"}, + NotBefore: time.Unix(1000, 0), + NotAfter: time.Unix(100000, 0), + ExtraExtensions: []pkix.Extension{ + { + Id: asn1.ObjectIdentifier{2, 5, 29, 14}, + Critical: true, + Value: []byte{1, 2, 3}, + }, + }, + } + certDER, err := CreateCertificate(rand.Reader, &template, &template, rsaPrivateKey.Public(), rsaPrivateKey) + if err != nil { + t.Fatalf("CreateCertificate() unexpected error: %v", err) + } + expectedErr := "x509: subject key identifier incorrectly marked critical" + _, err = ParseCertificate(certDER) + if err == nil || err.Error() != expectedErr { + t.Fatalf("ParseCertificate() unexpected error: %v, want: %s", err, expectedErr) + } +} -- 2.48.1