From 29fb6f6d46b67770084b4f12bcf8a01bd535041b Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Thu, 25 Jul 2024 22:40:08 +0000
Subject: [PATCH] upstream: Fix proxy multiplexing (-O proxy) bug
 
 If a mux started with ControlPersist then later has a forwarding added using
 mux proxy connection and the forwarding was used, then when the mux proxy
 session terminates, the mux master process will send a channel close to the
 server with a bad channel ID and crash the connection.
 
 This was caused by my stupidly reusing c->remote_id for mux channel
 associations when I should have just added another member to struct channel.

OpenBSD-Commit-ID: c9f474e0124e3fe456c5e43749b97d75e65b82b2
Reference:https://anongit.mindrot.org/openssh.git/commit/29fb6f6d46b67770084b4f12bcf8a01bd535041b
Conflict:NA
---
 channels.c |  6 ++++--
 channels.h |  2 ++
 mux.c      | 26 +++++++++++++-------------
 nchan.c    |  4 +++-
 4 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/channels.c b/channels.c
index 2c0aa65..02f5441 100644
--- a/channels.c
+++ b/channels.c
@@ -964,14 +964,16 @@ channel_format_status(const Channel *c)
 {
 	char *ret = NULL;
 
-	xasprintf(&ret, "t%d [%s] %s%u i%u/%zu o%u/%zu e[%s]/%zu "
-	    "fd %d/%d/%d sock %d cc %d io 0x%02x/0x%02x",
+	xasprintf(&ret, "t%d [%s] %s%u %s%u i%u/%zu o%u/%zu e[%s]/%zu "
+	    "fd %d/%d/%d sock %d cc %d %s%u io 0x%02x/0x%02x",
 	    c->type, c->xctype != NULL ? c->xctype : c->ctype,
 	    c->have_remote_id ? "r" : "nr", c->remote_id,
+	    c->mux_ctx != NULL ? "m" : "nm", c->mux_downstream_id,
 	    c->istate, sshbuf_len(c->input),
 	    c->ostate, sshbuf_len(c->output),
 	    channel_format_extended_usage(c), sshbuf_len(c->extended),
 	    c->rfd, c->wfd, c->efd, c->sock, c->ctl_chan,
+	    c->have_ctl_child_id ? "c" : "nc", c->ctl_child_id,
 	    c->io_want, c->io_ready);
 	return ret;
 }
diff --git a/channels.h b/channels.h
index 7e59914..f3dd87d 100644
--- a/channels.h
+++ b/channels.h
@@ -140,6 +140,8 @@ struct Channel {
 	u_int	io_ready;	/* bitmask of SSH_CHAN_IO_* */
 	int	pfds[4];	/* pollfd entries for rfd/wfd/efd/sock */
 	int     ctl_chan;	/* control channel (multiplexed connections) */
+	uint32_t ctl_child_id;	/* child session for mux controllers */
+	int	have_ctl_child_id;	/* non-zero if ctl_child_id is valid */
 	int     isatty;		/* rfd is a tty */
 #ifdef _AIX
 	int     wfd_isatty;	/* wfd is a tty */
diff --git a/mux.c b/mux.c
index b3ffde9..f391892 100644
--- a/mux.c
+++ b/mux.c
@@ -199,8 +199,8 @@ mux_master_session_cleanup_cb(struct ssh *ssh, int cid, int force, void *unused)
 			fatal_f("channel %d missing control channel %d",
 			    c->self, c->ctl_chan);
 		c->ctl_chan = -1;
-		cc->remote_id = 0;
-		cc->have_remote_id = 0;
+		cc->ctl_child_id = 0;
+		cc->have_ctl_child_id = 0;
 		chan_rcvd_oclose(ssh, cc);
 	}
 	channel_cancel_cleanup(ssh, c->self);
@@ -215,12 +215,12 @@ mux_master_control_cleanup_cb(struct ssh *ssh, int cid, int force, void *unused)
 	debug3_f("entering for channel %d", cid);
 	if (c == NULL)
 		fatal_f("channel_by_id(%i) == NULL", cid);
-	if (c->have_remote_id) {
-		if ((sc = channel_by_id(ssh, c->remote_id)) == NULL)
+	if (c->have_ctl_child_id) {
+		if ((sc = channel_by_id(ssh, c->ctl_child_id)) == NULL)
 			fatal_f("channel %d missing session channel %u",
-			    c->self, c->remote_id);
-		c->remote_id = 0;
-		c->have_remote_id = 0;
+			    c->self, c->ctl_child_id);
+		c->ctl_child_id = 0;
+		c->have_ctl_child_id = 0;
 		sc->ctl_chan = -1;
 		if (sc->type != SSH_CHANNEL_OPEN &&
 		    sc->type != SSH_CHANNEL_OPENING) {
@@ -416,7 +416,7 @@ mux_master_process_new_session(struct ssh *ssh, u_int rid,
 	    new_fd[0], new_fd[1], new_fd[2]);
 
 	/* XXX support multiple child sessions in future */
-	if (c->have_remote_id) {
+	if (c->have_ctl_child_id) {
 		debug2_f("session already open");
 		reply_error(reply, MUX_S_FAILURE, rid,
 		    "Multiple sessions not supported");
@@ -461,8 +461,8 @@ mux_master_process_new_session(struct ssh *ssh, u_int rid,
 	    CHAN_EXTENDED_WRITE, "client-session", CHANNEL_NONBLOCK_STDIO);
 
 	nc->ctl_chan = c->self;		/* link session -> control channel */
-	c->remote_id = nc->self;	/* link control -> session channel */
-	c->have_remote_id = 1;
+	c->ctl_child_id = nc->self;	/* link control -> session channel */
+	c->have_ctl_child_id = 1;
 
 	if (cctx->want_tty && escape_char != 0xffffffff) {
 		channel_register_filter(ssh, nc->self,
@@ -992,7 +992,7 @@ mux_master_process_stdio_fwd(struct ssh *ssh, u_int rid,
 	debug3_f("got fds stdin %d, stdout %d", new_fd[0], new_fd[1]);
 
 	/* XXX support multiple child sessions in future */
-	if (c->have_remote_id) {
+	if (c->have_ctl_child_id) {
 		debug2_f("session already open");
 		reply_error(reply, MUX_S_FAILURE, rid,
 		    "Multiple sessions not supported");
@@ -1019,8 +1019,8 @@ mux_master_process_stdio_fwd(struct ssh *ssh, u_int rid,
 	free(chost);
 
 	nc->ctl_chan = c->self;		/* link session -> control channel */
-	c->remote_id = nc->self;	/* link control -> session channel */
-	c->have_remote_id = 1;
+	c->ctl_child_id = nc->self;	/* link control -> session channel */
+	c->have_ctl_child_id = 1;
 
 	debug2_f("channel_new: %d control %d", nc->self, nc->ctl_chan);
 
diff --git a/nchan.c b/nchan.c
index d33426f..715feeb 100644
--- a/nchan.c
+++ b/nchan.c
@@ -208,7 +208,7 @@ chan_send_close2(struct ssh *ssh, Channel *c)
 {
 	int r;
 
-	debug2("channel %d: send close", c->self);
+	debug2("channel %d: send close2", c->self);
 	if (c->ostate != CHAN_OUTPUT_CLOSED ||
 	    c->istate != CHAN_INPUT_CLOSED) {
 		error("channel %d: cannot send close for istate/ostate %d/%d",
@@ -218,6 +218,8 @@ chan_send_close2(struct ssh *ssh, Channel *c)
 	} else {
 		if (!c->have_remote_id)
 			fatal_f("channel %d: no remote_id", c->self);
+		debug2("channel %d: send close for remote id %u", c->self,
+		    c->remote_id);
 		if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_CLOSE)) != 0 ||
 		    (r = sshpkt_put_u32(ssh, c->remote_id)) != 0 ||
 		    (r = sshpkt_send(ssh)) != 0)
-- 
2.43.0