upstream commit

Make remote channel ID a u_int

Previously we tracked the remote channel IDs in an int, but this is
strictly incorrect: the wire protocol uses uint32 and there is nothing
in-principle stopping a SSH implementation from sending, say, 0xffff0000.

In practice everyone numbers their channels sequentially, so this has
never been a problem.

ok markus@

Upstream-ID: b9f4cd3dc53155b4a5c995c0adba7da760d03e73

6 files changed, 65 insertions, 24 deletions

diff --git a/channels.c b/channels.c
index 935625c74..3ab4823a9 100644
--- a/channels.c
+++ b/channels.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: channels.c,v 1.366 2017/08/30 03:59:08 djm Exp $ */
+/* $OpenBSD: channels.c,v 1.368 2017/09/12 06:35:31 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -252,14 +252,14 @@ channel_by_id(struct ssh *ssh, int id)
 }
 
 Channel *
-channel_by_remote_id(struct ssh *ssh, int remote_id)
+channel_by_remote_id(struct ssh *ssh, u_int remote_id)
 {
 	Channel *c;
 	u_int i;
 
 	for (i = 0; i < ssh->chanctxt->channels_alloc; i++) {
 		c = ssh->chanctxt->channels[i];
-		if (c != NULL && c->remote_id == remote_id)
+		if (c != NULL && c->have_remote_id && c->remote_id == remote_id)
 			return c;
 	}
 	return NULL;
@@ -387,7 +387,6 @@ channel_new(struct ssh *ssh, char *ctype, int type, int rfd, int wfd, int efd,
 	c->local_window = window;
 	c->local_window_max = window;
 	c->local_maxpacket = maxpack;
-	c->remote_id = -1;
 	c->remote_name = xstrdup(remote_name);
 	c->ctl_chan = -1;
 	c->delayed = 1;		/* prevent call to channel_post handler */
@@ -703,7 +702,7 @@ channel_find_open(struct ssh *ssh)
 
 	for (i = 0; i < ssh->chanctxt->channels_alloc; i++) {
 		c = ssh->chanctxt->channels[i];
-		if (c == NULL || c->remote_id < 0)
+		if (c == NULL || !c->have_remote_id)
 			continue;
 		switch (c->type) {
 		case SSH_CHANNEL_CLOSED:
@@ -778,9 +777,10 @@ channel_open_message(struct ssh *ssh)
 		case SSH_CHANNEL_MUX_PROXY:
 		case SSH_CHANNEL_MUX_CLIENT:
 			if ((r = sshbuf_putf(buf, "  #%d %.300s "
-			    "(t%d r%d i%u/%zu o%u/%zu fd %d/%d cc %d)\r\n",
+			    "(t%d %s%u i%u/%zu o%u/%zu fd %d/%d cc %d)\r\n",
 			    c->self, c->remote_name,
-			    c->type, c->remote_id,
+			    c->type,
+			    c->have_remote_id ? "r" : "nr", c->remote_id,
 			    c->istate, sshbuf_len(c->input),
 			    c->ostate, sshbuf_len(c->output),
 			    c->rfd, c->wfd, c->ctl_chan)) != 0)
@@ -838,6 +838,9 @@ channel_request_start(struct ssh *ssh, int id, char *service, int wantconfirm)
 		logit("%s: %d: unknown channel id", __func__, id);
 		return;
 	}
+	if (!c->have_remote_id)
+		fatal(":%s: channel %d: no remote id", __func__, c->self);
+
 	debug2("channel %d: request %s confirm %d", id, service, wantconfirm);
 	if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_REQUEST)) != 0 ||
 	    (r = sshpkt_put_u32(ssh, c->remote_id)) != 0 ||
