Stop processing the Netscape cert type extension This is an old predecessor to the extended key usage extension, with no tests. Other implementations, such as Chromium's certificate verifier or Go's already do not process it. This doesn't remove the parser for the extension, or the config-based machinery. Folks who want to manually process the extension or construct it can continue to do so. Update-Note: Certificates with a critical Netscape cert type extension will now be rejected by the certificate verifier, matching the behavior of the Chromium verifier. Non-critical extensions will continue to work fine. They will instead be ignored. Change-Id: I5889fb48f00d8dc081fe784d5f1d73d1b884ca5c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65209 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>

diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 47f4aad..422e51c 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h

@@ -170,7 +170,6 @@
   uint32_t ex_flags;
   uint32_t ex_kusage;
   uint32_t ex_xkusage;
-  uint32_t ex_nscert;
   ASN1_OCTET_STRING *skid;
   AUTHORITY_KEYID *akid;
   STACK_OF(DIST_POINT) *crldp;

diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c
index 265e874..2985dce 100644
--- a/crypto/x509/v3_purp.c
+++ b/crypto/x509/v3_purp.c

@@ -73,8 +73,6 @@
   (((x)->ex_flags & EXFLAG_KUSAGE) && !((x)->ex_kusage & (usage)))
 #define xku_reject(x, usage) \
   (((x)->ex_flags & EXFLAG_XKUSAGE) && !((x)->ex_xkusage & (usage)))
-#define ns_reject(x, usage) \
-  (((x)->ex_flags & EXFLAG_NSCERT) && !((x)->ex_nscert & (usage)))
 
 static int check_purpose_ssl_client(const X509_PURPOSE *xp, const X509 *x,
                                     int ca);
@@ -186,8 +184,7 @@
 
 int X509_supported_extension(const X509_EXTENSION *ex) {
   int nid = OBJ_obj2nid(X509_EXTENSION_get_object(ex));
-  return nid == NID_netscape_cert_type ||    //
-         nid == NID_key_usage ||             //
+  return nid == NID_key_usage ||             //
          nid == NID_subject_alt_name ||      //
          nid == NID_basic_constraints ||     //
          nid == NID_certificate_policies ||  //
@@ -234,7 +231,6 @@
 int x509v3_cache_extensions(X509 *x) {
   BASIC_CONSTRAINTS *bs;
   ASN1_BIT_STRING *usage;
-  ASN1_BIT_STRING *ns;
   EXTENDED_KEY_USAGE *extusage;
   size_t i;
   int j;
@@ -348,17 +344,6 @@
     x->ex_flags |= EXFLAG_INVALID;
   }
 
-  if ((ns = X509_get_ext_d2i(x, NID_netscape_cert_type, &j, NULL))) {
-    if (ns->length > 0) {
-      x->ex_nscert = ns->data[0];
-    } else {
-      x->ex_nscert = 0;
-    }
-    x->ex_flags |= EXFLAG_NSCERT;
-    ASN1_BIT_STRING_free(ns);
-  } else if (j != -1) {
-    x->ex_flags |= EXFLAG_INVALID;
-  }
   x->skid = X509_get_ext_d2i(x, NID_subject_key_identifier, &j, NULL);
   if (x->skid == NULL && j != -1) {
     x->ex_flags |= EXFLAG_INVALID;
@@ -442,10 +427,6 @@
   if (ku_reject(x, X509v3_KU_DIGITAL_SIGNATURE | X509v3_KU_KEY_AGREEMENT)) {
     return 0;
   }
-  // nsCertType if present should allow SSL client use
-  if (ns_reject(x, NS_SSL_CLIENT)) {
-    return 0;
-  }
   return 1;
 }
 
@@ -465,9 +446,6 @@
     return check_ca(x);
   }
 
-  if (ns_reject(x, NS_SSL_SERVER)) {
-    return 0;
-  }
   if (ku_reject(x, X509v3_KU_TLS)) {
     return 0;
   }
@@ -495,16 +473,8 @@
     return 0;
   }
   if (ca) {
-    // check nsCertType if present
-    if ((x->ex_flags & EXFLAG_NSCERT) && (x->ex_nscert & NS_SMIME_CA) == 0) {
-      return 0;
-    }
-
     return check_ca(x);
   }
-  if (x->ex_flags & EXFLAG_NSCERT) {
-    return (x->ex_nscert & NS_SMIME) == NS_SMIME;
-  }
   return 1;
 }
 

diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 3493806..b40eb99 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc

@@ -7715,3 +7715,29 @@
       Verify(leaf.normal.get(), {root.trusted_any.get()},
              {intermediate.normal.get()}, {}, /*flags=*/0, set_server_trust));
 }
+
+TEST(X509Test, CriticalExtension) {
+  bssl::UniquePtr<EVP_PKEY> key = PrivateKeyFromPEM(kP256Key);
+  ASSERT_TRUE(key);
+
+  bssl::UniquePtr<X509> root =
+      MakeTestCert("Root", "Root", key.get(), /*is_ca=*/true);
+  ASSERT_TRUE(root);
+  ASSERT_TRUE(X509_sign(root.get(), key.get(), EVP_sha256()));
+
+  // Issue a certificate with a critical Netscape certificate type extension. We
+  // do not recognize this extension, so this certificate should be rejected.
+  bssl::UniquePtr<X509> leaf =
+      MakeTestCert("Root", "Leaf", key.get(), /*is_ca=*/false);
+  ASSERT_TRUE(leaf);
+  bssl::UniquePtr<ASN1_BIT_STRING> cert_type(ASN1_BIT_STRING_new());
+  ASSERT_TRUE(cert_type);
+  ASSERT_TRUE(ASN1_BIT_STRING_set_bit(cert_type.get(), /*n=*/0, /*value=*/1));
+  ASSERT_TRUE(X509_add1_ext_i2d(leaf.get(), NID_netscape_cert_type,
+                                cert_type.get(),
+                                /*crit=*/1, /*flags=*/0));
+  ASSERT_TRUE(X509_sign(leaf.get(), key.get(), EVP_sha256()));
+
+  EXPECT_EQ(X509_V_ERR_UNHANDLED_CRITICAL_EXTENSION,
+            Verify(leaf.get(), {root.get()}, {}, {}));
+}

diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index ff18c48..4993d95 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h

@@ -240,9 +240,6 @@
 #define EXFLAG_KUSAGE 0x2
 // EXFLAG_XKUSAGE indicates the certifcate has an extended key usage extension.
 #define EXFLAG_XKUSAGE 0x4
-// EXFLAG_NSCERT indicates the certificate has a legacy Netscape certificate
-// type extension.
-#define EXFLAG_NSCERT 0x8
 // EXFLAG_CA indicates the certificate has a basic constraints extension with
 // the CA bit set.
 #define EXFLAG_CA 0x10