Bug 3444: send authentication challenge from ssl_bump and url_rewrite_access

diff --git a/src/ClientRequestContext.h b/src/ClientRequestContext.h
index 4ea052eba050ec288a56e97e2bde875dd448229c..6290eaf9b293a02adfc902362bda0282352e5632 100644 (file)
--- 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(bool doSslBump);
+    void sslBumpAccessCheckDone(const allow_t &answer);
 #endif
 
     ClientHttpRequest *http;
@@ -69,7 +69,11 @@ public:
     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);
 };
 

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 9b4f98b37555feaa006df17a95624c6d3e6c6630..4663c156f3255c1c453ac48fcabfaef6a9adc179 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -722,17 +722,12 @@ clientAccessCheckDoneWrapper(allow_t answer, void *data)
     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;
-    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 = "<null>";
@@ -742,75 +737,102 @@ ClientRequestContext::clientAccessCheckDone(const allow_t &answer)
         proxy_auth_msg = http->request->auth_user_request->denyMessage("<null>");
 #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.
+    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;
+    }
 
-        /* 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 : "<null>"));
+    // 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 : "<null>"));
 #if USE_AUTH
-        if (auth_challenge)
-            debugs(33, 5, "Proxy Auth Message = " << (proxy_auth_msg ? proxy_auth_msg : "<null>"));
+    if (auth_challenge)
+        debugs(33, 5, "Proxy Auth Message = " << (proxy_auth_msg ? proxy_auth_msg : "<null>"));
 #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
-         * <pribeiro@isel.pt>
-         */
-        page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, answer != ACCESS_AUTH_REQUIRED);
+    /*
+     * 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
+     * <pribeiro@isel.pt>
+     */
+    page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, auth_challenge);
 
-        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;
-            }
-#else
-            // need auth, but not possible to do.
+        if (http->request->flags.sslBumped) {
+            /*SSL Bumped request, authentication is not possible*/
             status = HTTP_FORBIDDEN;
-#endif
-            if (page_id == ERR_NONE)
-                page_id = ERR_CACHE_ACCESS_DENIED;
+        } else if (!http->flags.accel) {
+            /* Proxy authorisation needed */
+            status = HTTP_PROXY_AUTHENTICATION_REQUIRED;
         } else {
-            status = HTTP_FORBIDDEN;
-
-            if (page_id == ERR_NONE)
-                page_id = ERR_ACCESS_DENIED;
+            /* 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;
 
-        clientStreamNode *node = (clientStreamNode *)http->client_stream.tail->prev->data;
-        clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(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 (page_id == ERR_NONE)
+            page_id = ERR_ACCESS_DENIED;
+    }
+
+    clientStreamNode *node = (clientStreamNode *)http->client_stream.tail->prev->data;
+    clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(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);
+    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))
         return;
-    }
 
     /* ACCESS_ALLOWED (or auth in grace period ACCESS_AUTH_EXPIRED_OK) continues here ... */
     safe_free(http->uri);
@@ -856,11 +878,12 @@ static void
 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 @@ static void
 sslBumpAccessCheckDoneWrapper(allow_t answer, void *data)
 {
     ClientRequestContext *calloutContext = static_cast<ClientRequestContext *>(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