From cb365059899d555e825d3f4d749cad8cf1c62482 Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Fri, 11 May 2018 08:32:06 +0000 Subject: [PATCH] Supply AccessLogEntry (ALE) for more fast ACL checks. (#182) Supplying ALE for fast ACL checks allows those checks to use ACLs that assemble values from logformat %codes. Today, such ACLs are limited to misplaced external ACLs (that should not be used with "fast" directives!), but it is likely that fast ACLs like annotate_client will eventually require ALE. The "has" ACL documentation promises ALE for every transaction, but our code does not deliver on that promise. This change fixes a dozen of easy cases where ALE was available nearby. Also a non-trivial cache_peer_access case was fixed, which proved to be more complex because of the significant call depth of the peerAllowedToUse() check, which is a known design problem of its own. More cases need fixing, and the whole concept of ALE probably needs to be revised because logformat %code expansion is needed in the increasing number of contexts that have nothing to do with access logging. Also fixed triggering of (probably pointless) level-1 warnings: * ALE missing adapted HttpRequest object * ALE missing URL With fix applied, any ACLChecklist with ALE synchronizes it at 'pre-check' stage without logging level-1 warnings. Warnings are triggered only if for some reason this 'pre-check' synchronization was bypassed. --- src/FwdState.cc | 4 ++ src/HttpRequest.cc | 1 + src/MemObject.h | 3 +- src/Notes.cc | 2 + src/acl/Acl.cc | 2 +- src/acl/Checklist.h | 7 ++- src/acl/FilledChecklist.cc | 19 ++++++- src/acl/FilledChecklist.h | 3 +- src/adaptation/AccessCheck.cc | 1 + src/carp.cc | 8 ++- src/carp.h | 4 +- src/client_side.cc | 16 ++++++ src/client_side_request.cc | 2 + src/comm/TcpAcceptor.cc | 1 + src/http.cc | 4 ++ src/icmp/net_db.cc | 8 ++- src/icmp/net_db.h | 3 +- src/neighbors.cc | 93 ++++++++++++++++++++++----------- src/neighbors.h | 21 ++++---- src/peer_select.cc | 39 ++++++++------ src/peer_sourcehash.cc | 8 ++- src/peer_sourcehash.h | 4 +- src/peer_userhash.cc | 8 ++- src/peer_userhash.h | 3 +- src/security/PeerConnector.cc | 2 + src/ssl/PeekingPeerConnector.cc | 1 + src/tests/stub_carp.cc | 4 +- src/tests/stub_libicmp.cc | 2 +- src/tunnel.cc | 2 + 29 files changed, 196 insertions(+), 79 deletions(-) diff --git a/src/FwdState.cc b/src/FwdState.cc index a10272bc1d..146dbb53dc 100644 --- a/src/FwdState.cc +++ b/src/FwdState.cc @@ -332,7 +332,9 @@ FwdState::Start(const Comm::ConnectionPointer &clientConn, StoreEntry *entry, Ht * we do NOT want the indirect client address to be tested here. */ ACLFilledChecklist ch(Config.accessList.miss, request, NULL); + ch.al = al; ch.src_addr = request->client_addr; + ch.syncAle(request, nullptr); if (ch.fastCheck().denied()) { err_type page_id; page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, 1); @@ -1219,6 +1221,8 @@ FwdState::pconnPop(const Comm::ConnectionPointer &dest, const char *domain) bool retriable = checkRetriable(); if (!retriable && Config.accessList.serverPconnForNonretriable) { ACLFilledChecklist ch(Config.accessList.serverPconnForNonretriable, request, NULL); + ch.al = al; + ch.syncAle(request, nullptr); retriable = ch.fastCheck().allowed(); } // always call shared pool first because we need to close an idle diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index cba9b7d182..54e86bd671 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -729,6 +729,7 @@ HttpRequest::manager(const CbcPointer &aMgr, const AccessLogEntry if (Config.accessList.spoof_client_ip) { ACLFilledChecklist *checklist = new ACLFilledChecklist(Config.accessList.spoof_client_ip, this, clientConnection->rfc931); checklist->al = al; + checklist->syncAle(this, nullptr); flags.spoofClientIp = checklist->fastCheck().allowed(); delete checklist; } else diff --git a/src/MemObject.h b/src/MemObject.h index ff9d2cc71f..3dc32017f0 100644 --- a/src/MemObject.h +++ b/src/MemObject.h @@ -28,6 +28,7 @@ typedef void STMCB (void *data, StoreIOBuffer wroteBuffer); typedef void STABH(void *); class store_client; +class PeerSelector; class MemObject { @@ -161,7 +162,7 @@ public: struct timeval start_ping; IRCB *ping_reply_callback; - void *ircb_data; + PeerSelector *ircb_data; struct { STABH *callback; diff --git a/src/Notes.cc b/src/Notes.cc index 4713c1441b..6c33b144e8 100644 --- a/src/Notes.cc +++ b/src/Notes.cc @@ -69,7 +69,9 @@ bool Note::match(HttpRequest *request, HttpReply *reply, const AccessLogEntry::Pointer &al, SBuf &matched) { ACLFilledChecklist ch(nullptr, request, nullptr); + ch.al = al; ch.reply = reply; + ch.syncAle(request, nullptr); if (reply) HTTPMSGLOCK(ch.reply); diff --git a/src/acl/Acl.cc b/src/acl/Acl.cc index 309514fb44..7b7dc41e44 100644 --- a/src/acl/Acl.cc +++ b/src/acl/Acl.cc @@ -141,7 +141,7 @@ ACL::matches(ACLChecklist *checklist) const } else { // make sure the ALE has as much data as possible if (requiresAle()) - checklist->syncAle(); + checklist->verifyAle(); // have to cast because old match() API is missing const result = const_cast(this)->match(checklist); diff --git a/src/acl/Checklist.h b/src/acl/Checklist.h index 0a7646678b..c82b7ce77b 100644 --- a/src/acl/Checklist.h +++ b/src/acl/Checklist.h @@ -13,6 +13,8 @@ #include #include +class HttpRequest; + /// ACL checklist callback typedef void ACLCB(allow_t, void *); @@ -164,7 +166,10 @@ public: virtual bool hasRequest() const = 0; virtual bool hasReply() const = 0; virtual bool hasAle() const = 0; - virtual void syncAle() const = 0; + /// assigns uninitialized adapted_request and url ALE components + virtual void syncAle(HttpRequest *adaptedRequest, const char *logUri) const = 0; + /// warns if there are uninitialized ALE components and fills them + virtual void verifyAle() const = 0; /// change the current ACL list /// \return a pointer to the old list value (may be nullptr) diff --git a/src/acl/FilledChecklist.cc b/src/acl/FilledChecklist.cc index 495cd235c4..d4b47993c2 100644 --- a/src/acl/FilledChecklist.cc +++ b/src/acl/FilledChecklist.cc @@ -79,7 +79,7 @@ showDebugWarning(const char *msg) } void -ACLFilledChecklist::syncAle() const +ACLFilledChecklist::verifyAle() const { // make sure the ALE fields used by Format::assemble to // fill the old external_acl_type codes are set if any @@ -93,6 +93,8 @@ ACLFilledChecklist::syncAle() const if (request) { if (!al->request) { showDebugWarning("HttpRequest object"); + // XXX: al->request should be original, + // but the request may be already adapted al->request = request; HTTPMSGLOCK(al->request); } @@ -105,6 +107,8 @@ ACLFilledChecklist::syncAle() const if (al->url.isEmpty()) { showDebugWarning("URL"); + // XXX: al->url should be the request URL from client, + // but request->url may be different (e.g.,redirected) al->url = request->url.absolute(); } } @@ -123,6 +127,19 @@ ACLFilledChecklist::syncAle() const #endif } +void +ACLFilledChecklist::syncAle(HttpRequest *adaptedRequest, const char *logUri) const +{ + if (!al) + return; + if (!al->adapted_request) { + al->adapted_request = adaptedRequest; + HTTPMSGLOCK(al->adapted_request); + } + if (al->url.isEmpty()) + al->url = logUri; +} + ConnStateData * ACLFilledChecklist::conn() const { diff --git a/src/acl/FilledChecklist.h b/src/acl/FilledChecklist.h index 3c1b0d4d17..0b02df3977 100644 --- a/src/acl/FilledChecklist.h +++ b/src/acl/FilledChecklist.h @@ -66,7 +66,8 @@ public: virtual bool hasRequest() const { return request != NULL; } virtual bool hasReply() const { return reply != NULL; } virtual bool hasAle() const { return al != NULL; } - virtual void syncAle() const; + virtual void syncAle(HttpRequest *adaptedRequest, const char *logUri) const; + virtual void verifyAle() const; public: Ip::Address src_addr; diff --git a/src/adaptation/AccessCheck.cc b/src/adaptation/AccessCheck.cc index 4baa5709f6..f99b2fd93e 100644 --- a/src/adaptation/AccessCheck.cc +++ b/src/adaptation/AccessCheck.cc @@ -135,6 +135,7 @@ Adaptation::AccessCheck::checkCandidates() if ((acl_checklist->reply = filter.reply)) HTTPMSGLOCK(acl_checklist->reply); acl_checklist->al = filter.al; + acl_checklist->syncAle(filter.request, nullptr); acl_checklist->nonBlockingCheck(AccessCheckCallbackWrapper, this); return; } diff --git a/src/carp.cc b/src/carp.cc index 3d23feaad5..434c541ca2 100644 --- a/src/carp.cc +++ b/src/carp.cc @@ -13,6 +13,7 @@ #include "HttpRequest.h" #include "mgr/Registration.h" #include "neighbors.h" +#include "PeerSelectState.h" #include "SquidConfig.h" #include "Store.h" #include "URL.h" @@ -143,8 +144,11 @@ carpInit(void) } CachePeer * -carpSelectParent(HttpRequest * request) +carpSelectParent(PeerSelector *ps) { + assert(ps); + HttpRequest *request = ps->request; + int k; CachePeer *p = NULL; CachePeer *tp; @@ -204,7 +208,7 @@ carpSelectParent(HttpRequest * request) debugs(39, 3, "carpSelectParent: key=" << key << " name=" << tp->name << " combined_hash=" << combined_hash << " score=" << std::setprecision(0) << score); - if ((score > high_score) && peerHTTPOkay(tp, request)) { + if ((score > high_score) && peerHTTPOkay(tp, ps)) { p = tp; high_score = score; } diff --git a/src/carp.h b/src/carp.h index 7f3901c1a1..ff8c3c88ae 100644 --- a/src/carp.h +++ b/src/carp.h @@ -12,10 +12,10 @@ #define SQUID_CARP_H_ class CachePeer; -class HttpRequest; +class PeerSelector; void carpInit(void); -CachePeer *carpSelectParent(HttpRequest *); +CachePeer *carpSelectParent(PeerSelector *); #endif /* SQUID_CARP_H_ */ diff --git a/src/client_side.cc b/src/client_side.cc index c4465aaeb9..9a4766adfa 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -447,11 +447,14 @@ ClientHttpRequest::logRequest() al->adapted_request = request; HTTPMSGLOCK(al->adapted_request); } + // no need checklist.syncAle(): already synced + checklist.al = al; accessLogLog(al, &checklist); bool updatePerformanceCounters = true; if (Config.accessList.stats_collection) { ACLFilledChecklist statsCheck(Config.accessList.stats_collection, request, NULL); + statsCheck.al = al; if (al->reply) { statsCheck.reply = al->reply; HTTPMSGLOCK(statsCheck.reply); @@ -1521,7 +1524,9 @@ bool ConnStateData::serveDelayedError(Http::Stream *context) bool allowDomainMismatch = false; if (Config.ssl_client.cert_error) { ACLFilledChecklist check(Config.ssl_client.cert_error, request, dash_str); + check.al = http->al; check.sslErrors = new Security::CertErrors(Security::CertError(SQUID_X509_V_ERR_DOMAIN_MISMATCH, srvCert)); + check.syncAle(request, http->log_uri); allowDomainMismatch = check.fastCheck().allowed(); delete check.sslErrors; check.sslErrors = NULL; @@ -1569,10 +1574,14 @@ clientTunnelOnError(ConnStateData *conn, Http::StreamPointer &context, HttpReque { if (conn->mayTunnelUnsupportedProto()) { ACLFilledChecklist checklist(Config.accessList.on_unsupported_protocol, request.getRaw(), nullptr); + checklist.al = (context && context->http) ? context->http->al : nullptr; checklist.requestErrorType = requestError; checklist.src_addr = conn->clientConnection->remote; checklist.my_addr = conn->clientConnection->local; checklist.conn(conn); + ClientHttpRequest *http = context ? context->http : nullptr; + const char *log_uri = http ? http->log_uri : nullptr; + checklist.syncAle(request.getRaw(), log_uri); allow_t answer = checklist.fastCheck(); if (answer.allowed() && answer.kind == 1) { debugs(33, 3, "Request will be tunneled to server"); @@ -2822,6 +2831,10 @@ ConnStateData::postHttpsAccept() HTTPMSGUNLOCK(acl_checklist->al->request); acl_checklist->al->request = request; HTTPMSGLOCK(acl_checklist->al->request); + Http::StreamPointer context = pipeline.front(); + ClientHttpRequest *http = context ? context->http : nullptr; + const char *log_uri = http ? http->log_uri : nullptr; + acl_checklist->syncAle(request, log_uri); acl_checklist->nonBlockingCheck(httpsSslBumpAccessCheckDone, this); #else fatal("FATAL: SSL-Bump requires --with-openssl"); @@ -3287,6 +3300,8 @@ ConnStateData::startPeekAndSplice() acl_checklist->banAction(allow_t(ACCESS_ALLOWED, Ssl::bumpNone)); acl_checklist->banAction(allow_t(ACCESS_ALLOWED, Ssl::bumpClientFirst)); acl_checklist->banAction(allow_t(ACCESS_ALLOWED, Ssl::bumpServerFirst)); + const char *log_uri = http ? http->log_uri : nullptr; + acl_checklist->syncAle(sslServerBump->request.getRaw(), log_uri); acl_checklist->nonBlockingCheck(httpsSslBumpStep2AccessCheckDone, this); return; } @@ -3732,6 +3747,7 @@ clientAclChecklistFill(ACLFilledChecklist &checklist, ClientHttpRequest *http) { checklist.setRequest(http->request); checklist.al = http->al; + checklist.syncAle(http->request, http->log_uri); // TODO: If http->getConn is always http->request->clientConnectionManager, // then call setIdent() inside checklist.setRequest(). Otherwise, restore diff --git a/src/client_side_request.cc b/src/client_side_request.cc index b6206104c0..81eedaf807 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -1779,8 +1779,10 @@ ClientHttpRequest::doCallouts() // Set appropriate MARKs and CONNMARKs if needed. if (getConn() && Comm::IsConnOpen(getConn()->clientConnection)) { ACLFilledChecklist ch(nullptr, request, nullptr); + ch.al = calloutContext->http->al; ch.src_addr = request->client_addr; ch.my_addr = request->my_addr; + ch.syncAle(request, log_uri); if (!calloutContext->toClientMarkingDone) { calloutContext->toClientMarkingDone = true; diff --git a/src/comm/TcpAcceptor.cc b/src/comm/TcpAcceptor.cc index fcd99ddbb6..3dc2953e63 100644 --- a/src/comm/TcpAcceptor.cc +++ b/src/comm/TcpAcceptor.cc @@ -267,6 +267,7 @@ logAcceptError(const Comm::ConnectionPointer &conn) ACLFilledChecklist ch(nullptr, nullptr, nullptr); ch.src_addr = conn->remote; ch.my_addr = conn->local; + ch.al = al; accessLogLog(al, &ch); } diff --git a/src/http.cc b/src/http.cc index d0909f8a53..1ccd70ffe5 100644 --- a/src/http.cc +++ b/src/http.cc @@ -801,7 +801,9 @@ HttpStateData::handle1xx(HttpReply *reply) // check whether the 1xx response forwarding is allowed by squid.conf if (Config.accessList.reply) { ACLFilledChecklist ch(Config.accessList.reply, originalRequest().getRaw()); + ch.al = fwd->al; ch.reply = reply; + ch.syncAle(originalRequest().getRaw(), nullptr); HTTPMSGLOCK(ch.reply); if (!ch.fastCheck().allowed()) { // TODO: support slow lookups? debugs(11, 3, HERE << "ignoring denied 1xx"); @@ -2334,6 +2336,8 @@ HttpStateData::finishingBrokenPost() } ACLFilledChecklist ch(Config.accessList.brokenPosts, originalRequest().getRaw()); + ch.al = fwd->al; + ch.syncAle(originalRequest().getRaw(), nullptr); if (!ch.fastCheck().allowed()) { debugs(11, 5, HERE << "didn't match brokenPosts"); return false; diff --git a/src/icmp/net_db.cc b/src/icmp/net_db.cc index 2e9d84381e..66034b76a1 100644 --- a/src/icmp/net_db.cc +++ b/src/icmp/net_db.cc @@ -32,6 +32,7 @@ #include "mgr/Registration.h" #include "mime_header.h" #include "neighbors.h" +#include "PeerSelectState.h" #include "SquidConfig.h" #include "SquidTime.h" #include "Store.h" @@ -1304,9 +1305,12 @@ netdbExchangeStart(void *data) } CachePeer * -netdbClosestParent(HttpRequest * request) +netdbClosestParent(PeerSelector *ps) { #if USE_ICMP + assert(ps); + HttpRequest *request = ps->request; + CachePeer *p = NULL; netdbEntry *n; const ipcache_addrs *ia; @@ -1350,7 +1354,7 @@ netdbClosestParent(HttpRequest * request) if (neighborType(p, request->url) != PEER_PARENT) continue; - if (!peerHTTPOkay(p, request)) /* not allowed */ + if (!peerHTTPOkay(p, ps)) /* not allowed */ continue; return p; diff --git a/src/icmp/net_db.h b/src/icmp/net_db.h index 33f57c6623..7bd9ab856b 100644 --- a/src/icmp/net_db.h +++ b/src/icmp/net_db.h @@ -16,6 +16,7 @@ class CachePeer; class HttpRequest; class netdbEntry; +class PeerSelector; class StoreEntry; class URL; @@ -80,7 +81,7 @@ void netdbBinaryExchange(StoreEntry *); void netdbExchangeStart(void *); void netdbExchangeUpdatePeer(Ip::Address &, CachePeer *, double, double); -CachePeer *netdbClosestParent(HttpRequest *); +CachePeer *netdbClosestParent(PeerSelector *); void netdbHostData(const char *host, int *samp, int *rtt, int *hops); #endif /* ICMP_NET_DB_H */ diff --git a/src/neighbors.cc b/src/neighbors.cc index 3c04f2e8f9..72dccf67f5 100644 --- a/src/neighbors.cc +++ b/src/neighbors.cc @@ -49,8 +49,8 @@ /* count mcast group peers every 15 minutes */ #define MCAST_COUNT_RATE 900 -bool peerAllowedToUse(const CachePeer *, HttpRequest *); -static int peerWouldBePinged(const CachePeer *, HttpRequest *); +bool peerAllowedToUse(const CachePeer *, PeerSelector *); +static int peerWouldBePinged(const CachePeer *, PeerSelector *); static void neighborRemove(CachePeer *); static void neighborAlive(CachePeer *, const MemObject *, const icp_common_t *); #if USE_HTCP @@ -134,9 +134,10 @@ neighborType(const CachePeer * p, const URL &url) * \return Whether it is appropriate to fetch REQUEST from PEER. */ bool -peerAllowedToUse(const CachePeer * p, HttpRequest * request) +peerAllowedToUse(const CachePeer * p, PeerSelector * ps) { - + assert(ps); + HttpRequest *request = ps->request; assert(request != NULL); if (neighborType(p, request->url) == PEER_SIBLING) { @@ -167,14 +168,18 @@ peerAllowedToUse(const CachePeer * p, HttpRequest * request) return true; ACLFilledChecklist checklist(p->access, request, NULL); - + checklist.al = ps->al; + checklist.syncAle(request, nullptr); return checklist.fastCheck().allowed(); } /* Return TRUE if it is okay to send an ICP request to this CachePeer. */ static int -peerWouldBePinged(const CachePeer * p, HttpRequest * request) +peerWouldBePinged(const CachePeer * p, PeerSelector * ps) { + assert(ps); + HttpRequest *request = ps->request; + if (p->icp.port == 0) return 0; @@ -196,7 +201,7 @@ peerWouldBePinged(const CachePeer * p, HttpRequest * request) if (!request->flags.hierarchical) return 0; - if (!peerAllowedToUse(p, request)) + if (!peerAllowedToUse(p, ps)) return 0; /* Ping dead peers every timeout interval */ @@ -245,12 +250,12 @@ peerConnClosed(CachePeer *p) /* Return TRUE if it is okay to send an HTTP request to this CachePeer. */ int -peerHTTPOkay(const CachePeer * p, HttpRequest * request) +peerHTTPOkay(const CachePeer * p, PeerSelector * ps) { if (!peerCanOpenMore(p) && !peerHasConnAvailable(p)) return 0; - if (!peerAllowedToUse(p, request)) + if (!peerAllowedToUse(p, ps)) return 0; if (!neighborUp(p)) @@ -260,13 +265,13 @@ peerHTTPOkay(const CachePeer * p, HttpRequest * request) } int -neighborsCount(HttpRequest * request) +neighborsCount(PeerSelector *ps) { CachePeer *p = NULL; int count = 0; for (p = Config.peers; p; p = p->next) - if (peerWouldBePinged(p, request)) + if (peerWouldBePinged(p, ps)) ++count; debugs(15, 3, "neighborsCount: " << count); @@ -275,8 +280,11 @@ neighborsCount(HttpRequest * request) } CachePeer * -getFirstUpParent(HttpRequest * request) +getFirstUpParent(PeerSelector *ps) { + assert(ps); + HttpRequest *request = ps->request; + CachePeer *p = NULL; for (p = Config.peers; p; p = p->next) { @@ -286,7 +294,7 @@ getFirstUpParent(HttpRequest * request) if (neighborType(p, request->url) != PEER_PARENT) continue; - if (!peerHTTPOkay(p, request)) + if (!peerHTTPOkay(p, ps)) continue; break; @@ -297,8 +305,11 @@ getFirstUpParent(HttpRequest * request) } CachePeer * -getRoundRobinParent(HttpRequest * request) +getRoundRobinParent(PeerSelector *ps) { + assert(ps); + HttpRequest *request = ps->request; + CachePeer *p; CachePeer *q = NULL; @@ -309,7 +320,7 @@ getRoundRobinParent(HttpRequest * request) if (neighborType(p, request->url) != PEER_PARENT) continue; - if (!peerHTTPOkay(p, request)) + if (!peerHTTPOkay(p, ps)) continue; if (p->weight == 0) @@ -336,8 +347,11 @@ getRoundRobinParent(HttpRequest * request) } CachePeer * -getWeightedRoundRobinParent(HttpRequest * request) +getWeightedRoundRobinParent(PeerSelector *ps) { + assert(ps); + HttpRequest *request = ps->request; + CachePeer *p; CachePeer *q = NULL; int weighted_rtt; @@ -349,7 +363,7 @@ getWeightedRoundRobinParent(HttpRequest * request) if (neighborType(p, request->url) != PEER_PARENT) continue; - if (!peerHTTPOkay(p, request)) + if (!peerHTTPOkay(p, ps)) continue; if (q && q->rr_count < p->rr_count) @@ -451,8 +465,11 @@ peerAlive(CachePeer *p) } CachePeer * -getDefaultParent(HttpRequest * request) +getDefaultParent(PeerSelector *ps) { + assert(ps); + HttpRequest *request = ps->request; + CachePeer *p = NULL; for (p = Config.peers; p; p = p->next) { @@ -462,7 +479,7 @@ getDefaultParent(HttpRequest * request) if (!p->options.default_parent) continue; - if (!peerHTTPOkay(p, request)) + if (!peerHTTPOkay(p, ps)) continue; debugs(15, 3, "getDefaultParent: returning " << p->host); @@ -573,7 +590,7 @@ int neighborsUdpPing(HttpRequest * request, StoreEntry * entry, IRCB * callback, - void *callback_data, + PeerSelector *ps, int *exprep, int *timeout) { @@ -599,7 +616,7 @@ neighborsUdpPing(HttpRequest * request, mem->ping_reply_callback = callback; - mem->ircb_data = callback_data; + mem->ircb_data = ps; reqnum = icpSetCacheKey((const cache_key *)entry->key); @@ -609,7 +626,7 @@ neighborsUdpPing(HttpRequest * request, debugs(15, 5, "neighborsUdpPing: Peer " << p->host); - if (!peerWouldBePinged(p, request)) + if (!peerWouldBePinged(p, ps)) continue; /* next CachePeer */ ++peers_pinged; @@ -734,9 +751,11 @@ neighborsUdpPing(HttpRequest * request, /* lookup the digest of a given CachePeer */ lookup_t -peerDigestLookup(CachePeer * p, HttpRequest * request) +peerDigestLookup(CachePeer * p, PeerSelector * ps) { #if USE_CACHE_DIGESTS + assert(ps); + HttpRequest *request = ps->request; const cache_key *key = request ? storeKeyPublicByRequest(request) : NULL; assert(p); assert(request); @@ -746,7 +765,7 @@ peerDigestLookup(CachePeer * p, HttpRequest * request) if (!p->digest) { debugs(15, 5, "peerDigestLookup: gone!"); return LOOKUP_NONE; - } else if (!peerHTTPOkay(p, request)) { + } else if (!peerHTTPOkay(p, ps)) { debugs(15, 5, "peerDigestLookup: !peerHTTPOkay"); return LOOKUP_NONE; } else if (!p->digest->flags.needed) { @@ -776,10 +795,12 @@ peerDigestLookup(CachePeer * p, HttpRequest * request) /* select best CachePeer based on cache digests */ CachePeer * -neighborsDigestSelect(HttpRequest * request) +neighborsDigestSelect(PeerSelector *ps) { CachePeer *best_p = NULL; #if USE_CACHE_DIGESTS + assert(ps); + HttpRequest *request = ps->request; int best_rtt = 0; int choice_count = 0; @@ -802,7 +823,7 @@ neighborsDigestSelect(HttpRequest * request) if (i == 1) first_ping = p; - lookup = peerDigestLookup(p, request); + lookup = peerDigestLookup(p, ps); if (lookup == LOOKUP_NONE) continue; @@ -952,7 +973,7 @@ neighborIgnoreNonPeer(const Ip::Address &from, icp_opcode opcode) * * from being used */ static int -ignoreMulticastReply(CachePeer * p, MemObject * mem) +ignoreMulticastReply(CachePeer * p, PeerSelector * ps) { if (p == NULL) return 0; @@ -960,7 +981,7 @@ ignoreMulticastReply(CachePeer * p, MemObject * mem) if (!p->options.mcast_responder) return 0; - if (peerHTTPOkay(p, mem->request.getRaw())) + if (peerHTTPOkay(p, ps)) return 0; return 1; @@ -1031,13 +1052,19 @@ neighborsUdpAck(const cache_key * key, icp_common_t * header, const Ip::Address return; } + if (!mem->ircb_data) { + debugs(12, DBG_IMPORTANT, "BUG: missing ICP callback data for " << *entry); + neighborCountIgnored(p); + return; + } + debugs(15, 3, "neighborsUdpAck: " << opcode_d << " for '" << storeKeyText(key) << "' from " << (p ? p->host : "source") << " "); if (p) { ntype = neighborType(p, mem->request->url); } - if (ignoreMulticastReply(p, mem)) { + if (ignoreMulticastReply(p, mem->ircb_data)) { neighborCountIgnored(p); } else if (opcode == ICP_MISS) { if (p == NULL) { @@ -1741,12 +1768,18 @@ neighborsHtcpReply(const cache_key * key, HtcpReplyData * htcp, const Ip::Addres return; } + if (!mem->ircb_data) { + debugs(12, DBG_IMPORTANT, "BUG: missing HTCP callback data for " << *e); + neighborCountIgnored(p); + return; + } + if (p) { ntype = neighborType(p, mem->request->url); neighborUpdateRtt(p, mem); } - if (ignoreMulticastReply(p, mem)) { + if (ignoreMulticastReply(p, mem->ircb_data)) { neighborCountIgnored(p); return; } diff --git a/src/neighbors.h b/src/neighbors.h index 5edd315b8e..f2e748e201 100644 --- a/src/neighbors.h +++ b/src/neighbors.h @@ -21,16 +21,17 @@ class HttpRequestMethod; class CachePeer; class StoreEntry; class URL; +class PeerSelector; CachePeer *getFirstPeer(void); -CachePeer *getFirstUpParent(HttpRequest *); +CachePeer *getFirstUpParent(PeerSelector *); CachePeer *getNextPeer(CachePeer *); -CachePeer *getSingleParent(HttpRequest *); -int neighborsCount(HttpRequest *); +CachePeer *getSingleParent(PeerSelector *); +int neighborsCount(PeerSelector *); int neighborsUdpPing(HttpRequest *, StoreEntry *, IRCB * callback, - void *data, + PeerSelector *ps, int *exprep, int *timeout); void neighborAddAcl(const char *, const char *); @@ -43,13 +44,13 @@ void neighborsHtcpClear(StoreEntry *, const char *, HttpRequest *, const HttpReq #endif CachePeer *peerFindByName(const char *); CachePeer *peerFindByNameAndPort(const char *, unsigned short); -CachePeer *getDefaultParent(HttpRequest * request); -CachePeer *getRoundRobinParent(HttpRequest * request); -CachePeer *getWeightedRoundRobinParent(HttpRequest * request); +CachePeer *getDefaultParent(PeerSelector*); +CachePeer *getRoundRobinParent(PeerSelector*); +CachePeer *getWeightedRoundRobinParent(PeerSelector*); void peerClearRRStart(void); void peerClearRR(void); -lookup_t peerDigestLookup(CachePeer * p, HttpRequest * request); -CachePeer *neighborsDigestSelect(HttpRequest * request); +lookup_t peerDigestLookup(CachePeer * p, PeerSelector *); +CachePeer *neighborsDigestSelect(PeerSelector *); void peerNoteDigestLookup(HttpRequest * request, CachePeer * p, lookup_t lookup); void peerNoteDigestGone(CachePeer * p); int neighborUp(const CachePeer * e); @@ -58,7 +59,7 @@ peer_t neighborType(const CachePeer *, const URL &); void peerConnectFailed(CachePeer *); void peerConnectSucceded(CachePeer *); void dump_peer_options(StoreEntry *, CachePeer *); -int peerHTTPOkay(const CachePeer *, HttpRequest *); +int peerHTTPOkay(const CachePeer *, PeerSelector *); // TODO: Consider moving this method to CachePeer class. /// \returns the effective connect timeout for the given peer diff --git a/src/peer_select.cc b/src/peer_select.cc index 0ac9a09061..b49f9fd0cc 100644 --- a/src/peer_select.cc +++ b/src/peer_select.cc @@ -134,8 +134,11 @@ PeerSelector::~PeerSelector() } static int -peerSelectIcpPing(HttpRequest * request, int direct, StoreEntry * entry) +peerSelectIcpPing(PeerSelector *ps, int direct, StoreEntry * entry) { + assert(ps); + HttpRequest *request = ps->request; + int n; assert(entry); assert(entry->ping_status == PING_NONE); @@ -149,7 +152,7 @@ peerSelectIcpPing(HttpRequest * request, int direct, StoreEntry * entry) if (direct != DIRECT_NO) return 0; - n = neighborsCount(request); + n = neighborsCount(ps); debugs(44, 3, "counted " << n << " neighbors"); @@ -465,6 +468,7 @@ PeerSelector::selectMore() ACLFilledChecklist *ch = new ACLFilledChecklist(Config.accessList.AlwaysDirect, request, NULL); ch->al = al; acl_checklist = ch; + acl_checklist->syncAle(request, nullptr); acl_checklist->nonBlockingCheck(CheckAlwaysDirectDone, this); return; } else if (never_direct == ACCESS_DUNNO) { @@ -473,6 +477,7 @@ PeerSelector::selectMore() ACLFilledChecklist *ch = new ACLFilledChecklist(Config.accessList.NeverDirect, request, NULL); ch->al = al; acl_checklist = ch; + acl_checklist->syncAle(request, nullptr); acl_checklist->nonBlockingCheck(CheckNeverDirectDone, this); return; } else if (request->flags.noDirect) { @@ -539,7 +544,7 @@ PeerSelector::selectMore() resolveSelected(); } -bool peerAllowedToUse(const CachePeer * p, HttpRequest * request); +bool peerAllowedToUse(const CachePeer *, PeerSelector*); /// Selects a pinned connection if it exists, is valid, and is allowed. void @@ -550,7 +555,7 @@ PeerSelector::selectPinned() return; CachePeer *pear = request->pinnedConnection()->pinnedPeer(); if (Comm::IsConnOpen(request->pinnedConnection()->validatePinnedConnection(request, pear))) { - const bool usePinned = pear ? peerAllowedToUse(pear, request) : (direct != DIRECT_NO); + const bool usePinned = pear ? peerAllowedToUse(pear, this) : (direct != DIRECT_NO); if (usePinned) { addSelection(pear, PINNED); if (entry) @@ -582,16 +587,16 @@ PeerSelector::selectSomeNeighbor() } #if USE_CACHE_DIGESTS - if ((p = neighborsDigestSelect(request))) { + if ((p = neighborsDigestSelect(this))) { if (neighborType(p, request->url) == PEER_PARENT) code = CD_PARENT_HIT; else code = CD_SIBLING_HIT; } else #endif - if ((p = netdbClosestParent(request))) { + if ((p = netdbClosestParent(this))) { code = CLOSEST_PARENT; - } else if (peerSelectIcpPing(request, direct, entry)) { + } else if (peerSelectIcpPing(this, direct, entry)) { debugs(44, 3, "Doing ICP pings"); ping.start = current_time; ping.n_sent = neighborsUdpPing(request, @@ -681,21 +686,21 @@ PeerSelector::selectSomeParent() if (direct == DIRECT_YES) return; - if ((p = peerSourceHashSelectParent(request))) { + if ((p = peerSourceHashSelectParent(this))) { code = SOURCEHASH_PARENT; #if USE_AUTH - } else if ((p = peerUserHashSelectParent(request))) { + } else if ((p = peerUserHashSelectParent(this))) { code = USERHASH_PARENT; #endif - } else if ((p = carpSelectParent(request))) { + } else if ((p = carpSelectParent(this))) { code = CARP; - } else if ((p = getRoundRobinParent(request))) { + } else if ((p = getRoundRobinParent(this))) { code = ROUNDROBIN_PARENT; - } else if ((p = getWeightedRoundRobinParent(request))) { + } else if ((p = getWeightedRoundRobinParent(this))) { code = ROUNDROBIN_PARENT; - } else if ((p = getFirstUpParent(request))) { + } else if ((p = getFirstUpParent(this))) { code = FIRSTUP_PARENT; - } else if ((p = getDefaultParent(request))) { + } else if ((p = getDefaultParent(this))) { code = DEFAULT_PARENT; } @@ -719,7 +724,7 @@ PeerSelector::selectAllParents() if (neighborType(p, request->url) != PEER_PARENT) continue; - if (!peerHTTPOkay(p, request)) + if (!peerHTTPOkay(p, this)) continue; addSelection(p, ANY_OLD_PARENT); @@ -730,7 +735,7 @@ PeerSelector::selectAllParents() * simply are not configured to handle the request. */ /* Add default parent as a last resort */ - if ((p = getDefaultParent(request))) { + if ((p = getDefaultParent(this))) { addSelection(p, DEFAULT_PARENT); } } @@ -814,7 +819,7 @@ PeerSelector::handleIcpReply(CachePeer *p, const peer_t type, icp_common_t *head if (p && request) peerNoteDigestLookup(request, p, - peerDigestLookup(p, request, entry)); + peerDigestLookup(p, this)); #endif diff --git a/src/peer_sourcehash.cc b/src/peer_sourcehash.cc index 1f4246ccfe..5176ac1cfa 100644 --- a/src/peer_sourcehash.cc +++ b/src/peer_sourcehash.cc @@ -13,6 +13,7 @@ #include "HttpRequest.h" #include "mgr/Registration.h" #include "neighbors.h" +#include "PeerSelectState.h" #include "SquidConfig.h" #include "Store.h" @@ -141,7 +142,7 @@ peerSourceHashRegisterWithCacheManager(void) } CachePeer * -peerSourceHashSelectParent(HttpRequest * request) +peerSourceHashSelectParent(PeerSelector *ps) { int k; const char *c; @@ -157,6 +158,9 @@ peerSourceHashSelectParent(HttpRequest * request) if (n_sourcehash_peers == 0) return NULL; + assert(ps); + HttpRequest *request = ps->request; + key = request->client_addr.toStr(ntoabuf, sizeof(ntoabuf)); /* calculate hash key */ @@ -175,7 +179,7 @@ peerSourceHashSelectParent(HttpRequest * request) debugs(39, 3, "peerSourceHashSelectParent: " << tp->name << " combined_hash " << combined_hash << " score " << std::setprecision(0) << score); - if ((score > high_score) && peerHTTPOkay(tp, request)) { + if ((score > high_score) && peerHTTPOkay(tp, ps)) { p = tp; high_score = score; } diff --git a/src/peer_sourcehash.h b/src/peer_sourcehash.h index cf8579dcdb..e8d163078d 100644 --- a/src/peer_sourcehash.h +++ b/src/peer_sourcehash.h @@ -12,10 +12,10 @@ #define SQUID_PEER_SOURCEHASH_H_ class CachePeer; -class HttpRequest; +class PeerSelector; void peerSourceHashInit(void); -CachePeer * peerSourceHashSelectParent(HttpRequest * request); +CachePeer * peerSourceHashSelectParent(PeerSelector*); #endif /* SQUID_PEER_SOURCEHASH_H_ */ diff --git a/src/peer_userhash.cc b/src/peer_userhash.cc index f2ca986c82..d92c1de113 100644 --- a/src/peer_userhash.cc +++ b/src/peer_userhash.cc @@ -18,6 +18,7 @@ #include "HttpRequest.h" #include "mgr/Registration.h" #include "neighbors.h" +#include "PeerSelectState.h" #include "SquidConfig.h" #include "Store.h" @@ -146,7 +147,7 @@ peerUserHashRegisterWithCacheManager(void) } CachePeer * -peerUserHashSelectParent(HttpRequest * request) +peerUserHashSelectParent(PeerSelector *ps) { int k; const char *c; @@ -161,6 +162,9 @@ peerUserHashSelectParent(HttpRequest * request) if (n_userhash_peers == 0) return NULL; + assert(ps); + HttpRequest *request = ps->request; + if (request->auth_user_request != NULL) key = request->auth_user_request->username(); @@ -183,7 +187,7 @@ peerUserHashSelectParent(HttpRequest * request) debugs(39, 3, "peerUserHashSelectParent: " << tp->name << " combined_hash " << combined_hash << " score " << std::setprecision(0) << score); - if ((score > high_score) && peerHTTPOkay(tp, request)) { + if ((score > high_score) && peerHTTPOkay(tp, ps)) { p = tp; high_score = score; } diff --git a/src/peer_userhash.h b/src/peer_userhash.h index 8b1fc08a87..e9cf3f44fe 100644 --- a/src/peer_userhash.h +++ b/src/peer_userhash.h @@ -13,9 +13,10 @@ class CachePeer; class HttpRequest; +class PeerSelector; void peerUserHashInit(void); -CachePeer * peerUserHashSelectParent(HttpRequest * request); +CachePeer * peerUserHashSelectParent(PeerSelector *); #endif /* SQUID_PEER_USERHASH_H_ */ diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc index 11b9c4e575..4c63c5d87a 100644 --- a/src/security/PeerConnector.cc +++ b/src/security/PeerConnector.cc @@ -131,6 +131,7 @@ Security::PeerConnector::initialize(Security::SessionPointer &serverSession) if (acl_access *acl = ::Config.ssl_client.cert_error) { ACLFilledChecklist *check = new ACLFilledChecklist(acl, request.getRaw(), dash_str); check->al = al; + check->syncAle(request.getRaw(), nullptr); // check->fd(fd); XXX: need client FD here SSL_set_ex_data(serverSession.get(), ssl_ex_index_cert_error_check, check); } @@ -324,6 +325,7 @@ Security::PeerConnector::sslCrtvdCheckForErrors(Ssl::CertValidationResponse cons if (acl_access *acl = ::Config.ssl_client.cert_error) { check = new ACLFilledChecklist(acl, request.getRaw(), dash_str); check->al = al; + check->syncAle(request.getRaw(), nullptr); } Security::CertErrors *errs = nullptr; diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc index eb95af8d1a..a79a2fff92 100644 --- a/src/ssl/PeekingPeerConnector.cc +++ b/src/ssl/PeekingPeerConnector.cc @@ -70,6 +70,7 @@ Ssl::PeekingPeerConnector::checkForPeekAndSplice() acl_checklist->banAction(allow_t(ACCESS_ALLOWED, Ssl::bumpSplice)); if (!srvBio->canBump()) acl_checklist->banAction(allow_t(ACCESS_ALLOWED, Ssl::bumpBump)); + acl_checklist->syncAle(request.getRaw(), nullptr); acl_checklist->nonBlockingCheck(Ssl::PeekingPeerConnector::cbCheckForPeekAndSpliceDone, this); } diff --git a/src/tests/stub_carp.cc b/src/tests/stub_carp.cc index 39e25ad9f4..98aa4b68b5 100644 --- a/src/tests/stub_carp.cc +++ b/src/tests/stub_carp.cc @@ -12,8 +12,8 @@ #include "tests/STUB.h" class CachePeer; -class HttpRequest; +class PeerSelector; void carpInit(void) STUB -CachePeer * carpSelectParent(HttpRequest *) STUB_RETVAL(NULL) +CachePeer *carpSelectParent(PeerSelector *ps) STUB_RETVAL(nullptr) diff --git a/src/tests/stub_libicmp.cc b/src/tests/stub_libicmp.cc index d9f716c271..66b1ab2e1b 100644 --- a/src/tests/stub_libicmp.cc +++ b/src/tests/stub_libicmp.cc @@ -35,6 +35,6 @@ void netdbDeleteAddrNetwork(Ip::Address &addr) STUB void netdbBinaryExchange(StoreEntry *) STUB void netdbExchangeStart(void *) STUB void netdbExchangeUpdatePeer(Ip::Address &, CachePeer *, double, double) STUB -CachePeer *netdbClosestParent(HttpRequest *) STUB_RETVAL(NULL) +CachePeer *netdbClosestParent(PeerSelector *) STUB_RETVAL(nullptr) void netdbHostData(const char *host, int *samp, int *rtt, int *hops) STUB diff --git a/src/tunnel.cc b/src/tunnel.cc index f13da5fd45..28bfb4b745 100644 --- a/src/tunnel.cc +++ b/src/tunnel.cc @@ -1102,8 +1102,10 @@ tunnelStart(ClientHttpRequest * http) * default is to allow. */ ACLFilledChecklist ch(Config.accessList.miss, request, NULL); + ch.al = http->al; ch.src_addr = request->client_addr; ch.my_addr = request->my_addr; + ch.syncAle(request, http->log_uri); if (ch.fastCheck().denied()) { debugs(26, 4, HERE << "MISS access forbidden."); err = new ErrorState(ERR_FORWARDING_DENIED, Http::scForbidden, request); -- 2.47.2