Name constraints with excluded names but no permitted names should allow names...

Name constraints with excluded names but no permitted names should allow names not matching the excluded names.

BUG=410574

Review URL: https://codereview.chromium.org/1546653004

Cr-Commit-Position: refs/heads/master@{#371603}

net/cert/internal/name_constraints.cc

@@ -496,10 +496,6 @@ bool NameConstraints::IsPermittedCert(
 }
 
 bool NameConstraints::IsPermittedDNSName(const std::string& name) const {
-  // If there are no name constraints for DNS names, all names are accepted.
-  if (!(ConstrainedNameTypes() & GENERAL_NAME_DNS_NAME))
-    return true;
-
   for (const std::string& excluded_name : excluded_subtrees_.dns_names) {
     // When matching wildcard hosts against excluded subtrees, consider it a
     // match if the constraint would match any expansion of the wildcard. Eg,
@@ -507,6 +503,12 @@ bool NameConstraints::IsPermittedDNSName(const std::string& name) const {
     if (DNSNameMatches(name, excluded_name, WILDCARD_PARTIAL_MATCH))
       return false;
   }
+
+  // If permitted subtrees are not constrained, any name that is not excluded is
+  // allowed.
+  if (!(permitted_subtrees_.present_name_types & GENERAL_NAME_DNS_NAME))
+    return true;
+
   for (const std::string& permitted_name : permitted_subtrees_.dns_names) {
     // When matching wildcard hosts against permitted subtrees, consider it a
     // match only if the constraint would match all expansions of the wildcard.
@@ -520,11 +522,6 @@ bool NameConstraints::IsPermittedDNSName(const std::string& name) const {
 
 bool NameConstraints::IsPermittedDirectoryName(
     const der::Input& name_rdn_sequence) const {
-  // If there are no name constraints for directory names, all names are
-  // accepted.
-  if (!(ConstrainedNameTypes() & GENERAL_NAME_DIRECTORY_NAME))
-    return true;
-
   for (const auto& excluded_name : excluded_subtrees_.directory_names) {
     if (VerifyNameInSubtree(
             name_rdn_sequence,
@@ -532,6 +529,12 @@ bool NameConstraints::IsPermittedDirectoryName(
       return false;
     }
   }
+
+  // If permitted subtrees are not constrained, any name that is not excluded is
+  // allowed.
+  if (!(permitted_subtrees_.present_name_types & GENERAL_NAME_DIRECTORY_NAME))
+    return true;
+
   for (const auto& permitted_name : permitted_subtrees_.directory_names) {
     if (VerifyNameInSubtree(
             name_rdn_sequence,
@@ -544,15 +547,16 @@ bool NameConstraints::IsPermittedDirectoryName(
 }
 
 bool NameConstraints::IsPermittedIP(const IPAddressNumber& ip) const {
-  // If there are no name constraints for IP Address names, all names are
-  // accepted.
-  if (!(ConstrainedNameTypes() & GENERAL_NAME_IP_ADDRESS))
-    return true;
-
   for (const auto& excluded_ip : excluded_subtrees_.ip_address_ranges) {
     if (IPNumberMatchesPrefix(ip, excluded_ip.first, excluded_ip.second))
       return false;
   }
+
+  // If permitted subtrees are not constrained, any name that is not excluded is
+  // allowed.
+  if (!(permitted_subtrees_.present_name_types & GENERAL_NAME_IP_ADDRESS))
+    return true;
+
   for (const auto& permitted_ip : permitted_subtrees_.ip_address_ranges) {
     if (IPNumberMatchesPrefix(ip, permitted_ip.first, permitted_ip.second))
       return true;

net/cert/internal/name_constraints_unittest.cc

@@ -193,11 +193,11 @@ TEST_P(ParseNameConstraints, DNSNamesExcludeOnly) {
       NameConstraints::CreateFromDer(der::Input(&a), is_critical()));
   ASSERT_TRUE(name_constraints);
 
-  // Only "excluded.permitted.example.com" is excluded, but since no dNSNames
-  // are permitted, everything is excluded.
-  EXPECT_FALSE(name_constraints->IsPermittedDNSName(""));
-  EXPECT_FALSE(name_constraints->IsPermittedDNSName("foo.com"));
-  EXPECT_FALSE(name_constraints->IsPermittedDNSName("permitted.example.com"));
+  // Only "excluded.permitted.example.com" is excluded, and since permitted is
+  // empty, any dNSName outside that is allowed.
+  EXPECT_TRUE(name_constraints->IsPermittedDNSName(""));
+  EXPECT_TRUE(name_constraints->IsPermittedDNSName("foo.com"));
+  EXPECT_TRUE(name_constraints->IsPermittedDNSName("permitted.example.com"));
   EXPECT_FALSE(
       name_constraints->IsPermittedDNSName("excluded.permitted.example.com"));
   EXPECT_FALSE(
@@ -349,11 +349,11 @@ TEST_P(ParseNameConstraints, DirectoryNamesExcludeOnly) {
   ASSERT_TRUE(LoadTestName("name-us-california-mountain_view.pem",
                            &name_us_ca_mountain_view));
 
-  // Only "C=US,ST=California" is excluded, but since no directoryNames are
-  // permitted, everything is excluded.
-  EXPECT_FALSE(name_constraints->IsPermittedDirectoryName(
+  // Only "C=US,ST=California" is excluded, and since permitted is empty,
+  // any directoryName outside that is allowed.
+  EXPECT_TRUE(name_constraints->IsPermittedDirectoryName(
       SequenceValueFromString(&name_empty)));
-  EXPECT_FALSE(name_constraints->IsPermittedDirectoryName(
+  EXPECT_TRUE(name_constraints->IsPermittedDirectoryName(
       SequenceValueFromString(&name_us)));
   EXPECT_FALSE(name_constraints->IsPermittedDirectoryName(
       SequenceValueFromString(&name_us_ca)));
@@ -364,7 +364,7 @@ TEST_P(ParseNameConstraints, DirectoryNamesExcludeOnly) {
 TEST_P(ParseNameConstraints, DirectoryNamesExcludeAll) {
   std::string constraints_der;
   ASSERT_TRUE(
-      LoadTestNameConstraint("directoryname-excluded.pem", &constraints_der));
+      LoadTestNameConstraint("directoryname-excludeall.pem", &constraints_der));
   scoped_ptr<NameConstraints> name_constraints(NameConstraints::CreateFromDer(
       der::Input(&constraints_der), is_critical()));
   ASSERT_TRUE(name_constraints);
@@ -567,11 +567,11 @@ TEST_P(ParseNameConstraints, IPAdressesExcludeOnly) {
       NameConstraints::CreateFromDer(der::Input(&a), is_critical()));
   ASSERT_TRUE(name_constraints);
 
-  // Only 192.168.5.0/255.255.255.0 is excluded, but since no iPAddresses
-  // are permitted, everything is excluded.
+  // Only 192.168.5.0/255.255.255.0 is excluded, and since permitted is empty,
+  // any iPAddress outside that is allowed.
   {
     const uint8_t ip4[] = {192, 168, 0, 1};
-    EXPECT_FALSE(name_constraints->IsPermittedIP(
+    EXPECT_TRUE(name_constraints->IsPermittedIP(
         IPAddressNumber(ip4, ip4 + arraysize(ip4))));
   }
   {
@@ -581,7 +581,7 @@ TEST_P(ParseNameConstraints, IPAdressesExcludeOnly) {
   }
   {
     const uint8_t ip6[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 13, 0, 0, 0, 1};
-    EXPECT_FALSE(name_constraints->IsPermittedIP(
+    EXPECT_TRUE(name_constraints->IsPermittedIP(
         IPAddressNumber(ip6, ip6 + arraysize(ip6))));
   }
 }