diff --git a/doc/configuration.txt b/doc/configuration.txt index 34e6846d5..1b57c6969 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -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 diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h index 80431757e..3d8e0a59e 100644 --- a/include/haproxy/proxy-t.h +++ b/include/haproxy/proxy-t.h @@ -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 index 000000000..28a10d3db --- /dev/null +++ b/reg-tests/http-rules/restrict_req_hdr_names.vtc @@ -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 == + txresp +} -start + +server s3 { + rxreq + expect req.http.x-my_hdr == on + txresp +} -start + +server s4 { + rxreq + expect req.http.x-my_hdr == + 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 diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c index d8902953e..e7cd65146 100644 --- a/src/cfgparse-listen.c +++ b/src/cfgparse-listen.c @@ -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); diff --git a/src/fcgi-app.c b/src/fcgi-app.c index 0bee447af..c9405ec4d 100644 --- a/src/fcgi-app.c +++ b/src/fcgi-app.c @@ -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++; diff --git a/src/http_ana.c b/src/http_ana.c index 4b3113e2d..c18d0764d 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -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 . The header value is replaced if * it matches the regex . is used for the replacement. If is * set to 1, the full-line is matched and replaced. Otherwise, comma-separated