diff --git a/ChangeLog b/ChangeLog index b90af22b0..a485000a8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,6 +11,18 @@ [ssh-add.c] check that the certificate matches the corresponding private key before grafting it on + - djm@cvs.openbsd.org 2010/05/14 23:29:23 + [channels.c channels.h mux.c ssh.c] + Pause the mux channel while waiting for reply from aynch callbacks. + Prevents misordering of replies if new requests arrive while waiting. + + Extend channel open confirm callback to allow signalling failure + conditions as well as success. Use this to 1) fix a memory leak, 2) + start using the above pause mechanism and 3) delay sending a success/ + failure message on mux slave session open until we receive a reply from + the server. + + motivated by and with feedback from markus@ 20100511 - (dtucker) [Makefile.in] Bug #1770: Link libopenbsd-compat twice to solve diff --git a/channels.c b/channels.c index a55d27817..0f750c4d4 100644 --- a/channels.c +++ b/channels.c @@ -1,4 +1,4 @@ -/* $OpenBSD: channels.c,v 1.303 2010/01/30 21:12:08 djm Exp $ */ +/* $OpenBSD: channels.c,v 1.304 2010/05/14 23:29:23 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -330,6 +330,7 @@ channel_new(char *ctype, int type, int rfd, int wfd, int efd, c->ctl_chan = -1; c->mux_rcb = NULL; c->mux_ctx = NULL; + c->mux_pause = 0; c->delayed = 1; /* prevent call to channel_post handler */ TAILQ_INIT(&c->status_confirms); debug("channel %d: new [%s]", found, remote_name); @@ -703,7 +704,7 @@ channel_register_status_confirm(int id, channel_confirm_cb *cb, } void -channel_register_open_confirm(int id, channel_callback_fn *fn, void *ctx) +channel_register_open_confirm(int id, channel_open_fn *fn, void *ctx) { Channel *c = channel_lookup(id); @@ -991,7 +992,7 @@ channel_pre_x11_open(Channel *c, fd_set *readset, fd_set *writeset) static void channel_pre_mux_client(Channel *c, fd_set *readset, fd_set *writeset) { - if (c->istate == CHAN_INPUT_OPEN && + if (c->istate == CHAN_INPUT_OPEN && !c->mux_pause && buffer_check_alloc(&c->input, CHAN_RBUF)) FD_SET(c->rfd, readset); if (c->istate == CHAN_INPUT_WAIT_DRAIN) { @@ -1840,7 +1841,7 @@ channel_post_mux_client(Channel *c, fd_set *readset, fd_set *writeset) if (!compat20) fatal("%s: entered with !compat20", __func__); - if (c->rfd != -1 && FD_ISSET(c->rfd, readset) && + if (c->rfd != -1 && !c->mux_pause && FD_ISSET(c->rfd, readset) && (c->istate == CHAN_INPUT_OPEN || c->istate == CHAN_INPUT_WAIT_DRAIN)) { /* @@ -2463,7 +2464,7 @@ channel_input_open_confirmation(int type, u_int32_t seq, void *ctxt) c->remote_maxpacket = packet_get_int(); if (c->open_confirm) { debug2("callback start"); - c->open_confirm(c->self, c->open_confirm_ctx); + c->open_confirm(c->self, 1, c->open_confirm_ctx); debug2("callback done"); } debug2("channel %d: open confirm rwindow %u rmax %u", c->self, @@ -2514,6 +2515,11 @@ channel_input_open_failure(int type, u_int32_t seq, void *ctxt) xfree(msg); if (lang != NULL) xfree(lang); + if (c->open_confirm) { + debug2("callback start"); + c->open_confirm(c->self, 0, c->open_confirm_ctx); + debug2("callback done"); + } } packet_check_eom(); /* Schedule the channel for cleanup/deletion. */ diff --git a/channels.h b/channels.h index cc71885f4..0680ed00e 100644 --- a/channels.h +++ b/channels.h @@ -1,4 +1,4 @@ -/* $OpenBSD: channels.h,v 1.103 2010/01/26 01:28:35 djm Exp $ */ +/* $OpenBSD: channels.h,v 1.104 2010/05/14 23:29:23 djm Exp $ */ /* * Author: Tatu Ylonen @@ -60,6 +60,7 @@ struct Channel; typedef struct Channel Channel; +typedef void channel_open_fn(int, int, void *); typedef void channel_callback_fn(int, void *); typedef int channel_infilter_fn(struct Channel *, char *, int); typedef void channel_filter_cleanup_fn(int, void *); @@ -130,7 +131,7 @@ struct Channel { char *ctype; /* type */ /* callback */ - channel_callback_fn *open_confirm; + channel_open_fn *open_confirm; void *open_confirm_ctx; channel_callback_fn *detach_user; int detach_close; @@ -151,6 +152,7 @@ struct Channel { /* multiplexing protocol hook, called for each packet received */ mux_callback_fn *mux_rcb; void *mux_ctx; + int mux_pause; }; #define CHAN_EXTENDED_IGNORE 0 @@ -208,7 +210,7 @@ void channel_stop_listening(void); void channel_send_open(int); void channel_request_start(int, char *, int); void channel_register_cleanup(int, channel_callback_fn *, int); -void channel_register_open_confirm(int, channel_callback_fn *, void *); +void channel_register_open_confirm(int, channel_open_fn *, void *); void channel_register_filter(int, channel_infilter_fn *, channel_outfilter_fn *, channel_filter_cleanup_fn *, void *); void channel_register_status_confirm(int, channel_confirm_cb *, diff --git a/mux.c b/mux.c index ac477a0e7..18dfd99f3 100644 --- a/mux.c +++ b/mux.c @@ -1,4 +1,4 @@ -/* $OpenBSD: mux.c,v 1.16 2010/04/23 22:27:38 djm Exp $ */ +/* $OpenBSD: mux.c,v 1.17 2010/05/14 23:29:23 djm Exp $ */ /* * Copyright (c) 2002-2008 Damien Miller * @@ -106,6 +106,7 @@ struct mux_session_confirm_ctx { char *term; struct termios tio; char **env; + u_int rid; }; /* fd to control socket */ @@ -149,7 +150,7 @@ struct mux_master_state { #define MUX_FWD_REMOTE 2 #define MUX_FWD_DYNAMIC 3 -static void mux_session_confirm(int, void *); +static void mux_session_confirm(int, int, void *); static int process_mux_master_hello(u_int, Channel *, Buffer *, Buffer *); static int process_mux_new_session(u_int, Channel *, Buffer *, Buffer *); @@ -301,6 +302,7 @@ process_mux_new_session(u_int rid, Channel *c, Buffer *m, Buffer *r) /* Reply for SSHMUX_COMMAND_OPEN */ cctx = xcalloc(1, sizeof(*cctx)); cctx->term = NULL; + cctx->rid = rid; cmd = reserved = NULL; if ((reserved = buffer_get_string_ret(m, NULL)) == NULL || buffer_get_int_ret(&cctx->want_tty, m) != 0 || @@ -454,14 +456,10 @@ process_mux_new_session(u_int rid, Channel *c, Buffer *m, Buffer *r) channel_send_open(nc->self); channel_register_open_confirm(nc->self, mux_session_confirm, cctx); + c->mux_pause = 1; /* stop handling messages until open_confirm done */ channel_register_cleanup(nc->self, mux_master_session_cleanup_cb, 1); - /* prepare reply */ - /* XXX defer until mux_session_confirm() fires */ - buffer_put_int(r, MUX_S_SESSION_OPENED); - buffer_put_int(r, rid); - buffer_put_int(r, nc->self); - + /* reply is deferred, sent by mux_session_confirm */ return 0; } @@ -1000,17 +998,31 @@ muxserver_listen(void) /* Callback on open confirmation in mux master for a mux client session. */ static void -mux_session_confirm(int id, void *arg) +mux_session_confirm(int id, int success, void *arg) { struct mux_session_confirm_ctx *cctx = arg; const char *display; - Channel *c; + Channel *c, *cc; int i; + Buffer reply; if (cctx == NULL) fatal("%s: cctx == NULL", __func__); if ((c = channel_by_id(id)) == NULL) fatal("%s: no channel for id %d", __func__, id); + if ((cc = channel_by_id(c->ctl_chan)) == NULL) + fatal("%s: channel %d lacks control channel %d", __func__, + id, c->ctl_chan); + + if (!success) { + debug3("%s: sending failure reply", __func__); + /* prepare reply */ + buffer_init(&reply); + buffer_put_int(&reply, MUX_S_FAILURE); + buffer_put_int(&reply, cctx->rid); + buffer_put_cstring(&reply, "Session open refused by peer"); + goto done; + } display = getenv("DISPLAY"); if (cctx->want_x_fwd && options.forward_x11 && display != NULL) { @@ -1033,6 +1045,21 @@ mux_session_confirm(int id, void *arg) client_session2_setup(id, cctx->want_tty, cctx->want_subsys, cctx->term, &cctx->tio, c->rfd, &cctx->cmd, cctx->env); + debug3("%s: sending success reply", __func__); + /* prepare reply */ + buffer_init(&reply); + buffer_put_int(&reply, MUX_S_SESSION_OPENED); + buffer_put_int(&reply, cctx->rid); + buffer_put_int(&reply, c->self); + + done: + /* Send reply */ + buffer_put_string(&cc->output, buffer_ptr(&reply), buffer_len(&reply)); + buffer_free(&reply); + + if (cc->mux_pause <= 0) + fatal("%s: mux_pause %d", __func__, cc->mux_pause); + cc->mux_pause = 0; /* start processing messages again */ c->open_confirm_ctx = NULL; buffer_free(&cctx->cmd); xfree(cctx->term); diff --git a/ssh.c b/ssh.c index 2230edd16..ee224e9ff 100644 --- a/ssh.c +++ b/ssh.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh.c,v 1.336 2010/04/10 00:00:16 djm Exp $ */ +/* $OpenBSD: ssh.c,v 1.337 2010/05/14 23:29:23 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -1175,12 +1175,15 @@ ssh_session(void) /* request pty/x11/agent/tcpfwd/shell for channel */ static void -ssh_session2_setup(int id, void *arg) +ssh_session2_setup(int id, int success, void *arg) { extern char **environ; const char *display; int interactive = tty_flag; + if (!success) + return; /* No need for error message, channels code sens one */ + display = getenv("DISPLAY"); if (options.forward_x11 && display != NULL) { char *proto, *data;