- djm@cvs.openbsd.org 2011/05/08 12:52:01

     [PROTOCOL.mux clientloop.c clientloop.h mux.c]
     improve our behaviour when TTY allocation fails: if we are in
     RequestTTY=auto mode (the default), then do not treat at TTY
     allocation error as fatal but rather just restore the local TTY
     to cooked mode and continue. This is more graceful on devices that
     never allocate TTYs.

     If RequestTTY is set to "yes" or "force", then failure to allocate
     a TTY is fatal.

     ok markus@

5 files changed, 111 insertions, 29 deletions

diff --git a/ChangeLog b/ChangeLog
index dee43400a..713798cbb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -50,6 +50,18 @@
    - jmc@cvs.openbsd.org 2011/05/07 23:20:25
      [ssh.1]
      +.It RequestTTY
+   - djm@cvs.openbsd.org 2011/05/08 12:52:01
+     [PROTOCOL.mux clientloop.c clientloop.h mux.c]
+     improve our behaviour when TTY allocation fails: if we are in
+     RequestTTY=auto mode (the default), then do not treat at TTY
+     allocation error as fatal but rather just restore the local TTY
+     to cooked mode and continue. This is more graceful on devices that
+     never allocate TTYs.
+     
+     If RequestTTY is set to "yes" or "force", then failure to allocate
+     a TTY is fatal.
+     
+     ok markus@
 
 20110510
  - (dtucker) [openbsd-compat/openssl-compat.{c,h}] Bug #1882: fix
diff --git a/PROTOCOL.mux b/PROTOCOL.mux
index 3da9e37ae..9ad256602 100644
--- a/PROTOCOL.mux
+++ b/PROTOCOL.mux
@@ -73,6 +73,13 @@ non-multiplexed ssh(1) connection. Two additional cases that the
 client must cope with are it receiving a signal itself and the
 server disconnecting without sending an exit message.
 
+A master may also send a MUX_S_TTY_ALLOC_FAIL before MUX_S_EXIT_MESSAGE
+if remote TTY allocation was unsuccessful. The client may use this to
+return its local tty to "cooked" mode.
+
+        uint32  MUX_S_TTY_ALLOC_FAIL
+        uint32  session id
+
 3. Health checks
 
 The client may request a health check/PID report from a server:
@@ -197,6 +204,7 @@ The MUX_S_PERMISSION_DENIED and MUX_S_FAILURE include a reason:
 #define MUX_S_ALIVE             0x80000005
 #define MUX_S_SESSION_OPENED    0x80000006
 #define MUX_S_REMOTE_PORT       0x80000007
+#define MUX_S_TTY_ALLOC_FAIL    0x80000008
 
 #define MUX_FWD_LOCAL   1
 #define MUX_FWD_REMOTE  2
@@ -208,7 +216,6 @@ XXX lock (maybe)
 XXX watch in/out traffic (pre/post crypto)
 XXX inject packet (what about replies)
 XXX server->client error/warning notifications
-XXX port0 rfwd (need custom response message)
 XXX send signals via mux
 
-$OpenBSD: PROTOCOL.mux,v 1.6 2011/05/06 22:20:10 djm Exp $
+$OpenBSD: PROTOCOL.mux,v 1.7 2011/05/08 12:52:01 djm Exp $
diff --git a/clientloop.c b/clientloop.c
index 5bd757dfb..ed1d8a238 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.233 2011/05/06 21:34:32 djm Exp $ */
+/* $OpenBSD: clientloop.c,v 1.234 2011/05/08 12:52:01 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -174,9 +174,11 @@ struct escape_filter_ctx {
 };
 
 /* Context for channel confirmation replies */
+enum confirm_action { CONFIRM_WARN = 0, CONFIRM_CLOSE, CONFIRM_TTY };
 struct channel_reply_ctx {
         const char *request_type;
-        int id, do_close;
+        int id;
+        enum confirm_action action;
 };
 
 /* Global request success/failure callbacks */
@@ -739,6 +741,15 @@ client_status_confirm(int type, Channel *c, void *ctx)
         char errmsg[256];
         int tochan;
 
