mod_ssl: add support for subjectAltName-based host name checking in proxy mode

(PR 54030)

factor out code from ssl_engine_init.c:ssl_check_public_cert()
to ssl_util_ssl.c:SSL_X509_match_name()

introduce new SSLProxyCheckPeerName directive, which should eventually
obsolete SSLProxyCheckPeerCN

ssl_engine_io.c:ssl_io_filter_handshake(): avoid code duplication
when aborting with HTTP_BAD_GATEWAY


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1425874 13f79535-47bb-0310-9956-ffa450edef68

10 files changed, 172 insertions, 70 deletions

diff --git a/CHANGES b/CHANGES
index 27d061975a..d15d466746 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_ssl: add support for subjectAltName-based host name checking
+     in proxy mode. PR 54030. [Kaspar Brand]
+
   *) mpm_event: Check that AsyncRequestWorkerFactor is not negative. PR 54254.
      [Jackie Zhang <jackie qq zhang gmail com>]
 
diff --git a/docs/log-message-tags/next-number b/docs/log-message-tags/next-number
index 7cce2de386..49ce078f63 100644
--- a/docs/log-message-tags/next-number
+++ b/docs/log-message-tags/next-number
@@ -1 +1 @@
-2411
+2413
diff --git a/docs/manual/mod/mod_ssl.xml b/docs/manual/mod/mod_ssl.xml
index 72b7ef99e2..9697b26ee8 100644
--- a/docs/manual/mod/mod_ssl.xml
+++ b/docs/manual/mod/mod_ssl.xml
@@ -1750,7 +1750,7 @@ SSLProxyCheckPeerExpire on
 
 <directivesynopsis>
 <name>SSLProxyCheckPeerCN</name>
-<description>Whether to check the remote server certificates CN field
+<description>Whether to check the remote server certificate's CN field
 </description>
 <syntax>SSLProxyCheckPeerCN on|off</syntax>
 <default>SSLProxyCheckPeerCN on</default>
@@ -1759,10 +1759,16 @@ SSLProxyCheckPeerExpire on
 
 <usage>
 <p>
-This directive sets whether the remote server certificates CN field is
+This directive sets whether the remote server certificate's CN field is
 compared against the hostname of the request URL. If both are not equal
 a 502 status code (Bad Gateway) is sent.
 </p>
+<p>
+SSLProxyCheckPeerCN has been superseded by
+<directive module="mod_ssl">SSLProxyCheckPeerName</directive>, and its
+setting is only taken into account when
+<code>SSLProxyCheckPeerName off</code> is specified at the same time.
+</p>
 <example><title>Example</title>
 <highlight language="config">
 SSLProxyCheckPeerCN on
@@ -1772,6 +1778,36 @@ SSLProxyCheckPeerCN on
 </directivesynopsis>
 
 <directivesynopsis>
+<name>SSLProxyCheckPeerName</name>
+<description>Configure host name checking for remote server certificates
+</description>
+<syntax>SSLProxyCheckPeerName on|off</syntax>
+<default>SSLProxyCheckPeerName on</default>
+<contextlist><context>server config</context>
+<context>virtual host</context></contextlist>
+
+<usage>
+<p>
+This directive configures host name checking for server certificates
+when mod_ssl is acting as an SSL client. The check will
+succeed if the host name from the request URI is found in
+either the subjectAltName extension or (one of) the CN attribute(s)
+in the certificate's subject. If the check fails, the SSL request
+is aborted and a 502 status code (Bad Gateway) is returned.
+The directive supersedes <directive module="mod_ssl">SSLProxyCheckPeerCN</directive>,
+which only checks for the expected host name in the first CN attribute.
+</p>
+<p>
+Wildcard matching is supported in one specific flavor: subjectAltName entries
+of type dNSName or CN attributes starting with <code>*.</code> will match
+for any DNS name with the same number of labels and the same suffix
+(i.e., <code>*.example.org</code> matches for <code>foo.example.org</code>,
+but not for <code>foo.bar.example.org</code>).
+</p>
+</usage>
+</directivesynopsis>
+
+<directivesynopsis>
 <name>SSLProxyEngine</name>
 <description>SSL Proxy Engine Operation Switch</description>
 <syntax>SSLProxyEngine on|off</syntax>
