- 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@
semaphore · May 14, 2011 · 555f3b8 · 555f3b8
1 parent f4b32aa
commit 555f3b8
Show file tree

Hide file tree

Showing 5 changed files with 111 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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);