]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: http-ana: Add a proxy option to restrict chars in request header names
authorChristopher Faulet <cfaulet@haproxy.com>
Mon, 16 May 2022 09:43:10 +0000 (11:43 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Mon, 16 May 2022 14:00:26 +0000 (16:00 +0200)
The "http-restrict-req-hdr-names" option can now be set to restrict allowed
characters in the request header names to the "[a-zA-Z0-9-]" charset.

Idea of this option is to not send header names with non-alphanumeric or
hyphen character. It is especially important for FastCGI application because
all those characters are converted to underscore. For instance,
"X-Forwarded-For" and "X_Forwarded_For" are both converted to
"HTTP_X_FORWARDED_FOR". So, header names can be mixed up by FastCGI
applications. And some HAProxy rules may be bypassed by mangling header
names. In addition, some non-HTTP compliant servers may incorrectly handle
requests when header names contain characters ouside the "[a-zA-Z0-9-]"
charset.

When this option is set, the policy must be specify:

  * preserve: It disables the filtering. It is the default mode for HTTP
              proxies with no FastCGI application configured.

  * delete: It removes request headers with a name containing a character
            outside the "[a-zA-Z0-9-]" charset. It is the default mode for
            HTTP backends with a configured FastCGI application.

  * reject: It rejects the request with a 403-Forbidden response if it
            contains a header name with a character outside the
            "[a-zA-Z0-9-]" charset.

The option is evaluated per-proxy and after http-request rules evaluation.

This patch may be backported to avoid any secuirty issue with FastCGI
application (so as far as 2.2).

doc/configuration.txt
include/haproxy/proxy-t.h
reg-tests/http-rules/restrict_req_hdr_names.vtc [new file with mode: 0644]
src/cfgparse-listen.c
src/fcgi-app.c
src/http_ana.c

index 34e6846d51f4030c76f9eb0232a92115def443cd..1b57c6969dba18ddd16ebbdf30ffeeaace0bab44 100644 (file)
@@ -3876,6 +3876,7 @@ option http-ignore-probes            (*)  X          X         X         -
 option http-keep-alive               (*)  X          X         X         X
 option http-no-delay                 (*)  X          X         X         X
 option http-pretend-keepalive        (*)  X          -         X         X
+option http-restrict-req-hdr-names        X          X         X         X
 option http-server-close             (*)  X          X         X         X
 option http-use-proxy-header         (*)  X          X         X         -
 option httpchk                            X          -         X         X
@@ -8904,6 +8905,33 @@ no option http-pretend-keepalive
   See also : "option httpclose", "option http-server-close", and
              "option http-keep-alive"
 
+option http-restrict-req-hdr-names { preserve | delete | reject }
+  Set HAProxy policy about HTTP request header names containing characters
+  outside the "[a-zA-Z0-9-]" charset
+  May be used in sections :   defaults | frontend | listen | backend
+                                 yes   |    yes   |   yes  |   yes
+  Arguments :
+      preserve  disable the filtering. It is the default mode for HTTP proxies
+                with no FastCGI application configured.
+
+      delete    remove request headers with a name containing a character
+                outside the "[a-zA-Z0-9-]" charset. It is the default mode for
+                HTTP backends with a configured FastCGI application.
+
+      reject    reject the request with a 403-Forbidden response if it contains a
+                header name with a character outside the "[a-zA-Z0-9-]" charset.
+
+  This option may be used to restrict the request header names to alphanumeric
+  and hyphen characters ([A-Za-z0-9-]). This may be mandatory to interoperate
+  with non-HTTP compliant servers that fail to handle some characters in header
+  names. It may also be mandatory for FastCGI applications because all
+  non-alphanumeric characters in header names are replaced by an underscore
+  ('_'). Thus, it is easily possible to mix up header names and bypass some
+  rules. For instance, "X-Forwarded-For" and "X_Forwarded-For" headers are both
+  converted to "HTTP_X_FORWARDED_FOR" in FastCGI.
+
+  Note this option is evaluated per proxy and after the http-request rules
+  evaluation.
 
 option http-server-close
 no option http-server-close
index 80431757ed949706e097120e9e542ca13ec78a64..3d8e0a59e0b51be39fb2a0d5868ad599ccd6ee27 100644 (file)
@@ -143,7 +143,12 @@ enum PR_SRV_STATE_FILE {
 #define PR_O2_SRC_ADDR 0x00100000      /* get the source ip and port for logs */
 
 #define PR_O2_FAKE_KA   0x00200000      /* pretend we do keep-alive with server even though we close */
-/* unused : 0x00400000..0x80000000 */
+
+#define PR_O2_RSTRICT_REQ_HDR_NAMES_BLK  0x00400000 /* reject request with header names containing chars ouside of [0-9a-zA-Z-] charset */
+#define PR_O2_RSTRICT_REQ_HDR_NAMES_DEL  0x00800000 /* remove request header names containing chars outside of [0-9a-zA-Z-] charset */
+#define PR_O2_RSTRICT_REQ_HDR_NAMES_NOOP 0x01000000 /* preserve request header names containing chars ouside of [0-9a-zA-Z-] charset */
+#define PR_O2_RSTRICT_REQ_HDR_NAMES_MASK 0x01c00000 /* mask for restrict-http-header-names option */
+/* unused : 0x0000000..0x80000000 */
 
 /* server health checks */
 #define PR_O2_CHK_NONE  0x00000000      /* no L7 health checks configured (TCP by default) */
diff --git a/reg-tests/http-rules/restrict_req_hdr_names.vtc b/reg-tests/http-rules/restrict_req_hdr_names.vtc
new file mode 100644 (file)
index 0000000..28a10d3
--- /dev/null
@@ -0,0 +1,123 @@
+varnishtest "http-restrict-req-hdr-names option tests"
+#REQUIRE_VERSION=2.6
+
+# This config tests "http-restrict-req-hdr-names" option
+
+feature ignore_unknown_macro
+
+server s1 {
+    rxreq
+    expect req.http.x-my_hdr  == on
+    txresp
+} -start
+
+server s2 {
+    rxreq
+    expect req.http.x-my_hdr  == <undef>
+    txresp
+} -start
+
+server s3 {
+    rxreq
+    expect req.http.x-my_hdr  == on
+    txresp
+} -start
+
+server s4 {
+    rxreq
+    expect req.http.x-my_hdr  == <undef>
+    txresp
+} -start
+
+server s5 {
+    rxreq
+    expect req.http.x-my_hdr  == on
+    txresp
+} -start
+
+haproxy h1 -conf {
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend fe1
+        bind "fd@${fe1}"
+        use_backend be-http1 if { path /req1 }
+        use_backend be-http2 if { path /req2 }
+        use_backend be-http3 if { path /req3 }
+        use_backend be-fcgi1 if { path /req4 }
+        use_backend be-fcgi2 if { path /req5 }
+        use_backend be-fcgi3 if { path /req6 }
+
+    backend be-http1
+        server s1 ${s1_addr}:${s1_port}
+
+    backend be-http2
+        option http-restrict-req-hdr-names delete
+        server s2 ${s2_addr}:${s2_port}
+
+    backend be-http3
+        option http-restrict-req-hdr-names reject
+
+    backend be-fcgi1
+        option http-restrict-req-hdr-names preserve
+        server s3 ${s3_addr}:${s3_port}
+
+    backend be-fcgi2
+        option http-restrict-req-hdr-names delete
+        server s4 ${s4_addr}:${s4_port}
+
+    backend be-fcgi3
+        option http-restrict-req-hdr-names reject
+
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+        option http-restrict-req-hdr-names preserve
+
+    frontend fe2
+        bind "fd@${fe2}"
+        default_backend be-fcgi4
+
+    backend be-fcgi4
+        server s5 ${s5_addr}:${s5_port}
+
+    fcgi-app my-fcgi-app
+        docroot ${testdir}
+} -start
+
+client c1 -connect ${h1_fe1_sock} {
+    txreq -req GET -url /req1 -hdr "X-my_hdr: on"
+    rxresp
+    expect resp.status == 200
+
+    txreq -req GET -url /req2 -hdr "X-my_hdr: on"
+    rxresp
+    expect resp.status == 200
+
+    txreq -req GET -url /req3 -hdr "X-my_hdr: on"
+    rxresp
+    expect resp.status == 403
+
+    txreq -req GET -url /req4 -hdr "X-my_hdr: on"
+    rxresp
+    expect resp.status == 200
+
+    txreq -req GET -url /req5 -hdr "X-my_hdr: on"
+    rxresp
+    expect resp.status == 200
+
+    txreq -req GET -url /req6 -hdr "X-my_hdr: on"
+    rxresp
+    expect resp.status == 403
+} -run
+
+client c2 -connect ${h1_fe2_sock} {
+    txreq -req GET -url /req1 -hdr "X-my_hdr: on"
+    rxresp
+    expect resp.status == 200
+} -run
index d8902953edb0597415e2e2435e78b606478f34ce..e7cd65146f4e7b69bdc6564312bc64f1d7492f6b 100644 (file)
@@ -2459,6 +2459,38 @@ stats_error_parsing:
                                }
                        } /* end while loop */
                }