diff --git a/modules/ssl/mod_ssl.c b/modules/ssl/mod_ssl.c
index 2e50dc979e..a6392d1d47 100644
--- a/modules/ssl/mod_ssl.c
+++ b/modules/ssl/mod_ssl.c
@@ -209,9 +209,12 @@ static const command_rec ssl_config_cmds[] = {
                "of the client certificate "
                "(`/path/to/file' - PEM encoded certificates)")
     SSL_CMD_SRV(ProxyCheckPeerExpire, FLAG,
-                "SSL Proxy: check the peers certificate expiration date")
+                "SSL Proxy: check the peer certificate's expiration date")
     SSL_CMD_SRV(ProxyCheckPeerCN, FLAG,
-                "SSL Proxy: check the peers certificate CN")
+                "SSL Proxy: check the peer certificate's CN")
+    SSL_CMD_SRV(ProxyCheckPeerName, FLAG,
+                "SSL Proxy: check the peer certificate's name "
+                "(must be present in subjectAltName extension or CN")
 
     /*
      * Per-directory context configuration directives
diff --git a/modules/ssl/ssl_engine_config.c b/modules/ssl/ssl_engine_config.c
index bda5c0e4e4..20c46daeb7 100644
--- a/modules/ssl/ssl_engine_config.c
+++ b/modules/ssl/ssl_engine_config.c
@@ -214,6 +214,7 @@ static SSLSrvConfigRec *ssl_config_server_new(apr_pool_t *p)
     sc->insecure_reneg         = UNSET;
     sc->proxy_ssl_check_peer_expire = SSL_ENABLED_UNSET;
     sc->proxy_ssl_check_peer_cn     = SSL_ENABLED_UNSET;
+    sc->proxy_ssl_check_peer_name   = SSL_ENABLED_UNSET;
 #ifndef OPENSSL_NO_TLSEXT
     sc->strict_sni_vhost_check = SSL_ENABLED_UNSET;
 #endif
@@ -352,6 +353,7 @@ void *ssl_config_server_merge(apr_pool_t *p, void *basev, void *addv)
     cfgMergeBool(insecure_reneg);
     cfgMerge(proxy_ssl_check_peer_expire, SSL_ENABLED_UNSET);
     cfgMerge(proxy_ssl_check_peer_cn, SSL_ENABLED_UNSET);
+    cfgMerge(proxy_ssl_check_peer_name, SSL_ENABLED_UNSET);
 #ifndef OPENSSL_NO_TLSEXT
     cfgMerge(strict_sni_vhost_check, SSL_ENABLED_UNSET);
 #endif
@@ -1696,6 +1698,15 @@ const char *ssl_cmd_SSLProxyCheckPeerCN(cmd_parms *cmd, void *dcfg, int flag)
     return NULL;
 }
 
+const char *ssl_cmd_SSLProxyCheckPeerName(cmd_parms *cmd, void *dcfg, int flag)
+{
+    SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
+
+    sc->proxy_ssl_check_peer_name = flag ? SSL_ENABLED_TRUE : SSL_ENABLED_FALSE;
+
+    return NULL;
+}
+
 const char  *ssl_cmd_SSLStrictSNIVHostCheck(cmd_parms *cmd, void *dcfg, int flag)
 {
 #ifndef OPENSSL_NO_TLSEXT
diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c
index 817f6fd88c..be3212defa 100644
--- a/modules/ssl/ssl_engine_init.c
+++ b/modules/ssl/ssl_engine_init.c
@@ -1154,7 +1154,6 @@ static void ssl_check_public_cert(server_rec *s,
                                   int type)
 {
     int is_ca, pathlen;
-    apr_array_header_t *ids;
 
     if (!cert) {
         return;
@@ -1187,56 +1186,12 @@ static void ssl_check_public_cert(server_rec *s,
         }
     }
 
-    /*
-     * 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, APLOGNO(01908)
-                             "%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;
-            }
-        }
-
-        if (matched == FALSE) {
-            ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, APLOGNO(01909)
-                         "%s certificate configured for %s does NOT include "
-                         "an ID which matches the server name",
-                         ssl_asn1_keystr(type), (mySrvConfig(s))->vhost_id);
-        }
+    if (SSL_X509_match_name(ptemp, cert, (const char *)s->server_hostname,
+                            TRUE, s) == FALSE) {
+        ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, APLOGNO(01909)
+                     "%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_engine_io.c b/modules/ssl/ssl_engine_io.c
index 16f56d3e38..a4e76ca31b 100644
--- a/modules/ssl/ssl_engine_io.c
+++ b/modules/ssl/ssl_engine_io.c
@@ -1057,6 +1057,7 @@ static apr_status_t ssl_io_filter_handshake(ssl_filter_ctx_t *filter_ctx)
 #endif
         const char *hostname_note = apr_table_get(c->notes,
                                                   "proxy-request-hostname");
+        BOOL proxy_ssl_check_peer_ok = TRUE;
         sc = mySrvConfig(server);
 
 #ifndef OPENSSL_NO_TLSEXT
@@ -1094,26 +1095,32 @@ static apr_status_t ssl_io_filter_handshake(ssl_filter_ctx_t *filter_ctx)
             return MODSSL_ERROR_BAD_GATEWAY;
         }
 
+        cert = SSL_get_peer_certificate(filter_ctx->pssl);
+
         if (sc->proxy_ssl_check_peer_expire != SSL_ENABLED_FALSE) {
-            cert = SSL_get_peer_certificate(filter_ctx->pssl);
             if (!cert
                 || (X509_cmp_current_time(
                      X509_get_notBefore(cert)) >= 0)
                 || (X509_cmp_current_time(
                      X509_get_notAfter(cert)) <= 0)) {
+                proxy_ssl_check_peer_ok = FALSE;
                 ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, c, APLOGNO(02004)
                               "SSL Proxy: Peer certificate is expired");
-                if (cert) {
-                    X509_free(cert);
-                }
-                /* ensure that the SSL structures etc are freed, etc: */
-                ssl_filter_io_shutdown(filter_ctx, c, 1);
-                apr_table_setn(c->notes, "SSL_connect_rv", "err");
-                return HTTP_BAD_GATEWAY;
             }
