Skip to content

ssh: add opt-in application-driven channels #1233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ejohnstown wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

ssh: add opt-in application-driven channels #1233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
96aa303
agent: let the application open the agent channel
ejohnstown Sep 2, 2026
d166aa8
agent: refuse a channel open after a disconnect
ejohnstown Sep 4, 2026
fa893cf
ssh: add opt-in application-driven channels
ejohnstown Sep 2, 2026
fa43c78
tests: cover application-driven channels
ejohnstown Sep 2, 2026
f359ec4
ssh: correct what a late app-channels enable does
ejohnstown Sep 4, 2026
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
85 changes: 85 additions & 0 deletions src/agent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Sep 4, 2026

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() latches its benign "not requested yet" return into ssh-error · Incorrect error handling

Every return is written into ssh->error, including the WS_BAD_ARGUMENT reported while the peer has not asked for agent forwarding — the polling agent.h documents as safe. wolfSSH_accept() (src/ssh.c:598) clears only the want/auth-pending codes, so a poll poisons the session and later accepts return WS_INVALID_STATE_E. The success path likewise clears a latched WS_WANT_WRITE. Sibling wolfSSH_AGENT_Relay() sets ssh->error only on genuine failure.

Fix: Record ssh->error only for genuine failures, leaving it untouched on WS_SUCCESS and on the WS_BAD_ARGUMENT no-agent-requested return.


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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -12725,6 +12726,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 @@ -12734,6 +12738,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 @@ -12745,6 +12752,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 @@ -12886,7 +12896,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
112 changes: 59 additions & 53 deletions src/ssh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,10 +567,6 @@ static int DoReceiveHandshake(WOLFSSH* ssh)
#endif /* !NO_WOLFSSH_SERVER || !NO_WOLFSSH_CLIENT */


/* Defined below, ahead of both drivers; either can be the only one built. */
static int SendAfterDisconnect(WOLFSSH* ssh);


#ifndef NO_WOLFSSH_SERVER

const char acceptError[] = "accept error: %s, %d";
Expand All @@ -579,6 +575,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 +596,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 +616,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 +642,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 +732,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 +781,17 @@ 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;
}
int agentRet = wolfSSH_AGENT_ChannelOpen(ssh);

ssh->error = SendChannelOpenSession(ssh, newChannel);
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);
}
if (agentRet < WS_SUCCESS) {
/* WS_FATAL_ERROR is the disconnect, which already
* recorded WS_DISCONNECT; keep that. */
if (agentRet != WS_FATAL_ERROR)
ssh->error = agentRet;
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 @@ -1134,11 +1116,8 @@ int wolfSSH_connect(WOLFSSH* ssh)
#endif /* NO_WOLFSSH_CLIENT */


/* A disconnect, sent or received, ends the session, so nothing further may
* go out. RFC 4253 section 11.1. Reads are deliberately not gated on this:
* channel data that arrived before the disconnect is still the caller's.
* Call only after ssh has been checked for NULL. */
static int SendAfterDisconnect(WOLFSSH* ssh)
/* See wolfssh/internal.h for the contract. */
int SendAfterDisconnect(WOLFSSH* ssh)
{
if (ssh->disconnected) {
WLOG(WS_LOG_DEBUG, "Send attempted after a disconnect");
Expand Down Expand Up @@ -3963,7 +3942,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 @@ -4957,6 +4937,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
8 changes: 6 additions & 2 deletions src/wolfsftp.c
View file
Open in desktop
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

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Sep 4, 2026

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_SFTP_accept() serves SFTP without a granted subsystem request in app-channels mode · Channel handling errors

The new guard drops into the SFTP state machine as soon as ssh->appChannels is set and userauth completed, with no requirement that the peer opened a channel or that a subsystem sftp request was granted. With appChannels on and no channelReqSubsysCb, DoChannelRequest replies CHANNEL_FAILURE (rej = ssh->appChannels) yet SFTP is still served on that channel. Adjacent to known finding #8852, which covers state committed in DoChannelRequest after a callback rejects; this is a separate gate removed in wolfSSH_SFTP_accept.

Fix: Gate the app-channels bypass on the session request having been granted (e.g. clientState >= CLIENT_DONE and session type SUBSYSTEM/sftp) rather than on userauth alone.

&& ssh->acceptState >= ACCEPT_SERVER_USERAUTH_SENT)) {
byte name[] = "sftp";

WLOG(WS_LOG_SFTP, "Trying to do SSH accept first");
Expand Down
Loading
Loading