Bug 1136616 - Allow underscores in reference DNS-IDs in mozilla::pkix name matching. r=briansmith, a=sledru
authorDavid Keeler <dkeeler@mozilla.com>
Mon, 02 Mar 2015 10:41:02 -0800
changeset 245442 e1a9cef7a5f6
parent 245441 c37f6a5a0c8a
child 245443 7e01afd0e736
push id675
push userryanvm@gmail.com
push date2015-03-03 19:16 +0000
treeherdermozilla-release@7e01afd0e736 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbriansmith, sledru
bugs1136616
milestone36.0
Bug 1136616 - Allow underscores in reference DNS-IDs in mozilla::pkix name matching. r=briansmith, a=sledru
security/pkix/lib/pkixnames.cpp
security/pkix/test/gtest/pkixnames_tests.cpp
--- a/security/pkix/lib/pkixnames.cpp
+++ b/security/pkix/lib/pkixnames.cpp
@@ -831,16 +831,20 @@ IsValidReferenceDNSID(Input hostname)
 bool
 IsValidPresentedDNSID(Input hostname)
 {
   return IsValidDNSID(hostname, ValidDNSIDMatchType::PresentedID);
 }
 
 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, ValidDNSIDMatchType matchType)
 {
   if (hostname.GetLength() > 253) {
     return false;
   }
 
   Reader input(hostname);
@@ -910,16 +914,19 @@ IsValidDNSID(Input hostname, ValidDNSIDM
       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
@@ -68,16 +68,39 @@ 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"),
+  // We require that the reference ID be valid, so we can't test this.
+  // DNS_ID_MISMATCH("example.1", "example.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_MISMATCH("d.*.b.a", "d.c.b.a"),
   DNS_ID_MISMATCH("d.c*.b.a", "d.c.b.a"),
   DNS_ID_MISMATCH("d.c*.b.a", "d.cc.b.a"),
 
   // case sensitivity
   DNS_ID_MATCH("abcdefghijklmnopqrstuvwxyz", "ABCDEFGHIJKLMNOPQRSTUVWXYZ"),
@@ -361,16 +384,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),
@@ -1216,16 +1249,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")),