From 2160d8594c2806d2ed71ecfa2d78b9fe6578474c Mon Sep 17 00:00:00 2001 From: Kaspar Brand Date: Wed, 28 Sep 2011 06:52:39 +0000 Subject: [PATCH] In ssl_check_public_cert(), also take dNSNames in the subjectAltName extension into account when checking the cert against the configured ServerName. PR 32652, PR 47051. Replace SSL_X509_getCN() by SSL_X509_getIDs(), which returns an array of a cert's DNS-IDs and CN-IDs (terms as coined by RFC 6125). git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1176752 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 4 +++ modules/ssl/ssl_engine_init.c | 62 ++++++++++++++++++++++++++--------- modules/ssl/ssl_util_ssl.c | 61 +++++++++++++++++++++++++--------- modules/ssl/ssl_util_ssl.h | 2 +- 4 files changed, 97 insertions(+), 32 deletions(-) diff --git a/CHANGES b/CHANGES index e734d8b215..fcf94e4395 100644 --- a/CHANGES +++ b/CHANGES @@ -12,6 +12,10 @@ Changes with Apache 2.3.15 PR 51714. [Stefan Fritsch, Jim Jagielski, Ruediger Pluem, Eric Covener, ] + *) mod_ssl: At startup, when checking a server certificate whether it + matches the configured ServerName, also take dNSName entries in the + subjectAltName extension into account. PR 32652, PR 47051. [Kaspar Brand] + *) mod_substitute: Reduce memory usage and copying of data. PR 50559. [Stefan Fritsch] diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index 341ee43431..a829db3727 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/modules/ssl/ssl_engine_init.c @@ -985,7 +985,7 @@ static void ssl_check_public_cert(server_rec *s, int type) { int is_ca, pathlen; - char *cn; + apr_array_header_t *ids; if (!cert) { return; @@ -1018,23 +1018,55 @@ static void ssl_check_public_cert(server_rec *s, } } - if (SSL_X509_getCN(ptemp, cert, &cn)) { - int fnm_flags = APR_FNM_PERIOD|APR_FNM_CASE_BLIND; - - if (apr_fnmatch_test(cn)) { - if (apr_fnmatch(cn, s->server_hostname, - fnm_flags) == APR_FNM_NOMATCH) { - ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, - "%s server certificate wildcard CommonName " - "(CN) `%s' does NOT match server name!?", - ssl_asn1_keystr(type), cn); + /* + * Check if the server name is covered by the certificate. + * Consider both dNSName entries in the subjectAltName extension + * and, as a fallback, commonName attributes in the subject DN. + * (DNS-IDs and CN-IDs as defined in RFC 6125). + */ + if (SSL_X509_getIDs(ptemp, cert, &ids)) { + char *cp; + int i; + char **id = (char **)ids->elts; + BOOL is_wildcard, matched = FALSE; + + for (i = 0; i < ids->nelts; i++) { + if (!id[i]) + continue; + + /* + * Determine if it is a wildcard ID - we're restrictive + * in the sense that we require the wildcard character to be + * THE left-most label (i.e., the ID must start with "*.") + */ + is_wildcard = (*id[i] == '*' && *(id[i]+1) == '.') ? TRUE : FALSE; + + /* + * If the ID includes a wildcard character, check if it matches + * for the left-most DNS label (i.e., the wildcard character + * is not allowed to match a dot). Otherwise, try a simple + * string compare, case insensitively. + */ + if ((is_wildcard == TRUE && + (cp = strchr(s->server_hostname, '.')) && + !strcasecmp(id[i]+1, cp)) || + !strcasecmp(id[i], s->server_hostname)) { + matched = TRUE; + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%sID '%s' in %s certificate configured " + "for %s matches server name", + is_wildcard ? "Wildcard " : "", + id[i], ssl_asn1_keystr(type), + (mySrvConfig(s))->vhost_id); + break; } } - else if (strNE(s->server_hostname, cn)) { + + if (matched == FALSE) { ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, - "%s server certificate CommonName (CN) `%s' " - "does NOT match server name!?", - ssl_asn1_keystr(type), cn); + "%s certificate configured for %s does NOT include " + "an ID which matches the server name", + ssl_asn1_keystr(type), (mySrvConfig(s))->vhost_id); } } } diff --git a/modules/ssl/ssl_util_ssl.c b/modules/ssl/ssl_util_ssl.c index ee46fd87db..5022d0da38 100644 --- a/modules/ssl/ssl_util_ssl.c +++ b/modules/ssl/ssl_util_ssl.c @@ -329,25 +329,54 @@ char *SSL_X509_NAME_to_string(apr_pool_t *p, X509_NAME *dn, unsigned int maxlen) return result; } -/* retrieve subject CommonName of certificate */ -BOOL SSL_X509_getCN(apr_pool_t *p, X509 *xs, char **cppCN) +/* return an array of (RFC 6125 coined) DNS-IDs and CN-IDs in a certificate */ +BOOL SSL_X509_getIDs(apr_pool_t *p, X509 *x509, apr_array_header_t **ids) { - X509_NAME *xsn; - X509_NAME_ENTRY *xsne; - int i, nid; - - xsn = X509_get_subject_name(xs); - for (i = 0; i < sk_X509_NAME_ENTRY_num((STACK_OF(X509_NAME_ENTRY) *) - xsn->entries); i++) { - xsne = sk_X509_NAME_ENTRY_value((STACK_OF(X509_NAME_ENTRY) *) - xsn->entries, i); - nid = OBJ_obj2nid((ASN1_OBJECT *)X509_NAME_ENTRY_get_object(xsne)); - if (nid == NID_commonName) { - *cppCN = SSL_X509_NAME_ENTRY_to_string(p, xsne); - return TRUE; + STACK_OF(GENERAL_NAME) *names; + BIO *bio; + X509_NAME *subj; + char **cpp; + int i, n; + + if (!x509 || !(*ids = apr_array_make(p, 0, sizeof(char *)))) { + *ids = NULL; + return FALSE; + } + + /* First, the DNS-IDs (dNSName entries in the subjectAltName extension) */ + if ((names = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL, NULL)) && + (bio = BIO_new(BIO_s_mem()))) { + GENERAL_NAME *name; + + for (i = 0; i < sk_GENERAL_NAME_num(names); i++) { + name = sk_GENERAL_NAME_value(names, i); + if (name->type == GEN_DNS) { + ASN1_STRING_print_ex(bio, name->d.ia5, ASN1_STRFLGS_ESC_CTRL| + ASN1_STRFLGS_UTF8_CONVERT); + n = BIO_pending(bio); + if (n > 0) { + cpp = (char **)apr_array_push(*ids); + *cpp = apr_palloc(p, n+1); + n = BIO_read(bio, *cpp, n); + (*cpp)[n] = NUL; + } + } } + BIO_free(bio); } - return FALSE; + + if (names) + sk_GENERAL_NAME_free(names); + + /* Second, the CN-IDs (commonName attributes in the subject DN) */ + subj = X509_get_subject_name(x509); + i = -1; + while ((i = X509_NAME_get_index_by_NID(subj, NID_commonName, i)) != -1) { + cpp = (char **)apr_array_push(*ids); + *cpp = SSL_X509_NAME_ENTRY_to_string(p, X509_NAME_get_entry(subj, i)); + } + + return apr_is_empty_array(*ids) ? FALSE : TRUE; } /* _________________________________________________________________ diff --git a/modules/ssl/ssl_util_ssl.h b/modules/ssl/ssl_util_ssl.h index 59a98e68b8..db2a2e30fe 100644 --- a/modules/ssl/ssl_util_ssl.h +++ b/modules/ssl/ssl_util_ssl.h @@ -68,7 +68,7 @@ BOOL SSL_X509_isSGC(X509 *); BOOL SSL_X509_getBC(X509 *, int *, int *); char *SSL_X509_NAME_ENTRY_to_string(apr_pool_t *p, X509_NAME_ENTRY *xsne); char *SSL_X509_NAME_to_string(apr_pool_t *, X509_NAME *, unsigned int); -BOOL SSL_X509_getCN(apr_pool_t *, X509 *, char **); +BOOL SSL_X509_getIDs(apr_pool_t *, X509 *, apr_array_header_t **); BOOL SSL_X509_INFO_load_file(apr_pool_t *, STACK_OF(X509_INFO) *, const char *); BOOL SSL_X509_INFO_load_path(apr_pool_t *, STACK_OF(X509_INFO) *, const char *); int SSL_CTX_use_certificate_chain(SSL_CTX *, char *, int, pem_password_cb *); -- 2.49.0