From b68f77d62602ba3d047ff715804001a77ed89feb Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfaulet@haproxy.com>
Date: Thu, 16 Jun 2022 16:24:16 +0200
Subject: [PATCH] BUG/MEDIUM: stream: Properly handle destructive client
 connection upgrades

When the protocol is changed for a client connection at the stream level
(from TCP to H1/H2), there are two cases. The stream may be reused or
not. The first case, when the stream is reused is working. The second one is
buggy since the conn-stream refactoring and leads to a crash.

In this case, the new mux don't reuse the stream. It must be silently
aborted. However, its front stream connector is still referencing the
connection. So it must be detached. But it must be performed in two stages,
to be sure to not loose the context for the upgrade and to be able to
rollback on error. So now, before the upgrade, we prepare to detach the
stconn and it is finally detached if the upgrade succeeds. There is a trick
here. Because we pretend the stconn is detached but its state is preserved.

This patch must be backported to 2.6.
---
 include/haproxy/stconn.h |  4 ++++
 src/stconn.c             | 32 ++++++++++++++++++++++++++++++++
 src/stream.c             |  7 ++++++-
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/include/haproxy/stconn.h b/include/haproxy/stconn.h
index 9c7be5032b..ff0de4354d 100644
--- a/include/haproxy/stconn.h
+++ b/include/haproxy/stconn.h
@@ -51,6 +51,10 @@ int sc_reset_endp(struct stconn *sc);
 
 struct appctx *sc_applet_create(struct stconn *sc, struct applet *app);
 
+void sc_conn_prepare_endp_upgrade(struct stconn *sc);
+void sc_conn_abort_endp_upgrade(struct stconn *sc);
+void sc_conn_commit_endp_upgrade(struct stconn *sc);
+
 /* The se_fl_*() set of functions manipulate the stream endpoint flags from
  * the stream endpoint itself. The sc_ep_*() set of functions manipulate the
  * stream endpoint flags from the the stream connector (ex. stconn).
diff --git a/src/stconn.c b/src/stconn.c
index 5d4ed3ed47..02a8dc452d 100644
--- a/src/stconn.c
+++ b/src/stconn.c
@@ -1943,3 +1943,35 @@ static int sc_applet_process(struct stconn *sc)
 		appctx_wakeup(__sc_appctx(sc));
 	return 0;
 }
+
+
+/* Prepares an endpoint upgrade. We don't now at this stage if the upgrade will
+ * succeed or not and if the stconn will be reused by the new endpoint. Thus,
+ * for now, only pretend the stconn is detached.
+ */
+void sc_conn_prepare_endp_upgrade(struct stconn *sc)
+{
+	BUG_ON(!sc_conn(sc) || !sc->app);
+	sc_ep_clr(sc, SE_FL_T_MUX);
+	sc_ep_set(sc, SE_FL_DETACHED);
+}
+
+/* Endpoint upgrade failed. Retore the stconn state. */
+void sc_conn_abort_endp_upgrade(struct stconn *sc)
+{
+	sc_ep_set(sc, SE_FL_T_MUX);
+	sc_ep_clr(sc, SE_FL_DETACHED);
+}
+
+/* Commit the endpoint upgrade. If stconn is attached, it means the new endpoint
+ * use it. So we do nothing. Otherwise, the stconn will be destroy with the
+ * overlying stream. So, it means we must commit the detach.
+*/
+void sc_conn_commit_endp_upgrade(struct stconn *sc)
+{
+	if (!sc_ep_test(sc, SE_FL_DETACHED))
+		return;
+	sc_detach_endp(&sc);
+	/* Because it was already set as detached, the sedesc must be preserved */
+	BUG_ON(!sc->sedesc);
+}
diff --git a/src/stream.c b/src/stream.c
index f3f776db62..2f698482f3 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -1485,10 +1485,15 @@ int stream_set_http_mode(struct stream *s, const struct mux_proto_list *mux_prot
 
 		if (conn->mux->flags & MX_FL_NO_UPG)
 			return 0;
+
+		sc_conn_prepare_endp_upgrade(sc);
 		if (conn_upgrade_mux_fe(conn, sc, &s->req.buf,
 					(mux_proto ? mux_proto->token : ist("")),
-					PROTO_MODE_HTTP)  == -1)
+					PROTO_MODE_HTTP)  == -1) {
+			sc_conn_abort_endp_upgrade(sc);
 			return 0;
+		}
+		sc_conn_commit_endp_upgrade(sc);
 
 		s->req.flags &= ~(CF_READ_PARTIAL|CF_AUTO_CONNECT);
 		s->req.total = 0;