wolfsshd: fix shell and SCP session setup

apps/wolfsshd/test/run_all_sshd_tests.sh

@@ -13,8 +13,10 @@ test_cases=(
  "sshd_sftp_idle_cpu_test.sh"
  "sshd_bad_subsystem_test.sh"
  "sshd_scp_fail.sh"
+ "sshd_scp_home_test.sh"
  "sshd_term_close_test.sh"
  "sshd_stdin_eof_test.sh"
+ "sshd_exit_signal_test.sh"
  "sshd_stdin_stall_test.sh"
  "sshd_channel_close_test.sh"
  "ssh_kex_algos.sh"

apps/wolfsshd/test/sshd_exit_signal_test.sh

@@ -0,0 +1,72 @@
+#!/bin/bash
+# bash, unlike most of this directory: the option array below needs it.
+
+# A remote command killed by a signal must not report success. wolfSSHd sends
+# 128 plus the signal number as the exit status, as a shell reports it.
+#
+# Needs the OpenSSH client, which exits with the remote exit status.
+
+if [ -z "$1" ] || [ -z "$2" ]; then
+    echo "expecting host and port as arguments"
+    echo "./sshd_exit_signal_test.sh 127.0.0.1 22222"
+    exit 1
+fi
+
+HOST="$1"
+PORT="$2"
+USER_NAME="${3:-`whoami`}"
+
+command -v ssh >/dev/null 2>&1 || {
+    echo "ssh not found, skipping"
+    exit 77
+}
+
+command -v timeout >/dev/null 2>&1 || {
+    echo "timeout not found, skipping"
+    exit 77
+}
+
+# ssh refuses a group/world readable identity file.
+KEYDIR=`mktemp -d 2>/dev/null` || KEYDIR=`mktemp -d -t sshdsig`
+if [ -z "$KEYDIR" ] || [ ! -d "$KEYDIR" ]; then
+    echo "could not create temp dir"
+    exit 1
+fi
+trap 'rm -rf "$KEYDIR"' EXIT
+
+cp ../../../keys/hansel-key-ecc.pem "$KEYDIR/id_ecdsa" || exit 1
+chmod 600 "$KEYDIR/id_ecdsa"
+
+SSH_OPTS=(-i "$KEYDIR/id_ecdsa" -p "$PORT"
+    -o IdentitiesOnly=yes
+    -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null
+    -o PreferredAuthentications=publickey -o PasswordAuthentication=no
+    -o BatchMode=yes -o ConnectTimeout=5 -o LogLevel=ERROR)
+
+if ! timeout 20 ssh "${SSH_OPTS[@]}" "$USER_NAME@$HOST" true >/dev/null 2>&1
+then
+    echo "no session with these options, skipping"
+    exit 77
+fi
+
+# Control: a plain exit status comes through unchanged.
+timeout 20 ssh "${SSH_OPTS[@]}" "$USER_NAME@$HOST" 'exit 3' \
+    > /dev/null 2> "$KEYDIR/exit.err"
+RESULT=$?
+if [ "$RESULT" != 3 ]; then
+    echo "expected exit status 3, got $RESULT"
+    cat "$KEYDIR/exit.err"
+    exit 1
+fi
+
+# The shell running the command kills itself, so it is what the daemon reaps.
+timeout 20 ssh "${SSH_OPTS[@]}" "$USER_NAME@$HOST" 'kill -KILL $$' \
+    > /dev/null 2> "$KEYDIR/kill.err"
+RESULT=$?
+if [ "$RESULT" != 137 ]; then
+    echo "expected exit status 137 for SIGKILL, got $RESULT"
+    cat "$KEYDIR/kill.err"
+    exit 1
+fi
+
+exit 0

apps/wolfsshd/test/sshd_scp_home_test.sh

@@ -0,0 +1,62 @@
+#!/bin/sh
+
+# An SCP transfer with a relative remote path resolves it from the user's home
+# directory, not from the daemon's working directory.
+
+# Not named PWD: the shell rewrites that variable on every cd, so a saved
+# copy would not survive the cd to the repository root below.
+TESTDIR=`pwd`
+. ./wolfssh_options.sh
+cd ../../..
+
+TEST_SCP_CLIENT="./examples/scpclient/wolfscp"
+USER="${3:-`whoami`}"
+PRIVATE_KEY="./keys/hansel-key-ecc.der"
+PUBLIC_KEY="./keys/hansel-key-ecc.pub"
+
+if [ -z "$1" ] || [ -z "$2" ]; then
+    echo "expecting host and port as arguments"
+    echo "./sshd_scp_home_test.sh 127.0.0.1 22222"
+    exit 1
+fi
+
+if ! wolfssh_has SCP || [ ! -x "$TEST_SCP_CLIENT" ]; then
+    echo "SCP client not available in this build, skipping"
+    exit 77
+fi
+
+# The check reads the file back from the home directory directly.
+case "$1" in
+    127.0.0.1|localhost) ;;
+    *) echo "remote host, skipping"; exit 77 ;;
+esac
+
+HOMEDIR=`getent passwd "$USER" 2>/dev/null | cut -d: -f6`
+[ -n "$HOMEDIR" ] || HOMEDIR=`eval echo "~$USER"`
+if [ -z "$HOMEDIR" ] || [ ! -d "$HOMEDIR" ]; then
+    echo "no home directory for $USER, skipping"
+    exit 77
+fi
+
+NAME="scp-home-$$.dat"
+OUTDIR="`pwd`/test-$$"
+mkdir "$OUTDIR" || exit 1
+trap 'rm -rf "$OUTDIR"; rm -f "$HOMEDIR/$NAME"' EXIT
+
+dd if=/dev/urandom of="$OUTDIR/$NAME" bs=1024 count=64 2>/dev/null
+
+echo "$TEST_SCP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -L$OUTDIR/$NAME:. -H $1 -p $2"
+$TEST_SCP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -L"$OUTDIR/$NAME":. \
+    -H $1 -p $2
+if [ "$?" != "0" ]; then
+    echo "Expecting to pass transfer"
+    exit 1
+fi
+
+if ! cmp -s "$OUTDIR/$NAME" "$HOMEDIR/$NAME"; then
+    echo "File did not land in $HOMEDIR"
+    exit 1
+fi
+
+cd "$TESTDIR"
+exit 0

