---------------------
PatchSet 11339 
Date: 2007/03/14 22:43:24
Author: hno
Branch: HEAD
Tag: (none) 
Log:
Clean up X-Forwarded-For acl_uses_indirect_client to make most acls behave

The acl_uses_indirect_client option was only partially implemented, not
working in most acl based directives outside http_access. This patch unifies
how the acl checklist is initialized making most code behave the same.

Adds two new functions:

aclChecklistCacheInit, initializes derived fields in an aclCheck_t. Currently
based on the request only.

aclCheckFastRequest, a simple wrapper around aclCheckFast setting up a
temporary aclCheck_t based on a request_t alone which is the most common use
of aclCheckFast.

Members: 
	src/HttpHeaderTools.c:1.38->1.39 
	src/acl.c:1.319->1.320 
	src/client_side.c:1.715->1.716 
	src/delay_pools.c:1.34->1.35 
	src/forward.c:1.123->1.124 
	src/http.c:1.428->1.429 
	src/neighbors.c:1.315->1.316 
	src/protos.h:1.526->1.527 
	src/ssl.c:1.136->1.137 

Index: squid/src/HttpHeaderTools.c
===================================================================
RCS file: /cvsroot/squid/squid/src/HttpHeaderTools.c,v
retrieving revision 1.38
retrieving revision 1.39
diff -u -r1.38 -r1.39
--- squid/src/HttpHeaderTools.c	11 Mar 2007 22:34:22 -0000	1.38
+++ squid/src/HttpHeaderTools.c	14 Mar 2007 22:43:24 -0000	1.39
@@ -1,6 +1,6 @@
 
 /*
- * $Id: HttpHeaderTools.c,v 1.38 2007/03/11 22:34:22 hno Exp $
+ * $Id: HttpHeaderTools.c,v 1.39 2007/03/14 22:43:24 hno Exp $
  *
  * DEBUG: section 66    HTTP Header Tools
  * AUTHOR: Alex Rousskov
@@ -443,7 +443,6 @@
 
     /* check with anonymizer tables */
     header_mangler *hm;
