rpms
/
haproxy
21
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Compare commits

merge into: rpms:c9
Branches Tags
rpms:i10c-beta
rpms:c10-beta
rpms:i8c
rpms:c8
rpms:i10cs
rpms:cs10
rpms:i9c
rpms:c9
rpms:i9c-beta
rpms:c9-beta
...
pull from: rpms:c9-beta
Branches Tags
rpms:i10c-beta
rpms:c10-beta
rpms:i8c
rpms:c8
rpms:i10cs
rpms:cs10
rpms:i9c
rpms:c9
rpms:i9c-beta
rpms:c9-beta

No commits in common. 'c9' and 'c9-beta' have entirely different histories.
c9 ... c9-beta

8 changed files with 1 additions and 754 deletions
Show Stats

119
SOURCES/RHEL-18169_h1-reject-special-char-URI-path-component.patch
Unescape View File

@ -1,119 +0,0 @@
From e5a741f94977840c58775b38f8ed830207f7e4d0 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
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 <NONE> (#-1), server <NONE> (#-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 <adenoyelle@haproxy.com>
(cherry picked from commit 9bf75c8e22a8f2537f27c557854a8803087046d0)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 9facd01c9ac85fe9bcb331594b80fa08e7406552)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 832b672eee54866c7a42a1d46078cc9ae0d544d9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
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

76
SOURCES/RHEL-18169_h2-pass-accept-invalid-http-request-request-parser.patch
Unescape View File

@ -1,76 +0,0 @@
From f86e994f5fb5851cd6e4f7f6b366e37765014b9f Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
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 <adenoyelle@haproxy.com>
(cherry picked from commit b6be1a4f858eb6602490c192235114c1a163fef9)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 26fa3a285df0748fc79e73e552161268b66fb527)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 014945a1508f43e88ac4e89950fa9037e4fb0679)
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
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 <list>
* will be used to create a linked list, so its contents may be destroyed.
+ *
+ * When <relaxed> 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

71
SOURCES/RHEL-18169_h2-reject-special-char-from-pseudo-path-header.patch
Unescape View File

@ -1,71 +0,0 @@
From af232e47e6264122bed3681210b054ff38ec8de8 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
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 <adenoyelle@haproxy.com>
(cherry picked from commit 462a8600ce9e478573a957e046b446a7dcffd286)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 648e59e30723b8fd4e71aab02cb679f6ea7446e7)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit c8e07f2fd8b5462527f102f7145d6027c0d041da)
[wt: minor ctx adjustments]
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
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

59
SOURCES/RHEL-18169_http-add-new-function-http_path_has_forbidden_char.patch
Unescape View File

@ -1,59 +0,0 @@
From 0f57ac20b046b70275192651d7b6c978032e6a36 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
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 <adenoyelle@haproxy.com>
(cherry picked from commit b491940181a88bb6c69ab2afc24b93a50adfa67c)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit f7666e5e43ce63e804ebffdf224d92cfd3367282)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit c699bb17b7e334c9d56e829422e29e5a204615ec)
[wt: adj minor ctx in http.h]
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
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 <ist> for forbidden characters for :path values (0x00..0x1F,
+ * 0x20, 0x23), starting at pointer <start> which must be within <ist>.
+ * 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

86
SOURCES/RHEL-18169_ist-add-new-function-ist_find_range.patch
Unescape View File

@ -1,86 +0,0 @@
From edcff741698c9519dc44f3aa13de421baad7ff43 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
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 <min>..<max> 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 <adenoyelle@haproxy.com>
(cherry picked from commit 451ac6628acc4b9eed3260501a49c60d4e4d4e55)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 3468f7f8e04c9c5ca5c985c7511e05e78fe1eded)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit b375df60341c7f7a4904c2d8041a09c66115c754)
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
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 <ist> 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<min><min><min><min>..<min> then subtract
+ * 0x<max><max><max><max>..<max> 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<min><min>..<min> */
+ l2 -= (~0UL / 255) * max; /* 0x<max><max>..<max> */
+ } 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 <chr> in string <ist> and returns
* the tail of the string starting with this character, or (ist.end,0) if not
* found.
--
2.43.0

46
SOURCES/RHEL-18169_regtest-add-accept-invalid-http-request.patch
Unescape View File

@ -1,46 +0,0 @@
From c7492154ef07d6c08aa1eb52502697bbc3f42a69 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
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 <adenoyelle@haproxy.com>
(cherry picked from commit 1660481fab69856a39ac44cf88b76cdbcc0ea954)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 90d0300cea6cda18a4e20369f4dc0b4c4783d6c9)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 65849396fd6f192d9f14e81702c6c3851e580345)
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
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

275
SOURCES/RHEL-7736_http-reject-empty-content-length-header.patch
Unescape View File

@ -1,275 +0,0 @@
From ba9afd2774c03e434165475b537d0462801f49bb Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
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 <adenoyelle@haproxy.com>
(cherry picked from commit a32f99f6f991d123ea3e307bf8aa63220836d365)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 65921ee12d88e9fb1fa9f6cd8198fd64b3a3f37f)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit d17c50010d591d1c070e1cb0567a06032d8869e9)
[wt: applied to h2_parse_cont_len_header() in src/h2.c instead]
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
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

23
SPECS/haproxy.spec
Unescape View File

@ -8,7 +8,7 @@
Name: haproxy
Version: 2.4.22
Release: 3%{?dist}
Release: 1%{?dist}
Summary: HAProxy reverse proxy for high availability environments
License: GPLv2+
@ -22,14 +22,6 @@ 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
@ -58,13 +50,6 @@ 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=
@ -147,12 +132,6 @@ done
%{_sysusersdir}/%{name}.conf
%changelog
* Tue Jan 23 2024 Ryan O'Hara <rohara@redhat.com> - 2.4.22-3
- Reject "#" as part of URI path component (CVE-2023-45539, RHEL-18169)
* Wed Jan 17 2024 Ryan O'Hara <rohara@redhat.com> - 2.4.22-2
- Reject any empty content-length header value (CVE-2023-40225, RHEL-7736)
* Tue Jun 06 2023 Ryan O'Hara <rohara@redhat.com> - 2.4.22-1
- Update to 2.4.22 (#2196530)

Write Preview
Loading…
Cancel
Save