]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
[MEDIUM] http: make x-forwarded-for addition conditional
authorWilly Tarreau <w@1wt.eu>
Fri, 19 Aug 2011 20:57:24 +0000 (22:57 +0200)
committerWilly Tarreau <w@1wt.eu>
Fri, 19 Aug 2011 20:57:24 +0000 (22:57 +0200)
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
include/types/proxy.h
src/cfgparse.c
src/proto_http.c

index 291cb013fc9278138004f83ab2c75ab2befc2fcb..284a667f0eac85f662484ffac41f6af7246b6025 100644 (file)
@@ -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
-  connections, only the first request of a connection will receive the header.
-  For this reason, it is important to ensure that "option httpclose" is set
-  when using this option.
+  It is important to note that by default, HAProxy works in tunnel mode and
+  only inspects the first request of a connection, meaning that only the first
+  request will have the header appended, which is certainly not what you want.
+  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
-  connections, only the first request of a connection will receive the header.
-  For this reason, it is important to ensure that "option httpclose" is set
-  when using this option.
+  It is important to note that by default, HAProxy works in tunnel mode and
+  only inspects the first request of a connection, meaning that only the first
+  request will have the header appended, which is certainly not what you want.
+  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
index da64bb1d7ed1e883f0a810576b1304bed2407759..d51eed1f09bacdbe65ed80aaba02fee84eabb718 100644 (file)
@@ -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 */
index f6a0ef2fe6ff002a2e3e2b822514b88cb0a3089e..1e3a92cf1316899bf7bdc171c3eec0c13130433f 100644 (file)
@@ -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) {
index 578ebf7a6a57869da03cee479aa99843634d2ee8..acf2ea3adee7adca4f9ce2216f6e9a5ea2e868ff 100644 (file)
@@ -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.
                         */