@@ -930,6 +933,9 @@ channel_set_fds(struct ssh *ssh, int id, int rfd, int wfd, int efd,
 
 	if (c == NULL || c->type != SSH_CHANNEL_LARVAL)
 		fatal("channel_activate for non-larval channel %d.", id);
+	if (!c->have_remote_id)
+		fatal(":%s: channel %d: no remote id", __func__, c->self);
+
 	channel_register_fds(ssh, c, rfd, wfd, efd, extusage, nonblock, is_tty);
 	c->type = SSH_CHANNEL_OPEN;
 	c->local_window = c->local_window_max = window_max;
@@ -1698,6 +1704,8 @@ channel_post_connecting(struct ssh *ssh, Channel *c,
 
 	if (!FD_ISSET(c->sock, writeset))
 		return;
+	if (!c->have_remote_id)
+		fatal(":%s: channel %d: no remote id", __func__, c->self);
 
 	if (getsockopt(c->sock, SOL_SOCKET, SO_ERROR, &err, &sz) < 0) {
 		err = errno;
@@ -1983,6 +1991,9 @@ channel_check_window(struct ssh *ssh, Channel *c)
 	    c->local_maxpacket*3) ||
 	    c->local_window < c->local_window_max/2) &&
 	    c->local_consumed > 0) {
+		if (!c->have_remote_id)
+			fatal(":%s: channel %d: no remote id",
+			    __func__, c->self);
 		if ((r = sshpkt_start(ssh,
 		    SSH2_MSG_CHANNEL_WINDOW_ADJUST)) != 0 ||
 		    (r = sshpkt_put_u32(ssh, c->remote_id)) != 0 ||
@@ -2338,6 +2349,9 @@ channel_output_poll_input_open(struct ssh *ssh, Channel *c)
 		return;
 	}
 
+	if (!c->have_remote_id)
+		fatal(":%s: channel %d: no remote id", __func__, c->self);
+
 	if (c->datagram) {
 		/* Check datagram will fit; drop if not */
 		if ((r = sshbuf_peek_string_direct(c->input, NULL, &dlen)) != 0)
@@ -2404,6 +2418,8 @@ channel_output_poll_extended_read(struct ssh *ssh, Channel *c)
 		len = c->remote_maxpacket;
 	if (len == 0)
 		return;
+	if (!c->have_remote_id)
+		fatal(":%s: channel %d: no remote id", __func__, c->self);
 	if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_EXTENDED_DATA)) != 0 ||
 	    (r = sshpkt_put_u32(ssh, c->remote_id)) != 0 ||
 	    (r = sshpkt_put_u32(ssh, SSH2_EXTENDED_DATA_STDERR)) != 0 ||
@@ -2570,6 +2586,7 @@ channel_proxy_downstream(struct ssh *ssh, Channel *downstream)
 		c->mux_ctx = downstream;	/* point to mux client */
 		c->mux_downstream_id = id;
 		c->remote_id = remote_id;
