Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
510 changes: 357 additions & 153 deletions examples/echoserver/echoserver.c

Large diffs are not rendered by default.

65 changes: 65 additions & 0 deletions src/agent.c
Original file line number Diff line number Diff line change
Expand Up @@ -1731,6 +1731,71 @@ 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;

WLOG_ENTER();

if (ssh == NULL)
ret = WS_SSH_NULL_E;
else if (!ssh->useAgent) {
/* Nothing asked for agent forwarding on this session. */
ret = WS_BAD_ARGUMENT;
}
else if (ssh->agent == NULL) {
/* Server side sets ssh->agent here and nowhere else, so a NULL one
* is the "not opened yet" test. Idempotent so a caller polling for
* the peer's request cannot end up with two agent channels. */
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) {
ret = SendChannelOpenSession(ssh, newChannel);

if (ret < WS_SUCCESS
&& ret != WS_WANT_WRITE && ret != WS_WANT_READ) {
ChannelDelete(newChannel, ssh->ctx->heap);
}
else {
/* Publish the agent even when the open is only queued, so
* a retry takes the already-open path above rather than
* opening a second channel. */
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);
}

if (ssh != NULL)
ssh->error = ret;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wolfSSH_AGENT_ChannelOpen() stores WS_BAD_ARGUMENT into ssh-error on its documented polling path · Channel handling errors

The function is documented as pollable and returns WS_BAD_ARGUMENT while ssh->useAgent is 0, but it also writes that value into ssh->error. wolfSSH_accept() (src/ssh.c:598) returns WS_INVALID_STATE_E for any non-zero ssh->error, so polling for the agent channel poisons the session's error slot and breaks a concurrently driven accept.

Fix: Only assign ssh->error for real failures, leaving it untouched on the not-yet-requested and idempotent-success paths.


WLOG_LEAVE(ret);
return ret;
}


int wolfSSH_AGENT_worker(WOLFSSH* ssh)
{
int ret = WS_SUCCESS;
Expand Down
12 changes: 11 additions & 1 deletion src/internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -1666,6 +1666,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;
Expand Down Expand Up @@ -12675,6 +12676,9 @@ static int DoChannelRequest(WOLFSSH* ssh,
if (ssh->ctx->channelReqShellCb) {
rej = ssh->ctx->channelReqShellCb(channel, ssh->channelReqCtx);
}
else {
rej = ssh->appChannels;
}
ssh->clientState = CLIENT_DONE;
}
else if (ChannelRequestIs(type, typeSz, "exec")) {
Expand All @@ -12684,6 +12688,9 @@ static int DoChannelRequest(WOLFSSH* ssh,
if (ssh->ctx->channelReqExecCb) {
rej = ssh->ctx->channelReqExecCb(channel, ssh->channelReqCtx);
}
else {
rej = ssh->appChannels;
}
ssh->clientState = CLIENT_DONE;

WLOG(WS_LOG_DEBUG, " command = %s", channel->command);
Expand All @@ -12695,6 +12702,9 @@ static int DoChannelRequest(WOLFSSH* ssh,
if (ssh->ctx->channelReqSubsysCb) {
rej = ssh->ctx->channelReqSubsysCb(channel, ssh->channelReqCtx);
}
else {
rej = ssh->appChannels;
}
ssh->clientState = CLIENT_DONE;

WLOG(WS_LOG_DEBUG, " subsystem = %s", channel->command);
Expand Down Expand Up @@ -12836,7 +12846,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));
Expand Down
96 changes: 52 additions & 44 deletions src/ssh.c
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,8 @@ const char acceptState[] = "accept state: %s";

int wolfSSH_accept(WOLFSSH* ssh)
{
byte stopState;

WLOG(WS_LOG_DEBUG, "Entering wolfSSH_accept()");

if (ssh == NULL)
Expand All @@ -598,6 +600,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) {
Expand All @@ -609,7 +620,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++;
}
Expand All @@ -631,7 +646,7 @@ int wolfSSH_accept(WOLFSSH* ssh)
}
}

