------------------------------------------------------------ revno: 11955 revision-id: squid3@treenet.co.nz-20111231045406-5dvgwsg1535k32ud parent: squidadm@squid-cache.org-20111231012610-d57caa2dms1v8pnj fixes bug(s): http://bugs.squid-cache.org/show_bug.cgi?id=3444 committer: Amos Jeffries branch nick: trunk timestamp: Fri 2011-12-30 21:54:06 -0700 message: Bug 3444: send authentication challenge from ssl_bump and url_rewrite_access ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20111231045406-5dvgwsg1535k32ud # target_branch: http://bzr.squid-cache.org/bzr/squid3/trunk/ # testament_sha1: b062d08d5440ddfd706546527a414e56a92bcdba # timestamp: 2011-12-31 04:55:49 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/trunk/ # base_revision_id: squidadm@squid-cache.org-20111231012610-\ # d57caa2dms1v8pnj # # Begin patch === modified file 'src/ClientRequestContext.h' --- src/ClientRequestContext.h 2011-10-21 16:20:42 +0000 +++ src/ClientRequestContext.h 2011-12-31 04:54:06 +0000 @@ -47,7 +47,7 @@ */ bool sslBumpAccessCheck(); /// The callback function for ssl-bump access check list - void sslBumpAccessCheckDone(bool doSslBump); + void sslBumpAccessCheckDone(const allow_t &answer); #endif ClientHttpRequest *http; @@ -69,7 +69,11 @@ bool sslBumpCheckDone; #endif -private: + /// 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); + CBDATA_CLASS(ClientRequestContext); }; === modified file 'src/client_side_request.cc' --- src/client_side_request.cc 2011-12-15 11:56:37 +0000 +++ src/client_side_request.cc 2011-12-31 04:54:06 +0000 @@ -722,95 +722,117 @@ calloutContext->clientAccessCheckDone(answer); } -void -ClientRequestContext::clientAccessCheckDone(const allow_t &answer) +bool +ClientRequestContext::maybeSendAuthChallenge(const allow_t &answer) { acl_checklist = NULL; err_type page_id; http_status status; + +#if USE_AUTH + char const *proxy_auth_msg = ""; + if (http->getConn() != NULL && http->getConn()->auth_user_request != NULL) + proxy_auth_msg = http->getConn()->auth_user_request->denyMessage(""); + else if (http->request->auth_user_request != NULL) + 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 USE_AUTH + 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) { +#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; + } +#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, +#if USE_AUTH + http->getConn() != NULL && http->getConn()->auth_user_request != NULL ? + http->getConn()->auth_user_request : http->request->auth_user_request); +#else + 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 USE_AUTH - char const *proxy_auth_msg = ""; - if (http->getConn() != NULL && http->getConn()->auth_user_request != NULL) - proxy_auth_msg = http->getConn()->auth_user_request->denyMessage(""); - else if (http->request->auth_user_request != NULL) - proxy_auth_msg = http->request->auth_user_request->denyMessage(""); -#endif - - 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 : "")); -#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, 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*/ - 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; -#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, -#if USE_AUTH - http->getConn() != NULL && http->getConn()->auth_user_request != NULL ? - http->getConn()->auth_user_request : http->request->auth_user_request); -#else - NULL); -#endif - http->getConn()->flags.readMore = true; // resume any pipeline reads. - node = (clientStreamNode *)http->client_stream.tail->data; - clientStreamRead(node, http, node->readBuffer); + if (maybeSendAuthChallenge(answer)) return; - } /* ACCESS_ALLOWED (or auth in grace period ACCESS_AUTH_EXPIRED_OK) continues here ... */ safe_free(http->uri); @@ -856,11 +878,12 @@ clientRedirectAccessCheckDone(allow_t answer, void *data) { ClientRequestContext *context = (ClientRequestContext *)data; - ClientHttpRequest *http = context->http; - context->acl_checklist = NULL; + + if (context->maybeSendAuthChallenge(answer)) + return; if (answer == ACCESS_ALLOWED) - redirectStart(http, clientRedirectDoneWrapper, context); + redirectStart(context->http, clientRedirectDoneWrapper, context); else context->clientRedirectDone(NULL); } @@ -1280,16 +1303,19 @@ sslBumpAccessCheckDoneWrapper(allow_t answer, void *data) { ClientRequestContext *calloutContext = static_cast(data); - - if (!calloutContext->httpStateIsValid()) - return; - calloutContext->sslBumpAccessCheckDone(answer == ACCESS_ALLOWED); + calloutContext->sslBumpAccessCheckDone(answer); } void -ClientRequestContext::sslBumpAccessCheckDone(bool doSslBump) +ClientRequestContext::sslBumpAccessCheckDone(const allow_t &answer) { - http->sslBumpNeeded(doSslBump); + if (!httpStateIsValid()) + return; + + if (maybeSendAuthChallenge(answer)) + return; + + http->sslBumpNeeded(answer == ACCESS_ALLOWED); http->doCallouts(); } #endif