+		c->have_remote_id = 1;
 		if ((r = sshbuf_put_u32(modified, remote_id)) != 0 ||
 		    (r = sshbuf_put_u32(modified, c->self)) != 0 ||
 		    (r = sshbuf_putb(modified, original)) != 0) {
@@ -2713,8 +2730,10 @@ channel_proxy_upstream(Channel *c, int type, u_int32_t seq, struct ssh *ssh)
 	switch (type) {
 	case SSH2_MSG_CHANNEL_OPEN_CONFIRMATION:
 		/* record remote_id for SSH2_MSG_CHANNEL_CLOSE */
-		if (cp && len > 4)
+		if (cp && len > 4) {
 			c->remote_id = PEEK_U32(cp);
+			c->have_remote_id = 1;
+		}
 		break;
 	case SSH2_MSG_CHANNEL_CLOSE:
 		if (c->flags & CHAN_CLOSE_SENT)
@@ -2923,14 +2942,15 @@ channel_input_open_confirmation(int type, u_int32_t seq, struct ssh *ssh)
 	 * Record the remote channel number and mark that the channel
 	 * is now open.
 	 */
-	c->remote_id = channel_parse_id(ssh, __func__, "open confirmation");
-	if ((r = sshpkt_get_u32(ssh, &remote_window)) != 0 ||
+	if ((r = sshpkt_get_u32(ssh, &c->remote_id)) != 0 ||
+	    (r = sshpkt_get_u32(ssh, &remote_window)) != 0 ||
 	    (r = sshpkt_get_u32(ssh, &remote_maxpacket)) != 0) {
 		error("%s: window/maxpacket: %s", __func__, ssh_err(r));
 		packet_disconnect("Invalid open confirmation message");
 	}
 	ssh_packet_check_eom(ssh);
 
+	c->have_remote_id = 1;
 	c->remote_window = remote_window;
 	c->remote_maxpacket = remote_maxpacket;
 	c->type = SSH_CHANNEL_OPEN;
diff --git a/channels.h b/channels.h
index f04c43afa..d1cf5dc6a 100644
--- a/channels.h
+++ b/channels.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: channels.h,v 1.128 2017/09/12 06:32:07 djm Exp $ */
+/* $OpenBSD: channels.h,v 1.129 2017/09/12 06:35:32 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -97,8 +97,9 @@ typedef int mux_callback_fn(struct ssh *, struct Channel *);
 struct Channel {
 	int     type;		/* channel type/state */
 	int     self;		/* my own channel identifier */
-	int     remote_id;	/* channel identifier for remote peer */
-				/* XXX should be uint32_t */
+	uint32_t remote_id;	/* channel identifier for remote peer */
+	int	have_remote_id;	/* non-zero if remote_id is valid */
+
 	u_int   istate;		/* input from channel (state of receive half) */
 	u_int   ostate;		/* output to channel  (state of transmit half) */
 	int     flags;		/* close sent/rcvd */
@@ -222,7 +223,7 @@ void channel_init_channels(struct ssh *ssh);
 /* channel management */
 
 Channel	*channel_by_id(struct ssh *, int);
-Channel	*channel_by_remote_id(struct ssh *, int);
+Channel	*channel_by_remote_id(struct ssh *, u_int);
 Channel	*channel_lookup(struct ssh *, int);
 Channel *channel_new(struct ssh *, char *, int, int, int, int,
 	    u_int, u_int, int, char *, int);
diff --git a/clientloop.c b/clientloop.c
index 1218b3b71..829eae024 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.303 2017/09/12 06:32:07 djm Exp $ */
+/* $OpenBSD: clientloop.c,v 1.304 2017/09/12 06:35:32 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1682,6 +1682,7 @@ client_input_channel_open(int type, u_int32_t seq, struct ssh *ssh)
 	} else if (c != NULL) {
 		debug("confirm %s", ctype);
 		c->remote_id = rchan;
+		c->have_remote_id = 1;
 		c->remote_window = rwindow;
 		c->remote_maxpacket = rmaxpack;
 		if (c->type != SSH_CHANNEL_CONNECTING) {
@@ -1749,6 +1750,9 @@ client_input_channel_req(int type, u_int32_t seq, struct ssh *ssh)
 		packet_check_eom();
 	}
 	if (reply && c != NULL && !(c->flags & CHAN_CLOSE_SENT)) {
+		if (!c->have_remote_id)
+			fatal("%s: channel %d: no remote_id",
+			    __func__, c->self);
 		packet_start(success ?
 		    SSH2_MSG_CHANNEL_SUCCESS : SSH2_MSG_CHANNEL_FAILURE);
 		packet_put_int(c->remote_id);
diff --git a/mux.c b/mux.c
index 9eee287b9..f8510426d 100644
--- a/mux.c
+++ b/mux.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: mux.c,v 1.66 2017/09/12 06:32:07 djm Exp $ */
+/* $OpenBSD: mux.c,v 1.67 2017/09/12 06:35:32 djm Exp $ */
 /*
  * Copyright (c) 2002-2008 Damien Miller <djm@openbsd.org>
  *
@@ -215,7 +215,8 @@ mux_master_session_cleanup_cb(struct ssh *ssh, int cid, void *unused)
 			fatal("%s: channel %d missing control channel %d",
 			    __func__, c->self, c->ctl_chan);
 		c->ctl_chan = -1;
-		cc->remote_id = -1;
+		cc->remote_id = 0;
+		cc->have_remote_id = 0;
 		chan_rcvd_oclose(ssh, cc);
 	}
 	channel_cancel_cleanup(ssh, c->self);
@@ -231,11 +232,12 @@ mux_master_control_cleanup_cb(struct ssh *ssh, int cid, void *unused)
 	debug3("%s: entering for channel %d", __func__, cid);
 	if (c == NULL)
 		fatal("%s: channel_by_id(%i) == NULL", __func__, cid);
-	if (c->remote_id != -1) {
+	if (c->have_remote_id) {
 		if ((sc = channel_by_id(ssh, c->remote_id)) == NULL)
-			fatal("%s: channel %d missing session channel %d",
+			fatal("%s: channel %d missing session channel %u",
 			    __func__, c->self, c->remote_id);
-		c->remote_id = -1;
+		c->remote_id = 0;
+		c->have_remote_id = 0;
 		sc->ctl_chan = -1;
 		if (sc->type != SSH_CHANNEL_OPEN &&
 		    sc->type != SSH_CHANNEL_OPENING) {
@@ -413,7 +415,7 @@ process_mux_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->remote_id != -1) {
+	if (c->have_remote_id) {
 		debug2("%s: session already open", __func__);
 		/* prepare reply */
 		buffer_put_int(r, MUX_S_FAILURE);
@@ -471,6 +473,7 @@ process_mux_new_session(struct ssh *ssh, u_int rid,
 
 	nc->ctl_chan = c->self;		/* link session -> control channel */
 	c->remote_id = nc->self; 	/* link control -> session channel */
+	c->have_remote_id = 1;
 
 	if (cctx->want_tty && escape_char != 0xffffffff) {
 		channel_register_filter(ssh, nc->self,
@@ -1007,7 +1010,7 @@ process_mux_stdio_fwd(struct ssh *ssh, u_int rid,
 	    new_fd[0], new_fd[1]);
 
 	/* XXX support multiple child sessions in future */
-	if (c->remote_id != -1) {
+	if (c->have_remote_id) {
 		debug2("%s: session already open", __func__);
 		/* prepare reply */
 		buffer_put_int(r, MUX_S_FAILURE);
@@ -1043,6 +1046,7 @@ process_mux_stdio_fwd(struct ssh *ssh, u_int rid,
 
 	nc->ctl_chan = c->self;		/* link session -> control channel */
 	c->remote_id = nc->self; 	/* link control -> session channel */
+	c->have_remote_id = 1;
 
 	debug2("%s: channel_new: %d linked to control channel %d",
 	    __func__, nc->self, nc->ctl_chan);
diff --git a/nchan.c b/nchan.c
index 74c855c90..24929556d 100644
--- a/nchan.c
+++ b/nchan.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: nchan.c,v 1.66 2017/09/12 06:32:07 djm Exp $ */
+/* $OpenBSD: nchan.c,v 1.67 2017/09/12 06:35:32 djm Exp $ */
 /*
  * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl.  All rights reserved.
  *
@@ -183,6 +183,9 @@ chan_send_eof2(struct ssh *ssh, Channel *c)
 	debug2("channel %d: send eof", c->self);
 	switch (c->istate) {
 	case CHAN_INPUT_WAIT_DRAIN:
+		if (!c->have_remote_id)
+			fatal("%s: channel %d: no remote_id",
+			    __func__, c->self);
 		if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_EOF)) != 0 ||
 		    (r = sshpkt_put_u32(ssh, c->remote_id)) != 0 ||
 		    (r = sshpkt_send(ssh)) != 0)
@@ -209,6 +212,9 @@ chan_send_close2(struct ssh *ssh, Channel *c)
 	} else if (c->flags & CHAN_CLOSE_SENT) {
 		error("channel %d: already sent close", c->self);
 	} else {
+		if (!c->have_remote_id)
+			fatal("%s: channel %d: no remote_id",
+			    __func__, c->self);
 		if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_CLOSE)) != 0 ||
 		    (r = sshpkt_put_u32(ssh, c->remote_id)) != 0 ||
 		    (r = sshpkt_send(ssh)) != 0)
@@ -230,6 +236,8 @@ chan_send_eow2(struct ssh *ssh, Channel *c)
 	}
 	if (!(datafellows & SSH_NEW_OPENSSH))
 		return;
+	if (!c->have_remote_id)
+		fatal("%s: channel %d: no remote_id", __func__, c->self);
 	if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_REQUEST)) != 0 ||
 	    (r = sshpkt_put_u32(ssh, c->remote_id)) != 0 ||
 	    (r = sshpkt_put_cstring(ssh, "eow@openssh.com")) != 0 ||
diff --git a/serverloop.c b/serverloop.c
index 567159410..ae75fc2ec 100644
--- a/serverloop.c
+++ b/serverloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: serverloop.c,v 1.197 2017/09/12 06:32:07 djm Exp $ */
+/* $OpenBSD: serverloop.c,v 1.198 2017/09/12 06:35:32 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -619,6 +619,7 @@ server_input_channel_open(int type, u_int32_t seq, struct ssh *ssh)
 	if (c != NULL) {
 		debug("server_input_channel_open: confirm %s", ctype);
 		c->remote_id = rchan;
+		c->have_remote_id = 1;
 		c->remote_window = rwindow;
 		c->remote_maxpacket = rmaxpack;
 		if (c->type != SSH_CHANNEL_CONNECTING) {
@@ -844,6 +845,9 @@ server_input_channel_req(int type, u_int32_t seq, struct ssh *ssh)
 	    c->type == SSH_CHANNEL_OPEN) && strcmp(c->ctype, "session") == 0)
 		success = session_input_channel_req(ssh, c, rtype);
 	if (reply && !(c->flags & CHAN_CLOSE_SENT)) {
+		if (!c->have_remote_id)
+			fatal("%s: channel %d: no remote_id",
+			    __func__, c->self);
 		packet_start(success ?
 		    SSH2_MSG_CHANNEL_SUCCESS : SSH2_MSG_CHANNEL_FAILURE);
 		packet_put_int(c->remote_id);