From: Alex Rousskov Date: Mon, 26 Jun 2017 00:10:34 +0000 (-0600) Subject: Minimize direct comparisons with ACCESS_ALLOWED and ACCESS_DENIED. X-Git-Tag: M-staged-PR71~95 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=06bf53844a91f9ec4d6a56f0c0a73317f964edc5;p=thirdparty%2Fsquid.git Minimize direct comparisons with ACCESS_ALLOWED and ACCESS_DENIED. No functionality changes expected. Added allow_t API to avoid direct comparisons with ACCESS_ALLOWED and ACCESS_DENIED. Developers using direct comparisons eventually mishandle exceptional ACCESS_DUNNO and ACCESS_AUTH_REQUIRED cases where neither "allow" nor "deny" rule matched. The new API cannot fully prevent such bugs, but should either led the developer to the right choice (usually .allowed()) or alert the reviewer about an unusual choice (i.e., denied()). The vast majority of checks use allowed(), but we could not eliminate the remaining denied() cases ("miss_access" and "cache" directives) for backward compatibility reasons -- previously "working" deployments may suddenly start blocking cache misses and/or stop caching: http://lists.squid-cache.org/pipermail/squid-dev/2017-May/008576.html --- diff --git a/src/DelayId.cc b/src/DelayId.cc index aa54fe4e66..50321ce045 100644 --- a/src/DelayId.cc +++ b/src/DelayId.cc @@ -101,7 +101,7 @@ DelayId::DelayClient(ClientHttpRequest * http, HttpReply *reply) if (http->getConn() != NULL) ch.conn(http->getConn()); - if (DelayPools::delay_data[pool].theComposite().getRaw() && ch.fastCheck() == ACCESS_ALLOWED) { + if (DelayPools::delay_data[pool].theComposite().getRaw() && ch.fastCheck().allowed()) { DelayId result (pool + 1); CompositePoolNode::CompositeSelectionDetails details; diff --git a/src/FwdState.cc b/src/FwdState.cc index 631bf8e0d8..206cbdc7b9 100644 --- a/src/FwdState.cc +++ b/src/FwdState.cc @@ -332,7 +332,7 @@ FwdState::Start(const Comm::ConnectionPointer &clientConn, StoreEntry *entry, Ht */ ACLFilledChecklist ch(Config.accessList.miss, request, NULL); ch.src_addr = request->client_addr; - if (ch.fastCheck() == ACCESS_DENIED) { + if (ch.fastCheck().denied()) { err_type page_id; page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, 1); @@ -1218,7 +1218,7 @@ FwdState::pconnPop(const Comm::ConnectionPointer &dest, const char *domain) bool retriable = checkRetriable(); if (!retriable && Config.accessList.serverPconnForNonretriable) { ACLFilledChecklist ch(Config.accessList.serverPconnForNonretriable, request, NULL); - retriable = (ch.fastCheck() == ACCESS_ALLOWED); + retriable = ch.fastCheck().allowed(); } // always call shared pool first because we need to close an idle // connection there if we have to use a standby connection. @@ -1270,7 +1270,7 @@ tos_t aclMapTOS(acl_tos * head, ACLChecklist * ch) { for (acl_tos *l = head; l; l = l->next) { - if (!l->aclList || ch->fastCheck(l->aclList) == ACCESS_ALLOWED) + if (!l->aclList || ch->fastCheck(l->aclList).allowed()) return l->tos; } @@ -1282,7 +1282,7 @@ nfmark_t aclMapNfmark(acl_nfmark * head, ACLChecklist * ch) { for (acl_nfmark *l = head; l; l = l->next) { - if (!l->aclList || ch->fastCheck(l->aclList) == ACCESS_ALLOWED) + if (!l->aclList || ch->fastCheck(l->aclList).allowed()) return l->nfmark; } @@ -1333,7 +1333,7 @@ getOutgoingAddress(HttpRequest * request, Comm::ConnectionPointer conn) if (conn->remote.isIPv4() != l->addr.isIPv4()) continue; /* check ACLs for this outgoing address */ - if (!l->aclList || ch.fastCheck(l->aclList) == ACCESS_ALLOWED) { + if (!l->aclList || ch.fastCheck(l->aclList).allowed()) { conn->local = l->addr; return; } diff --git a/src/HttpHeaderTools.cc b/src/HttpHeaderTools.cc index c1669042e9..a5bea3dfec 100644 --- a/src/HttpHeaderTools.cc +++ b/src/HttpHeaderTools.cc @@ -289,7 +289,7 @@ httpHdrMangle(HttpHeaderEntry * e, HttpRequest * request, HeaderManglers *hms) ACLFilledChecklist checklist(hm->access_list, request, NULL); - if (checklist.fastCheck() == ACCESS_ALLOWED) { + if (checklist.fastCheck().allowed()) { /* aclCheckFast returns true for allow. */ debugs(66, 7, "checklist for mangler is positive. Mangle"); retval = 1; @@ -479,7 +479,7 @@ httpHdrAdd(HttpHeader *heads, HttpRequest *request, const AccessLogEntryPointer ACLFilledChecklist checklist(NULL, request, NULL); for (HeaderWithAclList::const_iterator hwa = headersAdd.begin(); hwa != headersAdd.end(); ++hwa) { - if (!hwa->aclList || checklist.fastCheck(hwa->aclList) == ACCESS_ALLOWED) { + if (!hwa->aclList || checklist.fastCheck(hwa->aclList).allowed()) { const char *fieldValue = NULL; MemBuf mb; if (hwa->quoted) { diff --git a/src/HttpReply.cc b/src/HttpReply.cc index 2d69207be0..8b5c222519 100644 --- a/src/HttpReply.cc +++ b/src/HttpReply.cc @@ -517,7 +517,7 @@ HttpReply::calcMaxBodySize(HttpRequest& request) const HTTPMSGLOCK(ch.reply); for (AclSizeLimit *l = Config.ReplyBodySize; l; l = l -> next) { /* if there is no ACL list or if the ACLs listed match use this size value */ - if (!l->aclList || ch.fastCheck(l->aclList) == ACCESS_ALLOWED) { + if (!l->aclList || ch.fastCheck(l->aclList).allowed()) { debugs(58, 4, HERE << "bodySizeMax=" << bodySizeMax); bodySizeMax = l->size; // may be -1 break; diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index 6daf43885e..00c0fdb388 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -609,7 +609,7 @@ HttpRequest::getRangeOffsetLimit() for (AclSizeLimit *l = Config.rangeOffsetLimit; l; l = l -> next) { /* if there is no ACL list or if the ACLs listed match use this limit value */ - if (!l->aclList || ch.fastCheck(l->aclList) == ACCESS_ALLOWED) { + if (!l->aclList || ch.fastCheck(l->aclList).allowed()) { debugs(58, 4, HERE << "rangeOffsetLimit=" << rangeOffsetLimit); rangeOffsetLimit = l->size; // may be -1 break; @@ -724,7 +724,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; - flags.spoofClientIp = (checklist->fastCheck() == ACCESS_ALLOWED); + flags.spoofClientIp = checklist->fastCheck().allowed(); delete checklist; } else flags.spoofClientIp = true; diff --git a/src/Notes.cc b/src/Notes.cc index c1b25bc679..7d6d1fc079 100644 --- a/src/Notes.cc +++ b/src/Notes.cc @@ -75,10 +75,10 @@ Note::match(HttpRequest *request, HttpReply *reply, const AccessLogEntry::Pointe for (auto v: values) { assert(v->aclList); - const int ret = ch.fastCheck(v->aclList); + const auto ret = ch.fastCheck(v->aclList); debugs(93, 5, "Check for header name: " << theKey << ": " << v->value() << ", HttpRequest: " << request << " HttpReply: " << reply << " matched: " << ret); - if (ret == ACCESS_ALLOWED) { + if (ret.allowed()) { matched = v->format(al); return true; } diff --git a/src/acl/Acl.h b/src/acl/Acl.h index 9ed1b19c30..57300d6c87 100644 --- a/src/acl/Acl.h +++ b/src/acl/Acl.h @@ -16,6 +16,7 @@ #include "dlink.h" #include "sbuf/forward.h" +#include #include class ConfigParser; @@ -133,6 +134,22 @@ public: return code; } + /// Whether an "allow" rule matched. If in doubt, use this popular method. + /// Also use this method to treat exceptional ACCESS_DUNNO and + /// ACCESS_AUTH_REQUIRED outcomes as if a "deny" rule matched. + /// See also: denied(). + bool allowed() const { return code == ACCESS_ALLOWED; } + + /// Whether a "deny" rule matched. Avoid this rarely used method. + /// Use this method (only) to treat exceptional ACCESS_DUNNO and + /// ACCESS_AUTH_REQUIRED outcomes as if an "allow" rule matched. + /// See also: allowed(). + bool denied() const { return code == ACCESS_DENIED; } + + /// whether there was either a default rule, a rule without any ACLs, or a + /// a rule with ACLs that all matched + bool someRuleMatched() const { return allowed() || denied(); } + aclMatchCode code; ///< ACCESS_* code int kind; ///< which custom access list verb matched }; diff --git a/src/acl/Tree.h b/src/acl/Tree.h index a554f26d88..34f4796833 100644 --- a/src/acl/Tree.h +++ b/src/acl/Tree.h @@ -52,7 +52,7 @@ protected: inline const char * AllowOrDeny(const allow_t &action) { - return action == ACCESS_ALLOWED ? "allow" : "deny"; + return action.allowed() ? "allow" : "deny"; } template diff --git a/src/adaptation/AccessCheck.cc b/src/adaptation/AccessCheck.cc index 6ce6d91975..97242dbe6a 100644 --- a/src/adaptation/AccessCheck.cc +++ b/src/adaptation/AccessCheck.cc @@ -174,7 +174,7 @@ Adaptation::AccessCheck::noteAnswer(allow_t answer) Must(!candidates.empty()); // the candidate we were checking must be there debugs(93,5, HERE << topCandidate() << " answer=" << answer); - if (answer == ACCESS_ALLOWED) { // the rule matched + if (answer.allowed()) { // the rule matched ServiceGroupPointer g = topGroup(); if (g != NULL) { // the corresponding group found callBack(g); diff --git a/src/adaptation/icap/Launcher.cc b/src/adaptation/icap/Launcher.cc index 4b9c51d213..0b19e96618 100644 --- a/src/adaptation/icap/Launcher.cc +++ b/src/adaptation/icap/Launcher.cc @@ -145,7 +145,7 @@ bool Adaptation::Icap::Launcher::canRepeat(Adaptation::Icap::XactAbortInfo &info cl->reply = info.icapReply; HTTPMSGLOCK(cl->reply); - bool result = cl->fastCheck() == ACCESS_ALLOWED; + bool result = cl->fastCheck().allowed(); delete cl; return result; } diff --git a/src/auth/UserRequest.cc b/src/auth/UserRequest.cc index 9f6970a79e..195cfb8559 100644 --- a/src/auth/UserRequest.cc +++ b/src/auth/UserRequest.cc @@ -469,7 +469,7 @@ schemesConfig(HttpRequest *request, HttpReply *rep) ch.reply = rep; HTTPMSGLOCK(ch.reply); const allow_t answer = ch.fastCheck(Auth::TheConfig.schemeAccess); - if (answer == ACCESS_ALLOWED) + if (answer.allowed()) return Auth::TheConfig.schemeLists.at(answer.kind).authConfigs; } return Auth::TheConfig.schemes; diff --git a/src/client_side.cc b/src/client_side.cc index 2ec8da96ae..700c54bcca 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -462,7 +462,7 @@ ClientHttpRequest::logRequest() statsCheck.reply = al->reply; HTTPMSGLOCK(statsCheck.reply); } - updatePerformanceCounters = (statsCheck.fastCheck() == ACCESS_ALLOWED); + updatePerformanceCounters = statsCheck.fastCheck().allowed(); } if (updatePerformanceCounters) { @@ -1527,7 +1527,7 @@ bool ConnStateData::serveDelayedError(Http::Stream *context) if (Config.ssl_client.cert_error) { ACLFilledChecklist check(Config.ssl_client.cert_error, request, dash_str); check.sslErrors = new Security::CertErrors(Security::CertError(SQUID_X509_V_ERR_DOMAIN_MISMATCH, srvCert)); - allowDomainMismatch = (check.fastCheck() == ACCESS_ALLOWED); + allowDomainMismatch = check.fastCheck().allowed(); delete check.sslErrors; check.sslErrors = NULL; } @@ -1581,7 +1581,7 @@ clientTunnelOnError(ConnStateData *conn, Http::StreamPointer &context, HttpReque checklist.my_addr = conn->clientConnection->local; checklist.conn(conn); allow_t answer = checklist.fastCheck(); - if (answer == ACCESS_ALLOWED && answer.kind == 1) { + if (answer.allowed() && answer.kind == 1) { debugs(33, 3, "Request will be tunneled to server"); if (context) { assert(conn->pipeline.front() == context); // XXX: still assumes HTTP/1 semantics @@ -1826,7 +1826,7 @@ ConnStateData::proxyProtocolValidateClient() ch.my_addr = clientConnection->local; ch.conn(this); - if (ch.fastCheck() != ACCESS_ALLOWED) + if (!ch.fastCheck().allowed()) return proxyProtocolError("PROXY client not permitted by ACLs"); return true; @@ -2446,7 +2446,7 @@ ConnStateData::whenClientIpKnown() ACLFilledChecklist identChecklist(Ident::TheConfig.identLookup, NULL, NULL); identChecklist.src_addr = clientConnection->remote; identChecklist.my_addr = clientConnection->local; - if (identChecklist.fastCheck() == ACCESS_ALLOWED) + if (identChecklist.fastCheck().allowed()) Ident::Start(clientConnection, clientIdentDone, this); } #endif @@ -2474,7 +2474,7 @@ ConnStateData::whenClientIpKnown() if (pools[pool]->access) { ch.changeAcl(pools[pool]->access); allow_t answer = ch.fastCheck(); - if (answer == ACCESS_ALLOWED) { + if (answer.allowed()) { /* request client information from db after we did all checks this will save hash lookup if client failed checks */ @@ -2706,7 +2706,7 @@ httpsSslBumpAccessCheckDone(allow_t answer, void *data) if (!connState->isOpen()) return; - if (answer == ACCESS_ALLOWED) { + if (answer.allowed()) { debugs(33, 2, "sslBump action " << Ssl::bumpMode(answer.kind) << "needed for " << connState->clientConnection); connState->sslBumpMode = static_cast(answer.kind); } else { @@ -2862,7 +2862,7 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer (ca->alg == Ssl::algSetValidBefore && certProperties.setValidBefore) ) continue; - if (ca->aclList && checklist.fastCheck(ca->aclList) == ACCESS_ALLOWED) { + if (ca->aclList && checklist.fastCheck(ca->aclList).allowed()) { const char *alg = Ssl::CertAdaptAlgorithmStr[ca->alg]; const char *param = ca->param; @@ -2885,7 +2885,7 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer certProperties.signAlgorithm = Ssl::algSignEnd; for (sslproxy_cert_sign *sg = Config.ssl_client.cert_sign; sg != NULL; sg = sg->next) { - if (sg->aclList && checklist.fastCheck(sg->aclList) == ACCESS_ALLOWED) { + if (sg->aclList && checklist.fastCheck(sg->aclList).allowed()) { certProperties.signAlgorithm = (Ssl::CertSignAlgorithm)sg->alg; break; } @@ -3170,7 +3170,7 @@ void httpsSslBumpStep2AccessCheckDone(allow_t answer, void *data) debugs(33, 5, "Answer: " << answer << " kind:" << answer.kind); assert(connState->serverBump()); Ssl::BumpMode bumpAction; - if (answer == ACCESS_ALLOWED) { + if (answer.allowed()) { bumpAction = (Ssl::BumpMode)answer.kind; } else bumpAction = Ssl::bumpSplice; diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc index 81fc88dfc1..875118d220 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -873,7 +873,7 @@ clientReplyContext::blockedHit() const std::unique_ptr chl(clientAclChecklistCreate(Config.accessList.sendHit, http)); chl->reply = const_cast(rep); // ACLChecklist API bug HTTPMSGLOCK(chl->reply); - return chl->fastCheck() != ACCESS_ALLOWED; // when in doubt, block + return !chl->fastCheck().allowed(); // when in doubt, block } // This does not happen, I hope, because we are called from CacheHit, which @@ -2097,7 +2097,7 @@ clientReplyContext::processReplyAccessResult(const allow_t &accessAllowed) << ' ' << http->uri << " is " << accessAllowed << ", because it matched " << (AclMatchedName ? AclMatchedName : "NO ACL's")); - if (accessAllowed != ACCESS_ALLOWED) { + if (!accessAllowed.allowed()) { ErrorState *err; err_type page_id; page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, 1); diff --git a/src/client_side_request.cc b/src/client_side_request.cc index d4628cbcc2..2822d1a7a8 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -449,13 +449,7 @@ clientFollowXForwardedForCheck(allow_t answer, void *data) ClientHttpRequest *http = calloutContext->http; HttpRequest *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) { + if (answer.allowed() && request->x_forwarded_for_iterator.size() != 0) { /* * Remove the last comma-delimited element from the @@ -497,8 +491,7 @@ clientFollowXForwardedForCheck(allow_t answer, void *data) 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) { @@ -513,7 +506,7 @@ clientFollowXForwardedForCheck(allow_t answer, void *data) request->x_forwarded_for_iterator.clean(); request->flags.done_follow_x_forwarded_for = true; - if (answer != ACCESS_ALLOWED && answer != ACCESS_DENIED) { + if (!answer.someRuleMatched()) { debugs(28, DBG_CRITICAL, "ERROR: Processing X-Forwarded-For. Stopping at IP address: " << request->indirect_client_addr ); } @@ -769,7 +762,7 @@ ClientRequestContext::clientAccessCheckDone(const allow_t &answer) proxy_auth_msg = http->request->auth_user_request->denyMessage(""); #endif - if (answer != ACCESS_ALLOWED) { + if (!answer.allowed()) { // auth has a grace period where credentials can be expired but okay not to challenge. /* Send an auth challenge or error */ @@ -880,7 +873,7 @@ clientRedirectAccessCheckDone(allow_t answer, void *data) ClientHttpRequest *http = context->http; context->acl_checklist = NULL; - if (answer == ACCESS_ALLOWED) + if (answer.allowed()) redirectStart(http, clientRedirectDoneWrapper, context); else { Helper::Reply const nilReply(Helper::Error); @@ -911,7 +904,7 @@ clientStoreIdAccessCheckDone(allow_t answer, void *data) ClientHttpRequest *http = context->http; context->acl_checklist = NULL; - if (answer == ACCESS_ALLOWED) + if (answer.allowed()) storeIdStart(http, clientStoreIdDoneWrapper, context); else { debugs(85, 3, "access denied expected ERR reply handling: " << answer); @@ -1397,7 +1390,7 @@ void ClientRequestContext::checkNoCacheDone(const allow_t &answer) { acl_checklist = NULL; - if (answer == ACCESS_DENIED) { + if (answer.denied()) { http->request->flags.noCache = true; // dont read reply from cache http->request->flags.cachable = false; // dont store reply into cache } @@ -1496,7 +1489,7 @@ ClientRequestContext::sslBumpAccessCheckDone(const allow_t &answer) if (!httpStateIsValid()) return; - const Ssl::BumpMode bumpMode = answer == ACCESS_ALLOWED ? + const Ssl::BumpMode bumpMode = answer.allowed() ? static_cast(answer.kind) : Ssl::bumpSplice; http->sslBumpNeed(bumpMode); // for processRequest() to bump if needed http->al->ssl.bumpMode = bumpMode; // for logging diff --git a/src/clients/Client.cc b/src/clients/Client.cc index 24a3b9bf33..6448b35223 100644 --- a/src/clients/Client.cc +++ b/src/clients/Client.cc @@ -522,7 +522,7 @@ Client::blockCaching() ACLFilledChecklist ch(acl, originalRequest().getRaw()); ch.reply = const_cast(entry->getReply()); // ACLFilledChecklist API bug HTTPMSGLOCK(ch.reply); - if (ch.fastCheck() != ACCESS_ALLOWED) { // when in doubt, block + if (!ch.fastCheck().allowed()) { // when in doubt, block debugs(20, 3, "store_miss prohibits caching"); return true; } diff --git a/src/clients/FtpClient.cc b/src/clients/FtpClient.cc index aa8e1ca542..2455dde99d 100644 --- a/src/clients/FtpClient.cc +++ b/src/clients/FtpClient.cc @@ -705,7 +705,7 @@ Ftp::Client::sendPassive() bool doEpsv = true; if (Config.accessList.ftp_epsv) { ACLFilledChecklist checklist(Config.accessList.ftp_epsv, fwd->request, NULL); - doEpsv = (checklist.fastCheck() == ACCESS_ALLOWED); + doEpsv = checklist.fastCheck().allowed(); } if (!doEpsv) { debugs(9, 5, "EPSV support manually disabled. Sending PASV for FTP Channel (" << ctrl.conn->remote <<")"); diff --git a/src/external_acl.cc b/src/external_acl.cc index f560af5c74..a7c791b98e 100644 --- a/src/external_acl.cc +++ b/src/external_acl.cc @@ -456,7 +456,7 @@ external_acl::maybeCacheable(const allow_t &result) const if (result == ACCESS_DUNNO) return false; // non-cacheable response - if ((result == ACCESS_ALLOWED ? ttl : negative_ttl) <= 0) + if ((result.allowed() ? ttl : negative_ttl) <= 0) return false; // not caching this type of response return true; @@ -615,7 +615,7 @@ aclMatchExternal(external_acl_data *acl, ACLFilledChecklist *ch) /* Make sure the user is authenticated */ debugs(82, 3, HERE << acl->def->name << " check user authenticated."); const allow_t ti = AuthenticateAcl(ch); - if (ti != ACCESS_ALLOWED) { + if (!ti.allowed()) { debugs(82, 2, HERE << acl->def->name << " user not authenticated (" << ti << ")"); return ti; } @@ -802,7 +802,7 @@ external_acl_entry_expired(external_acl * def, const ExternalACLEntryPointer &en if (def->cache_size <= 0 || entry->result == ACCESS_DUNNO) return 1; - if (entry->date + (entry->result == ACCESS_ALLOWED ? def->ttl : def->negative_ttl) < squid_curtime) + if (entry->date + (entry->result.allowed() ? def->ttl : def->negative_ttl) < squid_curtime) return 1; else return 0; @@ -815,7 +815,7 @@ external_acl_grace_expired(external_acl * def, const ExternalACLEntryPointer &en return 1; int ttl; - ttl = entry->result == ACCESS_ALLOWED ? def->ttl : def->negative_ttl; + ttl = entry->result.allowed() ? def->ttl : def->negative_ttl; ttl = (ttl * (100 - def->grace)) / 100; if (entry->date + ttl <= squid_curtime) diff --git a/src/htcp.cc b/src/htcp.cc index 7ebc44fcaf..d199c0b7ae 100644 --- a/src/htcp.cc +++ b/src/htcp.cc @@ -775,7 +775,7 @@ htcpAccessAllowed(acl_access * acl, const htcpSpecifier::Pointer &s, Ip::Address ACLFilledChecklist checklist(acl, s->request.getRaw(), nullptr); checklist.src_addr = from; checklist.my_addr.setNoAddr(); - return (checklist.fastCheck() == ACCESS_ALLOWED); + return checklist.fastCheck().allowed(); } static void diff --git a/src/http.cc b/src/http.cc index 1b0605fbc6..e8b5fbeda2 100644 --- a/src/http.cc +++ b/src/http.cc @@ -807,7 +807,7 @@ HttpStateData::handle1xx(HttpReply *reply) ACLFilledChecklist ch(Config.accessList.reply, originalRequest().getRaw()); ch.reply = reply; HTTPMSGLOCK(ch.reply); - if (ch.fastCheck() != ACCESS_ALLOWED) { // TODO: support slow lookups? + if (!ch.fastCheck().allowed()) { // TODO: support slow lookups? debugs(11, 3, HERE << "ignoring denied 1xx"); proceedAfter1xx(); return; @@ -2318,7 +2318,7 @@ HttpStateData::finishingBrokenPost() } ACLFilledChecklist ch(Config.accessList.brokenPosts, originalRequest().getRaw()); - if (ch.fastCheck() != ACCESS_ALLOWED) { + if (!ch.fastCheck().allowed()) { debugs(11, 5, HERE << "didn't match brokenPosts"); return false; } diff --git a/src/http/Stream.cc b/src/http/Stream.cc index 3c13c868fa..b4c8dc5564 100644 --- a/src/http/Stream.cc +++ b/src/http/Stream.cc @@ -296,7 +296,7 @@ Http::Stream::sendStartOfMessage(HttpReply *rep, StoreIOBuffer bodyData) chl->reply = rep; HTTPMSGLOCK(chl->reply); const allow_t answer = chl->fastCheck(); - if (answer == ACCESS_ALLOWED) { + if (answer.allowed()) { writeQuotaHandler = pool->createBucket(); fd_table[clientConnection->fd].writeQuotaHandler = writeQuotaHandler; break; diff --git a/src/icp_v2.cc b/src/icp_v2.cc index d9ed9f0df2..4cb551eb58 100644 --- a/src/icp_v2.cc +++ b/src/icp_v2.cc @@ -417,7 +417,7 @@ icpAccessAllowed(Ip::Address &from, HttpRequest * icp_request) ACLFilledChecklist checklist(Config.accessList.icp, icp_request, NULL); checklist.src_addr = from; checklist.my_addr.setNoAddr(); - return (checklist.fastCheck() == ACCESS_ALLOWED); + return checklist.fastCheck().allowed(); } char const * diff --git a/src/log/access_log.cc b/src/log/access_log.cc index ee16f6a65b..0bb1b063e7 100644 --- a/src/log/access_log.cc +++ b/src/log/access_log.cc @@ -84,7 +84,7 @@ accessLogLogTo(CustomLog* log, AccessLogEntry::Pointer &al, ACLChecklist * check xstrncpy(al->hier.host, dash_str, SQUIDHOSTNAMELEN); for (; log; log = log->next) { - if (log->aclList && checklist && checklist->fastCheck(log->aclList) != ACCESS_ALLOWED) + if (log->aclList && checklist && !checklist->fastCheck(log->aclList).allowed()) continue; // The special-case "none" type has no logfile object set diff --git a/src/neighbors.cc b/src/neighbors.cc index 2087b688c1..65ae2ef590 100644 --- a/src/neighbors.cc +++ b/src/neighbors.cc @@ -168,7 +168,7 @@ peerAllowedToUse(const CachePeer * p, HttpRequest * request) ACLFilledChecklist checklist(p->access, request, NULL); - return (checklist.fastCheck() == ACCESS_ALLOWED); + return checklist.fastCheck().allowed(); } /* Return TRUE if it is okay to send an ICP request to this CachePeer. */ diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc index 4b3b6ceaca..0baa345a68 100644 --- a/src/security/PeerConnector.cc +++ b/src/security/PeerConnector.cc @@ -338,7 +338,7 @@ Security::PeerConnector::sslCrtvdCheckForErrors(Ssl::CertValidationResponse cons bool allowed = false; if (check) { check->sslErrors = new Security::CertErrors(Security::CertError(i->error_no, i->cert, i->error_depth)); - if (check->fastCheck() == ACCESS_ALLOWED) + if (check->fastCheck().allowed()) allowed = true; } // else the Config.ssl_client.cert_error access list is not defined diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc index ca9f71d615..5968695c1c 100644 --- a/src/servers/FtpServer.cc +++ b/src/servers/FtpServer.cc @@ -1547,7 +1547,7 @@ Ftp::Server::handleUploadRequest(String &, String &) ClientHttpRequest *http = pipeline.front()->http; HttpRequest *request = http->request; ACLFilledChecklist bodyContinuationCheck(Config.accessList.forceRequestBodyContinuation, request, NULL); - if (bodyContinuationCheck.fastCheck() == ACCESS_ALLOWED) { + if (bodyContinuationCheck.fastCheck().allowed()) { request->forcedBodyContinuation = true; if (checkDataConnPost()) { // Write control Msg diff --git a/src/servers/Http1Server.cc b/src/servers/Http1Server.cc index 2cbfbe0f49..9f808b0086 100644 --- a/src/servers/Http1Server.cc +++ b/src/servers/Http1Server.cc @@ -257,7 +257,7 @@ Http::One::Server::processParsedRequest(Http::StreamPointer &context) if (Config.accessList.forceRequestBodyContinuation) { ACLFilledChecklist bodyContinuationCheck(Config.accessList.forceRequestBodyContinuation, request.getRaw(), NULL); - if (bodyContinuationCheck.fastCheck() == ACCESS_ALLOWED) { + if (bodyContinuationCheck.fastCheck().allowed()) { debugs(33, 5, "Body Continuation forced"); request->forcedBodyContinuation = true; //sendControlMsg diff --git a/src/snmp_core.cc b/src/snmp_core.cc index e931b55f83..b6721a4893 100644 --- a/src/snmp_core.cc +++ b/src/snmp_core.cc @@ -383,7 +383,6 @@ snmpDecodePacket(SnmpRequest * rq) u_char *Community; u_char *buf = rq->buf; int len = rq->len; - allow_t allow = ACCESS_DENIED; if (!Config.accessList.snmp) { debugs(49, DBG_IMPORTANT, "WARNING: snmp_access not configured. agent query DENIED from : " << rq->from); @@ -402,9 +401,8 @@ snmpDecodePacket(SnmpRequest * rq) ACLFilledChecklist checklist(Config.accessList.snmp, NULL, NULL); checklist.src_addr = rq->from; checklist.snmp_community = (char *) Community; - allow = checklist.fastCheck(); - if (allow == ACCESS_ALLOWED && (snmp_coexist_V2toV1(PDU))) { + if (checklist.fastCheck().allowed() && (snmp_coexist_V2toV1(PDU))) { rq->community = Community; rq->PDU = PDU; debugs(49, 5, "snmpAgentParse: reqid=[" << PDU->reqid << "]"); diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc index 1fbffd56a9..917b4bfc1c 100644 --- a/src/ssl/PeekingPeerConnector.cc +++ b/src/ssl/PeekingPeerConnector.cc @@ -36,7 +36,7 @@ Ssl::PeekingPeerConnector::cbCheckForPeekAndSpliceDone(allow_t answer, void *dat void Ssl::PeekingPeerConnector::checkForPeekAndSpliceDone(allow_t answer) { - const Ssl::BumpMode finalAction = (answer.code == ACCESS_ALLOWED) ? + const Ssl::BumpMode finalAction = answer.allowed() ? static_cast(answer.kind): checkForPeekAndSpliceGuess(); checkForPeekAndSpliceMatched(finalAction); diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 3bd7bcb3f3..f90812715b 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -329,7 +329,7 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx) assert(!filledCheck->sslErrors); filledCheck->sslErrors = new Security::CertErrors(Security::CertError(error_no, broken_cert)); filledCheck->serverCert = peer_cert; - if (check->fastCheck() == ACCESS_ALLOWED) { + if (check->fastCheck().allowed()) { debugs(83, 3, "bypassing SSL error " << error_no << " in " << buffer); ok = 1; } else { diff --git a/src/tunnel.cc b/src/tunnel.cc index 42001ea784..bb899a49dd 100644 --- a/src/tunnel.cc +++ b/src/tunnel.cc @@ -1097,7 +1097,7 @@ tunnelStart(ClientHttpRequest * http) ACLFilledChecklist ch(Config.accessList.miss, request, NULL); ch.src_addr = request->client_addr; ch.my_addr = request->my_addr; - if (ch.fastCheck() == ACCESS_DENIED) { + if (ch.fastCheck().denied()) { debugs(26, 4, HERE << "MISS access forbidden."); err = new ErrorState(ERR_FORWARDING_DENIED, Http::scForbidden, request); http->al->http.code = Http::scForbidden;