From: Amos Jeffries Date: Tue, 8 May 2012 01:13:51 +0000 (-0600) Subject: Revert revno11955 fix for bug 3444 X-Git-Tag: BumpSslServerFirst.take08~7^2~5 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9d5e7196f8afb1b60387b8439f8f040d0e63b4f2;p=thirdparty%2Fsquid.git Revert revno11955 fix for bug 3444 --- diff --git a/src/ClientRequestContext.h b/src/ClientRequestContext.h index 6290eaf9b2..4ea052eba0 100644 --- a/src/ClientRequestContext.h +++ b/src/ClientRequestContext.h @@ -47,7 +47,7 @@ public: */ bool sslBumpAccessCheck(); /// The callback function for ssl-bump access check list - void sslBumpAccessCheckDone(const allow_t &answer); + void sslBumpAccessCheckDone(bool doSslBump); #endif ClientHttpRequest *http; @@ -69,11 +69,7 @@ public: bool sslBumpCheckDone; #endif - /// Send authentication response (challenge or error) if ACL result indicates one is needed - /// \return true if an error page of any kind has been sent back to the client. - // NP: public only until ACLChecklist::nonBlockingCheck() takes Async::Pointer to a call - bool maybeSendAuthChallenge(const allow_t &answer); - +private: CBDATA_CLASS(ClientRequestContext); }; diff --git a/src/client_side_request.cc b/src/client_side_request.cc index 5baaa82cbb..422a412498 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -740,12 +740,17 @@ clientAccessCheckDoneWrapper(allow_t answer, void *data) calloutContext->clientAccessCheckDone(answer); } -bool -ClientRequestContext::maybeSendAuthChallenge(const allow_t &answer) +void +ClientRequestContext::clientAccessCheckDone(const allow_t &answer) { acl_checklist = NULL; err_type page_id; http_status status; + debugs(85, 2, "The request " << + RequestMethodStr(http->request->method) << " " << + http->uri << " is " << answer << + ", because it matched '" << + (AclMatchedName ? AclMatchedName : "NO ACL's") << "'" ); #if USE_AUTH char const *proxy_auth_msg = ""; @@ -755,102 +760,75 @@ ClientRequestContext::maybeSendAuthChallenge(const allow_t &answer) proxy_auth_msg = http->request->auth_user_request->denyMessage(""); #endif - bool auth_challenge = false; - switch (answer) { - case ACCESS_ALLOWED: - case ACCESS_AUTH_EXPIRED_OK: - // No authentication challenge on these ACL results - return auth_challenge; - - case ACCESS_DENIED: - case ACCESS_DUNNO: - // MAYBE challenge on these ACL results - auth_challenge |= aclIsProxyAuth(AclMatchedName); - break; - - case ACCESS_AUTH_REQUIRED: - case ACCESS_AUTH_EXPIRED_BAD: - // Send an auth challenge or error - auth_challenge = true; - } + if (answer != ACCESS_ALLOWED && answer != ACCESS_AUTH_EXPIRED_OK) { + // auth has a grace period where credentials can be expired but okay not to challenge. - // auth has a grace period where credentials can be expired but okay not to challenge. - debugs(85, 5, "Access Denied: " << http->uri); - debugs(85, 5, "AclMatchedName = " << (AclMatchedName ? AclMatchedName : "")); + /* Send an auth challenge or error */ + // XXX: do we still need aclIsProxyAuth() ? + bool auth_challenge = (answer == ACCESS_AUTH_REQUIRED || answer == ACCESS_AUTH_EXPIRED_BAD || aclIsProxyAuth(AclMatchedName)); + debugs(85, 5, "Access Denied: " << http->uri); + debugs(85, 5, "AclMatchedName = " << (AclMatchedName ? AclMatchedName : "")); #if USE_AUTH - if (auth_challenge) - debugs(33, 5, "Proxy Auth Message = " << (proxy_auth_msg ? proxy_auth_msg : "")); + if (auth_challenge) + debugs(33, 5, "Proxy Auth Message = " << (proxy_auth_msg ? proxy_auth_msg : "")); #endif - /* - * NOTE: get page_id here, based on AclMatchedName because if - * USE_DELAY_POOLS is enabled, then AclMatchedName gets clobbered in - * the clientCreateStoreEntry() call just below. Pedro Ribeiro - * - */ - page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, auth_challenge); + /* + * NOTE: get page_id here, based on AclMatchedName because if + * USE_DELAY_POOLS is enabled, then AclMatchedName gets clobbered in + * the clientCreateStoreEntry() call just below. Pedro Ribeiro + * + */ + page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, answer != ACCESS_AUTH_REQUIRED); - http->logType = LOG_TCP_DENIED; + http->logType = LOG_TCP_DENIED; - if (auth_challenge) { + if (auth_challenge) { #if USE_AUTH - if (http->request->flags.sslBumped) { - /*SSL Bumped request, authentication is not possible*/ - status = HTTP_FORBIDDEN; - } else if (!http->flags.accel) { - /* Proxy authorisation needed */ - status = HTTP_PROXY_AUTHENTICATION_REQUIRED; - } else { - /* WWW authorisation needed */ - status = HTTP_UNAUTHORIZED; - } + if (http->request->flags.sslBumped) { + /*SSL Bumped request, authentication is not possible*/ + status = HTTP_FORBIDDEN; + } else if (!http->flags.accel) { + /* Proxy authorisation needed */ + status = HTTP_PROXY_AUTHENTICATION_REQUIRED; + } else { + /* WWW authorisation needed */ + status = HTTP_UNAUTHORIZED; + } #else - // need auth, but not possible to do. - status = HTTP_FORBIDDEN; + // need auth, but not possible to do. + status = HTTP_FORBIDDEN; #endif - if (page_id == ERR_NONE) - page_id = ERR_CACHE_ACCESS_DENIED; - } else { - status = HTTP_FORBIDDEN; + if (page_id == ERR_NONE) + page_id = ERR_CACHE_ACCESS_DENIED; + } else { + status = HTTP_FORBIDDEN; - if (page_id == ERR_NONE) - page_id = ERR_ACCESS_DENIED; - } + if (page_id == ERR_NONE) + page_id = ERR_ACCESS_DENIED; + } - clientStreamNode *node = (clientStreamNode *)http->client_stream.tail->prev->data; - clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); - assert (repContext); - Ip::Address tmpnoaddr; - tmpnoaddr.SetNoAddr(); - repContext->setReplyToError(page_id, status, - http->request->method, - NULL, - http->getConn() != NULL ? http->getConn()->clientConnection->remote : tmpnoaddr, - http->request, - NULL, + clientStreamNode *node = (clientStreamNode *)http->client_stream.tail->prev->data; + clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); + assert (repContext); + Ip::Address tmpnoaddr; + tmpnoaddr.SetNoAddr(); + repContext->setReplyToError(page_id, status, + http->request->method, NULL, + http->getConn() != NULL ? http->getConn()->clientConnection->remote : tmpnoaddr, + http->request, + NULL, #if USE_AUTH - http->getConn() != NULL && http->getConn()->auth_user_request != NULL ? - http->getConn()->auth_user_request : http->request->auth_user_request); + http->getConn() != NULL && http->getConn()->auth_user_request != NULL ? + http->getConn()->auth_user_request : http->request->auth_user_request); #else - NULL); + NULL); #endif - http->getConn()->flags.readMore = true; // resume any pipeline reads. - node = (clientStreamNode *)http->client_stream.tail->data; - clientStreamRead(node, http, node->readBuffer); - return true; -} - -void -ClientRequestContext::clientAccessCheckDone(const allow_t &answer) -{ - debugs(85, 2, "The request " << - RequestMethodStr(http->request->method) << " " << - http->uri << " is " << answer << - ", because it matched '" << - (AclMatchedName ? AclMatchedName : "NO ACL's") << "'" ); - - if (maybeSendAuthChallenge(answer)) + http->getConn()->flags.readMore = true; // resume any pipeline reads. + node = (clientStreamNode *)http->client_stream.tail->data; + clientStreamRead(node, http, node->readBuffer); return; + } /* ACCESS_ALLOWED (or auth in grace period ACCESS_AUTH_EXPIRED_OK) continues here ... */ safe_free(http->uri); @@ -896,12 +874,11 @@ static void clientRedirectAccessCheckDone(allow_t answer, void *data) { ClientRequestContext *context = (ClientRequestContext *)data; - - if (context->maybeSendAuthChallenge(answer)) - return; + ClientHttpRequest *http = context->http; + context->acl_checklist = NULL; if (answer == ACCESS_ALLOWED) - redirectStart(context->http, clientRedirectDoneWrapper, context); + redirectStart(http, clientRedirectDoneWrapper, context); else context->clientRedirectDone(NULL); } @@ -1323,19 +1300,16 @@ static void sslBumpAccessCheckDoneWrapper(allow_t answer, void *data) { ClientRequestContext *calloutContext = static_cast(data); - calloutContext->sslBumpAccessCheckDone(answer); + + if (!calloutContext->httpStateIsValid()) + return; + calloutContext->sslBumpAccessCheckDone(answer == ACCESS_ALLOWED); } void -ClientRequestContext::sslBumpAccessCheckDone(const allow_t &answer) +ClientRequestContext::sslBumpAccessCheckDone(bool doSslBump) { - if (!httpStateIsValid()) - return; - - if (maybeSendAuthChallenge(answer)) - return; - - http->sslBumpNeeded(answer == ACCESS_ALLOWED); + http->sslBumpNeeded(doSslBump); http->doCallouts(); } #endif