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 <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

crypto/x509/internal.h

@@ -170,7 +170,6 @@ struct x509_st {
   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;

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_PURPOSE_get_trust(const X509_PURPOSE *xp) { return xp->trust; }
 
 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 @@ static int setup_crldp(X509 *x) {
 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 @@ int x509v3_cache_extensions(X509 *x) {
     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 @@ static int check_purpose_ssl_client(const X509_PURPOSE *xp, const X509 *x,
   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 @@ static int check_purpose_ssl_server(const X509_PURPOSE *xp, const X509 *x,
     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 @@ static int purpose_smime(const X509 *x, int ca) {
     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;
 }
 

crypto/x509/x509_test.cc

@@ -7715,3 +7715,29 @@ TEST(X509Test, Trust) {
       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()}, {}, {}));
+}

include/openssl/x509.h

@@ -240,9 +240,6 @@ OPENSSL_EXPORT void X509_get0_uids(const X509 *x509,
 #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