-    aclCheck_t *checklist;
     assert(e);
     if (e->id == HDR_OTHER) {
 	for (hm = Config.header_access[HDR_OTHER].next; hm; hm = hm->next) {
@@ -456,9 +455,7 @@
 	hm = &Config.header_access[e->id];
     if (!hm->access_list)
 	return 1;
-    checklist = aclChecklistCreate(hm->access_list, request, NULL);
-    if (1 == aclCheckFast(hm->access_list, checklist)) {
-	/* aclCheckFast returns 1 for allow. */
+    if (aclCheckFastRequest(hm->access_list, request)) {
 	retval = 1;
     } else if (NULL == hm->replacement) {
 	/* It was denied, and we don't have any replacement */
@@ -471,7 +468,6 @@
 	stringReset(&e->value, hm->replacement);
 	retval = -1;
     }
-    aclChecklistFree(checklist);
 
     return retval != 0;
 }
Index: squid/src/acl.c
===================================================================
RCS file: /cvsroot/squid/squid/src/acl.c,v
retrieving revision 1.319
retrieving revision 1.320
diff -u -r1.319 -r1.320
--- squid/src/acl.c	21 Jan 2007 12:53:57 -0000	1.319
+++ squid/src/acl.c	14 Mar 2007 22:43:24 -0000	1.320
@@ -1,6 +1,6 @@
 
 /*
- * $Id: acl.c,v 1.319 2007/01/21 12:53:57 adrian Exp $
+ * $Id: acl.c,v 1.320 2007/03/14 22:43:24 hno Exp $
  *
  * DEBUG: section 28    Access Control
  * AUTHOR: Duane Wessels
@@ -2115,6 +2115,7 @@
     allow_t allow = ACCESS_DENIED;
     int answer;
     debug(28, 5) ("aclCheckFast: list: %p\n", A);
+    aclChecklistCacheInit(checklist);
     while (A) {
 	allow = A->allow;
 	answer = aclMatchAclList(A->acl_list, checklist);
@@ -2131,6 +2132,15 @@
     return allow == ACCESS_DENIED;
 }
 
+int
+aclCheckFastRequest(const acl_access * A, request_t * request)
+{
+    aclCheck_t ch;
+    memset(&ch, 0, sizeof(ch));
+    ch.request = request;
+    return aclCheckFast(A, &ch);
+}
+
 static void
 aclCheck(aclCheck_t * checklist)
 {
@@ -2366,6 +2376,32 @@
     aclCheck(checklist);
 }
 
+/* Fills in common derived fields */
+void
+aclChecklistCacheInit(aclCheck_t * checklist)
+{
+    request_t *request = checklist->request;
+    if (request != NULL && checklist->src_addr.s_addr == 0) {
+#if FOLLOW_X_FORWARDED_FOR
+	if (Config.onoff.acl_uses_indirect_client) {
+	    checklist->src_addr = request->indirect_client_addr;
+	} else
+#endif /* FOLLOW_X_FORWARDED_FOR */
+	    checklist->src_addr = request->client_addr;
+	checklist->my_addr = request->my_addr;
+	checklist->my_port = request->my_port;
+#if 0 && USE_IDENT
+	/*
+	 * this is currently broken because 'request->user_ident' has been
+	 * moved to conn->rfc931 and we don't have access to the parent
+	 * ConnStateData here.
+	 */
+	if (request->user_ident[0])
+	    xstrncpy(checklist.rfc931, request->user_ident, USER_IDENT_SZ);
+#endif
+    }
+}
+
 aclCheck_t *
 aclChecklistCreate(const acl_access * A, request_t * request, const char *ident)
 {
@@ -2378,17 +2414,8 @@
      * pointer, so lock it.
      */
     cbdataLock(A);
-    if (request != NULL) {
+    if (request)
 	checklist->request = requestLink(request);
-#if FOLLOW_X_FORWARDED_FOR
-	if (Config.onoff.acl_uses_indirect_client) {
-	    checklist->src_addr = request->indirect_client_addr;
-	} else
-#endif /* FOLLOW_X_FORWARDED_FOR */
-	    checklist->src_addr = request->client_addr;
-	checklist->my_addr = request->my_addr;
-	checklist->my_port = request->my_port;
-    }
     for (i = 0; i < ACL_ENUM_MAX; i++)
 	checklist->state[i] = ACL_LOOKUP_NONE;
 #if USE_IDENT
@@ -2405,6 +2432,7 @@
     checklist->callback = callback;
     checklist->callback_data = callback_data;
     cbdataLock(callback_data);
+    aclChecklistCacheInit(checklist);
     aclCheck(checklist);
 }
 
Index: squid/src/client_side.c
===================================================================
RCS file: /cvsroot/squid/squid/src/client_side.c,v
retrieving revision 1.715
retrieving revision 1.716
diff -u -r1.715 -r1.716
--- squid/src/client_side.c	14 Mar 2007 14:08:26 -0000	1.715
+++ squid/src/client_side.c	14 Mar 2007 22:43:25 -0000	1.716
@@ -1,6 +1,6 @@
 
 /*
- * $Id: client_side.c,v 1.715 2007/03/14 14:08:26 hno Exp $
+ * $Id: client_side.c,v 1.716 2007/03/14 22:43:25 hno Exp $
  *
  * DEBUG: section 33    Client-side Routines
  * AUTHOR: Duane Wessels
@@ -2481,7 +2481,7 @@
     while (bs) {
 	checklist = clientAclChecklistCreate(bs->access_list, http);
 	checklist->reply = reply;
-	if (1 != aclCheckFast(bs->access_list, checklist)) {
+	if (aclCheckFast(bs->access_list, checklist)) {
 	    /* deny - skip this entry */
 	    bs = (body_size *) bs->node.next;
 	} else {
Index: squid/src/delay_pools.c
===================================================================
RCS file: /cvsroot/squid/squid/src/delay_pools.c,v
retrieving revision 1.34
retrieving revision 1.35
diff -u -r1.34 -r1.35
--- squid/src/delay_pools.c	23 Oct 2006 11:22:21 -0000	1.34
+++ squid/src/delay_pools.c	14 Mar 2007 22:43:25 -0000	1.35
@@ -1,6 +1,6 @@
 
 /*
- * $Id: delay_pools.c,v 1.34 2006/10/23 11:22:21 hno Exp $
+ * $Id: delay_pools.c,v 1.35 2007/03/14 22:43:25 hno Exp $
  *
  * DEBUG: section 77    Delay Pools
  * AUTHOR: David Luyer <david@luyer.net>
@@ -317,14 +317,6 @@
     r = http->request;
 
     memset(&ch, '\0', sizeof(ch));
-#if FOLLOW_X_FORWARDED_FOR
-    if (Config.onoff.delay_pool_uses_indirect_client) {
-	ch.src_addr = r->indirect_client_addr;
-    } else
-#endif /* FOLLOW_X_FORWARDED_FOR */
-	ch.src_addr = r->client_addr;
-    ch.my_addr = r->my_addr;
-    ch.my_port = r->my_port;
     ch.conn = http->conn;
     ch.request = r;
     if (r->client_addr.s_addr == INADDR_BROADCAST) {
Index: squid/src/forward.c
===================================================================
RCS file: /cvsroot/squid/squid/src/forward.c,v
retrieving revision 1.123
retrieving revision 1.124
diff -u -r1.123 -r1.124
--- squid/src/forward.c	25 Feb 2007 11:09:19 -0000	1.123
+++ squid/src/forward.c	14 Mar 2007 22:43:25 -0000	1.124
@@ -1,6 +1,6 @@
 
 /*
- * $Id: forward.c,v 1.123 2007/02/25 11:09:19 hno Exp $
+ * $Id: forward.c,v 1.124 2007/03/14 22:43:25 hno Exp $
  *
  * DEBUG: section 17    Request Forwarding
  * AUTHOR: Duane Wessels
@@ -402,6 +402,7 @@
 {
     acl_address *l;
     struct in_addr addr;
+    aclChecklistCacheInit(ch);
     for (l = head; l; l = l->next) {
 	if (aclMatchAclList(l->acl_list, ch))
 	    return l->addr;
@@ -414,6 +415,7 @@
 aclMapTOS(acl_tos * head, aclCheck_t * ch)
 {
     acl_tos *l;
+    aclChecklistCacheInit(ch);
     for (l = head; l; l = l->next) {
 	if (aclMatchAclList(l->acl_list, ch))
 	    return l->tos;
@@ -427,9 +429,6 @@
     aclCheck_t ch;
     memset(&ch, '\0', sizeof(aclCheck_t));
     if (request) {
-	ch.src_addr = request->client_addr;
-	ch.my_addr = request->my_addr;
-	ch.my_port = request->my_port;
 	ch.request = request;
     }
     return aclMapAddr(Config.accessList.outgoing_address, &ch);
@@ -441,9 +440,6 @@
     aclCheck_t ch;
     memset(&ch, '\0', sizeof(aclCheck_t));
     if (request) {
-	ch.src_addr = request->client_addr;
-	ch.my_addr = request->my_addr;
-	ch.my_port = request->my_port;
 	ch.request = request;
     }
     return aclMapTOS(Config.accessList.outgoing_tos, &ch);
@@ -801,7 +797,6 @@
 fwdStart(int fd, StoreEntry * e, request_t * r)
 {
     FwdState *fwdState;
-    aclCheck_t ch;
     int answer;
     ErrorState *err;
     /*
@@ -813,12 +808,7 @@
 	/*      
 	 * Check if this host is allowed to fetch MISSES from us (miss_access)
 	 */
-	memset(&ch, '\0', sizeof(aclCheck_t));
-	ch.src_addr = r->client_addr;
-	ch.my_addr = r->my_addr;
-	ch.my_port = r->my_port;
-	ch.request = r;
-	answer = aclCheckFast(Config.accessList.miss, &ch);
+	answer = aclCheckFastRequest(Config.accessList.miss, r);
 	if (answer == 0) {
 	    err_type page_id;
 	    page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, 1);
Index: squid/src/http.c
===================================================================
RCS file: /cvsroot/squid/squid/src/http.c,v
retrieving revision 1.428
retrieving revision 1.429
diff -u -r1.428 -r1.429
--- squid/src/http.c	8 Mar 2007 23:24:51 -0000	1.428
+++ squid/src/http.c	14 Mar 2007 22:43:25 -0000	1.429
@@ -1,6 +1,6 @@
 
 /*
- * $Id: http.c,v 1.428 2007/03/08 23:24:51 hno Exp $
+ * $Id: http.c,v 1.429 2007/03/14 22:43:25 hno Exp $
  *
  * DEBUG: section 11    Hypertext Transfer Protocol (HTTP)
  * AUTHOR: Harvest Derived
@@ -1504,15 +1504,12 @@
 httpSendRequestEntryDone(int fd, void *data)
 {
     HttpStateData *httpState = data;
-    aclCheck_t ch;
     debug(11, 5) ("httpSendRequestEntryDone: FD %d\n",
 	fd);
-    memset(&ch, '\0', sizeof(ch));
-    ch.request = httpState->request;
     if (!Config.accessList.brokenPosts) {
 	debug(11, 5) ("httpSendRequestEntryDone: No brokenPosts list\n");
 	httpSendComplete(fd, NULL, 0, 0, data);
-    } else if (!aclCheckFast(Config.accessList.brokenPosts, &ch)) {
+    } else if (!aclCheckFastRequest(Config.accessList.brokenPosts, httpState->request)) {
 	debug(11, 5) ("httpSendRequestEntryDone: didn't match brokenPosts\n");
 	httpSendComplete(fd, NULL, 0, 0, data);
     } else {
Index: squid/src/neighbors.c
===================================================================
RCS file: /cvsroot/squid/squid/src/neighbors.c,v
retrieving revision 1.315
retrieving revision 1.316
diff -u -r1.315 -r1.316
--- squid/src/neighbors.c	5 Feb 2007 15:17:12 -0000	1.315
+++ squid/src/neighbors.c	14 Mar 2007 22:43:25 -0000	1.316
@@ -1,6 +1,6 @@
 
 /*
- * $Id: neighbors.c,v 1.315 2007/02/05 15:17:12 hno Exp $
+ * $Id: neighbors.c,v 1.316 2007/03/14 22:43:25 hno Exp $
  *
  * DEBUG: section 15    Neighbor Routines
  * AUTHOR: Harvest Derived
@@ -118,7 +118,6 @@
 {
     const struct _domain_ping *d = NULL;
     int do_ping = 1;
-    aclCheck_t checklist;
     assert(request != NULL);
     if (neighborType(p, request) == PEER_SIBLING) {
 	if (request->flags.nocache)
@@ -144,21 +143,7 @@
 	return do_ping;
     if (p->access == NULL)
 	return do_ping;
-    memset(&checklist, '\0', sizeof(checklist));
-    checklist.src_addr = request->client_addr;
-    checklist.my_addr = request->my_addr;
-    checklist.my_port = request->my_port;
-    checklist.request = request;
-#if 0 && USE_IDENT
-    /*
-     * this is currently broken because 'request->user_ident' has been
-     * moved to conn->rfc931 and we don't have access to the parent
-     * ConnStateData here.
-     */
-    if (request->user_ident[0])
-	xstrncpy(checklist.rfc931, request->user_ident, USER_IDENT_SZ);
-#endif
-    return aclCheckFast(p->access, &checklist);
+    return aclCheckFastRequest(p->access, request);
 }
 
 /* Return TRUE if it is okay to send an ICP request to this peer.   */
Index: squid/src/protos.h
===================================================================
RCS file: /cvsroot/squid/squid/src/protos.h,v
retrieving revision 1.526
retrieving revision 1.527
diff -u -r1.526 -r1.527
--- squid/src/protos.h	26 Feb 2007 09:11:10 -0000	1.526
+++ squid/src/protos.h	14 Mar 2007 22:43:25 -0000	1.527
@@ -1,6 +1,6 @@
 
 /*
- * $Id: protos.h,v 1.526 2007/02/26 09:11:10 hno Exp $
+ * $Id: protos.h,v 1.527 2007/03/14 22:43:25 hno Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -56,8 +56,10 @@
 extern aclCheck_t *aclChecklistCreate(const struct _acl_access *,
     request_t *,
     const char *ident);
+void aclChecklistCacheInit(aclCheck_t * checklist);
 extern void aclNBCheck(aclCheck_t *, PF *, void *);
 extern int aclCheckFast(const struct _acl_access *A, aclCheck_t *);
+int aclCheckFastRequest(const acl_access * A, request_t * request);
 extern void aclChecklistFree(aclCheck_t *);
 extern int aclMatchAclList(const acl_list * list, aclCheck_t * checklist);
 extern void aclDestroyAccessList(struct _acl_access **list);
Index: squid/src/ssl.c
===================================================================
RCS file: /cvsroot/squid/squid/src/ssl.c,v
retrieving revision 1.136
retrieving revision 1.137
diff -u -r1.136 -r1.137
--- squid/src/ssl.c	2 Feb 2007 12:22:16 -0000	1.136
+++ squid/src/ssl.c	14 Mar 2007 22:43:25 -0000	1.137
@@ -1,6 +1,6 @@
 
 /*
- * $Id: ssl.c,v 1.136 2007/02/02 12:22:16 hno Exp $
+ * $Id: ssl.c,v 1.137 2007/03/14 22:43:25 hno Exp $
  *
  * DEBUG: section 26    Secure Sockets Layer Proxy
  * AUTHOR: Duane Wessels
@@ -486,7 +486,6 @@
     SslStateData *sslState = NULL;
     int sock;
     ErrorState *err = NULL;
-    aclCheck_t ch;
     int answer;
     int fd = http->conn->fd;
     request_t *request = http->request;
@@ -500,12 +499,7 @@
 	/*
 	 * Check if this host is allowed to fetch MISSES from us (miss_access)
 	 */
-	memset(&ch, '\0', sizeof(aclCheck_t));
-	ch.src_addr = request->client_addr;
-	ch.my_addr = request->my_addr;
-	ch.my_port = request->my_port;
-	ch.request = request;
-	answer = aclCheckFast(Config.accessList.miss, &ch);
+	answer = aclCheckFastRequest(Config.accessList.miss, http->request);
 	if (answer == 0) {
 	    err = errorCon(ERR_FORWARDING_DENIED, HTTP_FORBIDDEN, request);
 	    *status_ptr = HTTP_FORBIDDEN;