RepoMirrors
/
openssh
mirror of git://anongit.mindrot.org/openssh.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.

ok markus@

OpenBSD-Commit-ID: c9f474e0124e3fe456c5e43749b97d75e65b82b2
This commit is contained in:
djm@openbsd.org 2024-07-25 22:40:08 +00:00 committed by Damien Miller
parent 53d1d30743
commit 29fb6f6d46
No known key found for this signature in database
4 changed files with 26 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
channels.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: channels.c,v 1.438 2024/05/17 00:30:23 djm Exp $ */ /* $OpenBSD: channels.c,v 1.439 2024/07/25 22:40:08 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -1016,14 +1016,16 @@ channel_format_status(const Channel *c)
{ {
char *ret = NULL; char *ret = NULL;
xasprintf(&ret, "t%d [%s] %s%u i%u/%zu o%u/%zu e[%s]/%zu " 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 io 0x%02x/0x%02x", "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->type, c->xctype != NULL ? c->xctype : c->ctype,
c->have_remote_id ? "r" : "nr", c->remote_id, 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->istate, sshbuf_len(c->input),
c->ostate, sshbuf_len(c->output), c->ostate, sshbuf_len(c->output),
channel_format_extended_usage(c), sshbuf_len(c->extended), channel_format_extended_usage(c), sshbuf_len(c->extended),
c->rfd, c->wfd, c->efd, c->sock, c->ctl_chan, 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); c->io_want, c->io_ready);
return ret; return ret;
} }

4
channels.h
View File

@ -1,4 +1,4 @@
/* $OpenBSD: channels.h,v 1.156 2024/05/23 23:47:16 jsg Exp $ */ /* $OpenBSD: channels.h,v 1.157 2024/07/25 22:40:08 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
@ -139,6 +139,8 @@ struct Channel {
u_int io_ready; /* bitmask of SSH_CHAN_IO_* */ u_int io_ready; /* bitmask of SSH_CHAN_IO_* */
int pfds[4]; /* pollfd entries for rfd/wfd/efd/sock */ int pfds[4]; /* pollfd entries for rfd/wfd/efd/sock */
int ctl_chan; /* control channel (multiplexed connections) */ 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 */ int isatty; /* rfd is a tty */
#ifdef _AIX #ifdef _AIX
int wfd_isatty; /* wfd is a tty */ int wfd_isatty; /* wfd is a tty */

