exit-signal sent, but ignored

[Damien Miller]

On Sun, 4 Apr 2021, Christian Weisgerber wrote:

> I've been puzzling over the disparate exit codes returned by ssh
> when the remote command is killed by a signal:
> 
> $ ssh host1 'kill -HUP $$'; echo $?
> 129
> $ ssh host2 'kill -HUP $$'; echo $?
> 255
> 
> This results from the interaction of
> (a) the behavior of the login shell and
> (b) an implementation gap in OpenSSH's ssh(1) client.
> 
> If the login shell is bash, its exit triggers the WIFSIGNALED()
> check in session_exit_message() and an "exit-signal" message is
> sent.  However, client_input_channel_req() does not handle
> "exit-signal" at all, exit_status is not set and retains its
> initial value -1.  Therefore ssh returns 255.
> 
> If the login shell is OpenBSD's ksh, its exit triggers the WIFEXITED()
> check instead and an "exit-status" message is sent.  This is handled
> in client_input_channel_req, and ssh returns the received exit code
> of 128 + signal.
> 
> This is not the place to discuss the correctness of the respective
> shell behavior.
> 
> However, shouldn't ssh(1) handle "exit-signal" and synthesize an exit
> status of 128 + signal?

That's reasonable.

This implements exit-signal at the client:

diff --git a/clientloop.c b/clientloop.c
index 9fcf12e..0678cd3 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -1710,14 +1710,27 @@ client_input_channel_open(int type, u_int32_t seq, struct ssh *ssh)
 	return r;
 }
 
+static int
+sig_ntod(const char *s)
+{
+	int i;
+
+	for (i = 0; i < NSIG; i++) {
+		if (strcmp(sys_signame[i], s) == 0)
+			return i;
+	}
+	return -1;
+}
+
 static int
 client_input_channel_req(int type, u_int32_t seq, struct ssh *ssh)
 {
 	Channel *c = NULL;
-	char *rtype = NULL;
+	char *rtype = NULL, *signame = NULL;
 	u_char reply;
 	u_int id, exitval;
-	int r, success = 0;
+	u_char coredumped;
+	int r, success = 0, signum;
 
 	if ((r = sshpkt_get_u32(ssh, &id)) != 0)
 		return r;
@@ -1740,8 +1753,11 @@ client_input_channel_req(int type, u_int32_t seq, struct ssh *ssh)
 			goto out;
 		chan_rcvd_eow(ssh, c);
 	} else if (strcmp(rtype, "exit-status") == 0) {
-		if ((r = sshpkt_get_u32(ssh, &exitval)) != 0)
+		if ((r = sshpkt_get_u32(ssh, &exitval)) != 0 ||
+		    (r = sshpkt_get_end(ssh)) != 0) {
+			error_fr(r, "parse exit-status");
 			goto out;
+		}
 		if (c->ctl_chan != -1) {
 			mux_exit_message(ssh, c, exitval);
 			success = 1;
@@ -1751,11 +1767,39 @@ client_input_channel_req(int type, u_int32_t seq, struct ssh *ssh)
 			exit_status = exitval;
 		} else {
 			/* Probably for a mux channel that has already closed */
-			debug_f("no sink for exit-status on channel %d",
-			    id);
+			debug_f("no sink for exit-status on channel %d", id);
 		}
-		if ((r = sshpkt_get_end(ssh)) != 0)
+	} else if (strcmp(rtype, "exit-signal") == 0) {
+		if ((r = sshpkt_get_cstring(ssh, &signame, NULL)) != 0 ||
+		    (r = sshpkt_get_u8(ssh, &coredumped)) != 0 ||
+		    (r = sshpkt_get_cstring(ssh, NULL, NULL)) != 0 || /* msg */
+		    (r = sshpkt_get_cstring(ssh, NULL, NULL)) != 0 || /* lang */
+		    (r = sshpkt_get_end(ssh)) != 0) {
+			goto out;
+			error_fr(r, "parse exit-signal");
 			goto out;
+		}
+		if ((signum = sig_ntod(signame)) == -1)
+			exitval = 255;
+		else
+			exitval = 128 + signum;
+		if (c->ctl_chan != -1) {
+			debug("received SIG%s%s on mux channel %u",
+			    signame, coredumped ? " (core dumped)" : "", id);
+			/* XXX extend mux protocol to pass through exit-sig */
+			mux_exit_message(ssh, c, exitval);
+			success = 1;
+		} else if ((int)id == session_ident) {
+			/* Record exit value of local session */
+			debug("received SIG%s%s", signame,
+			    coredumped ? " (core dumped)" : "");
+			success = 1;
+			exit_status = exitval;
+		} else {
+			/* Probably for a mux channel that has already closed */
+			debug_f("no sink for exit-signal SIG%s on channel %u",
+			    signame, id);
+		}
 	}
 	if (reply && c != NULL && !(c->flags & CHAN_CLOSE_SENT)) {
 		if (!c->have_remote_id)
@@ -1769,6 +1813,7 @@ client_input_channel_req(int type, u_int32_t seq, struct ssh *ssh)
 	r = 0;
  out:
 	free(rtype);
+	free(signame);
 	return r;
 }