From: Christos Tsantilas <chtsanti@users.sourceforge.net>
Date: Tue, 17 Apr 2012 16:08:41 +0000 (+0300)
Subject: Ssl certificate domain mismatch errors on IP-based URLs
X-Git-Tag: BumpSslServerFirst.take08~16
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=35cd42816e3f869a37804485776489d18955336f;p=thirdparty%2Fsquid.git

Ssl certificate domain mismatch errors on IP-based URLs

The ssl::certDomainMismatch acl can not be used with ip-based urls. For example
let's assume that a user enters https://74.125.65.99/, using a Google IP addressin the URL instead of www.google.com. If the sslBump used with
"sslproxy_cert_error allow all" and "sslproxy_cert_adapt setCommonName ssl::certDomainMismatch"
the browser displays a browser "Server's certificate does not match the URL"
error.

This is because for all cases we have the ip address instead of the hostname
we are detecting the cert domain mismatch errors when the first GET request
comes. At the time the sslproxy_cert_adatp access list processed the error is
not detected yet.
For intercepted connections this is the desired behaviour.

This patch fix the ssl-bump-first to check for domain-mismatch errors while
retrieving the SSL certificate from the server, hoping that CONNECT is using
a user-entered address (a host name or a user-entered IP).
---

diff --git a/src/client_side.cc b/src/client_side.cc
index fb5c12b643..f756ad29bb 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3915,10 +3915,12 @@ ConnStateData::httpsPeeked(Comm::ConnectionPointer serverConnection)
         debugs(33, 5, HERE << "Error while bumping: " << sslConnectHostOrIp);
         Ip::Address intendedDest;
         intendedDest = sslConnectHostOrIp.termedBuf();
+        const bool isConnectRequest = !port->spoof_client_ip && !port->intercepted;
+
         // Squid serves its own error page and closes, so we want
         // a CN that causes no additional browser errors. Possible
-        // only when bumping CONNECT which uses a host name.
-        if (intendedDest.IsAnyAddr())
+        // only when bumping CONNECT with a user-typed address.
+        if (intendedDest.IsAnyAddr() || isConnectRequest)
             sslCommonName = sslConnectHostOrIp;
         else if (sslServerBump->serverCert.get())
             sslCommonName = Ssl::CommonHostName(sslServerBump->serverCert.get());
diff --git a/src/forward.cc b/src/forward.cc
index 20367f33ae..1f2babf997 100644
--- a/src/forward.cc
+++ b/src/forward.cc
@@ -50,6 +50,7 @@
 #include "MemObject.h"
 #include "pconn.h"
 #include "PeerSelectState.h"
+#include "ProtoPort.h"
 #include "SquidTime.h"
 #include "Store.h"
 #include "icmp/net_db.h"
@@ -679,9 +680,12 @@ FwdState::negotiateSSL(int fd)
                 }
             }
 
-            if (request->flags.sslPeek && request->GetHostIsNumeric()) {
-                // If possible, set host name to server certificate CN unless
-                // we already got the right name (from the CONNECT request).
+            const bool isConnectRequest = !request->clientConnectionManager->port->spoof_client_ip && 
+                !request->clientConnectionManager->port->intercepted;
+            // For intercepted connections, set host name to server
+            // certificate CN. Otherwise, we hope that CONNECT is using
+            // a user-entered address (a host name or a user-entered IP).
+            if (request->flags.sslPeek && !isConnectRequest) {
                 if (X509 *srvX509 = errDetails->peerCert()) {
                     if (const char *name = Ssl::CommonHostName(srvX509)) {
                         request->SetHost(name);
@@ -770,11 +774,12 @@ FwdState::initiateSSL()
     } else {
         // While we are peeking at the certificate, we may not know the server
         // name that the client will request (after interception or CONNECT)
-        // unless it was the CONNECT request which used a host name. Some
-        // browsers are using IP addresses in CONNECT requests.
+        // unless it was the CONNECT request with a user-typed address.
         const char *hostname = request->GetHost();
         const bool hostnameIsIp = request->GetHostIsNumeric();
-        if (!request->flags.sslPeek || !hostnameIsIp)
+        const bool isConnectRequest = !request->clientConnectionManager->port->spoof_client_ip && 
+            !request->clientConnectionManager->port->intercepted;
+        if (!request->flags.sslPeek || isConnectRequest)
             SSL_set_ex_data(ssl, ssl_ex_index_server, (void*)hostname);
 
         // Use SNI TLS extension only when we connect directly