28
mux.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: mux.c,v 1.101 2023/11/23 03:37:05 dtucker Exp $ */ /* $OpenBSD: mux.c,v 1.102 2024/07/25 22:40:08 djm Exp $ */
/* /*
* Copyright (c) 2002-2008 Damien Miller <djm@openbsd.org> * Copyright (c) 2002-2008 Damien Miller <djm@openbsd.org>
* *
@ -201,8 +201,8 @@ mux_master_session_cleanup_cb(struct ssh *ssh, int cid, int force, void *unused)
fatal_f("channel %d missing control channel %d", fatal_f("channel %d missing control channel %d",
c->self, c->ctl_chan); c->self, c->ctl_chan);
c->ctl_chan = -1; c->ctl_chan = -1;
cc->remote_id = 0; cc->ctl_child_id = 0;
cc->have_remote_id = 0; cc->have_ctl_child_id = 0;
chan_rcvd_oclose(ssh, cc); chan_rcvd_oclose(ssh, cc);
} }
channel_cancel_cleanup(ssh, c->self); channel_cancel_cleanup(ssh, c->self);
@ -217,12 +217,12 @@ mux_master_control_cleanup_cb(struct ssh *ssh, int cid, int force, void *unused)
debug3_f("entering for channel %d", cid); debug3_f("entering for channel %d", cid);
if (c == NULL) if (c == NULL)
fatal_f("channel_by_id(%i) == NULL", cid); fatal_f("channel_by_id(%i) == NULL", cid);
if (c->have_remote_id) { if (c->have_ctl_child_id) {
if ((sc = channel_by_id(ssh, c->remote_id)) == NULL) if ((sc = channel_by_id(ssh, c->ctl_child_id)) == NULL)
fatal_f("channel %d missing session channel %u", fatal_f("channel %d missing session channel %u",
c->self, c->remote_id); c->self, c->ctl_child_id);
c->remote_id = 0; c->ctl_child_id = 0;
c->have_remote_id = 0; c->have_ctl_child_id = 0;
sc->ctl_chan = -1; sc->ctl_chan = -1;
if (sc->type != SSH_CHANNEL_OPEN && if (sc->type != SSH_CHANNEL_OPEN &&
sc->type != SSH_CHANNEL_OPENING) { sc->type != SSH_CHANNEL_OPENING) {
@ -418,7 +418,7 @@ mux_master_process_new_session(struct ssh *ssh, u_int rid,
new_fd[0], new_fd[1], new_fd[2]); new_fd[0], new_fd[1], new_fd[2]);
/* XXX support multiple child sessions in future */ /* XXX support multiple child sessions in future */
if (c->have_remote_id) { if (c->have_ctl_child_id) {
debug2_f("session already open"); debug2_f("session already open");
reply_error(reply, MUX_S_FAILURE, rid, reply_error(reply, MUX_S_FAILURE, rid,
"Multiple sessions not supported"); "Multiple sessions not supported");
@ -463,8 +463,8 @@ mux_master_process_new_session(struct ssh *ssh, u_int rid,
CHAN_EXTENDED_WRITE, "client-session", CHANNEL_NONBLOCK_STDIO); CHAN_EXTENDED_WRITE, "client-session", CHANNEL_NONBLOCK_STDIO);
nc->ctl_chan = c->self; /* link session -> control channel */ nc->ctl_chan = c->self; /* link session -> control channel */
c->remote_id = nc->self; /* link control -> session channel */ c->ctl_child_id = nc->self; /* link control -> session channel */
c->have_remote_id = 1; c->have_ctl_child_id = 1;
if (cctx->want_tty && escape_char != 0xffffffff) { if (cctx->want_tty && escape_char != 0xffffffff) {
channel_register_filter(ssh, nc->self, channel_register_filter(ssh, nc->self,
@ -1003,7 +1003,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]); debug3_f("got fds stdin %d, stdout %d", new_fd[0], new_fd[1]);
/* XXX support multiple child sessions in future */ /* XXX support multiple child sessions in future */
if (c->have_remote_id) { if (c->have_ctl_child_id) {
debug2_f("session already open"); debug2_f("session already open");
reply_error(reply, MUX_S_FAILURE, rid, reply_error(reply, MUX_S_FAILURE, rid,
"Multiple sessions not supported"); "Multiple sessions not supported");
@ -1035,8 +1035,8 @@ mux_master_process_stdio_fwd(struct ssh *ssh, u_int rid,
free(chost); free(chost);
nc->ctl_chan = c->self; /* link session -> control channel */ nc->ctl_chan = c->self; /* link session -> control channel */
c->remote_id = nc->self; /* link control -> session channel */ c->ctl_child_id = nc->self; /* link control -> session channel */
c->have_remote_id = 1; c->have_ctl_child_id = 1;
debug2_f("channel_new: %d control %d", nc->self, nc->ctl_chan); debug2_f("channel_new: %d control %d", nc->self, nc->ctl_chan);

6
nchan.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: nchan.c,v 1.75 2024/02/01 02:37:33 djm Exp $ */ /* $OpenBSD: nchan.c,v 1.76 2024/07/25 22:40:08 djm Exp $ */
/* /*
* Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl. All rights reserved. * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl. All rights reserved.
* *
@ -208,7 +208,7 @@ chan_send_close2(struct ssh *ssh, Channel *c)
{ {
int r; int r;
debug2("channel %d: send close", c->self); debug2("channel %d: send_close2", c->self);
if (c->ostate != CHAN_OUTPUT_CLOSED || if (c->ostate != CHAN_OUTPUT_CLOSED ||
c->istate != CHAN_INPUT_CLOSED) { c->istate != CHAN_INPUT_CLOSED) {
error("channel %d: cannot send close for istate/ostate %d/%d", error("channel %d: cannot send close for istate/ostate %d/%d",
@ -218,6 +218,8 @@ chan_send_close2(struct ssh *ssh, Channel *c)
} else { } else {
if (!c->have_remote_id) if (!c->have_remote_id)
fatal_f("channel %d: no remote_id", c->self); 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 || if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_CLOSE)) != 0 ||
(r = sshpkt_put_u32(ssh, c->remote_id)) != 0 || (r = sshpkt_put_u32(ssh, c->remote_id)) != 0 ||
(r = sshpkt_send(ssh)) != 0) (r = sshpkt_send(ssh)) != 0)