From: Amos Jeffries Date: Fri, 16 Dec 2011 11:29:40 +0000 (+1300) Subject: Bug 3391: forwarded_for log functionality broken X-Git-Tag: BumpSslServerFirst.take05~12^2~119 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d42040188b431d11ccd32b86521f656eb67d187d;p=thirdparty%2Fsquid.git Bug 3391: forwarded_for log functionality broken --- diff --git a/src/AccessLogEntry.cc b/src/AccessLogEntry.cc new file mode 100644 index 0000000000..96b4410c20 --- /dev/null +++ b/src/AccessLogEntry.cc @@ -0,0 +1,19 @@ +#include "config.h" +#include "AccessLogEntry.h" +#include "HttpRequest.h" + +void +AccessLogEntry::getLogClientIp(char *buf, size_t bufsz) const +{ +#if FOLLOW_X_FORWARDED_FOR + if (Config.onoff.log_uses_indirect_client && request) + request->indirect_client_addr.NtoA(buf, bufsz); + else +#endif + if (tcpClient != NULL) + tcpClient->remote.NtoA(buf, bufsz); + else if (cache.caddr.IsNoAddr()) // e.g., ICAP OPTIONS lack client + strncpy(buf, "-", 1); + else + cache.caddr.NtoA(buf, bufsz); +} diff --git a/src/AccessLogEntry.h b/src/AccessLogEntry.h index 5c2d80616d..687d4a444b 100644 --- a/src/AccessLogEntry.h +++ b/src/AccessLogEntry.h @@ -52,6 +52,11 @@ public: AccessLogEntry() : url(NULL), tcpClient(), reply(NULL), request(NULL), adapted_request(NULL) {} + /// Fetch the client IP log string into the given buffer. + /// Knows about several alternate locations of the IP + /// including indirect forwarded-for IP if configured to log that + void getLogClientIp(char *buf, size_t bufsz) const; + const char *url; /// TCP/IP level details about the client connection diff --git a/src/Makefile.am b/src/Makefile.am index 7c3ffabe30..d6de297721 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -253,6 +253,7 @@ libsquid_la_SOURCES = \ squid_SOURCES = \ $(ACL_REGISTRATION_SOURCES) \ + AccessLogEntry.cc \ AccessLogEntry.h \ AsyncEngine.cc \ AsyncEngine.h \ @@ -1260,6 +1261,7 @@ tests_testBoilerplate_DEPENDENCIES = \ ## Tests of the CacheManager module. tests_testCacheManager_SOURCES = \ + AccessLogEntry.cc \ $(ACL_REGISTRATION_SOURCES) \ debug.cc \ HttpParser.cc \ @@ -1598,6 +1600,7 @@ tests_testDiskIO_DEPENDENCIES = \ ## Tests of the Even module. tests_testEvent_SOURCES = \ + AccessLogEntry.cc \ $(ACL_REGISTRATION_SOURCES) \ BodyPipe.cc \ CacheDigest.cc \ @@ -1791,6 +1794,7 @@ tests_testEvent_DEPENDENCIES = \ ## Tests of the EventLoop module. tests_testEventLoop_SOURCES = \ + AccessLogEntry.cc \ $(ACL_REGISTRATION_SOURCES) \ BodyPipe.cc \ CacheDigest.cc \ @@ -1982,6 +1986,7 @@ tests_testEventLoop_DEPENDENCIES = \ $(SQUID_CPPUNIT_LA) tests_test_http_range_SOURCES = \ + AccessLogEntry.cc \ $(ACL_REGISTRATION_SOURCES) \ BodyPipe.cc \ cache_cf.cc \ @@ -2203,6 +2208,7 @@ tests_testHttpParser_DEPENDENCIES = \ ## Tests of the HttpRequest module. tests_testHttpRequest_SOURCES = \ + AccessLogEntry.cc \ $(ACL_REGISTRATION_SOURCES) \ HttpParser.cc \ HttpParser.h \ @@ -3121,6 +3127,7 @@ tests_testNull_DEPENDENCIES = \ ## Tests of the URL module. ## TODO: Trim this down once the insanity is over. tests_testURL_SOURCES = \ + AccessLogEntry.cc \ $(ACL_REGISTRATION_SOURCES) \ BodyPipe.cc \ cache_cf.cc \ diff --git a/src/format/Format.cc b/src/format/Format.cc index 4c6f780260..6344c4cfe2 100644 --- a/src/format/Format.cc +++ b/src/format/Format.cc @@ -315,10 +315,8 @@ Format::Format::assemble(MemBuf &mb, AccessLogEntry *al, int logSequenceNumber) break; case LFT_CLIENT_IP_ADDRESS: - if (al->cache.caddr.IsNoAddr()) // e.g., ICAP OPTIONS lack client - out = "-"; - else - out = al->cache.caddr.NtoA(tmp,1024); + al->getLogClientIp(tmp, sizeof(tmp)); + out = tmp; break; case LFT_CLIENT_FQDN: diff --git a/src/log/FormatHttpdCombined.cc b/src/log/FormatHttpdCombined.cc index 72bc6daba0..50fb3d646a 100644 --- a/src/log/FormatHttpdCombined.cc +++ b/src/log/FormatHttpdCombined.cc @@ -44,8 +44,6 @@ void Log::Format::HttpdCombined(AccessLogEntry * al, Logfile * logfile) { - char clientip[MAX_IPSTRLEN]; - const char *user_ident = ::Format::QuoteUrlEncodeUsername(al->cache.rfc931); const char *user_auth = ::Format::QuoteUrlEncodeUsername(al->cache.authuser); @@ -58,8 +56,11 @@ Log::Format::HttpdCombined(AccessLogEntry * al, Logfile * logfile) if (!agent || *agent == '\0') agent = "-"; + char clientip[MAX_IPSTRLEN]; + al->getLogClientIp(clientip, MAX_IPSTRLEN); + logfilePrintf(logfile, "%s %s %s [%s] \"%s %s %s/%d.%d\" %d %"PRId64" \"%s\" \"%s\" %s%s:%s%s", - al->cache.caddr.NtoA(clientip,MAX_IPSTRLEN), + clientip, user_ident ? user_ident : dash_str, user_auth ? user_auth : dash_str, Time::FormatHttpd(squid_curtime), diff --git a/src/log/FormatHttpdCommon.cc b/src/log/FormatHttpdCommon.cc index 3dce6a5ae8..d5c16fc903 100644 --- a/src/log/FormatHttpdCommon.cc +++ b/src/log/FormatHttpdCommon.cc @@ -43,12 +43,14 @@ void Log::Format::HttpdCommon(AccessLogEntry * al, Logfile * logfile) { - char clientip[MAX_IPSTRLEN]; const char *user_auth = ::Format::QuoteUrlEncodeUsername(al->cache.authuser); const char *user_ident = ::Format::QuoteUrlEncodeUsername(al->cache.rfc931); + char clientip[MAX_IPSTRLEN]; + al->getLogClientIp(clientip, MAX_IPSTRLEN); + logfilePrintf(logfile, "%s %s %s [%s] \"%s %s %s/%d.%d\" %d %"PRId64" %s%s:%s%s", - al->cache.caddr.NtoA(clientip,MAX_IPSTRLEN), + clientip, user_ident ? user_ident : dash_str, user_auth ? user_auth : dash_str, Time::FormatHttpd(squid_curtime), diff --git a/src/log/FormatSquidNative.cc b/src/log/FormatSquidNative.cc index 76d3107194..21ed037148 100644 --- a/src/log/FormatSquidNative.cc +++ b/src/log/FormatSquidNative.cc @@ -62,10 +62,7 @@ Log::Format::SquidNative(AccessLogEntry * al, Logfile * logfile) safe_free(user); char clientip[MAX_IPSTRLEN]; - if (al->tcpClient != NULL) - al->tcpClient->remote.NtoA(clientip, sizeof(clientip)); - else - al->cache.caddr.NtoA(clientip, sizeof(clientip)); + al->getLogClientIp(clientip, MAX_IPSTRLEN); logfilePrintf(logfile, "%9ld.%03d %6d %s %s%s/%03d %"PRId64" %s %s %s %s%s/%s %s%s", (long int) current_time.tv_sec, diff --git a/src/log/FormatSquidReferer.cc b/src/log/FormatSquidReferer.cc index 585187dd65..6932b9cdbb 100644 --- a/src/log/FormatSquidReferer.cc +++ b/src/log/FormatSquidReferer.cc @@ -50,11 +50,12 @@ Log::Format::SquidReferer(AccessLogEntry *al, Logfile *logfile) return; char clientip[MAX_IPSTRLEN]; + al->getLogClientIp(clientip, MAX_IPSTRLEN); logfilePrintf(logfile, "%9ld.%03d %s %s %s\n", (long int) current_time.tv_sec, (int) current_time.tv_usec / 1000, - al->cache.caddr.NtoA(clientip, MAX_IPSTRLEN), + clientip, referer, al->url ? al->url : "-"); } diff --git a/src/log/FormatSquidUseragent.cc b/src/log/FormatSquidUseragent.cc index ce9f676a1a..dc1ae23a5b 100644 --- a/src/log/FormatSquidUseragent.cc +++ b/src/log/FormatSquidUseragent.cc @@ -43,16 +43,17 @@ void Log::Format::SquidUserAgent(AccessLogEntry * al, Logfile * logfile) { - char clientip[MAX_IPSTRLEN]; - const char *agent = al->request->header.getStr(HDR_USER_AGENT); // do not log unless there is something to be displayed. if (!agent || *agent == '\0') return; + char clientip[MAX_IPSTRLEN]; + al->getLogClientIp(clientip, MAX_IPSTRLEN); + logfilePrintf(logfile, "%s [%s] \"%s\"\n", - al->cache.caddr.NtoA(clientip,MAX_IPSTRLEN), + clientip, Time::FormatHttpd(squid_curtime), agent); }