wolfsshd: apply ForceCommand to SFTP and SCP

apps/wolfsshd/configuration.c

@@ -462,8 +462,6 @@ void wolfSSHD_ConfigFree(WOLFSSHD_CONFIG* conf)
     }
 }
 
-#define MAX_LINE_SIZE 160
-
 typedef struct {
     int tag;
     const char* name;
@@ -1739,7 +1737,7 @@ static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth)
     WFILE *f;
     WOLFSSHD_CONFIG* currentConfig;
     int ret = WS_SUCCESS;
-    char buf[MAX_LINE_SIZE];
+    char buf[WOLFSSHD_MAX_LINE_SIZE];
     const char* current;
 
     if (conf == NULL || filename == NULL)
@@ -1761,9 +1759,26 @@ static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth)
     depth++;
 
     currentConfig = conf;
-    while ((current = XFGETS(buf, MAX_LINE_SIZE, f)) != NULL) {
+    while ((current = XFGETS(buf, WOLFSSHD_MAX_LINE_SIZE, f)) != NULL) {
         int currentSz = (int)XSTRLEN(current);
 
+        /* A full buffer with no newline is a cut line, and its leftover is
+         * read as a line of its own, so going on applies a truncated
+         * directive. One more byte tells a cut line from one that ends at
+         * the buffer: a newline, or the end of the file. */
+        if (currentSz == WOLFSSHD_MAX_LINE_SIZE - 1 &&
+                buf[currentSz - 1] != '\n') {
+            char next;
+
+            if (WFREAD(NULL, &next, 1, 1, f) == 1 && next != '\n') {
+                wolfSSH_Log(WS_LOG_ERROR,
+                    "[SSHD] Config line longer than %d in %s",
+                    WOLFSSHD_MAX_LINE_SIZE - 1, filename);
+                ret = WS_FATAL_ERROR;
+                break;
+            }
+        }
+
         /* remove leading spaces and tabs */
         while (currentSz > 0 &&
                 (current[0] == ' ' || current[0] == '\t')) {

apps/wolfsshd/configuration.h

@@ -40,6 +40,9 @@ typedef struct WOLFSSHD_CONFIG WOLFSSHD_CONFIG;
 #define WOLFSSHD_WIN_STORE_CONFIG
 #endif
 
+/* Longest config line. ConfigLoad() refuses a longer one. */
+#define WOLFSSHD_MAX_LINE_SIZE 1024
+
 #include "auth.h"
 
 /* 0 so that privilege separation is default on after struct memset'd on init */

apps/wolfsshd/test/sshd_forcedcmd_test.sh

@@ -21,7 +21,45 @@ source ./start_sshd.sh
 # root daemon holding the shared test port, so every later test in the suite
 # would talk to this config. stop_wolfsshd clears PID, so this is a no-op after
 # each explicit stop below.
-trap stop_wolfsshd EXIT
+cleanup() {
+    stop_wolfsshd
+    rm -f "$TESTDIR/scp_src.dat" "$TESTDIR/scp_dst.dat"
+    rm -f "$TESTDIR/sshd_config_test_forcedcmd_args"
+    rm -f "$TESTDIR/sshd_config_test_forcedcmd_prefix"
+}
+trap cleanup EXIT
+
+# The daemon's decisions are read from log.txt, which the whole run shares:
+# log_mark notes its length and log_since prints what came after.
+log_mark() {
+    sudo cat "$TESTDIR/log.txt" | wc -l
+}
+
+log_since() {
+    sudo cat "$TESTDIR/log.txt" | tail -n +`expr $1 + 1`
+}
+
+# Native SCP is an exec, a path of its own. wolfscp masks its exit code, so
+# the decision is read from the log. Fails the test unless SCP is denied.
+check_scp_denied() {
+    if [ ! -x "$TEST_SCP" ]; then
+        return 0
+    fi
+    MARK=`log_mark`
+    rm -f "$TESTDIR/scp_dst.dat"
+    echo "scp payload" > "$TESTDIR/scp_src.dat"
+    $TEST_SCP -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY \
+        -S"$TESTDIR/scp_src.dat:$TESTDIR/scp_dst.dat" \
+        -H $TEST_HOST -p $TEST_PORT > /dev/null 2>&1 || true
+    if ! log_since $MARK | grep -q "denying SCP"; then
+        echo "SCP should be denied with $1"
+        exit 1
+    fi
+    if [ -f "$TESTDIR/scp_dst.dat" ]; then
+        echo "SCP wrote its destination despite $1"
+        exit 1
+    fi
+}
 
 cat <<EOF > sshd_config_test_forcedcmd
 Port $TEST_PORT
@@ -44,6 +82,7 @@ cd ../../..
 
 TEST_CLIENT="./examples/client/client"
 TEST_SFTP="./examples/sftpclient/wolfsftp"
+TEST_SCP="./examples/scpclient/wolfscp"
 PRIVATE_KEY="./keys/hansel-key-ecc.der"
 PUBLIC_KEY="./keys/hansel-key-ecc.pub"
 
@@ -59,12 +98,91 @@ fi
 set -e
 echo exit | $TEST_SFTP -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p $TEST_PORT
 
+check_scp_denied "ForceCommand internal-sftp"
+
+cd "$TESTDIR"
+stop_wolfsshd
+
+# sshd_config(5) permits options after "internal-sftp", but none is
+# applied: shell is still refused and SFTP is refused too.
+cat <<EOF > sshd_config_test_forcedcmd_args
+Port $TEST_PORT
+Protocol 2
+LoginGraceTime 600
+PermitRootLogin yes
+PasswordAuthentication yes
+PermitEmptyPasswords no
+UsePrivilegeSeparation no
+UseDNS no
+HostKey $TESTDIR/../../../keys/server-key.pem
+AuthorizedKeysFile $TESTDIR/authorized_keys_test
+
+Match User $USER
+	ForceCommand internal-sftp -u 0002
+EOF
+
+start_wolfsshd "sshd_config_test_forcedcmd_args"
+cd ../../..
+
+# The refused login exits non-zero and this runs under the "set -e" above.
+# Running "internal-sftp -u 0002" as a command would print no "bob" either,
+# so the refusal is read from the log.
+MARK=`log_mark`
+RESULT=$( $TEST_CLIENT -c 'echo bob' -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p $TEST_PORT ) || true
+if echo $RESULT | grep bob ||
+        ! log_since $MARK | grep -q "Only SFTP connections allowed"; then
+    echo "Shell login should fail with internal-sftp and options"
+    exit 1
+fi
+
+MARK=`log_mark`
+if echo exit | $TEST_SFTP -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY \
+        -h $TEST_HOST -p $TEST_PORT; then
+    echo "SFTP should fail with internal-sftp and options"
+    exit 1
+fi
+if ! log_since $MARK | grep -q "internal-sftp options are not supported"; then
+    echo "SFTP should be refused for its internal-sftp options"
+    exit 1
+fi
+
 cd "$TESTDIR"
 stop_wolfsshd
 
-# A configured ForceCommand that is not "internal-sftp" must still permit the
-# SFTP subsystem. Only a certificate force-command denies file transfer, so a
-# long-standing ForceCommand config keeps working.
+# A name that only starts with "internal-sftp" is some other command.
+sed 's/ForceCommand internal-sftp -u 0002/ForceCommand internal-sftpd/' \
+    sshd_config_test_forcedcmd_args > sshd_config_test_forcedcmd_prefix
+
+start_wolfsshd "sshd_config_test_forcedcmd_prefix"
+cd ../../..
+
+# Refused as a forced command, not as internal-sftp with an option "d".
+MARK=`log_mark`
+if echo exit | $TEST_SFTP -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY \
+        -h $TEST_HOST -p $TEST_PORT; then
+    echo "SFTP should fail with a command named like internal-sftp"
+    exit 1
+fi
+if ! log_since $MARK | grep -q "denying subsystem request" ||
+        log_since $MARK | grep -q "options are not supported"; then
+    echo "internal-sftpd was taken for internal-sftp"
+    exit 1
+fi
+
+# The shell runs it as the forced command, without the internal-sftp refusal.
+MARK=`log_mark`
+$TEST_CLIENT -c 'echo bob' -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY \
+    -h $TEST_HOST -p $TEST_PORT > /dev/null 2>&1 || true
+if log_since $MARK | grep -q "Only SFTP connections allowed"; then
+    echo "Shell refused internal-sftpd as internal-sftp"
+    exit 1
+fi
+
+cd "$TESTDIR"
+stop_wolfsshd
+
+# A ForceCommand other than "internal-sftp" denies the SFTP subsystem and
+# native SCP: the client may only run the forced command.
 cat <<EOF > sshd_config_test_forcedcmd_sftp
 Port $TEST_PORT
 Protocol 2
@@ -84,7 +202,13 @@ EOF
 start_wolfsshd "sshd_config_test_forcedcmd_sftp"
 cd ../../..
 
-echo exit | $TEST_SFTP -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p $TEST_PORT
+if echo exit | $TEST_SFTP -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY \
+        -h $TEST_HOST -p $TEST_PORT; then
+    echo "SFTP should fail with a forced command other than internal-sftp"
+    exit 1
+fi
+
+check_scp_denied "a forced command"
 
 cd "$TESTDIR"
 stop_wolfsshd

apps/wolfsshd/test/sshd_ossh_cert_test.sh

@@ -228,8 +228,8 @@ HostKey $HOSTKEY
 TrustedUserCAKeys $WORK/trusted-cas.pub
 EOF
 
-# Second config, adding a ForceCommand that is not "internal-sftp". Used to
-# separate the config-sourced command from the certificate's.
+# Second config, adding a ForceCommand that is not "internal-sftp", which
+# denies SFTP on its own.
 cat > "$WORK/sshd_config_ossh_fc" <<EOF
 Port $PORT
 Protocol 2
@@ -243,6 +243,22 @@ Match User $LOGINUSER
 	ForceCommand /bin/echo
 EOF
 
+# Third config, a ForceCommand "internal-sftp", which permits SFTP. Only here
+# does a certificate's force-command change the outcome, so it shows the
+# configured command does not mask the certificate's.
+cat > "$WORK/sshd_config_ossh_isftp" <<EOF
+Port $PORT
+Protocol 2
+UsePrivilegeSeparation no
+UseDNS no
+PasswordAuthentication no
+HostKey $HOSTKEY
+TrustedUserCAKeys $WORK/trusted-cas.pub
+
+Match User $LOGINUSER
+	ForceCommand internal-sftp
+EOF
+
 CONFIG="$WORK/sshd_config_ossh"
 FAIL=0
 DRIVER=""
@@ -453,13 +469,16 @@ run_suite() { # driver
         sftp_check "internal-sftp sftp" "$ED" \
             "$CERTS/$LOGINUSER-ossh-internalsftp-cert.pub" 0
 
-        # A configured ForceCommand is not a certificate force-command: on its
-        # own it must not deny SFTP, and it must not mask one carried by a
-        # certificate.
+        # A configured ForceCommand denies SFTP on its own.
         CONFIG="$WORK/sshd_config_ossh_fc"
         sftp_check "config forcecmd sftp" "$ED" \
+            "$CERTS/$LOGINUSER-ossh-cert.pub"              1
+
+        # A configured "internal-sftp" does not mask a certificate's.
+        CONFIG="$WORK/sshd_config_ossh_isftp"
+        sftp_check "config isftp sftp" "$ED" \
             "$CERTS/$LOGINUSER-ossh-cert.pub"              0
-        sftp_check "config+cert sftp deny" "$ED" \
+        sftp_check "config isftp+cert sftp deny" "$ED" \
             "$CERTS/$LOGINUSER-ossh-forcecmd-cert.pub"     1
         CONFIG="$WORK/sshd_config_ossh"
     else
@@ -476,6 +495,12 @@ run_suite() { # driver
             "$CERTS/$LOGINUSER-ossh-forcecmd-cert.pub"     1
         scp_check "internal-sftp scp"   "$ED" \
             "$CERTS/$LOGINUSER-ossh-internalsftp-cert.pub" 1
+
+        # A configured ForceCommand denies it too.
+        CONFIG="$WORK/sshd_config_ossh_fc"
+        scp_check "config forcecmd scp" "$ED" \
+            "$CERTS/$LOGINUSER-ossh-cert.pub"              1
+        CONFIG="$WORK/sshd_config_ossh"
     fi
 }