diff options
author | Viktor Dukhovni <openssl-users@dukhovni.org> | 2018-05-15 23:41:20 -0400 |
---|---|---|
committer | Viktor Dukhovni <openssl-users@dukhovni.org> | 2018-05-23 11:12:13 -0400 |
commit | d02d80b2e80adfdde49f76cf7c7af4e013f45005 (patch) | |
tree | e9e137e02f0751435765ff251b07d58f710213e0 /crypto | |
parent | de9f5b3554274e27949941cbe74a07c8a5f25dbf (diff) |
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>
Diffstat (limited to 'crypto')
-rw-r--r-- | crypto/asn1/a_strex.c | 50 | ||||
-rw-r--r-- | crypto/include/internal/asn1_int.h | 1 | ||||
-rw-r--r-- | crypto/x509v3/v3_ncons.c | 143 |
3 files changed, 125 insertions, 69 deletions
diff --git a/crypto/asn1/a_strex.c b/crypto/asn1/a_strex.c index 6a67bc8c33..ea4dd1c5b1 100644 --- a/crypto/asn1/a_strex.c +++ b/crypto/asn1/a_strex.c @@ -624,53 +624,3 @@ int ASN1_STRING_to_UTF8(unsigned char **out, const ASN1_STRING *in) *out = stmp.data; return stmp.length; } - -/* Return 1 if host is a valid hostname and 0 otherwise */ -int asn1_valid_host(const ASN1_STRING *host) -{ - int hostlen = host->length; - const unsigned char *hostptr = host->data; - int type = host->type; - int i; - signed char width = -1; - unsigned short chflags = 0, prevchflags; - - if (type > 0 && type < 31) - width = tag2nbyte[type]; - if (width == -1 || hostlen == 0) - return 0; - /* Treat UTF8String as width 1 as any MSB set is invalid */ - if (width == 0) - width = 1; - for (i = 0 ; i < hostlen; i+= width) { - prevchflags = chflags; - /* Value must be <= 0x7F: check upper bytes are all zeroes */ - if (width == 4) { - if (*hostptr++ != 0 || *hostptr++ != 0 || *hostptr++ != 0) - return 0; - } else if (width == 2) { - if (*hostptr++ != 0) - return 0; - } - if (*hostptr > 0x7f) - return 0; - chflags = char_type[*hostptr++]; - if (!(chflags & (CHARTYPE_HOST_ANY | CHARTYPE_HOST_WILD))) { - /* Nothing else allowed at start or end of string */ - if (i == 0 || i == hostlen - 1) - return 0; - /* Otherwise invalid if not dot or hyphen */ - if (!(chflags & (CHARTYPE_HOST_DOT | CHARTYPE_HOST_HYPHEN))) - return 0; - /* - * If previous is dot or hyphen then illegal unless both - * are hyphens: as .- -. .. are all illegal - */ - if (prevchflags & (CHARTYPE_HOST_DOT | CHARTYPE_HOST_HYPHEN) - && ((prevchflags & CHARTYPE_HOST_DOT) - || (chflags & CHARTYPE_HOST_DOT))) - return 0; - } - } - return 1; -} diff --git a/crypto/include/internal/asn1_int.h b/crypto/include/internal/asn1_int.h index fdd5f1e648..962c3c6302 100644 --- a/crypto/include/internal/asn1_int.h +++ b/crypto/include/internal/asn1_int.h @@ -107,5 +107,4 @@ struct asn1_pctx_st { unsigned long str_flags; } /* ASN1_PCTX */ ; -int asn1_valid_host(const ASN1_STRING *host); int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb); diff --git a/crypto/x509v3/v3_ncons.c b/crypto/x509v3/v3_ncons.c index 1f1996b68e..77edcd4dfb 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; } |