summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDamien Miller <djm@mindrot.org>2008-05-19 15:35:33 +1000
committerDamien Miller <djm@mindrot.org>2008-05-19 15:35:33 +1000
commit5771ed7d1b5af26bc991151ab977e77bf1e87666 (patch)
treed24e88330fb1b3b2de2e1ed770ba0f28963c5049
parent7207f64a23a49a719aad3083c068f50e5034ccb8 (diff)
- 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)
-rw-r--r--ChangeLog14
-rw-r--r--clientloop.c106
-rw-r--r--clientloop.h4
-rw-r--r--ssh.c19
4 files changed, 90 insertions, 53 deletions
diff --git a/ChangeLog b/ChangeLog
index 5ea4afcac..bd6b640b1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -92,6 +92,18 @@
92 sending channel errors instead of than exiting with fatal(). 92 sending channel errors instead of than exiting with fatal().
93 bz#1090; MaxSessions config bits and manpage from junyer AT gmail.com 93 bz#1090; MaxSessions config bits and manpage from junyer AT gmail.com
94 ok markus@ 94 ok markus@
95 - djm@cvs.openbsd.org 2008/05/08 13:06:11
96 [clientloop.c clientloop.h ssh.c]
97 Use new channel status confirmation callback system to properly deal
98 with "important" channel requests that fail, in particular command exec,
99 shell and subsystem requests. Previously we would optimistically assume
100 that the requests would always succeed, which could cause hangs if they
101 did not (e.g. when the server runs out of fds) or were unimplemented by
102 the server (bz #1384)
103 Also, properly report failing multiplex channel requests via the mux
104 client stderr (subject to LogLevel in the mux master) - better than
105 silently failing.
106 most bits ok markus@ (as part of a larger diff)
95 107
9620080403 10820080403
97 - (djm) [openbsd-compat/bsd-poll.c] Include stdlib.h to avoid compile- 109 - (djm) [openbsd-compat/bsd-poll.c] Include stdlib.h to avoid compile-
@@ -3952,4 +3964,4 @@
3952 OpenServer 6 and add osr5bigcrypt support so when someone migrates 3964 OpenServer 6 and add osr5bigcrypt support so when someone migrates
3953 passwords between UnixWare and OpenServer they will still work. OK dtucker@ 3965 passwords between UnixWare and OpenServer they will still work. OK dtucker@
3954 3966
3955$Id: ChangeLog,v 1.4923 2008/05/19 05:34:50 djm Exp $ 3967$Id: ChangeLog,v 1.4924 2008/05/19 05:36:08 djm Exp $
diff --git a/clientloop.c b/clientloop.c
index edd801440..c40f2c303 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -1,4 +1,4 @@
1/* $OpenBSD: clientloop.c,v 1.189 2008/05/08 12:02:23 djm Exp $ */ 1/* $OpenBSD: clientloop.c,v 1.190 2008/05/08 13:06:10 djm Exp $ */
2/* 2/*
3 * Author: Tatu Ylonen <ylo@cs.hut.fi> 3 * Author: Tatu Ylonen <ylo@cs.hut.fi>
4 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland 4 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -173,6 +173,11 @@ struct confirm_ctx {
173 char **env; 173 char **env;
174}; 174};
175 175
176struct channel_reply_ctx {
177 const char *request_type;
178 int id, do_close;
179};
180
176/*XXX*/ 181/*XXX*/
177extern Kex *xxx_kex; 182extern Kex *xxx_kex;
178 183
@@ -645,25 +650,60 @@ client_process_net_input(fd_set *readset)
645} 650}
646 651
647static void 652static void
648client_subsystem_reply(int type, u_int32_t seq, void *ctxt) 653client_status_confirm(int type, Channel *c, void *ctx)
649{ 654{
650 int id; 655 struct channel_reply_ctx *cr = (struct channel_reply_ctx *)ctx;
651 Channel *c; 656 char errmsg[256];
657 int tochan;
658
659 /* XXX supress on mux _client_ quietmode */
660 tochan = options.log_level >= SYSLOG_LEVEL_ERROR &&
661 c->ctl_fd != -1 && c->extended_usage == CHAN_EXTENDED_WRITE;
662
663 if (type == SSH2_MSG_CHANNEL_SUCCESS) {
664 debug2("%s request accepted on channel %d",
665 cr->request_type, c->self);
666 } else if (type == SSH2_MSG_CHANNEL_FAILURE) {
667 if (tochan) {
668 snprintf(errmsg, sizeof(errmsg),
669 "%s request failed\r\n", cr->request_type);
670 } else {
671 snprintf(errmsg, sizeof(errmsg),
672 "%s request failed on channel %d",
673 cr->request_type, c->self);
674 }
675 /* If error occurred on primary session channel, then exit */
676 if (cr->do_close && c->self == session_ident)
677 fatal("%s", errmsg);
678 /* If error occurred on mux client, append to their stderr */
679 if (tochan)
680 buffer_append(&c->extended, errmsg, strlen(errmsg));
681 else
682 error("%s", errmsg);
683 if (cr->do_close) {
684 chan_read_failed(c);
685 chan_write_failed(c);
686 }
687 }
688 xfree(cr);
689}
652 690
653 id = packet_get_int(); 691static void
654 packet_check_eom(); 692client_abandon_status_confirm(Channel *c, void *ctx)
693{
694 xfree(ctx);
695}
655 696
656 if ((c = channel_lookup(id)) == NULL) { 697static void
657 error("%s: no channel for id %d", __func__, id); 698client_expect_confirm(int id, const char *request, int do_close)
658 return; 699{
659 } 700 struct channel_reply_ctx *cr = xmalloc(sizeof(*cr));
660 701
661 if (type == SSH2_MSG_CHANNEL_SUCCESS) 702 cr->request_type = request;
662 debug2("Request suceeded on channel %d", id); 703 cr->do_close = do_close;
663 else if (type == SSH2_MSG_CHANNEL_FAILURE) { 704
664 error("Request failed on channel %d", id); 705 channel_register_status_confirm(id, client_status_confirm,
665 channel_free(c); 706 client_abandon_status_confirm, cr);
666 }
667} 707}
668 708
669static void 709static void
@@ -698,8 +738,7 @@ client_extra_session2_setup(int id, void *arg)
698 } 738 }
699 739
700 client_session2_setup(id, cctx->want_tty, cctx->want_subsys, 740 client_session2_setup(id, cctx->want_tty, cctx->want_subsys,
701 cctx->term, &cctx->tio, c->rfd, &cctx->cmd, cctx->env, 741 cctx->term, &cctx->tio, c->rfd, &cctx->cmd, cctx->env);
702 client_subsystem_reply);
703 742
704 c->open_confirm_ctx = NULL; 743 c->open_confirm_ctx = NULL;
705 buffer_free(&cctx->cmd); 744 buffer_free(&cctx->cmd);
@@ -1366,7 +1405,9 @@ client_process_buffered_input_packets(void)
1366static int 1405static int
1367simple_escape_filter(Channel *c, char *buf, int len) 1406simple_escape_filter(Channel *c, char *buf, int len)
1368{ 1407{
1369 /* XXX we assume c->extended is writeable */ 1408 if (c->extended_usage != CHAN_EXTENDED_WRITE)
1409 return 0;
1410
1370 return process_escapes(&c->input, &c->output, &c->extended, buf, len); 1411 return process_escapes(&c->input, &c->output, &c->extended, buf, len);
1371} 1412}
1372 1413
@@ -1962,8 +2003,7 @@ client_input_global_request(int type, u_int32_t seq, void *ctxt)
1962 2003
1963void 2004void
1964client_session2_setup(int id, int want_tty, int want_subsystem, 2005client_session2_setup(int id, int want_tty, int want_subsystem,
1965 const char *term, struct termios *tiop, int in_fd, Buffer *cmd, char **env, 2006 const char *term, struct termios *tiop, int in_fd, Buffer *cmd, char **env)
1966 dispatch_fn *subsys_repl)
1967{ 2007{
1968 int len; 2008 int len;
1969 Channel *c = NULL; 2009 Channel *c = NULL;
@@ -1981,7 +2021,8 @@ client_session2_setup(int id, int want_tty, int want_subsystem,
1981 if (ioctl(in_fd, TIOCGWINSZ, &ws) < 0) 2021 if (ioctl(in_fd, TIOCGWINSZ, &ws) < 0)
1982 memset(&ws, 0, sizeof(ws)); 2022 memset(&ws, 0, sizeof(ws));
1983 2023
1984 channel_request_start(id, "pty-req", 0); 2024 channel_request_start(id, "pty-req", 1);
2025 client_expect_confirm(id, "PTY allocation", 0);
1985 packet_put_cstring(term != NULL ? term : ""); 2026 packet_put_cstring(term != NULL ? term : "");
1986 packet_put_int((u_int)ws.ws_col); 2027 packet_put_int((u_int)ws.ws_col);
1987 packet_put_int((u_int)ws.ws_row); 2028 packet_put_int((u_int)ws.ws_row);
@@ -2036,22 +2077,21 @@ client_session2_setup(int id, int want_tty, int want_subsystem,
2036 if (len > 900) 2077 if (len > 900)
2037 len = 900; 2078 len = 900;
2038 if (want_subsystem) { 2079 if (want_subsystem) {
2039 debug("Sending subsystem: %.*s", len, (u_char*)buffer_ptr(cmd)); 2080 debug("Sending subsystem: %.*s",
2040 channel_request_start(id, "subsystem", subsys_repl != NULL); 2081 len, (u_char*)buffer_ptr(cmd));
2041 if (subsys_repl != NULL) { 2082 channel_request_start(id, "subsystem", 1);
2042 /* register callback for reply */ 2083 client_expect_confirm(id, "subsystem", 1);
2043 /* XXX we assume that client_loop has already been called */
2044 dispatch_set(SSH2_MSG_CHANNEL_FAILURE, subsys_repl);
2045 dispatch_set(SSH2_MSG_CHANNEL_SUCCESS, subsys_repl);
2046 }
2047 } else { 2084 } else {
2048 debug("Sending command: %.*s", len, (u_char*)buffer_ptr(cmd)); 2085 debug("Sending command: %.*s",
2049 channel_request_start(id, "exec", 0); 2086 len, (u_char*)buffer_ptr(cmd));
2087 channel_request_start(id, "exec", 1);
2088 client_expect_confirm(id, "exec", 1);
2050 } 2089 }
2051 packet_put_string(buffer_ptr(cmd), buffer_len(cmd)); 2090 packet_put_string(buffer_ptr(cmd), buffer_len(cmd));
2052 packet_send(); 2091 packet_send();
2053 } else { 2092 } else {
2054 channel_request_start(id, "shell", 0); 2093 channel_request_start(id, "shell", 1);
2094 client_expect_confirm(id, "shell", 1);
2055 packet_send(); 2095 packet_send();
2056 } 2096 }
2057} 2097}
diff --git a/clientloop.h b/clientloop.h
index c7d2233d0..cb2d7c089 100644
--- a/clientloop.h
+++ b/clientloop.h
@@ -1,4 +1,4 @@
1/* $OpenBSD: clientloop.h,v 1.17 2007/08/07 07:32:53 djm Exp $ */ 1/* $OpenBSD: clientloop.h,v 1.18 2008/05/08 13:06:11 djm Exp $ */
2 2
3/* 3/*
4 * Author: Tatu Ylonen <ylo@cs.hut.fi> 4 * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -43,7 +43,7 @@ void client_x11_get_proto(const char *, const char *, u_int,
43 char **, char **); 43 char **, char **);
44void client_global_request_reply_fwd(int, u_int32_t, void *); 44void client_global_request_reply_fwd(int, u_int32_t, void *);
45void client_session2_setup(int, int, int, const char *, struct termios *, 45void client_session2_setup(int, int, int, const char *, struct termios *,
46 int, Buffer *, char **, dispatch_fn *); 46 int, Buffer *, char **);
47int client_request_tun_fwd(int, int, int); 47int client_request_tun_fwd(int, int, int);
48 48
49/* Multiplexing protocol version */ 49/* Multiplexing protocol version */
diff --git a/ssh.c b/ssh.c
index b144a7130..cfefadef9 100644
--- a/ssh.c
+++ b/ssh.c
@@ -1,4 +1,4 @@
1/* $OpenBSD: ssh.c,v 1.310 2008/05/08 12:02:23 djm Exp $ */ 1/* $OpenBSD: ssh.c,v 1.311 2008/05/08 13:06:11 djm Exp $ */
2/* 2/*
3 * Author: Tatu Ylonen <ylo@cs.hut.fi> 3 * Author: Tatu Ylonen <ylo@cs.hut.fi>
4 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland 4 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1039,21 +1039,6 @@ ssh_session(void)
1039 options.escape_char : SSH_ESCAPECHAR_NONE, 0); 1039 options.escape_char : SSH_ESCAPECHAR_NONE, 0);
1040} 1040}
1041 1041
1042static void
1043ssh_subsystem_reply(int type, u_int32_t seq, void *ctxt)
1044{
1045 int id, len;
1046
1047 id = packet_get_int();
1048 len = buffer_len(&command);
1049 if (len > 900)
1050 len = 900;
1051 packet_check_eom();
1052 if (type == SSH2_MSG_CHANNEL_FAILURE)
1053 fatal("Request for subsystem '%.*s' failed on channel %d",
1054 len, (u_char *)buffer_ptr(&command), id);
1055}
1056
1057void 1042void
1058client_global_request_reply_fwd(int type, u_int32_t seq, void *ctxt) 1043client_global_request_reply_fwd(int type, u_int32_t seq, void *ctxt)
1059{ 1044{
@@ -1150,7 +1135,7 @@ ssh_session2_setup(int id, void *arg)
1150 } 1135 }
1151 1136
1152 client_session2_setup(id, tty_flag, subsystem_flag, getenv("TERM"), 1137 client_session2_setup(id, tty_flag, subsystem_flag, getenv("TERM"),
1153 NULL, fileno(stdin), &command, environ, &ssh_subsystem_reply); 1138 NULL, fileno(stdin), &command, environ);
1154 1139
1155 packet_set_interactive(interactive); 1140 packet_set_interactive(interactive);
1156} 1141}