apps/wolfsshd/wolfsshd.c

@@ -1772,6 +1772,19 @@ static int SCP_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
          * privilege level */
         exit(1);
     }
+
+    /* Relative SCP paths resolve from home, inside a chroot too, as for
+     * SFTP and the shell. Like OpenSSH, a chroot without the home dir falls
+     * back to "/" quietly. */
+    if (ret == WS_SUCCESS && chdir(pPasswd->pw_dir) != 0) {
+        if (error != 1) {
+            wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error going to user home dir "
+                "%s, using /", pPasswd->pw_dir);
+        }
+        if (chdir("/") != 0) {
+            wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error going to /");
+        }
+    }
 #else
     /* impersonate the logged on user for file permissions */
     if (ImpersonateLoggedOnUser(wolfSSHD_GetAuthToken(conn->auth)) == FALSE) {
@@ -2604,6 +2617,8 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
     if (cnslOut != NULL) {
         CloseHandle(cnslOut);
     }
+    /* The resize callback must not reach ptyIn once it is closed. */
+    wolfSSH_SetTerminalResizeCtx(ssh, NULL);
     if (ptyIn != NULL) {
         CloseHandle(ptyIn);
     }
@@ -2679,6 +2694,42 @@ static int SHELL_SetNonBlocking(int fd)
     #define WOLFSSHD_CHILD_REAP_WAIT_US 50000
 #endif
 
+/* Close a descriptor that is still open and mark it closed. */
+static void SHELL_CloseFd(int* fd)
+{
+    if (*fd >= 0) {
+        close(*fd);
+        *fd = -1;
+    }
+}
+
+/* Take down a shell child that the parent cannot service: kill it, reap it,
+ * and close the parent's descriptors to it. */
+static void SHELL_AbortChild(pid_t childPid, int* childFd, int* stdoutPipe,
+    int* stderrPipe, int* stdinPipe)
+{
+    int status;
+    int i;
+
+    if (kill(childPid, SIGKILL) == 0) {
+        while (waitpid(childPid, &status, 0) < 0 && errno == EINTR) {
+            /* retry */
+        }
+    }
+    else {
+        /* EPERM while the child is still setting up at the raised
+         * privilege level. Closing the pty below hangs it up. */
+        wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to kill child process %d, "
+            "errno %d", (int)childPid, errno);
+    }
+    SHELL_CloseFd(childFd);
+    for (i = 0; i < 2; i++) {
+        SHELL_CloseFd(&stdoutPipe[i]);
+        SHELL_CloseFd(&stderrPipe[i]);
+        SHELL_CloseFd(&stdinPipe[i]);
+    }
+}
+
 /* Send out the last of a shell's output once the child has been reaped. The
  * SSH socket is non blocking, so retry a bounded number of times on a full
  * window, a rekey or a would block. 'ext' selects the extended (stderr) data
@@ -2909,6 +2960,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
         char cmd[MAX_COMMAND_SZ];
         char shell[MAX_COMMAND_SZ];
         int ret;
+        int chrooted;
 
         signal(SIGINT,  SIG_DFL);
         signal(SIGCHLD, SIG_DFL);
@@ -2946,11 +2998,12 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
         }
 
         rc = SetupChroot(usrConf);
+        chrooted = (rc == 1);
         if (rc < 0) {
             wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting chroot");
             exit(1);
         }
-        else if (rc == 1) {
+        else if (chrooted) {
             rc = chdir("/");
             if (rc != 0) {
                 wolfSSH_Log(WS_LOG_ERROR,
@@ -2986,9 +3039,16 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
 
         rc = chdir(pPasswd->pw_dir);
         if (rc != 0) {
-            /* not error'ing out if unable to find home directory */
-            wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error going to user home dir %s",
-            pPasswd->pw_dir);
+            /* not error'ing out if unable to find home directory. Like
+             * OpenSSH, a chroot without the home dir falls back to "/"
+             * quietly. */
+            if (!chrooted) {
+                wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error going to user home "
+                    "dir %s, using /", pPasswd->pw_dir);
+            }
+            if (chdir("/") != 0) {
+                wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error going to /");
+            }
         }
 
         /* default to /bin/sh if user shell is not set */