+        /*
+         * If a TTY was explicitly requested, then a failure to allocate
+         * one is fatal.
+         */
+        if (cr->action == CONFIRM_TTY &&
+            (options.request_tty == REQUEST_TTY_FORCE ||
+            options.request_tty == REQUEST_TTY_YES))
+                cr->action = CONFIRM_CLOSE;
+
         /* XXX supress on mux _client_ quietmode */
         tochan = options.log_level >= SYSLOG_LEVEL_ERROR &&
             c->ctl_chan != -1 && c->extended_usage == CHAN_EXTENDED_WRITE;
@@ -756,14 +767,27 @@ client_status_confirm(int type, Channel *c, void *ctx)
                             cr->request_type, c->self);
                 }
                 /* If error occurred on primary session channel, then exit */
-                if (cr->do_close && c->self == session_ident)
+                if (cr->action == CONFIRM_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
+                /*
+                 * 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) {
+                if (cr->action == CONFIRM_TTY) {
+                        /*
+                         * If a TTY allocation error occurred, then arrange
+                         * for the correct TTY to leave raw mode.
+                         */
+                        if (c->self == session_ident)
+                                leave_raw_mode(0);
+                        else
+                                mux_tty_alloc_failed(c);
+                } else if (cr->action == CONFIRM_CLOSE) {
                         chan_read_failed(c);
                         chan_write_failed(c);
                 }
@@ -778,12 +802,13 @@ client_abandon_status_confirm(Channel *c, void *ctx)
 }
 
 static void
-client_expect_confirm(int id, const char *request, int do_close)
+client_expect_confirm(int id, const char *request,
+    enum confirm_action action)
 {
         struct channel_reply_ctx *cr = xmalloc(sizeof(*cr));
 
         cr->request_type = request;
-        cr->do_close = do_close;
+        cr->action = action;
 
         channel_register_status_confirm(id, client_status_confirm,
             client_abandon_status_confirm, cr);
@@ -1983,7 +2008,7 @@ client_session2_setup(int id, int want_tty, int want_subsystem,
                         memset(&ws, 0, sizeof(ws));
 
                 channel_request_start(id, "pty-req", 1);
-                client_expect_confirm(id, "PTY allocation", 1);
+                client_expect_confirm(id, "PTY allocation", CONFIRM_TTY);
                 packet_put_cstring(term != NULL ? term : "");
                 packet_put_int((u_int)ws.ws_col);
                 packet_put_int((u_int)ws.ws_row);
@@ -2042,18 +2067,18 @@ client_session2_setup(int id, int want_tty, int want_subsystem,
                         debug("Sending subsystem: %.*s",
                             len, (u_char*)buffer_ptr(cmd));
                         channel_request_start(id, "subsystem", 1);
-                        client_expect_confirm(id, "subsystem", 1);
+                        client_expect_confirm(id, "subsystem", CONFIRM_CLOSE);
                 } else {
                         debug("Sending command: %.*s",
                             len, (u_char*)buffer_ptr(cmd));
                         channel_request_start(id, "exec", 1);
-                        client_expect_confirm(id, "exec", 1);
+                        client_expect_confirm(id, "exec", CONFIRM_CLOSE);
                 }
                 packet_put_string(buffer_ptr(cmd), buffer_len(cmd));
                 packet_send();
         } else {
                 channel_request_start(id, "shell", 1);
-                client_expect_confirm(id, "shell", 1);
+                client_expect_confirm(id, "shell", CONFIRM_CLOSE);
                 packet_send();
         }
 }
diff --git a/clientloop.h b/clientloop.h
index 37d072906..ad588d14d 100644
--- a/clientloop.h
+++ b/clientloop.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.h,v 1.26 2011/04/17 22:42:41 djm Exp $ */
+/* $OpenBSD: clientloop.h,v 1.27 2011/05/08 12:52:01 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -70,3 +70,5 @@ void	 client_register_global_confirm(global_confirm_cb *, void *);
 void    muxserver_listen(void);
 void    muxclient(const char *);
 void    mux_exit_message(Channel *, int);
+void    mux_tty_alloc_failed(Channel *);
+
diff --git a/mux.c b/mux.c
index 1afd1bdf3..101d7524b 100644
--- a/mux.c
+++ b/mux.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: mux.c,v 1.27 2011/05/06 21:34:32 djm Exp $ */
+/* $OpenBSD: mux.c,v 1.28 2011/05/08 12:52:01 djm Exp $ */
 /*
  * Copyright (c) 2002-2008 Damien Miller <djm@openbsd.org>
  *
@@ -153,6 +153,7 @@ struct mux_master_state {
 #define MUX_S_ALIVE             0x80000005
 #define MUX_S_SESSION_OPENED    0x80000006
 #define MUX_S_REMOTE_PORT       0x80000007
+#define MUX_S_TTY_ALLOC_FAIL    0x80000008
 
 /* type codes for MUX_C_OPEN_FWD and MUX_C_CLOSE_FWD */
 #define MUX_FWD_LOCAL   1
