From 4de663269367f689cb1607d621d77db2d88d95f0 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Thu, 12 Sep 2024 09:33:32 +0200 Subject: [PATCH] MINOR: proxy: Rename accept-invalid-http-* options With these options, it is possible to accept some invalid messages that may considered as unsafe and may result as vulnerabilities. The naming is not explicit enough on this point. These option must really be considered as dangerous and only used as a temporary workaround. Unfortunately, when used, it is probably because there are some legacy and unsupported applications in place. Nevermind. The documentation warns about the use of these options. Now the name of the options itself is a warning. So now, "accept-invalid-http-request" and "accept-invalid-http-response" options are deprecated and replaced by "accept-unsafe-violations-in-http-request" and "accept-unsafe-violations-in-http-response" options. --- doc/configuration.txt | 60 +++++++++++++++++--------- doc/management.txt | 11 ++--- reg-tests/http-rules/normalize_uri.vtc | 4 +- src/cfgparse-listen.c | 39 +++++++++++++++++ src/h1.c | 2 +- src/h1_htx.c | 6 +-- src/h2.c | 2 +- src/h3.c | 2 +- src/proxy.c | 4 +- 9 files changed, 94 insertions(+), 36 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index 1ed37805c..46c3ab4c0 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -5418,8 +5418,10 @@ mode X X X X monitor fail - X X - monitor-uri X X X - option abortonclose (*) X - X X -option accept-invalid-http-request (*) X X X - -option accept-invalid-http-response (*) X - X X +option accept-invalid-http-request (deprecated) (*) X X X - +option accept-invalid-http-response (deprecated) (*) X - X X +option accept-unsafe-violations-in-http-request (*) X X X - +option accept-unsafe-violations-in-http-response (*) X - X X option allbackups (*) X - X X option checkcache (*) X - X X option clitcpka (*) X X X - @@ -8830,8 +8832,24 @@ no option abortonclose See also : "timeout queue" and server's "maxconn" and "maxqueue" parameters -option accept-invalid-http-request -no option accept-invalid-http-request +option accept-invalid-http-request (deprecated) +no option accept-invalid-http-request (deprecated) + Enable or disable relaxing of HTTP request parsing + + The "accept-invalid-http-request" keyword is deprecated, use "option + accept-unsafe-violations-in-http-request" instead. + + +option accept-invalid-http-response (deprecated) +no option accept-invalid-http-response (deprecated) + Enable or disable relaxing of HTTP response parsing + + The "accept-invalid-http-response" keyword is deprecated, use "option + accept-unsafe-violations-in-http-response" instead. + + +option accept-unsafe-violations-in-http-request +no option accept-unsafe-violations-in-http-request Enable or disable relaxing of HTTP request parsing May be used in the following contexts: http @@ -8892,12 +8910,12 @@ no option accept-invalid-http-request If this option has been enabled in a "defaults" section, it can be disabled in a specific instance by prepending the "no" keyword before it. - See also : "option accept-invalid-http-response" and "show errors" on the - stats socket. + See also : "option accept-unsafe-violations-in-http-response" and "show + errors" on the stats socket. -option accept-invalid-http-response -no option accept-invalid-http-response +option accept-unsafe-violations-in-http-response +no option accept-unsafe-violations-in-http-response Enable or disable relaxing of HTTP response parsing May be used in the following contexts: http @@ -8907,11 +8925,11 @@ no option accept-invalid-http-response Arguments : none - Similarly to "option accept-invalid-http-request", this option may be used to - relax parsing rules of HTTP responses. It should only be enabled for trusted - legacy servers to accept some invalid responses. Most of rules concern the H1 - parsing for historical reason. Newer HTTP versions tends to be cleaner and - applications follow more stickly these protocols. + Similarly to "option accept-unsafe-violations-in-http-request", this option + may be used to relax parsing rules of HTTP responses. It should only be + enabled for trusted legacy servers to accept some invalid responses. Most of + rules concern the H1 parsing for historical reason. Newer HTTP versions tends + to be cleaner and applications follow more stickly these protocols. When this option is set, the following rules are observed: @@ -8942,8 +8960,8 @@ no option accept-invalid-http-response If this option has been enabled in a "defaults" section, it can be disabled in a specific instance by prepending the "no" keyword before it. - See also : "option accept-invalid-http-request" and "show errors" on the - stats socket. + See also : "option accept-unsafe-violations-in-http-request" and "show + errors" on the stats socket. option allbackups @@ -25033,8 +25051,8 @@ path : string the derivative forms. See also the "url" and "base" fetch methods. Please note that any fragment reference in the URI ('#' after the path) is strictly forbidden by the HTTP standard and will be rejected. However, if the frontend - receiving the request has "option accept-invalid-http-request", then this - fragment part will be accepted and will also appear in the path. + receiving the request has "option accept-unsafe-violations-in-http-request", + then this fragment part will be accepted and will also appear in the path. ACL derivatives : path : exact string match @@ -25055,8 +25073,8 @@ pathq : string result in both cases. Please note that any fragment reference in the URI ('#' after the path) is strictly forbidden by the HTTP standard and will be rejected. However, if the frontend receiving the request has "option - accept-invalid-http-request", then this fragment part will be accepted and - will also appear in the path. + accept-unsafe-violations-in-http-request", then this fragment part will be + accepted and will also appear in the path. query : string This extracts the request's query string, which starts after the first @@ -25324,8 +25342,8 @@ url : string also "path" and "base". Please note that any fragment reference in the URI ('#' after the path) is strictly forbidden by the HTTP standard and will be rejected. However, if the frontend receiving the request has "option - accept-invalid-http-request", then this fragment part will be accepted and - will also appear in the url. + accept-unsafe-violations-in-http-request", then this fragment part will be + accepted and will also appear in the url. ACL derivatives : url : exact string match diff --git a/doc/management.txt b/doc/management.txt index 360ffa181..2117a98de 100644 --- a/doc/management.txt +++ b/doc/management.txt @@ -4488,11 +4488,12 @@ best thing to do is to connect to the CLI and issue "show errors", which will report the last captured faulty request and response for each frontend and backend, with all the necessary information to indicate precisely the first character of the input stream that was rejected. This is sometimes needed to -prove to customers or to developers that a bug is present in their code. In -this case it is often possible to relax the checks (but still keep the -captures) using "option accept-invalid-http-request" or its equivalent for -responses coming from the server "option accept-invalid-http-response". Please -see the configuration manual for more details. +prove to customers or to developers that a bug is present in their code. In this +case it is often possible to relax the checks (but still keep the captures) +using "option accept-unsafe-violations-in-http-request" or its equivalent for +responses coming from the server "option +accept-unsafe-violations-in-http-response". Please see the configuration manual +for more details. Example : diff --git a/reg-tests/http-rules/normalize_uri.vtc b/reg-tests/http-rules/normalize_uri.vtc index ad7b44acf..0a9063050 100644 --- a/reg-tests/http-rules/normalize_uri.vtc +++ b/reg-tests/http-rules/normalize_uri.vtc @@ -127,7 +127,7 @@ haproxy h1 -conf { frontend fe_fragment_strip bind "fd@${fe_fragment_strip}" - option accept-invalid-http-request + option accept-unsafe-violations-in-http-request http-request set-var(txn.before) url http-request normalize-uri fragment-strip @@ -140,7 +140,7 @@ haproxy h1 -conf { frontend fe_fragment_encode bind "fd@${fe_fragment_encode}" - option accept-invalid-http-request + option accept-unsafe-violations-in-http-request http-request set-var(txn.before) url http-request normalize-uri fragment-encode diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c index 9f2786e8d..0c59549f1 100644 --- a/src/cfgparse-listen.c +++ b/src/cfgparse-listen.c @@ -2324,6 +2324,45 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) goto out; } } + else if (strcmp(args[1], "accept-invalid-http-request") == 0 || + strcmp(args[1], "accept-invalid-http-response") == 0) { + unsigned int val; + + if (alertif_too_many_args_idx(0, 1, file, linenum, args, &err_code)) + goto out; + if (warnifnotcap(curproxy, PR_MODE_HTTP, file, linenum, args[1], NULL)) { + err_code |= ERR_WARN; + goto out; + } + + if (args[1][22] == 'q') { + ha_alert("parsing [%s:%d]: option '%s' is deprecated. please use 'option accept-unsafe-violations-in-http-request' if absolutely needed.\n", + file, linenum, args[1]); + val = PR_O2_REQBUG_OK; + } + else { + ha_alert("parsing [%s:%d]: option '%s' is deprecated. please use 'option accept-unsafe-violations-in-http-response' if absolutely needed.\n", + file, linenum, args[1]); + val = PR_O2_RSPBUG_OK; + } + + curproxy->no_options2 &= ~val; + curproxy->options2 &= ~val; + + switch (kwm) { + case KWM_STD: + curproxy->options2 |= val; + break; + case KWM_NO: + curproxy->no_options2 |= val; + break; + case KWM_DEF: /* already cleared */ + break; + } + + err_code |= ERR_WARN; + goto out; + } else { const char *best = proxy_find_best_option(args[1], common_options); diff --git a/src/h1.c b/src/h1.c index 09047fd2c..faf18ff13 100644 --- a/src/h1.c +++ b/src/h1.c @@ -628,7 +628,7 @@ int h1_headers_to_hdr_list(char *start, const char *stop, } if (likely((unsigned char)*ptr >= 128)) { /* non-ASCII chars are forbidden unless option - * accept-invalid-http-request is enabled in the frontend. + * accept-unsafe-violations-in-http-request is enabled in the frontend. * In any case, we capture the faulty char. */ if (h1m->err_pos < -1) diff --git a/src/h1_htx.c b/src/h1_htx.c index c5b9dd81d..37364b8c2 100644 --- a/src/h1_htx.c +++ b/src/h1_htx.c @@ -51,7 +51,7 @@ static int h1_process_req_vsn(struct h1m *h1m, union h1_sl *sl) { /* RFC7230#2.6 has enforced the format of the HTTP version string to be * exactly one digit "." one digit. This check may be disabled using - * option accept-invalid-http-request. + * option accept-unsafe-violations-in-http-request. */ if (h1m->err_pos == -2) { /* PR_O2_REQBUG_OK not set */ if (sl->rq.v.len != 8) @@ -93,9 +93,9 @@ static int h1_process_res_vsn(struct h1m *h1m, union h1_sl *sl) { /* RFC7230#2.6 has enforced the format of the HTTP version string to be * exactly one digit "." one digit. This check may be disabled using - * option accept-invalid-http-request. + * option accept-unsafe-violations-in-http-response. */ - if (h1m->err_pos == -2) { /* PR_O2_REQBUG_OK not set */ + if (h1m->err_pos == -2) { /* PR_O2_RSPBUG_OK not set */ if (sl->st.v.len != 8) return 0; diff --git a/src/h2.c b/src/h2.c index a9e8b65b1..731c21c47 100644 --- a/src/h2.c +++ b/src/h2.c @@ -303,7 +303,7 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr, * will be used to create a linked list, so its contents may be destroyed. * * When is non-nul, some non-dangerous checks will be ignored. This - * is in order to satisfy "option accept-invalid-http-request" for + * is in order to satisfy "option accept-unsafe-violations-in-http-request" for * interoperability purposes. */ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *msgf, unsigned long long *body_len, int relaxed) diff --git a/src/h3.c b/src/h3.c index 9cc7aa274..9d8a73e15 100644 --- a/src/h3.c +++ b/src/h3.c @@ -652,7 +652,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf, if (!relaxed) { /* we need to reject any control chars or '#' from the path, - * unless option accept-invalid-http-request is set. + * unless option accept-unsafe-violations-in-http-request is set. */ ctl = ist_find_range(list[hdr_idx].v, 0, '#'); if (unlikely(ctl) && http_path_has_forbidden_char(list[hdr_idx].v, ctl)) { diff --git a/src/proxy.c b/src/proxy.c index 89a337081..e719d86e2 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -113,8 +113,8 @@ const struct cfg_opt cfg_opts2[] = { "splice-response", 0, 0, 0, 0 }, { "splice-auto", 0, 0, 0, 0 }, #endif - { "accept-invalid-http-request", PR_O2_REQBUG_OK, PR_CAP_FE, 0, PR_MODE_HTTP }, - { "accept-invalid-http-response", PR_O2_RSPBUG_OK, PR_CAP_BE, 0, PR_MODE_HTTP }, + { "accept-unsafe-violations-in-http-request", PR_O2_REQBUG_OK, PR_CAP_FE, 0, PR_MODE_HTTP }, + { "accept-unsafe-violations-in-http-response", PR_O2_RSPBUG_OK, PR_CAP_BE, 0, PR_MODE_HTTP }, { "dontlog-normal", PR_O2_NOLOGNORM, PR_CAP_FE, 0, 0 }, { "log-separate-errors", PR_O2_LOGERRORS, PR_CAP_FE, 0, 0 }, { "log-health-checks", PR_O2_LOGHCHKS, PR_CAP_BE, 0, 0 },