From 3d674977fc644112c6c23ad3798d9d18d04be265 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Sat, 7 Jun 2008 17:20:05 +1200 Subject: [PATCH] Author: Alex Rousskov Bug 1628: Port follow_x_forwarded_for from 2.6 --- configure.in | 18 ++++++ squid3.dox | 3 +- src/ACLChecklist.cc | 10 ++- src/DelayId.cc | 8 ++- src/HttpRequest.cc | 3 + src/HttpRequest.h | 8 +++ src/cf.data.pre | 86 ++++++++++++++++++++++++++ src/client_side.cc | 3 + src/client_side_request.cc | 121 +++++++++++++++++++++++++++++++++++++ src/structs.h | 22 ++++++- 10 files changed, 275 insertions(+), 7 deletions(-) diff --git a/configure.in b/configure.in index fb69fb2786..4d4a59ceaf 100755 --- 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 be82f9d9ad..d2b89f7c43 100644 --- 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 7954566584..e41ebb01c8 100644 --- 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 a1b985292c..4e9cb1e66e 100644 --- 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 0acd6de5c2..f099d6919f 100644 --- 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 ffa3ec0f48..39a0694dc5 100644 --- 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 3bf9a20f16..503bd75e27 100644 --- 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 7432dd339e..8b6450b282 100644 --- 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 0be698fcc1..528af52071 100644 --- 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 bfb24f0f8a..4a9d011b05 100644 --- 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; -- 2.39.2