wolfsshd: refuse sessions it cannot serve

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"

apps/wolfsshd/test/sshd_bad_subsystem_test.sh

@@ -0,0 +1,68 @@
+#!/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=`whoami`
+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
+
+# OpenSSH refuses a key file other users can read.
+KEY=`mktemp`
+cat "$PRIVATE_KEY" > "$KEY"
+chmod 600 "$KEY"
+OUT=`mktemp`
+
+ssh_to_sshd() {
+    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"
+    rm -f "$KEY" "$OUT"
+    exit 1
+fi
+
+# A subsystem nothing serves: the client reports the refusal and exits
+# non-zero.
+ssh_to_sshd "$1" "$2" -s no-such-subsystem > "$OUT" 2>&1
+RESULT=$?
+if [ "$RESULT" = "0" ]; then
+    echo "Expecting the unknown subsystem request to fail"
+    cat "$OUT"
+    rm -f "$KEY" "$OUT"
+    exit 1
+fi
+if ! grep -q "subsystem request failed" "$OUT"; then
+    echo "Expecting the client to report the refused subsystem request:"
+    cat "$OUT"
+    rm -f "$KEY" "$OUT"
+    exit 1
+fi
+
+rm -f "$KEY" "$OUT"
+cd "$TESTDIR"
+exit 0

apps/wolfsshd/wolfsshd.c

@@ -359,6 +359,80 @@ 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
+            if (WSTRNCMP(cmd, "scp", 3) == 0) {
+                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
+            else if (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;
+    }
+
+    if (rej) {
+        wolfSSH_Log(WS_LOG_ERROR,
+            "[SSHD] Refusing session request from %s: %s [%s]",
+            conn->ip, reason, cmd != NULL ? cmd : "");
+    }
+
+    return rej;
+}
+
+
 /* Initializes and sets up the WOLFSSH_CTX struct based on the configure options
  * return WS_SUCCESS on success
  */
@@ -386,6 +460,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 */
@@ -2545,6 +2622,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. */

src/agent.c

@@ -1731,6 +1731,91 @@ int wolfSSH_AGENT_enable(WOLFSSH* ssh, byte isEnabled)
 }
 
 
+int wolfSSH_AGENT_ChannelOpen(WOLFSSH* ssh)
+{
+    WOLFSSH_AGENT_CTX* newAgent = NULL;
+    WOLFSSH_CHANNEL* newChannel = NULL;
+    int ret = WS_SUCCESS;
+    /* wolfSSH_accept() clears only want-read/want-write/auth-pending, so a
+     * WS_BAD_ARGUMENT latched by a poll kills the handshake. */
+    int recordError = 0;
+
+    WLOG_ENTER();
+
+    if (ssh == NULL)
+        ret = WS_SSH_NULL_E;
+    else if (ssh->ctx->side != WOLFSSH_ENDPOINT_SERVER) {
+        /* Server side only. wolfSSH_connect() sets ssh->agent too, so the
+         * checks below would report a channel a client never opened. */
+        ret = WS_BAD_ARGUMENT;
+    }
+    else if (SendAfterDisconnect(ssh)) {
+        /* The session is over, so neither a new open nor the flush of one
+         * queued before the disconnect may go out. RFC 4253 section 11.1.
+         * WS_DISCONNECT is in ssh->error, where the rest of the API puts
+         * it. */
+        ret = WS_FATAL_ERROR;
+    }
+    else if (!ssh->useAgent) {
+        /* Nothing asked for agent forwarding on this session. */
+        ret = WS_BAD_ARGUMENT;
+    }
+    else if (ssh->agent == NULL) {
+        /* Nothing else sets ssh->agent, so a NULL one means "not opened
+         * yet". Idempotent, so a poll cannot open a second channel. */
+        WLOG(WS_LOG_AGENT, "Starting agent channel");
+
+        newAgent = wolfSSH_AGENT_new(ssh->ctx->heap);
+        if (newAgent == NULL)
+            ret = WS_MEMORY_E;
+
+        if (ret == WS_SUCCESS) {
+            newChannel = ChannelNew(ssh, ID_CHANTYPE_AUTH_AGENT,
+                    ssh->ctx->windowSz, ssh->ctx->maxPacketSz);
+            if (newChannel == NULL)
+                ret = WS_MEMORY_E;
+        }
+
+        if (ret == WS_SUCCESS) {
+            recordError = 1;
+            ret = SendChannelOpenSession(ssh, newChannel);
+
+            if (ret < WS_SUCCESS
+                    && ret != WS_WANT_WRITE && ret != WS_WANT_READ) {
+                ChannelDelete(newChannel, ssh->ctx->heap);
+            }
+            else {
+                /* Publish on a queued open too, so a retry takes the
+                 * already-open path rather than opening a second. */
+                ChannelAppend(ssh, newChannel);
+                newAgent->channel = newChannel->channel;
+                ssh->agent = newAgent;
+                newAgent = NULL;
+                if (ssh->ctx->agentCb) {
+                    ssh->ctx->agentCb(WOLFSSH_AGENT_LOCAL_SETUP,
+                            ssh->agentCbCtx);
+                }
+            }
+        }
+
+        if (newAgent != NULL)
+            wolfSSH_AGENT_free(newAgent);
+    }
+    else if (wolfSSH_OutputPending(ssh)) {
+        /* Any queued output, not just this open. Flush it rather than
+         * report a success the peer hasn't seen. */
+        recordError = 1;
+        ret = wolfSSH_SendPacket(ssh);
+    }
+
+    if (recordError)
+        ssh->error = ret;
+
+    WLOG_LEAVE(ret);
+    return ret;
+}
+
+
 int wolfSSH_AGENT_worker(WOLFSSH* ssh)
 {
     int ret = WS_SUCCESS;

src/internal.c

@@ -1672,6 +1672,7 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx)
     ssh->highwaterMark = ctx->highwaterMark;
     ssh->msgHighwaterMark = ctx->msgHighwaterMark;
     ssh->maxAuthAttempts = ctx->maxAuthAttempts;
