Fix ssl::server_name ACL badly broken since inception.

The original server_name code mishandled all SNI checks and some rare
host checks:

* The SNI-derived value was pointing to an already freed memory storage.
* Missing host-derived values were not detected (host() is never nil).
* Mismatches were re-checked with an undocumented "none" value
  instead of being treated as mismatches.

Same for ssl::server_name_regex.

Also set SNI for more server-first and client-first transactions.

This is a Measurement Factory project.

diff --git a/src/acl/ServerName.cc b/src/acl/ServerName.cc
index f49209bdc08573074083baa8d9c03442da1600ba..ae93e03920fbd51e12f3339ebed741b9eb2a3b01 100644 (file)
--- a/src/acl/ServerName.cc
+++ b/src/acl/ServerName.cc
@@ -90,27 +90,28 @@ ACLServerNameStrategy::match (ACLData<MatchType> * &data, ACLFilledChecklist *ch
 {
     assert(checklist != NULL && checklist->request != NULL);
 
-    if (checklist->conn() && checklist->conn()->serverBump()) {
-        if (X509 *peer_cert = checklist->conn()->serverBump()->serverCert.get()) {
-            if (Ssl::matchX509CommonNames(peer_cert, (void *)data, check_cert_domain<MatchType>))
-                return 1;
+    const char *serverName = NULL;
+    SBuf serverNameKeeper; // because c_str() is not constant
+    if (ConnStateData *conn = checklist->conn()) {
+        if (conn->serverBump()) {
+            if (X509 *peer_cert = conn->serverBump()->serverCert.get())
+                return Ssl::matchX509CommonNames(peer_cert, (void *)data, check_cert_domain<MatchType>);
         }
-    }
 
-    const char *serverName = NULL;
-    if (checklist->conn() && !checklist->conn()->sslCommonName().isEmpty()) {
-        SBuf scn = checklist->conn()->sslCommonName();
-        serverName = scn.c_str();
+        if (conn->sslCommonName().isEmpty()) {
+            const char *host = checklist->request->GetHost();
+            if (host && *host) // paranoid first condition: host() is never nil
+                serverName = host;
+        } else {
+            serverNameKeeper = conn->sslCommonName();
+            serverName = serverNameKeeper.c_str();
+        }
     }
 
-    if (serverName == NULL)
-        serverName = checklist->request->GetHost();
-
-    if (serverName && data->match(serverName)) {
-        return 1;
-    }
+    if (!serverName)
+        serverName = "none";
 
-    return data->match("none");
+    return data->match(serverName);
 }
 
 ACLServerNameStrategy *

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 4edc18cfcdf31ac12ae48d806bbffaa7d0e4ba68..44a3290cf5ca948de9a1aca204e57e2dbc890e72 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -1167,6 +1167,9 @@ IF USE_OPENSSL
          # During each Ssl-Bump step, Squid may improve its understanding of a
          # "true server name". Unlike dstdomain, this ACL does not perform
          # DNS lookups.
+         # The "none" name can be used to match transactions where Squid
+         # could not compute the server name using any information source
+         # already available at the ACL evaluation time.
 
        acl aclname ssl::server_name_regex [-i] \.foo\.com ...
          # regex matches server name obtained from various sources [fast]