diff --git a/src/internal.c b/src/internal.c index b2153e323..6f717405e 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1781,6 +1781,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; @@ -4198,8 +4199,11 @@ void ChannelDelete(WOLFSSH_CHANNEL* channel, void* heap) channel->channel); } ShrinkBuffer(&channel->extDataBuffer, 1); - if (channel->command) + /* Scrub the peer's command line, which can carry credentials. */ + if (channel->command != NULL) { + WS_FORCEZERO(channel->command, channel->commandSz); WFREE(channel->command, heap, DYNTYPE_STRING); + } WFREE(channel, heap, DYNTYPE_CHANNEL); } } @@ -13071,6 +13075,17 @@ static void SetTerminalSize(WOLFSSH* ssh, word32 widthChar, word32 heightRows, #endif /* WOLFSSH_TERM */ +/* Wipe the old command ahead of the GetStringAlloc() that frees it, so a + * repeat request leaves no credentials behind in the freed block. */ +static void ScrubChannelCommand(WOLFSSH_CHANNEL* channel) +{ + if (channel->command != NULL) { + WS_FORCEZERO(channel->command, channel->commandSz); + channel->commandSz = 0; + } +} + + static int DoChannelRequest(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) { @@ -13080,7 +13095,7 @@ static int DoChannelRequest(WOLFSSH* ssh, word32 typeSz; char type[32]; byte wantReply; - int ret, rej = 0; + int ret, rej = 0, sessionReq = 0; WLOG(WS_LOG_DEBUG, "Entering DoChannelRequest()"); @@ -13097,8 +13112,7 @@ static int DoChannelRequest(WOLFSSH* ssh, WLOG(WS_LOG_DEBUG, "Leaving DoChannelRequest(), ret = %d", ret); return ret; } - - if (ret == WS_SUCCESS) { + else { channel = ChannelFind(ssh, channelId, WS_CHANNEL_ID_SELF); if (channel == NULL) ret = WS_INVALID_CHANID; @@ -13120,39 +13134,70 @@ static int DoChannelRequest(WOLFSSH* ssh, nameSz = (word32)sizeof(name); valueSz = (word32)sizeof(value); ret = GetString(name, &nameSz, buf, len, &begin); - if (ret == WS_SUCCESS) + if (ret != WS_SUCCESS) + WLOG(WS_LOG_DEBUG, " name = %s", ""); + else { ret = GetString(value, &valueSz, buf, len, &begin); - - WLOG(WS_LOG_DEBUG, " %s = %s", name, value); + if (ret != WS_SUCCESS) + WLOG(WS_LOG_DEBUG, " %s = %s", name, ""); + else + 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); } + else { + rej = ssh->appChannels; + } + sessionReq = 1; ssh->clientState = CLIENT_DONE; } else if (ChannelRequestIs(type, typeSz, "exec")) { - ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL, + ScrubChannelCommand(channel); + ret = GetStringAlloc(ssh->ctx->heap, + &channel->command, &channel->commandSz, buf, len, &begin); - channel->sessionType = WOLFSSH_SESSION_EXEC; - if (ssh->ctx->channelReqExecCb) { - rej = ssh->ctx->channelReqExecCb(channel, ssh->channelReqCtx); + if (ret == WS_SUCCESS) + WLOG(WS_LOG_DEBUG, " command = %s", channel->command); + else + WLOG(WS_LOG_DEBUG, " command = %s", ""); + if (ret == WS_SUCCESS) { + channel->sessionType = WOLFSSH_SESSION_EXEC; + if (ssh->ctx->channelReqExecCb) { + rej = ssh->ctx->channelReqExecCb(channel, + ssh->channelReqCtx); + } + else { + rej = ssh->appChannels; + } } + sessionReq = 1; ssh->clientState = CLIENT_DONE; - - WLOG(WS_LOG_DEBUG, " command = %s", channel->command); } else if (ChannelRequestIs(type, typeSz, "subsystem")) { - ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL, + ScrubChannelCommand(channel); + ret = GetStringAlloc(ssh->ctx->heap, + &channel->command, &channel->commandSz, buf, len, &begin); - channel->sessionType = WOLFSSH_SESSION_SUBSYSTEM; - if (ssh->ctx->channelReqSubsysCb) { - rej = ssh->ctx->channelReqSubsysCb(channel, ssh->channelReqCtx); + if (ret == WS_SUCCESS) + WLOG(WS_LOG_DEBUG, " subsystem = %s", channel->command); + else + WLOG(WS_LOG_DEBUG, " subsystem = %s", ""); + if (ret == WS_SUCCESS) { + channel->sessionType = WOLFSSH_SESSION_SUBSYSTEM; + if (ssh->ctx->channelReqSubsysCb) { + rej = ssh->ctx->channelReqSubsysCb(channel, + ssh->channelReqCtx); + } + else { + rej = ssh->appChannels; + } } + sessionReq = 1; ssh->clientState = CLIENT_DONE; - - WLOG(WS_LOG_DEBUG, " subsystem = %s", channel->command); } #ifdef WOLFSSH_TERM else if (ChannelRequestIs(type, typeSz, "pty-req")) { @@ -13287,11 +13332,25 @@ static int DoChannelRequest(WOLFSSH* ssh, *idx = len; } + /* Record the answer, not the ask: sessionType and command are set before + * the reject decision and stay set on a refusal, so they cannot say + * whether the session was granted. Set even without a wantReply, which + * changes only whether the peer is told. + * + * Look the channel up again rather than reusing the pointer from + * before the callback. A callback may close its own channel, and + * wolfSSH_ChannelFree() frees it, so the old pointer can be dead. */ + if (sessionReq) { + channel = ChannelFind(ssh, channelId, WS_CHANNEL_ID_SELF); + if (channel != NULL) + channel->sessionGranted = (ret == WS_SUCCESS && !rej); + } + if (wantReply) { 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)); diff --git a/src/ssh.c b/src/ssh.c index 83f41d762..8502ff650 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -624,6 +624,8 @@ const char acceptState[] = "accept state: %s"; int wolfSSH_accept(WOLFSSH* ssh) { + byte stopState; + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_accept()"); if (ssh == NULL) @@ -643,6 +645,15 @@ int wolfSSH_accept(WOLFSSH* ssh) return WS_INVALID_STATE_E; } + /* In application-driven mode the state machine stops as soon as the + * user is authenticated; everything past that is the application's. + * Only stop there if the session has not already gone by: the loop + * below tests the stop state exactly, so a state it has stepped over + * would never terminate it. */ + stopState = (ssh->appChannels + && ssh->acceptState <= ACCEPT_SERVER_USERAUTH_SENT) ? + ACCEPT_SERVER_USERAUTH_SENT : ACCEPT_CLIENT_SESSION_ESTABLISHED; + /* check if data pending to be sent */ if (ssh->outputBuffer.length > 0 && ssh->acceptState < ACCEPT_CLIENT_SESSION_ESTABLISHED) { @@ -654,7 +665,11 @@ int wolfSSH_accept(WOLFSSH* ssh) ssh->acceptState != ACCEPT_SERVER_USERAUTH_ACCEPT_SENT && ssh->acceptState != ACCEPT_SERVER_KEXINIT_SENT && ssh->acceptState != ACCEPT_KEYED && - ssh->acceptState != ACCEPT_SERVER_CHANNEL_ACCEPT_SENT) { + ssh->acceptState != ACCEPT_SERVER_CHANNEL_ACCEPT_SENT && + /* Never step over where this call is meant to stop. The + * loop below tests for that state exactly, and the SCP and + * SFTP re-entry states sort after it. */ + ssh->acceptState != stopState) { WLOG(WS_LOG_DEBUG, "Advancing accept state"); ssh->acceptState++; } @@ -676,7 +691,7 @@ int wolfSSH_accept(WOLFSSH* ssh) } } - while (ssh->acceptState != ACCEPT_CLIENT_SESSION_ESTABLISHED) { + while (ssh->acceptState != stopState) { switch (ssh->acceptState) { case ACCEPT_BEGIN: @@ -766,6 +781,12 @@ int wolfSSH_accept(WOLFSSH* ssh) } ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT; WLOG(WS_LOG_DEBUG, acceptState, "SERVER_USERAUTH_SENT"); + if (stopState == ACCEPT_SERVER_USERAUTH_SENT) { + /* The application takes it from here. Tested through + * stopState so a callback that changed the flag during + * this call cannot half-apply it. */ + break; + } FALL_THROUGH; case ACCEPT_SERVER_USERAUTH_SENT: @@ -789,8 +810,13 @@ int wolfSSH_accept(WOLFSSH* ssh) } } + /* Divert only into a granted session. The type and + * command stay set on a refusal, so they do not say + * what was granted. */ #ifdef WOLFSSH_SCP - if (ChannelCommandIsScp(ssh)) { + if (ssh->channelList != NULL + && ssh->channelList->sessionGranted + && ChannelCommandIsScp(ssh)) { ssh->acceptState = ACCEPT_INIT_SCP_TRANSFER; WLOG(WS_LOG_DEBUG, acceptState, "ACCEPT_INIT_SCP_TRANSFER"); return WS_SCP_INIT; @@ -799,9 +825,13 @@ int wolfSSH_accept(WOLFSSH* ssh) #if defined(WOLFSSH_SFTP) && !defined(NO_WOLFSSH_SERVER) { const char* cmd = wolfSSH_GetSessionCommand(ssh); - if (cmd != NULL && - WOLFSSH_SESSION_SUBSYSTEM == wolfSSH_GetSessionType(ssh) - && (WSTRNCMP(cmd, "sftp", 4) == 0)) { + if (cmd != NULL + && ssh->channelList->sessionGranted + && WOLFSSH_SESSION_SUBSYSTEM + == wolfSSH_GetSessionType(ssh) + && wolfSSH_GetSessionCommandSz(ssh) + == (word32)WSTRLEN("sftp") + && (WSTRCMP(cmd, "sftp") == 0)) { ssh->acceptState = ACCEPT_INIT_SFTP; return wolfSSH_SFTP_accept(ssh); } @@ -4528,12 +4558,29 @@ WS_SessionType wolfSSH_GetSessionType(const WOLFSSH* ssh) const char* wolfSSH_GetSessionCommand(const WOLFSSH* ssh) { + const char* cmd = NULL; + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_GetSessionCommand()"); - if (ssh && ssh->channelList) - return ssh->channelList->command; + if (ssh) { + cmd = wolfSSH_ChannelGetSessionCommand(ssh->channelList); + } - return NULL; + return cmd; +} + + +word32 wolfSSH_GetSessionCommandSz(const WOLFSSH* ssh) +{ + word32 commandSz = 0; + + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_GetSessionCommandSz()"); + + if (ssh) { + commandSz = wolfSSH_ChannelGetSessionCommandSz(ssh->channelList); + } + + return commandSz; } @@ -4772,7 +4819,8 @@ WOLFSSH_CHANNEL* wolfSSH_ChannelFwdNewRemote(WOLFSSH* ssh, if (newChannel != NULL) ChannelAppend(ssh, newChannel); - WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_ChannelFwdNewRemote(), newChannel = %p, ret = %d", + WLOG(WS_LOG_DEBUG, + "Leaving wolfSSH_ChannelFwdNewRemote(), newChannel = %p, ret = %d", newChannel, ret); return newChannel; } @@ -5686,7 +5734,7 @@ const char* wolfSSH_ChannelGetSessionCommand(const WOLFSSH_CHANNEL* channel) { const char* cmd = NULL; - WLOG(WS_LOG_DEBUG, "Entering wolfSSH_ChannelGetCommand()"); + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_ChannelGetSessionCommand()"); if (channel) { cmd = channel->command; @@ -5696,6 +5744,20 @@ const char* wolfSSH_ChannelGetSessionCommand(const WOLFSSH_CHANNEL* channel) } +word32 wolfSSH_ChannelGetSessionCommandSz(const WOLFSSH_CHANNEL* channel) +{ + word32 commandSz = 0; + + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_ChannelGetSessionCommandSz()"); + + if (channel) { + commandSz = channel->commandSz; + } + + return commandSz; +} + + int wolfSSH_CTX_SetChannelOpenCb(WOLFSSH_CTX* ctx, WS_CallbackChannelOpen cb) { int ret = WS_SSH_CTX_NULL_E; @@ -5766,6 +5828,32 @@ int wolfSSH_CTX_SetChannelReqSubsysCb(WOLFSSH_CTX* ctx, } +int wolfSSH_CTX_SetAppChannels(WOLFSSH_CTX* ctx, byte enable) +{ + int ret = WS_SSH_CTX_NULL_E; + + if (ctx != NULL) { + ctx->appChannels = (enable != 0); + ret = WS_SUCCESS; + } + + return ret; +} + + +int wolfSSH_SetAppChannels(WOLFSSH* ssh, byte enable) +{ + int ret = WS_SSH_NULL_E; + + if (ssh != NULL) { + ssh->appChannels = (enable != 0); + ret = WS_SUCCESS; + } + + return ret; +} + + int wolfSSH_SetChannelOpenCtx(WOLFSSH* ssh, void* ctx) { int ret = WS_SSH_NULL_E; diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 88cca98f8..70a27ed95 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -1383,8 +1383,33 @@ int wolfSSH_SFTP_accept(WOLFSSH* ssh) if (ssh->error == WS_WANT_READ || ssh->error == WS_WANT_WRITE) ssh->error = WS_SUCCESS; + /* The grant is what says this session may be served, so it is asked + * for in every accept state. Below the user-auth stop the legacy + * branch would run the handshake itself, which in this mode returns + * with no channel open at all; at the stop or past it there is no + * accept() left that could have checked anything. */ + if (ssh->appChannels) { + /* Application-driven mode parks accept() here for good, so the + * sftp grant it would have checked is the application's subsystem + * callback: serve only a session channel it granted sftp on. The + * request having named sftp is not enough, so this asks for the + * grant as well -- unlike wolfSSH_accept()'s divert, which reads + * only the type and command. The name matches whole, length + * and bytes: sftpx, or sftp with an embedded NUL, is some + * other subsystem. */ + const WOLFSSH_CHANNEL* channel = ssh->channelList; + + if (channel == NULL || !channel->sessionGranted + || channel->sessionType != WOLFSSH_SESSION_SUBSYSTEM + || channel->command == NULL + || channel->commandSz != (word32)WSTRLEN("sftp") + || WSTRCMP(channel->command, "sftp") != 0) { + WLOG(WS_LOG_SFTP, "No sftp subsystem granted on the session"); + return WS_INVALID_STATE_E; + } + } /* check accept is done, if not call wolfSSH accept */ - if (ssh->acceptState < ACCEPT_CLIENT_SESSION_ESTABLISHED) { + else if (ssh->acceptState < ACCEPT_CLIENT_SESSION_ESTABLISHED) { byte name[] = "sftp"; WLOG(WS_LOG_SFTP, "Trying to do SSH accept first"); diff --git a/tests/regress.c b/tests/regress.c index c8c3016de..9bfa3efc5 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -1486,6 +1486,208 @@ static void AssertHandshakeRejectsMutatedReply(const char* keyAlgo, } #ifndef WOLFSSH_NO_RSA_SHA2_256 +/* Counts the shell requests the application-driven server answered. */ +static int appChannelsShellReqCount; + +static int AppChannelsShellCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + (void)channel; + (void)ctx; + appChannelsShellReqCount++; + return 0; +} + +/* Drive an application-driven server: wolfSSH_accept() is expected to return + * at userauth, so the channel open and the shell request are answered by + * wolfSSH_worker() calls the application makes itself. */ +static void RunAppChannelsHandshake(KexReplyHarness* harness, + KexReplyRunResult* result) +{ + word32 step; + + WMEMSET(result, 0, sizeof(*result)); + result->clientRet = WS_FATAL_ERROR; + result->serverRet = WS_FATAL_ERROR; + + for (step = 0; step < REGRESS_MAX_HANDSHAKE_STEPS; step++) { + if (!result->clientSuccess) { + result->clientRet = wolfSSH_connect(harness->client); + result->clientErr = wolfSSH_get_error(harness->client); + if (result->clientRet == WS_SUCCESS) { + result->clientSuccess = 1; + } + else if (!IsHandshakeRetryable(result->clientErr)) { + result->steps = step + 1; + return; + } + } + + if (!result->serverSuccess) { + result->serverRet = wolfSSH_accept(harness->server); + result->serverErr = wolfSSH_get_error(harness->server); + if (result->serverRet == WS_SUCCESS) { + result->serverSuccess = 1; + } + else if (!IsHandshakeRetryable(result->serverErr)) { + result->steps = step + 1; + return; + } + } + else if (harness->server->clientState < CLIENT_DONE) { + result->serverRet = wolfSSH_worker(harness->server, NULL); + result->serverErr = wolfSSH_get_error(harness->server); + if (result->serverRet < WS_SUCCESS + && result->serverErr != WS_CHAN_RXD + && !IsHandshakeRetryable(result->serverErr)) { + result->steps = step + 1; + return; + } + } + + if (result->clientSuccess && result->serverSuccess + && harness->server->clientState >= CLIENT_DONE) { + result->steps = step + 1; + return; + } + } + + result->steps = REGRESS_MAX_HANDSHAKE_STEPS; +} + +/* With wolfSSH_SetAppChannels() on, accept() stops once the user is + * authenticated and the shell request lands on the callback instead. */ +static void TestAppChannelsAcceptStopsAtUserAuth(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + appChannelsShellReqCount = 0; + + InitKexReplyHarness(&harness, "rsa-sha2-256", REGRESS_SERVER_KEY_PATH, + 0, NULL); + AssertIntEQ(wolfSSH_CTX_SetChannelReqShellCb(harness.serverCtx, + AppChannelsShellCb), WS_SUCCESS); + AssertIntEQ(wolfSSH_SetAppChannels(harness.server, 1), WS_SUCCESS); + + RunAppChannelsHandshake(&harness, &result); + + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertIntEQ(harness.server->acceptState, ACCEPT_SERVER_USERAUTH_SENT); + AssertIntEQ(harness.server->clientState, CLIENT_DONE); + AssertIntEQ(appChannelsShellReqCount, 1); + AssertIntEQ(harness.client->connectState, + CONNECT_SERVER_CHANNEL_REQUEST_DONE); + AssertFalse(harness.clientIo.sawDisconnect); + AssertFalse(harness.serverIo.sawDisconnect); + + FreeKexReplyHarness(&harness); +} + +/* Same mode, no callback registered: nothing can start the shell once + * accept() has returned, so the request is refused. The default mode + * accepts it, which AssertHandshakeSucceeds() covers. */ +static void TestAppChannelsNoShellCbRejects(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + InitKexReplyHarness(&harness, "rsa-sha2-256", REGRESS_SERVER_KEY_PATH, + 0, NULL); + AssertIntEQ(wolfSSH_SetAppChannels(harness.server, 1), WS_SUCCESS); + + RunAppChannelsHandshake(&harness, &result); + + /* RunAppChannelsHandshake() also leaves clientSuccess clear when it + * runs out of steps with neither side erroring, so pin the refusal + * itself: the client stopped early, and for the right reason. */ + AssertFalse(result.clientSuccess); + AssertTrue(result.steps < REGRESS_MAX_HANDSHAKE_STEPS); + AssertIntEQ(result.clientErr, WS_CHANOPEN_FAILED); + AssertTrue(harness.client->connectState < + CONNECT_SERVER_CHANNEL_REQUEST_DONE); + AssertIntEQ(harness.server->acceptState, ACCEPT_SERVER_USERAUTH_SENT); + + FreeKexReplyHarness(&harness); +} + +/* The flag is documented as a context setting first, so pin the setter + * returns and the inheritance wolfSSH_new() does. */ +static void TestAppChannelsCtxInherits(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + + AssertIntEQ(wolfSSH_CTX_SetAppChannels(NULL, 1), WS_SSH_CTX_NULL_E); + AssertIntEQ(wolfSSH_SetAppChannels(NULL, 1), WS_SSH_NULL_E); + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(ssh->appChannels, 0); + wolfSSH_free(ssh); + + AssertIntEQ(wolfSSH_CTX_SetAppChannels(ctx, 1), WS_SUCCESS); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(ssh->appChannels, 1); + AssertIntEQ(wolfSSH_SetAppChannels(ssh, 0), WS_SUCCESS); + AssertIntEQ(ssh->appChannels, 0); + wolfSSH_free(ssh); + + wolfSSH_CTX_free(ctx); +} + +/* Turning the mode on after accept() established the session must not leave + * the accept loop hunting for a state it has already stepped past. The flag + * still reaches DoChannelRequest() from there, which is what ssh.h promises, + * so pin both halves: accept() stays put, the requests that follow flip. */ +static void TestAppChannelsLateEnableReturns(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + /* SSH_MSG_CHANNEL_REQUEST body: channel 0, "shell", wantReply. */ + static byte payShell[] = { + 0x00,0x00,0x00,0x00, /* channelId = 0 */ + 0x00,0x00,0x00,0x05, /* typeSz = 5 */ + 0x73,0x68,0x65,0x6C,0x6C, /* "shell" */ + 0x01 /* wantReply = 1 */ + }; + word32 idx; + + InitKexReplyHarness(&harness, "rsa-sha2-256", REGRESS_SERVER_KEY_PATH, + 0, NULL); + + RunKexReplyHandshake(&harness, &result); + + AssertTrue(result.serverSuccess); + AssertIntEQ(harness.server->acceptState, + ACCEPT_CLIENT_SESSION_ESTABLISHED); + + /* Default mode, no callback registered: the request is granted. */ + idx = 0; + AssertIntEQ(wolfSSH_TestDoChannelRequest(harness.server, payShell, + (word32)sizeof(payShell), &idx), WS_SUCCESS); + AssertIntEQ(wolfSSH_worker(harness.client, NULL), WS_SUCCESS); + + AssertIntEQ(wolfSSH_SetAppChannels(harness.server, 1), WS_SUCCESS); + AssertIntEQ(wolfSSH_accept(harness.server), WS_SUCCESS); + AssertIntEQ(harness.server->acceptState, + ACCEPT_CLIENT_SESSION_ESTABLISHED); + + /* Same request, same session, mode now on: refused instead. */ + idx = 0; + AssertIntEQ(wolfSSH_TestDoChannelRequest(harness.server, payShell, + (word32)sizeof(payShell), &idx), WS_SUCCESS); + AssertTrue(wolfSSH_worker(harness.client, NULL) < WS_SUCCESS); + AssertIntEQ(wolfSSH_get_error(harness.client), WS_CHANOPEN_FAILED); + + FreeKexReplyHarness(&harness); +} + static void TestKexDhReplyRejectsRsaSha2_256SigNameDowngrade(void) { AssertHandshakeSucceeds("rsa-sha2-256", REGRESS_SERVER_KEY_PATH); @@ -3272,264 +3474,1029 @@ static void TestChannelOpenConfCallbackRejects(void) in, sizeof(in)); RepointHarnessInput(&harness, in, inSz); - ret = DoReceive(harness.ssh); - AssertIntEQ(ret, WS_FATAL_ERROR); - AssertIntEQ(wolfSSH_get_error(harness.ssh), WS_BAD_ARGUMENT); - AssertIntEQ(harness.io.inOff, harness.io.inSz); - AssertIntEQ(openConfCbCalls, 1); - AssertIntEQ(harness.ssh->serverState, serverState); - AssertIntEQ(harness.ssh->defaultPeerChannelId, defaultPeerChannelId); + ret = DoReceive(harness.ssh); + AssertIntEQ(ret, WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(harness.ssh), WS_BAD_ARGUMENT); + AssertIntEQ(harness.io.inOff, harness.io.inSz); + AssertIntEQ(openConfCbCalls, 1); + AssertIntEQ(harness.ssh->serverState, serverState); + AssertIntEQ(harness.ssh->defaultPeerChannelId, defaultPeerChannelId); + + FreeChannelOpenHarness(&harness); +} + +/* A failure callback that returns an error is reported in place of + * WS_CHANOPEN_FAILED, and the refused channel is left on the list for the + * application to retire. */ +static void TestChannelOpenFailCallbackRejects(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[128]; + word32 inSz; + int cbCtx = 0; + int ret; + + channel = SeedOpenRespHarness(&harness, &cbCtx, WS_BAD_ARGUMENT); + + inSz = BuildChannelOpenFailPacket(channel->channel, + OPEN_ADMINISTRATIVELY_PROHIBITED, "no", in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + ret = DoReceive(harness.ssh); + AssertIntEQ(ret, WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(harness.ssh), WS_BAD_ARGUMENT); + AssertIntEQ(harness.io.inOff, harness.io.inSz); + AssertIntEQ(openFailCbCalls, 1); + AssertIntEQ(harness.ssh->channelListSz, 1); + + FreeChannelOpenHarness(&harness); +} + +/* What the close callback saw, including whether the channel was still on + * the list when it ran. */ +static int closeCbCalls; +static word32 closeCbChannel; +static void* closeCbCtx; +static word32 closeCbListSz; +static int closeCbReturn; + +static int RecordingChannelCloseCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + AssertNotNull(channel); + AssertNotNull(channel->ssh); + closeCbCalls++; + closeCbChannel = channel->channel; + closeCbCtx = ctx; + closeCbListSz = channel->ssh->channelListSz; + + return closeCbReturn; +} + +/* Drives a peer close of a confirmed channel through the recording callback, + * set to return cbReturn, and returns what DoReceive() reported. */ +static int CloseThroughRecordingCb(ChannelOpenHarness* harness, void* cbCtx, + int cbReturn, word32* selfChannelId) +{ + WOLFSSH_CHANNEL* channel; + byte in[64]; + word32 inSz; + int ret; + + closeCbCalls = 0; + closeCbChannel = REGRESS_NO_CHANNEL; + closeCbCtx = NULL; + closeCbListSz = 0; + closeCbReturn = cbReturn; + + InitChannelOpenHarness(harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelCloseCb(harness->ctx, + RecordingChannelCloseCb), WS_SUCCESS); + AssertIntEQ(wolfSSH_SetChannelCloseCtx(harness->ssh, cbCtx), WS_SUCCESS); + + channel = SeedUnconfirmedChannel(harness); + *selfChannelId = channel->channel; + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; + + inSz = BuildChannelClosePacket(*selfChannelId, in, sizeof(in)); + RepointHarnessInput(harness, in, inSz); + + ret = DoReceive(harness->ssh); + AssertIntEQ(harness->io.inOff, harness->io.inSz); + + return ret; +} + +/* The close callback is an application's only notice that a peer closed a + * channel, and it has to run while the channel is still findable: + * DoChannelClose() retires it a few lines later, and after that there is + * nothing left to name. */ +static void TestChannelCloseCallbackRuns(void) +{ + ChannelOpenHarness harness; + word32 selfChannelId; + int cbCtx = 0; + int ret; + + ret = CloseThroughRecordingCb(&harness, &cbCtx, WS_SUCCESS, + &selfChannelId); + AssertIntEQ(ret, WS_CHANNEL_CLOSED); + AssertIntEQ(closeCbCalls, 1); + AssertIntEQ(closeCbChannel, selfChannelId); + AssertTrue(closeCbCtx == &cbCtx); + AssertIntEQ(closeCbListSz, 1); + + /* And the channel is gone by the time the caller is told. */ + AssertIntEQ(harness.ssh->channelListSz, 0); + + FreeChannelOpenHarness(&harness); +} + +/* The close callback's return is discarded: the peer has closed whatever + * the application thinks, so the close completes either way. */ +static void TestChannelCloseCallbackReturnIgnored(void) +{ + ChannelOpenHarness harness; + word32 selfChannelId; + int cbCtx = 0; + int ret; + + ret = CloseThroughRecordingCb(&harness, &cbCtx, WS_BAD_ARGUMENT, + &selfChannelId); + AssertIntEQ(ret, WS_CHANNEL_CLOSED); + AssertIntEQ(closeCbCalls, 1); + AssertIntEQ(harness.ssh->channelListSz, 0); + + FreeChannelOpenHarness(&harness); +} + +/* Builds a plaintext SSH_MSG_CHANNEL_REQUEST whose type-specific tail is a + * single string, which is the shape of both "exec" and "subsystem". A NULL + * "arg" leaves the tail off, which is the shape of "shell". */ +static word32 BuildChannelStringRequestPacket(word32 recipientChannelId, + const char* type, byte wantReply, const char* arg, + byte* out, word32 outSz) +{ + byte payload[128]; + word32 idx = 0; + + idx = AppendUint32(payload, sizeof(payload), idx, recipientChannelId); + idx = AppendString(payload, sizeof(payload), idx, type); + idx = AppendByte(payload, sizeof(payload), idx, wantReply); + if (arg != NULL) { + idx = AppendString(payload, sizeof(payload), idx, arg); + } + + return WrapPacket(MSGID_CHANNEL_REQUEST, payload, idx, out, outSz); +} + +/* What a session request callback saw. */ +static int sessionReqCbCalls; +static WS_SessionType sessionReqCbType; +static char sessionReqCbCommand[32]; +static void* sessionReqCbCtx; +static int sessionReqCbReturn; + +static int RecordingSessionReqCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + const char* command; + + sessionReqCbCalls++; + sessionReqCbCtx = ctx; + sessionReqCbCommand[0] = 0; + + if (channel != NULL) { + sessionReqCbType = wolfSSH_ChannelGetSessionType(channel); + command = wolfSSH_ChannelGetSessionCommand(channel); + if (command != NULL) { + WSTRNCPY(sessionReqCbCommand, command, + sizeof(sessionReqCbCommand) - 1); + sessionReqCbCommand[sizeof(sessionReqCbCommand) - 1] = 0; + } + } + + return sessionReqCbReturn; +} + +/* Drives one session request through a fresh harness, with the callback + * returning cbReturn, and returns the message id the server answered with. */ +static byte RunSessionRequest(const char* type, const char* arg, int cbReturn, + WS_SessionType expectType) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[128]; + word32 inSz; + int cbCtx = 0; + byte replyId; + + sessionReqCbCalls = 0; + sessionReqCbType = WOLFSSH_SESSION_UNKNOWN; + sessionReqCbCommand[0] = 0; + sessionReqCbCtx = NULL; + sessionReqCbReturn = cbReturn; + + InitChannelOpenHarness(&harness, NULL, 0); + if (WSTRCMP(type, "exec") == 0) { + AssertIntEQ(wolfSSH_CTX_SetChannelReqExecCb(harness.ctx, + RecordingSessionReqCb), WS_SUCCESS); + } + else { + AssertIntEQ(wolfSSH_CTX_SetChannelReqSubsysCb(harness.ctx, + RecordingSessionReqCb), WS_SUCCESS); + } + AssertIntEQ(wolfSSH_SetChannelReqCtx(harness.ssh, &cbCtx), WS_SUCCESS); + + channel = SeedUnconfirmedChannel(&harness); + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; + + inSz = BuildChannelStringRequestPacket(channel->channel, type, 1, arg, + in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + AssertIntEQ(DoReceive(harness.ssh), WS_SUCCESS); + AssertIntEQ(harness.io.inOff, harness.io.inSz); + AssertIntEQ(sessionReqCbCalls, 1); + AssertTrue(sessionReqCbCtx == &cbCtx); + AssertIntEQ(sessionReqCbType, expectType); + AssertIntEQ(WSTRCMP(sessionReqCbCommand, arg), 0); + + replyId = ParseMsgId(harness.io.out, harness.io.outSz); + FreeChannelOpenHarness(&harness); + + return replyId; +} + +/* The exec callback is the only place an application can vet a remote + * command, and its return is what decides the reply on the wire. + * DoChannelRequest() tests only for nonzero, so a bare 1 rejects the same + * as a WS_ error. */ +static void TestChannelReqExecCallbackRuns(void) +{ + AssertIntEQ(RunSessionRequest("exec", "ls", WS_SUCCESS, + WOLFSSH_SESSION_EXEC), MSGID_CHANNEL_SUCCESS); + AssertIntEQ(RunSessionRequest("exec", "ls", WS_BAD_ARGUMENT, + WOLFSSH_SESSION_EXEC), MSGID_CHANNEL_FAILURE); + AssertIntEQ(RunSessionRequest("exec", "ls", 1, + WOLFSSH_SESSION_EXEC), MSGID_CHANNEL_FAILURE); +} + +/* Same contract for the subsystem callback, which is how a server decides + * whether to serve SFTP on a channel. */ +static void TestChannelReqSubsysCallbackRuns(void) +{ + AssertIntEQ(RunSessionRequest("subsystem", "sftp", WS_SUCCESS, + WOLFSSH_SESSION_SUBSYSTEM), MSGID_CHANNEL_SUCCESS); + AssertIntEQ(RunSessionRequest("subsystem", "sftp", WS_BAD_ARGUMENT, + WOLFSSH_SESSION_SUBSYSTEM), MSGID_CHANNEL_FAILURE); + AssertIntEQ(RunSessionRequest("subsystem", "sftp", 1, + WOLFSSH_SESSION_SUBSYSTEM), MSGID_CHANNEL_FAILURE); +} + +/* What a length-aware session request callback saw. */ +static word32 sessionReqCbCommandSz; +static word32 sessionReqCbCommandStrLen; + +static int LengthRecordingSessionReqCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + const char* command; + + (void)ctx; + + sessionReqCbCalls++; + sessionReqCbCommandSz = wolfSSH_ChannelGetSessionCommandSz(channel); + command = wolfSSH_ChannelGetSessionCommand(channel); + sessionReqCbCommandStrLen = (command == NULL) ? + 0 : (word32)WSTRLEN(command); + + return 0; +} + +/* Drives one session request carrying a command that the C string alone + * cannot describe, and checks what the callback could see of it. */ +static void CheckSessionReqCbSeesCommandSz(const char* type, + const byte* command, word32 commandSz, word32 expectStrLen) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte payload[128]; + byte in[128]; + word32 idx = 0; + word32 inSz; + + sessionReqCbCalls = 0; + sessionReqCbCommandSz = 0; + sessionReqCbCommandStrLen = 0; + + InitChannelOpenHarness(&harness, NULL, 0); + if (WSTRCMP(type, "exec") == 0) { + AssertIntEQ(wolfSSH_CTX_SetChannelReqExecCb(harness.ctx, + LengthRecordingSessionReqCb), WS_SUCCESS); + } + else { + AssertIntEQ(wolfSSH_CTX_SetChannelReqSubsysCb(harness.ctx, + LengthRecordingSessionReqCb), WS_SUCCESS); + } + + channel = SeedUnconfirmedChannel(&harness); + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; + + /* Built here rather than with BuildChannelStringRequestPacket(): that + * takes the command as a C string, which cannot carry the NUL. */ + idx = AppendUint32(payload, sizeof(payload), idx, channel->channel); + idx = AppendString(payload, sizeof(payload), idx, type); + idx = AppendByte(payload, sizeof(payload), idx, 1); + idx = AppendUint32(payload, sizeof(payload), idx, commandSz); + idx = AppendData(payload, sizeof(payload), idx, command, commandSz); + inSz = WrapPacket(MSGID_CHANNEL_REQUEST, payload, idx, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + AssertIntEQ(DoReceive(harness.ssh), WS_SUCCESS); + AssertIntEQ(sessionReqCbCalls, 1); + AssertIntEQ(sessionReqCbCommandSz, commandSz); + AssertIntEQ(sessionReqCbCommandStrLen, expectStrLen); + + /* The session-wide accessor reports the same channel's command. */ + AssertIntEQ(wolfSSH_GetSessionCommandSz(harness.ssh), commandSz); + + FreeChannelOpenHarness(&harness); +} + +/* An application vetting a command in its callback needs the wire length. + * The string it is handed stops at an embedded NUL, so "sftp\0evil" reads + * there as "sftp" and passes a name check the whole name has to fail; the + * length is what tells the two apart. */ +static void TestSessionReqCallbackSeesCommandSz(void) +{ + static const byte nulCommand[] = { + 's', 'f', 't', 'p', 0, 'e', 'v', 'i', 'l' + }; + static const byte plainCommand[] = { 'l', 's' }; + + /* The control: with no NUL in it, length and C string agree, so the + * cases below are the NUL and not the accessor reporting anything it + * likes. */ + CheckSessionReqCbSeesCommandSz("exec", plainCommand, + (word32)sizeof(plainCommand), (word32)sizeof(plainCommand)); + CheckSessionReqCbSeesCommandSz("exec", nulCommand, + (word32)sizeof(nulCommand), 4); + CheckSessionReqCbSeesCommandSz("subsystem", nulCommand, + (word32)sizeof(nulCommand), 4); + + /* Nothing to report is zero, not a read through a NULL. */ + AssertIntEQ(wolfSSH_ChannelGetSessionCommandSz(NULL), 0); + AssertIntEQ(wolfSSH_GetSessionCommandSz(NULL), 0); +} + +/* Drives one session request whose command string runs past the end of the + * packet, and returns the message id the server answered with. */ +static byte RunMalformedSessionRequest(const char* type) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte payload[128]; + byte in[128]; + word32 idx = 0; + word32 inSz; + byte replyId; + + sessionReqCbCalls = 0; + sessionReqCbReturn = 0; + + InitChannelOpenHarness(&harness, NULL, 0); + if (WSTRCMP(type, "exec") == 0) { + AssertIntEQ(wolfSSH_CTX_SetChannelReqExecCb(harness.ctx, + RecordingSessionReqCb), WS_SUCCESS); + } + else { + AssertIntEQ(wolfSSH_CTX_SetChannelReqSubsysCb(harness.ctx, + RecordingSessionReqCb), WS_SUCCESS); + } + + channel = SeedUnconfirmedChannel(&harness); + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; + + /* The command's length header claims more than the packet holds. */ + idx = AppendUint32(payload, sizeof(payload), idx, channel->channel); + idx = AppendString(payload, sizeof(payload), idx, type); + idx = AppendByte(payload, sizeof(payload), idx, 1); + idx = AppendUint32(payload, sizeof(payload), idx, 64); + idx = AppendData(payload, sizeof(payload), idx, (const byte*)"ls", 2); + inSz = WrapPacket(MSGID_CHANNEL_REQUEST, payload, idx, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + /* A malformed packet ends the connection, but the refusal goes out + * first. */ + AssertIntEQ(DoReceive(harness.ssh), WS_FATAL_ERROR); + AssertIntEQ(harness.ssh->error, WS_BUFFER_E); + AssertIntEQ(sessionReqCbCalls, 0); + AssertIntEQ(channel->sessionGranted, 0); + + replyId = ParseMsgId(harness.io.out, harness.io.outSz); + FreeChannelOpenHarness(&harness); + + return replyId; +} + +/* A command that failed to parse is refused without asking the callback: + * there is nothing to vet, and channel->command still holds whatever an + * earlier request on the channel left behind. */ +static void TestMalformedSessionRequestSkipsCallback(void) +{ + AssertIntEQ(RunMalformedSessionRequest("exec"), MSGID_CHANNEL_FAILURE); + AssertIntEQ(RunMalformedSessionRequest("subsystem"), + MSGID_CHANNEL_FAILURE); +} + +/* A request callback owns its channel and may close it. The grant is + * recorded after the callback returns, so it has to find the channel + * again: wolfSSH_ChannelFree() frees it, and writing through the old + * pointer would touch freed memory. */ +static int freeChannelCbCalls; + +static int FreeingSessionReqCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + (void)ctx; + freeChannelCbCalls++; + AssertIntEQ(wolfSSH_ChannelFree(channel), WS_SUCCESS); + return 0; +} + +static void TestSessionReqCallbackMayFreeChannel(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[128]; + word32 inSz; + + freeChannelCbCalls = 0; + + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqShellCb(harness.ctx, + FreeingSessionReqCb), WS_SUCCESS); + + channel = SeedUnconfirmedChannel(&harness); + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; + + inSz = BuildChannelStringRequestPacket(channel->channel, "shell", 1, + NULL, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + AssertIntEQ(DoReceive(harness.ssh), WS_FATAL_ERROR); + + AssertIntEQ(freeChannelCbCalls, 1); + /* The channel the grant would have been recorded on is gone, so the + * reply cannot be sent either and the session says why. */ + AssertIntEQ(harness.ssh->channelListSz, 0); + AssertNull(harness.ssh->channelList); + AssertIntEQ(wolfSSH_get_error(harness.ssh), WS_INVALID_CHANID); + AssertIntEQ(harness.io.outSz, 0); + + FreeChannelOpenHarness(&harness); +} + +/* accept() re-entered while it is already parked, with a reply still + * queued, has to flush and stay put. Stepping the state on from here + * would put the stop behind it, and the loop tests for that state + * exactly, so the session would run on to established instead. */ +static void TestAppChannelsAcceptKeepsStopWithPendingOutput(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_SetAppChannels(harness.ssh, 1), WS_SUCCESS); + + channel = SeedUnconfirmedChannel(&harness); + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; + + /* A blocked send leaves the channel data queued. Nothing here drives + * a channel request, so clientState stays short of CLIENT_DONE and a + * state stepped past the stop fails the accept below rather than + * spinning in it. */ + harness.io.blockNext = 1; + AssertIntEQ(wolfSSH_stream_send(harness.ssh, (byte*)"x", 1), 1); + AssertTrue(harness.ssh->outputBuffer.length > 0); + AssertTrue(harness.ssh->clientState < CLIENT_DONE); + + AssertIntEQ(harness.ssh->acceptState, ACCEPT_SERVER_USERAUTH_SENT); + AssertIntEQ(wolfSSH_accept(harness.ssh), WS_SUCCESS); + AssertIntEQ(harness.ssh->acceptState, ACCEPT_SERVER_USERAUTH_SENT); + AssertIntEQ(harness.ssh->outputBuffer.length, 0); + + FreeChannelOpenHarness(&harness); +} + +#ifdef WOLFSSH_SFTP +/* SSH_MSG_CHANNEL_DATA carrying an SFTP INIT, version 3. */ +static word32 BuildSftpInitDataPacket(word32 recipientChannelId, byte* out, + word32 outSz) +{ + static const byte init[] = { + 0x00,0x00,0x00,0x05, /* length */ + WOLFSSH_FTP_INIT, + 0x00,0x00,0x00,0x03 /* version = 3 */ + }; + byte payload[32]; + word32 idx = 0; + + idx = AppendUint32(payload, sizeof(payload), idx, recipientChannelId); + idx = AppendUint32(payload, sizeof(payload), idx, (word32)sizeof(init)); + idx = AppendData(payload, sizeof(payload), idx, init, sizeof(init)); + + return WrapPacket(MSGID_CHANNEL_DATA, payload, idx, out, outSz); +} + +/* An application-driven server with a confirmed session channel, its request + * callback for type registered to grant, and one request of that type driven + * through it. Returns the channel; the harness input is left empty. */ +static WOLFSSH_CHANNEL* SeedAppChannelsSession(ChannelOpenHarness* harness, + const char* type, const char* arg) +{ + WOLFSSH_CHANNEL* channel; + byte in[128]; + word32 inSz; + + sessionReqCbCalls = 0; + sessionReqCbReturn = 0; + + InitChannelOpenHarness(harness, NULL, 0); + AssertIntEQ(wolfSSH_SetAppChannels(harness->ssh, 1), WS_SUCCESS); + if (WSTRCMP(type, "shell") == 0) { + AssertIntEQ(wolfSSH_CTX_SetChannelReqShellCb(harness->ctx, + RecordingSessionReqCb), WS_SUCCESS); + } + else { + AssertIntEQ(wolfSSH_CTX_SetChannelReqSubsysCb(harness->ctx, + RecordingSessionReqCb), WS_SUCCESS); + } + + channel = SeedUnconfirmedChannel(harness); + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; + + inSz = BuildChannelStringRequestPacket(channel->channel, type, 1, arg, + in, sizeof(in)); + RepointHarnessInput(harness, in, inSz); + AssertIntEQ(DoReceive(harness->ssh), WS_SUCCESS); + AssertIntEQ(sessionReqCbCalls, 1); + AssertIntEQ(ParseMsgId(harness->io.out, harness->io.outSz), + MSGID_CHANNEL_SUCCESS); + RepointHarnessInput(harness, NULL, 0); + + return channel; +} + +/* wolfSSH_SFTP_accept() in application-driven mode. accept() parks short of + * the session, so the sftp grant it would have checked is the application's + * subsystem callback: with no session channel there is nothing to serve. */ +static void TestSftpAcceptAppChannelsNeedsSession(void) +{ + ChannelOpenHarness harness; + + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_SetAppChannels(harness.ssh, 1), WS_SUCCESS); + + AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_INVALID_STATE_E); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + AssertIntEQ(harness.ssh->acceptState, ACCEPT_SERVER_USERAUTH_SENT); + + FreeChannelOpenHarness(&harness); +} + +/* Called ahead of accept(), which is how a server that set the flag on the + * context reaches this entry point. The refusal has to come from the gate: + * running the handshake instead returns with no channel open in this mode, + * and the SFTP exchange then fails on the missing channel. */ +static void TestSftpAcceptAppChannelsRefusesPreAccept(void) +{ + ChannelOpenHarness harness; + + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_SetAppChannels(harness.ssh, 1), WS_SUCCESS); + harness.ssh->acceptState = ACCEPT_BEGIN; + + AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_INVALID_STATE_E); + /* Nothing sent, so no handshake was started ... */ + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.ssh->acceptState, ACCEPT_BEGIN); + /* ... and the subsystem name the legacy branch sets was not set. */ + AssertNull(harness.ssh->channelName); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} + +/* A granted shell is not an sftp grant: the INIT the peer pushes on that + * channel stays unread. */ +static void TestSftpAcceptAppChannelsRefusesShell(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[64]; + word32 inSz; + + channel = SeedAppChannelsSession(&harness, "shell", NULL); + inSz = BuildSftpInitDataPacket(channel->channel, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_INVALID_STATE_E); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.io.inOff, 0); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} + +/* A name that only starts with sftp is not an sftp grant. */ +static void TestSftpAcceptAppChannelsRefusesPrefixName(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[64]; + word32 inSz; + + channel = SeedAppChannelsSession(&harness, "subsystem", "sftpx"); + inSz = BuildSftpInitDataPacket(channel->channel, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_INVALID_STATE_E); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.io.inOff, 0); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} + +/* A granted name whose wire length runs past an embedded NUL is not an + * sftp grant: the four bytes ahead of the NUL match, the name does not. */ +static void TestSftpAcceptAppChannelsRefusesNulName(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + static const byte nulName[] = { + 's', 'f', 't', 'p', 0, 'e', 'v', 'i', 'l' + }; + byte payload[128]; + byte in[128]; + word32 idx = 0; + word32 inSz; + + sessionReqCbCalls = 0; + sessionReqCbReturn = 0; + + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_SetAppChannels(harness.ssh, 1), WS_SUCCESS); + AssertIntEQ(wolfSSH_CTX_SetChannelReqSubsysCb(harness.ctx, + RecordingSessionReqCb), WS_SUCCESS); + + channel = SeedUnconfirmedChannel(&harness); + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; + + /* Built here rather than with BuildChannelStringRequestPacket(): that + * takes the name as a C string, which cannot carry the NUL. */ + idx = AppendUint32(payload, sizeof(payload), idx, channel->channel); + idx = AppendString(payload, sizeof(payload), idx, "subsystem"); + idx = AppendByte(payload, sizeof(payload), idx, 1); + idx = AppendUint32(payload, sizeof(payload), idx, (word32)sizeof(nulName)); + idx = AppendData(payload, sizeof(payload), idx, nulName, sizeof(nulName)); + inSz = WrapPacket(MSGID_CHANNEL_REQUEST, payload, idx, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + /* The callback reads a C string, so it sees sftp and grants it. */ + AssertIntEQ(DoReceive(harness.ssh), WS_SUCCESS); + AssertIntEQ(sessionReqCbCalls, 1); + AssertIntEQ(WSTRCMP(sessionReqCbCommand, "sftp"), 0); + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + MSGID_CHANNEL_SUCCESS); + + inSz = BuildSftpInitDataPacket(channel->channel, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_INVALID_STATE_E); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.io.inOff, 0); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} + +/* Four bytes that are not sftp are not an sftp grant. */ +static void TestSftpAcceptAppChannelsRefusesSameLengthName(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[64]; + word32 inSz; + + channel = SeedAppChannelsSession(&harness, "subsystem", "sfxp"); + inSz = BuildSftpInitDataPacket(channel->channel, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_INVALID_STATE_E); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.io.inOff, 0); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} + +/* The grant the mode relies on: the subsystem callback took sftp, so the + * INIT is answered with a VERSION and accept() stays parked. */ +static void TestSftpAcceptAppChannelsServesGrantedSftp(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[64]; + word32 inSz; + /* Offset of the SFTP type byte in the packet the server sends: the + * SSH packet header, then the CHANNEL_DATA payload of recipient + * channel and data-string length, then the SFTP length field. */ + const word32 sftpIdx = LENGTH_SZ + PAD_LENGTH_SZ + MSG_ID_SZ + + UINT32_SZ + UINT32_SZ + UINT32_SZ; + + channel = SeedAppChannelsSession(&harness, "subsystem", "sftp"); + inSz = BuildSftpInitDataPacket(channel->channel, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_SFTP_COMPLETE); + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + MSGID_CHANNEL_DATA); + AssertTrue(harness.io.outSz > sftpIdx); + AssertIntEQ(harness.io.out[sftpIdx], WOLFSSH_FTP_VERSION); + AssertIntEQ(harness.ssh->acceptState, ACCEPT_SERVER_USERAUTH_SENT); FreeChannelOpenHarness(&harness); } -/* A failure callback that returns an error is reported in place of - * WS_CHANOPEN_FAILED, and the refused channel is left on the list for the - * application to retire. */ -static void TestChannelOpenFailCallbackRejects(void) + +/* An established session is gated too. A server that turns the mode on + * late is past everything accept() would have checked, so the grant is + * the only thing left saying what the channel is: a granted shell is not + * an sftp grant, whatever state accept() finished in. */ +static void TestSftpAcceptAppChannelsRefusesEstablishedShell(void) { ChannelOpenHarness harness; WOLFSSH_CHANNEL* channel; - byte in[128]; + byte in[64]; word32 inSz; - int cbCtx = 0; - int ret; - - channel = SeedOpenRespHarness(&harness, &cbCtx, WS_BAD_ARGUMENT); - inSz = BuildChannelOpenFailPacket(channel->channel, - OPEN_ADMINISTRATIVELY_PROHIBITED, "no", in, sizeof(in)); + channel = SeedAppChannelsSession(&harness, "shell", NULL); + harness.ssh->acceptState = ACCEPT_CLIENT_SESSION_ESTABLISHED; + inSz = BuildSftpInitDataPacket(channel->channel, in, sizeof(in)); RepointHarnessInput(&harness, in, inSz); - ret = DoReceive(harness.ssh); - AssertIntEQ(ret, WS_FATAL_ERROR); - AssertIntEQ(wolfSSH_get_error(harness.ssh), WS_BAD_ARGUMENT); - AssertIntEQ(harness.io.inOff, harness.io.inSz); - AssertIntEQ(openFailCbCalls, 1); - AssertIntEQ(harness.ssh->channelListSz, 1); + AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_INVALID_STATE_E); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.io.inOff, 0); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); FreeChannelOpenHarness(&harness); } -/* What the close callback saw, including whether the channel was still on - * the list when it ran. */ -static int closeCbCalls; -static word32 closeCbChannel; -static void* closeCbCtx; -static word32 closeCbListSz; -static int closeCbReturn; - -static int RecordingChannelCloseCb(WOLFSSH_CHANNEL* channel, void* ctx) +/* The other half of that: asking in every state must not refuse a session + * the callback did grant sftp on, wherever accept() left off. */ +static void TestSftpAcceptAppChannelsServesEstablishedSftp(void) { - AssertNotNull(channel); - AssertNotNull(channel->ssh); - closeCbCalls++; - closeCbChannel = channel->channel; - closeCbCtx = ctx; - closeCbListSz = channel->ssh->channelListSz; + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[64]; + word32 inSz; - return closeCbReturn; + channel = SeedAppChannelsSession(&harness, "subsystem", "sftp"); + harness.ssh->acceptState = ACCEPT_CLIENT_SESSION_ESTABLISHED; + inSz = BuildSftpInitDataPacket(channel->channel, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_SFTP_COMPLETE); + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + MSGID_CHANNEL_DATA); + + FreeChannelOpenHarness(&harness); } -/* Drives a peer close of a confirmed channel through the recording callback, - * set to return cbReturn, and returns what DoReceive() reported. */ -static int CloseThroughRecordingCb(ChannelOpenHarness* harness, void* cbCtx, - int cbReturn, word32* selfChannelId) +/* A refused "subsystem sftp" still leaves sessionType/command set on the + * channel, so check wolfSSH_SFTP_accept() looks at the grant, not the + * leftovers. rejectVia 0 registers no callback at all (app channels alone + * refuse); 1 registers one that rejects. */ +static void CheckSftpAcceptRefusesUngranted(int rejectVia) { + ChannelOpenHarness harness; WOLFSSH_CHANNEL* channel; - byte in[64]; + byte in[128]; word32 inSz; - int ret; - closeCbCalls = 0; - closeCbChannel = REGRESS_NO_CHANNEL; - closeCbCtx = NULL; - closeCbListSz = 0; - closeCbReturn = cbReturn; + sessionReqCbCalls = 0; + sessionReqCbReturn = (rejectVia == 0) ? 0 : 1; - InitChannelOpenHarness(harness, NULL, 0); - AssertIntEQ(wolfSSH_CTX_SetChannelCloseCb(harness->ctx, - RecordingChannelCloseCb), WS_SUCCESS); - AssertIntEQ(wolfSSH_SetChannelCloseCtx(harness->ssh, cbCtx), WS_SUCCESS); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_SetAppChannels(harness.ssh, 1), WS_SUCCESS); + if (rejectVia != 0) { + AssertIntEQ(wolfSSH_CTX_SetChannelReqSubsysCb(harness.ctx, + RecordingSessionReqCb), WS_SUCCESS); + } - channel = SeedUnconfirmedChannel(harness); - *selfChannelId = channel->channel; + channel = SeedUnconfirmedChannel(&harness); AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); channel->openConfirmed = 1; - inSz = BuildChannelClosePacket(*selfChannelId, in, sizeof(in)); - RepointHarnessInput(harness, in, inSz); + inSz = BuildChannelStringRequestPacket(channel->channel, "subsystem", 1, + "sftp", in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + AssertIntEQ(DoReceive(harness.ssh), WS_SUCCESS); + /* With a callback registered, it did the refusing, not app channels + * standing in for a missing one. */ + AssertIntEQ(sessionReqCbCalls, (rejectVia == 0) ? 0 : 1); + /* Either way the peer is told the subsystem was refused. */ + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + MSGID_CHANNEL_FAILURE); - ret = DoReceive(harness->ssh); - AssertIntEQ(harness->io.inOff, harness->io.inSz); + inSz = BuildSftpInitDataPacket(channel->channel, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); - return ret; + AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_INVALID_STATE_E); + AssertIntEQ(harness.io.outSz, 0); + + FreeChannelOpenHarness(&harness); } -/* The close callback is an application's only notice that a peer closed a - * channel, and it has to run while the channel is still findable: - * DoChannelClose() retires it a few lines later, and after that there is - * nothing left to name. */ -static void TestChannelCloseCallbackRuns(void) -{ - ChannelOpenHarness harness; - word32 selfChannelId; - int cbCtx = 0; - int ret; - ret = CloseThroughRecordingCb(&harness, &cbCtx, WS_SUCCESS, - &selfChannelId); - AssertIntEQ(ret, WS_CHANNEL_CLOSED); - AssertIntEQ(closeCbCalls, 1); - AssertIntEQ(closeCbChannel, selfChannelId); - AssertTrue(closeCbCtx == &cbCtx); - AssertIntEQ(closeCbListSz, 1); +static void TestSftpAcceptAppChannelsRefusesNoCb(void) +{ + CheckSftpAcceptRefusesUngranted(0); +} - /* And the channel is gone by the time the caller is told. */ - AssertIntEQ(harness.ssh->channelListSz, 0); - FreeChannelOpenHarness(&harness); +static void TestSftpAcceptAppChannelsRefusesRejectedCb(void) +{ + CheckSftpAcceptRefusesUngranted(1); } -/* The close callback's return is discarded: the peer has closed whatever - * the application thinks, so the close completes either way. */ -static void TestChannelCloseCallbackReturnIgnored(void) + +/* wolfSSH_accept()'s divert to the built-in server matches the subsystem + * name whole, by length as well as bytes. The last case is the control: + * with no name that does divert, a harness that never reached the check + * would pass every refusal above it. */ +static void TestAcceptDivertMatchesSftpNameWhole(void) { + static const struct { + const char* name; + word32 nameSz; + byte divert; + } cases[] = { + { "sftpx", 5, 0 }, /* longer than sftp */ + { "sfxp", 4, 0 }, /* the length of sftp, other bytes */ + { "sftp\0evil", 9, 0 }, /* sftp up to an embedded NUL */ + { "sftp", 4, 1 }, + }; ChannelOpenHarness harness; - word32 selfChannelId; - int cbCtx = 0; - int ret; + WOLFSSH_CHANNEL* channel; + byte payload[128]; + byte in[128]; + word32 idx; + word32 inSz; + word32 i; - ret = CloseThroughRecordingCb(&harness, &cbCtx, WS_BAD_ARGUMENT, - &selfChannelId); - AssertIntEQ(ret, WS_CHANNEL_CLOSED); - AssertIntEQ(closeCbCalls, 1); - AssertIntEQ(harness.ssh->channelListSz, 0); + for (i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + InitChannelOpenHarness(&harness, NULL, 0); + channel = SeedUnconfirmedChannel(&harness); + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; - FreeChannelOpenHarness(&harness); + idx = 0; + idx = AppendUint32(payload, sizeof(payload), idx, channel->channel); + idx = AppendString(payload, sizeof(payload), idx, "subsystem"); + idx = AppendByte(payload, sizeof(payload), idx, 1); + idx = AppendUint32(payload, sizeof(payload), idx, cases[i].nameSz); + idx = AppendData(payload, sizeof(payload), idx, + (const byte*)cases[i].name, cases[i].nameSz); + inSz = WrapPacket(MSGID_CHANNEL_REQUEST, payload, idx, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + /* Neither app-channels nor a callback, so the request is granted + * and the session is the one wolfSSH_accept() goes on to serve. */ + AssertIntEQ(DoReceive(harness.ssh), WS_SUCCESS); + AssertIntEQ(channel->commandSz, cases[i].nameSz); + RepointHarnessInput(&harness, NULL, 0); + + harness.ssh->acceptState = ACCEPT_SERVER_CHANNEL_ACCEPT_SENT; + if (cases[i].divert) { + /* The built-in server has the session, and stops on the INIT + * the empty input cannot supply. */ + wolfSSH_accept(harness.ssh); + AssertIntEQ(harness.ssh->acceptState, ACCEPT_INIT_SFTP); + } + else { + AssertIntEQ(wolfSSH_accept(harness.ssh), WS_SUCCESS); + AssertIntEQ(harness.ssh->acceptState, + ACCEPT_CLIENT_SESSION_ESTABLISHED); + } + + FreeChannelOpenHarness(&harness); + } } -/* Builds a plaintext SSH_MSG_CHANNEL_REQUEST whose type-specific tail is a - * single string, which is the shape of both "exec" and "subsystem". */ -static word32 BuildChannelStringRequestPacket(word32 recipientChannelId, - const char* type, byte wantReply, const char* arg, - byte* out, word32 outSz) + + +/* The divert asks for the grant, not just the name. A subsystem callback + * that refuses sftp answers CHANNEL_FAILURE, yet sessionType and command + * are recorded ahead of that answer and stay set, so the name alone would + * hand the refused session to the built-in server. */ +static void CheckAcceptDivertNeedsSftpGrant(int reject) { - byte payload[128]; - word32 idx = 0; + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[128]; + word32 inSz; - idx = AppendUint32(payload, sizeof(payload), idx, recipientChannelId); - idx = AppendString(payload, sizeof(payload), idx, type); - idx = AppendByte(payload, sizeof(payload), idx, wantReply); - idx = AppendString(payload, sizeof(payload), idx, arg); + sessionReqCbCalls = 0; + sessionReqCbReturn = reject; - return WrapPacket(MSGID_CHANNEL_REQUEST, payload, idx, out, outSz); -} + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqSubsysCb(harness.ctx, + RecordingSessionReqCb), WS_SUCCESS); -/* What a session request callback saw. */ -static int sessionReqCbCalls; -static WS_SessionType sessionReqCbType; -static char sessionReqCbCommand[32]; -static void* sessionReqCbCtx; -static int sessionReqCbReturn; + channel = SeedUnconfirmedChannel(&harness); + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; -static int RecordingSessionReqCb(WOLFSSH_CHANNEL* channel, void* ctx) -{ - const char* command; + inSz = BuildChannelStringRequestPacket(channel->channel, "subsystem", 1, + "sftp", in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); - sessionReqCbCalls++; - sessionReqCbCtx = ctx; - sessionReqCbCommand[0] = 0; + AssertIntEQ(DoReceive(harness.ssh), WS_SUCCESS); + AssertIntEQ(sessionReqCbCalls, 1); + AssertIntEQ(channel->sessionGranted, reject ? 0 : 1); + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + reject ? MSGID_CHANNEL_FAILURE : MSGID_CHANNEL_SUCCESS); + RepointHarnessInput(&harness, NULL, 0); - if (channel != NULL) { - sessionReqCbType = wolfSSH_ChannelGetSessionType(channel); - command = wolfSSH_ChannelGetSessionCommand(channel); - if (command != NULL) { - WSTRNCPY(sessionReqCbCommand, command, - sizeof(sessionReqCbCommand) - 1); - sessionReqCbCommand[sizeof(sessionReqCbCommand) - 1] = 0; - } + harness.ssh->acceptState = ACCEPT_SERVER_CHANNEL_ACCEPT_SENT; + if (reject) { + AssertIntEQ(wolfSSH_accept(harness.ssh), WS_SUCCESS); + AssertIntEQ(harness.ssh->acceptState, + ACCEPT_CLIENT_SESSION_ESTABLISHED); + } + else { + /* The control: the same name, granted, does reach the built-in + * server, which stops on the INIT the empty input cannot supply. */ + wolfSSH_accept(harness.ssh); + AssertIntEQ(harness.ssh->acceptState, ACCEPT_INIT_SFTP); } - return sessionReqCbReturn; + FreeChannelOpenHarness(&harness); } -/* Drives one session request through a fresh harness, with the callback - * returning cbReturn, and returns the message id the server answered with. */ -static byte RunSessionRequest(const char* type, const char* arg, int cbReturn, - WS_SessionType expectType) + +static void TestAcceptDivertNeedsSftpGrant(void) +{ + CheckAcceptDivertNeedsSftpGrant(1); + CheckAcceptDivertNeedsSftpGrant(0); +} +#endif /* WOLFSSH_SFTP */ + +#ifdef WOLFSSH_SCP + +/* Same for the SCP divert, which reads the command with no grant test of + * its own. An exec callback that refuses "scp ..." must not leave the + * built-in SCP server holding the session it just refused. */ +static void CheckAcceptDivertNeedsScpGrant(int reject) { ChannelOpenHarness harness; WOLFSSH_CHANNEL* channel; byte in[128]; word32 inSz; - int cbCtx = 0; - byte replyId; sessionReqCbCalls = 0; - sessionReqCbType = WOLFSSH_SESSION_UNKNOWN; - sessionReqCbCommand[0] = 0; - sessionReqCbCtx = NULL; - sessionReqCbReturn = cbReturn; + sessionReqCbReturn = reject; InitChannelOpenHarness(&harness, NULL, 0); - if (WSTRCMP(type, "exec") == 0) { - AssertIntEQ(wolfSSH_CTX_SetChannelReqExecCb(harness.ctx, - RecordingSessionReqCb), WS_SUCCESS); - } - else { - AssertIntEQ(wolfSSH_CTX_SetChannelReqSubsysCb(harness.ctx, - RecordingSessionReqCb), WS_SUCCESS); - } - AssertIntEQ(wolfSSH_SetChannelReqCtx(harness.ssh, &cbCtx), WS_SUCCESS); + AssertIntEQ(wolfSSH_CTX_SetChannelReqExecCb(harness.ctx, + RecordingSessionReqCb), WS_SUCCESS); channel = SeedUnconfirmedChannel(&harness); AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); channel->openConfirmed = 1; - inSz = BuildChannelStringRequestPacket(channel->channel, type, 1, arg, - in, sizeof(in)); + inSz = BuildChannelStringRequestPacket(channel->channel, "exec", 1, + "scp -t /tmp/f", in, sizeof(in)); RepointHarnessInput(&harness, in, inSz); AssertIntEQ(DoReceive(harness.ssh), WS_SUCCESS); - AssertIntEQ(harness.io.inOff, harness.io.inSz); AssertIntEQ(sessionReqCbCalls, 1); - AssertTrue(sessionReqCbCtx == &cbCtx); - AssertIntEQ(sessionReqCbType, expectType); - AssertIntEQ(WSTRCMP(sessionReqCbCommand, arg), 0); + AssertIntEQ(channel->sessionGranted, reject ? 0 : 1); + RepointHarnessInput(&harness, NULL, 0); - replyId = ParseMsgId(harness.io.out, harness.io.outSz); - FreeChannelOpenHarness(&harness); + harness.ssh->acceptState = ACCEPT_SERVER_CHANNEL_ACCEPT_SENT; + if (reject) { + AssertIntEQ(wolfSSH_accept(harness.ssh), WS_SUCCESS); + AssertIntEQ(harness.ssh->acceptState, + ACCEPT_CLIENT_SESSION_ESTABLISHED); + } + else { + AssertIntEQ(wolfSSH_accept(harness.ssh), WS_SCP_INIT); + AssertIntEQ(harness.ssh->acceptState, ACCEPT_INIT_SCP_TRANSFER); + } - return replyId; + FreeChannelOpenHarness(&harness); } -/* The exec callback is the only place an application can vet a remote - * command, and its return is what decides the reply on the wire. - * DoChannelRequest() tests only for nonzero, so a bare 1 rejects the same - * as a WS_ error. */ -static void TestChannelReqExecCallbackRuns(void) -{ - AssertIntEQ(RunSessionRequest("exec", "ls", WS_SUCCESS, - WOLFSSH_SESSION_EXEC), MSGID_CHANNEL_SUCCESS); - AssertIntEQ(RunSessionRequest("exec", "ls", WS_BAD_ARGUMENT, - WOLFSSH_SESSION_EXEC), MSGID_CHANNEL_FAILURE); - AssertIntEQ(RunSessionRequest("exec", "ls", 1, - WOLFSSH_SESSION_EXEC), MSGID_CHANNEL_FAILURE); -} -/* Same contract for the subsystem callback, which is how a server decides - * whether to serve SFTP on a channel. */ -static void TestChannelReqSubsysCallbackRuns(void) +static void TestAcceptDivertNeedsScpGrant(void) { - AssertIntEQ(RunSessionRequest("subsystem", "sftp", WS_SUCCESS, - WOLFSSH_SESSION_SUBSYSTEM), MSGID_CHANNEL_SUCCESS); - AssertIntEQ(RunSessionRequest("subsystem", "sftp", WS_BAD_ARGUMENT, - WOLFSSH_SESSION_SUBSYSTEM), MSGID_CHANNEL_FAILURE); - AssertIntEQ(RunSessionRequest("subsystem", "sftp", 1, - WOLFSSH_SESSION_SUBSYSTEM), MSGID_CHANNEL_FAILURE); + CheckAcceptDivertNeedsScpGrant(1); + CheckAcceptDivertNeedsScpGrant(0); } +#endif /* WOLFSSH_SCP */ + /* A username change after the first userauth request must end the session. */ static void TestUsernameChangeDisconnects(void) { @@ -13575,6 +14542,28 @@ int main(int argc, char** argv) TestChannelCloseCallbackReturnIgnored(); TestChannelReqExecCallbackRuns(); TestChannelReqSubsysCallbackRuns(); + TestSessionReqCallbackSeesCommandSz(); + TestMalformedSessionRequestSkipsCallback(); + TestSessionReqCallbackMayFreeChannel(); + TestAppChannelsAcceptKeepsStopWithPendingOutput(); +#ifdef WOLFSSH_SFTP + TestSftpAcceptAppChannelsNeedsSession(); + TestSftpAcceptAppChannelsRefusesPreAccept(); + TestSftpAcceptAppChannelsRefusesShell(); + TestSftpAcceptAppChannelsRefusesPrefixName(); + TestSftpAcceptAppChannelsRefusesNulName(); + TestSftpAcceptAppChannelsRefusesSameLengthName(); + TestSftpAcceptAppChannelsServesGrantedSftp(); + TestSftpAcceptAppChannelsRefusesEstablishedShell(); + TestSftpAcceptAppChannelsServesEstablishedSftp(); + TestSftpAcceptAppChannelsRefusesNoCb(); + TestSftpAcceptAppChannelsRefusesRejectedCb(); + TestAcceptDivertMatchesSftpNameWhole(); + TestAcceptDivertNeedsSftpGrant(); +#endif +#ifdef WOLFSSH_SCP + TestAcceptDivertNeedsScpGrant(); +#endif TestSecondSessionChannelRejected(); TestUsernameChangeDisconnects(); TestSameUserRetryAllowed(); @@ -13780,6 +14769,10 @@ int main(int argc, char** argv) #ifdef KEXDH_REPLY_REGRESS_KEX_ALGO #ifndef WOLFSSH_NO_RSA_SHA2_256 + TestAppChannelsCtxInherits(); + TestAppChannelsAcceptStopsAtUserAuth(); + TestAppChannelsNoShellCbRejects(); + TestAppChannelsLateEnableReturns(); TestKexDhReplyRejectsRsaSha2_256SigNameDowngrade(); #endif #ifndef WOLFSSH_NO_RSA_SHA2_512 diff --git a/tests/unit.c b/tests/unit.c index f67c84084..80f2df2b1 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -9295,6 +9295,68 @@ static int test_DoChannelRequest(void) } #endif /* WOLFSSH_SHELL && WOLFSSH_TERM */ + /* Application-driven channels flip the no-callback default: with + * accept() already returned there is nothing left to start a shell, + * exec or subsystem, so all three are refused rather than accepted. */ + { + static const byte paySubsys[] = { + 0x00,0x00,0x00,0x00, /* channelId = 0 */ + 0x00,0x00,0x00,0x09, /* typeSz = 9 */ + 0x73,0x75,0x62,0x73,0x79,0x73, + 0x74,0x65,0x6D, /* "subsystem" */ + 0x01, /* wantReply = 1 */ + 0x00,0x00,0x00,0x04, /* nameSz = 4 */ + 0x73,0x66,0x74,0x70 /* "sftp" */ + }; + struct { + const char* label; + const byte* payload; + word32 payloadSz; + int errBase; + } appCases[] = { + { "shell", payShell, (word32)sizeof(payShell), -495 }, + { "exec", payExec, (word32)sizeof(payExec), -497 }, + { "subsystem", paySubsys, (word32)sizeof(paySubsys), -499 } + }; + int a; + + for (a = 0; a < (int)(sizeof(appCases) / sizeof(appCases[0])); a++) { + word32 idxApp = 0; + int retApp, capMsgId; + + if (wolfSSH_SetAppChannels(ssh, 1) != WS_SUCCESS) { + printf("DoChannelRequest[app-%s]: set failed\n", + appCases[a].label); + result = appCases[a].errBase; + goto done; + } + + s_chanReqCaptureSz = 0; + WMEMSET(s_chanReqCapture, 0, sizeof(s_chanReqCapture)); + + retApp = wolfSSH_TestDoChannelRequest(ssh, + (byte*)appCases[a].payload, appCases[a].payloadSz, + &idxApp); + wolfSSH_SetAppChannels(ssh, 0); + + if (retApp != WS_SUCCESS) { + printf("DoChannelRequest[app-%s]: ret=%d, expected=%d\n", + appCases[a].label, retApp, WS_SUCCESS); + result = appCases[a].errBase; + goto done; + } + + capMsgId = CaptureMsgId(s_chanReqCapture, s_chanReqCaptureSz); + if (capMsgId != (int)MSGID_CHANNEL_FAILURE) { + printf("DoChannelRequest[app-%s]: msg_id=0x%02x, " + "expected=0x%02x\n", appCases[a].label, capMsgId, + MSGID_CHANNEL_FAILURE); + result = appCases[a].errBase - 1; + goto done; + } + } + } + done: wolfSSH_free(ssh); wolfSSH_CTX_free(ctx); @@ -16848,6 +16910,230 @@ static int test_SshResourceFree_zeroesSecrets(void) return result; } + +/* Verify ChannelDelete wipes the peer's exec/subsystem command line before + * releasing it. A command can carry a password or token in its arguments, + * and the buffer sits right below the inputBuffer this function already + * scrubs. The retain-on-free allocator is installed just around + * ChannelDelete so the freed bytes can be read back without touching + * freed memory. */ +static int test_ChannelDelete_zeroesCommand(void) +{ + static const char command[] = "sh -c 'login --password hunter2'"; + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + WOLFSSH_CHANNEL* channel = NULL; + const byte* commandBytes; + word32 commandSz; + word32 i; + int result = 0; + wolfSSL_Malloc_cb prevMf = NULL; + wolfSSL_Free_cb prevFf = NULL; + wolfSSL_Realloc_cb prevRf = NULL; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -710; + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { + result = -711; + goto out; + } + + channel = ChannelNew(ssh, ID_CHANTYPE_SESSION, + DEFAULT_WINDOW_SZ, DEFAULT_MAX_PACKET_SZ); + if (channel == NULL) { + result = -712; + goto out; + } + + commandSz = (word32)WSTRLEN(command); + channel->command = (char*)WMALLOC(commandSz + 1, NULL, DYNTYPE_STRING); + if (channel->command == NULL) { + result = -713; + goto out; + } + WMEMCPY(channel->command, command, commandSz + 1); + channel->commandSz = commandSz; + commandBytes = (const byte*)channel->command; + + wolfSSL_GetAllocators(&prevMf, &prevFf, &prevRf); + /* Allocators unchanged on failure; nothing to restore. */ + if (wolfSSL_SetAllocators(RetainMalloc, RetainFree, + RetainRealloc) != 0) { + result = -714; + goto out; + } + ChannelDelete(channel, NULL); + wolfSSL_SetAllocators(prevMf, prevFf, prevRf); + channel = NULL; + + if (!IsRetained((void*)commandBytes)) { + result = -715; + goto out; + } + + for (i = 0; i < commandSz; i++) { + if (commandBytes[i] != 0) { + result = -716; + goto out; + } + } + +out: + DrainRetained(); + /* Only the setup-failure paths reach here with a channel; it is never + * on ssh->channelList, so wolfSSH_free() would not release it. */ + if (channel != NULL) + ChannelDelete(channel, ssh->ctx->heap); + if (ssh != NULL) + wolfSSH_free(ssh); + if (ctx != NULL) + wolfSSH_CTX_free(ctx); + return result; +} + +#ifdef WOLFSSH_TEST_INTERNAL + +/* [uint32 channelId][string "exec"][byte wantReply][string command] */ +static word32 BuildExecRequestPayload(byte* out, word32 outSz, + const char* command) +{ + word32 commandSz = (word32)WSTRLEN(command); + word32 idx = 0; + + if (outSz < 17 + commandSz) + return 0; + + PutU32BE(out + idx, 0); idx += UINT32_SZ; + PutU32BE(out + idx, 4); idx += UINT32_SZ; + WMEMCPY(out + idx, "exec", 4); idx += 4; + out[idx++] = 1; + PutU32BE(out + idx, commandSz); idx += UINT32_SZ; + WMEMCPY(out + idx, command, commandSz); idx += commandSz; + + return idx; +} + + +/* Verify a repeat exec request wipes the command line it replaces. + * GetStringAlloc() frees the old buffer to take the new one, so without + * the scrub the earlier command, credentials and all, stays readable in + * the freed block. Only the last one ever reaches ChannelDelete(). The + * retain-on-free allocator is installed just around the second request + * so the freed bytes can be read back. */ +static int test_DoChannelRequest_zeroesReplacedCommand(void) +{ + static const char first[] = "sh -c 'login --password hunter2'"; + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + WOLFSSH_CHANNEL* ch = NULL; + const byte* commandBytes; + byte payload[128]; + word32 payloadSz; + word32 commandSz; + word32 idx; + word32 i; + int result = 0; + wolfSSL_Malloc_cb prevMf = NULL; + wolfSSL_Free_cb prevFf = NULL; + wolfSSL_Realloc_cb prevRf = NULL; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -720; + wolfSSH_SetIOSend(ctx, CaptureIoSendChanReq); + + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { + result = -721; + goto out; + } + + ch = ChannelNew(ssh, ID_CHANTYPE_SESSION, + DEFAULT_WINDOW_SZ, DEFAULT_MAX_PACKET_SZ); + if (ch == NULL) { + result = -722; + goto out; + } + if (ChannelAppend(ssh, ch) != WS_SUCCESS) { + ChannelDelete(ch, ssh->ctx->heap); + result = -723; + goto out; + } + + idx = 0; + payloadSz = BuildExecRequestPayload(payload, sizeof(payload), first); + if (payloadSz == 0) { + result = -724; + goto out; + } + if (wolfSSH_TestDoChannelRequest(ssh, payload, payloadSz, &idx) + != WS_SUCCESS) { + result = -725; + goto out; + } + + commandSz = ch->commandSz; + commandBytes = (const byte*)ch->command; + if (commandBytes == NULL || commandSz != (word32)WSTRLEN(first)) { + result = -726; + goto out; + } + + idx = 0; + payloadSz = BuildExecRequestPayload(payload, sizeof(payload), "ls"); + if (payloadSz == 0) { + result = -727; + goto out; + } + + wolfSSL_GetAllocators(&prevMf, &prevFf, &prevRf); + /* Allocators unchanged on failure; nothing to restore. */ + if (wolfSSL_SetAllocators(RetainMalloc, RetainFree, + RetainRealloc) != 0) { + result = -728; + goto out; + } + result = wolfSSH_TestDoChannelRequest(ssh, payload, payloadSz, &idx); + wolfSSL_SetAllocators(prevMf, prevFf, prevRf); + if (result != WS_SUCCESS) { + result = -729; + goto out; + } + result = 0; + + if (!IsRetained((void*)commandBytes)) { + result = -730; + goto out; + } + + for (i = 0; i < commandSz; i++) { + if (commandBytes[i] != 0) { + result = -731; + goto out; + } + } + + /* The replacement arrived whole, so the scrub hit the old buffer + * rather than the one in use. */ + if (ch->command == NULL || WSTRCMP(ch->command, "ls") != 0 + || ch->commandSz != 2) { + result = -732; + goto out; + } + +out: + DrainRetained(); + if (ssh != NULL) + wolfSSH_free(ssh); + if (ctx != NULL) + wolfSSH_CTX_free(ctx); + return result; +} + +#endif /* WOLFSSH_TEST_INTERNAL */ + #endif /* WOLFSSH_TEST_CAPTURING_ALLOCATOR */ #ifndef WOLFSSH_NO_DH @@ -21276,6 +21562,18 @@ int wolfSSH_UnitTest(int argc, char** argv) printf("SshResourceFree_zeroesSecrets: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + + unitResult = test_ChannelDelete_zeroesCommand(); + printf("ChannelDelete_zeroesCommand: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + +#ifdef WOLFSSH_TEST_INTERNAL + unitResult = test_DoChannelRequest_zeroesReplacedCommand(); + printf("DoChannelRequest_zeroesReplacedCommand: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; +#endif #endif #ifndef WOLFSSH_NO_DH diff --git a/wolfssh/internal.h b/wolfssh/internal.h index be5905afd..d5b9efa53 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -898,6 +898,7 @@ struct WOLFSSH_CTX { word32 maxAuthAttempts; /* server cap on failed userauth */ byte side; /* client or server */ byte showBanner; + byte appChannels; /* app drives channels, see ssh.h */ #ifdef WOLFSSH_AGENT byte agentEnabled; #endif /* WOLFSSH_AGENT */ @@ -1167,6 +1168,7 @@ struct WOLFSSH { byte serverState; byte processReplyState; byte isKeying; + byte appChannels; /* app drives channels, see ssh.h */ byte authId; /* if using public key or password */ byte supportedAuth[4]; /* supported auth IDs public key , password */ @@ -1411,6 +1413,11 @@ struct WOLFSSH_CHANNEL { byte openConfirmed : 1; byte ptyReq : 1; /* flag for if interactive pty request was received */ byte fwdSetupTxd : 1; /* a LOCAL_SETUP succeeded, a cleanup is owed */ + byte sessionGranted : 1; /* a shell, exec or subsystem request was + * answered CHANNEL_SUCCESS. sessionType and + * command are recorded before that answer is + * decided and stay set on a refusal, so they + * do not say whether anything was granted. */ word32 channel; word32 windowSz; word32 maxPacketSz; @@ -1440,6 +1447,7 @@ struct WOLFSSH_CHANNEL { * Accumulates unread data, does not overwrite * it. */ char* command; + word32 commandSz; struct WOLFSSH* ssh; struct WOLFSSH_CHANNEL* next; }; diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index f768fe9e7..bd461a77d 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -436,6 +436,8 @@ WOLFSSH_API WS_SessionType wolfSSH_ChannelGetSessionType( const WOLFSSH_CHANNEL* channel); WOLFSSH_API const char* wolfSSH_ChannelGetSessionCommand( const WOLFSSH_CHANNEL* channel); +WOLFSSH_API word32 wolfSSH_ChannelGetSessionCommandSz( + const WOLFSSH_CHANNEL* channel); WOLFSSH_API int wolfSSH_ChannelIsPty(const WOLFSSH_CHANNEL* channel); /* Channel callbacks */ @@ -461,6 +463,32 @@ WOLFSSH_API int wolfSSH_CTX_SetChannelReqSubsysCb(WOLFSSH_CTX* ctx, WOLFSSH_API int wolfSSH_SetChannelReqCtx(WOLFSSH* ssh, void* ctx); WOLFSSH_API void* wolfSSH_GetChannelReqCtx(WOLFSSH* ssh); +/* Application-driven channel handling, server side, off by default. + * + * Off, wolfSSH_accept() runs the session state machine through to an + * established session with the first channel open, as it always has, and a + * shell, exec, or subsystem request with no callback registered for it is + * accepted. + * + * On, wolfSSH_accept() returns WS_SUCCESS as soon as the user has + * authenticated, and the application owns every channel from there, driving + * the session with wolfSSH_worker() and the callbacks above. A shell, exec, + * or subsystem request with no callback registered is then rejected: with + * accept() already returned, nothing is left to service it. + * + * Set it on the context before wolfSSH_new(), or on a session before the + * first wolfSSH_accept() call. Turning it on later still applies to the + * channel requests that follow, but it cannot move where accept() returns + * on a session that has already gone past the user-auth stop. + * + * accept() never reaches the built-in SCP entry point in this mode, so + * WS_SCP_INIT is off the table. wolfSSH_SFTP_accept() still serves, but only + * a session channel the subsystem callback granted sftp on; called ahead of + * that it returns WS_INVALID_STATE_E without recording an error. A pending + * want-read or want-write is still cleared, as on any other call. */ +WOLFSSH_API int wolfSSH_CTX_SetAppChannels(WOLFSSH_CTX* ctx, byte enable); +WOLFSSH_API int wolfSSH_SetAppChannels(WOLFSSH* ssh, byte enable); + typedef int (*WS_CallbackChannelEof)(WOLFSSH_CHANNEL* channel, void* ctx); WOLFSSH_API int wolfSSH_CTX_SetChannelEofCb(WOLFSSH_CTX* ctx, WS_CallbackChannelEof cb); @@ -863,6 +891,7 @@ WOLFSSH_API int wolfSSH_ConvertConsole(WOLFSSH* ssh, WOLFSSH_HANDLE handle, WOLFSSH_API int wolfSSH_DoModes(const byte* modes, word32 modesSz, int fd); WOLFSSH_API WS_SessionType wolfSSH_GetSessionType(const WOLFSSH* ssh); WOLFSSH_API const char* wolfSSH_GetSessionCommand(const WOLFSSH* ssh); +WOLFSSH_API word32 wolfSSH_GetSessionCommandSz(const WOLFSSH* ssh); WOLFSSH_API int wolfSSH_SetChannelType(WOLFSSH* ssh, byte type, byte* name, word32 nameSz); WOLFSSH_API int wolfSSH_ChangeTerminalSize(WOLFSSH* ssh, word32 columns,