diff options
| -rw-r--r-- | ChangeLog | 14 | ||||
| -rw-r--r-- | clientloop.c | 106 | ||||
| -rw-r--r-- | clientloop.h | 4 | ||||
| -rw-r--r-- | ssh.c | 19 |
4 files changed, 90 insertions, 53 deletions
@@ -92,6 +92,18 @@ sending channel errors instead of than exiting with fatal(). bz#1090; MaxSessions config bits and manpage from junyer AT gmail.com ok markus@ + - djm@cvs.openbsd.org 2008/05/08 13:06:11 + [clientloop.c clientloop.h ssh.c] + Use new channel status confirmation callback system to properly deal + with "important" channel requests that fail, in particular command exec, + shell and subsystem requests. Previously we would optimistically assume + that the requests would always succeed, which could cause hangs if they + did not (e.g. when the server runs out of fds) or were unimplemented by + the server (bz #1384) + Also, properly report failing multiplex channel requests via the mux + client stderr (subject to LogLevel in the mux master) - better than + silently failing. + most bits ok markus@ (as part of a larger diff) 20080403 - (djm) [openbsd-compat/bsd-poll.c] Include stdlib.h to avoid compile- @@ -3952,4 +3964,4 @@ OpenServer 6 and add osr5bigcrypt support so when someone migrates passwords between UnixWare and OpenServer they will still work. OK dtucker@ -$Id: ChangeLog,v 1.4923 2008/05/19 05:34:50 djm Exp $ +$Id: ChangeLog,v 1.4924 2008/05/19 05:36:08 djm Exp $ diff --git a/clientloop.c b/clientloop.c index edd80144..c40f2c30 100644 --- a/clientloop.c +++ b/clientloop.c @@ -1,4 +1,4 @@ -/* $OpenBSD: clientloop.c,v 1.189 2008/05/08 12:02:23 djm Exp $ */ +/* $OpenBSD: clientloop.c,v 1.190 2008/05/08 13:06:10 djm Exp $ */ /* * Author: Tatu Ylonen <ylo@cs.hut.fi> * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland @@ -173,6 +173,11 @@ struct confirm_ctx { char **env; }; +struct channel_reply_ctx { + const char *request_type; + int id, do_close; +}; + /*XXX*/ extern Kex *xxx_kex; @@ -645,25 +650,60 @@ client_process_net_input(fd_set *readset) } static void -client_subsystem_reply(int type, u_int32_t seq, void *ctxt) +client_status_confirm(int type, Channel *c, void *ctx) { - int id; - Channel *c; + struct channel_reply_ctx *cr = (struct channel_reply_ctx *)ctx; + char errmsg[256]; + int tochan; + + /* XXX supress on mux _client_ quietmode */ + tochan = options.log_level >= SYSLOG_LEVEL_ERROR && + c->ctl_fd != -1 && c->extended_usage == CHAN_EXTENDED_WRITE; + + if (type == SSH2_MSG_CHANNEL_SUCCESS) { + debug2("%s request accepted on channel %d", + cr->request_type, c->self); + } else if (type == SSH2_MSG_CHANNEL_FAILURE) { + if (tochan) { + snprintf(errmsg, sizeof(errmsg), + "%s request failed\r\n", cr->request_type); + } else { + snprintf(errmsg, sizeof(errmsg), + "%s request failed on channel %d", + cr->request_type, c->self); + } + /* If error occurred on primary session channel, then exit */ + if (cr->do_close && c->self == session_ident) + fatal("%s", errmsg); + /* If error occurred on mux client, append to their stderr */ + if (tochan) + buffer_append(&c->extended, errmsg, strlen(errmsg)); + else + error("%s", errmsg); + if (cr->do_close) { + chan_read_failed(c); + chan_write_failed(c); + } + } + xfree(cr); +} - id = packet_get_int(); - packet_check_eom(); +static void +client_abandon_status_confirm(Channel *c, void *ctx) +{ + xfree(ctx); +} - if ((c = channel_lookup(id)) == NULL) { - error("%s: no channel for id %d", __func__, id); - return; - } +static void +client_expect_confirm(int id, const char *request, int do_close) +{ + struct channel_reply_ctx *cr = xmalloc(sizeof(*cr)); - if (type == SSH2_MSG_CHANNEL_SUCCESS) - debug2("Request suceeded on channel %d", id); - else if (type == SSH2_MSG_CHANNEL_FAILURE) { - error("Request failed on channel %d", id); - channel_free(c); - } + cr->request_type = request; + cr->do_close = do_close; + + channel_register_status_confirm(id, client_status_confirm, + client_abandon_status_confirm, cr); } static void @@ -698,8 +738,7 @@ client_extra_session2_setup(int id, void *arg) } client_session2_setup(id, cctx->want_tty, cctx->want_subsys, - cctx->term, &cctx->tio, c->rfd, &cctx->cmd, cctx->env, - client_subsystem_reply); + cctx->term, &cctx->tio, c->rfd, &cctx->cmd, cctx->env); c->open_confirm_ctx = NULL; buffer_free(&cctx->cmd); @@ -1366,7 +1405,9 @@ client_process_buffered_input_packets(void) static int simple_escape_filter(Channel *c, char *buf, int len) { - /* XXX we assume c->extended is writeable */ + if (c->extended_usage != CHAN_EXTENDED_WRITE) + return 0; + return process_escapes(&c->input, &c->output, &c->extended, buf, len); } @@ -1962,8 +2003,7 @@ client_input_global_request(int type, u_int32_t seq, void *ctxt) void client_session2_setup(int id, int want_tty, int want_subsystem, - const char *term, struct termios *tiop, int in_fd, Buffer *cmd, char **env, - dispatch_fn *subsys_repl) + const char *term, struct termios *tiop, int in_fd, Buffer *cmd, char **env) { int len; Channel *c = NULL; @@ -1981,7 +2021,8 @@ client_session2_setup(int id, int want_tty, int want_subsystem, if (ioctl(in_fd, TIOCGWINSZ, &ws) < 0) memset(&ws, 0, sizeof(ws)); - channel_request_start(id, "pty-req", 0); + channel_request_start(id, "pty-req", 1); + client_expect_confirm(id, "PTY allocation", 0); packet_put_cstring(term != NULL ? term : ""); packet_put_int((u_int)ws.ws_col); packet_put_int((u_int)ws.ws_row); @@ -2036,22 +2077,21 @@ client_session2_setup(int id, int want_tty, int want_subsystem, if (len > 900) len = 900; if (want_subsystem) { - debug("Sending subsystem: %.*s", len, (u_char*)buffer_ptr(cmd)); - channel_request_start(id, "subsystem", subsys_repl != NULL); - if (subsys_repl != NULL) { - /* register callback for reply */ - /* XXX we assume that client_loop has already been called */ - dispatch_set(SSH2_MSG_CHANNEL_FAILURE, subsys_repl); - dispatch_set(SSH2_MSG_CHANNEL_SUCCESS, subsys_repl); - } + debug("Sending subsystem: %.*s", + len, (u_char*)buffer_ptr(cmd)); + channel_request_start(id, "subsystem", 1); + client_expect_confirm(id, "subsystem", 1); } else { - debug("Sending command: %.*s", len, (u_char*)buffer_ptr(cmd)); - channel_request_start(id, "exec", 0); + debug("Sending command: %.*s", + len, (u_char*)buffer_ptr(cmd)); + channel_request_start(id, "exec", 1); + client_expect_confirm(id, "exec", 1); } packet_put_string(buffer_ptr(cmd), buffer_len(cmd)); packet_send(); } else { - channel_request_start(id, "shell", 0); + channel_request_start(id, "shell", 1); + client_expect_confirm(id, "shell", 1); packet_send(); } } diff --git a/clientloop.h b/clientloop.h index c7d2233d..cb2d7c08 100644 --- a/clientloop.h +++ b/clientloop.h @@ -1,4 +1,4 @@ -/* $OpenBSD: clientloop.h,v 1.17 2007/08/07 07:32:53 djm Exp $ */ +/* $OpenBSD: clientloop.h,v 1.18 2008/05/08 13:06:11 djm Exp $ */ /* * Author: Tatu Ylonen <ylo@cs.hut.fi> @@ -43,7 +43,7 @@ void client_x11_get_proto(const char *, const char *, u_int, char **, char **); void client_global_request_reply_fwd(int, u_int32_t, void *); void client_session2_setup(int, int, int, const char *, struct termios *, - int, Buffer *, char **, dispatch_fn *); + int, Buffer *, char **); int client_request_tun_fwd(int, int, int); /* Multiplexing protocol version */ @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh.c,v 1.310 2008/05/08 12:02:23 djm Exp $ */ +/* $OpenBSD: ssh.c,v 1.311 2008/05/08 13:06:11 djm Exp $ */ /* * Author: Tatu Ylonen <ylo@cs.hut.fi> * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland @@ -1039,21 +1039,6 @@ ssh_session(void) options.escape_char : SSH_ESCAPECHAR_NONE, 0); } -static void -ssh_subsystem_reply(int type, u_int32_t seq, void *ctxt) -{ - int id, len; - - id = packet_get_int(); - len = buffer_len(&command); - if (len > 900) - len = 900; - packet_check_eom(); - if (type == SSH2_MSG_CHANNEL_FAILURE) - fatal("Request for subsystem '%.*s' failed on channel %d", - len, (u_char *)buffer_ptr(&command), id); -} - void client_global_request_reply_fwd(int type, u_int32_t seq, void *ctxt) { @@ -1150,7 +1135,7 @@ ssh_session2_setup(int id, void *arg) } client_session2_setup(id, tty_flag, subsystem_flag, getenv("TERM"), - NULL, fileno(stdin), &command, environ, &ssh_subsystem_reply); + NULL, fileno(stdin), &command, environ); packet_set_interactive(interactive); } |