-            X509_free(cert);
         }
-        if ((sc->proxy_ssl_check_peer_cn != SSL_ENABLED_FALSE) &&
+        if ((sc->proxy_ssl_check_peer_name != SSL_ENABLED_FALSE) &&
+            hostname_note) {
+            apr_table_unset(c->notes, "proxy-request-hostname");
+            if (!cert
+                || SSL_X509_match_name(c->pool, cert, hostname_note,
+                                       TRUE, server) == FALSE) {
+                proxy_ssl_check_peer_ok = FALSE;
+                ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, c, APLOGNO(02411)
+                              "SSL Proxy: Peer certificate does not match "
+                              "for hostname %s", hostname_note);
+            }
+        }
+        else if ((sc->proxy_ssl_check_peer_cn != SSL_ENABLED_FALSE) &&
             hostname_note) {
             const char *hostname;
             int match = 0;
@@ -1132,17 +1139,25 @@ static apr_status_t ssl_io_filter_handshake(ssl_filter_ctx_t *filter_ctx)
             }
 
             if (!match) {
+                proxy_ssl_check_peer_ok = FALSE;
                 ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, c, APLOGNO(02005)
                               "SSL Proxy: Peer certificate CN mismatch:"
                               " Certificate CN: %s Requested hostname: %s",
                               hostname, hostname_note);
-                /* ensure that the SSL structures etc are freed, etc: */
-                ssl_filter_io_shutdown(filter_ctx, c, 1);
-                apr_table_setn(c->notes, "SSL_connect_rv", "err");
-                return HTTP_BAD_GATEWAY;
             }
         }
 
+        if (cert) {
+            X509_free(cert);
+        }
+
+        if (proxy_ssl_check_peer_ok != TRUE) {
+            /* ensure that the SSL structures etc are freed, etc: */
+            ssl_filter_io_shutdown(filter_ctx, c, 1);
+            apr_table_setn(c->notes, "SSL_connect_rv", "err");
+            return HTTP_BAD_GATEWAY;
+        }
+
         apr_table_setn(c->notes, "SSL_connect_rv", "ok");
         return APR_SUCCESS;
     }
diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h
index 16e0bcd499..3ff3014bba 100644
--- a/modules/ssl/ssl_private.h
+++ b/modules/ssl/ssl_private.h
@@ -710,6 +710,7 @@ struct SSLSrvConfigRec {
     modssl_ctx_t    *proxy;
     ssl_enabled_t    proxy_ssl_check_peer_expire;
     ssl_enabled_t    proxy_ssl_check_peer_cn;
+    ssl_enabled_t    proxy_ssl_check_peer_name;
 #ifndef OPENSSL_NO_TLSEXT
     ssl_enabled_t    strict_sni_vhost_check;
 #endif
@@ -808,6 +809,7 @@ const char *ssl_cmd_SSLSessionTicketKeyFile(cmd_parms *cmd, void *dcfg, const ch
 #endif
 const char  *ssl_cmd_SSLProxyCheckPeerExpire(cmd_parms *cmd, void *dcfg, int flag);
 const char  *ssl_cmd_SSLProxyCheckPeerCN(cmd_parms *cmd, void *dcfg, int flag);
+const char  *ssl_cmd_SSLProxyCheckPeerName(cmd_parms *cmd, void *dcfg, int flag);
 
 const char *ssl_cmd_SSLOCSPOverrideResponder(cmd_parms *cmd, void *dcfg, int flag);
 const char *ssl_cmd_SSLOCSPDefaultResponder(cmd_parms *cmd, void *dcfg, const char *arg);
diff --git a/modules/ssl/ssl_util_ssl.c b/modules/ssl/ssl_util_ssl.c
index 48b561dc3b..6cdbdc1b18 100644
--- a/modules/ssl/ssl_util_ssl.c
+++ b/modules/ssl/ssl_util_ssl.c
@@ -338,6 +338,82 @@ BOOL SSL_X509_getIDs(apr_pool_t *p, X509 *x509, apr_array_header_t **ids)
     return apr_is_empty_array(*ids) ? FALSE : TRUE;
 }
 
+/* 
+ * Check if a certificate matches for a particular name, by iterating over its
+ * DNS-IDs and CN-IDs (RFC 6125), optionally with basic wildcard matching.
+ * If server_rec is non-NULL, some (debug/trace) logging is enabled.
+ */
+BOOL SSL_X509_match_name(apr_pool_t *p, X509 *x509, const char *name,
+                         BOOL allow_wildcard, server_rec *s)
+{
+    BOOL matched = FALSE;
+    apr_array_header_t *ids;
+
+    /*
+     * At some day in the future, this might be replaced with X509_check_host()
+     * (available in OpenSSL 1.0.2 and later), but two points should be noted:
+     * 1) wildcard matching in X509_check_host() might yield different
+     *    results (by default, it supports a broader set of patterns, e.g.
+     *    wildcards in non-initial positions);
+     * 2) we lose the option of logging each DNS- and CN-ID (until a match
+     *    is found).
+     */
+
+    if (SSL_X509_getIDs(p, x509, &ids)) {
+        char *cp;
+        int i;
+        char **id = (char **)ids->elts;
+        BOOL is_wildcard;
+
+        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 (and the caller is
+             * allowing wildcards), 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.
+             */
+            if ((allow_wildcard == TRUE && is_wildcard == TRUE &&
+                 (cp = strchr(name, '.')) && !strcasecmp(id[i]+1, cp)) ||
+                !strcasecmp(id[i], name)) {
+                matched = TRUE;
+            }
+
+            if (s) {
+                ap_log_error(APLOG_MARK, APLOG_TRACE3, 0, s,
+                             "[%s] SSL_X509_match_name: expecting name '%s', "
+                             "%smatched by ID '%s'",
+                             (mySrvConfig(s))->vhost_id, name,
+                             matched == TRUE ? "" : "NOT ", id[i]);
+            }
+
+            if (matched == TRUE) {
+                break;
+            }
+        }
+
+    }
+
+    if (s) {
+        ssl_log_xerror(SSLLOG_MARK, APLOG_DEBUG, 0, p, s, x509,
+                       APLOGNO(02412) "[%s] Cert %s for name '%s'",
+                       (mySrvConfig(s))->vhost_id,
+                       matched == TRUE ? "matches" : "does not match",
+                       name);
+    }
+
+    return matched;
+}
+
 /*  _________________________________________________________________
 **
 **  Low-Level CA Certificate Loading
diff --git a/modules/ssl/ssl_util_ssl.h b/modules/ssl/ssl_util_ssl.h
index 1688bb6706..4b882db289 100644
--- a/modules/ssl/ssl_util_ssl.h
+++ b/modules/ssl/ssl_util_ssl.h
@@ -68,6 +68,7 @@ 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 *, int);
 BOOL        SSL_X509_getIDs(apr_pool_t *, X509 *, apr_array_header_t **);
+BOOL        SSL_X509_match_name(apr_pool_t *, X509 *, const char *, BOOL, server_rec *);
 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 *);