Bug 1136616 - Allow underscores in reference DNS-IDs in mozilla::pkix name matching. r=briansmith, a=lmandel
authorDavid Keeler <dkeeler@mozilla.com>
Tue, 03 Mar 2015 13:34:45 -0800
changeset 250318 d59086707825
parent 250317 73c7414f883d
child 250319 73a7e99cfd2a
push id4544
push userryanvm@gmail.com
push date2015-03-09 19:49 +0000
treeherdermozilla-beta@d59086707825 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbriansmith, lmandel
bugs1136616
milestone37.0
Bug 1136616 - Allow underscores in reference DNS-IDs in mozilla::pkix name matching. r=briansmith, a=lmandel
security/pkix/lib/pkixnames.cpp
security/pkix/test/gtest/pkixnames_tests.cpp
--- a/security/pkix/lib/pkixnames.cpp
+++ b/security/pkix/lib/pkixnames.cpp
@@ -1818,16 +1818,20 @@ IsValidReferenceDNSID(Input hostname)
 bool
 IsValidPresentedDNSID(Input hostname)
 {
   return IsValidDNSID(hostname, IDRole::PresentedID, AllowWildcards::Yes);
 }
 
 namespace {
 
+// RFC 5280 Section 4.2.1.6 says that a dNSName "MUST be in the 'preferred name
+// syntax', as specified by Section 3.5 of [RFC1034] and as modified by Section
+// 2.1 of [RFC1123]" except "a dNSName of ' ' MUST NOT be used." Additionally,
+// we allow underscores for compatibility with existing practice.
 bool
 IsValidDNSID(Input hostname, IDRole idRole, AllowWildcards allowWildcards)
 {
   if (hostname.GetLength() > 253) {
     return false;
   }
 
   Reader input(hostname);
@@ -1912,16 +1916,19 @@ IsValidDNSID(Input hostname, IDRole idRo
       case 'f': case 'F': case 's': case 'S':
       case 'g': case 'G': case 't': case 'T':
       case 'h': case 'H': case 'u': case 'U':
       case 'i': case 'I': case 'v': case 'V':
       case 'j': case 'J': case 'w': case 'W':
       case 'k': case 'K': case 'x': case 'X':
       case 'l': case 'L': case 'y': case 'Y':
       case 'm': case 'M': case 'z': case 'Z':
+      // We allow underscores for compatibility with existing practices.
+      // See bug 1136616.
+      case '_':
         labelIsAllNumeric = false;
         labelEndsWithHyphen = false;
         ++labelLength;
         if (labelLength > MAX_LABEL_LENGTH) {
           return false;
         }
         break;
 
--- a/security/pkix/test/gtest/pkixnames_tests.cpp
+++ b/security/pkix/test/gtest/pkixnames_tests.cpp
@@ -81,16 +81,37 @@ static const PresentedMatchesReference D
 
   DNS_ID_MATCH("a", "a"),
   DNS_ID_MISMATCH("b", "a"),
 
   DNS_ID_MATCH("*.b.a", "c.b.a"),
   DNS_ID_MISMATCH("*.b.a", "b.a"),
   DNS_ID_MISMATCH("*.b.a", "b.a."),
 
+  // We allow underscores for compatibility with existing practices.
+  DNS_ID_MATCH("a_b", "a_b"),
+  DNS_ID_MATCH("*.example.com", "uses_underscore.example.com"),
+  DNS_ID_MATCH("*.uses_underscore.example.com", "a.uses_underscore.example.com"),
+
+  // See bug 1139039
+  DNS_ID_MATCH("_.example.com", "_.example.com"),
+  DNS_ID_MATCH("*.example.com", "_.example.com"),
+  DNS_ID_MATCH("_", "_"),
+  DNS_ID_MATCH("___", "___"),
+  DNS_ID_MATCH("example_", "example_"),
+  DNS_ID_MATCH("_example", "_example"),
+  DNS_ID_MATCH("*._._", "x._._"),
+
+  // See bug 1139039
+  // A DNS-ID must not end in an all-numeric label. We don't consider
+  // underscores to be numeric.
+  DNS_ID_MATCH("_1", "_1"),
+  DNS_ID_MATCH("example._1", "example._1"),
+  DNS_ID_MATCH("example.1_", "example.1_"),
+
   // Wildcard not in leftmost label
   DNS_ID_MATCH("d.c.b.a", "d.c.b.a"),
   DNS_ID_BAD_DER("d.*.b.a", "d.c.b.a"),
   DNS_ID_BAD_DER("d.c*.b.a", "d.c.b.a"),
   DNS_ID_BAD_DER("d.c*.b.a", "d.cc.b.a"),
 
   // case sensitivity
   DNS_ID_MATCH("abcdefghijklmnopqrstuvwxyz", "ABCDEFGHIJKLMNOPQRSTUVWXYZ"),
@@ -382,16 +403,26 @@ static const InputValidity DNSNAMES_VALI
   I("xn--\0", false, false),
 
   // Allowed character set
   I("a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z", true, true),
   I("A.B.C.D.E.F.G.H.I.J.K.L.M.N.O.P.Q.R.S.T.U.V.W.X.Y.Z", true, true),
   I("0.1.2.3.4.5.6.7.8.9.a", true, true), // "a" needed to avoid numeric last label
   I("a-b", true, true), // hyphen (a label cannot start or end with a hyphen)
 
+  // Underscores
+  I("a_b", true, true),
+  // See bug 1139039
+  I("_", true, true),
+  I("a_", true, true),
+  I("_a", true, true),
+  I("_1", true, true),
+  I("1_", true, true),
+  I("___", true, true),
+
   // An invalid character in various positions
   I("!", false, false),
   I("!a", false, false),
   I("a!", false, false),
   I("a!b", false, false),
   I("a.!", false, false),
   I("a.a!", false, false),
   I("a.!a", false, false),
@@ -1385,16 +1416,24 @@ static const CheckCertHostnameParams CHE
   // One CN, one RDN, CN is not in the first or last AVA, CN matches.
   WITHOUT_SAN("b", RDN(OU("a") + CN("b") + OU("c")), Success),
   // One CN, multiple RDNs, CN is not in the first or last RDN, CN matches.
   WITHOUT_SAN("b", RDN(OU("a")) + RDN(CN("b")) + RDN(OU("c")), Success),
 
   // Empty CN does not match.
   WITHOUT_SAN("example.com", RDN(CN("")), Result::ERROR_BAD_CERT_DOMAIN),
 
+  WITHOUT_SAN("uses_underscore.example.com", RDN(CN("*.example.com")), Success),
+  WITHOUT_SAN("a.uses_underscore.example.com",
+              RDN(CN("*.uses_underscore.example.com")), Success),
+  WITH_SAN("uses_underscore.example.com", RDN(CN("foo")),
+           DNSName("*.example.com"), Success),
+  WITH_SAN("a.uses_underscore.example.com", RDN(CN("foo")),
+           DNSName("*.uses_underscore.example.com"), Success),
+
   // Do not match a DNSName that is encoded in a malformed IPAddress.
   WITH_SAN("example.com", RDN(CN("foo")), IPAddress(example_com),
            Result::ERROR_BAD_CERT_DOMAIN),
 
   // We skip over the malformed IPAddress and match the DNSName entry because
   // we've heard reports of real-world certificates that have malformed
   // IPAddress SANs.
   WITH_SAN("example.org", RDN(CN("foo")),
@@ -2321,16 +2360,44 @@ static const NameConstraintParams NAME_C
     Result::ERROR_BAD_DER,
     Result::ERROR_BAD_DER
   },
   { ByteString(), RFC822Name("@a.example.com"),
     GeneralSubtree(RFC822Name(".example.com")),
     Result::ERROR_BAD_DER,
     Result::ERROR_BAD_DER
   },
+
+  /////////////////////////////////////////////////////////////////////////////
+  // Test name constraints with underscores.
+  //
+  { ByteString(), DNSName("uses_underscore.example.com"),
+    GeneralSubtree(DNSName("uses_underscore.example.com")),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { ByteString(), DNSName("uses_underscore.example.com"),
+    GeneralSubtree(DNSName("example.com")),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { ByteString(), DNSName("a.uses_underscore.example.com"),
+    GeneralSubtree(DNSName("uses_underscore.example.com")),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { ByteString(), RFC822Name("a@uses_underscore.example.com"),
+    GeneralSubtree(RFC822Name("uses_underscore.example.com")),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { ByteString(), RFC822Name("uses_underscore@example.com"),
+    GeneralSubtree(RFC822Name("example.com")),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { ByteString(), RFC822Name("a@a.uses_underscore.example.com"),
+    GeneralSubtree(RFC822Name(".uses_underscore.example.com")),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
 };
 
 class pkixnames_CheckNameConstraints
   : public ::testing::Test
   , public ::testing::WithParamInterface<NameConstraintParams>
 {
 };