Bug 1628: follow_x_forwarded_for shoudl not cause allow/deny behavior

clientFollowXForwardedForCheck() needs to always set the
request->indirect_client_addr properly at completion and call
calloutContext->clientAccessCheck(); unconditionally to begin actual
access ACL tests.

Calling clientAccessCheckDone(answer) is equivalent to processing an
http_access line with denial.

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 022e40943e4439f4da8e81abcb44e63f97eb440f..5b6ca63393ee4370f375cce2f8f19de5ff60c9ab 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -482,10 +482,19 @@ clientFollowXForwardedForCheck(int answer, void *data)
     request->x_forwarded_for_iterator.clean();
     request->flags.done_follow_x_forwarded_for = 1;
 
-    if (answer == ACCESS_DENIED)
-        calloutContext->clientAccessCheckDone(answer);
-    else
-        calloutContext->clientAccessCheck();
+    /* If follow XFF is denied, we reset the indirect_client_addr
+       to the direct client. Thats the one we are configured to check for */
+    if (answer == ACCESS_DENIED) {
+        request->indirect_client_addr = request->client_addr;
+    }
+    /* on a failure, leave it as undefined state ?? */
+    else if (answer != ACCESS_ALLOWED) {
+        debugs(28, DBG_CRITICAL, "Follow X-Forwarded-For encountered an error. Ignoring address: " << request->indirect_client_addr );
+        request->indirect_client_addr = request->client_addr;
+    }
+
+    /* process actual access ACL as normal. */
+    calloutContext->clientAccessCheck();
 }
 #endif /* FOLLOW_X_FORWARDED_FOR */
 
@@ -504,9 +513,8 @@ ClientRequestContext::clientAccessCheck()
         return;
     }
 #endif /* FOLLOW_X_FORWARDED_FOR */
-    
-    acl_checklist =
-        clientAclChecklistCreate(Config.accessList.http, http);
+
+    acl_checklist = clientAclChecklistCreate(Config.accessList.http, http);
     acl_checklist->nonBlockingCheck(clientAccessCheckDoneWrapper, this);
 }