@@ -1054,6 +1055,27 @@ mux_exit_message(Channel *c, int exitval)
         buffer_free(&m);
 }
 
+void
+mux_tty_alloc_failed(Channel *c)
+{
+        Buffer m;
+        Channel *mux_chan;
+
+        debug3("%s: channel %d: TTY alloc failed", __func__, c->self);
+
+        if ((mux_chan = channel_by_id(c->ctl_chan)) == NULL)
+                fatal("%s: channel %d missing mux channel %d",
+                    __func__, c->self, c->ctl_chan);
+
+        /* Append exit message packet to control socket output queue */
+        buffer_init(&m);
+        buffer_put_int(&m, MUX_S_TTY_ALLOC_FAIL);
+        buffer_put_int(&m, c->self);
+
+        buffer_put_string(&mux_chan->output, buffer_ptr(&m), buffer_len(&m));
+        buffer_free(&m);
+}
+
 /* Prepare a mux master to listen on a Unix domain socket. */
 void
 muxserver_listen(void)
@@ -1612,7 +1634,7 @@ mux_client_request_session(int fd)
         char *e, *term;
         u_int i, rid, sid, esid, exitval, type, exitval_seen;
         extern char **environ;
-        int devnull;
+        int devnull, rawmode;
 
         debug3("%s: entering", __func__);
 
@@ -1708,6 +1730,7 @@ mux_client_request_session(int fd)
         signal(SIGTERM, control_client_sighandler);
         signal(SIGWINCH, control_client_sigrelay);
 
+        rawmode = tty_flag;
         if (tty_flag)
                 enter_raw_mode(options.request_tty == REQUEST_TTY_FORCE);
 
@@ -1723,22 +1746,35 @@ mux_client_request_session(int fd)
                 if (mux_client_read_packet(fd, &m) != 0)
                         break;
                 type = buffer_get_int(&m);
-                if (type != MUX_S_EXIT_MESSAGE) {
+                switch (type) {
+                case MUX_S_TTY_ALLOC_FAIL:
+                        if ((esid = buffer_get_int(&m)) != sid)
+                                fatal("%s: tty alloc fail on unknown session: "
+                                    "my id %u theirs %u",
+                                    __func__, sid, esid);
+                        leave_raw_mode(options.request_tty ==
+                            REQUEST_TTY_FORCE);
+                        rawmode = 0;
+                        continue;
+                case MUX_S_EXIT_MESSAGE:
+                        if ((esid = buffer_get_int(&m)) != sid)
+                                fatal("%s: exit on unknown session: "
+                                    "my id %u theirs %u",
+                                    __func__, sid, esid);
+                        if (exitval_seen)
+                                fatal("%s: exitval sent twice", __func__);
+                        exitval = buffer_get_int(&m);
+                        exitval_seen = 1;
+                        continue;
+                default:
                         e = buffer_get_string(&m, NULL);
                         fatal("%s: master returned error: %s", __func__, e);
                 }
-                if ((esid = buffer_get_int(&m)) != sid)
-                        fatal("%s: exit on unknown session: my id %u theirs %u",
-                            __func__, sid, esid);
-                debug("%s: master session id: %u", __func__, sid);
-                if (exitval_seen)
-                        fatal("%s: exitval sent twice", __func__);
-                exitval = buffer_get_int(&m);
-                exitval_seen = 1;
         }
 
         close(fd);
-        leave_raw_mode(options.request_tty == REQUEST_TTY_FORCE);
+        if (rawmode)
+                leave_raw_mode(options.request_tty == REQUEST_TTY_FORCE);
 
         if (muxclient_terminate) {
                 debug2("Exiting on signal %d", muxclient_terminate);