@@ -3048,24 +3108,30 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
         /* The shell child is already forked, and nothing below can reach the
          * peer without an id to address, so take it down with us. */
         wolfSSH_Log(WS_LOG_ERROR, "[SSHD] No session channel to service");
-        kill(childPid, SIGKILL);
+        SHELL_AbortChild(childPid, &childFd, stdoutPipe, stderrPipe,
+            stdinPipe);
         return WS_FATAL_ERROR;
     }
 
     signal(SIGINT, SIG_DFL);
 
     rc = tcgetattr(childFd, &tios);
-    if (rc != 0) {
-        return WS_FATAL_ERROR;
+    if (rc == 0) {
+        rc = tcsetattr(childFd, TCSAFLUSH, &tios);
     }
-    rc = tcsetattr(childFd, TCSAFLUSH, &tios);
     if (rc != 0) {
+        wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting up the shell's pty");
+        SHELL_AbortChild(childPid, &childFd, stdoutPipe, stderrPipe,
+            stdinPipe);
         return WS_FATAL_ERROR;
     }
 
     /* set initial size of terminal based on saved size */
 #if defined(HAVE_SYS_IOCTL_H)
-    wolfSSH_DoModes(ssh->modes, ssh->modesSz, childFd);
+    if (ssh->modesSz > 0 &&
+            wolfSSH_DoModes(ssh->modes, ssh->modesSz, childFd) != 0) {
+        wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error applying terminal modes");
+    }
     {
         struct winsize s;
 
@@ -3561,6 +3627,9 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
         stdinPipe[1] = -1;
     }
 
+    /* The resize callback must not reach childFd once it is closed. */
+    wolfSSH_SetTerminalResizeCtx(ssh, NULL);
+
     /* Close the pty master so the shell gets SIGHUP and can exit on its own */
     if (childFd >= 0) {
         close(childFd);
@@ -3600,8 +3669,16 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
                     "status");
         }
         else {
-            if (wolfSSH_SetExitStatus(ssh, (word32)WEXITSTATUS(waitStatus)) !=
-                WS_SUCCESS) {
+            word32 exitStatus = 1;
+
+            /* A signaled child reports 128 plus the signal, as a shell does */
+            if (WIFEXITED(waitStatus)) {
+                exitStatus = (word32)WEXITSTATUS(waitStatus);
+            }
+            else if (WIFSIGNALED(waitStatus)) {
+                exitStatus = 128 + (word32)WTERMSIG(waitStatus);
+            }
+            if (wolfSSH_SetExitStatus(ssh, exitStatus) != WS_SUCCESS) {
                 wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Issue setting child's exit "
                     "status");
             }
@@ -5050,7 +5127,7 @@ int main(int argc, char** argv)
             isService = 0;
         }
         if (WSTRCMP(argv[i], "-r") == 0) {
-            if (argc < i + 1) {
+            if (i + 1 >= argc) {
                 /* was expecting command to run after -r argument */
                 return -1;
             }

src/internal.c

@@ -13272,7 +13272,8 @@ int wolfSSH_DoModes(const byte* modes, word32 modesSz, int fd)
      * byte left over.
      */
 
-    tcgetattr(fd, &term);
+    if (tcgetattr(fd, &term) != 0)
+        return -1;
 
     while (idx + TERMINAL_MODE_SZ <= modesSz && modes[idx] != WOLFSSH_TTY_OP_END
             && modes[idx] < WOLFSSH_TTY_INVALID) {
@@ -13511,7 +13512,8 @@ int wolfSSH_DoModes(const byte* modes, word32 modesSz, int fd)
         idx += TERMINAL_MODE_SZ;
     }
 
-    tcsetattr(fd, TCSANOW, &term);
+    if (tcsetattr(fd, TCSANOW, &term) != 0)
+        return -1;
 
     return 0;
 }