Author: Alex Rousskov <rousskov@measurement-factory.com>

Bug 1628: Port follow_x_forwarded_for from 2.6

diff --git a/configure.in b/configure.in
index fb69fb278688a1e78cac86b3e5c2ffb5b7908014..4d4a59ceafdcbc7145abbe70b5e6be6b352565b9 100755 (executable)
--- a/configure.in
+++ b/configure.in
@@ -1363,6 +1363,24 @@ AC_ARG_ENABLE(leakfinder,
   fi
 ])
 
+follow_xff=1
+AC_ARG_ENABLE(follow-x-forwarded-for,
+[  --enable-follow-x-forwarded-for
+                          Enable support for following the X-Forwarded-For
+                          HTTP header to try to find the IP address of the
+                          original or indirect client when a request has
+                          been forwarded through other proxies.],
+[ if test "$enableval" = "yes" ; then
+    echo "follow X-Forwarded-For enabled"
+    follow_xff=1
+  fi
+])
+if test $follow_xff = 1; then
+    AC_DEFINE(FOLLOW_X_FORWARDED_FOR, 1, [Enable following X-Forwarded-For headers])
+else
+    AC_DEFINE(FOLLOW_X_FORWARDED_FOR, 0)
+fi
+
 use_ident=1
 AC_ARG_ENABLE(ident-lookups,
 [  --disable-ident-lookups

diff --git a/squid3.dox b/squid3.dox
index be82f9d9ad043f868a206d9965ba3a5fa80f4392..d2b89f7c43d06f422653eac51a3b8ec8779171ff 100644 (file)
--- a/squid3.dox
+++ b/squid3.dox
@@ -1045,7 +1045,8 @@ PREDEFINED             = \
                        USE_WCCP \
                        USE_WCCPv2 \
                        USE_WIN32_SERVICE \
-                       USE_XPROF_STATS
+                       USE_XPROF_STATS \
+                       FOLLOW_X_FORWARDED_FOR
 
 
 # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then 

diff --git a/src/ACLChecklist.cc b/src/ACLChecklist.cc
index 795456658485f80c1610a8531bda501ca4d30a8c..e41ebb01c80a7c9a3fcd2968ba8d7b3e00ced6fa 100644 (file)
--- a/src/ACLChecklist.cc
+++ b/src/ACLChecklist.cc
@@ -582,9 +582,15 @@ aclChecklistCreate(const acl_access * A, HttpRequest * request, const char *iden
     if (A)
         checklist->accessList = cbdataReference(A);
 
-    if (request != NULL) {
+    if (request != NULL)
+    {
         checklist->request = HTTPMSGLOCK(request);
-        checklist->src_addr = request->client_addr;
+#if FOLLOW_X_FORWARDED_FOR
+        if (Config.onoff.acl_uses_indirect_client)
+            checklist->src_addr = request->indirect_client_addr;
+        else
+#endif /* FOLLOW_X_FORWARDED_FOR */
+            checklist->src_addr = request->client_addr;
         checklist->my_addr = request->my_addr;
     }
 

diff --git a/src/DelayId.cc b/src/DelayId.cc
index a1b985292cc969ff67067d7ca48da9b4a8b46186..4e9cb1e66ec2b4de547d2eb20d5b5256e65317ac 100644 (file)
--- a/src/DelayId.cc
+++ b/src/DelayId.cc
@@ -105,8 +105,14 @@ DelayId::DelayClient(ClientHttpRequest * http)
         return DelayId();
     }
 
-    for (pool = 0; pool < DelayPools::pools(); pool++) {
+    for (pool = 0; pool < DelayPools::pools(); pool++)
+    {
         ACLChecklist ch;
+#if FOLLOW_X_FORWARDED_FOR
+    if (Config.onoff.delay_pool_uses_indirect_client)
+        ch.src_addr = r->indirect_client_addr;
+    else
+#endif /* FOLLOW_X_FORWARDED_FOR */        
         ch.src_addr = r->client_addr;
         ch.my_addr = r->my_addr;
 

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
index 0acd6de5c23818565f8559f8251345a2dc9e9400..f099d6919f7c52f8851fe08949b473bc7d3e82d4 100644 (file)
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@ -95,6 +95,9 @@ HttpRequest::init()
     extacl_passwd = null_string;
     extacl_log = null_string;
     pstate = psReadyToParseStartLine;
+#if FOLLOW_X_FORWARDED_FOR
+    indirect_client_addr.SetEmpty();
+#endif /* FOLLOW_X_FORWARDED_FOR */
 }
 
 void

diff --git a/src/HttpRequest.h b/src/HttpRequest.h
index ffa3ec0f48f0b9ad1ea886df427f366ace162fe5..39a0694dc56b03f63dbde846eda227774441afc3 100644 (file)
--- a/src/HttpRequest.h
+++ b/src/HttpRequest.h
@@ -122,6 +122,10 @@ public:
 
     IPAddress client_addr;
 
+#if FOLLOW_X_FORWARDED_FOR
+    IPAddress indirect_client_addr;
+#endif /* FOLLOW_X_FORWARDED_FOR */
+
     IPAddress my_addr;
 
     HierarchyLogEntry hier;
@@ -144,6 +148,10 @@ public:
 
     String extacl_log;         /* String to be used for access.log purposes */
 
+#if FOLLOW_X_FORWARDED_FOR
+    String x_forwarded_for_iterator; /* XXX a list of IP addresses */
+#endif /* FOLLOW_X_FORWARDED_FOR */
+
 public:
     bool multipartRangeRequest() const;
 

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 3bf9a20f161cfbeaf9f1023d9eafbce73eec693f..503bd75e2753cc554a947cf00c757ec448717f23 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -660,6 +660,92 @@ acl CONNECT method CONNECT
 NOCOMMENT_END
 DOC_END
 
+NAME: follow_x_forwarded_for
+TYPE: acl_access
+IFDEF: FOLLOW_X_FORWARDED_FOR
+LOC: Config.accessList.followXFF
+DEFAULT: none
+DEFAULT_IF_NONE: deny all
+DOC_START
+       Allowing or Denying the X-Forwarded-For header to be followed to
+       find the original source of a request.
+
+       Requests may pass through a chain of several other proxies
+       before reaching us.  The X-Forwarded-For header will contain a
+       comma-separated list of the IP addresses in the chain, with the
+       rightmost address being the most recent.
+
+       If a request reaches us from a source that is allowed by this
+       configuration item, then we consult the X-Forwarded-For header
+       to see where that host received the request from.  If the
+       X-Forwarded-For header contains multiple addresses, and if
+       acl_uses_indirect_client is on, then we continue backtracking
+       until we reach an address for which we are not allowed to
+       follow the X-Forwarded-For header, or until we reach the first
+       address in the list.  (If acl_uses_indirect_client is off, then
+       it's impossible to backtrack through more than one level of
+       X-Forwarded-For addresses.)
+
+       The end result of this process is an IP address that we will
+       refer to as the indirect client address.  This address may
+       be treated as the client address for access control, delay
+       pools and logging, depending on the acl_uses_indirect_client,
+       delay_pool_uses_indirect_client and log_uses_indirect_client
+       options.
+
+       SECURITY CONSIDERATIONS:
+
+               Any host for which we follow the X-Forwarded-For header
+               can place incorrect information in the header, and Squid
+               will use the incorrect information as if it were the
+               source address of the request.  This may enable remote
+               hosts to bypass any access control restrictions that are
+               based on the client's source addresses.
+
+       For example:
+
+               acl localhost src 127.0.0.1
+               acl my_other_proxy srcdomain .proxy.example.com
+               follow_x_forwarded_for allow localhost
+               follow_x_forwarded_for allow my_other_proxy
+DOC_END
+
+NAME: acl_uses_indirect_client
+COMMENT: on|off
+TYPE: onoff
+IFDEF: FOLLOW_X_FORWARDED_FOR
+DEFAULT: on
+LOC: Config.onoff.acl_uses_indirect_client
+DOC_START
+       Controls whether the indirect client address
+       (see follow_x_forwarded_for) is used instead of the
+       direct client address in acl matching.
+DOC_END
+
+NAME: delay_pool_uses_indirect_client
+COMMENT: on|off
+TYPE: onoff
+IFDEF: FOLLOW_X_FORWARDED_FOR && DELAY_POOLS
+DEFAULT: on
+LOC: Config.onoff.delay_pool_uses_indirect_client
+DOC_START
+       Controls whether the indirect client address
+       (see follow_x_forwarded_for) is used instead of the
+       direct client address in delay pools.
+DOC_END
+
+NAME: log_uses_indirect_client
+COMMENT: on|off
+TYPE: onoff
+IFDEF: FOLLOW_X_FORWARDED_FOR
+DEFAULT: on
+LOC: Config.onoff.log_uses_indirect_client
+DOC_START
+       Controls whether the indirect client address
+       (see follow_x_forwarded_for) is used instead of the
+       direct client address in the access log.
+DOC_END
+
 NAME: http_access
 TYPE: acl_access
 LOC: Config.accessList.http

diff --git a/src/client_side.cc b/src/client_side.cc
index 7432dd339ed2f88653bf750e9cd55d4ea2042472..8b6450b282881c91be710d4dedd86b8e0148f3b7 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2249,6 +2249,9 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c
     request->flags.internal = http->flags.internal;
     setLogUri (http, urlCanonicalClean(request));
     request->client_addr = conn->peer;
+#if FOLLOW_X_FORWARDED_FOR
+    request->indirect_client_addr = conn->peer;
+#endif /* FOLLOW_X_FORWARDED_FOR */
     request->my_addr = conn->me;
     request->http_ver = http_ver;
 

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 0be698fcc1a9179034b492c0bc17be87bfbba1c8..528af520712aed8aa4635b65fc5a3aed0dd7dfdc 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -71,6 +71,9 @@ static void adaptationAclCheckDoneWrapper(Adaptation::ServicePointer service, vo
 
 static const char *const crlf = "\r\n";
 
+static void
+clientFollowXForwardedForCheck(int answer, void *data);
+
 CBDATA_CLASS_INIT(ClientRequestContext);
 
 void *
@@ -343,6 +346,10 @@ clientBeginRequest(const HttpRequestMethod& method, char const *url, CSCB * stre
 
     request->client_addr.SetNoAddr();
 
+#if FOLLOW_X_FORWARDED_FOR
+    request->indirect_client_addr.SetNoAddr();
+#endif /* FOLLOW_X_FORWARDED_FOR */
+    
     request->my_addr.SetNoAddr();      /* undefined for internal requests */
 
     request->my_addr.SetPort(0);
@@ -380,10 +387,121 @@ ClientRequestContext::httpStateIsValid()
     return false;
 }
 
+#if FOLLOW_X_FORWARDED_FOR
+/**
+ * clientFollowXForwardedForCheck() checks the indirect_client_addr
+ * against the followXFF ACL, or cleans up and passes control to
+ * clientAccessCheck().
+ */
+
+static void
+clientFollowXForwardedForCheck(int answer, void *data)
+{
+    ClientRequestContext *calloutContext = (ClientRequestContext *) data;
+    ClientHttpRequest *http = NULL;
+    HttpRequest *request = NULL;
+
+    if (!calloutContext->httpStateIsValid())
+        return;
+
+        http = calloutContext->http;
+    request = http->request;
+    /*
+     * answer should be be ACCESS_ALLOWED or ACCESS_DENIED if we are
+     * called as a result of ACL checks, or -1 if we are called when
+     * there's nothing left to do.
+     */
+    if (answer == ACCESS_ALLOWED &&
+        request->x_forwarded_for_iterator.size () != 0)
+    {
+        /*
+        * The IP address currently in request->indirect_client_addr
+        * is trusted to use X-Forwarded-For.  Remove the last
+        * comma-delimited element from x_forwarded_for_iterator and use
+        * it to to replace indirect_client_addr, then repeat the cycle.
+        */
+        const char *p;
+        const char *asciiaddr;
+        int l;
+        struct in_addr addr;
+        p = request->x_forwarded_for_iterator.buf();
+        l = request->x_forwarded_for_iterator.size();
+
+        /*
+        * XXX x_forwarded_for_iterator should really be a list of
+        * IP addresses, but it's a String instead.  We have to
+        * walk backwards through the String, biting off the last
+        * comma-delimited part each time.  As long as the data is in
+        * a String, we should probably implement and use a variant of
+        * strListGetItem() that walks backwards instead of forwards
+        * through a comma-separated list.  But we don't even do that;
+        * we just do the work in-line here.
+        */
+        /* skip trailing space and commas */
+        while (l > 0 && (p[l-1] == ',' || xisspace(p[l-1])))
+            l--;
+        request->x_forwarded_for_iterator.cut(l);
+        /* look for start of last item in list */
+        while (l > 0 && ! (p[l-1] == ',' || xisspace(p[l-1])))
+            l--;
+        asciiaddr = p+l;
+        if (inet_aton(asciiaddr, &addr) != 0)
+        {
+            request->indirect_client_addr = addr;
+            request->x_forwarded_for_iterator.cut(l);
+            if (! Config.onoff.acl_uses_indirect_client)
+            {
+                /*
+                * If acl_uses_indirect_client is off, then it's impossible
+                * to follow more than one level of X-Forwarded-For.
+                */
+                request->x_forwarded_for_iterator.clean();
+            }
+            calloutContext->acl_checklist =
+            clientAclChecklistCreate(Config.accessList.followXFF, http);
+            calloutContext->acl_checklist->
+            nonBlockingCheck(clientFollowXForwardedForCheck, data);
+            return;
+        }
+    } /*if (answer == ACCESS_ALLOWED &&
+        request->x_forwarded_for_iterator.size () != 0)*/
+
+    /* clean up, and pass control to clientAccessCheck */
+    if (Config.onoff.log_uses_indirect_client)
+    {
+        /*
+        * Ensure that the access log shows the indirect client
+        * instead of the direct client.
+        */
+        ConnStateData *conn = http->getConn();
+        conn->log_addr = request->indirect_client_addr;
+    }
+    request->x_forwarded_for_iterator.clean();
+    request->flags.done_follow_x_forwarded_for = 1;
+
+    if (answer == ACCESS_DENIED)
+        calloutContext->clientAccessCheckDone(answer);
+    else
+        calloutContext->clientAccessCheck();
+}
+#endif /* FOLLOW_X_FORWARDED_FOR */
+
 /* This is the entry point for external users of the client_side routines */
 void
 ClientRequestContext::clientAccessCheck()
 {
+#if FOLLOW_X_FORWARDED_FOR
+    if (!http->request->flags.done_follow_x_forwarded_for &&
+        Config.accessList.followXFF &&
+        http->request->header.has(HDR_X_FORWARDED_FOR))
+    {
+        http->request->x_forwarded_for_iterator = 
+            http->request->header.getList(HDR_X_FORWARDED_FOR);
+        clientFollowXForwardedForCheck(ACCESS_ALLOWED, this);
+        return;
+    }
+#endif /* FOLLOW_X_FORWARDED_FOR */
+    
     acl_checklist =
         clientAclChecklistCreate(Config.accessList.http, http);
     acl_checklist->nonBlockingCheck(clientAccessCheckDoneWrapper, this);
@@ -811,6 +929,9 @@ ClientRequestContext::clientRedirectDone(char *result)
         new_request->http_ver = old_request->http_ver;
         new_request->header.append(&old_request->header);
         new_request->client_addr = old_request->client_addr;
+#if FOLLOW_X_FORWARDED_FOR
+        new_request->indirect_client_addr = old_request->indirect_client_addr;
+#endif /* FOLLOW_X_FORWARDED_FOR */
         new_request->my_addr = old_request->my_addr;
         new_request->flags = old_request->flags;
         new_request->flags.redirected = 1;

diff --git a/src/structs.h b/src/structs.h
index bfb24f0f8a890b56ec095df214b4edd7c4b4c79b..4a9d011b05e59ae5971b9c0b8f5483b23d870ff2 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -448,8 +448,8 @@ struct SquidConfig
         int ie_refresh;
         int vary_ignore_expire;
         int pipeline_prefetch;
-#if USE_SQUID_ESI
 
+#if USE_SQUID_ESI
         int surrogate_is_remote;
 #endif
 
@@ -465,10 +465,17 @@ struct SquidConfig
         int global_internal_static;
         int dns_require_A;
         int debug_override_X;
-#if USE_ZPH_QOS        
+
+#if FOLLOW_X_FORWARDED_FOR
+        int acl_uses_indirect_client;
+        int delay_pool_uses_indirect_client;
+        int log_uses_indirect_client;
+#endif /* FOLLOW_X_FORWARDED_FOR */
+
+#if USE_ZPH_QOS
         int zph_tos_parent;
         int zph_preserve_miss_tos;
-#endif        
+#endif
     } onoff;
 
     class ACL *aclList;
@@ -508,6 +515,9 @@ struct SquidConfig
 #if USE_SSL
         acl_access *ssl_bump;
 #endif
+#if FOLLOW_X_FORWARDED_FOR
+        acl_access *followXFF;
+#endif /* FOLLOW_X_FORWARDED_FOR */
 
     } accessList;
     acl_deny_info_list *denyInfoList;
@@ -1045,6 +1055,9 @@ struct request_flags
 #if HTTP_VIOLATIONS
         nocache_hack = 0;
 #endif
+#if FOLLOW_X_FORWARDED_FOR
+    done_follow_x_forwarded_for = 0;
+#endif /* FOLLOW_X_FORWARDED_FOR */
     }
 
     unsigned int range:1;
@@ -1081,6 +1094,9 @@ struct request_flags
     // that are safe for a related (e.g., ICAP-adapted) request to inherit
     request_flags cloneAdaptationImmune() const;
 
+#if FOLLOW_X_FORWARDED_FOR
+    unsigned int done_follow_x_forwarded_for;
+#endif /* FOLLOW_X_FORWARDED_FOR */
 private:
 
     unsigned int reset_tcp:1;