Devolve channel-request handling to Channel vtable.

Instead of the central code in ssh2_connection_filter_queue doing both
the job of parsing the channel request and deciding whether it's
acceptable, each Channel vtable now has a method for every channel
request type we recognise.

agentf.c

@@ -156,6 +156,9 @@ static const struct ChannelVtable agentf_channelvt = {
     agentf_set_input_wanted,
     agentf_log_close_msg,
     chan_no_eager_close,
+    chan_no_exit_status,
+    chan_no_exit_signal,
+    chan_no_exit_signal_numeric,
 };
 
 Channel *agentf_new(SshChannel *c)

portfwd.c

@@ -451,6 +451,9 @@ static const struct ChannelVtable PortForwarding_channelvt = {
     pfd_set_input_wanted,
     pfd_log_close_msg,
     chan_no_eager_close,
+    chan_no_exit_status,
+    chan_no_exit_signal,
+    chan_no_exit_signal_numeric,
 };
 
 /*

ssh2connection.c

@@ -444,7 +444,7 @@ static int ssh2_connection_filter_queue(struct ssh2_connection_state *s)
     struct ssh2_channel *c;
     struct outstanding_channel_request *ocr;
     unsigned localid, remid, winsize, pktsize, ext_type;
-    int want_reply, reply_type, expect_halfopen;
+    int want_reply, reply_success, expect_halfopen;
     const char *error;
     PacketProtocolLayer *ppl = &s->ppl; /* for ppl_logevent */
 
@@ -466,13 +466,7 @@ static int ssh2_connection_filter_queue(struct ssh2_connection_state *s)
             /* type = */ get_string(pktin);
             want_reply = get_bool(pktin);
 
-            /*
+            reply_success = FALSE;
-             * 'reply_type' is the message type we'll send in
-             * response, if want_reply is set. Initialise it to the
-             * default value of REQUEST_FAILURE, for any request we
-             * don't recognise and handle below.
-             */
-            reply_type = SSH2_MSG_REQUEST_FAILURE;
 
             /*
              * We currently don't support any incoming global requests
@@ -481,7 +475,9 @@ static int ssh2_connection_filter_queue(struct ssh2_connection_state *s)
              */
 
             if (want_reply) {
-                pktout = ssh_bpp_new_pktout(s->ppl.bpp, reply_type);
+                int type = (reply_success ? SSH2_MSG_REQUEST_SUCCESS :
+                            SSH2_MSG_REQUEST_FAILURE);
+                pktout = ssh_bpp_new_pktout(s->ppl.bpp, type);
                 pq_push(s->ppl.out_pq, pktout);
             }
             pq_pop(s->ppl.in_pq);
@@ -773,13 +769,7 @@ static int ssh2_connection_filter_queue(struct ssh2_connection_state *s)
                 type = get_string(pktin);
                 want_reply = get_bool(pktin);
 
