------------------------------------------------------------ revno: 12127 revision-id: squid3@treenet.co.nz-20120508011351-c0gt21oq3ofnynky parent: squid3@treenet.co.nz-20120506012922-unklmt5kx72uq6me committer: Amos Jeffries branch nick: trunk timestamp: Mon 2012-05-07 19:13:51 -0600 message: Revert revno11955 fix for bug 3444 ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20120508011351-c0gt21oq3ofnynky # target_branch: http://bzr.squid-cache.org/bzr/squid3/trunk/ # testament_sha1: 2a2b939a4a245c492fe8e335da247b3caff612f7 # timestamp: 2012-05-08 01:56:14 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/trunk/ # base_revision_id: squid3@treenet.co.nz-20120506012922-\ # unklmt5kx72uq6me # # Begin patch === modified file 'src/ClientRequestContext.h' --- src/ClientRequestContext.h 2011-12-31 04:54:06 +0000 +++ src/ClientRequestContext.h 2012-05-08 01:13:51 +0000 @@ -47,7 +47,7 @@ */ 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 @@ 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); }; === modified file 'src/client_side_request.cc' --- src/client_side_request.cc 2012-04-25 22:32:00 +0000 +++ src/client_side_request.cc 2012-05-08 01:13:51 +0000 @@ -740,12 +740,17 @@ 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 @@ 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; - } - - // 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 : "")); + if (answer != ACCESS_ALLOWED && answer != ACCESS_AUTH_EXPIRED_OK) { + // 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 || 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); - - http->logType = LOG_TCP_DENIED; - - if (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; + + if (auth_challenge) { #if USE_AUTH - if (http->request->flags.sslBumped) { - /*SSL Bumped request, authentication is not possible*/ + 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; - } else if (!http->flags.accel) { - /* Proxy authorisation needed */ - status = HTTP_PROXY_AUTHENTICATION_REQUIRED; +#endif + if (page_id == ERR_NONE) + page_id = ERR_CACHE_ACCESS_DENIED; } else { - /* WWW authorisation needed */ - status = HTTP_UNAUTHORIZED; + status = HTTP_FORBIDDEN; + + if (page_id == ERR_NONE) + page_id = ERR_ACCESS_DENIED; } -#else - // 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_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 @@ 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 @@ 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