[MEDIUM] http: make x-forwarded-for addition conditional

If "option forwardfor" has the "if-none" argument, then the header is
only added when the request did not already have one. This option has
security implications, and should not be set blindly.

doc/configuration.txt

@@ -3069,7 +3069,7 @@ no option forceclose
   See also : "option httpclose" and "option http-pretend-keepalive"
 
 
-option forwardfor [ except <network> ] [ header <name> ]
+option forwardfor [ except <network> ] [ header <name> ] [ if-none ]
   Enable insertion of the X-Forwarded-For header to requests sent to servers
   May be used in sections :   defaults | frontend | listen | backend
                                  yes   |    yes   |   yes  |   yes
@@ -3105,15 +3105,23 @@ option forwardfor [ except <network> ] [ header <name> ]
   network will not cause an addition of this header. Most common uses are with
   private networks or 127.0.0.1.
 
+  Alternatively, the keyword "if-none" states that the header will only be
+  added if it is not present. This should only be used in perfectly trusted
+  environment, as this might cause a security issue if headers reaching haproxy
+  are under the control of the end-user.
+
   This option may be specified either in the frontend or in the backend. If at
   least one of them uses it, the header will be added. Note that the backend's
   setting of the header subargument takes precedence over the frontend's if
-  both are defined.
+  both are defined. In the case of the "if-none" argument, if at least one of
+  the frontend or the backend does not specify it, it wants the addition to be
+  mandatory, so it wins.
 
-  It is important to note that as long as HAProxy does not support keep-alive
+  It is important to note that by default, HAProxy works in tunnel mode and
-  connections, only the first request of a connection will receive the header.
+  only inspects the first request of a connection, meaning that only the first
-  For this reason, it is important to ensure that "option httpclose" is set
+  request will have the header appended, which is certainly not what you want.
-  when using this option.
+  In order to fix this, ensure that any of the "httpclose", "forceclose" or
+  "http-server-close" options is set when using this option.
 
   Examples :
     # Public HTTP address also used by stunnel on the same machine
@@ -3126,7 +3134,8 @@ option forwardfor [ except <network> ] [ header <name> ]
         mode http
         option forwardfor header X-Client
 
-  See also : "option httpclose"
+  See also : "option httpclose", "option http-server-close",
+             "option forceclose"
 
 
 option http-no-delay
@@ -3685,10 +3694,11 @@ option originalto [ except <network> ] [ header <name> ]
   setting of the header subargument takes precedence over the frontend's if
   both are defined.
 
-  It is important to note that as long as HAProxy does not support keep-alive
+  It is important to note that by default, HAProxy works in tunnel mode and
-  connections, only the first request of a connection will receive the header.
+  only inspects the first request of a connection, meaning that only the first
-  For this reason, it is important to ensure that "option httpclose" is set
+  request will have the header appended, which is certainly not what you want.
-  when using this option.
+  In order to fix this, ensure that any of the "httpclose", "forceclose" or
+  "http-server-close" options is set when using this option.
 
   Examples :
     # Original Destination address
@@ -3701,7 +3711,8 @@ option originalto [ except <network> ] [ header <name> ]
         mode http
         option originalto header X-Client-Dst
 
-  See also : "option httpclose"
+  See also : "option httpclose", "option http-server-close",
+             "option forceclose"
 
 
 option persist

include/types/proxy.h