+               else if (strcmp(args[1], "http-restrict-req-hdr-names") == 0) {
+                       if (kwm != KWM_STD) {
+                               ha_alert("parsing [%s:%d]: negation/default is not supported for option '%s'.\n",
+                                        file, linenum, args[1]);
+                               err_code |= ERR_ALERT | ERR_FATAL;
+                               goto out;
+                       }
+
+                       if (alertif_too_many_args(2, file, linenum, args, &err_code))
+                               goto out;
+
+                       if (*(args[2]) == 0) {
+                               ha_alert("parsing [%s:%d] : missing parameter. option '%s' expects 'preserve', 'reject' or 'delete' option.\n",
+                                        file, linenum, args[1]);
+                               err_code |= ERR_ALERT | ERR_FATAL;
+                               goto out;
+                       }
+
+                       curproxy->options2 &= ~PR_O2_RSTRICT_REQ_HDR_NAMES_MASK;
+                       if (strcmp(args[2], "preserve") == 0)
+                               curproxy->options2 |= PR_O2_RSTRICT_REQ_HDR_NAMES_NOOP;
+                       else if (strcmp(args[2], "reject") == 0)
+                               curproxy->options2 |= PR_O2_RSTRICT_REQ_HDR_NAMES_BLK;
+                       else if (strcmp(args[2], "delete") == 0)
+                               curproxy->options2 |= PR_O2_RSTRICT_REQ_HDR_NAMES_DEL;
+                       else {
+                               ha_alert("parsing [%s:%d] : invalid parameter '%s'. option '%s' expects 'preserve', 'reject' or 'delete' option.\n",
+                                        file, linenum, args[2], args[1]);
+                               err_code |= ERR_ALERT | ERR_FATAL;
+                               goto out;
+                       }
+               }
                else {
                        const char *best = proxy_find_best_option(args[1], common_options);
 
index 0bee447af12d100f665ed36be91bf7f0ec66a0dc..c9405ec4dd65d15656001ef112a921fc925dd020 100644 (file)
@@ -663,6 +663,12 @@ static int cfg_fcgi_apps_postparser()
                        goto end;
                }
 
