!87 Upgrade to 2.6.15
From: @starlet-dx Reviewed-by: @wu-leilei Signed-off-by: @wu-leilei
This commit is contained in:
commit
c3766f3305
@ -1,40 +0,0 @@
|
||||
From 827a6299e6995c5c3ba620d8b7cbacdaef67f2c4 Mon Sep 17 00:00:00 2001
|
||||
From: Christopher Faulet <cfaulet@haproxy.com>
|
||||
Date: Thu, 22 Dec 2022 09:47:01 +0100
|
||||
Subject: [PATCH] BUG/MEDIUM: mux-h2: Refuse interim responses with end-stream
|
||||
flag set
|
||||
|
||||
As state in RFC9113#8.1, HEADERS frame with the ES flag set that carries an
|
||||
informational status code is malformed. However, there is no test on this
|
||||
condition.
|
||||
|
||||
On 2.4 and higher, it is hard to predict consequences of this bug because
|
||||
end of the message is only reported with a flag. But on 2.2 and lower, it
|
||||
leads to a crash because there is an unexpected extra EOM block at the end
|
||||
of an interim response.
|
||||
|
||||
Now, when a ES flag is detected on a HEADERS frame for an interim message, a
|
||||
stream error is sent (RST_STREAM/PROTOCOL_ERROR).
|
||||
|
||||
This patch should solve the issue #1972. It should be backported as far as
|
||||
2.0.
|
||||
---
|
||||
src/mux_h2.c | 5 +++++
|
||||
1 file changed, 5 insertions(+)
|
||||
|
||||
diff --git a/src/mux_h2.c b/src/mux_h2.c
|
||||
index ac7afcd2a6bc..22b1f1e79a65 100644
|
||||
--- a/src/mux_h2.c
|
||||
+++ b/src/mux_h2.c
|
||||
@@ -4782,6 +4782,11 @@ static int h2c_decode_headers(struct h2c *h2c, struct buffer *rxbuf, uint32_t *f
|
||||
*flags |= H2_SF_HEADERS_RCVD;
|
||||
|
||||
if (h2c->dff & H2_F_HEADERS_END_STREAM) {
|
||||
+ if (msgf & H2_MSGF_RSP_1XX) {
|
||||
+ /* RFC9113#8.1 : HEADERS frame with the ES flag set that carries an informational status code is malformed */
|
||||
+ TRACE_STATE("invalid interim response with ES flag!", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_H2C_ERR|H2_EV_PROTO_ERR, h2c->conn);
|
||||
+ goto fail;
|
||||
+ }
|
||||
/* no more data are expected for this message */
|
||||
htx->flags |= HTX_FL_EOM;
|
||||
}
|
||||
@ -1,172 +0,0 @@
|
||||
From a8598a2eb11b6c989e81f0dbf10be361782e8d32 Mon Sep 17 00:00:00 2001
|
||||
From: Willy Tarreau <w@1wt.eu>
|
||||
Date: Thu, 9 Feb 2023 21:36:54 +0100
|
||||
Subject: [PATCH] BUG/CRITICAL: http: properly reject empty http header field
|
||||
names
|
||||
|
||||
The HTTP header parsers surprizingly accepts empty header field names,
|
||||
and this is a leftover from the original code that was agnostic to this.
|
||||
|
||||
When muxes were introduced, for H2 first, the HPACK decompressor needed
|
||||
to feed headers lists, and since empty header names were strictly
|
||||
forbidden by the protocol, the lists of headers were purposely designed
|
||||
to be terminated by an empty header field name (a principle that is
|
||||
similar to H1's empty line termination). This principle was preserved
|
||||
and generalized to other protocols migrated to muxes (H1/FCGI/H3 etc)
|
||||
without anyone ever noticing that the H1 parser was still able to deliver
|
||||
empty header field names to this list. In addition to this it turns out
|
||||
that the HPACK decompressor, despite a comment in the code, may
|
||||
successfully decompress an empty header field name, and this mistake
|
||||
was propagated to the QPACK decompressor as well.
|
||||
|
||||
The impact is that an empty header field name may be used to truncate
|
||||
the list of headers and thus make some headers disappear. While for
|
||||
H2/H3 the impact is limited as haproxy sees a request with missing
|
||||
headers, and headers are not used to delimit messages, in the case of
|
||||
HTTP/1, the impact is significant because the presence (and sometimes
|
||||
contents) of certain sensitive headers is detected during the parsing.
|
||||
Thus, some of these headers may be seen, marked as present, their value
|
||||
extracted, but never delivered to upper layers and obviously not
|
||||
forwarded to the other side either. This can have for consequence that
|
||||
certain important header fields such as Connection, Upgrade, Host,
|
||||
Content-length, Transfer-Encoding etc are possibly seen as different
|
||||
between what haproxy uses to parse/forward/route and what is observed
|
||||
in http-request rules and of course, forwarded. One direct consequence
|
||||
is that it is possible to exploit this property in HTTP/1 to make
|
||||
affected versions of haproxy forward more data than is advertised on
|
||||
the other side, and bypass some access controls or routing rules by
|
||||
crafting extraneous requests. Note, however, that responses to such
|
||||
requests will normally not be passed back to the client, but this can
|
||||
still cause some harm.
|
||||
|
||||
This specific risk can be mostly worked around in configuration using
|
||||
the following rule that will rely on the bug's impact to precisely
|
||||
detect the inconsistency between the known body size and the one
|
||||
expected to be advertised to the server (the rule works from 2.0 to
|
||||
2.8-dev):
|
||||
|
||||
http-request deny if { fc_http_major 1 } !{ req.body_size 0 } !{ req.hdr(content-length) -m found } !{ req.hdr(transfer-encoding) -m found } !{ method CONNECT }
|
||||
|
||||
This will exclusively block such carefully crafted requests delivered
|
||||
over HTTP/1. HTTP/2 and HTTP/3 do not need content-length, and a body
|
||||
that arrives without being announced with a content-length will be
|
||||
forwarded using transfer-encoding, hence will not cause discrepancies.
|
||||
In HAProxy 2.0 in legacy mode ("no option http-use-htx"), this rule will
|
||||
simply have no effect but will not cause trouble either.
|
||||
|
||||
A clean solution would consist in modifying the loops iterating over
|
||||
these headers lists to check the header name's pointer instead of its
|
||||
length (since both are zero at the end of the list), but this requires
|
||||
to touch tens of places and it's very easy to miss one. Functions such
|
||||
as htx_add_header(), htx_add_trailer(), htx_add_all_headers() would be
|
||||
good starting points for such a possible future change.
|
||||
|
||||
Instead the current fix focuses on blocking empty headers where they
|
||||
are first inserted, hence in the H1/HPACK/QPACK decoders. One benefit
|
||||
of the current solution (for H1) is that it allows "show errors" to
|
||||
report a precise diagnostic when facing such invalid HTTP/1 requests,
|
||||
with the exact location of the problem and the originating address:
|
||||
|
||||
$ printf "GET / HTTP/1.1\r\nHost: localhost\r\n:empty header\r\n\r\n" | nc 0 8001
|
||||
HTTP/1.1 400 Bad request
|
||||
Content-length: 90
|
||||
Cache-Control: no-cache
|
||||
Connection: close
|
||||
Content-Type: text/html
|
||||
|
||||
<html><body><h1>400 Bad request</h1>
|
||||
Your browser sent an invalid request.
|
||||
</body></html>
|
||||
|
||||
$ socat /var/run/haproxy.stat <<< "show errors"
|
||||
Total events captured on [10/Feb/2023:16:29:37.530] : 1
|
||||
|
||||
[10/Feb/2023:16:29:34.155] frontend decrypt (#2): invalid request
|
||||
backend <NONE> (#-1), server <NONE> (#-1), event #0, src 127.0.0.1:31092
|
||||
buffer starts at 0 (including 0 out), 16334 free,
|
||||
len 50, wraps at 16336, error at position 33
|
||||
H1 connection flags 0x00000000, H1 stream flags 0x00000810
|
||||
H1 msg state MSG_HDR_NAME(17), H1 msg flags 0x00001410
|
||||
H1 chunk len 0 bytes, H1 body len 0 bytes :
|
||||
|
||||
00000 GET / HTTP/1.1\r\n
|
||||
00016 Host: localhost\r\n
|
||||
00033 :empty header\r\n
|
||||
00048 \r\n
|
||||
|
||||
I want to address sincere and warm thanks for their great work to the
|
||||
team composed of the following security researchers who found the issue
|
||||
together and reported it: Bahruz Jabiyev, Anthony Gavazzi, and Engin
|
||||
Kirda from Northeastern University, Kaan Onarlioglu from Akamai
|
||||
Technologies, Adi Peleg and Harvey Tuch from Google. And kudos to Amaury
|
||||
Denoyelle from HAProxy Technologies for spotting that the HPACK and
|
||||
QPACK decoders would let this pass despite the comment explicitly
|
||||
saying otherwise.
|
||||
|
||||
This fix must be backported as far as 2.0. The QPACK changes can be
|
||||
dropped before 2.6. In 2.0 there is also the equivalent code for legacy
|
||||
mode, which doesn't suffer from the list truncation, but it would better
|
||||
be fixed regardless.
|
||||
|
||||
CVE-2023-25725 was assigned to this issue.
|
||||
---
|
||||
src/h1.c | 4 ++++
|
||||
src/hpack-dec.c | 9 +++++++++
|
||||
src/qpack-dec.c | 9 +++++++++
|
||||
3 files changed, 22 insertions(+)
|
||||
|
||||
diff --git a/src/h1.c b/src/h1.c
|
||||
index 3330a5fcb68b..88a54c4a593d 100644
|
||||
--- a/src/h1.c
|
||||
+++ b/src/h1.c
|
||||
@@ -834,6 +834,10 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
|
||||
|
||||
if (likely(*ptr == ':')) {
|
||||
col = ptr - start;
|
||||
+ if (col <= sol) {
|
||||
+ state = H1_MSG_HDR_NAME;
|
||||
+ goto http_msg_invalid;
|
||||
+ }
|
||||
EAT_AND_JUMP_OR_RETURN(ptr, end, http_msg_hdr_l1_sp, http_msg_ood, state, H1_MSG_HDR_L1_SP);
|
||||
}
|
||||
|
||||
diff --git a/src/hpack-dec.c b/src/hpack-dec.c
|
||||
index 147021cc36e9..052a7c3da8f6 100644
|
||||
--- a/src/hpack-dec.c
|
||||
+++ b/src/hpack-dec.c
|
||||
@@ -420,6 +420,15 @@ int hpack_decode_frame(struct hpack_dht *dht, const uint8_t *raw, uint32_t len,
|
||||
/* <name> and <value> are correctly filled here */
|
||||
}
|
||||
|
||||
+ /* We must not accept empty header names (forbidden by the spec and used
|
||||
+ * as a list termination).
|
||||
+ */
|
||||
+ if (!name.len) {
|
||||
+ hpack_debug_printf("##ERR@%d##\n", __LINE__);
|
||||
+ ret = -HPACK_ERR_INVALID_ARGUMENT;
|
||||
+ goto leave;
|
||||
+ }
|
||||
+
|
||||
/* here's what we have here :
|
||||
* - name.len > 0
|
||||
* - value is filled with either const data or data allocated from tmp
|
||||
diff --git a/src/qpack-dec.c b/src/qpack-dec.c
|
||||
index 0da6cf89a727..2d811564538a 100644
|
||||
--- a/src/qpack-dec.c
|
||||
+++ b/src/qpack-dec.c
|
||||
@@ -531,6 +531,15 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
|
||||
len -= value_len;
|
||||
}
|
||||
|
||||
+ /* We must not accept empty header names (forbidden by the spec and used
|
||||
+ * as a list termination).
|
||||
+ */
|
||||
+ if (!name.len) {
|
||||
+ qpack_debug_printf(stderr, "##ERR@%d\n", __LINE__);
|
||||
+ ret = -QPACK_DECOMPRESSION_FAILED;
|
||||
+ goto out;
|
||||
+ }
|
||||
+
|
||||
list[hdr_idx].n = name;
|
||||
list[hdr_idx].v = value;
|
||||
++hdr_idx;
|
||||
@ -1,75 +0,0 @@
|
||||
From 22b44d5f2c7ce1ed0e4b62c639991d5abbd42a50 Mon Sep 17 00:00:00 2001
|
||||
From: Amaury Denoyelle <adenoyelle@haproxy.com>
|
||||
Date: Wed, 7 Dec 2022 14:31:42 +0100
|
||||
Subject: [PATCH] BUG/MEDIUM: h3: reject request with invalid header name
|
||||
|
||||
Reject request containing invalid header name. This concerns every
|
||||
header containing uppercase letter or a non HTTP token such as a space.
|
||||
|
||||
For the moment, this kind of errors triggers a connection close. In the
|
||||
future, it should be handled only with a stream reset. To reduce
|
||||
backport surface, this will be implemented in another commit.
|
||||
|
||||
Thanks to Yuki Mogi from FFRI Security, Inc. for having reported this.
|
||||
|
||||
This must be backported up to 2.6.
|
||||
|
||||
(cherry picked from commit d6fb7a0e0f3a79afa1f4b6fc7b62053c3955dc4a)
|
||||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
||||
(cherry picked from commit 3ca4223c5e1f18a19dc93b0b09ffdbd295554d46)
|
||||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
||||
---
|
||||
src/h3.c | 30 +++++++++++++++++++++++++++++-
|
||||
1 file changed, 29 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/h3.c b/src/h3.c
|
||||
index 97e821e..5f1c68a 100644
|
||||
--- a/src/h3.c
|
||||
+++ b/src/h3.c
|
||||
@@ -352,7 +352,27 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
|
||||
//struct ist scheme = IST_NULL, authority = IST_NULL;
|
||||
struct ist authority = IST_NULL;
|
||||
int hdr_idx, ret;
|
||||
- int cookie = -1, last_cookie = -1;
|
||||
+ int cookie = -1, last_cookie = -1, i;
|
||||
+
|
||||
+ /* RFC 9114 4.1.2. Malformed Requests and Responses
|
||||
+ *
|
||||
+ * A malformed request or response is one that is an otherwise valid
|
||||
+ * sequence of frames but is invalid due to:
|
||||
+ * - the presence of prohibited fields or pseudo-header fields,
|
||||
+ * - the absence of mandatory pseudo-header fields,
|
||||
+ * - invalid values for pseudo-header fields,
|
||||
+ * - pseudo-header fields after fields,
|
||||
+ * - an invalid sequence of HTTP messages,
|
||||
+ * - the inclusion of uppercase field names, or
|
||||
+ * - the inclusion of invalid characters in field names or values.
|
||||
+ *
|
||||
+ * [...]
|
||||
+ *
|
||||
+ * Intermediaries that process HTTP requests or responses (i.e., any
|
||||
+ * intermediary not acting as a tunnel) MUST NOT forward a malformed
|
||||
+ * request or response. Malformed requests or responses that are
|
||||
+ * detected MUST be treated as a stream error of type H3_MESSAGE_ERROR.
|
||||
+ */
|
||||
|
||||
TRACE_ENTER(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
|
||||
|
||||
@@ -416,6 +436,14 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
|
||||
if (isteq(list[hdr_idx].n, ist("")))
|
||||
break;
|
||||
|
||||
+ for (i = 0; i < list[hdr_idx].n.len; ++i) {
|
||||
+ const char c = list[hdr_idx].n.ptr[i];
|
||||
+ if ((uint8_t)(c - 'A') < 'Z' - 'A' || !HTTP_IS_TOKEN(c)) {
|
||||
+ TRACE_ERROR("invalid characters in field name", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
|
||||
+ return -1;
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
if (isteq(list[hdr_idx].n, ist("cookie"))) {
|
||||
http_cookie_register(list, hdr_idx, &cookie, &last_cookie);
|
||||
continue;
|
||||
--
|
||||
1.7.10.4
|
||||
|
||||
@ -1,274 +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
|
||||
|
||||
Origin: https://github.com/FireBurn/haproxy/commit/ba9afd2774c03e434165475b537d0462801f49bb
|
||||
|
||||
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 c7d00858ea45..603c0321027e 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 0d2b1e5f2251..ec7a7c123163 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 73de48be0105..eeda311b70c1 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 dd1f7d9b674a..e1554642e6fa 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
|
||||
@ -1,111 +0,0 @@
|
||||
From d7be206d3570138cfadca87bb768293804629bc7 Mon Sep 17 00:00:00 2001
|
||||
From: Christopher Faulet <cfaulet@haproxy.com>
|
||||
Date: Tue, 28 Feb 2023 15:39:38 +0100
|
||||
Subject: [PATCH] BUG/MEDIUM: connection: Clear flags when a conn is removed
|
||||
from an idle list
|
||||
|
||||
When a connection is removed from the safe list or the idle list,
|
||||
CO_FL_SAFE_LIST and CO_FL_IDLE_LIST flags must be cleared. It is performed
|
||||
when the connection is reused. But not when it is moved into the
|
||||
toremove_conns list. It may be an issue because the multiplexer owning the
|
||||
connection may be woken up before the connection is really removed. If the
|
||||
connection flags are not sanitized, it may think the connection is idle and
|
||||
reinsert it in the corresponding list. From this point, we can imagine
|
||||
several bugs. An UAF or a connection reused with an invalid state for
|
||||
instance.
|
||||
|
||||
To avoid any issue, the connection flags are sanitized when an idle
|
||||
connection is moved into the toremove_conns list. The same is performed at
|
||||
right places in the multiplexers. Especially because the connection release
|
||||
may be delayed (for h2 and fcgi connections).
|
||||
|
||||
This patch shoudld fix the issue #2057. It must carefully be backported as
|
||||
far as 2.2. Especially on the 2.2 where the code is really different. But
|
||||
some conflicts should be expected on the 2.4 too.
|
||||
|
||||
(cherry picked from commit 5e1b0e7bf86a300def07388df0ea7f4b3f9e68b9)
|
||||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
||||
(cherry picked from commit 7902ebadb1ffbe0237ce974b950ca595894f3774)
|
||||
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||
---
|
||||
src/mux_fcgi.c | 4 +++-
|
||||
src/mux_h1.c | 4 +++-
|
||||
src/mux_h2.c | 7 ++++++-
|
||||
src/server.c | 2 ++
|
||||
4 files changed, 14 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
|
||||
index 31bc3ad..4981f6b 100644
|
||||
--- a/src/mux_fcgi.c
|
||||
+++ b/src/mux_fcgi.c
|
||||
@@ -3227,8 +3227,10 @@ struct task *fcgi_timeout_task(struct task *t, void *context, unsigned int state
|
||||
/* We're about to destroy the connection, so make sure nobody attempts
|
||||
* to steal it from us.
|
||||
*/
|
||||
- if (fconn->conn->flags & CO_FL_LIST_MASK)
|
||||
+ if (fconn->conn->flags & CO_FL_LIST_MASK) {
|
||||
conn_delete_from_tree(&fconn->conn->hash_node->node);
|
||||
+ fconn->conn->flags &= ~CO_FL_LIST_MASK;
|
||||
+ }
|
||||
|
||||
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
}
|
||||
diff --git a/src/mux_h1.c b/src/mux_h1.c
|
||||
index 754b572..56b08a7 100644
|
||||
--- a/src/mux_h1.c
|
||||
+++ b/src/mux_h1.c
|
||||
@@ -3282,8 +3282,10 @@ struct task *h1_timeout_task(struct task *t, void *context, unsigned int state)
|
||||
/* We're about to destroy the connection, so make sure nobody attempts
|
||||
* to steal it from us.
|
||||
*/
|
||||
- if (h1c->conn->flags & CO_FL_LIST_MASK)
|
||||
+ if (h1c->conn->flags & CO_FL_LIST_MASK) {
|
||||
conn_delete_from_tree(&h1c->conn->hash_node->node);
|
||||
+ h1c->conn->flags &= ~CO_FL_LIST_MASK;
|
||||
+ }
|
||||
|
||||
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
}
|
||||
diff --git a/src/mux_h2.c b/src/mux_h2.c
|
||||
index 94017e9..f4cb5b1 100644
|
||||
--- a/src/mux_h2.c
|
||||
+++ b/src/mux_h2.c
|
||||
@@ -4163,6 +4163,7 @@ static int h2_process(struct h2c *h2c)
|
||||
if (conn->flags & CO_FL_LIST_MASK) {
|
||||
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
conn_delete_from_tree(&conn->hash_node->node);
|
||||
+ conn->flags &= ~CO_FL_LIST_MASK;
|
||||
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
}
|
||||
}
|
||||
@@ -4305,6 +4309,7 @@ do_leave:
|
||||
if (h2c->conn->flags & CO_FL_LIST_MASK) {
|
||||
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
conn_delete_from_tree(&h2c->conn->hash_node->node);
|
||||
+ h2c->conn->flags &= ~CO_FL_LIST_MASK;
|
||||
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
}
|
||||
|
||||
diff --git a/src/server.c b/src/server.c
|
||||
index cccf2f5..8a282bc 100644
|
||||
--- a/src/server.c
|
||||
+++ b/src/server.c
|
||||
@@ -5717,6 +5717,7 @@ static int srv_migrate_conns_to_remove(struct eb_root *idle_tree, struct mt_list
|
||||
|
||||
hash_node = ebmb_entry(node, struct conn_hash_node, node);
|
||||
eb_delete(node);
|
||||
+ hash_node->conn->flags &= ~CO_FL_LIST_MASK;
|
||||
MT_LIST_APPEND(toremove_list, &hash_node->conn->toremove_list);
|
||||
i++;
|
||||
|
||||
@@ -5774,6 +5775,7 @@ void srv_release_conn(struct server *srv, struct connection *conn)
|
||||
/* Remove the connection from any tree (safe, idle or available) */
|
||||
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
conn_delete_from_tree(&conn->hash_node->node);
|
||||
+ conn->flags &= ~CO_FL_LIST_MASK;
|
||||
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
}
|
||||
|
||||
--
|
||||
1.7.10.4
|
||||
|
||||
@ -1,189 +0,0 @@
|
||||
From a53fdaf7203e45f67c44d7e250cec36875ea8e01 Mon Sep 17 00:00:00 2001
|
||||
From: Christopher Faulet <cfaulet@haproxy.com>
|
||||
Date: Thu, 16 Mar 2023 11:43:05 +0100
|
||||
Subject: [PATCH] BUG/MEDIUM: connection: Preserve flags when a conn is
|
||||
removed from an idle list
|
||||
|
||||
The commit 5e1b0e7bf ("BUG/MEDIUM: connection: Clear flags when a conn is
|
||||
removed from an idle list") introduced a regression. CO_FL_SAFE_LIST and
|
||||
CO_FL_IDLE_LIST flags are used when the connection is released to properly
|
||||
decrement used/idle connection counters. if a connection is idle, these
|
||||
flags must be preserved till the connection is really released. It may be
|
||||
removed from the list but not immediately released. If these flags are lost
|
||||
when it is finally released, the current number of used connections is
|
||||
erroneously decremented. If means this counter may become negative and the
|
||||
counters tracking the number of idle connecitons is not decremented,
|
||||
suggesting a leak.
|
||||
|
||||
So, the above commit is reverted and instead we improve a bit the way to
|
||||
detect an idle connection. The function conn_get_idle_flag() must now be
|
||||
used to know if a connection is in an idle list. It returns the connection
|
||||
flag corresponding to the idle list if the connection is idle
|
||||
(CO_FL_SAFE_LIST or CO_FL_IDLE_LIST) or 0 otherwise. But if the connection
|
||||
is scheduled to be removed, 0 is also returned, regardless the connection
|
||||
flags.
|
||||
|
||||
This new function is used when the connection is temporarily removed from
|
||||
the list to be used, mainly in muxes.
|
||||
|
||||
This patch should fix #2078 and #2057. It must be backported as far as 2.2.
|
||||
|
||||
(cherry picked from commit 3a7b539b124bccaa57478e0a5a6d66338594615a)
|
||||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
||||
(cherry picked from commit a81a1e2aea0793aa624565a14cb7579b907f116a)
|
||||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
||||
---
|
||||
include/haproxy/connection.h | 10 ++++++++++
|
||||
src/connection.c | 2 +-
|
||||
src/mux_fcgi.c | 6 ++----
|
||||
src/mux_h1.c | 6 ++----
|
||||
src/mux_h2.c | 10 ++--------
|
||||
src/server.c | 1 -
|
||||
src/ssl_sock.c | 2 +-
|
||||
7 files changed, 18 insertions(+), 19 deletions(-)
|
||||
|
||||
diff --git a/include/haproxy/connection.h b/include/haproxy/connection.h
|
||||
index 4d289e7..8cf22ef 100644
|
||||
--- a/include/haproxy/connection.h
|
||||
+++ b/include/haproxy/connection.h
|
||||
@@ -316,6 +316,16 @@ static inline void conn_set_private(struct connection *conn)
|
||||
}
|
||||
}
|
||||
|
||||
+/* Used to know if a connection is in an idle list. It returns connection flag
|
||||
+ * corresponding to the idle list if the connection is idle (CO_FL_SAFE_LIST or
|
||||
+ * CO_FL_IDLE_LIST) or 0 otherwise. Note that if the connection is scheduled to
|
||||
+ * be removed, 0 is returned, regardless the connection flags.
|
||||
+ */
|
||||
+static inline unsigned int conn_get_idle_flag(const struct connection *conn)
|
||||
+{
|
||||
+ return (!MT_LIST_INLIST(&conn->toremove_list) ? conn->flags & CO_FL_LIST_MASK : 0);
|
||||
+}
|
||||
+
|
||||
static inline void conn_force_unsubscribe(struct connection *conn)
|
||||
{
|
||||
if (!conn->subs)
|
||||
diff --git a/src/connection.c b/src/connection.c
|
||||
index 4a73dbc..5a459fd 100644
|
||||
--- a/src/connection.c
|
||||
+++ b/src/connection.c
|
||||
@@ -146,7 +146,7 @@ int conn_notify_mux(struct connection *conn, int old_flags, int forced_wake)
|
||||
((conn->flags ^ old_flags) & CO_FL_NOTIFY_DONE) ||
|
||||
((old_flags & CO_FL_WAIT_XPRT) && !(conn->flags & CO_FL_WAIT_XPRT))) &&
|
||||
conn->mux && conn->mux->wake) {
|
||||
- uint conn_in_list = conn->flags & CO_FL_LIST_MASK;
|
||||
+ uint conn_in_list = conn_get_idle_flag(conn);
|
||||
struct server *srv = objt_server(conn->target);
|
||||
|
||||
if (conn_in_list) {
|
||||
diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
|
||||
index 4981f6b..2c417dd 100644
|
||||
--- a/src/mux_fcgi.c
|
||||
+++ b/src/mux_fcgi.c
|
||||
@@ -3043,7 +3043,7 @@ struct task *fcgi_io_cb(struct task *t, void *ctx, unsigned int state)
|
||||
conn = fconn->conn;
|
||||
TRACE_POINT(FCGI_EV_FCONN_WAKE, conn);
|
||||
|
||||
- conn_in_list = conn->flags & CO_FL_LIST_MASK;
|
||||
+ conn_in_list = conn_get_idle_flag(conn);
|
||||
if (conn_in_list)
|
||||
conn_delete_from_tree(&conn->hash_node->node);
|
||||
|
||||
@@ -3227,10 +3227,8 @@ struct task *fcgi_timeout_task(struct task *t, void *context, unsigned int state
|
||||
/* We're about to destroy the connection, so make sure nobody attempts
|
||||
* to steal it from us.
|
||||
*/
|
||||
- if (fconn->conn->flags & CO_FL_LIST_MASK) {
|
||||
+ if (fconn->conn->flags & CO_FL_LIST_MASK)
|
||||
conn_delete_from_tree(&fconn->conn->hash_node->node);
|
||||
- fconn->conn->flags &= ~CO_FL_LIST_MASK;
|
||||
- }
|
||||
|
||||
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
}
|
||||
diff --git a/src/mux_h1.c b/src/mux_h1.c
|
||||
index 56b08a7..6f59b31 100644
|
||||
--- a/src/mux_h1.c
|
||||
+++ b/src/mux_h1.c
|
||||
@@ -3158,7 +3158,7 @@ struct task *h1_io_cb(struct task *t, void *ctx, unsigned int state)
|
||||
/* Remove the connection from the list, to be sure nobody attempts
|
||||
* to use it while we handle the I/O events
|
||||
*/
|
||||
- conn_in_list = conn->flags & CO_FL_LIST_MASK;
|
||||
+ conn_in_list = conn_get_idle_flag(conn);
|
||||
if (conn_in_list)
|
||||
conn_delete_from_tree(&conn->hash_node->node);
|
||||
|
||||
@@ -3282,10 +3282,8 @@ struct task *h1_timeout_task(struct task *t, void *context, unsigned int state)
|
||||
/* We're about to destroy the connection, so make sure nobody attempts
|
||||
* to steal it from us.
|
||||
*/
|
||||
- if (h1c->conn->flags & CO_FL_LIST_MASK) {
|
||||
+ if (h1c->conn->flags & CO_FL_LIST_MASK)
|
||||
conn_delete_from_tree(&h1c->conn->hash_node->node);
|
||||
- h1c->conn->flags &= ~CO_FL_LIST_MASK;
|
||||
- }
|
||||
|
||||
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
}
|
||||
diff --git a/src/mux_h2.c b/src/mux_h2.c
|
||||
index f4cb5b1..b62a8f6 100644
|
||||
--- a/src/mux_h2.c
|
||||
+++ b/src/mux_h2.c
|
||||
@@ -4027,11 +4027,10 @@ struct task *h2_io_cb(struct task *t, void *ctx, unsigned int state)
|
||||
conn = h2c->conn;
|
||||
TRACE_ENTER(H2_EV_H2C_WAKE, conn);
|
||||
|
||||
- conn_in_list = conn->flags & CO_FL_LIST_MASK;
|
||||
-
|
||||
/* Remove the connection from the list, to be sure nobody attempts
|
||||
* to use it while we handle the I/O events
|
||||
*/
|
||||
+ conn_in_list = conn_get_idle_flag(conn);
|
||||
if (conn_in_list)
|
||||
conn_delete_from_tree(&conn->hash_node->node);
|
||||
|
||||
@@ -4163,7 +4162,6 @@ static int h2_process(struct h2c *h2c)
|
||||
if (conn->flags & CO_FL_LIST_MASK) {
|
||||
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
conn_delete_from_tree(&conn->hash_node->node);
|
||||
- conn->flags &= ~CO_FL_LIST_MASK;
|
||||
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
}
|
||||
}
|
||||
@@ -4309,7 +4304,6 @@ do_leave:
|
||||
if (h2c->conn->flags & CO_FL_LIST_MASK) {
|
||||
HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
conn_delete_from_tree(&h2c->conn->hash_node->node);
|
||||
- h2c->conn->flags &= ~CO_FL_LIST_MASK;
|
||||
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
}
|
||||
|
||||
diff --git a/src/server.c b/src/server.c
|
||||
index 8a282bc..d701eae 100644
|
||||
--- a/src/server.c
|
||||
+++ b/src/server.c
|
||||
@@ -5717,7 +5717,6 @@ static int srv_migrate_conns_to_remove(struct eb_root *idle_tree, struct mt_list
|
||||
|
||||
hash_node = ebmb_entry(node, struct conn_hash_node, node);
|
||||
eb_delete(node);
|
||||
- hash_node->conn->flags &= ~CO_FL_LIST_MASK;
|
||||
MT_LIST_APPEND(toremove_list, &hash_node->conn->toremove_list);
|
||||
i++;
|
||||
|
||||
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
|
||||
index 919a08a..b2f9374 100644
|
||||
--- a/src/ssl_sock.c
|
||||
+++ b/src/ssl_sock.c
|
||||
@@ -6481,7 +6481,7 @@ struct task *ssl_sock_io_cb(struct task *t, void *context, unsigned int state)
|
||||
return NULL;
|
||||
}
|
||||
conn = ctx->conn;
|
||||
- conn_in_list = conn->flags & CO_FL_LIST_MASK;
|
||||
+ conn_in_list = conn_get_idle_flag(conn);
|
||||
if (conn_in_list)
|
||||
conn_delete_from_tree(&conn->hash_node->node);
|
||||
HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
|
||||
--
|
||||
1.7.10.4
|
||||
|
||||
@ -1,37 +0,0 @@
|
||||
From 3311c72eabc15fc3824cf09537785ccbb1c3b88f Mon Sep 17 00:00:00 2001
|
||||
From: Willy Tarreau <w@1wt.eu>
|
||||
Date: Mon, 20 Mar 2023 19:11:08 +0100
|
||||
Subject: [PATCH] BUG/MEDIUM: stream: do not try to free a failed stream-conn
|
||||
|
||||
In stream_free() if we fail to allocate s->scb() we go to the path where
|
||||
we try to free it, and it doesn't like being called with a null at all.
|
||||
It's easily reproducible with -dMfail,no-cache and "tune.fail-alloc 10"
|
||||
in the global section.
|
||||
|
||||
This must be backported to 2.6.
|
||||
|
||||
(cherry picked from commit a45e7e81ec54fd0562d8ab4776b4c05584d6d180)
|
||||
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
|
||||
(cherry picked from commit 8daac8191313b625efeaf2799a4c05b585a44474)
|
||||
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
|
||||
---
|
||||
src/stream.c | 2 +-
|
||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/stream.c b/src/stream.c
|
||||
index 7eaf039..224b9b8 100644
|
||||
--- a/src/stream.c
|
||||
+++ b/src/stream.c
|
||||
@@ -575,8 +575,8 @@ struct stream *stream_new(struct session *sess, struct stconn *sc, struct buffer
|
||||
out_fail_accept:
|
||||
flt_stream_release(s, 0);
|
||||
LIST_DELETE(&s->list);
|
||||
- out_fail_alloc_scb:
|
||||
sc_free(s->scb);
|
||||
+ out_fail_alloc_scb:
|
||||
out_fail_attach_scf:
|
||||
task_destroy(t);
|
||||
out_fail_alloc:
|
||||
--
|
||||
1.7.10.4
|
||||
|
||||
@ -1,85 +0,0 @@
|
||||
From 61882cc68c8336016f158f74c0944e1b047c6f5f Mon Sep 17 00:00:00 2001
|
||||
From: Aurelien DARRAGON <adarragon@haproxy.com>
|
||||
Date: Fri, 18 Nov 2022 09:17:29 +0100
|
||||
Subject: [PATCH] BUG/MINOR: http_ana/txn: don't re-initialize txn and req var
|
||||
lists
|
||||
|
||||
In http_create_txn(): vars_init_head() was performed on both s->vars_txn
|
||||
and s->var_reqres lists.
|
||||
|
||||
But this is wrong, these two lists are already initialized upon stream
|
||||
creation in stream_new().
|
||||
Moreover, between stream_new() and http_create_txn(), some variable may
|
||||
be defined (e.g.: by the frontend), resulting in lists not being empty.
|
||||
|
||||
Because of this "extra" list initialization, already defined variables
|
||||
can be lost.
|
||||
This causes txn dependant code not being able to access previously defined
|
||||
variables as well as memory leak because http_destroy_txn() relies on these
|
||||
lists to perform the purge.
|
||||
|
||||
This proved to be the case when a frontend sets variables and lua sample
|
||||
fetch is used in backend section as described in GH #1935.
|
||||
Many thanks to Darragh O'Toole for his detailed report.
|
||||
|
||||
Removing extra var_init_head (x2) in http_create_txn() to fix the issue.
|
||||
Adding somme comments in the code in an attempt to prevent future misuses
|
||||
of s->var_reqres, and s->var_txn lists.
|
||||
|
||||
It should be backported in every stable version.
|
||||
(This is an old bug that seems to exist since 1.6-dev6)
|
||||
|
||||
[cf: On 2.0 and 1.8, for the legacy HTTP code, vars_init() are used during
|
||||
the TXN cleanup, when the stream is reused. So, these calls must be
|
||||
moved from http_init_txn() to http_reset_txn() and not removed.]
|
||||
|
||||
(cherry picked from commit 5ad2b642625b89cdf4f5fd26a598fc480abdc806)
|
||||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
||||
---
|
||||
src/http_ana.c | 6 ++++--
|
||||
src/stream.c | 11 ++++++++++-
|
||||
2 files changed, 14 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/src/http_ana.c b/src/http_ana.c
|
||||
index 2b2cfdc..379b480 100644
|
||||
--- a/src/http_ana.c
|
||||
+++ b/src/http_ana.c
|
||||
@@ -5224,8 +5224,10 @@ struct http_txn *http_create_txn(struct stream *s)
|
||||
|
||||
txn->auth.method = HTTP_AUTH_UNKNOWN;
|
||||
|
||||
- vars_init_head(&s->vars_txn, SCOPE_TXN);
|
||||
- vars_init_head(&s->vars_reqres, SCOPE_REQ);
|
||||
+ /* here we don't want to re-initialize s->vars_txn and s->vars_reqres
|
||||
+ * variable lists, because they were already initialized upon stream
|
||||
+ * creation in stream_new(), and thus may already contain some variables
|
||||
+ */
|
||||
|
||||
return txn;
|
||||
}
|
||||
diff --git a/src/stream.c b/src/stream.c
|
||||
index c04dd56..7eaf039 100644
|
||||
--- a/src/stream.c
|
||||
+++ b/src/stream.c
|
||||
@@ -435,8 +435,17 @@ struct stream *stream_new(struct session *sess, struct stconn *sc, struct buffer
|
||||
s->req_cap = NULL;
|
||||
s->res_cap = NULL;
|
||||
|
||||
- /* Initialise all the variables contexts even if not used.
|
||||
+ /* Initialize all the variables contexts even if not used.
|
||||
* This permits to prune these contexts without errors.
|
||||
+ *
|
||||
+ * We need to make sure that those lists are not re-initialized
|
||||
+ * by stream-dependant underlying code because we could lose
|
||||
+ * track of already defined variables, leading to data inconsistency
|
||||
+ * and memory leaks...
|
||||
+ *
|
||||
+ * For reference: we had a very old bug caused by vars_txn and
|
||||
+ * vars_reqres being accidentally re-initialized in http_create_txn()
|
||||
+ * (https://github.com/haproxy/haproxy/issues/1935)
|
||||
*/
|
||||
vars_init_head(&s->vars_txn, SCOPE_TXN);
|
||||
vars_init_head(&s->vars_reqres, SCOPE_REQ);
|
||||
--
|
||||
1.7.10.4
|
||||
|
||||
@ -1,108 +0,0 @@
|
||||
From cb9a8fdbaef4a642eeb84ac9feaf636720d18360 Mon Sep 17 00:00:00 2001
|
||||
From: William Lallemand <wlallemand@haproxy.org>
|
||||
Date: Fri, 17 Feb 2023 16:23:52 +0100
|
||||
Subject: [PATCH] BUG/MINOR: mworker: prevent incorrect values in uptime
|
||||
|
||||
Since the recent changes on the clocks, now.tv_sec is not to be used
|
||||
between processes because it's a clock which is local to the process and
|
||||
does not contain a real unix timestamp. This patch fixes the issue by
|
||||
using "data.tv_sec" which is the wall clock instead of "now.tv_sec'.
|
||||
It prevents having incoherent timestamps.
|
||||
|
||||
It also introduces some checks on negatives values in order to never
|
||||
displays a netative value if it was computed from a wrong value set by a
|
||||
previous haproxy version.
|
||||
|
||||
It must be backported as far as 2.0.
|
||||
|
||||
(cherry picked from commit 5a7f83af84d2a08f69ce1629c7609c98f43411ab)
|
||||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
||||
(cherry picked from commit 7b8337e0cbaeebec26eab0062f0706f9388e3e2c)
|
||||
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||
---
|
||||
src/haproxy.c | 2 +-
|
||||
src/mworker.c | 21 ++++++++++++++++-----
|
||||
2 files changed, 17 insertions(+), 6 deletions(-)
|
||||
|
||||
diff --git a/src/haproxy.c b/src/haproxy.c
|
||||
index 5a3f6c7..0cb0662 100644
|
||||
--- a/src/haproxy.c
|
||||
+++ b/src/haproxy.c
|
||||
@@ -3432,7 +3432,7 @@ int main(int argc, char **argv)
|
||||
if (child->reloads == 0 &&
|
||||
child->options & PROC_O_TYPE_WORKER &&
|
||||
child->pid == -1) {
|
||||
- child->timestamp = now.tv_sec;
|
||||
+ child->timestamp = date.tv_sec;
|
||||
child->pid = ret;
|
||||
child->version = strdup(haproxy_version);
|
||||
break;
|
||||
diff --git a/src/mworker.c b/src/mworker.c
|
||||
index 686fc75..26b16cc 100644
|
||||
--- a/src/mworker.c
|
||||
+++ b/src/mworker.c
|
||||
@@ -559,13 +559,16 @@ static int cli_io_handler_show_proc(struct appctx *appctx)
|
||||
struct stconn *sc = appctx_sc(appctx);
|
||||
struct mworker_proc *child;
|
||||
int old = 0;
|
||||
- int up = now.tv_sec - proc_self->timestamp;
|
||||
+ int up = date.tv_sec - proc_self->timestamp;
|
||||
char *uptime = NULL;
|
||||
char *reloadtxt = NULL;
|
||||
|
||||
if (unlikely(sc_ic(sc)->flags & (CF_WRITE_ERROR|CF_SHUTW)))
|
||||
return 1;
|
||||
|
||||
+ if (up < 0) /* must never be negative because of clock drift */
|
||||
+ up = 0;
|
||||
+
|
||||
chunk_reset(&trash);
|
||||
|
||||
memprintf(&reloadtxt, "%d [failed: %d]", proc_self->reloads, proc_self->failedreloads);
|
||||
@@ -579,7 +582,9 @@ static int cli_io_handler_show_proc(struct appctx *appctx)
|
||||
|
||||
chunk_appendf(&trash, "# workers\n");
|
||||
list_for_each_entry(child, &proc_list, list) {
|
||||
- up = now.tv_sec - child->timestamp;
|
||||
+ up = date.tv_sec - child->timestamp;
|
||||
+ if (up < 0) /* must never be negative because of clock drift */
|
||||
+ up = 0;
|
||||
|
||||
if (!(child->options & PROC_O_TYPE_WORKER))
|
||||
continue;
|
||||
@@ -600,7 +605,9 @@ static int cli_io_handler_show_proc(struct appctx *appctx)
|
||||
|
||||
chunk_appendf(&trash, "# old workers\n");
|
||||
list_for_each_entry(child, &proc_list, list) {
|
||||
- up = now.tv_sec - child->timestamp;
|
||||
+ up = date.tv_sec - child->timestamp;
|
||||
+ if (up <= 0) /* must never be negative because of clock drift */
|
||||
+ up = 0;
|
||||
|
||||
if (!(child->options & PROC_O_TYPE_WORKER))
|
||||
continue;
|
||||
@@ -618,7 +625,9 @@ static int cli_io_handler_show_proc(struct appctx *appctx)
|
||||
chunk_appendf(&trash, "# programs\n");
|
||||
old = 0;
|
||||
list_for_each_entry(child, &proc_list, list) {
|
||||
- up = now.tv_sec - child->timestamp;
|
||||
+ up = date.tv_sec - child->timestamp;
|
||||
+ if (up < 0) /* must never be negative because of clock drift */
|
||||
+ up = 0;
|
||||
|
||||
if (!(child->options & PROC_O_TYPE_PROG))
|
||||
continue;
|
||||
@@ -635,7 +644,9 @@ static int cli_io_handler_show_proc(struct appctx *appctx)
|
||||
if (old) {
|
||||
chunk_appendf(&trash, "# old programs\n");
|
||||
list_for_each_entry(child, &proc_list, list) {
|
||||
- up = now.tv_sec - child->timestamp;
|
||||
+ up = date.tv_sec - child->timestamp;
|
||||
+ if (up < 0) /* must never be negative because of clock drift */
|
||||
+ up = 0;
|
||||
|
||||
if (!(child->options & PROC_O_TYPE_PROG))
|
||||
continue;
|
||||
--
|
||||
1.7.10.4
|
||||
|
||||
@ -1,62 +0,0 @@
|
||||
From 44eef1b3b566ec73a6d242ca347e6b6111dfabaa Mon Sep 17 00:00:00 2001
|
||||
From: Aurelien DARRAGON <adarragon@haproxy.com>
|
||||
Date: Tue, 7 Feb 2023 15:51:58 +0100
|
||||
Subject: [PATCH] BUG/MINOR: protocol: fix minor memory leak in
|
||||
protocol_bind_all()
|
||||
|
||||
In protocol_bind_all() (involved in startup sequence):
|
||||
We only free errmsg (set by fam->bind() attempt) when we make use of it.
|
||||
But this could lead to some memory leaks because there are some cases
|
||||
where we ignore the error message (e.g: verbose=0 with ERR_WARN messages).
|
||||
|
||||
As long as errmsg is set, we should always free it.
|
||||
|
||||
As mentioned earlier, this really is a minor leak because it can only occur on
|
||||
specific conditions (error paths) during the startup phase.
|
||||
|
||||
This may be backported up to 2.4.
|
||||
|
||||
--
|
||||
Backport notes:
|
||||
|
||||
-> 2.4 only:
|
||||
|
||||
Replace this:
|
||||
|
||||
| ha_warning("Binding [%s:%d] for %s %s: %s\n",
|
||||
| listener->bind_conf->file, listener->bind_conf->line,
|
||||
| proxy_type_str(px), px->id, errmsg);
|
||||
|
||||
By this:
|
||||
|
||||
| else if (lerr & ERR_WARN)
|
||||
| ha_warning("Starting %s %s: %s\n",
|
||||
| proxy_type_str(px), px->id, errmsg);
|
||||
|
||||
(cherry picked from commit 8429627e3c2eb472dc94ec8d3d7275ef68a79128)
|
||||
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||
(cherry picked from commit da9a15ff0326d59ba38f5a1b258820d91c7df649)
|
||||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
||||
---
|
||||
src/protocol.c | 4 +++-
|
||||
1 file changed, 3 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/protocol.c b/src/protocol.c
|
||||
index 03f7085..146733a 100644
|
||||
--- a/src/protocol.c
|
||||
+++ b/src/protocol.c
|
||||
@@ -92,8 +92,10 @@ int protocol_bind_all(int verbose)
|
||||
ha_warning("Binding [%s:%d] for %s %s: %s\n",
|
||||
listener->bind_conf->file, listener->bind_conf->line,
|
||||
proxy_type_str(px), px->id, errmsg);
|
||||
- ha_free(&errmsg);
|
||||
}
|
||||
+ if (lerr != ERR_NONE)
|
||||
+ ha_free(&errmsg);
|
||||
+
|
||||
if (lerr & ERR_ABORT)
|
||||
break;
|
||||
|
||||
--
|
||||
1.7.10.4
|
||||
|
||||
@ -1,45 +0,0 @@
|
||||
From f233b187f5f101a724c6fdde5b0a9e4fb6d6d50e Mon Sep 17 00:00:00 2001
|
||||
From: Aurelien DARRAGON <adarragon@haproxy.com>
|
||||
Date: Wed, 14 Jun 2023 09:53:32 +0200
|
||||
Subject: [PATCH] BUG/MINOR: server: inherit from netns in srv_settings_cpy()
|
||||
|
||||
When support for 'namespace' keyword was added for the 'default-server'
|
||||
directive in 22f41a2 ("MINOR: server: Make 'default-server' support
|
||||
'namespace' keyword."), we forgot to copy the attribute from the parent
|
||||
to the newly created server.
|
||||
|
||||
This resulted in the 'namespace' keyword being parsed without errors when
|
||||
used from a 'default-server' directive, but in practise the option was
|
||||
simply ignored.
|
||||
|
||||
There's no need to duplicate the netns struct because it is stored in
|
||||
a shared list, so copying the pointer does the job.
|
||||
|
||||
This patch partially fixes GH #2038 and should be backported to all
|
||||
stable versions.
|
||||
|
||||
(cherry picked from commit 19b5a7c7a5b4b01970e06d20928ed1d87ca6efcd)
|
||||
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
|
||||
(cherry picked from commit 60d185d9320ae1293c466e004b059b0310dcb13c)
|
||||
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
|
||||
(cherry picked from commit 20e6fa6abf259c20ad471f44d646d0f0ee28f3ec)
|
||||
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
|
||||
---
|
||||
src/server.c | 1 +
|
||||
1 file changed, 1 insertion(+)
|
||||
|
||||
diff --git a/src/server.c b/src/server.c
|
||||
index e9e09dd..7935668 100644
|
||||
--- a/src/server.c
|
||||
+++ b/src/server.c
|
||||
@@ -2280,6 +2280,7 @@ void srv_settings_cpy(struct server *srv, const struct server *src, int srv_tmpl
|
||||
if (srv_tmpl)
|
||||
srv->srvrq = src->srvrq;
|
||||
|
||||
+ srv->netns = src->netns;
|
||||
srv->check.via_socks4 = src->check.via_socks4;
|
||||
srv->socks4_addr = src->socks4_addr;
|
||||
}
|
||||
--
|
||||
1.7.10.4
|
||||
|
||||
@ -1,36 +0,0 @@
|
||||
From b3dc43ddadfd98b745823deaed6e7743b59442eb Mon Sep 17 00:00:00 2001
|
||||
From: Christopher Faulet <cfaulet@haproxy.com>
|
||||
Date: Tue, 27 Sep 2022 09:14:47 +0200
|
||||
Subject: [PATCH] BUG/MINOR: stream: Perform errors handling in right order in
|
||||
stream_new()
|
||||
|
||||
The frontend SC is attached before the backend one is allocated. Thus an
|
||||
allocation error on backend SC must be handled before an error on the
|
||||
frontend SC.
|
||||
|
||||
This patch must be backported to 2.6.
|
||||
|
||||
(cherry picked from commit 4cfc038cb19996f5d2fe60284fdb556503a5f9ef)
|
||||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
||||
---
|
||||
src/stream.c | 4 ++--
|
||||
1 file changed, 2 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/src/stream.c b/src/stream.c
|
||||
index 0cbbb70..fc8b82a 100644
|
||||
--- a/src/stream.c
|
||||
+++ b/src/stream.c
|
||||
@@ -566,9 +566,9 @@ struct stream *stream_new(struct session *sess, struct stconn *sc, struct buffer
|
||||
out_fail_accept:
|
||||
flt_stream_release(s, 0);
|
||||
LIST_DELETE(&s->list);
|
||||
- out_fail_attach_scf:
|
||||
- sc_free(s->scb);
|
||||
out_fail_alloc_scb:
|
||||
+ sc_free(s->scb);
|
||||
+ out_fail_attach_scf:
|
||||
task_destroy(t);
|
||||
out_fail_alloc:
|
||||
pool_free(pool_head_stream, s);
|
||||
--
|
||||
1.7.10.4
|
||||
@ -1,39 +0,0 @@
|
||||
From d4dba38ab101eee4cbd0c8d8aa21181825ef6472 Mon Sep 17 00:00:00 2001
|
||||
From: Aurelien DARRAGON <adarragon@haproxy.com>
|
||||
Date: Thu, 11 May 2023 18:49:14 +0200
|
||||
Subject: [PATCH] BUG/MINOR: errors: handle malloc failure in usermsgs_put()
|
||||
|
||||
usermsgs_buf.size is set without first checking if previous malloc
|
||||
attempt succeeded.
|
||||
|
||||
This could fool the buffer API into assuming that the buffer is
|
||||
initialized, resulting in unsafe read/writes.
|
||||
|
||||
Guarding usermsgs_buf.size assignment with the malloc attempt result
|
||||
to make the buffer initialization safe against malloc failures.
|
||||
|
||||
This partially fixes GH #2130.
|
||||
|
||||
It should be backported up to 2.6.
|
||||
|
||||
Conflict:NA
|
||||
Reference:https://github.com/haproxy/haproxy/commit/d4dba38ab101eee4cbd0c8d8aa21181825ef6472
|
||||
|
||||
---
|
||||
src/errors.c | 3 ++-
|
||||
1 file changed, 2 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/errors.c b/src/errors.c
|
||||
index 2e9d6afb7e04..5913cb1d509d 100644
|
||||
--- a/src/errors.c
|
||||
+++ b/src/errors.c
|
||||
@@ -229,7 +229,8 @@ static void usermsgs_put(const struct ist *msg)
|
||||
/* Allocate the buffer if not already done. */
|
||||
if (unlikely(b_is_null(&usermsgs_buf))) {
|
||||
usermsgs_buf.area = malloc(USER_MESSAGES_BUFSIZE * sizeof(char));
|
||||
- usermsgs_buf.size = USER_MESSAGES_BUFSIZE;
|
||||
+ if (usermsgs_buf.area)
|
||||
+ usermsgs_buf.size = USER_MESSAGES_BUFSIZE;
|
||||
}
|
||||
|
||||
if (likely(!b_is_null(&usermsgs_buf))) {
|
||||
BIN
haproxy-2.6.15.tar.gz
Normal file
BIN
haproxy-2.6.15.tar.gz
Normal file
Binary file not shown.
Binary file not shown.
25
haproxy.spec
25
haproxy.spec
@ -4,8 +4,8 @@
|
||||
%global _hardened_build 1
|
||||
|
||||
Name: haproxy
|
||||
Version: 2.6.6
|
||||
Release: 6
|
||||
Version: 2.6.15
|
||||
Release: 1
|
||||
Summary: The Reliable, High Performance TCP/HTTP Load Balancer
|
||||
|
||||
License: GPLv2+
|
||||
@ -16,22 +16,8 @@ Source2: %{name}.cfg
|
||||
Source3: %{name}.logrotate
|
||||
Source4: %{name}.sysconfig
|
||||
|
||||
Patch0: CVE-2023-25725.patch
|
||||
Patch1: CVE-2023-0056.patch
|
||||
Patch2: CVE-2023-25950.patch
|
||||
Patch3: CVE-2023-40225.patch
|
||||
|
||||
Patch1001: backport-BUG-MINOR-stream-Perform-errors-handling-in-right-order-in-stream_new.patch
|
||||
Patch1002: backport-BUG-MINOR-http_ana-txn-don-t-re-initialize-txn-and-req-var-lists.patch
|
||||
Patch1003: backport-BUG-MEDIUM-connection-Clear-flags-when-a-conn-is-removed-from-an-idle-list.patch
|
||||
Patch1004: backport-BUG-MINOR-mworker-prevent-incorrect-values-in-uptime.patch
|
||||
Patch1005: backport-BUG-MEDIUM-connection-Preserve-flags-when-a-conn-is-removed-from-an-idle-list.patch
|
||||
Patch1006: backport-BUG-MINOR-protocol-fix-minor-memory-leak-in-protocol_bind_all.patch
|
||||
Patch1007: backport-BUG-MEDIUM-stream-do-not-try-to-free-a-failed-stream-conn.patch
|
||||
Patch1008: backport-BUG-MINOR-server-inherit-from-netns-in-srv_settings_cpy.patch
|
||||
Patch1009: backport-errors-handle-malloc-failure-in-usermsgs_put.patch
|
||||
Patch1010: backport-ssl_sock-add-check-for-ha_meth.patch
|
||||
Patch1011: backport-thread-add-a-check-for-pthread_create.patch
|
||||
Patch0: backport-ssl_sock-add-check-for-ha_meth.patch
|
||||
Patch1: backport-thread-add-a-check-for-pthread_create.patch
|
||||
|
||||
|
||||
BuildRequires: gcc lua-devel pcre2-devel openssl-devel systemd-devel systemd libatomic
|
||||
@ -134,6 +120,9 @@ exit 0
|
||||
%{_mandir}/man1/*
|
||||
|
||||
%changelog
|
||||
* Wed Oct 11 2023 yaoxin <yao_xin001@hoperun.com> - 2.6.15-1
|
||||
- Upgrade to 2.6.15
|
||||
|
||||
* Wed Sep 27 2023 xinghe <xinghe2@h-partners.com> - 2.6.6-6
|
||||
- Type:bugfix
|
||||
- CVE:NA
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user