=== modified file 'src/ExternalACL.h' --- src/ExternalACL.h 2009-03-08 19:34:36 +0000 +++ src/ExternalACL.h 2012-06-28 09:22:30 +0000 @@ -36,7 +36,8 @@ #include "acl/Checklist.h" -class external_acl; +/** \todo CLEANUP: kill this typedef. */ +typedef struct _external_acl_data external_acl_data; class ExternalACLLookup : public ACLChecklist::AsyncState { @@ -45,14 +46,15 @@ static ExternalACLLookup *Instance(); virtual void checkForAsync(ACLChecklist *)const; + // If possible, starts an asynchronous lookup of an external ACL. + // Otherwise, asserts (or bails if background refresh is requested). + static void Start(ACLChecklist *checklist, external_acl_data *acl, bool bg); + private: static ExternalACLLookup instance_; static void LookupDone(void *data, void *result); }; -/** \todo CLEANUP: kill this typedef. */ -typedef struct _external_acl_data external_acl_data; - #include "acl/Acl.h" class ACLExternal : public ACL @@ -61,7 +63,7 @@ public: MEMPROXY_CLASS(ACLExternal); - static void ExternalAclLookup(ACLChecklist * ch, ACLExternal *, EAH * callback, void *callback_data); + static void ExternalAclLookup(ACLChecklist * ch, ACLExternal *); ACLExternal(char const *); === modified file 'src/ExternalACLEntry.cc' --- src/ExternalACLEntry.cc 2012-02-05 06:09:46 +0000 +++ src/ExternalACLEntry.cc 2012-06-28 08:54:59 +0000 @@ -69,7 +69,7 @@ ExternalACLEntry::ExternalACLEntry() { lru.next = lru.prev = NULL; - result = 0; + result = ACCESS_DENIED; date = 0; def = NULL; } === modified file 'src/ExternalACLEntry.h' --- src/ExternalACLEntry.h 2011-02-11 12:53:06 +0000 +++ src/ExternalACLEntry.h 2012-06-28 08:54:59 +0000 @@ -44,7 +44,7 @@ #ifndef SQUID_EXTERNALACLENTRY_H #define SQUID_EXTERNALACLENTRY_H - +#include "acl/Acl.h" #include "cbdata.h" /****************************************************************** @@ -58,9 +58,9 @@ { public: - ExternalACLEntryData() : result (-1) {} + ExternalACLEntryData() : result(ACCESS_DUNNO) {} - int result; + allow_t result; #if USE_AUTH // TODO use an AuthUser to hold this info String user; @@ -89,7 +89,7 @@ void update(ExternalACLEntryData const &); dlink_node lru; - int result; + allow_t result; time_t date; #if USE_AUTH String user; === modified file 'src/acl/Acl.cc' --- src/acl/Acl.cc 2012-06-04 10:23:57 +0000 +++ src/acl/Acl.cc 2012-06-28 09:22:40 +0000 @@ -317,16 +317,22 @@ ACLList::matches (ACLChecklist *checklist) const { assert (_acl); + // XXX: AclMatchedName does not contain a matched ACL name when the acl + // does not match (or contains stale name if no ACLs are checked). In + // either case, we get misleading debugging and possibly incorrect error + // messages. Unfortunately, deny_info's "when none http_access + // lines match" exception essentially requires this mess. + // TODO: Rework by using an acl-free deny_info for the no-match cases? AclMatchedName = _acl->name; debugs(28, 3, "ACLList::matches: checking " << (op ? null_string : "!") << _acl->name); if (_acl->checklistMatches(checklist) != op) { debugs(28, 4, "ACLList::matches: result is false"); - return checklist->lastACLResult(false); + return false; } debugs(28, 4, "ACLList::matches: result is true"); - return checklist->lastACLResult(true); + return true; } === modified file 'src/acl/Acl.h' --- src/acl/Acl.h 2011-07-21 11:09:47 +0000 +++ src/acl/Acl.h 2012-06-28 09:10:04 +0000 @@ -39,6 +39,10 @@ #include "cbdata.h" #include "dlink.h" +#if HAVE_OSTREAM +#include +#endif + class ConfigParser; class ACLChecklist; @@ -105,12 +109,35 @@ /// \ingroup ACLAPI typedef enum { + // Authorization ACL result states ACCESS_DENIED, ACCESS_ALLOWED, ACCESS_DUNNO, - ACCESS_REQ_PROXY_AUTH + + // Authentication ACL result states + ACCESS_AUTH_REQUIRED, // Missing Credentials } allow_t; +inline std::ostream & +operator <<(std::ostream &o, const allow_t a) +{ + switch (a) { + case ACCESS_DENIED: + o << "DENIED"; + break; + case ACCESS_ALLOWED: + o << "ALLOWED"; + break; + case ACCESS_DUNNO: + o << "DUNNO"; + break; + case ACCESS_AUTH_REQUIRED: + o << "AUTH_REQUIRED"; + break; + } + return o; +} + /// \ingroup ACLAPI class acl_access { === modified file 'src/acl/Checklist.cc' --- src/acl/Checklist.cc 2012-02-05 06:09:46 +0000 +++ src/acl/Checklist.cc 2012-06-28 09:22:40 +0000 @@ -36,29 +36,14 @@ #include "squid-old.h" #include "acl/Checklist.h" -allow_t const & -ACLChecklist::currentAnswer() const -{ - return allow_; -} - -void -ACLChecklist::currentAnswer(allow_t const newAnswer) -{ - allow_ = newAnswer; -} - void ACLChecklist::matchNonBlocking() { if (checking()) return; - /** Deny if no rules present. */ - currentAnswer(ACCESS_DENIED); - if (callerGone()) { - checkCallback(currentAnswer()); + checkCallback(ACCESS_DUNNO); // the answer does not really matter return; } @@ -67,25 +52,25 @@ * We cannot select a sensible default for all callers here. */ if (accessList == NULL) { debugs(28, DBG_CRITICAL, "SECURITY ERROR: ACL " << this << " checked with nothing to match against!!"); - currentAnswer(ACCESS_DENIED); - checkCallback(currentAnswer()); + checkCallback(ACCESS_DUNNO); return; } + allow_t lastSeenKeyword = ACCESS_DUNNO; /* NOTE: This holds a cbdata reference to the current access_list * entry, not the whole list. */ while (accessList != NULL) { /** \par * If the _acl_access is no longer valid (i.e. its been - * freed because of a reconfigure), then bail on this - * access check. For now, return ACCESS_DENIED. + * freed because of a reconfigure), then bail with ACCESS_DUNNO. */ if (!cbdataReferenceValid(accessList)) { cbdataReferenceDone(accessList); debugs(28, 4, "ACLChecklist::check: " << this << " accessList is invalid"); - continue; + checkCallback(ACCESS_DUNNO); + return; } checking (true); @@ -107,6 +92,8 @@ return; } + lastSeenKeyword = accessList->allow; + /* * Reference the next access entry */ @@ -119,11 +106,14 @@ cbdataReferenceDone(A); } - /** If dropped off the end of the list return inversion of last line allow/deny action. */ - debugs(28, 3, HERE << this << " NO match found, returning " << - (currentAnswer() != ACCESS_DENIED ? ACCESS_DENIED : ACCESS_ALLOWED)); + calcImplicitAnswer(lastSeenKeyword); + checkCallback(currentAnswer()); +} - checkCallback(currentAnswer() != ACCESS_DENIED ? ACCESS_DENIED : ACCESS_ALLOWED); +bool +ACLChecklist::asyncNeeded() const +{ + return state_ != NullState::Instance(); } bool @@ -148,28 +138,32 @@ } void -ACLChecklist::markFinished() +ACLChecklist::markFinished(const allow_t &finalAnswer, const char *reason) { assert (!finished() && !asyncInProgress()); finished_ = true; - debugs(28, 3, "ACLChecklist::markFinished: " << this << - " checklist processing finished"); + allow_ = finalAnswer; + debugs(28, 3, HERE << this << " answer " << allow_ << " for " << reason); } +/// Called first (and once) by all checks to initialize their state void -ACLChecklist::preCheck() +ACLChecklist::preCheck(const char *what) { - debugs(28, 3, "ACLChecklist::preCheck: " << this << " checking '" << accessList->cfgline << "'"); - /* what is our result on a match? */ - currentAnswer(accessList->allow); + debugs(28, 3, HERE << this << " checking " << what); + finished_ = false; } void ACLChecklist::checkAccessList() { - preCheck(); + debugs(28, 3, HERE << this << " checking '" << accessList->cfgline << "'"); /* does the current AND clause match */ - matchAclList(accessList->aclList, false); + if (matchAclList(accessList->aclList, false)) + markFinished(accessList->allow, "first matching rule won"); + + // If we are not finished() here, the caller must distinguish between + // slow async calls and pure rule mismatches using asyncInProgress(). } void @@ -196,62 +190,128 @@ delete this; } -void +/// An ACLChecklist::matchNodes() wrapper to simplify profiling. +bool ACLChecklist::matchAclList(const ACLList * head, bool const fast) { + // TODO: remove by using object con/destruction-based PROF_* macros. PROF_start(aclMatchAclList); - const ACLList *node = head; - - finished_ = false; - - while (node) { - bool nodeMatched = node->matches(this); - - if (fast) - changeState(NullState::Instance()); - - if (finished()) { - PROF_stop(aclMatchAclList); - return; - } - - if (!nodeMatched || state_ != NullState::Instance()) { - - bool async = state_ != NullState::Instance(); - - checkForAsync(); - - bool async_in_progress = asyncInProgress(); - debugs(28, 3, "aclmatchAclList: async=" << (async ? 1 : 0) << - " nodeMatched=" << (nodeMatched ? 1 : 0) << - " async_in_progress=" << (async_in_progress ? 1 : 0) << - " lastACLResult() = " << (lastACLResult() ? 1 : 0) << - " finished() = " << finished()); + const bool result = matchNodes(head, fast); + PROF_stop(aclMatchAclList); + return result; +} + +/** Returns true if and only if there was a match. If false is returned: + finished() indicates an error or exception of some kind, while + !finished() means there was a mismatch or an allowed slow async call. + If async calls are allowed (i.e. 'fast' was false), then those last + two cases can be distinguished using asyncInProgress(). +*/ +bool +ACLChecklist::matchNodes(const ACLList * head, bool const fast) +{ + assert(!finished()); + + for (const ACLList *node = head; node; node = node->next) { + + const NodeMatchingResult resultBeforeAsync = matchNode(*node, fast); + + if (resultBeforeAsync == nmrMatch) + continue; + + if (resultBeforeAsync == nmrMismatch || resultBeforeAsync == nmrFinished) + return false; + + assert(resultBeforeAsync == nmrNeedsAsync); + + // Ideally, this should be inside match() itself, but that requires + // prohibiting slow ACLs in options that do not support them. + // TODO: rename to maybeStartAsync()? + checkForAsync(); + + // Some match() code claims that an async lookup is needed, but then + // fails to start an async lookup when given a chance. We catch such + // cases here and call matchNode() again, hoping that some cached data + // prevents us from going async again. + // This is inefficient and ugly, but fixing all match() code, including + // the code it calls, such as ipcache_nbgethostbyname(), takes time. + if (!asyncInProgress()) { // failed to start an async operation if (finished()) { - debugs(28, 3, "aclmatchAclList: " << this << " returning (AND list entry failed to match)"); - PROF_stop(aclMatchAclList); - return; + debugs(28, 3, HERE << this << " finished after failing to go async: " << currentAnswer()); + return false; // an exceptional case } - if (async && nodeMatched && !asyncInProgress() && lastACLResult()) { - // async acl, but using cached response, and it was a match - node = node->next; + const NodeMatchingResult resultAfterAsync = matchNode(*node, true); + // the second call disables slow checks so we cannot go async again + assert(resultAfterAsync != nmrNeedsAsync); + if (resultAfterAsync == nmrMatch) continue; - } - debugs(28, 3, "aclmatchAclList: " << this << " returning (AND list entry awaiting an async lookup)"); - PROF_stop(aclMatchAclList); - return; + assert(resultAfterAsync == nmrMismatch || resultAfterAsync == nmrFinished); + return false; } - node = node->next; - } - - debugs(28, 3, "aclmatchAclList: " << this << " returning true (AND list satisfied)"); - - markFinished(); - PROF_stop(aclMatchAclList); + assert(!finished()); // async operation is truly asynchronous + debugs(28, 3, HERE << this << " awaiting async operation"); + return false; + } + + debugs(28, 3, HERE << this << " success: all ACLs matched"); + return true; +} + + +/// Check whether a single ACL matches, returning NodeMatchingResult +ACLChecklist::NodeMatchingResult +ACLChecklist::matchNode(const ACLList &node, bool const fast) +{ + const bool nodeMatched = node.matches(this); + const bool needsAsync = asyncNeeded(); + const bool matchFinished = finished(); + + debugs(28, 3, HERE << this << + " matched=" << nodeMatched << + " async=" << needsAsync << + " finished=" << matchFinished); + + /* There are eight possible outcomes of the matches() call based on + (matched, async, finished) permutations. We support these four: + matched,!async,!finished: a match (must check next rule node) + !matched,!async,!finished: a mismatch (whole rule fails to match) + !matched,!async,finished: error or special condition (propagate) + !matched,async,!finished: ACL needs to make an async call (pause) + */ + + if (nodeMatched) { + // matches() should return false in all special cases + assert(!needsAsync && !matchFinished); + return nmrMatch; + } + + if (matchFinished) { + // we cannot be done and need an async call at the same time + assert(!needsAsync); + debugs(28, 3, HERE << this << " exception: " << currentAnswer()); + return nmrFinished; + } + + if (!needsAsync) { + debugs(28, 3, HERE << this << " simple mismatch"); + return nmrMismatch; + } + + /* we need an async call */ + + if (fast) { + changeState(NullState::Instance()); // disable async checks + markFinished(ACCESS_DUNNO, "async required but prohibited"); + debugs(28, 3, HERE << this << " DUNNO because cannot async"); + return nmrFinished; + } + + debugs(28, 3, HERE << this << " going async"); + return nmrNeedsAsync; } ACLChecklist::ACLChecklist() : @@ -261,8 +321,7 @@ async_(false), finished_(false), allow_(ACCESS_DENIED), - state_(NullState::Instance()), - lastACLResult_(false) + state_(NullState::Instance()) { } @@ -320,6 +379,7 @@ void ACLChecklist::nonBlockingCheck(ACLCB * callback_, void *callback_data_) { + preCheck("slow rules"); callback = callback_; callback_data = cbdataReference(callback_data_); matchNonBlocking(); @@ -329,11 +389,14 @@ ACLChecklist::fastCheck(const ACLList * list) { PROF_start(aclCheckFast); - currentAnswer(ACCESS_DUNNO); - matchAclList(list, true); - // assume ALLOWED on matches due to not having an acl_access object - if (finished()) - currentAnswer(ACCESS_ALLOWED); + + preCheck("fast ACLs"); + + // assume DENY/ALLOW on mis/matches due to not having acl_access object + if (matchAclList(list, true)) + markFinished(ACCESS_ALLOWED, "all ACLs matched"); + else if (!finished()) + markFinished(ACCESS_DENIED, "ACL mismatched"); PROF_stop(aclCheckFast); return currentAnswer(); } @@ -345,33 +408,55 @@ ACLChecklist::fastCheck() { PROF_start(aclCheckFast); - currentAnswer(ACCESS_DUNNO); - + + preCheck("fast rules"); + + allow_t lastSeenKeyword = ACCESS_DUNNO; debugs(28, 5, "aclCheckFast: list: " << accessList); const acl_access *acl = cbdataReference(accessList); while (acl != NULL && cbdataReferenceValid(acl)) { - matchAclList(acl->aclList, true); + // on a match, finish + if (matchAclList(acl->aclList, true)) + markFinished(acl->allow, "first matching rule won"); + + // if finished (on a match or in exceptional cases), stop if (finished()) { - currentAnswer(acl->allow); + cbdataReferenceDone(acl); PROF_stop(aclCheckFast); - cbdataReferenceDone(acl); return currentAnswer(); } - /* - * Reference the next access entry - */ + // on a mismatch, try the next access rule + lastSeenKeyword = acl->allow; const acl_access *A = acl; acl = cbdataReference(acl->next); cbdataReferenceDone(A); } - debugs(28, 5, "aclCheckFast: no matches, returning: " << currentAnswer()); + // There were no rules to match or no rules matched + calcImplicitAnswer(lastSeenKeyword); PROF_stop(aclCheckFast); return currentAnswer(); } +/// When no rules matched, the answer is the inversion of the last seen rule +/// action (or ACCESS_DUNNO if the reversal is not possible). The caller +/// should set lastSeenAction to ACCESS_DUNNO if there were no rules to see. +void +ACLChecklist::calcImplicitAnswer(const allow_t &lastSeenAction) +{ + allow_t implicitRuleAnswer = ACCESS_DUNNO; + if (lastSeenAction == ACCESS_DENIED) // reverse last seen "deny" + implicitRuleAnswer = ACCESS_ALLOWED; + else if (lastSeenAction == ACCESS_ALLOWED) // reverse last seen "allow" + implicitRuleAnswer = ACCESS_DENIED; + // else we saw no rules and will respond with ACCESS_DUNNO + + debugs(28, 3, HERE << this << " NO match found, last action " << + lastSeenAction << " so returning " << implicitRuleAnswer); + markFinished(implicitRuleAnswer, "implicit rule won"); +} bool ACLChecklist::checking() const === modified file 'src/acl/Checklist.h' --- src/acl/Checklist.h 2011-07-21 11:09:47 +0000 +++ src/acl/Checklist.h 2012-06-28 09:22:40 +0000 @@ -92,47 +92,85 @@ virtual ~ACLChecklist(); /** - * Trigger off a non-blocking access check for a set of *_access options.. - * The callback specified will be called with true/false - * when the results of the ACL tests are known. + * Start a non-blocking (async) check for a list of allow/deny rules. + * Each rule comes with a list of ACLs. + * + * The callback specified will be called with the result of the check. + * + * The first rule where all ACLs match wins. If there is such a rule, + * the result becomes that rule keyword (ACCESS_ALLOWED or ACCESS_DENIED). + * + * If there are rules but all ACL lists mismatch, an implicit rule is used. + * Its result is the negation of the keyword of the last seen rule. + * + * Some ACLs may stop the check prematurely by setting an exceptional + * check result (e.g., ACCESS_AUTH_REQUIRED) instead of declaring a + * match or mismatch. + * + * If there are no rules to check at all, the result becomes ACCESS_DUNNO. + * Calling this method with no rules to check wastes a lot of CPU cycles + * and will result in a DBG_CRITICAL debugging message. */ void nonBlockingCheck(ACLCB * callback, void *callback_data); /** - * Trigger a blocking access check for a set of *_access options. - * - * ACLs which cannot be satisfied directly from available data are ignored. - * This means any proxy_auth, external_acl, DNS lookups, Ident lookups etc - * which have not already been performed and cached will not be checked. - * - * If there is no access list to check the default is to return ALLOWED. - * However callers should perform their own check and default based on local - * knowledge of the ACL usage rather than depend on this default. - * That will also save on work setting up ACLChecklist fields for a no-op. - * - * \retval ACCESS_DUNNO Unable to determine any result - * \retval ACCESS_ALLOWED Access Allowed - * \retval ACCESS_DENIED Access Denied + * Perform a blocking (immediate) check for a list of allow/deny rules. + * Each rule comes with a list of ACLs. + * + * The first rule where all ACLs match wins. If there is such a rule, + * the result becomes that rule keyword (ACCESS_ALLOWED or ACCESS_DENIED). + * + * If there are rules but all ACL lists mismatch, an implicit rule is used + * Its result is the negation of the keyword of the last seen rule. + * + * Some ACLs may stop the check prematurely by setting an exceptional + * check result (e.g., ACCESS_AUTH_REQUIRED) instead of declaring a + * match or mismatch. + * + * Some ACLs may require an async lookup which is prohibited by this + * method. In this case, the exceptional check result of ACCESS_DUNNO is + * immediately returned. + * + * If there are no rules to check at all, the result becomes ACCESS_DUNNO. */ allow_t const & fastCheck(); /** - * A version of fastCheck() for use when there is a one-line set of ACLs - * to be tested and a match determins the result action to be done. - * - * \retval ACCESS_DUNNO Unable to determine any result - * \retval ACCESS_ALLOWED ACLs all matched + * Perform a blocking (immediate) check whether a list of ACLs matches. + * This method is meant to be used with squid.conf ACL-driven options that + * lack allow/deny keywords and are tested one ACL list at a time. Whether + * the checks for other occurrences of the same option continue after this + * call is up to the caller and option semantics. + * + * If all ACLs match, the result becomes ACCESS_ALLOWED. + * + * If all ACLs mismatch, the result becomes ACCESS_DENIED. + * + * Some ACLs may stop the check prematurely by setting an exceptional + * check result (e.g., ACCESS_AUTH_REQUIRED) instead of declaring a + * match or mismatch. + * + * Some ACLs may require an async lookup which is prohibited by this + * method. In this case, the exceptional check result of ACCESS_DUNNO is + * immediately returned. + * + * If there are no ACLs to check at all, the result becomes ACCESS_ALLOWED. */ allow_t const & fastCheck(const ACLList * list); + // whether the last checked ACL of the current rule needs + // an async operation to determine whether there was a match + bool asyncNeeded() const; bool asyncInProgress() const; void asyncInProgress(bool const); + /// whether markFinished() was called bool finished() const; - void markFinished(); + /// called when no more ACLs should be checked; sets the final answer and + /// prints a debugging message explaining the reason for that answer + void markFinished(const allow_t &newAnswer, const char *reason); - allow_t const & currentAnswer() const; - void currentAnswer(allow_t const); + const allow_t ¤tAnswer() const { return allow_; } void changeState(AsyncState *); AsyncState *asyncState() const; @@ -156,20 +194,25 @@ void *callback_data; /** - * Attempt to check the current checklist against current data. - * This is the core routine behind all ACL test routines. - * As much as possible of current tests are performed immediately - * and the result is maybe delayed to wait for async lookups. - * - * When all tests are done callback is presented with one of: - * - ACCESS_ALLOWED Access explicitly Allowed - * - ACCESS_DENIED Access explicitly Denied + * Performs non-blocking check starting with the current rule. + * Used by nonBlockingCheck() to initiate the checks and by + * async operation callbacks to resume checks after the async + * operation updates the current Squid state. See nonBlockingCheck() + * for details on final result determination. */ void matchNonBlocking(); private: /* internal methods */ - void preCheck(); - void matchAclList(const ACLList * list, bool const fast); + /// possible outcomes when trying to match a single ACL node in a list + typedef enum { nmrMatch, nmrMismatch, nmrFinished, nmrNeedsAsync } + NodeMatchingResult; + + /// prepare for checking ACLs; called once per check + void preCheck(const char *what); + bool matchAclList(const ACLList * list, bool const fast); + bool matchNodes(const ACLList * head, bool const fast); + NodeMatchingResult matchNode(const ACLList &node, bool const fast); + void calcImplicitAnswer(const allow_t &lastSeenAction); bool async_; bool finished_; @@ -180,13 +223,7 @@ bool checking() const; void checking (bool const); - bool lastACLResult_; bool callerGone(); - -public: - bool lastACLResult(bool x) { return lastACLResult_ = x; } - - bool lastACLResult() const { return lastACLResult_; } }; #endif /* SQUID_ACLCHECKLIST_H */ === modified file 'src/adaptation/AccessCheck.cc' --- src/adaptation/AccessCheck.cc 2012-02-05 06:09:46 +0000 +++ src/adaptation/AccessCheck.cc 2012-06-28 08:54:23 +0000 @@ -141,7 +141,7 @@ debugs(93, 8, HERE << "callback answer=" << answer); AccessCheck *ac = (AccessCheck*)data; - /** \todo AYJ 2008-06-12: If answer == ACCESS_REQ_PROXY_AUTH + /** \todo AYJ 2008-06-12: If answer == ACCESS_AUTH_REQUIRED * we should be kicking off an authentication before continuing * with this request. see bug 2400 for details. */ === modified file 'src/auth/Acl.cc' --- src/auth/Acl.cc 2012-02-05 06:09:46 +0000 +++ src/auth/Acl.cc 2012-06-28 09:22:30 +0000 @@ -6,10 +6,14 @@ #include "auth/AclProxyAuth.h" #include "HttpRequest.h" -/** retval -1 user not authenticated (authentication error?) - retval 0 user not authorized OR user authentication is in pgrogress - retval +1 user authenticated and authorized */ -int +/** + * \retval ACCESS_AUTH_REQUIRED credentials missing. challenge required. + * \retval ACCESS_DENIED user not authenticated (authentication error?) + * \retval ACCESS_DUNNO user authentication is in progress + * \retval ACCESS_DENIED user not authorized + * \retval ACCESS_ALLOWED user authenticated and authorized + */ +allow_t AuthenticateAcl(ACLChecklist *ch) { ACLFilledChecklist *checklist = Filled(ch); @@ -18,20 +22,20 @@ if (NULL == request) { fatal ("requiresRequest SHOULD have been true for this ACL!!"); - return 0; + return ACCESS_DENIED; } else if (request->flags.sslBumped) { debugs(28, 5, "SslBumped request: It is an encapsulated request do not authenticate"); checklist->auth_user_request = checklist->conn() != NULL ? checklist->conn()->auth_user_request : request->auth_user_request; if (checklist->auth_user_request != NULL) - return 1; + return ACCESS_ALLOWED; else - return 0; + return ACCESS_DENIED; } else if (request->flags.accelerated) { /* WWW authorization on accelerated requests */ headertype = HDR_AUTHORIZATION; } else if (request->flags.intercepted || request->flags.spoof_client_ip) { - debugs(28, DBG_IMPORTANT, HERE << " authentication not applicable on intercepted requests."); - return -1; + debugs(28, DBG_IMPORTANT, "NOTICE: Authentication not applicable on intercepted requests."); + return ACCESS_DENIED; } else { /* Proxy authorization on proxy requests */ headertype = HDR_PROXY_AUTHORIZATION; @@ -45,25 +49,28 @@ switch (result) { case AUTH_ACL_CANNOT_AUTHENTICATE: - debugs(28, 4, HERE << "returning 0 user authenticated but not authorised."); - return 0; + debugs(28, 4, HERE << "returning " << ACCESS_DENIED << " user authenticated but not authorised."); + return ACCESS_DENIED; case AUTH_AUTHENTICATED: - return 1; + return ACCESS_ALLOWED; break; case AUTH_ACL_HELPER: - debugs(28, 4, HERE << "returning 0 sending credentials to helper."); + debugs(28, 4, HERE << "returning " << ACCESS_DUNNO << " sending credentials to helper."); checklist->changeState(ProxyAuthLookup::Instance()); - return 0; + return ACCESS_DUNNO; // XXX: break this down into DUNNO, EXPIRED_OK, EXPIRED_BAD states case AUTH_ACL_CHALLENGE: - debugs(28, 4, HERE << "returning 0 sending authentication challenge."); - checklist->changeState (ProxyAuthNeeded::Instance()); - return 0; + debugs(28, 4, HERE << "returning " << ACCESS_AUTH_REQUIRED << " sending authentication challenge."); + /* Client is required to resend the request with correct authentication + * credentials. (This may be part of a stateful auth protocol.) + * The request is denied. + */ + return ACCESS_AUTH_REQUIRED; default: fatal("unexpected authenticateAuthenticate reply\n"); - return 0; + return ACCESS_DENIED; } } === modified file 'src/auth/Acl.h' --- src/auth/Acl.h 2011-02-11 12:53:06 +0000 +++ src/auth/Acl.h 2012-06-28 08:56:58 +0000 @@ -3,13 +3,15 @@ #if USE_AUTH +#include "acl/Acl.h" + // ACL-related code used by authentication-related code. This code is not in // auth/Gadgets to avoid making auth/libauth dependent on acl/libstate because // acl/libstate already depends on auth/libauth. class ACLChecklist; /// \ingroup AuthAPI -extern int AuthenticateAcl(ACLChecklist *ch); +extern allow_t AuthenticateAcl(ACLChecklist *ch); #endif /* USE_AUTH */ #endif /* SQUID_AUTH_ACL_H */ === modified file 'src/auth/AclMaxUserIp.cc' --- src/auth/AclMaxUserIp.cc 2012-02-05 06:09:46 +0000 +++ src/auth/AclMaxUserIp.cc 2012-06-28 09:22:30 +0000 @@ -150,16 +150,29 @@ ACLMaxUserIP::match(ACLChecklist *cl) { ACLFilledChecklist *checklist = Filled(cl); + allow_t answer = AuthenticateAcl(checklist); int ti; - if ((ti = AuthenticateAcl(checklist)) != 1) + // convert to tri-state ACL match 1,0,-1 + switch (answer) { + case ACCESS_ALLOWED: + // check for a match + ti = match(checklist->auth_user_request, checklist->src_addr); + checklist->auth_user_request = NULL; return ti; - ti = match(checklist->auth_user_request, checklist->src_addr); - - checklist->auth_user_request = NULL; - - return ti; + case ACCESS_DENIED: + return 0; // non-match + + case ACCESS_DUNNO: + case ACCESS_AUTH_REQUIRED: + default: + // If the answer is not allowed or denied (matches/not matches) and + // async authentication is not needed (asyncNeeded), then we are done. + if (!checklist->asyncNeeded()) + checklist->markFinished(answer, "AuthenticateAcl exception"); + return -1; // other + } } wordlist * === modified file 'src/auth/AclProxyAuth.cc' --- src/auth/AclProxyAuth.cc 2012-06-28 05:12:14 +0000 +++ src/auth/AclProxyAuth.cc 2012-06-28 09:22:30 +0000 @@ -79,14 +79,26 @@ int ACLProxyAuth::match(ACLChecklist *checklist) { - int ti; - - if ((ti = AuthenticateAcl(checklist)) != 1) - return ti; - - ti = matchProxyAuth(checklist); - - return ti; + allow_t answer = AuthenticateAcl(checklist); + + // convert to tri-state ACL match 1,0,-1 + switch (answer) { + case ACCESS_ALLOWED: + // check for a match + return matchProxyAuth(checklist); + + case ACCESS_DENIED: + return 0; // non-match + + case ACCESS_DUNNO: + case ACCESS_AUTH_REQUIRED: + default: + // If the answer is not allowed or denied (matches/not matches) and + // async authentication is not needed (asyncNeeded), then we are done. + if (!checklist->asyncNeeded()) + checklist->markFinished(answer, "AuthenticateAcl exception"); + return -1; // other + } } wordlist * @@ -117,14 +129,6 @@ return true; } -ProxyAuthNeeded ProxyAuthNeeded::instance_; - -ProxyAuthNeeded * -ProxyAuthNeeded::Instance() -{ - return &instance_; -} - ProxyAuthLookup ProxyAuthLookup::instance_; ProxyAuthLookup * @@ -170,19 +174,6 @@ checklist->matchNonBlocking(); } -void -ProxyAuthNeeded::checkForAsync(ACLChecklist *checklist) const -{ - /* Client is required to resend the request with correct authentication - * credentials. (This may be part of a stateful auth protocol.) - * The request is denied. - */ - debugs(28, 6, "ACLChecklist::checkForAsync: requiring Proxy Auth header."); - checklist->currentAnswer(ACCESS_REQ_PROXY_AUTH); - checklist->changeState (ACLChecklist::NullState::Instance()); - checklist->markFinished(); -} - ACL * ACLProxyAuth::clone() const { === modified file 'src/auth/AclProxyAuth.h' --- src/auth/AclProxyAuth.h 2012-06-28 05:12:14 +0000 +++ src/auth/AclProxyAuth.h 2012-06-28 09:22:30 +0000 @@ -53,17 +53,6 @@ static void LookupDone(void *data); }; -class ProxyAuthNeeded : public ACLChecklist::AsyncState -{ - -public: - static ProxyAuthNeeded *Instance(); - virtual void checkForAsync(ACLChecklist *)const; - -private: - static ProxyAuthNeeded instance_; -}; - class ACLProxyAuth : public ACL { === modified file 'src/client_side_request.cc' --- src/client_side_request.cc 2012-06-04 10:23:57 +0000 +++ src/client_side_request.cc 2012-06-28 09:10:04 +0000 @@ -761,12 +761,15 @@ #endif if (answer != ACCESS_ALLOWED) { - /* Send an error */ - int require_auth = (answer == ACCESS_REQ_PROXY_AUTH || aclIsProxyAuth(AclMatchedName)); + // auth has a grace period where credentials can be expired but okay not to challenge. + + /* Send an auth challenge or error */ + // XXX: do we still need aclIsProxyAuth() ? + bool auth_challenge = (answer == ACCESS_AUTH_REQUIRED || aclIsProxyAuth(AclMatchedName)); debugs(85, 5, "Access Denied: " << http->uri); debugs(85, 5, "AclMatchedName = " << (AclMatchedName ? AclMatchedName : "")); #if USE_AUTH - if (require_auth) + if (auth_challenge) debugs(33, 5, "Proxy Auth Message = " << (proxy_auth_msg ? proxy_auth_msg : "")); #endif @@ -776,11 +779,11 @@ * the clientCreateStoreEntry() call just below. Pedro Ribeiro * */ - page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, answer != ACCESS_REQ_PROXY_AUTH); + page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, answer != ACCESS_AUTH_REQUIRED); http->logType = LOG_TCP_DENIED; - if (require_auth) { + if (auth_challenge) { #if USE_AUTH if (http->request->flags.sslBumped) { /*SSL Bumped request, authentication is not possible*/ === modified file 'src/external_acl.cc' --- src/external_acl.cc 2012-02-05 06:09:46 +0000 +++ src/external_acl.cc 2012-06-28 09:22:40 +0000 @@ -769,14 +769,12 @@ } } -static int +static allow_t aclMatchExternal(external_acl_data *acl, ACLFilledChecklist *ch) { - int result; - external_acl_entry *entry; const char *key = ""; debugs(82, 9, HERE << "acl=\"" << acl->def->name << "\""); - entry = ch->extacl_entry; + external_acl_entry *entry = ch->extacl_entry; if (entry) { if (cbdataReferenceValid(entry) && entry->def == acl->def) { @@ -807,63 +805,101 @@ debugs(82, 9, HERE << "No helper entry available"); #if USE_AUTH if (acl->def->require_auth) { - int ti; /* Make sure the user is authenticated */ - debugs(82, 3, "aclMatchExternal: " << acl->def->name << " check user authenticated."); - if ((ti = AuthenticateAcl(ch)) != 1) { - debugs(82, 2, "aclMatchExternal: " << acl->def->name << " user not authenticated (" << ti << ")"); + debugs(82, 3, HERE << acl->def->name << " check user authenticated."); + const allow_t ti = AuthenticateAcl(ch); + if (ti != ACCESS_ALLOWED) { + debugs(82, 2, HERE << acl->def->name << " user not authenticated (" << ti << ")"); return ti; } - debugs(82, 3, "aclMatchExternal: " << acl->def->name << " user is authenticated."); + debugs(82, 3, HERE << acl->def->name << " user is authenticated."); } #endif key = makeExternalAclKey(ch, acl); if (!key) { /* Not sufficient data to process */ - return -1; + return ACCESS_DUNNO; } entry = static_cast(hash_lookup(acl->def->cache, key)); - if (!entry || external_acl_grace_expired(acl->def, entry)) { - debugs(82, 2, "aclMatchExternal: " << acl->def->name << "(\"" << key << "\") = lookup needed"); - debugs(82, 2, "aclMatchExternal: \"" << key << "\": entry=@" << + external_acl_entry *staleEntry = entry; + if (entry && external_acl_entry_expired(acl->def, entry)) + entry = NULL; + + if (entry && external_acl_grace_expired(acl->def, entry)) { + // refresh in the background + ExternalACLLookup::Start(ch, acl, true); + debugs(82, 4, HERE << "no need to wait for the refresh of '" << + key << "' in '" << acl->def->name << "' (ch=" << ch << ")."); + } + + if (!entry) { + debugs(82, 2, HERE << acl->def->name << "(\"" << key << "\") = lookup needed"); + debugs(82, 2, HERE << "\"" << key << "\": entry=@" << entry << ", age=" << (entry ? (long int) squid_curtime - entry->date : 0)); if (acl->def->theHelper->stats.queue_size <= (int)acl->def->theHelper->childs.n_active) { - debugs(82, 2, "aclMatchExternal: \"" << key << "\": queueing a call."); + debugs(82, 2, HERE << "\"" << key << "\": queueing a call."); ch->changeState(ExternalACLLookup::Instance()); - debugs(82, 2, "aclMatchExternal: \"" << key << "\": return -1."); - return -1; // to get here we have to have an expired cache entry. MUST not use. + debugs(82, 2, HERE << "\"" << key << "\": return -1."); + return ACCESS_DUNNO; // expired cached or simply absent entry } else { - if (!entry) { - debugs(82, 1, "aclMatchExternal: '" << acl->def->name << + if (!staleEntry) { + debugs(82, DBG_IMPORTANT, "WARNING: external ACL '" << acl->def->name << "' queue overload. Request rejected '" << key << "'."); external_acl_message = "SYSTEM TOO BUSY, TRY AGAIN LATER"; - return -1; + return ACCESS_DUNNO; } else { - debugs(82, 1, "aclMatchExternal: '" << acl->def->name << + debugs(82, DBG_IMPORTANT, "WARNING: external ACL '" << acl->def->name << "' queue overload. Using stale result. '" << key << "'."); + entry = staleEntry; /* Fall thru to processing below */ } } } } + debugs(82, 4, HERE << "entry = { date=" << + (long unsigned int) entry->date << + ", result=" << entry->result << + " tag=" << entry->tag << + " log=" << entry->log << " }"); +#if USE_AUTH + debugs(82, 4, HERE << "entry user=" << entry->user); +#endif + external_acl_cache_touch(acl->def, entry); - result = entry->result; external_acl_message = entry->message.termedBuf(); - debugs(82, 2, "aclMatchExternal: " << acl->def->name << " = " << result); + debugs(82, 2, HERE << acl->def->name << " = " << entry->result); copyResultsFromEntry(ch->request, entry); - return result; + return entry->result; } int ACLExternal::match(ACLChecklist *checklist) { - return aclMatchExternal (data, Filled(checklist)); + allow_t answer = aclMatchExternal(data, Filled(checklist)); + + // convert to tri-state ACL match 1,0,-1 + switch (answer) { + case ACCESS_ALLOWED: + return 1; // match + + case ACCESS_DENIED: + return 0; // non-match + + case ACCESS_DUNNO: + case ACCESS_AUTH_REQUIRED: + default: + // If the answer is not allowed or denied (matches/not matches) and + // async authentication is not needed (asyncNeeded), then we are done. + if (!checklist->asyncNeeded()) + checklist->markFinished(answer, "aclMatchExternal exception"); + return -1; // other + } } wordlist * @@ -1282,7 +1318,7 @@ char *value; char *t = NULL; ExternalACLEntryData entryData; - entryData.result = 0; + entryData.result = ACCESS_DENIED; external_acl_entry *entry = NULL; debugs(82, 2, "externalAclHandleReply: reply=\"" << reply << "\""); @@ -1291,7 +1327,7 @@ status = strwordtok(reply, &t); if (status && strcmp(status, "OK") == 0) - entryData.result = 1; + entryData.result = ACCESS_ALLOWED; while ((token = strwordtok(NULL, &t))) { value = strchr(token, '='); @@ -1353,53 +1389,28 @@ } void -ACLExternal::ExternalAclLookup(ACLChecklist *checklist, ACLExternal * me, EAH * callback, void *callback_data) -{ - MemBuf buf; - external_acl_data *acl = me->data; +ACLExternal::ExternalAclLookup(ACLChecklist *checklist, ACLExternal * me) +{ + ExternalACLLookup::Start(checklist, me->data, false); +} + +void +ExternalACLLookup::Start(ACLChecklist *checklist, external_acl_data *acl, bool inBackground) +{ external_acl *def = acl->def; - externalAclState *state; - dlink_node *node; - externalAclState *oldstate = NULL; - bool graceful = 0; ACLFilledChecklist *ch = Filled(checklist); -#if USE_AUTH - if (acl->def->require_auth) { - int ti; - /* Make sure the user is authenticated */ - debugs(82, 3, "aclMatchExternal: " << acl->def->name << " check user authenticated."); - - if ((ti = AuthenticateAcl(ch)) != 1) { - debugs(82, 1, "externalAclLookup: " << acl->def->name << - " user authentication failure (" << ti << ", ch=" << ch << ")"); - callback(callback_data, NULL); - return; - } - debugs(82, 3, "aclMatchExternal: " << acl->def->name << " user is authenticated."); - } -#endif - const char *key = makeExternalAclKey(ch, acl); - - if (!key) { - debugs(82, 1, "externalAclLookup: lookup in '" << def->name << - "', prerequisit failure (ch=" << ch << ")"); - callback(callback_data, NULL); - return; - } - - debugs(82, 2, "externalAclLookup: lookup in '" << def->name << "' for '" << key << "'"); - - external_acl_entry *entry = static_cast(hash_lookup(def->cache, key)); - - if (entry && external_acl_entry_expired(def, entry)) - entry = NULL; + assert(key); + + debugs(82, 2, HERE << (inBackground ? "bg" : "fg") << " lookup in '" << + def->name << "' for '" << key << "'"); /* Check for a pending lookup to hook into */ // only possible if we are caching results. + externalAclState *oldstate = NULL; if (def->cache_size > 0) { - for (node = def->queue.head; node; node = node->next) { + for (dlink_node *node = def->queue.head; node; node = node->next) { externalAclState *oldstatetmp = static_cast(node->data); if (strcmp(key, oldstatetmp->key) == 0) { @@ -1409,35 +1420,21 @@ } } - if (entry && external_acl_grace_expired(def, entry)) { - if (oldstate) { - debugs(82, 4, "externalAclLookup: in grace period, but already pending lookup ('" << key << "', ch=" << ch << ")"); - callback(callback_data, entry); - return; - } else { - graceful = 1; // grace expired, (neg)ttl did not, and we must start a new lookup. - } - } - - // The entry is in the cache, grace_ttl did not expired. - if (!graceful && entry && !external_acl_grace_expired(def, entry)) { - /* Should not really happen, but why not.. */ - callback(callback_data, entry); - debugs(82, 4, "externalAclLookup: no lookup pending for '" << key << "', and grace not expired"); - debugs(82, 4, "externalAclLookup: (what tha' hell?)"); + // A background refresh has no need to piggiback on a pending request: + // When the pending request completes, the cache will be refreshed anyway. + if (oldstate && inBackground) { + debugs(82, 7, HERE << "'" << def->name << "' queue is already being refreshed (ch=" << ch << ")"); return; } - /* No pending lookup found. Sumbit to helper */ - state = cbdataAlloc(externalAclState); - + externalAclState *state = cbdataAlloc(externalAclState); state->def = cbdataReference(def); state->key = xstrdup(key); - if (!graceful) { - state->callback = callback; - state->callback_data = cbdataReference(callback_data); + if (!inBackground) { + state->callback = &ExternalACLLookup::LookupDone; + state->callback_data = cbdataReference(checklist); } if (oldstate) { @@ -1445,16 +1442,19 @@ state->queue = oldstate->queue; oldstate->queue = state; } else { + /* No pending lookup found. Sumbit to helper */ + /* Check for queue overload */ if (def->theHelper->stats.queue_size >= (int)def->theHelper->childs.n_running) { - debugs(82, 1, "externalAclLookup: '" << def->name << "' queue overload (ch=" << ch << ")"); + debugs(82, 7, HERE << "'" << def->name << "' queue is too long"); + assert(inBackground); // or the caller should have checked cbdataFree(state); - callback(callback_data, entry); return; } /* Send it off to the helper */ + MemBuf buf; buf.init(); buf.Printf("%s\n", key); @@ -1468,28 +1468,6 @@ buf.clean(); } - if (graceful) { - /* No need to wait during grace period */ - debugs(82, 4, "externalAclLookup: no need to wait for the result of '" << - key << "' in '" << def->name << "' (ch=" << ch << ")."); - debugs(82, 4, "externalAclLookup: using cached entry " << entry); - - if (entry != NULL) { - debugs(82, 4, "externalAclLookup: entry = { date=" << - (long unsigned int) entry->date << - ", result=" << entry->result << - " tag=" << entry->tag << - " log=" << entry->log << " }"); -#if USE_AUTH - debugs(82, 4, "externalAclLookup: user=" << entry->user); -#endif - copyResultsFromEntry(ch->request, entry); - } - - callback(callback_data, entry); - return; - } - debugs(82, 4, "externalAclLookup: will wait for the result of '" << key << "' in '" << def->name << "' (ch=" << ch << ")."); } @@ -1574,9 +1552,10 @@ ACLExternal *me = dynamic_cast (acl); assert (me); checklist->asyncInProgress(true); - ACLExternal::ExternalAclLookup(checklist, me, LookupDone, checklist); + ACLExternal::ExternalAclLookup(checklist, me); } +/// Called when an async lookup returns void ExternalACLLookup::LookupDone(void *data, void *result) { === modified file 'src/http.cc' --- src/http.cc 2012-05-21 02:35:41 +0000 +++ src/http.cc 2012-06-28 09:22:30 +0000 @@ -745,7 +745,7 @@ if (Config.accessList.reply) { ACLFilledChecklist ch(Config.accessList.reply, originalRequest(), NULL); ch.reply = HTTPMSGLOCK(reply); - if (!ch.fastCheck()) { // TODO: support slow lookups? + if (ch.fastCheck() != ACCESS_ALLOWED) { // TODO: support slow lookups? debugs(11, 3, HERE << "ignoring denied 1xx"); proceedAfter1xx(); return; @@ -2191,7 +2191,7 @@ } ACLFilledChecklist ch(Config.accessList.brokenPosts, originalRequest(), NULL); - if (!ch.fastCheck()) { + if (ch.fastCheck() != ACCESS_ALLOWED) { debugs(11, 5, HERE << "didn't match brokenPosts"); return false; } === modified file 'src/ident/AclIdent.cc' --- src/ident/AclIdent.cc 2012-02-05 06:09:46 +0000 +++ src/ident/AclIdent.cc 2012-06-28 09:22:40 +0000 @@ -89,10 +89,14 @@ return data->match(checklist->rfc931); } else if (checklist->conn() != NULL && checklist->conn()->clientConnection != NULL && checklist->conn()->clientConnection->rfc931[0]) { return data->match(checklist->conn()->clientConnection->rfc931); - } else { + } else if (checklist->conn() != NULL && Comm::IsConnOpen(checklist->conn()->clientConnection)) { debugs(28, 3, HERE << "switching to ident lookup state"); checklist->changeState(IdentLookup::Instance()); return 0; + } else { + debugs(28, DBG_IMPORTANT, HERE << "Can't start ident lookup. No client connection" ); + checklist->markFinished(ACCESS_DUNNO, "cannot start ident lookup"); + return -1; } } @@ -127,15 +131,12 @@ IdentLookup::checkForAsync(ACLChecklist *cl)const { ACLFilledChecklist *checklist = Filled(cl); - if (checklist->conn() != NULL && Comm::IsConnOpen(checklist->conn()->clientConnection)) { - debugs(28, 3, HERE << "Doing ident lookup" ); - checklist->asyncInProgress(true); - Ident::Start(checklist->conn()->clientConnection, LookupDone, checklist); - } else { - debugs(28, DBG_IMPORTANT, "IdentLookup::checkForAsync: Can't start ident lookup. No client connection" ); - checklist->currentAnswer(ACCESS_DENIED); - checklist->markFinished(); - } + const ConnStateData *conn = checklist->conn(); + // check that ACLIdent::match() tested this lookup precondition + assert(conn && Comm::IsConnOpen(conn->clientConnection)); + debugs(28, 3, HERE << "Doing ident lookup" ); + checklist->asyncInProgress(true); + Ident::Start(checklist->conn()->clientConnection, LookupDone, checklist); } void === modified file 'src/peer_select.cc' --- src/peer_select.cc 2012-02-05 06:37:08 +0000 +++ src/peer_select.cc 2012-06-28 09:15:39 +0000 @@ -187,12 +187,7 @@ case ACCESS_DENIED: // not relevant. case ACCESS_DUNNO: // not relevant. break; - case ACCESS_REQ_PROXY_AUTH: -#if WHEN_AUTH_CASES_PORT case ACCESS_AUTH_REQUIRED: - case ACCESS_AUTH_EXPIRED_OK: - case ACCESS_AUTH_EXPIRED_BAD: -#endif debugs(44, DBG_IMPORTANT, "WARNING: never_direct resulted in " << answer << ". Username ACLs are not reliable here."); break; } @@ -215,12 +210,7 @@ case ACCESS_DENIED: // not relevant. case ACCESS_DUNNO: // not relevant. break; - case ACCESS_REQ_PROXY_AUTH: -#if WHEN_AUTH_CASES_PORT case ACCESS_AUTH_REQUIRED: - case ACCESS_AUTH_EXPIRED_OK: - case ACCESS_AUTH_EXPIRED_BAD: -#endif debugs(44, DBG_IMPORTANT, "WARNING: always_direct resulted in " << answer << ". Username ACLs are not reliable here."); break; }