From 74a296d9c2b1d23c2f81308449bc6ebb8accaaf5 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2026 12:01:19 -0700 Subject: [PATCH] wolfsshd: refuse sessions it cannot serve A shell, exec or subsystem request is answered as it arrives, through the channel request callbacks, so a session this build cannot serve, or a second one on a channel already running one, is refused with CHANNEL_FAILURE rather than accepted and then dropped once the session is up. What the daemon serves is unchanged. - SessionRequestCb() takes a shell with WOLFSSH_SHELL, an exec with WOLFSSH_SHELL or an scp command with WOLFSSH_SCP, and the sftp subsystem with WOLFSSH_SFTP; anything else is refused and logged - a request whose command did not fit is refused rather than read through a NULL - a second program start is refused on a channel whose grant already stands, so sftp or scp cannot take over a running session - the sftp name is matched whole and scp only as its own token, both by length and bytes, so "scpbackup" or a name with an embedded NUL is some other command - sshd_bad_subsystem_test.sh asks for an unknown subsystem with the OpenSSH client and expects the refusal --- apps/wolfsshd/test/run_all_sshd_tests.sh | 1 + apps/wolfsshd/test/sshd_bad_subsystem_test.sh | 90 +++++++++++++++++ apps/wolfsshd/wolfsshd.c | 99 +++++++++++++++++++ 3 files changed, 190 insertions(+) create mode 100755 apps/wolfsshd/test/sshd_bad_subsystem_test.sh diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 78c76f46..fb7395d5 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -9,6 +9,7 @@ test_cases=( "sshd_large_sftp_test.sh" "sshd_bad_sftp_test.sh" "sshd_sftp_idle_cpu_test.sh" + "sshd_bad_subsystem_test.sh" "sshd_scp_fail.sh" "sshd_term_close_test.sh" "sshd_stdin_eof_test.sh" diff --git a/apps/wolfsshd/test/sshd_bad_subsystem_test.sh b/apps/wolfsshd/test/sshd_bad_subsystem_test.sh new file mode 100755 index 00000000..bf91f5f2 --- /dev/null +++ b/apps/wolfsshd/test/sshd_bad_subsystem_test.sh @@ -0,0 +1,90 @@ +#!/bin/sh + +# sshd local test: a subsystem the daemon does not serve is refused at the +# request, so the client sees CHANNEL_FAILURE rather than a session that +# is accepted and then dropped. Uses the system OpenSSH client, since the +# in-tree clients only ask for sftp. + +# 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` +cd ../../.. + +USER="$3" +if [ -z "$USER" ]; then + USER=`whoami` +fi +PRIVATE_KEY="./keys/hansel-key-ecc.pem" + +if [ -z "$1" ] || [ -z "$2" ]; then + echo "expecting host and port as arguments" + echo "./sshd_bad_subsystem_test.sh 127.0.0.1 22222" + exit 1 +fi + +if ! command -v ssh >/dev/null 2>&1; then + echo "OpenSSH client not found, skipping" + exit 77 +fi + +# The regression this test looks for is a request the daemon never answers, +# which leaves the client waiting. Bound every call so that hangs the test +# rather than the suite. +if ! command -v timeout >/dev/null 2>&1; then + echo "timeout not found, skipping" + exit 77 +fi +TIMEOUT="timeout 20" + +# OpenSSH refuses a key file other users can read. +KEY=`mktemp 2>/dev/null` || KEY=`mktemp -t sshdbadsubsys` +OUT=`mktemp 2>/dev/null` || OUT=`mktemp -t sshdbadsubsysout` +if [ -z "$KEY" ] || [ ! -f "$KEY" ] || [ -z "$OUT" ] || [ ! -f "$OUT" ]; then + echo "could not create temp files" + rm -f "$KEY" "$OUT" + exit 1 +fi +trap 'rm -f "$KEY" "$OUT"' EXIT + +cat "$PRIVATE_KEY" > "$KEY" || exit 1 +chmod 600 "$KEY" + +ssh_to_sshd() { + $TIMEOUT ssh -p "$2" -i "$KEY" -o IdentitiesOnly=yes \ + -o StrictHostKeyChecking=no \ + -o UserKnownHostsFile=/dev/null -o PreferredAuthentications=publickey \ + -o BatchMode=yes -o ConnectTimeout=5 "$USER@$1" "$3" "$4" +} + +# Control: the same client and key can run a command. +ssh_to_sshd "$1" "$2" "echo ok" > "$OUT" 2>&1 +RESULT=$? +if [ "$RESULT" != "0" ] || ! grep -q "^ok" "$OUT"; then + echo "Control exec through OpenSSH failed ($RESULT):" + cat "$OUT" + exit 1 +fi + +# A subsystem nothing serves: the client reports the refusal and exits +# non-zero. Check the timeout first, its 124 is non-zero too but means the +# request went unanswered, the opposite of what this test wants. +ssh_to_sshd "$1" "$2" -s no-such-subsystem > "$OUT" 2>&1 +RESULT=$? +if [ "$RESULT" = "124" ]; then + echo "The unknown subsystem request went unanswered:" + cat "$OUT" + exit 1 +fi +if [ "$RESULT" = "0" ]; then + echo "Expecting the unknown subsystem request to fail" + cat "$OUT" + exit 1 +fi +if ! grep -q "subsystem request failed" "$OUT"; then + echo "Expecting the client to report the refused subsystem request:" + cat "$OUT" + exit 1 +fi + +cd "$TESTDIR" +exit 0 diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index a3d0389d..218b4b19 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -471,6 +471,101 @@ static void CleanupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, (void)conf; } +/* Answers a shell, exec or subsystem request as it arrives: a session this + * build cannot serve is refused with CHANNEL_FAILURE, rather than accepted + * and then dropped once the session is up. Returns 0 to accept and 1 to + * refuse. The command is NULL when the request carried none that fit. */ +static int SessionRequestCb(WOLFSSH_CHANNEL* channel, void* vCtx) +{ + WOLFSSHD_CONNECTION* conn = (WOLFSSHD_CONNECTION*)vCtx; + const char* cmd; + const char* reason = NULL; + int rej = 1; + + if (conn == NULL || channel == NULL) { + return 1; + } + + cmd = wolfSSH_ChannelGetSessionCommand(channel); + switch (wolfSSH_ChannelGetSessionType(channel)) { + case WOLFSSH_SESSION_SHELL: + #ifdef WOLFSSH_SHELL + rej = 0; + #else + reason = "shell support is disabled"; + #endif + break; + + case WOLFSSH_SESSION_EXEC: + if (cmd == NULL) { + reason = "exec request carried no command"; + break; + } + #ifdef WOLFSSH_SCP + { + word32 cmdSz = wolfSSH_ChannelGetSessionCommandSz(channel); + + /* "scp" must stand as its own token; a prefix match + * grants "scpbackup", and a NUL makes it another. */ + if (cmdSz >= (word32)WSTRLEN("scp") + && WSTRNCMP(cmd, "scp", 3) == 0 + && (cmdSz == (word32)WSTRLEN("scp") + || cmd[3] == ' ')) { + rej = 0; + break; + } + } + #endif + #ifdef WOLFSSH_SHELL + rej = 0; + #else + reason = "exec support is disabled"; + #endif + break; + + case WOLFSSH_SESSION_SUBSYSTEM: + if (cmd == NULL) { + reason = "subsystem request carried no name"; + } + #ifdef WOLFSSH_SFTP + /* Matched whole, length and bytes, as the sftp divert asks: + * sftp with an embedded NUL is another subsystem. */ + else if (wolfSSH_ChannelGetSessionCommandSz(channel) + == (word32)WSTRLEN("sftp") + && WSTRCMP(cmd, "sftp") == 0) { + rej = 0; + } + #endif + else { + reason = "unknown or unsupported subsystem"; + } + break; + + case WOLFSSH_SESSION_UNKNOWN: + case WOLFSSH_SESSION_TERMINAL: + default: + reason = "unsupported session type"; + break; + } + + /* One program start per channel, as RFC 4254 section 6.5 allows. This + * request's grant is recorded once the callback returns, so a flag + * already set is an earlier request's. */ + if (!rej && channel->sessionGranted) { + rej = 1; + reason = "a session is already running on the channel"; + } + + if (rej) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Refusing session request from %s: %s [%s]", + conn->ip, reason, cmd != NULL ? cmd : ""); + } + + return rej; +} + + #if defined(WOLFSSH_CERTS) && defined(WOLFSSH_WINDOWS_CERT_STORE) /* Returns 1 only for the store hives that need elevation to write: the three * LOCAL_MACHINE locations. Every other hive (per-user, per-service, @@ -893,6 +988,9 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, if (ret == WS_SUCCESS) { wolfSSH_SetUserAuth(*ctx, DefaultUserAuth); wolfSSH_SetUserAuthResult(*ctx, UserAuthResult); + wolfSSH_CTX_SetChannelReqShellCb(*ctx, SessionRequestCb); + wolfSSH_CTX_SetChannelReqExecCb(*ctx, SessionRequestCb); + wolfSSH_CTX_SetChannelReqSubsysCb(*ctx, SessionRequestCb); } /* set banner to display on connection */ @@ -3511,6 +3609,7 @@ static void* HandleConnection(void* arg) /* let UserAuthResult reach this connection to cancel the grace timer * and to reach conn->auth for the cert force-command */ wolfSSH_SetUserAuthResultCtx(ssh, conn); + wolfSSH_SetChannelReqCtx(ssh, conn); #if defined(WOLFSSH_OSSH_CERTS) && !defined(_WIN32) /* Unix-only: each connection is a forked child with its own copy of the * auth struct. Windows does not enforce OpenSSH certs. */