@@ -80,12 +80,12 @@ enum {
 #define PR_O_COOK_ANY   (PR_O_COOK_RW | PR_O_COOK_IND | PR_O_COOK_INS | PR_O_COOK_PFX)
 #define PR_O_DISPATCH   0x00000040      /* use dispatch mode */
 #define PR_O_KEEPALIVE  0x00000080      /* follow keep-alive sessions */
-#define PR_O_FWDFOR     0x00000100      /* insert x-forwarded-for with client address */
+#define PR_O_FWDFOR     0x00000100      /* conditionally insert x-forwarded-for with client address */
 #define PR_O_BIND_SRC   0x00000200      /* bind to a specific source address when connect()ing */
 #define PR_O_NULLNOLOG  0x00000400      /* a connect without request will not be logged */
 #define PR_O_COOK_NOC   0x00000800      /* add a 'Cache-control' header with the cookie */
 #define PR_O_COOK_POST  0x00001000      /* don't insert cookies for requests other than a POST */
-/* unused: 0x00002000 */
+#define PR_O_FF_ALWAYS  0x00002000      /* always set x-forwarded-for */
 #define PR_O_PERSIST    0x00004000      /* server persistence stays effective even when server is down */
 #define PR_O_LOGASAP    0x00008000      /* log as soon as possible, without waiting for the session to complete */
 #define PR_O_HTTP_CLOSE 0x00010000      /* force 'connection: close' in both directions */

src/cfgparse.c

@@ -3484,7 +3484,7 @@ stats_error_parsing:
 			 * set default options (ie: bitfield, header name, etc) 
 			 */
 
-			curproxy->options |= PR_O_FWDFOR;
+			curproxy->options |= PR_O_FWDFOR | PR_O_FF_ALWAYS;
 
 			free(curproxy->fwdfor_hdr_name);
 			curproxy->fwdfor_hdr_name = strdup(DEF_XFORWARDFOR_HDR);
@@ -3516,9 +3516,12 @@ stats_error_parsing:
 					curproxy->fwdfor_hdr_name = strdup(args[cur_arg+1]);
 					curproxy->fwdfor_hdr_len  = strlen(curproxy->fwdfor_hdr_name);
 					cur_arg += 2;
+				} else if (!strcmp(args[cur_arg], "if-none")) {
+					curproxy->options &= ~PR_O_FF_ALWAYS;
+					cur_arg += 1;
 				} else {
 					/* unknown suboption - catchall */
-					Alert("parsing [%s:%d] : '%s %s' only supports optional values: 'except' and 'header'.\n",
+					Alert("parsing [%s:%d] : '%s %s' only supports optional values: 'except', 'header' and 'if-none'.\n",
 					      file, linenum, args[0], args[1]);
 					err_code |= ERR_ALERT | ERR_FATAL;
 					goto out;
@@ -3538,7 +3541,7 @@ stats_error_parsing:
 			curproxy->orgto_hdr_name = strdup(DEF_XORIGINALTO_HDR);
 			curproxy->orgto_hdr_len  = strlen(DEF_XORIGINALTO_HDR);
 
-			/* loop to go through arguments - start at 2, since 0+1 = "option" "forwardfor" */
+			/* loop to go through arguments - start at 2, since 0+1 = "option" "originalto" */
 			cur_arg = 2;
 			while (*(args[cur_arg])) {
 				if (!strcmp(args[cur_arg], "except")) {
@@ -6089,11 +6092,11 @@ out_uri_auth_compat:
 				curproxy->uri_auth = NULL;
 			}
 
-			if (curproxy->options & PR_O_FWDFOR) {
+			if (curproxy->options & (PR_O_FWDFOR | PR_O_FF_ALWAYS)) {
 				Warning("config : 'option %s' ignored for %s '%s' as it requires HTTP mode.\n",
 					"forwardfor", proxy_type_str(curproxy), curproxy->id);
 				err_code |= ERR_WARN;
-				curproxy->options &= ~PR_O_FWDFOR;
+				curproxy->options &= ~(PR_O_FWDFOR | PR_O_FF_ALWAYS);
 			}
 
 			if (curproxy->options & PR_O_ORGTO) {

src/proto_http.c

@@ -3541,7 +3541,15 @@ int http_process_request(struct session *s, struct buffer *req, int an_bit)
 	 * asks for it.
 	 */
 	if ((s->fe->options | s->be->options) & PR_O_FWDFOR) {
-		if (s->req->prod->addr.c.from.ss_family == AF_INET) {
+		struct hdr_ctx ctx = { .idx = 0 };
+
+		if (!((s->fe->options | s->be->options) & PR_O_FF_ALWAYS) &&
+		    http_find_header2("X-Forwarded-For", 15, txn->req.sol, &txn->hdr_idx, &ctx)) {
+			/* The header is set to be added only if none is present
+			 * and we found it, so don't do anything.
+			 */
+		}
+		else if (s->req->prod->addr.c.from.ss_family == AF_INET) {
 			/* Add an X-Forwarded-For header unless the source IP is
 			 * in the 'except' network range.
 			 */