From 76a2da3fd413f607b0c0ea2f0eeabb088857f020 Mon Sep 17 00:00:00 2001 From: Christian Lachner Date: Sat, 2 Nov 2019 12:11:07 +0100 Subject: [PATCH] haproxy: Update patches for HAProxy v2.0.8 - Add new patches (see https://www.haproxy.org/bugs/bugs-2.0.8.html) Signed-off-by: Christian Lachner --- net/haproxy/Makefile | 2 +- ...e-of-n-in-header-values-replacements.patch | 65 +++++++++ ...not-emit-logs-on-backend-connections.patch | 133 ++++++++++++++++++ ...avoid-confusion-in-time-parsing-init.patch | 33 +++++ ...e-kw--io_release-if-kw--parse-failed.patch | 48 +++++++ ...s-arent-full-anymore-if-nothing-sent.patch | 33 +++++ ...-from-mux-until-SI_ST_EST-is-reached.patch | 112 +++++++++++++++ ...h => 006-OPENWRT-add-uclibc-support.patch} | 0 ...h => 007-OPENWRT-openssl-deprecated.patch} | 0 9 files changed, 425 insertions(+), 1 deletion(-) create mode 100644 net/haproxy/patches/000-MINOR-config-warn-on-presence-of-n-in-header-values-replacements.patch create mode 100644 net/haproxy/patches/001-BUG-MINOR-mux-h2-do-not-emit-logs-on-backend-connections.patch create mode 100644 net/haproxy/patches/002-MINOR-tcp-avoid-confusion-in-time-parsing-init.patch create mode 100644 net/haproxy/patches/003-BUG-MINOR-cli-dont-call-the-kw--io_release-if-kw--parse-failed.patch create mode 100644 net/haproxy/patches/004-BUG-MINOR-mux-h2-Dont-pretend-mux-buffers-arent-full-anymore-if-nothing-sent.patch create mode 100644 net/haproxy/patches/005-BUG-MAJOR-stream-int-Dont-receive-data-from-mux-until-SI_ST_EST-is-reached.patch rename net/haproxy/patches/{000-OPENWRT-add-uclibc-support.patch => 006-OPENWRT-add-uclibc-support.patch} (100%) rename net/haproxy/patches/{001-OPENWRT-openssl-deprecated.patch => 007-OPENWRT-openssl-deprecated.patch} (100%) diff --git a/net/haproxy/Makefile b/net/haproxy/Makefile index c88467145f..3e891f418d 100644 --- a/net/haproxy/Makefile +++ b/net/haproxy/Makefile @@ -11,7 +11,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=haproxy PKG_VERSION:=2.0.8 -PKG_RELEASE:=1 +PKG_RELEASE:=2 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz PKG_SOURCE_URL:=https://www.haproxy.org/download/2.0/src diff --git a/net/haproxy/patches/000-MINOR-config-warn-on-presence-of-n-in-header-values-replacements.patch b/net/haproxy/patches/000-MINOR-config-warn-on-presence-of-n-in-header-values-replacements.patch new file mode 100644 index 0000000000..b354e8bfc2 --- /dev/null +++ b/net/haproxy/patches/000-MINOR-config-warn-on-presence-of-n-in-header-values-replacements.patch @@ -0,0 +1,65 @@ +commit 41898a216e92c80c1354b67613834be1b3e97864 +Author: Willy Tarreau +Date: Fri Oct 25 14:16:14 2019 +0200 + + MINOR: config: warn on presence of "\n" in header values/replacements + + Yves Lafon reported an interesting case where an old rsprep rule used + to conditionally append a header field by inserting a \n in the exising + value was breaking H2 in HTX mode, with the browser rightfully reporting + a PROTOCOL_ERROR when facing the \n. In legacy mode, since the response + is first parsed again as an HTTP/1 message before being converted to H2 + the issue does not happen. We should definitely discourage from using + this old trick nowadays, http-request and http-response rules were made + exactly to end this. Let's detect this and emit a warning when present. + In 2.0 there is already a warning recalling that these rules are + deprecated and which explains what to do instead, so the user now gets + all the relevant information to convert them. + + There is no upstream commit ID for this patch because these rules were + indeed removed from 2.1. This patch could be backported to 1.9 as it + can also trigger the problem when HTX is enabled. + +diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c +index 5454f3bb..9c3e107a 100644 +--- a/src/cfgparse-listen.c ++++ b/src/cfgparse-listen.c +@@ -294,6 +294,12 @@ static int create_cond_regex_rule(const char *file, int line, + goto err_free; + } + ++ if (repl && strchr(repl, '\n')) { ++ ha_warning("parsing [%s:%d] : '%s' : hack involving '\\n' character in replacement string will fail with HTTP/2.\n", ++ file, line, cmd); ++ ret_code |= ERR_WARN; ++ } ++ + if (dir == SMP_OPT_DIR_REQ && warnif_misplaced_reqxxx(px, file, line, cmd)) + ret_code |= ERR_WARN; + +@@ -4039,6 +4045,12 @@ stats_error_parsing: + goto out; + } + ++ if (strchr(args[1], '\n')) { ++ ha_warning("parsing [%s:%d] : '%s' : hack involving '\\n' character in new header value will fail with HTTP/2.\n", ++ file, linenum, args[0]); ++ err_code |= ERR_WARN; ++ } ++ + wl = calloc(1, sizeof(*wl)); + wl->cond = cond; + wl->s = strdup(args[1]); +@@ -4157,6 +4169,12 @@ stats_error_parsing: + goto out; + } + ++ if (strchr(args[1], '\n')) { ++ ha_warning("parsing [%s:%d] : '%s' : hack involving '\\n' character in new header value will fail with HTTP/2.\n", ++ file, linenum, args[0]); ++ err_code |= ERR_WARN; ++ } ++ + wl = calloc(1, sizeof(*wl)); + wl->cond = cond; + wl->s = strdup(args[1]); diff --git a/net/haproxy/patches/001-BUG-MINOR-mux-h2-do-not-emit-logs-on-backend-connections.patch b/net/haproxy/patches/001-BUG-MINOR-mux-h2-do-not-emit-logs-on-backend-connections.patch new file mode 100644 index 0000000000..fe92210514 --- /dev/null +++ b/net/haproxy/patches/001-BUG-MINOR-mux-h2-do-not-emit-logs-on-backend-connections.patch @@ -0,0 +1,133 @@ +commit 21178a582238ee1c57d0aef73c97711741dd93ed +Author: Willy Tarreau +Date: Wed Oct 23 11:06:35 2019 +0200 + + BUG/MINOR: mux-h2: do not emit logs on backend connections + + The logs were added to the H2 mux so that we can report logs in case + of errors that prevent a stream from being created, but as a side effect + these logs are emitted twice for backend connections: once by the H2 mux + itself and another time by the upper layer stream. It can even happen + more with connection retries. + + This patch makes sure we do not emit logs for backend connections. + + It should be backported to 2.0 and 1.9. + + (cherry picked from commit 9364a5fda33a2f591d5e2640249a54af8955fb8b) + Signed-off-by: Christopher Faulet + +diff --git a/src/mux_h2.c b/src/mux_h2.c +index 8841c0e0..afa68e80 100644 +--- a/src/mux_h2.c ++++ b/src/mux_h2.c +@@ -1661,7 +1661,8 @@ static int h2c_handle_settings(struct h2c *h2c) + h2c->st0 = H2_CS_FRAME_A; + return 1; + fail: +- sess_log(h2c->conn->owner); ++ if (!(h2c->flags & H2_CF_IS_BACK)) ++ sess_log(h2c->conn->owner); + h2c_error(h2c, error); + return 0; + } +@@ -2318,7 +2319,8 @@ static void h2_process_demux(struct h2c *h2c) + /* RFC7540#3.5: a GOAWAY frame MAY be omitted */ + if (h2c->st0 == H2_CS_ERROR) { + h2c->st0 = H2_CS_ERROR2; +- sess_log(h2c->conn->owner); ++ if (!(h2c->flags & H2_CF_IS_BACK)) ++ sess_log(h2c->conn->owner); + } + goto fail; + } +@@ -2327,7 +2329,8 @@ static void h2_process_demux(struct h2c *h2c) + /* RFC7540#3.5: a GOAWAY frame MAY be omitted */ + h2c_error(h2c, H2_ERR_PROTOCOL_ERROR); + h2c->st0 = H2_CS_ERROR2; +- sess_log(h2c->conn->owner); ++ if (!(h2c->flags & H2_CF_IS_BACK)) ++ sess_log(h2c->conn->owner); + goto fail; + } + +@@ -2335,7 +2338,8 @@ static void h2_process_demux(struct h2c *h2c) + /* RFC7540#3.5: a GOAWAY frame MAY be omitted */ + h2c_error(h2c, H2_ERR_FRAME_SIZE_ERROR); + h2c->st0 = H2_CS_ERROR2; +- sess_log(h2c->conn->owner); ++ if (!(h2c->flags & H2_CF_IS_BACK)) ++ sess_log(h2c->conn->owner); + goto fail; + } + +@@ -2363,7 +2367,7 @@ static void h2_process_demux(struct h2c *h2c) + + if ((int)hdr.len < 0 || (int)hdr.len > global.tune.bufsize) { + h2c_error(h2c, H2_ERR_FRAME_SIZE_ERROR); +- if (!h2c->nb_streams) { ++ if (!h2c->nb_streams && !(h2c->flags & H2_CF_IS_BACK)) { + /* only log if no other stream can report the error */ + sess_log(h2c->conn->owner); + } +@@ -2381,7 +2385,8 @@ static void h2_process_demux(struct h2c *h2c) + */ + if (hdr.len < 1) { + h2c_error(h2c, H2_ERR_FRAME_SIZE_ERROR); +- sess_log(h2c->conn->owner); ++ if (!(h2c->flags & H2_CF_IS_BACK)) ++ sess_log(h2c->conn->owner); + goto fail; + } + hdr.len--; +@@ -2396,7 +2401,8 @@ static void h2_process_demux(struct h2c *h2c) + * frame payload or greater => error. + */ + h2c_error(h2c, H2_ERR_PROTOCOL_ERROR); +- sess_log(h2c->conn->owner); ++ if (!(h2c->flags & H2_CF_IS_BACK)) ++ sess_log(h2c->conn->owner); + goto fail; + } + +@@ -2420,7 +2426,8 @@ static void h2_process_demux(struct h2c *h2c) + ret = h2_frame_check(h2c->dft, 1, h2c->dsi, h2c->dfl, global.tune.bufsize); + if (ret != H2_ERR_NO_ERROR) { + h2c_error(h2c, ret); +- sess_log(h2c->conn->owner); ++ if (!(h2c->flags & H2_CF_IS_BACK)) ++ sess_log(h2c->conn->owner); + goto fail; + } + } +@@ -2458,7 +2465,7 @@ static void h2_process_demux(struct h2c *h2c) + * this state MUST be treated as a connection error + */ + h2c_error(h2c, H2_ERR_PROTOCOL_ERROR); +- if (!h2c->nb_streams) { ++ if (!h2c->nb_streams && !(h2c->flags & H2_CF_IS_BACK)) { + /* only log if no other stream can report the error */ + sess_log(h2c->conn->owner); + } +@@ -2608,7 +2615,8 @@ static void h2_process_demux(struct h2c *h2c) + * frames so this one is out of sequence. + */ + h2c_error(h2c, H2_ERR_PROTOCOL_ERROR); +- sess_log(h2c->conn->owner); ++ if (!(h2c->flags & H2_CF_IS_BACK)) ++ sess_log(h2c->conn->owner); + goto fail; + + case H2_FT_HEADERS: +@@ -2714,10 +2722,8 @@ static int h2_process_mux(struct h2c *h2c) + if (unlikely(h2c->st0 == H2_CS_PREFACE && (h2c->flags & H2_CF_IS_BACK))) { + if (unlikely(h2c_bck_send_preface(h2c) <= 0)) { + /* RFC7540#3.5: a GOAWAY frame MAY be omitted */ +- if (h2c->st0 == H2_CS_ERROR) { ++ if (h2c->st0 == H2_CS_ERROR) + h2c->st0 = H2_CS_ERROR2; +- sess_log(h2c->conn->owner); +- } + goto fail; + } + h2c->st0 = H2_CS_SETTINGS1; diff --git a/net/haproxy/patches/002-MINOR-tcp-avoid-confusion-in-time-parsing-init.patch b/net/haproxy/patches/002-MINOR-tcp-avoid-confusion-in-time-parsing-init.patch new file mode 100644 index 0000000000..5808c17364 --- /dev/null +++ b/net/haproxy/patches/002-MINOR-tcp-avoid-confusion-in-time-parsing-init.patch @@ -0,0 +1,33 @@ +commit 74a1e4393f7a7b194abb4f428fd02c7c088f6c67 +Author: William Dauchy +Date: Wed Oct 23 19:31:36 2019 +0200 + + MINOR: tcp: avoid confusion in time parsing init + + We never enter val_fc_time_value when an associated fetcher such as `fc_rtt` is + called without argument. meaning `type == ARGT_STOP` will never be true and so + the default `data.sint = TIME_UNIT_MS` will never be set. remove this part to + avoid thinking default data.sint is set to ms while reading the code. + + Signed-off-by: William Dauchy + + [Cf: This patch may safely backported as far as 1.7. But no matter if not.] + + (cherry picked from commit b705b4d7d308d1132a772f3ae2d6113447022a60) + Signed-off-by: Christopher Faulet + +diff --git a/src/proto_tcp.c b/src/proto_tcp.c +index c3578ea2..cfd58e60 100644 +--- a/src/proto_tcp.c ++++ b/src/proto_tcp.c +@@ -1569,10 +1569,6 @@ smp_fetch_dport(const struct arg *args, struct sample *smp, const char *kw, void + */ + static int val_fc_time_value(struct arg *args, char **err) + { +- if (args[0].type == ARGT_STOP) { +- args[0].type = ARGT_SINT; +- args[0].data.sint = TIME_UNIT_MS; +- } + if (args[0].type == ARGT_STR) { + if (strcmp(args[0].data.str.area, "us") == 0) { + free(args[0].data.str.area); diff --git a/net/haproxy/patches/003-BUG-MINOR-cli-dont-call-the-kw--io_release-if-kw--parse-failed.patch b/net/haproxy/patches/003-BUG-MINOR-cli-dont-call-the-kw--io_release-if-kw--parse-failed.patch new file mode 100644 index 0000000000..2763f08c9a --- /dev/null +++ b/net/haproxy/patches/003-BUG-MINOR-cli-dont-call-the-kw--io_release-if-kw--parse-failed.patch @@ -0,0 +1,48 @@ +commit d4f20fadd9c3145de0eb5f5434f57b9fffc61062 +Author: William Lallemand +Date: Fri Oct 25 21:10:14 2019 +0200 + + BUG/MINOR: cli: don't call the kw->io_release if kw->parse failed + + The io_release() callback of the cli_kw is supposed to be used to clean + what an io_handler() has made. It is called once the work in the IO + handler is finished, or when the connection was aborted by the client. + + This patch fixes a bug where the io_release callback was called even + when the parse() callback failed. Which means that the io_release() could + called even if the io_handler() was not called. + + Should be backported in every versions that have a cli_kw->release(). + (as far as 1.7) + + (cherry picked from commit 90b098c921e15f912dbde42658e34780f0ba446d) + Signed-off-by: Christopher Faulet + +diff --git a/src/cli.c b/src/cli.c +index 9a9f80f9..c063fbf0 100644 +--- a/src/cli.c ++++ b/src/cli.c +@@ -570,10 +570,19 @@ static int cli_parse_request(struct appctx *appctx) + + appctx->io_handler = kw->io_handler; + appctx->io_release = kw->io_release; +- /* kw->parse could set its own io_handler or ip_release handler */ +- if ((!kw->parse || kw->parse(args, payload, appctx, kw->private) == 0) && appctx->io_handler) { +- appctx->st0 = CLI_ST_CALLBACK; +- } ++ ++ if (kw->parse && kw->parse(args, payload, appctx, kw->private) != 0) ++ goto fail; ++ ++ /* kw->parse could set its own io_handler or io_release handler */ ++ if (!appctx->io_handler) ++ goto fail; ++ ++ appctx->st0 = CLI_ST_CALLBACK; ++ return 1; ++fail: ++ appctx->io_handler = NULL; ++ appctx->io_release = NULL; + return 1; + } + diff --git a/net/haproxy/patches/004-BUG-MINOR-mux-h2-Dont-pretend-mux-buffers-arent-full-anymore-if-nothing-sent.patch b/net/haproxy/patches/004-BUG-MINOR-mux-h2-Dont-pretend-mux-buffers-arent-full-anymore-if-nothing-sent.patch new file mode 100644 index 0000000000..97aee88c83 --- /dev/null +++ b/net/haproxy/patches/004-BUG-MINOR-mux-h2-Dont-pretend-mux-buffers-arent-full-anymore-if-nothing-sent.patch @@ -0,0 +1,33 @@ +commit 074230876d05bdf3fe33893889b326da14ab8ae9 +Author: Christopher Faulet +Date: Thu Oct 24 10:31:01 2019 +0200 + + BUG/MINOR: mux-h2: Don't pretend mux buffers aren't full anymore if nothing sent + + In h2_send(), when something is sent, we remove the flags + (H2_CF_MUX_MFULL|H2_CF_DEM_MROOM) on the h2 connection. This way, we are able to + wake up all streams waiting to send data. Unfortunatly, these flags are + unconditionally removed, even when nothing was sent. So if the h2c is blocked + because the mux buffers are full and we are unable to send anything, all streams + in the send_list are woken up for nothing. Now, we only remove these flags if at + least a send succeeds. + + This patch must be backport to 2.0. + + (cherry picked from commit 69fe5cea213afd0c7465094e9dfead93143dcf3f) + Signed-off-by: Christopher Faulet + +diff --git a/src/mux_h2.c b/src/mux_h2.c +index afa68e80..ac34a723 100644 +--- a/src/mux_h2.c ++++ b/src/mux_h2.c +@@ -2943,7 +2943,8 @@ static int h2_send(struct h2c *h2c) + offer_buffers(NULL, tasks_run_queue); + + /* wrote at least one byte, the buffer is not full anymore */ +- h2c->flags &= ~(H2_CF_MUX_MFULL | H2_CF_DEM_MROOM); ++ if (sent) ++ h2c->flags &= ~(H2_CF_MUX_MFULL | H2_CF_DEM_MROOM); + } + + if (conn->flags & CO_FL_SOCK_WR_SH) { diff --git a/net/haproxy/patches/005-BUG-MAJOR-stream-int-Dont-receive-data-from-mux-until-SI_ST_EST-is-reached.patch b/net/haproxy/patches/005-BUG-MAJOR-stream-int-Dont-receive-data-from-mux-until-SI_ST_EST-is-reached.patch new file mode 100644 index 0000000000..e4717e8feb --- /dev/null +++ b/net/haproxy/patches/005-BUG-MAJOR-stream-int-Dont-receive-data-from-mux-until-SI_ST_EST-is-reached.patch @@ -0,0 +1,112 @@ +commit 27ebcefd41b3e44395c3fe71939ef98b03f98e7b +Author: Christopher Faulet +Date: Fri Oct 25 10:21:01 2019 +0200 + + BUG/MAJOR: stream-int: Don't receive data from mux until SI_ST_EST is reached + + This bug is pretty pernicious and have serious consequences : In 2.1, an + infinite loop in process_stream() because the backend stream-interface remains + in the ready state (SI_ST_RDY). In 2.0, a call in loop to process_stream() + because the stream-interface remains blocked in the connect state + (SI_ST_CON). In both cases, it happens after a connection retry attempt. In 1.9, + it seems to not happen. But it may be just by chance or just because it is + harder to get right conditions to trigger the bug. However, reading the code, + the bug seems to exist too. + + Here is how the bug happens in 2.1. When we try to establish a new connection to + a server, the corresponding stream-interface is first set to the connect state + (SI_ST_CON). When the underlying connection is known to be connected (the flag + CO_FL_CONNECTED set), the stream-interface is switched to the ready state + (SI_ST_RDY). It is a transient state between the connect state (SI_ST_CON) and + the established state (SI_ST_EST). It must be handled on the next call to + process_stream(), which is responsible to operate the transition. During all + this time, errors can occur. A connection error or a client abort. The transient + state SI_ST_RDY was introduced to let a chance to process_stream() to catch + these errors before considering the connection as fully established. + Unfortunatly, if a read0 is catched in states SI_ST_CON or SI_ST_RDY, it is + possible to have a shutdown without transition to SI_ST_DIS (in fact, here, + SI_ST_CON is swichted to SI_ST_RDY). This happens if the request was fully + received and analyzed. In this case, the flag SI_FL_NOHALF is set on the backend + stream-interface. If an error is also reported during the connect, the behavior + is undefined because an error is returned to the client and a connection retry + is performed. So on the next connection attempt to the server, if another error + is reported, a client abort is detected. But the shutdown for writes was already + done. So the transition to the state SI_ST_DIS is impossible. We stay in the + state SI_ST_RDY. Because it is a transient state, we loop in process_stream() to + perform the transition. + + It is hard to understand how the bug happens reading the code and even harder to + explain. But there is a trivial way to hit the bug by sending h2 requests to a + server only speaking h1. For instance, with the following config : + + listen tst + bind *:80 + server www 127.0.0.1:8000 proto h2 # in reality, it is a HTTP/1.1 server + + It is a configuration error, but it is an easy way to observe the bug. Note it + may happen with a valid configuration. + + So, after a careful analyzis, it appears that si_cs_recv() should never be + called for a not fully established stream-interface. This way the connection + retries will be performed before reporting an error to the client. Thus, if a + shutdown is performed because a read0 is handled, the stream-interface is + inconditionnaly set to the transient state SI_ST_DIS. + + This patch must be backported to 2.0 and 1.9. However on these versions, this + patch reveals a design flaw about connections and a bad way to perform the + connection retries. We are working on it. + + (cherry picked from commit 04400bc7875fcc362495b0f25e75ba6fc2f44850) + Signed-off-by: Christopher Faulet + +diff --git a/src/stream_interface.c b/src/stream_interface.c +index ef0fea7f..211fe2d7 100644 +--- a/src/stream_interface.c ++++ b/src/stream_interface.c +@@ -1215,6 +1215,10 @@ int si_cs_recv(struct conn_stream *cs) + int read_poll = MAX_READ_POLL_LOOPS; + int flags = 0; + ++ /* If not established yet, do nothing. */ ++ if (si->state != SI_ST_EST) ++ return 0; ++ + /* If another call to si_cs_recv() failed, and we subscribed to + * recv events already, give up now. + */ +@@ -1293,8 +1297,6 @@ int si_cs_recv(struct conn_stream *cs) + ic->total += ret; + cur_read += ret; + ic->flags |= CF_READ_PARTIAL; +- if (si->state == SI_ST_CON) +- si->state = SI_ST_RDY; + } + + if (cs->flags & CS_FL_EOS) +@@ -1391,8 +1393,6 @@ int si_cs_recv(struct conn_stream *cs) + + ic->flags |= CF_READ_PARTIAL; + ic->total += ret; +- if (si->state == SI_ST_CON) +- si->state = SI_ST_RDY; + + if ((ic->flags & CF_READ_DONTWAIT) || --read_poll <= 0) { + /* we're stopped by the channel's policy */ +@@ -1544,16 +1544,7 @@ static void stream_int_read0(struct stream_interface *si) + + si_done_get(si); + +- /* Don't change the state to SI_ST_DIS yet if we're still +- * in SI_ST_CON, otherwise it means sess_establish() hasn't +- * been called yet, and so the analysers would not run. However +- * it's fine to switch to SI_ST_RDY as we have really validated +- * the connection. +- */ +- if (si->state == SI_ST_EST) +- si->state = SI_ST_DIS; +- else if (si->state == SI_ST_CON) +- si->state = SI_ST_RDY; ++ si->state = SI_ST_DIS; + si->exp = TICK_ETERNITY; + return; + } diff --git a/net/haproxy/patches/000-OPENWRT-add-uclibc-support.patch b/net/haproxy/patches/006-OPENWRT-add-uclibc-support.patch similarity index 100% rename from net/haproxy/patches/000-OPENWRT-add-uclibc-support.patch rename to net/haproxy/patches/006-OPENWRT-add-uclibc-support.patch diff --git a/net/haproxy/patches/001-OPENWRT-openssl-deprecated.patch b/net/haproxy/patches/007-OPENWRT-openssl-deprecated.patch similarity index 100% rename from net/haproxy/patches/001-OPENWRT-openssl-deprecated.patch rename to net/haproxy/patches/007-OPENWRT-openssl-deprecated.patch -- 2.30.2