Reintroduce support for validating DNS commonName subjects when name constraints are present.

crypto/x509/internal.h

@@ -451,11 +451,6 @@ unsigned char *x509v3_hex_to_bytes(const char *str, size_t *len);
 // with |cmp| followed by '.', and zero otherwise.
 int x509v3_conf_name_matches(const char *name, const char *cmp);
 
-// x509v3_looks_like_dns_name returns one if |in| looks like a DNS name and zero
-// otherwise.
-OPENSSL_EXPORT int x509v3_looks_like_dns_name(const unsigned char *in,
-                                              size_t len);
-
 // x509v3_cache_extensions fills in a number of fields relating to X.509
 // extensions in |x|. It returns one on success and zero if some extensions were
 // invalid.
@@ -538,6 +533,8 @@ int X509V3_add_value_int(const char *name, const ASN1_INTEGER *aint,
   ERR_add_error_data(6, "section:", (val)->section, ",name:", (val)->name, \
                      ",value:", (val)->value);
 
+int NAME_CONSTRAINTS_check_CN(X509 *x, NAME_CONSTRAINTS *nc);
+
 // GENERAL_NAME_cmp returns zero if |a| and |b| are equal and a non-zero
 // value otherwise. Note this function does not provide a comparison suitable
 // for sorting.
@@ -576,6 +573,8 @@ int DIST_POINT_set_dpname(DIST_POINT_NAME *dpn, X509_NAME *iname);
 // For example the IPv4 mask `255.255.255.0` would return valid, but `255.0.255.0` would be invalid.
 OPENSSL_EXPORT int validate_cidr_mask(CBS *cidr_mask);
 
+OPENSSL_EXPORT int cn2dnsid(ASN1_STRING *cn, unsigned char **dnsid, size_t *idlen);
+
 #if defined(__cplusplus)
 }  // extern C
 #endif

crypto/x509/test/make_many_constraints.go

