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 <filippo@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go
index 726409e988a00b8eff3e992bdbb5dd56bef90f1a..001b0017750f76e0c3fe4cd6a6fef4aded01b724 100644 (file)
--- 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 0f528d4cc29d7b739b0b47d2106dac6d8a8cc37f..a29f914c8ea49920a9cbcb96dc748201f3203a99 100644 (file)
--- 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)
+       }
+}