Limit scope of CN name constraints

Don't apply DNS name constraints to the subject CN when there's a
least one DNS-ID subjectAlternativeName.

Don't apply DNS name constraints to subject CN's that are sufficiently
unlike DNS names.  Checked name must have at least two labels, with
all labels non-empty, no trailing '.' and all hyphens must be
internal in each label.  In addition to the usual LDH characters,
we also allow "_", since some sites use these for hostnames despite
all the standards.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>

1 files changed, 125 insertions, 18 deletions

diff --git a/crypto/x509v3/v3_ncons.c b/crypto/x509v3/v3_ncons.c
index 1f1996b..77edcd4 100644
--- a/crypto/x509v3/v3_ncons.c
+++ b/crypto/x509v3/v3_ncons.c
@@ -297,48 +297,155 @@ int NAME_CONSTRAINTS_check(X509 *x, NAME_CONSTRAINTS *nc)
 
 }
 
+static int cn2dnsid(ASN1_STRING *cn, unsigned char **dnsid, size_t *idlen)
+{
+    int utf8_length;    /* Return type of ASN1_STRING_to_UTF8 */
+    int i;
+    unsigned char *utf8_value;
+    int isdnsname = 0;
+
+    /* 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'.
+     */
+    if ((utf8_length = ASN1_STRING_to_UTF8(&utf8_value, cn)) < 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 ((size_t)utf8_length != strlen((char *)utf8_value))
+        return X509_V_ERR_UNSPECIFIED;
+
+    /*
+     * 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 (i = 0; i < utf8_length; ++i) {
+        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, provided no DNS-ID
+ * subjectAlternativeName values are present in the certificate.
+ */
 int NAME_CONSTRAINTS_check_CN(X509 *x, NAME_CONSTRAINTS *nc)
 {
     int r, i;
+    GENERAL_NAMES *gens = NULL;
     X509_NAME *nm;
-
     ASN1_STRING stmp;
     GENERAL_NAME gntmp;
+
     stmp.flags = 0;
     stmp.type = V_ASN1_IA5STRING;
     gntmp.type = GEN_DNS;
     gntmp.d.dNSName = &stmp;
 
+    gens = X509_get_ext_d2i(x, NID_subject_alt_name, NULL, NULL);
+    if (gens != NULL) {
+        for (i = 0; i < sk_GENERAL_NAME_num(gens); i++) {
+            GENERAL_NAME *gen = sk_GENERAL_NAME_value(gens, i);
+
+            if (gen->type == GEN_DNS) {
+                GENERAL_NAMES_free(gens);
+                return X509_V_OK;
+            }
+        }
+        GENERAL_NAMES_free(gens);
+    }
+
     nm = X509_get_subject_name(x);
 
     /* Process any commonName attributes in subject name */
 
     for (i = -1;;) {
         X509_NAME_ENTRY *ne;
-        ASN1_STRING *hn;
+        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);
-        hn = X509_NAME_ENTRY_get_data(ne);
-        /* Only process attributes that look like host names */
-        if (asn1_valid_host(hn)) {
-            unsigned char *h;
-            int hlen = ASN1_STRING_to_UTF8(&h, hn);
-            if (hlen <= 0)
-                return X509_V_ERR_OUT_OF_MEM;
+        cn = X509_NAME_ENTRY_get_data(ne);
 
-            stmp.length = hlen;
-            stmp.data = h;
-
-            r = nc_match(&gntmp, nc);
-
-            OPENSSL_free(h);
+        /* Only process attributes that look like host names */
+        if ((r = cn2dnsid(cn, &idval, &idlen)) != X509_V_OK)
+            return r;
+        if (idlen == 0)
+            continue;
 
-            if (r != X509_V_OK)
-                    return r;
-        }
+        stmp.length = idlen;
+        stmp.data = idval;
+        r = nc_match(&gntmp, nc);
+        OPENSSL_free(idval);
+        if (r != X509_V_OK)
+            return r;
     }
     return X509_V_OK;
 }