while (ssh->acceptState != ACCEPT_CLIENT_SESSION_ESTABLISHED) {
while (ssh->acceptState != stopState) {
switch (ssh->acceptState) {

case ACCEPT_BEGIN:
Expand Down Expand Up @@ -721,6 +736,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:
Expand Down Expand Up @@ -764,52 +785,12 @@ int wolfSSH_accept(WOLFSSH* ssh)
#endif /* WOLFSSH_SFTP and !NO_WOLFSSH_SERVER */
#ifdef WOLFSSH_AGENT
if (ssh->useAgent) {
WOLFSSH_AGENT_CTX* newAgent;
WOLFSSH_CHANNEL* newChannel;

WLOG(WS_LOG_AGENT, "Starting agent channel");

newAgent = wolfSSH_AGENT_new(ssh->ctx->heap);
if (newAgent == NULL) {
ssh->error = WS_MEMORY_E;
WLOG(WS_LOG_DEBUG, acceptError,
"SERVER_USERAUTH_ACCEPT_DONE", ssh->error);
return WS_ERROR;
}

newChannel = ChannelNew(ssh, ID_CHANTYPE_AUTH_AGENT,
ssh->ctx->windowSz, ssh->ctx->maxPacketSz);
if (newChannel == NULL) {
wolfSSH_AGENT_free(newAgent);
ssh->error = WS_MEMORY_E;
WLOG(WS_LOG_DEBUG, acceptError,
"SERVER_USERAUTH_ACCEPT_DONE", ssh->error);
return WS_FATAL_ERROR;
}

ssh->error = SendChannelOpenSession(ssh, newChannel);
ssh->error = wolfSSH_AGENT_ChannelOpen(ssh);
if (ssh->error < WS_SUCCESS) {
if (ssh->error == WS_WANT_WRITE ||
ssh->error == WS_WANT_READ) {
ChannelAppend(ssh, newChannel);
}
else {
ChannelDelete(newChannel, ssh->ctx->heap);
wolfSSH_AGENT_free(newAgent);
}
WLOG(WS_LOG_DEBUG, acceptError,
"SERVER_USERAUTH_ACCEPT_DONE", ssh->error);
return WS_FATAL_ERROR;
}
ChannelAppend(ssh, newChannel);
newAgent->channel = newChannel->channel;
if (ssh->ctx->agentCb) {
ssh->ctx->agentCb(WOLFSSH_AGENT_LOCAL_SETUP,
ssh->agentCbCtx);
}
if (ssh->agent != NULL)
wolfSSH_AGENT_free(ssh->agent);
ssh->agent = newAgent;
}
#endif /* WOLFSSH_AGENT */
ssh->acceptState = ACCEPT_CLIENT_SESSION_ESTABLISHED;
Expand Down Expand Up @@ -3937,7 +3918,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;
}
Expand Down Expand Up @@ -4931,6 +4913,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;
Expand Down
34 changes: 34 additions & 0 deletions src/wolfscp.c
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,40 @@ int DoScpSource(WOLFSSH* ssh)
return ret;
}

/* Contract is in wolfssh/wolfscp.h. */
int wolfSSH_SCP_accept(WOLFSSH* ssh)
{
int ret;

if (ssh == NULL)
return WS_BAD_ARGUMENT;

/* Clear a want left by the previous call so the retry starts clean,
* the way the other re-entrant entry points do. */
if (ssh->error == WS_WANT_READ || ssh->error == WS_WANT_WRITE)
ssh->error = WS_SUCCESS;

ret = DoScpRequest(ssh);

if (ret >= WS_SUCCESS) {
/* The tail of DoScpRequest() passes a read count through, so treat
* anything non-negative as done the way wolfSSH_accept() does. */
ret = WS_SCP_COMPLETE;
}
else {
/* A non-blocking want on a read path surfaces as a generic error
* with the want recorded in ssh->error (see GetInputData), so
* report it as the want the caller is told to retry on. */
int err = wolfSSH_get_error(ssh);

if (err == WS_WANT_READ || err == WS_WANT_WRITE)
ret = err;
}

return ret;
}


int DoScpRequest(WOLFSSH* ssh)
{
int ret = WS_SUCCESS;
Expand Down
8 changes: 6 additions & 2 deletions src/wolfsftp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1383,8 +1383,12 @@ int wolfSSH_SFTP_accept(WOLFSSH* ssh)
if (ssh->error == WS_WANT_READ || ssh->error == WS_WANT_WRITE)
ssh->error = WS_SUCCESS;

/* check accept is done, if not call wolfSSH accept */
if (ssh->acceptState < ACCEPT_CLIENT_SESSION_ESTABLISHED) {
/* check accept is done, if not call wolfSSH accept. In
* application-driven mode accept() parks at ACCEPT_SERVER_USERAUTH_SENT
* and never advances, so that state counts as done here. */
if (ssh->acceptState < ACCEPT_CLIENT_SESSION_ESTABLISHED
&& !(ssh->appChannels
&& ssh->acceptState >= ACCEPT_SERVER_USERAUTH_SENT)) {
byte name[] = "sftp";

WLOG(WS_LOG_SFTP, "Trying to do SSH accept first");
Expand Down
3 changes: 3 additions & 0 deletions tests/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -7906,6 +7906,9 @@ static void test_wolfSSH_KeyboardInteractive(void)
argsCount = 0;
args[argsCount++] = ".";
args[argsCount++] = "-1";
/* Echo mode: "test" is not an account on the host, so the echoserver's
* shell callback would refuse the shell request this client sends. */
args[argsCount++] = "-f";
args[argsCount++] = "-i";
args[argsCount++] = "test:test";
args[argsCount++] = "-p";
Expand Down
Loading
Loading