Mailing List Archive

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;
}

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Damien Miller:

> > However, shouldn't ssh(1) handle "exit-signal" and synthesize an exit
> > status of 128 + signal?
>
> This implements exit-signal at the client:
>
> diff --git a/clientloop.c b/clientloop.c
> index 9fcf12e..0678cd3 100644

Yes! I successfully tested this for plain as well as shared
connections.

Portability of sys_signame[] may be a problem, I don't know.

--
Christian "naddy" Weisgerber naddy@mips.inka.de
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev