MINOR: mux-h1: Add global option accpet payload for any HTTP/1.0 requests

Since the 2.5, for security reason, HTTP/1.0 GET/HEAD/DELETE requests with a
payload are rejected (See e136bd12a "MEDIUM: mux-h1: Reject HTTP/1.0
GET/HEAD/DELETE requests with a payload" for details). However it may be an
issue for old clients.

To avoid any compatibility issue with such clients,
"h1-accept-payload-with-any-method" global option was added. It must only be
set if there is a good reason to do so because it may lead to a request
smuggling attack on some servers or intermediaries.

This patch should solve the issue #1691. it may be backported to 2.5.
This commit is contained in:
Christopher Faulet 2022-05-13 09:20:13 +02:00
parent ae053b30da
commit 0f9c0f5801
2 changed files with 35 additions and 7 deletions

View File

@ -1013,6 +1013,7 @@ The following keywords are supported in the "global" section :
- httpclient.resolvers.prefer - httpclient.resolvers.prefer
- httpclient.ssl.ca-file - httpclient.ssl.ca-file
- httpclient.ssl.verify - httpclient.ssl.verify
- h1-accept-payload-with-any-method
- h1-case-adjust - h1-case-adjust
- h1-case-adjust-file - h1-case-adjust-file
- insecure-fork-wanted - insecure-fork-wanted
@ -1450,6 +1451,20 @@ hard-stop-after <time>
See also: grace See also: grace
h1-accept-payload-with-any-method
Does not reject HTTP/1.0 GET/HEAD/DELETE requests with a payload.
While It is explicitly allowed in HTTP/1.1, HTTP/1.0 is not clear on this
point and some old servers don't expect any payload and never look for body
length (via Content-Length or Transfer-Encoding headers). It means that some
intermediaries may properly handle the payload for HTTP/1.0 GET/HEAD/DELETE
requests, while some others may totally ignore it. That may lead to security
issues because a request smuggling attack is possible. Thus, by default,
HAProxy rejects HTTP/1.0 GET/HEAD/DELETE requests with a payload.
However, it may be an issue with some old clients. In this case, this global
option may be set.
h1-case-adjust <from> <to> h1-case-adjust <from> <to>
Defines the case adjustment to apply, when enabled, to the header name Defines the case adjustment to apply, when enabled, to the header name
<from>, to change it to <to> before sending it to HTTP/1 clients or <from>, to change it to <to> before sending it to HTTP/1 clients or

View File

@ -149,7 +149,7 @@ struct h1_hdr_entry {
/* Declare the headers map */ /* Declare the headers map */
static struct h1_hdrs_map hdrs_map = { .name = NULL, .map = EB_ROOT }; static struct h1_hdrs_map hdrs_map = { .name = NULL, .map = EB_ROOT };
static int accept_payload_with_any_method = 0;
/* trace source and events */ /* trace source and events */
static void h1_trace(enum trace_level level, uint64_t mask, static void h1_trace(enum trace_level level, uint64_t mask,
@ -1602,12 +1602,14 @@ static size_t h1_handle_headers(struct h1s *h1s, struct h1m *h1m, struct htx *ht
} }
/* Reject HTTP/1.0 GET/HEAD/DELETE requests with a payload. There is a /* Reject HTTP/1.0 GET/HEAD/DELETE requests with a payload except if
* payload if the c-l is not null or the the payload is chunk-encoded. * accept_payload_with_any_method global option is set.
* A parsing error is reported but a A 413-Payload-Too-Large is returned *There is a payload if the c-l is not null or the the payload is
* instead of a 400-Bad-Request. * chunk-encoded. A parsing error is reported but a A
* 413-Payload-Too-Large is returned instead of a 400-Bad-Request.
*/ */
if (!(h1m->flags & (H1_MF_RESP|H1_MF_VER_11)) && if (!accept_payload_with_any_method &&
!(h1m->flags & (H1_MF_RESP|H1_MF_VER_11)) &&
(((h1m->flags & H1_MF_CLEN) && h1m->body_len) || (h1m->flags & H1_MF_CHNK)) && (((h1m->flags & H1_MF_CLEN) && h1m->body_len) || (h1m->flags & H1_MF_CHNK)) &&
(h1sl.rq.meth == HTTP_METH_GET || h1sl.rq.meth == HTTP_METH_HEAD || h1sl.rq.meth == HTTP_METH_DELETE)) { (h1sl.rq.meth == HTTP_METH_GET || h1sl.rq.meth == HTTP_METH_HEAD || h1sl.rq.meth == HTTP_METH_DELETE)) {
h1s->flags |= H1S_F_PARSING_ERROR; h1s->flags |= H1S_F_PARSING_ERROR;
@ -4137,6 +4139,17 @@ static int cfg_h1_headers_case_adjust_postparser()
return err_code; return err_code;
} }
/* config parser for global "h1-accept-payload_=-with-any-method" */
static int cfg_parse_h1_accept_payload_with_any_method(char **args, int section_type, struct proxy *curpx,
const struct proxy *defpx, const char *file, int line,
char **err)
{
if (too_many_args(0, args, err, NULL))
return -1;
accept_payload_with_any_method = 1;
return 0;
}
/* config parser for global "h1-outgoing-header-case-adjust" */ /* config parser for global "h1-outgoing-header-case-adjust" */
static int cfg_parse_h1_header_case_adjust(char **args, int section_type, struct proxy *curpx, static int cfg_parse_h1_header_case_adjust(char **args, int section_type, struct proxy *curpx,
@ -4168,9 +4181,9 @@ static int cfg_parse_h1_headers_case_adjust_file(char **args, int section_type,
return 0; return 0;
} }
/* config keyword parsers */ /* config keyword parsers */
static struct cfg_kw_list cfg_kws = {{ }, { static struct cfg_kw_list cfg_kws = {{ }, {
{ CFG_GLOBAL, "h1-accept-payload-with-any-method", cfg_parse_h1_accept_payload_with_any_method },
{ CFG_GLOBAL, "h1-case-adjust", cfg_parse_h1_header_case_adjust }, { CFG_GLOBAL, "h1-case-adjust", cfg_parse_h1_header_case_adjust },
{ CFG_GLOBAL, "h1-case-adjust-file", cfg_parse_h1_headers_case_adjust_file }, { CFG_GLOBAL, "h1-case-adjust-file", cfg_parse_h1_headers_case_adjust_file },
{ 0, NULL, NULL }, { 0, NULL, NULL },