Merge pull request #10430 from gladiac1337/haproxy-2.0.8-up

haproxy: Update patches for HAProxy v2.0.8 + migrate to procd

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
@@ -171,8 +171,6 @@ define Package/haproxy/install
 	$(INSTALL_CONF) ./files/haproxy.cfg $(1)/etc/
 	$(INSTALL_DIR) $(1)/etc/init.d
 	$(INSTALL_BIN) ./files/haproxy.init $(1)/etc/init.d/haproxy
-	$(INSTALL_DIR) $(1)/etc/hotplug.d/net
-	$(INSTALL_BIN) ./files/haproxy.hotplug $(1)/etc/hotplug.d/net/90-haproxy
 endef
 
 Package/haproxy-nossl/install = $(Package/haproxy/install)

net/haproxy/files/haproxy.hotplug

@@ -1,7 +0,0 @@
-#!/bin/sh
-
-if [ "$ACTION" = add ]; then
-	/etc/init.d/haproxy enabled && \
-		/etc/init.d/haproxy start
-fi
-

net/haproxy/files/haproxy.init

@@ -1,29 +1,25 @@
 #!/bin/sh /etc/rc.common
-# Copyright (C) 2009-2010 OpenWrt.org
+# Copyright (C) 2009-2019 OpenWrt.org
 
 START=99
 STOP=80
 
-SERVICE_USE_PID=1
+USE_PROCD=1
+
 EXTRA_COMMANDS="check"
 
 HAPROXY_BIN="/usr/sbin/haproxy"
 HAPROXY_CONFIG="/etc/haproxy.cfg"
-HAPROXY_PID="/var/run/haproxy.pid"
-
-start() {
-	service_start $HAPROXY_BIN -q -D -f "$HAPROXY_CONFIG" -p "$HAPROXY_PID"
-}
-
-stop() {
-	kill -9 $(cat $HAPROXY_PID | tr "\n" " ")
-	service_stop $HAPROXY_BIN
-}
 
-reload() {
-	$HAPROXY_BIN -D -q -f $HAPROXY_CONFIG -p $HAPROXY_PID -sf $(cat $HAPROXY_PID)
+start_service() {
+	procd_open_instance
+	procd_set_param respawn
+	procd_set_param file "$HAPROXY_CONFIG"
+	procd_set_param reload_signal USR2
+	procd_set_param command $HAPROXY_BIN -q -W -db -f "$HAPROXY_CONFIG"
+	procd_close_instance
 }
 
 check() {
-        $HAPROXY_BIN -c -q -V -f $HAPROXY_CONFIG
+	$HAPROXY_BIN -c -q -V -f $HAPROXY_CONFIG
 }

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 <w@1wt.eu>
+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]);

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 <w@1wt.eu>
+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 <cfaulet@haproxy.com>
+
+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;

net/haproxy/patches/002-MINOR-tcp-avoid-confusion-in-time-parsing-init.patch

@@ -0,0 +1,33 @@
+commit 74a1e4393f7a7b194abb4f428fd02c7c088f6c67
+Author: William Dauchy <w.dauchy@criteo.com>
+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 <w.dauchy@criteo.com>
+    
+    [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 <cfaulet@haproxy.com>
+
+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);

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 <wlallemand@haproxy.com>
+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 <cfaulet@haproxy.com>
+
+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;
+ }
+ 

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 <cfaulet@haproxy.com>
+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 <cfaulet@haproxy.com>
+
+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) {

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 <cfaulet@haproxy.com>
+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 <cfaulet@haproxy.com>
+
+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;
+ }