+    ssh->appChannels = ctx->appChannels;
     ssh->highwaterCtx  = (void*)ssh;
     ssh->reqSuccessCtx = (void*)ssh;
     ssh->fs            = NULL;
@@ -12666,6 +12667,61 @@ static void SetTerminalSize(WOLFSSH* ssh, word32 widthChar, word32 heightRows,
 #endif /* WOLFSSH_TERM */
 
 
+/* Answers a shell, exec, or subsystem request. The session type, and the
+ * command for the two that carry one, are set for the callback to read and
+ * kept only if it accepts; a refused request leaves the channel as it was
+ * and the accept loop still waiting, so nothing serves a session the
+ * application turned down. Without a callback the request is accepted,
+ * unless the application drives its own channels. */
+static int DoChannelRequestSession(WOLFSSH* ssh, WOLFSSH_CHANNEL* channel,
+        byte sessionType, WS_CallbackChannelReq cb,
+        byte* buf, word32 len, word32* idx, int* rej)
+{
+    char* prevCommand = NULL;
+    byte prevType = channel->sessionType;
+    byte hasCommand = (sessionType != WOLFSSH_SESSION_SHELL);
+    int ret = WS_SUCCESS;
+
+    if (hasCommand) {
+        prevCommand = channel->command;
+        channel->command = NULL;
+        ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL,
+                buf, len, idx);
+        if (ret == WS_SUCCESS) {
+            WLOG(WS_LOG_DEBUG, "  command = %s", channel->command);
+        }
+    }
+
+    if (ret == WS_SUCCESS) {
+        channel->sessionType = sessionType;
+        if (cb != NULL) {
+            *rej = cb(channel, ssh->channelReqCtx);
+        }
+        else {
+            *rej = ssh->appChannels;
+        }
+    }
+
+    if (ret == WS_SUCCESS && !*rej) {
+        if (prevCommand != NULL) {
+            WFREE(prevCommand, ssh->ctx->heap, DYNTYPE_STRING);
+        }
+        ssh->clientState = CLIENT_DONE;
+    }
+    else {
+        if (hasCommand) {
+            if (channel->command != NULL) {
+                WFREE(channel->command, ssh->ctx->heap, DYNTYPE_STRING);
+            }
+            channel->command = prevCommand;
+        }
+        channel->sessionType = prevType;
+    }
+
+    return ret;
+}
+
+
 static int DoChannelRequest(WOLFSSH* ssh,
                             byte* buf, word32 len, word32* idx)
 {
@@ -12721,33 +12777,17 @@ static int DoChannelRequest(WOLFSSH* ssh,
             WLOG(WS_LOG_DEBUG, "  %s = %s", name, value);
         }
         else if (ChannelRequestIs(type, typeSz, "shell")) {
-            channel->sessionType = WOLFSSH_SESSION_SHELL;
-            if (ssh->ctx->channelReqShellCb) {
-                rej = ssh->ctx->channelReqShellCb(channel, ssh->channelReqCtx);
-            }
-            ssh->clientState = CLIENT_DONE;
+            ret = DoChannelRequestSession(ssh, channel, WOLFSSH_SESSION_SHELL,
+                    ssh->ctx->channelReqShellCb, buf, len, &begin, &rej);
         }
         else if (ChannelRequestIs(type, typeSz, "exec")) {
-            ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL,
-                    buf, len, &begin);
-            channel->sessionType = WOLFSSH_SESSION_EXEC;
-            if (ssh->ctx->channelReqExecCb) {
-                rej = ssh->ctx->channelReqExecCb(channel, ssh->channelReqCtx);
-            }
-            ssh->clientState = CLIENT_DONE;
-
-            WLOG(WS_LOG_DEBUG, "  command = %s", channel->command);
+            ret = DoChannelRequestSession(ssh, channel, WOLFSSH_SESSION_EXEC,
+                    ssh->ctx->channelReqExecCb, buf, len, &begin, &rej);
         }
         else if (ChannelRequestIs(type, typeSz, "subsystem")) {
-            ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL,
-                    buf, len, &begin);
-            channel->sessionType = WOLFSSH_SESSION_SUBSYSTEM;
-            if (ssh->ctx->channelReqSubsysCb) {
-                rej = ssh->ctx->channelReqSubsysCb(channel, ssh->channelReqCtx);
-            }
-            ssh->clientState = CLIENT_DONE;
-
-            WLOG(WS_LOG_DEBUG, "  subsystem = %s", channel->command);
+            ret = DoChannelRequestSession(ssh, channel,
+                    WOLFSSH_SESSION_SUBSYSTEM, ssh->ctx->channelReqSubsysCb,
+                    buf, len, &begin, &rej);
         }
         #ifdef WOLFSSH_TERM
         else if (ChannelRequestIs(type, typeSz, "pty-req")) {
@@ -12886,7 +12926,7 @@ static int DoChannelRequest(WOLFSSH* ssh,
         int replyRet;
 
         if (rej) {
-            WLOG(WS_LOG_DEBUG, "Callback rejecting channel request.");
+            WLOG(WS_LOG_DEBUG, "Rejecting channel request.");
         }
         replyRet = SendChannelSuccess(ssh, channelId,
                 (ret == WS_SUCCESS && !rej));