-                /*
+                reply_success = FALSE;
-                 * 'reply_type' is the message type we'll send in
-                 * response, if want_reply is set. Initialise it to
-                 * the default value of CHANNEL_FAILURE, for any
-                 * request we don't recognise and handle below.
-                 */
-                reply_type = SSH2_MSG_CHANNEL_FAILURE;
 
                 if (c->closes & CLOSES_SENT_CLOSE) {
                     /*
@@ -793,141 +783,71 @@ static int ssh2_connection_filter_queue(struct ssh2_connection_state *s)
                 }
 
                 /*
-                 * Having got the channel number, we now look at the
+                 * Try every channel request name we recognise, no
-                 * request type string to see if it's something we
+                 * matter what the channel, and see if the Channel
-                 * recognise.
+                 * instance will accept it.
                  */
-                if (c == s->mainchan) {
+                if (ptrlen_eq_string(type, "exit-status")) {
-                    int exitcode;
+                    int exitcode = toint(get_uint32(pktin));
+                    reply_success = chan_rcvd_exit_status(c->chan, exitcode);
+                } else if (ptrlen_eq_string(type, "exit-signal")) {
+                    ptrlen signame;
+                    int signum;
+                    int core = FALSE;
+                    ptrlen errmsg;
+                    int format;
 
                     /*
-                     * We recognise "exit-status" and "exit-signal" on
+                     * ICK: older versions of OpenSSH (e.g. 3.4p1)
-                     * the primary channel.
+                     * provide an `int' for the signal, despite its
+                     * having been a `string' in the drafts of RFC
+                     * 4254 since at least 2001. (Fixed in session.c
+                     * 1.147.) Try to infer which we can safely parse
+                     * it as.
                      */
-                    if (ptrlen_eq_string(type, "exit-status")) {
-                        exitcode = toint(get_uint32(pktin));
-                        ssh_got_exitcode(s->ppl.ssh, exitcode);
-                        ppl_logevent(("Server sent command exit status %d",
-                                      exitcode));
-                        reply_type = SSH2_MSG_CHANNEL_SUCCESS;
-                    } else if (ptrlen_eq_string(type, "exit-signal")) {
-                        char *fmt_sig = NULL, *fmt_msg = NULL;
-                        ptrlen errmsg;
-                        int core = FALSE;
-                        int format;
 
-                        /*
+                    size_t startpos = BinarySource_UPCAST(pktin)->pos;
-                         * ICK: older versions of OpenSSH (e.g. 3.4p1)
-                         * provide an `int' for the signal, despite
-                         * its having been a `string' in the drafts of
-                         * RFC 4254 since at least 2001. (Fixed in
-                         * session.c 1.147.) Try to infer which we can
-                         * safely parse it as.
-                         */
 
-                        size_t startpos = BinarySource_UPCAST(pktin)->pos;
+                    for (format = 0; format < 2; format++) {
+                        BinarySource_UPCAST(pktin)->pos = startpos;
+                        BinarySource_UPCAST(pktin)->err = BSE_NO_ERROR;
 
-                        for (format = 0; format < 2; format++) {
+                        /* placate compiler warnings about unin */
-                            BinarySource_UPCAST(pktin)->pos = startpos;
+                        signame = make_ptrlen(NULL, 0);
-                            BinarySource_UPCAST(pktin)->err = BSE_NO_ERROR;
+                        signum = 0;
 
-                            if (format == 0) {
+                        if (format == 0) /* standard string-based format */
-                                /* standard string-based format */
+                            signame = get_string(pktin);
-                                ptrlen signame = get_string(pktin);
+                        else      /* nonstandard integer format */
-                                fmt_sig = dupprintf(" \"%.*s\"",
+                            signum = toint(get_uint32(pktin));
-                                                    PTRLEN_PRINTF(signame));
 
-                                /*
+                        core = get_bool(pktin);
-                                 * Really hideous method of translating the
+                        errmsg = get_string(pktin); /* error message */
-                                 * signal description back into a locally
+                        get_string(pktin);     /* language tag */
-                                 * meaningful number.
-                                 */
 
-                                if (0)
+                        if (!get_err(pktin) && get_avail(pktin) == 0)
-                                    ;
+                            break;             /* successful parse */
-#define TRANSLATE_SIGNAL(s)                                             \
+                    }
-                                else if (ptrlen_eq_string(signame, #s)) \
-                                    exitcode = 128 + SIG ## s
-#ifdef SIGABRT
-                                TRANSLATE_SIGNAL(ABRT);
-#endif
-#ifdef SIGALRM
-                                TRANSLATE_SIGNAL(ALRM);
-#endif
-#ifdef SIGFPE
-                                TRANSLATE_SIGNAL(FPE);
-#endif
-#ifdef SIGHUP
-                                TRANSLATE_SIGNAL(HUP);
-#endif
-#ifdef SIGILL
-                                TRANSLATE_SIGNAL(ILL);
-#endif
-#ifdef SIGINT
-                                TRANSLATE_SIGNAL(INT);
-#endif
-#ifdef SIGKILL
-                                TRANSLATE_SIGNAL(KILL);
-#endif
-#ifdef SIGPIPE
-                                TRANSLATE_SIGNAL(PIPE);
-#endif
-#ifdef SIGQUIT
-                                TRANSLATE_SIGNAL(QUIT);
-#endif
-#ifdef SIGSEGV
-                                TRANSLATE_SIGNAL(SEGV);
-#endif
-#ifdef SIGTERM
-                                TRANSLATE_SIGNAL(TERM);
-#endif
-#ifdef SIGUSR1
-                                TRANSLATE_SIGNAL(USR1);
-#endif
-#ifdef SIGUSR2
-                                TRANSLATE_SIGNAL(USR2);
-#endif
-#undef TRANSLATE_SIGNAL
-                                else
-                                    exitcode = 128;
-                            } else {
-                                /* nonstandard integer format */
-                                unsigned signum = get_uint32(pktin);
-                                fmt_sig = dupprintf(" %u", signum);
-                                exitcode = 128 + signum;
-                            }
 
-                            core = get_bool(pktin);
+                    switch (format) {
-                            errmsg = get_string(pktin); /* error message */
+                      case 0:
-                            get_string(pktin);     /* language tag */
+                        reply_success = chan_rcvd_exit_signal(
-                            if (!get_err(pktin) && get_avail(pktin) == 0)
+                            c->chan, signame, core, errmsg);
-                                break;             /* successful parse */
+                        break;
-
+                      case 1:
-                            sfree(fmt_sig);
+                        reply_success = chan_rcvd_exit_signal_numeric(
-                        }
+                            c->chan, signum, core, errmsg);
-
+                        break;
-                        if (format == 2) {
+                      default:
-                            fmt_sig = NULL;
+                        /* Couldn't parse this message in either format */
-                            exitcode = 128;
+                        reply_success = FALSE;
-                        }
+                        break;
-
-                        if (errmsg.len) {
-                            fmt_msg = dupprintf(" (\"%.*s\")",
-                                                PTRLEN_PRINTF(errmsg));
-                        }
-
-                        ssh_got_exitcode(s->ppl.ssh, exitcode);
-                        ppl_logevent(("Server exited on signal%s%s%s",
-                                      fmt_sig ? fmt_sig : "",
-                                      core ? " (core dumped)" : "",
-                                      fmt_msg ? fmt_msg : ""));
-                        sfree(fmt_sig);
-                        sfree(fmt_msg);
-                        reply_type = SSH2_MSG_CHANNEL_SUCCESS;
                     }
                 }
                 if (want_reply) {
-                    pktout = ssh_bpp_new_pktout(s->ppl.bpp, reply_type);
+                    int type = (reply_success ? SSH2_MSG_CHANNEL_SUCCESS :
+                                SSH2_MSG_CHANNEL_FAILURE);
+                    pktout = ssh_bpp_new_pktout(s->ppl.bpp, type);
                     put_uint32(pktout, c->remoteid);
                     pq_push(s->ppl.out_pq, pktout);
                 }
@@ -2193,6 +2113,11 @@ static int mainchan_send(Channel *chan, int is_stderr, const void *, int);
 static void mainchan_send_eof(Channel *chan);
 static void mainchan_set_input_wanted(Channel *chan, int wanted);
 static char *mainchan_log_close_msg(Channel *chan);
+static int mainchan_rcvd_exit_status(Channel *chan, int status);
+static int mainchan_rcvd_exit_signal(
+    Channel *chan, ptrlen signame, int core_dumped, ptrlen msg);
+static int mainchan_rcvd_exit_signal_numeric(
+    Channel *chan, int signum, int core_dumped, ptrlen msg);
 
 static const struct ChannelVtable mainchan_channelvt = {
     mainchan_free,
@@ -2203,6 +2128,9 @@ static const struct ChannelVtable mainchan_channelvt = {
     mainchan_set_input_wanted,
     mainchan_log_close_msg,
     chan_no_eager_close,
+    mainchan_rcvd_exit_status,
+    mainchan_rcvd_exit_signal,
+    mainchan_rcvd_exit_signal_numeric,
 };
 
 static mainchan *mainchan_new(struct ssh2_connection_state *s)
@@ -2299,6 +2227,119 @@ static char *mainchan_log_close_msg(Channel *chan)
     return dupstr("Main session channel closed");
 }
 
+static int mainchan_rcvd_exit_status(Channel *chan, int status)
+{
+    assert(chan->vt == &mainchan_channelvt);
+    mainchan *mc = container_of(chan, mainchan, chan);
+    struct ssh2_connection_state *s = mc->connlayer;
+    PacketProtocolLayer *ppl = &s->ppl; /* for ppl_logevent */
+
+    ssh_got_exitcode(s->ppl.ssh, status);
+    ppl_logevent(("Session sent command exit status %d", status));
+    return TRUE;
+}
+
+static void ssh2_log_exit_signal_common(
+    struct ssh2_connection_state *s, const char *sigdesc,
+    int core_dumped, ptrlen msg)
+{
+    PacketProtocolLayer *ppl = &s->ppl; /* for ppl_logevent */
+
+    const char *core_msg = core_dumped ? " (core dumped)" : "";
+    const char *msg_pre = (msg.len ? " (" : "");
+    const char *msg_post = (msg.len ? ")" : "");
+    ppl_logevent(("Session exited on %s%s%s%.*s%s",
+                  sigdesc, core_msg, msg_pre, PTRLEN_PRINTF(msg), msg_post));
+}
+
+static int mainchan_rcvd_exit_signal(
+    Channel *chan, ptrlen signame, int core_dumped, ptrlen msg)
+{
+    assert(chan->vt == &mainchan_channelvt);
+    mainchan *mc = container_of(chan, mainchan, chan);
+    struct ssh2_connection_state *s = mc->connlayer;
+    int exitcode;
+    char *signame_str;
+
+    /*
+     * Translate the signal description back into a locally
+     * meaningful number.
+     */
+
+    if (0)
+        ;
+#define TRANSLATE_SIGNAL(s)                                     \
+    else if (ptrlen_eq_string(signame, #s))      \
+        exitcode = 128 + SIG ## s
+#ifdef SIGABRT
+    TRANSLATE_SIGNAL(ABRT);
+#endif
+#ifdef SIGALRM
+    TRANSLATE_SIGNAL(ALRM);
+#endif
+#ifdef SIGFPE
+    TRANSLATE_SIGNAL(FPE);
+#endif
+#ifdef SIGHUP
+    TRANSLATE_SIGNAL(HUP);
+#endif
+#ifdef SIGILL
+    TRANSLATE_SIGNAL(ILL);
+#endif
+#ifdef SIGINT
+    TRANSLATE_SIGNAL(INT);
+#endif
+#ifdef SIGKILL
+    TRANSLATE_SIGNAL(KILL);
+#endif
+#ifdef SIGPIPE
+    TRANSLATE_SIGNAL(PIPE);
+#endif
+#ifdef SIGQUIT
+    TRANSLATE_SIGNAL(QUIT);
+#endif
+#ifdef SIGSEGV
+    TRANSLATE_SIGNAL(SEGV);
+#endif
+#ifdef SIGTERM
+    TRANSLATE_SIGNAL(TERM);
+#endif
+#ifdef SIGUSR1
+    TRANSLATE_SIGNAL(USR1);
+#endif
+#ifdef SIGUSR2
+    TRANSLATE_SIGNAL(USR2);
+#endif
+#undef TRANSLATE_SIGNAL
+    else
+        exitcode = 128;
+
+    ssh_got_exitcode(s->ppl.ssh, exitcode);
+    if (exitcode == 128)
+        signame_str = dupprintf("unrecognised signal \"%.*s\"",
+                                PTRLEN_PRINTF(signame));
+    else
+        signame_str = dupprintf("signal SIG%.*s", PTRLEN_PRINTF(signame));
+    ssh2_log_exit_signal_common(s, signame_str, core_dumped, msg);
+    sfree(signame_str);
+    return TRUE;
+}
+
+static int mainchan_rcvd_exit_signal_numeric(
+    Channel *chan, int signum, int core_dumped, ptrlen msg)
+{
+    assert(chan->vt == &mainchan_channelvt);
+    mainchan *mc = container_of(chan, mainchan, chan);
+    struct ssh2_connection_state *s = mc->connlayer;
+    char *signum_str;
+
+    ssh_got_exitcode(s->ppl.ssh, 128 + signum);
+    signum_str = dupprintf("signal %d", signum);
+    ssh2_log_exit_signal_common(s, signum_str, core_dumped, msg);
+    sfree(signum_str);
+    return TRUE;
+}
+
 /*
  * List of signal names defined by RFC 4254. These include all the ISO
  * C signals, but are a subset of the POSIX required signals.

sshchan.h

@@ -26,6 +26,14 @@ struct ChannelVtable {
     char *(*log_close_msg)(Channel *);
 
     int (*want_close)(Channel *, int sent_local_eof, int rcvd_remote_eof);
+
+    /* A method for every channel request we know of. All of these
+     * return TRUE for success or FALSE for failure. */
+    int (*rcvd_exit_status)(Channel *, int status);
+    int (*rcvd_exit_signal)(
+        Channel *chan, ptrlen signame, int core_dumped, ptrlen msg);
+    int (*rcvd_exit_signal_numeric)(
+        Channel *chan, int signum, int core_dumped, ptrlen msg);
 };
 
 struct Channel {
@@ -42,6 +50,12 @@ struct Channel {
     ((ch)->vt->set_input_wanted(ch, wanted))
 #define chan_log_close_msg(ch) ((ch)->vt->log_close_msg(ch))
 #define chan_want_close(ch, leof, reof) ((ch)->vt->want_close(ch, leof, reof))
+#define chan_rcvd_exit_status(ch, status) \
+    ((ch)->vt->rcvd_exit_status(ch, status))
+#define chan_rcvd_exit_signal(ch, sig, core, msg)   \
+    ((ch)->vt->rcvd_exit_signal(ch, sig, core, msg))
+#define chan_rcvd_exit_signal_numeric(ch, sig, core, msg)   \
+    ((ch)->vt->rcvd_exit_signal_numeric(ch, sig, core, msg))
 
 /*
  * Reusable methods you can put in vtables to give default handling of
@@ -56,6 +70,11 @@ void chan_remotely_opened_failure(Channel *chan, const char *errtext);
  * closing until both directions have had an EOF */
 int chan_no_eager_close(Channel *, int, int);
 
+/* default implementations that refuse all the channel requests */
+int chan_no_exit_status(Channel *, int);
+int chan_no_exit_signal(Channel *, ptrlen, int, ptrlen);
+int chan_no_exit_signal_numeric(Channel *, int, int, ptrlen);
+
 /*
  * Constructor for a trivial do-nothing implementation of
  * ChannelVtable. Used for 'zombie' channels, i.e. channels whose

sshcommon.c

@@ -277,6 +277,9 @@ static const struct ChannelVtable zombiechan_channelvt = {
     zombiechan_set_input_wanted,
     zombiechan_log_close_msg,
     zombiechan_want_close,
+    chan_no_exit_status,
+    chan_no_exit_signal,
+    chan_no_exit_signal_numeric,
 };
 
 Channel *zombiechan_new(void)
@@ -340,6 +343,23 @@ int chan_no_eager_close(Channel *chan, int sent_local_eof, int rcvd_remote_eof)
     return FALSE;     /* default: never proactively ask for a close */
 }
 
+int chan_no_exit_status(Channel *chan, int status)
+{
+    return FALSE;
+}
+
+int chan_no_exit_signal(
+    Channel *chan, ptrlen signame, int core_dumped, ptrlen msg)
+{
+    return FALSE;
+}
+
+int chan_no_exit_signal_numeric(
+    Channel *chan, int signum, int core_dumped, ptrlen msg)
+{
+    return FALSE;
+}
+
 /* ----------------------------------------------------------------------
  * Common routine to marshal tty modes into an SSH packet.
  */

unix/uxpgnt.c

@@ -125,6 +125,11 @@ static int time_to_die = FALSE;
 void chan_remotely_opened_confirmation(Channel *chan) { }
 void chan_remotely_opened_failure(Channel *chan, const char *err) { }
 int chan_no_eager_close(Channel *chan, int s, int r) { return FALSE; }
+int chan_no_exit_status(Channel *ch, int s) { return FALSE; }
+int chan_no_exit_signal(Channel *ch, ptrlen s, int c, ptrlen m)
+{ return FALSE; }
+int chan_no_exit_signal_numeric(Channel *ch, int s, int c, ptrlen m)
+{ return FALSE; }
 
 /*
  * These functions are part of the plug for our connection to the X

x11fwd.c

@@ -731,6 +731,9 @@ static const struct ChannelVtable X11Connection_channelvt = {
     x11_set_input_wanted,
     x11_log_close_msg,
     chan_no_eager_close,
+    chan_no_exit_status,
+    chan_no_exit_signal,
+    chan_no_exit_signal_numeric,
 };
 
 /*