diff --git a/SOURCES/RHEL-18169_h1-reject-special-char-URI-path-component.patch b/SOURCES/RHEL-18169_h1-reject-special-char-URI-path-component.patch new file mode 100644 index 0000000..b3af9cd --- /dev/null +++ b/SOURCES/RHEL-18169_h1-reject-special-char-URI-path-component.patch @@ -0,0 +1,119 @@ +From e5a741f94977840c58775b38f8ed830207f7e4d0 Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Tue, 8 Aug 2023 16:17:22 +0200 +Subject: [PATCH] BUG/MINOR: h1: do not accept '#' as part of the URI component + +Seth Manesse and Paul Plasil reported that the "path" sample fetch +function incorrectly accepts '#' as part of the path component. This +can in some cases lead to misrouted requests for rules that would apply +on the suffix: + + use_backend static if { path_end .png .jpg .gif .css .js } + +Note that this behavior can be selectively configured using +"normalize-uri fragment-encode" and "normalize-uri fragment-strip". + +The problem is that while the RFC says that this '#' must never be +emitted, as often it doesn't suggest how servers should handle it. A +diminishing number of servers still do accept it and trim it silently, +while others are rejecting it, as indicated in the conversation below +with other implementers: + + https://lists.w3.org/Archives/Public/ietf-http-wg/2023JulSep/0070.html + +Looking at logs from publicly exposed servers, such requests appear at +a rate of roughly 1 per million and only come from attacks or poorly +written web crawlers incorrectly following links found on various pages. + +Thus it looks like the best solution to this problem is to simply reject +such ambiguous requests by default, and include this in the list of +controls that can be disabled using "option accept-invalid-http-request". + +We're already rejecting URIs containing any control char anyway, so we +should also reject '#'. + +In the H1 parser for the H1_MSG_RQURI state, there is an accelerated +parser for bytes 0x21..0x7e that has been tightened to 0x24..0x7e (it +should not impact perf since 0x21..0x23 are not supposed to appear in +a URI anyway). This way '#' falls through the fine-grained filter and +we can add the special case for it also conditionned by a check on the +proxy's option "accept-invalid-http-request", with no overhead for the +vast majority of valid URIs. Here this information is available through +h1m->err_pos that's set to -2 when the option is here (so we don't need +to change the API to expose the proxy). Example with a trivial GET +through netcat: + + [08/Aug/2023:16:16:52.651] frontend layer1 (#2): invalid request + backend (#-1), server (#-1), event #0, src 127.0.0.1:50812 + buffer starts at 0 (including 0 out), 16361 free, + len 23, wraps at 16336, error at position 7 + H1 connection flags 0x00000000, H1 stream flags 0x00000810 + H1 msg state MSG_RQURI(4), H1 msg flags 0x00001400 + H1 chunk len 0 bytes, H1 body len 0 bytes : + + 00000 GET /aa#bb HTTP/1.0\r\n + 00021 \r\n + +This should be progressively backported to all stable versions along with +the following patch: + + REGTESTS: http-rules: add accept-invalid-http-request for normalize-uri tests + +Similar fixes for h2 and h3 will come in followup patches. + +Thanks to Seth Manesse and Paul Plasil for reporting this problem with +detailed explanations. + +(cherry picked from commit 2eab6d354322932cfec2ed54de261e4347eca9a6) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 9bf75c8e22a8f2537f27c557854a8803087046d0) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 9facd01c9ac85fe9bcb331594b80fa08e7406552) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 832b672eee54866c7a42a1d46078cc9ae0d544d9) +Signed-off-by: Willy Tarreau +--- + src/h1.c | 15 +++++++++++---- + 1 file changed, 11 insertions(+), 4 deletions(-) + +diff --git a/src/h1.c b/src/h1.c +index eeda311b7..91d3dc47a 100644 +--- a/src/h1.c ++++ b/src/h1.c +@@ -480,13 +480,13 @@ int h1_headers_to_hdr_list(char *start, const char *stop, + case H1_MSG_RQURI: + http_msg_rquri: + #ifdef HA_UNALIGNED_LE +- /* speedup: skip bytes not between 0x21 and 0x7e inclusive */ ++ /* speedup: skip bytes not between 0x24 and 0x7e inclusive */ + while (ptr <= end - sizeof(int)) { +- int x = *(int *)ptr - 0x21212121; ++ int x = *(int *)ptr - 0x24242424; + if (x & 0x80808080) + break; + +- x -= 0x5e5e5e5e; ++ x -= 0x5b5b5b5b; + if (!(x & 0x80808080)) + break; + +@@ -498,8 +498,15 @@ int h1_headers_to_hdr_list(char *start, const char *stop, + goto http_msg_ood; + } + http_msg_rquri2: +- if (likely((unsigned char)(*ptr - 33) <= 93)) /* 33 to 126 included */ ++ if (likely((unsigned char)(*ptr - 33) <= 93)) { /* 33 to 126 included */ ++ if (*ptr == '#') { ++ if (h1m->err_pos < -1) /* PR_O2_REQBUG_OK not set */ ++ goto invalid_char; ++ if (h1m->err_pos == -1) /* PR_O2_REQBUG_OK set: just log */ ++ h1m->err_pos = ptr - start + skip; ++ } + EAT_AND_JUMP_OR_RETURN(ptr, end, http_msg_rquri2, http_msg_ood, state, H1_MSG_RQURI); ++ } + + if (likely(HTTP_IS_SPHT(*ptr))) { + sl.rq.u.len = ptr - sl.rq.u.ptr; +-- +2.43.0 + diff --git a/SOURCES/RHEL-18169_h2-pass-accept-invalid-http-request-request-parser.patch b/SOURCES/RHEL-18169_h2-pass-accept-invalid-http-request-request-parser.patch new file mode 100644 index 0000000..126e1f4 --- /dev/null +++ b/SOURCES/RHEL-18169_h2-pass-accept-invalid-http-request-request-parser.patch @@ -0,0 +1,76 @@ +From f86e994f5fb5851cd6e4f7f6b366e37765014b9f Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Tue, 8 Aug 2023 15:38:28 +0200 +Subject: [PATCH] MINOR: h2: pass accept-invalid-http-request down the request + parser + +We're adding a new argument "relaxed" to h2_make_htx_request() so that +we can control its level of acceptance of certain invalid requests at +the proxy level with "option accept-invalid-http-request". The goal +will be to add deactivable checks that are still desirable to have by +default. For now no test is subject to it. + +(cherry picked from commit d93a00861d714313faa0395ff9e2acb14b0a2fca) + [ad: backported for following fix : BUG/MINOR: h2: reject more chars + from the :path pseudo header] +Signed-off-by: Amaury Denoyelle +(cherry picked from commit b6be1a4f858eb6602490c192235114c1a163fef9) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 26fa3a285df0748fc79e73e552161268b66fb527) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 014945a1508f43e88ac4e89950fa9037e4fb0679) +Signed-off-by: Willy Tarreau +--- + include/haproxy/h2.h | 2 +- + src/h2.c | 6 +++++- + src/mux_h2.c | 3 ++- + 3 files changed, 8 insertions(+), 3 deletions(-) + +diff --git a/include/haproxy/h2.h b/include/haproxy/h2.h +index 8d2aa9511..4f872b99d 100644 +--- a/include/haproxy/h2.h ++++ b/include/haproxy/h2.h +@@ -207,7 +207,7 @@ extern struct h2_frame_definition h2_frame_definition[H2_FT_ENTRIES]; + /* various protocol processing functions */ + + int h2_parse_cont_len_header(unsigned int *msgf, struct ist *value, unsigned long long *body_len); +-int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *msgf, unsigned long long *body_len); ++int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *msgf, unsigned long long *body_len, int relaxed); + int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *msgf, unsigned long long *body_len, char *upgrade_protocol); + int h2_make_htx_trailers(struct http_hdr *list, struct htx *htx); + +diff --git a/src/h2.c b/src/h2.c +index e1554642e..94c384111 100644 +--- a/src/h2.c ++++ b/src/h2.c +@@ -399,8 +399,12 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr, + * + * The Cookie header will be reassembled at the end, and for this, the + * 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 ++ * interoperability purposes. + */ +-int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *msgf, unsigned long long *body_len) ++int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *msgf, unsigned long long *body_len, int relaxed) + { + struct ist phdr_val[H2_PHDR_NUM_ENTRIES]; + uint32_t fields; /* bit mask of H2_PHDR_FND_* */ +diff --git a/src/mux_h2.c b/src/mux_h2.c +index 0ab86534c..61fd1a4d2 100644 +--- a/src/mux_h2.c ++++ b/src/mux_h2.c +@@ -4917,7 +4917,8 @@ static int h2c_decode_headers(struct h2c *h2c, struct buffer *rxbuf, uint32_t *f + if (h2c->flags & H2_CF_IS_BACK) + outlen = h2_make_htx_response(list, htx, &msgf, body_len, upgrade_protocol); + else +- outlen = h2_make_htx_request(list, htx, &msgf, body_len); ++ outlen = h2_make_htx_request(list, htx, &msgf, body_len, ++ !!(((const struct session *)h2c->conn->owner)->fe->options2 & PR_O2_REQBUG_OK)); + + if (outlen < 0 || htx_free_space(htx) < global.tune.maxrewrite) { + /* too large headers? this is a stream error only */ +-- +2.43.0 + diff --git a/SOURCES/RHEL-18169_h2-reject-special-char-from-pseudo-path-header.patch b/SOURCES/RHEL-18169_h2-reject-special-char-from-pseudo-path-header.patch new file mode 100644 index 0000000..d5faba6 --- /dev/null +++ b/SOURCES/RHEL-18169_h2-reject-special-char-from-pseudo-path-header.patch @@ -0,0 +1,71 @@ +From af232e47e6264122bed3681210b054ff38ec8de8 Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Tue, 8 Aug 2023 15:40:49 +0200 +Subject: [PATCH] BUG/MINOR: h2: reject more chars from the :path pseudo header + +This is the h2 version of this previous fix: + + BUG/MINOR: h1: do not accept '#' as part of the URI component + +In addition to the current NUL/CR/LF, this will also reject all other +control chars, the space and '#' from the :path pseudo-header, to avoid +taking the '#' for a part of the path. It's still possible to fall back +to the previous behavior using "option accept-invalid-http-request". + +This patch modifies the request parser to change the ":path" pseudo header +validation function with a new one that rejects 0x00-0x1F (control chars), +space and '#'. This way such chars will be dropped early in the chain, and +the search for '#' doesn't incur a second pass over the header's value. + +This should be progressively backported to stable versions, along with the +following commits it relies on: + + REGTESTS: http-rules: add accept-invalid-http-request for normalize-uri tests + REORG: http: move has_forbidden_char() from h2.c to http.h + MINOR: ist: add new function ist_find_range() to find a character range + MINOR: http: add new function http_path_has_forbidden_char() + MINOR: h2: pass accept-invalid-http-request down the request parser + +(cherry picked from commit b3119d4fb4588087e2483a80b01d322683719e29) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 462a8600ce9e478573a957e046b446a7dcffd286) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 648e59e30723b8fd4e71aab02cb679f6ea7446e7) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit c8e07f2fd8b5462527f102f7145d6027c0d041da) +[wt: minor ctx adjustments] +Signed-off-by: Willy Tarreau +--- + src/h2.c | 15 +++++++++++---- + 1 file changed, 11 insertions(+), 4 deletions(-) + +diff --git a/src/h2.c b/src/h2.c +index 94c384111..e190c52b5 100644 +--- a/src/h2.c ++++ b/src/h2.c +@@ -440,11 +440,18 @@ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *ms + } + + /* RFC7540#10.3: intermediaries forwarding to HTTP/1 must take care of +- * rejecting NUL, CR and LF characters. ++ * rejecting NUL, CR and LF characters. For :path we reject all CTL ++ * chars, spaces, and '#'. + */ +- ctl = ist_find_ctl(list[idx].v); +- if (unlikely(ctl) && has_forbidden_char(list[idx].v, ctl)) +- goto fail; ++ if (phdr == H2_PHDR_IDX_PATH && !relaxed) { ++ ctl = ist_find_range(list[idx].v, 0, '#'); ++ if (unlikely(ctl) && http_path_has_forbidden_char(list[idx].v, ctl)) ++ goto fail; ++ } else { ++ ctl = ist_find_ctl(list[idx].v); ++ if (unlikely(ctl) && has_forbidden_char(list[idx].v, ctl)) ++ goto fail; ++ } + + if (phdr > 0 && phdr < H2_PHDR_NUM_ENTRIES) { + /* insert a pseudo header by its index (in phdr) and value (in value) */ +-- +2.43.0 + diff --git a/SOURCES/RHEL-18169_http-add-new-function-http_path_has_forbidden_char.patch b/SOURCES/RHEL-18169_http-add-new-function-http_path_has_forbidden_char.patch new file mode 100644 index 0000000..bb5837e --- /dev/null +++ b/SOURCES/RHEL-18169_http-add-new-function-http_path_has_forbidden_char.patch @@ -0,0 +1,59 @@ +From 0f57ac20b046b70275192651d7b6c978032e6a36 Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Tue, 8 Aug 2023 15:24:54 +0200 +Subject: [PATCH] MINOR: http: add new function http_path_has_forbidden_char() + +As its name implies, this function checks if a path component has any +forbidden headers starting at the designated location. The goal is to +seek from the result of a successful ist_find_range() for more precise +chars. Here we're focusing on 0x00-0x1F, 0x20 and 0x23 to make sure +we're not too strict at this point. + +(cherry picked from commit 30f58f4217d585efeac3d85cb1b695ba53b7760b) + [ad: backported for following fix : BUG/MINOR: h2: reject more chars + from the :path pseudo header] +Signed-off-by: Amaury Denoyelle +(cherry picked from commit b491940181a88bb6c69ab2afc24b93a50adfa67c) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit f7666e5e43ce63e804ebffdf224d92cfd3367282) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit c699bb17b7e334c9d56e829422e29e5a204615ec) +[wt: adj minor ctx in http.h] +Signed-off-by: Willy Tarreau +--- + include/haproxy/http.h | 19 +++++++++++++++++++ + 1 file changed, 19 insertions(+) + +diff --git a/include/haproxy/http.h b/include/haproxy/http.h +index 8a86cb6e9..e8c5b850f 100644 +--- a/include/haproxy/http.h ++++ b/include/haproxy/http.h +@@ -134,6 +134,25 @@ static inline enum http_etag_type http_get_etag_type(const struct ist etag) + return ETAG_INVALID; + } + ++/* Looks into for forbidden characters for :path values (0x00..0x1F, ++ * 0x20, 0x23), starting at pointer which must be within . ++ * Returns non-zero if such a character is found, 0 otherwise. When run on ++ * unlikely header match, it's recommended to first check for the presence ++ * of control chars using ist_find_ctl(). ++ */ ++static inline int http_path_has_forbidden_char(const struct ist ist, const char *start) ++{ ++ do { ++ if ((uint8_t)*start <= 0x23) { ++ if ((uint8_t)*start < 0x20) ++ return 1; ++ if ((1U << ((uint8_t)*start & 0x1F)) & ((1<<3) | (1<<0))) ++ return 1; ++ } ++ start++; ++ } while (start < istend(ist)); ++ return 0; ++} + + #endif /* _HAPROXY_HTTP_H */ + +-- +2.43.0 + diff --git a/SOURCES/RHEL-18169_ist-add-new-function-ist_find_range.patch b/SOURCES/RHEL-18169_ist-add-new-function-ist_find_range.patch new file mode 100644 index 0000000..5040292 --- /dev/null +++ b/SOURCES/RHEL-18169_ist-add-new-function-ist_find_range.patch @@ -0,0 +1,86 @@ +From edcff741698c9519dc44f3aa13de421baad7ff43 Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Tue, 8 Aug 2023 15:23:19 +0200 +Subject: [PATCH] MINOR: ist: add new function ist_find_range() to find a + character range + +This looks up the character range .. in the input string and +returns a pointer to the first one found. It's essentially the equivalent +of ist_find_ctl() in that it searches by 32 or 64 bits at once, but deals +with a range. + +(cherry picked from commit 197668de975e495f0c0f0e4ff51b96203fa9842d) + [ad: backported for following fix : BUG/MINOR: h2: reject more chars + from the :path pseudo header] +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 451ac6628acc4b9eed3260501a49c60d4e4d4e55) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 3468f7f8e04c9c5ca5c985c7511e05e78fe1eded) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit b375df60341c7f7a4904c2d8041a09c66115c754) +Signed-off-by: Willy Tarreau +--- + include/import/ist.h | 47 ++++++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 47 insertions(+) + +diff --git a/include/import/ist.h b/include/import/ist.h +index 539a27d26..31566b105 100644 +--- a/include/import/ist.h ++++ b/include/import/ist.h +@@ -746,6 +746,53 @@ static inline const char *ist_find_ctl(const struct ist ist) + return NULL; + } + ++/* Returns a pointer to the first character found that belongs to the ++ * range [min:max] inclusive, or NULL if none is present. The function is ++ * optimized for strings having no such chars by processing up to sizeof(long) ++ * bytes at once on architectures supporting efficient unaligned accesses. ++ * Despite this it is not very fast (~0.43 byte/cycle) and should mostly be ++ * used on low match probability when it can save a call to a much slower ++ * function. Will not work for characters 0x80 and above. It's optimized for ++ * min and max to be known at build time. ++ */ ++static inline const char *ist_find_range(const struct ist ist, unsigned char min, unsigned char max) ++{ ++ const union { unsigned long v; } __attribute__((packed)) *u; ++ const char *curr = (void *)ist.ptr - sizeof(long); ++ const char *last = curr + ist.len; ++ unsigned long l1, l2; ++ ++ /* easier with an exclusive boundary */ ++ max++; ++ ++ do { ++ curr += sizeof(long); ++ if (curr > last) ++ break; ++ u = (void *)curr; ++ /* add 0x.. then subtract ++ * 0x.. to the value to generate a ++ * carry in the lower byte if the byte contains a lower value. ++ * If we generate a bit 7 that was not there, it means the byte ++ * was min..max. ++ */ ++ l2 = u->v; ++ l1 = ~l2 & ((~0UL / 255) * 0x80); /* 0x808080...80 */ ++ l2 += (~0UL / 255) * min; /* 0x.. */ ++ l2 -= (~0UL / 255) * max; /* 0x.. */ ++ } while ((l1 & l2) == 0); ++ ++ last += sizeof(long); ++ if (__builtin_expect(curr < last, 0)) { ++ do { ++ if ((unsigned char)(*curr - min) < (unsigned char)(max - min)) ++ return curr; ++ curr++; ++ } while (curr < last); ++ } ++ return NULL; ++} ++ + /* looks for first occurrence of character in string and returns + * the tail of the string starting with this character, or (ist.end,0) if not + * found. +-- +2.43.0 + diff --git a/SOURCES/RHEL-18169_regtest-add-accept-invalid-http-request.patch b/SOURCES/RHEL-18169_regtest-add-accept-invalid-http-request.patch new file mode 100644 index 0000000..aae2a08 --- /dev/null +++ b/SOURCES/RHEL-18169_regtest-add-accept-invalid-http-request.patch @@ -0,0 +1,46 @@ +From c7492154ef07d6c08aa1eb52502697bbc3f42a69 Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Tue, 8 Aug 2023 19:52:45 +0200 +Subject: [PATCH] REGTESTS: http-rules: add accept-invalid-http-request for + normalize-uri tests + +We'll soon block the '#' by default so let's prepare the test to continue +to work. + +(cherry picked from commit 069d0e221e58a46119d7c049bb07fa4bcb8d0075) + [ad: backported for following fix : BUG/MINOR: h2: reject more chars + from the :path pseudo header] +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 1660481fab69856a39ac44cf88b76cdbcc0ea954) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 90d0300cea6cda18a4e20369f4dc0b4c4783d6c9) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 65849396fd6f192d9f14e81702c6c3851e580345) +Signed-off-by: Willy Tarreau +--- + reg-tests/http-rules/normalize_uri.vtc | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/reg-tests/http-rules/normalize_uri.vtc b/reg-tests/http-rules/normalize_uri.vtc +index 6a1dc31dc..56acf2cef 100644 +--- a/reg-tests/http-rules/normalize_uri.vtc ++++ b/reg-tests/http-rules/normalize_uri.vtc +@@ -127,6 +127,7 @@ haproxy h1 -conf { + + frontend fe_fragment_strip + bind "fd@${fe_fragment_strip}" ++ option accept-invalid-http-request + + http-request set-var(txn.before) url + http-request normalize-uri fragment-strip +@@ -139,6 +140,7 @@ haproxy h1 -conf { + + frontend fe_fragment_encode + bind "fd@${fe_fragment_encode}" ++ option accept-invalid-http-request + + http-request set-var(txn.before) url + http-request normalize-uri fragment-encode +-- +2.43.0 + diff --git a/SOURCES/RHEL-7736_http-reject-empty-content-length-header.patch b/SOURCES/RHEL-7736_http-reject-empty-content-length-header.patch new file mode 100644 index 0000000..e30c9f3 --- /dev/null +++ b/SOURCES/RHEL-7736_http-reject-empty-content-length-header.patch @@ -0,0 +1,275 @@ +From ba9afd2774c03e434165475b537d0462801f49bb Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Wed, 9 Aug 2023 08:32:48 +0200 +Subject: [PATCH] BUG/MAJOR: http: reject any empty content-length header value + +The content-length header parser has its dedicated function, in order +to take extreme care about invalid, unparsable, or conflicting values. +But there's a corner case in it, by which it stops comparing values +when reaching the end of the header. This has for a side effect that +an empty value or a value that ends with a comma does not deserve +further analysis, and it acts as if the header was absent. + +While this is not necessarily a problem for the value ending with a +comma as it will be cause a header folding and will disappear, it is a +problem for the first isolated empty header because this one will not +be recontructed when next ones are seen, and will be passed as-is to the +backend server. A vulnerable HTTP/1 server hosted behind haproxy that +would just use this first value as "0" and ignore the valid one would +then not be protected by haproxy and could be attacked this way, taking +the payload for an extra request. + +In field the risk depends on the server. Most commonly used servers +already have safe content-length parsers, but users relying on haproxy +to protect a known-vulnerable server might be at risk (and the risk of +a bug even in a reputable server should never be dismissed). + +A configuration-based work-around consists in adding the following rule +in the frontend, to explicitly reject requests featuring an empty +content-length header that would have not be folded into an existing +one: + + http-request deny if { hdr_len(content-length) 0 } + +The real fix consists in adjusting the parser so that it always expects a +value at the beginning of the header or after a comma. It will now reject +requests and responses having empty values anywhere in the C-L header. + +This needs to be backported to all supported versions. Note that the +modification was made to functions h1_parse_cont_len_header() and +http_parse_cont_len_header(). Prior to 2.8 the latter was in +h2_parse_cont_len_header(). One day the two should be refused but the +former is also used by Lua. + +The HTTP messaging reg-tests were completed to test these cases. + +Thanks to Ben Kallus of Dartmouth College and Narf Industries for +reporting this! (this is in GH #2237). + +(cherry picked from commit 6492f1f29d738457ea9f382aca54537f35f9d856) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit a32f99f6f991d123ea3e307bf8aa63220836d365) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit 65921ee12d88e9fb1fa9f6cd8198fd64b3a3f37f) +Signed-off-by: Amaury Denoyelle +(cherry picked from commit d17c50010d591d1c070e1cb0567a06032d8869e9) +[wt: applied to h2_parse_cont_len_header() in src/h2.c instead] +Signed-off-by: Willy Tarreau +--- + reg-tests/http-messaging/h1_to_h1.vtc | 26 ++++++++++++ + reg-tests/http-messaging/h2_to_h1.vtc | 60 +++++++++++++++++++++++++++ + src/h1.c | 20 +++++++-- + src/h2.c | 20 +++++++-- + 4 files changed, 120 insertions(+), 6 deletions(-) + +diff --git a/reg-tests/http-messaging/h1_to_h1.vtc b/reg-tests/http-messaging/h1_to_h1.vtc +index c7d00858e..603c03210 100644 +--- a/reg-tests/http-messaging/h1_to_h1.vtc ++++ b/reg-tests/http-messaging/h1_to_h1.vtc +@@ -275,3 +275,29 @@ client c3h1 -connect ${h1_feh1_sock} { + # arrive here. + expect_close + } -run ++ ++client c4h1 -connect ${h1_feh1_sock} { ++ # this request is invalid and advertises an invalid C-L ending with an ++ # empty value, which results in a stream error. ++ txreq \ ++ -req "GET" \ ++ -url "/test31.html" \ ++ -hdr "content-length: 0," \ ++ -hdr "connection: close" ++ rxresp ++ expect resp.status == 400 ++ expect_close ++} -run ++ ++client c5h1 -connect ${h1_feh1_sock} { ++ # this request is invalid and advertises an empty C-L, which results ++ # in a stream error. ++ txreq \ ++ -req "GET" \ ++ -url "/test41.html" \ ++ -hdr "content-length:" \ ++ -hdr "connection: close" ++ rxresp ++ expect resp.status == 400 ++ expect_close ++} -run +diff --git a/reg-tests/http-messaging/h2_to_h1.vtc b/reg-tests/http-messaging/h2_to_h1.vtc +index 0d2b1e5f2..ec7a7c123 100644 +--- a/reg-tests/http-messaging/h2_to_h1.vtc ++++ b/reg-tests/http-messaging/h2_to_h1.vtc +@@ -10,6 +10,8 @@ barrier b1 cond 2 -cyclic + barrier b2 cond 2 -cyclic + barrier b3 cond 2 -cyclic + barrier b4 cond 2 -cyclic ++barrier b5 cond 2 -cyclic ++barrier b6 cond 2 -cyclic + + server s1 { + rxreq +@@ -31,6 +33,12 @@ server s1 { + + barrier b4 sync + # the next request is never received ++ ++ barrier b5 sync ++ # the next request is never received ++ ++ barrier b6 sync ++ # the next request is never received + } -repeat 2 -start + + haproxy h1 -conf { +@@ -121,6 +129,32 @@ client c1h2 -connect ${h1_feh2_sock} { + txdata -data "this is sent and ignored" + rxrst + } -run ++ ++ # fifth request is invalid and advertises an invalid C-L ending with an ++ # empty value, which results in a stream error. ++ stream 9 { ++ barrier b5 sync ++ txreq \ ++ -req "GET" \ ++ -scheme "https" \ ++ -url "/test5.html" \ ++ -hdr "content-length" "0," \ ++ -nostrend ++ rxrst ++ } -run ++ ++ # sixth request is invalid and advertises an empty C-L, which results ++ # in a stream error. ++ stream 11 { ++ barrier b6 sync ++ txreq \ ++ -req "GET" \ ++ -scheme "https" \ ++ -url "/test6.html" \ ++ -hdr "content-length" "" \ ++ -nostrend ++ rxrst ++ } -run + } -run + + # HEAD requests : don't work well yet +@@ -263,4 +297,30 @@ client c3h2 -connect ${h1_feh2_sock} { + txdata -data "this is sent and ignored" + rxrst + } -run ++ ++ # fifth request is invalid and advertises invalid C-L ending with an ++ # empty value, which results in a stream error. ++ stream 9 { ++ barrier b5 sync ++ txreq \ ++ -req "POST" \ ++ -scheme "https" \ ++ -url "/test25.html" \ ++ -hdr "content-length" "0," \ ++ -nostrend ++ rxrst ++ } -run ++ ++ # sixth request is invalid and advertises an empty C-L, which results ++ # in a stream error. ++ stream 11 { ++ barrier b6 sync ++ txreq \ ++ -req "POST" \ ++ -scheme "https" \ ++ -url "/test26.html" \ ++ -hdr "content-length" "" \ ++ -nostrend ++ rxrst ++ } -run + } -run +diff --git a/src/h1.c b/src/h1.c +index 73de48be0..eeda311b7 100644 +--- a/src/h1.c ++++ b/src/h1.c +@@ -34,13 +34,20 @@ int h1_parse_cont_len_header(struct h1m *h1m, struct ist *value) + int not_first = !!(h1m->flags & H1_MF_CLEN); + struct ist word; + +- word.ptr = value->ptr - 1; // -1 for next loop's pre-increment ++ word.ptr = value->ptr; + e = value->ptr + value->len; + +- while (++word.ptr < e) { ++ while (1) { ++ if (word.ptr >= e) { ++ /* empty header or empty value */ ++ goto fail; ++ } ++ + /* skip leading delimiter and blanks */ +- if (unlikely(HTTP_IS_LWS(*word.ptr))) ++ if (unlikely(HTTP_IS_LWS(*word.ptr))) { ++ word.ptr++; + continue; ++ } + + /* digits only now */ + for (cl = 0, n = word.ptr; n < e; n++) { +@@ -79,6 +86,13 @@ int h1_parse_cont_len_header(struct h1m *h1m, struct ist *value) + h1m->flags |= H1_MF_CLEN; + h1m->curr_len = h1m->body_len = cl; + *value = word; ++ ++ /* Now either n==e and we're done, or n points to the comma, ++ * and we skip it and continue. ++ */ ++ if (n++ == e) ++ break; ++ + word.ptr = n; + } + /* here we've reached the end with a single value or a series of +diff --git a/src/h2.c b/src/h2.c +index dd1f7d9b6..e1554642e 100644 +--- a/src/h2.c ++++ b/src/h2.c +@@ -80,13 +80,20 @@ int h2_parse_cont_len_header(unsigned int *msgf, struct ist *value, unsigned lon + int not_first = !!(*msgf & H2_MSGF_BODY_CL); + struct ist word; + +- word.ptr = value->ptr - 1; // -1 for next loop's pre-increment ++ word.ptr = value->ptr; + e = value->ptr + value->len; + +- while (++word.ptr < e) { ++ while (1) { ++ if (word.ptr >= e) { ++ /* empty header or empty value */ ++ goto fail; ++ } ++ + /* skip leading delimiter and blanks */ +- if (unlikely(HTTP_IS_LWS(*word.ptr))) ++ if (unlikely(HTTP_IS_LWS(*word.ptr))) { ++ word.ptr++; + continue; ++ } + + /* digits only now */ + for (cl = 0, n = word.ptr; n < e; n++) { +@@ -125,6 +132,13 @@ int h2_parse_cont_len_header(unsigned int *msgf, struct ist *value, unsigned lon + *msgf |= H2_MSGF_BODY_CL; + *body_len = cl; + *value = word; ++ ++ /* Now either n==e and we're done, or n points to the comma, ++ * and we skip it and continue. ++ */ ++ if (n++ == e) ++ break; ++ + word.ptr = n; + } + /* here we've reached the end with a single value or a series of +-- +2.43.0 + diff --git a/SPECS/haproxy.spec b/SPECS/haproxy.spec index fca9d6b..09500d2 100644 --- a/SPECS/haproxy.spec +++ b/SPECS/haproxy.spec @@ -8,7 +8,7 @@ Name: haproxy Version: 2.4.22 -Release: 1%{?dist} +Release: 3%{?dist} Summary: HAProxy reverse proxy for high availability environments License: GPLv2+ @@ -22,6 +22,14 @@ Source4: %{name}.sysconfig Source5: %{name}.sysusers Source6: halog.1 +Patch0: RHEL-7736_http-reject-empty-content-length-header.patch +Patch1: RHEL-18169_h1-reject-special-char-URI-path-component.patch +Patch2: RHEL-18169_h2-pass-accept-invalid-http-request-request-parser.patch +Patch3: RHEL-18169_h2-reject-special-char-from-pseudo-path-header.patch +Patch4: RHEL-18169_http-add-new-function-http_path_has_forbidden_char.patch +Patch5: RHEL-18169_ist-add-new-function-ist_find_range.patch +Patch6: RHEL-18169_regtest-add-accept-invalid-http-request.patch + BuildRequires: gcc BuildRequires: lua-devel BuildRequires: pcre2-devel @@ -50,6 +58,13 @@ availability environments. Indeed, it can: %prep %setup -q +%patch -P0 -p1 +%patch -P1 -p1 +%patch -P2 -p1 +%patch -P3 -p1 +%patch -P4 -p1 +%patch -P5 -p1 +%patch -P6 -p1 %build regparm_opts= @@ -132,6 +147,12 @@ done %{_sysusersdir}/%{name}.conf %changelog +* Tue Jan 23 2024 Ryan O'Hara - 2.4.22-3 +- Reject "#" as part of URI path component (CVE-2023-45539, RHEL-18169) + +* Wed Jan 17 2024 Ryan O'Hara - 2.4.22-2 +- Reject any empty content-length header value (CVE-2023-40225, RHEL-7736) + * Tue Jun 06 2023 Ryan O'Hara - 2.4.22-1 - Update to 2.4.22 (#2196530)