+               /* By default, for FCGI-ready backend, HTTP request header names
+                * are restricted and the "delete" policy is set
+                */
+               if (fcgi_conf && !(px->options2 & PR_O2_RSTRICT_REQ_HDR_NAMES_MASK))
+                       px->options2 |= PR_O2_RSTRICT_REQ_HDR_NAMES_DEL;
+
                for (srv = px->srv; srv; srv = srv->next) {
                        if (srv->mux_proto && isteq(srv->mux_proto->token, ist("fcgi"))) {
                                nb_fcgi_srv++;
index 4b3113e2dc8dc19988d1f96465ebb556be3649d9..c18d0764dc50be49659e1d55f2c5e71ab932a653 100644 (file)
@@ -60,6 +60,7 @@ static void http_debug_hdr(const char *dir, struct stream *s, const struct ist n
 
 static enum rule_result http_req_get_intercept_rule(struct proxy *px, struct list *def_rules, struct list *rules, struct stream *s);
 static enum rule_result http_res_get_intercept_rule(struct proxy *px, struct list *def_rules, struct list *rules, struct stream *s);
+static enum rule_result http_req_restrict_header_names(struct stream *s, struct htx *htx, struct proxy *px);
 
 static void http_manage_client_side_cookies(struct stream *s, struct channel *req);
 static void http_manage_server_side_cookies(struct stream *s, struct channel *res);
@@ -404,6 +405,12 @@ int http_process_req_common(struct stream *s, struct channel *req, int an_bit, s
                }
        }
 
+       if (px->options2 & (PR_O2_RSTRICT_REQ_HDR_NAMES_BLK|PR_O2_RSTRICT_REQ_HDR_NAMES_DEL)) {
+               verdict = http_req_restrict_header_names(s, htx, px);
+               if (verdict == HTTP_RULE_RES_DENY)
+                       goto deny;
+       }
+
        if (conn && (conn->flags & CO_FL_EARLY_DATA) &&
            (conn->flags & (CO_FL_EARLY_SSL_HS | CO_FL_SSL_WAIT_HS))) {
                struct http_hdr_ctx ctx;
@@ -2586,6 +2593,50 @@ int http_apply_redirect_rule(struct redirect_rule *rule, struct stream *s, struc
        goto out;
 }
 
+/* This function filters the request header names to only allow [0-9a-zA-Z-]
+ * characters. Depending on the proxy configuration, headers with a name not
+ * matching this charset are removed or the request is rejected with a
+ * 403-Forbidden response if such name are found. It returns HTTP_RULE_RES_CONT
+ * to continue the request processing or HTTP_RULE_RES_DENY if the request is
+ * rejected.
+ */
+static enum rule_result http_req_restrict_header_names(struct stream *s, struct htx *htx, struct proxy *px)
+{
+       struct htx_blk *blk;
+       enum rule_result rule_ret = HTTP_RULE_RES_CONT;
+
+       blk = htx_get_first_blk(htx);
+       while (blk) {
+               enum htx_blk_type type = htx_get_blk_type(blk);
+
+               if (type == HTX_BLK_HDR) {
+                       struct ist n = htx_get_blk_name(htx, blk);
+                       int i;
+
+                       for (i = 0; i < istlen(n); i++) {
+                               if (!isalnum((unsigned char)n.ptr[i]) && n.ptr[i] != '-') {
+                                       /* Block the request or remove the header */
+                                       if (px->options2 & PR_O2_RSTRICT_REQ_HDR_NAMES_BLK)
+                                               goto block;
+                                       blk = htx_remove_blk(htx, blk);
+                                       continue;
+                               }
+                       }
+               }
+               if (type == HTX_BLK_EOH)
+                       break;
+
+               blk = htx_get_next_blk(htx, blk);
+       }
+  out:
+       return rule_ret;
+  block:
+       /* Block the request returning a 403-Forbidden response */
+       s->txn->status = 403;
+       rule_ret = HTTP_RULE_RES_DENY;
+       goto out;
+}
+
 /* Replace all headers matching the name <name>. The header value is replaced if
  * it matches the regex <re>. <str> is used for the replacement. If <full> is
  * set to 1, the full-line is matched and replaced. Otherwise, comma-separated