From aac733f9cac1b1e80884a7ac184427fc696264a8 Mon Sep 17 00:00:00 2001
From: "William A. Rowe Jr" <wrowe@apache.org>
Date: Mon, 6 Jun 2016 20:23:24 +0000
Subject: [PATCH] Correct the behavior and interaction between
 SSLProxyCheckPeer[CN|Name], such that disabling either disables both, and
 that enabling either will trigger the more comprehensive
 SSLProxyCheckPeerName behavior.

Only a single configuration remains to enable the legacy behavior, which
is to explicitly disable SSLProxyCheckPeerName and enable SSLProxyCheckPeerCN.

Changes to the proxy config directives leads us to a different 2.4 fix...
https://github.com/wrowe/patches/blob/master/fix_proxy_check_peer-2.4.x.patch

Backports: 1746647
Reviewed by: wrowe, icing, rpluem



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1747069 13f79535-47bb-0310-9956-ffa450edef68
---
 CHANGES                     |  7 +++++
 STATUS                      | 10 -------
 docs/manual/mod/mod_ssl.xml | 60 ++++++++++++++++++++++++++-----------
 modules/ssl/ssl_engine_io.c |  4 ++-
 4 files changed, 52 insertions(+), 29 deletions(-)

diff --git a/CHANGES b/CHANGES
index 2f1663b859..75c02610d9 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,13 @@
 
 Changes with Apache 2.4.21
 
+  *) mod_ssl: Correct the interaction between SSLProxyCheckPeerCN and newer
+     SSLProxyCheckPeerName directives since release 2.4.5, such that disabling
+     either disables both, and that enabling either triggers the new, more
+     comprehensive SSLProxyCheckPeerName behavior. Only a single configuration
+     remains to enable the legacy behavior, which is to explicitly disable
+     SSLProxyCheckPeerName, and enable SSLProxyCheckPeerCN. [William Rowe]
+
   *) mod_include: add the <!--#comment ...> syntax in order to include comments
      in a SSI file. [Christophe Jaillet based on a suggestion from Rob]
 
diff --git a/STATUS b/STATUS
index bcda410943..7e28675c3d 100644
--- a/STATUS
+++ b/STATUS
@@ -115,16 +115,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
 
-  *) Correct the behavior and interaction between SSLProxyCheckPeer[CN|Name], 
-     such that disabling either disables both, and that enabling either will
-     trigger the more comprehensive SSLProxyCheckPeerName behavior.
-     Only a single configuration remains to enable the legacy behavior, which is
-     to explicitly disable SSLProxyCheckPeerName and enable SSLProxyCheckPeerCN.
-     2.4.x patch: https://github.com/wrowe/patches/blob/master/fix_proxy_check_peer-2.4.x.patch
-     icing: raw patch version https://raw.githubusercontent.com/wrowe/patches/master/fix_proxy_check_peer-2.4.x.patch
-     +1: wrowe, icing, rpluem
-
-
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]
 
diff --git a/docs/manual/mod/mod_ssl.xml b/docs/manual/mod/mod_ssl.xml
index 2927eea2d4..08348df192 100644
--- a/docs/manual/mod/mod_ssl.xml
+++ b/docs/manual/mod/mod_ssl.xml
@@ -1917,17 +1917,32 @@ SSLProxyCheckPeerExpire on
 <p>
 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.
+a 502 status code (Bad Gateway) is sent. <code>SSLProxyCheckPeerCN</code> is
+superseded by <directive module="mod_ssl">SSLProxyCheckPeerName</directive>
+in release 2.4.5 and later. 
 </p>
 <p>
-In 2.4.5 and later, 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.
+In all releases 2.4.5 through 2.4.20, setting
+<code>SSLProxyCheckPeerName off</code> was sufficient to enable this behavior
+(as the <code>SSLProxyCheckPeerCN</code> default was <code>on</code>.) In 
+these releases, both directives must be set to <code>off</code> to completely
+avoid remote server certificate name validation. Many users reported this
+to be very confusing.
+</p>
+<p>
+As of release 2.4.21, all configurations which enable either one of the
+<code>SSLProxyCheckPeerName</code> or <code>SSLProxyCheckPeerCN</code> options
+will use the new <directive module="mod_ssl">SSLProxyCheckPeerName</directive>
+behavior, and all configurations which disable either one of the 
+<code>SSLProxyCheckPeerName</code> or <code>SSLProxyCheckPeerCN</code> options
+will supress all remote server certificate name validation. Only the following
+configuration will trigger the legacy certificate CN comparison in 2.4.21 and
+later releases;
 </p>
 <example><title>Example</title>
 <highlight language="config">
 SSLProxyCheckPeerCN on
+SSLProxyCheckPeerName off
 </highlight>
 </example>
 </usage>
@@ -1945,21 +1960,30 @@ SSLProxyCheckPeerCN on
 
 <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.
+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 matches one of the CN attribute(s) of the certificate's 
+subject, or matches the subjectAltName extension. If the check fails, the SSL 
+request is aborted and a 502 status code (Bad Gateway) is returned.
+</p>
+<p>
+Wildcard matching is supported for specific cases: an subjectAltName entry
+of type dNSName, or CN attributes starting with <code>*.</code> will match
+with any host name of the same number of name elements and the same suffix.
+E.g. <code>*.example.org</code> will match <code>foo.example.org</code>,
+but will not match <code>foo.bar.example.org</code>, because the number of
+elements in the respective host names differs.
 </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>).
+This feature was introduced in 2.4.5 and superseded the behavior of the 
+<directive module="mod_ssl">SSLProxyCheckPeerCN</directive> directive, which 
+only tested the exact value in the first CN attribute against the host name.
+However, many users were confused by the behavior of using these directives
+individually, so the mutual behavior of <code>SSLProxyCheckPeerName</code> 
+and <code>SSLProxyCheckPeerCN</code> directives were improved in release 
+2.4.21. See the <directive module="mod_ssl">SSLProxyCheckPeerCN</directive> 
+directive description for the original behavior and details of these 
+improvements.
 </p>
 </usage>
 </directivesynopsis>
diff --git a/modules/ssl/ssl_engine_io.c b/modules/ssl/ssl_engine_io.c
index 0737640ada..ea2319328d 100644
--- a/modules/ssl/ssl_engine_io.c
+++ b/modules/ssl/ssl_engine_io.c
@@ -1189,6 +1189,8 @@ static apr_status_t ssl_io_filter_handshake(ssl_filter_ctx_t *filter_ctx)
             }
         }
         if ((sc->proxy_ssl_check_peer_name != SSL_ENABLED_FALSE) &&
+            ((sc->proxy_ssl_check_peer_cn != SSL_ENABLED_FALSE) ||
+             (sc->proxy_ssl_check_peer_name == SSL_ENABLED_TRUE)) &&
             hostname_note) {
             apr_table_unset(c->notes, "proxy-request-hostname");
             if (!cert
@@ -1200,7 +1202,7 @@ static apr_status_t ssl_io_filter_handshake(ssl_filter_ctx_t *filter_ctx)
                               "for hostname %s", hostname_note);
             }
         }
-        else if ((sc->proxy_ssl_check_peer_cn != SSL_ENABLED_FALSE) &&
+        else if ((sc->proxy_ssl_check_peer_cn == SSL_ENABLED_TRUE) &&
             hostname_note) {
             const char *hostname;
             int match = 0;
-- 
2.50.1