@@ -139,10 +139,10 @@ func main() {
 	}{
 		{"many_names1.pem", 513, 513},
 		{"many_names2.pem", 1025, 0},
-		{"many_names3.pem", 1, 1025},
+		{"many_names3.pem", 0, 1025},
 		{"some_names1.pem", 256, 256},
 		{"some_names2.pem", 513, 0},
-		{"some_names3.pem", 1, 513},
+		{"some_names3.pem", 0, 513},
 	}
 	for i, leaf := range leaves {
 		leafTemplate := x509.Certificate{

crypto/x509/test/many_names3.pem

@@ -1,5 +1,5 @@
 -----BEGIN CERTIFICATE-----
-MIJqrDCCaZSgAwIBAgIBBDANBgkqhkiG9w0BAQsFADANMQswCQYDVQQDEwJDQTAg
+MIJqmDCCaYCgAwIBAgIBBDANBgkqhkiG9w0BAQsFADANMQswCQYDVQQDEwJDQTAg
 Fw0wMDAxMDEwMDAwMDBaGA8yMTAwMDEwMTAwMDAwMFowgmfXMRAwDgYDVQQDEwd0
 MC50ZXN0MRYwFAYJKoZIhvcNAQkBFgd0MEB0ZXN0MRYwFAYJKoZIhvcNAQkBFgd0
 MUB0ZXN0MRYwFAYJKoZIhvcNAQkBFgd0MkB0ZXN0MRYwFAYJKoZIhvcNAQkBFgd0
@@ -560,12 +560,12 @@ AQ8AMIIBCgKCAQEAugvahBkSAUF1fC49vb1bvlPrcl80kop1iLpiuYoz4Qptwy57
 oiXCzepBrhtp5UQSjHD4D4hKtgdMgVxX+LRtwgW3mnu/vBu7rzpr/DS8io99p3lq
 Z1Aky+aNlcMj6MYy8U+YFEevb/V0lRY9oqwmW7BHnXikm/vi6sjIS350U8zb/mRz
 YeIs2R65LUduTL50+UMgat9ocewI2dv8aO9Dph+8NdGtg8LFYyTTHcUxJoMr1PTO
-gnmET19WJH4PrFwk7ZE1QJQQ1L4iKmPeQistuQIDAQABo0kwRzAOBgNVHQ8BAf8E
-BAMCBaAwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADASBgNVHREE
-CzAJggd0MC50ZXN0MA0GCSqGSIb3DQEBCwUAA4IBAQAi7LIMyX5Ec514hvjROZ8b
-7i4UR3xd5IbniVSej+PKZhG2inN6aX9bksdda0ddYZeRSHAkNJuoabeankQJ/x5x
-sxBntWSVLCxz6S8NRrLAPKKPBvFb/W5ns57LP9SrLIij9l/NSd+K/CQNTlfcdorg
-4ltPVNwSMp/XXjH6rQYJSbo9MhDoxeqPpv73e4jY0DfGn1a8uwyCXalLjh4EkUyS
-Ye0N7MoUKV0IucrXKdgj2sHgBFqNKJ/GVQ422xZRbYqsyIJ0bPD6Fc8VcqfVrvYg
-lCYJfu7Xij5n3mjQaSYcbVxH71X8fYhhNq1tk+WtQOXirz2EkSuh1rNGU/LT8Q6r
+gnmET19WJH4PrFwk7ZE1QJQQ1L4iKmPeQistuQIDAQABozUwMzAOBgNVHQ8BAf8E
+BAMCBaAwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADANBgkqhkiG
+9w0BAQsFAAOCAQEAtMIpnGzOBkJXEBmCsRVbTrg9QgYRlGPG48+cXT2QbIutAmbj
+miF+OYg/bRsQtuqcKjnJYog+x6UCU3d34jaMEfEXfHSwF7xPQrqJm45MXhG3so4E
++el5GMAS+SKFQK3w8NPoGhGwn82sz4XV6HMG+ANUxMlCrOcx2jh5UW+7ITjdRwJd
+ReJ/JaMpneJdwGFSU9Vn+t7PFb51/pOYqO/PuEANzphovjMVcFZ6mtAQwYDkQZBJ
+Vy1/7bPoNmbKD0GAS6HpS+xaJ/DnjjD6Kal2T7GMyvRMogj5BeZ/uEkXCEhvoaBT
+os1gaqqnGpZ6JSEDctzjgpCtEPR40yiz1wv1CA==
 -----END CERTIFICATE-----

crypto/x509/test/some_names3.pem

@@ -1,5 +1,5 @@
 -----BEGIN CERTIFICATE-----
-MII2kzCCNXugAwIBAgIBBzANBgkqhkiG9w0BAQsFADANMQswCQYDVQQDEwJDQTAg
+MII2fzCCNWegAwIBAgIBBzANBgkqhkiG9w0BAQsFADANMQswCQYDVQQDEwJDQTAg
 Fw0wMDAxMDEwMDAwMDBaGA8yMTAwMDEwMTAwMDAwMFowgjO+MRAwDgYDVQQDEwd0
 MC50ZXN0MRYwFAYJKoZIhvcNAQkBFgd0MEB0ZXN0MRYwFAYJKoZIhvcNAQkBFgd0
 MUB0ZXN0MRYwFAYJKoZIhvcNAQkBFgd0MkB0ZXN0MRYwFAYJKoZIhvcNAQkBFgd0
@@ -282,13 +282,12 @@ M+EKbcMue/hFrLGQXB6a2eQZFn+j3hmexeQF9T8iWxh2S6rzAr1Yj+qXeDBaMf4o
 BEiEhBxIsaIlws3qQa4baeVEEoxw+A+ISrYHTIFcV/i0bcIFt5p7v7wbu686a/w0
 vIqPfad5amdQJMvmjZXDI+jGMvFPmBRHr2/1dJUWPaKsJluwR514pJv74urIyEt+
 dFPM2/5kc2HiLNkeuS1Hbky+dPlDIGrfaHHsCNnb/GjvQ6YfvDXRrYPCxWMk0x3F
-MSaDK9T0zoJ5hE9fViR+D6xcJO2RNUCUENS+Iipj3kIrLbkCAwEAAaNJMEcwDgYD
+MSaDK9T0zoJ5hE9fViR+D6xcJO2RNUCUENS+Iipj3kIrLbkCAwEAAaM1MDMwDgYD
 VR0PAQH/BAQDAgWgMBMGA1UdJQQMMAoGCCsGAQUFBwMBMAwGA1UdEwEB/wQCMAAw
-EgYDVR0RBAswCYIHdDAudGVzdDANBgkqhkiG9w0BAQsFAAOCAQEAQA/0vvY1gLA2
-0jrPkBVWte7OHzWVkwq7mqgQPR4L9qLLu7Vhelp4dW8n95s1wCbca5j5SJEGv4Uv
-0fI1OOK7XQeYdNlHBmvMVW47GoBSo6tuYNPI/y4xnM6ypEZiPKkdj9Ar9qNgURfV
-z3s1czip915dyTWgwBy7CTxOlG8NW0uiFgEc9iiDDfQsPwVXiVtxOPtjhPeI3F0J
-jh3wctFxBnAvLV9SsDxpWujM1dd/1SSQ25jKQhbKNtiDAC8v+Q043r8ZGHjRdxe8
-W2tVWH/iz9c+ze0P0ao7LKv8eGzoIsrBqICS86X4Zv5lGeTGaD2osF1oNvmmoSlh
-536yFa415g==
+DQYJKoZIhvcNAQELBQADggEBAH6ad2kFE0qGDe3ErMdwTGjbBz3T12dDvAUVhGHQ
+uZShOdPsXMHD2mUqFgLE0iJFeXB7jOSAKtzmKHNmxZ4W0UZ7eMPPogkgIbG3d3yR
+8zBO21CUyOQWChywpKcAou9ji3Kq6pb4+mqq0a5TGIYyGJKSUTv09KI+iHgwteCX
+DHzzhuTs8AhodmNO5K/F9YFWJWvQ1NrwyUmOFEw8/UcljyKxFrP2VEov0fWeiTRB
+Ps6VaFBW7SEEi8fAM9W5kfsl+iWRvwFcFdXGQt1HbeywCu58DLI4uceHCFb+3MMO
+Xv7wJ5UhQODuzwuq7CuZvlxR2tiFoPP+s5fPH0L8MBP5z6w=
 -----END CERTIFICATE-----

crypto/x509/v3_ncons.c

@@ -305,6 +305,132 @@ int NAME_CONSTRAINTS_check(X509 *x, NAME_CONSTRAINTS *nc) {
   return X509_V_OK;
 }
 
+int cn2dnsid(ASN1_STRING *cn, unsigned char **dnsid, size_t *idlen) {
+  assert(dnsid != NULL && idlen != NULL);
+
+  // Don't leave outputs uninitialized
+  *dnsid = NULL;
+  *idlen = 0;
+
+  // Per RFC 6125, DNS-IDs representing internationalized domain names appear
+  // in certificates in A-label encoded form:
+  //
+  // https://tools.ietf.org/html/rfc6125#section-6.4.2
+  //
+  // The same applies to CNs which are intended to represent DNS names.
+  // However, while in the SAN DNS-IDs are IA5Strings, as CNs they may be
+  // needlessly encoded in 16-bit Unicode.  We perform a conversion to UTF-8
+  // to ensure that we get an ASCII representation of any CNs that are
+  // representable as ASCII, but just not encoded as ASCII.  The UTF-8 form
+  // may contain some non-ASCII octets, and that's fine, such CNs are not
+  // valid legacy DNS names.
+  //
+  // Note, 'int' is the return type of ASN1_STRING_to_UTF8() so that's what
+  // we must use for 'utf8_length'.
+  unsigned char *utf8_value;
+  int utf8_length = ASN1_STRING_to_UTF8(&utf8_value, cn);
+  if (utf8_length < 0) {
+    return X509_V_ERR_OUT_OF_MEM;
+  }
+
+  // Some certificates have had names that include a *trailing* NUL byte.
+  // Remove these harmless NUL characters. They would otherwise yield false
+  // alarms with the following embedded NUL check.
+  while (utf8_length > 0 && utf8_value[utf8_length - 1] == '\0') {
+    --utf8_length;
+  }
+
+  // Reject *embedded* NULs
+  if (OPENSSL_memchr(utf8_value, 0, utf8_length) != NULL) {
+    OPENSSL_free(utf8_value);
+    return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
+  }
+
+  int isdnsname = 0;
+
+  // XXX: Deviation from strict DNS name syntax, also check names with '_'
+  // Check DNS name syntax, any '-' or '.' must be internal,
+  // and on either side of each '.' we can't have a '-' or '.'.
+  //
+  // If the name has just one label, we don't consider it a DNS name.  This
+  // means that "CN=sometld" cannot be precluded by DNS name constraints, but
+  // that is not a problem.
+  for (int i = 0; i < utf8_length; ++i) {
+    const unsigned char c = utf8_value[i];
+
+    if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') ||
+        (c >= '0' && c <= '9') || c == '_') {
+      continue;
+    }
+
+    // Dot and hyphen cannot be first or last.
+    if (i > 0 && i < utf8_length - 1) {
+      if (c == '-') {
+        continue;
+      }
+
+      // Next to a dot the preceding and following characters must not be
+      // another dot or a hyphen.  Otherwise, record that the name is
+      // plausible, since it has two or more labels.
+      if (c == '.' && utf8_value[i + 1] != '.' && utf8_value[i - 1] != '-' &&
+          utf8_value[i + 1] != '-') {
+        isdnsname = 1;
+        continue;
+      }
+    }
+    isdnsname = 0;
+    break;
+  }
+
+  if (isdnsname) {
+    *dnsid = utf8_value;
+    *idlen = (size_t)utf8_length;
+    return X509_V_OK;
+  }
+  OPENSSL_free(utf8_value);
+  return X509_V_OK;
+}
+
+// Check CN against DNS-ID name constraints.
+int NAME_CONSTRAINTS_check_CN(X509 *x, NAME_CONSTRAINTS *nc) {
+  int ret;
+  const X509_NAME *nm = X509_get_subject_name(x);
+  ASN1_STRING stmp = {.length = 0, .type = V_ASN1_IA5STRING, .data = NULL, .flags = 0};
+  GENERAL_NAME gntmp = {.type = GEN_DNS, .d = {.dNSName = &stmp}};
+
+  // Process any commonName attributes in subject name
+  for (int i = -1;;) {
+    X509_NAME_ENTRY *ne;
+    ASN1_STRING *cn;
+    unsigned char *idval;
+    size_t idlen;
+
+    i = X509_NAME_get_index_by_NID(nm, NID_commonName, i);
+    if (i == -1) {
+      break;
+    }
+    ne = X509_NAME_get_entry(nm, i);
+    cn = X509_NAME_ENTRY_get_data(ne);
+
+    // Only process attributes that look like host names
+    if ((ret = cn2dnsid(cn, &idval, &idlen)) != X509_V_OK) {
+      return ret;
+    }
+    if (idlen == 0) {
+      continue;
+    }
+
+    stmp.length = idlen;
+    stmp.data = idval;
+    ret = nc_match(&gntmp, nc);
+    OPENSSL_free(idval);
+    if (ret != X509_V_OK) {
+      return ret;
+    }
+  }
+  return X509_V_OK;
+}
+
 static int nc_match(GENERAL_NAME *gen, NAME_CONSTRAINTS *nc) {
   GENERAL_SUBTREE *sub;
   int r, match = 0;

crypto/x509/v3_utl.c

@@ -880,56 +880,13 @@ static int equal_wildcard(const unsigned char *pattern, size_t pattern_len,
                         subject_len, flags);
 }
 
-int x509v3_looks_like_dns_name(const unsigned char *in, size_t len) {
-  // This function is used as a heuristic for whether a common name is a
-  // hostname to be matched, or merely a decorative name to describe the
-  // subject. This heuristic must be applied to both name constraints and the
-  // common name fallback, so it must be loose enough to accept hostname
-  // common names, and tight enough to reject decorative common names.
-
-  if (len > 0 && in[len - 1] == '.') {
-    len--;
-  }
-
-  // Wildcards are allowed in front.
-  if (len >= 2 && in[0] == '*' && in[1] == '.') {
-    in += 2;
-    len -= 2;
-  }
-
-  if (len == 0) {
-    return 0;
-  }
-
-  size_t label_start = 0;
-  for (size_t i = 0; i < len; i++) {
-    unsigned char c = in[i];
-    if (OPENSSL_isalnum(c) || (c == '-' && i > label_start) ||
-        // These are not valid characters in hostnames, but commonly found
-        // in deployments outside the Web PKI.
-        c == '_' || c == ':') {
-      continue;
-    }
-
-    // Labels must not be empty.
-    if (c == '.' && i > label_start && i < len - 1) {
-      label_start = i + 1;
-      continue;
-    }
-
-    return 0;
-  }
-
-  return 1;
-}
-
 // Compare an ASN1_STRING to a supplied string. If they match return 1. If
 // cmp_type > 0 only compare if string matches the type, otherwise convert it
 // to UTF8.
 
 static int do_check_string(const ASN1_STRING *a, int cmp_type, equal_fn equal,
-                           unsigned int flags, int check_type, const char *b,
-                           size_t blen, char **peername) {
+                           unsigned int flags, const char *b, size_t blen,
+                           char **peername) {
   int rv = 0;
 
   if (!a->data || !a->length) {
@@ -947,7 +904,7 @@ static int do_check_string(const ASN1_STRING *a, int cmp_type, equal_fn equal,
     if (rv > 0 && peername) {
       *peername = OPENSSL_strndup((char *)a->data, a->length);
       if (*peername == NULL) {
-        return -1;
+        rv = -1;
       }
     }
   } else {
@@ -957,18 +914,11 @@ static int do_check_string(const ASN1_STRING *a, int cmp_type, equal_fn equal,
     if (astrlen < 0) {
       return -1;
     }
-    // We check the common name against DNS name constraints if it passes
-    // |x509v3_looks_like_dns_name|. Thus we must not consider common names
-    // for DNS fallbacks if they fail this check.
-    if (check_type == GEN_DNS && !x509v3_looks_like_dns_name(astr, astrlen)) {
-      rv = 0;
-    } else {
-      rv = equal(astr, astrlen, (unsigned char *)b, blen, flags);
-    }
+    rv = equal(astr, astrlen, (unsigned char *)b, blen, flags);
     if (rv > 0 && peername) {
       *peername = OPENSSL_strndup((char *)astr, astrlen);
       if (*peername == NULL) {
-        return -1;
+        rv = -1;
       }
     }
     OPENSSL_free(astr);
@@ -1001,11 +951,13 @@ static int do_x509_check(const X509 *x, const char *chk, size_t chklen,
 
   GENERAL_NAMES *gens = X509_get_ext_d2i(x, NID_subject_alt_name, NULL, NULL);
   if (gens) {
+    int san_present = 0;
     for (size_t i = 0; i < sk_GENERAL_NAME_num(gens); i++) {
       const GENERAL_NAME *gen = sk_GENERAL_NAME_value(gens, i);
       if (gen->type != check_type) {
         continue;
       }
+      san_present = 1;
       const ASN1_STRING *cstr;
       if (check_type == GEN_EMAIL) {
         cstr = gen->d.rfc822Name;
@@ -1015,13 +967,18 @@ static int do_x509_check(const X509 *x, const char *chk, size_t chklen,
         cstr = gen->d.iPAddress;
       }
       // Positive on success, negative on error!
-      if ((rv = do_check_string(cstr, alt_type, equal, flags, check_type, chk,
-                                chklen, peername)) != 0) {
+      if ((rv = do_check_string(cstr, alt_type, equal, flags,
+                                chk, chklen, peername)) != 0) {
         break;
       }
     }
     GENERAL_NAMES_free(gens);
-    return rv;
+    if(rv != 0) {
+      return rv;
+    }
+    if(san_present && !(flags & X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT)) {
+      return 0;
+    }
   }
 
   // We're done if CN-ID is not pertinent
@@ -1035,8 +992,8 @@ static int do_x509_check(const X509 *x, const char *chk, size_t chklen,
     const X509_NAME_ENTRY *ne = X509_NAME_get_entry(name, j);
     const ASN1_STRING *str = X509_NAME_ENTRY_get_data(ne);
     // Positive on success, negative on error!
-    if ((rv = do_check_string(str, -1, equal, flags, check_type, chk, chklen,
-                              peername)) != 0) {
+    if ((rv = do_check_string(str, -1, equal, flags,
+                              chk, chklen, peername)) != 0) {
       return rv;
     }
   }