From 96aa3032ed1b1a8e4ae45d1de92fd5ed24ddb8fd Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2026 09:40:30 -0700 Subject: [PATCH 1/7] agent: let the application open the agent channel The one server-side site that opens auth-agent@openssh.com sits inside wolfSSH_accept(), so an application driving its own channels cannot reach it: the session records the request and no channel follows. - add wolfSSH_AGENT_ChannelOpen(), the same open lifted out of accept(), which still calls it - it reports WS_BAD_ARGUMENT until the peer asks and on a client session, and is idempotent after, so an application can poll it - publish the agent on a queued open too, so a retry after WS_WANT_WRITE finds it rather than opening a second channel and leaking the first - flush what is left of a queued open on the next call, rather than reporting a success the peer never saw - record ssh->error from the send alone, so neither a poll ahead of the request nor a failed allocation stops accept() continuing --- src/agent.c | 78 ++++++++++++++++++++++++++++++++++++ src/ssh.c | 42 +------------------- tests/regress.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++ wolfssh/agent.h | 12 ++++++ 4 files changed, 193 insertions(+), 41 deletions(-) diff --git a/src/agent.c b/src/agent.c index 33c0eb936..f46d72966 100644 --- a/src/agent.c +++ b/src/agent.c @@ -1731,6 +1731,84 @@ 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 (!ssh->useAgent) { + /* Nothing asked for agent forwarding on this session. */ + ret = WS_BAD_ARGUMENT; + } + else if (ssh->agent == NULL) { + /* Nothing else sets ssh->agent, so a NULL one means "not opened + * yet". Idempotent, so a poll cannot open a second channel. */ + WLOG(WS_LOG_AGENT, "Starting agent channel"); + + newAgent = wolfSSH_AGENT_new(ssh->ctx->heap); + if (newAgent == NULL) + ret = WS_MEMORY_E; + + if (ret == WS_SUCCESS) { + newChannel = ChannelNew(ssh, ID_CHANTYPE_AUTH_AGENT, + ssh->ctx->windowSz, ssh->ctx->maxPacketSz); + if (newChannel == NULL) + ret = WS_MEMORY_E; + } + + if (ret == WS_SUCCESS) { + recordError = 1; + ret = SendChannelOpenSession(ssh, newChannel); + + if (ret < WS_SUCCESS + && ret != WS_WANT_WRITE && ret != WS_WANT_READ) { + ChannelDelete(newChannel, ssh->ctx->heap); + } + else { + /* Publish on a queued open too, so a retry takes the + * already-open path rather than opening a second. */ + ChannelAppend(ssh, newChannel); + newAgent->channel = newChannel->channel; + ssh->agent = newAgent; + newAgent = NULL; + if (ssh->ctx->agentCb) { + ssh->ctx->agentCb(WOLFSSH_AGENT_LOCAL_SETUP, + ssh->agentCbCtx); + } + } + } + + if (newAgent != NULL) + wolfSSH_AGENT_free(newAgent); + } + else if (wolfSSH_OutputPending(ssh)) { + /* Any queued output, not just this open. Flush it rather than + * report a success the peer hasn't seen. */ + recordError = 1; + ret = wolfSSH_SendPacket(ssh); + } + + if (recordError) + ssh->error = ret; + + WLOG_LEAVE(ret); + return ret; +} + + int wolfSSH_AGENT_worker(WOLFSSH* ssh) { int ret = WS_SUCCESS; diff --git a/src/ssh.c b/src/ssh.c index c02609769..cf1ac32f6 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -764,52 +764,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; diff --git a/tests/regress.c b/tests/regress.c index c87c888d7..3c248d991 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -4437,6 +4437,102 @@ static void TestAgentChannelNullAgentSendsOpenFail(void) FreeChannelOpenHarness(&harness); } + +/* Nothing asked for forwarding, so the open is refused rather than started. + * The refusal is the documented answer to a poll, so it must not land in + * ssh->error: wolfSSH_accept() would then abort with WS_INVALID_STATE_E. */ +static void TestAgentChannelOpenWithoutRequest(void) +{ + ChannelOpenHarness harness; + + InitChannelOpenHarness(&harness, NULL, 0); + + AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_BAD_ARGUMENT); + AssertNull(harness.ssh->agent); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + /* The handshake survives the poll: no input, so accept only wants read. */ + AssertIntEQ(wolfSSH_accept(harness.ssh), WS_FATAL_ERROR); + AssertIntEQ(harness.ssh->error, WS_WANT_READ); + + FreeChannelOpenHarness(&harness); +} + +/* A queued open publishes the agent, so the caller's next poll must finish + * the send rather than report a success the peer never saw, and must not + * open a second channel. */ +static void TestAgentChannelOpenFlushesQueuedOpen(void) +{ + ChannelOpenHarness harness; + word32 outSz; + + InitChannelOpenHarness(&harness, NULL, 0); + harness.ssh->useAgent = 1; + harness.io.blockNext = 1; + + AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_WANT_WRITE); + AssertNotNull(harness.ssh->agent); + AssertIntEQ(harness.ssh->channelListSz, 1); + AssertIntEQ(harness.io.outSz, 0); + + AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_SUCCESS); + AssertIntEQ(harness.ssh->channelListSz, 1); + AssertTrue(harness.io.outSz > 0); + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + MSGID_CHANNEL_OPEN); + + /* The flushed open is the answer wolfSSH_accept() retries on: success, + * no second channel, no new packet, ssh->error untouched. */ + outSz = harness.io.outSz; + harness.ssh->error = WS_SUCCESS; + + AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_SUCCESS); + AssertIntEQ(harness.ssh->channelListSz, 1); + AssertIntEQ(harness.io.outSz, outSz); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} + +/* A send that fails outright, rather than blocking, leaves nothing behind, + * so a later poll starts the open over. */ +static void TestAgentChannelOpenSendFailureCleansUp(void) +{ + ChannelOpenHarness harness; + + InitChannelOpenHarness(&harness, NULL, 0); + harness.ssh->useAgent = 1; + /* No room, so MemSend reports a general error. */ + harness.io.outCap = 0; + + AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_SOCKET_ERROR_E); + AssertNull(harness.ssh->agent); + AssertIntEQ(harness.ssh->channelListSz, 0); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.ssh->error, WS_SOCKET_ERROR_E); + + FreeChannelOpenHarness(&harness); +} + +#ifndef NO_WOLFSSH_CLIENT +/* Server-side call. A client has an ssh->agent of its own, so answering the + * poll from it would report a channel that was never opened. */ +static void TestAgentChannelOpenOnClientRefused(void) +{ + ChannelOpenHarness harness; + + InitChannelOpenHarnessClient(&harness, NULL, 0); + harness.ssh->useAgent = 1; + + AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_BAD_ARGUMENT); + AssertIntEQ(harness.ssh->channelListSz, 0); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} +#endif /* !NO_WOLFSSH_CLIENT */ #endif @@ -13427,6 +13523,12 @@ int main(int argc, char** argv) #endif #ifdef WOLFSSH_AGENT TestAgentChannelNullAgentSendsOpenFail(); + TestAgentChannelOpenWithoutRequest(); + TestAgentChannelOpenFlushesQueuedOpen(); + TestAgentChannelOpenSendFailureCleansUp(); +#ifndef NO_WOLFSSH_CLIENT + TestAgentChannelOpenOnClientRefused(); +#endif #endif #endif /* NO_WOLFSSH_SERVER */ #if defined(WOLFSSH_AGENT) && !defined(WOLFSSH_NO_ED25519) \ diff --git a/wolfssh/agent.h b/wolfssh/agent.h index 581e3eba9..8f57bc5df 100644 --- a/wolfssh/agent.h +++ b/wolfssh/agent.h @@ -181,6 +181,18 @@ WOLFSSH_API int wolfSSH_CTX_set_agent_cb(WOLFSSH_CTX* ctx, WOLFSSH_API int wolfSSH_set_agent_cb_ctx(WOLFSSH* ssh, void* ctx); WOLFSSH_API int wolfSSH_CTX_AGENT_enable(WOLFSSH_CTX* ctx, byte isEnabled); WOLFSSH_API int wolfSSH_AGENT_enable(WOLFSSH* ssh, byte isEnabled); +/* Server side. Opens the auth-agent@openssh.com channel to the client once + * the peer's auth-agent-req@openssh.com asks for forwarding. wolfSSH_accept() + * does it on the default path; an application driving its own channels polls + * this instead. Opens one channel, then flushes what of the open is queued. + * Returns WS_SUCCESS, WS_BAD_ARGUMENT before the peer asks or on a client + * session, WS_WANT_READ or WS_WANT_WRITE while output is still queued, + * WS_SSH_NULL_E, WS_MEMORY_E, or whatever the send reports. WS_SUCCESS says + * the open went out, not that the peer took it; a refusal reaches the + * channel-open-fail callback. + * Only the send records in ssh->error, so a poll ahead of the peer's request + * leaves the session fit for wolfSSH_accept(). */ +WOLFSSH_API int wolfSSH_AGENT_ChannelOpen(WOLFSSH* ssh); WOLFSSH_LOCAL int wolfSSH_AGENT_worker(WOLFSSH* ssh); WOLFSSH_API int wolfSSH_AGENT_Relay(WOLFSSH* ssh, const byte* msg, word32* msgSz, byte* rsp, word32* rspSz); From d166aa895c219362e6096de6e041c92f5c037606 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 4 Sep 2026 13:41:16 -0700 Subject: [PATCH 2/7] agent: refuse a channel open after a disconnect wolfSSH_AGENT_ChannelOpen() answers a poll on a session that is over with WS_FATAL_ERROR and WS_DISCONNECT in ssh->error, the shape every other public sender uses: no channel opened, nothing on the wire, RFC 4253 section 11.1. wolfSSH_accept() gates the open it drives, so the new public entry point is the only way in. - promote SendAfterDisconnect() to WOLFSSH_LOCAL so agent.c uses the same helper as every other public sender - leave an open queued before the disconnect unflushed, the rule wolfSSH_shutdown() applies to all but its own disconnect - keep WS_DISCONNECT in ssh->error at the accept() call site, which used to overwrite it with the status the open returns --- src/agent.c | 7 +++++++ src/ssh.c | 20 +++++++++----------- tests/regress.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ wolfssh/agent.h | 5 +++-- wolfssh/internal.h | 6 ++++++ 5 files changed, 69 insertions(+), 13 deletions(-) diff --git a/src/agent.c b/src/agent.c index f46d72966..ba8618112 100644 --- a/src/agent.c +++ b/src/agent.c @@ -1749,6 +1749,13 @@ int wolfSSH_AGENT_ChannelOpen(WOLFSSH* ssh) * 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; diff --git a/src/ssh.c b/src/ssh.c index cf1ac32f6..36646514c 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -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"; @@ -764,8 +760,13 @@ int wolfSSH_accept(WOLFSSH* ssh) #endif /* WOLFSSH_SFTP and !NO_WOLFSSH_SERVER */ #ifdef WOLFSSH_AGENT if (ssh->useAgent) { - ssh->error = wolfSSH_AGENT_ChannelOpen(ssh); - if (ssh->error < WS_SUCCESS) { + int agentRet = wolfSSH_AGENT_ChannelOpen(ssh); + + 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; @@ -1094,11 +1095,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"); diff --git a/tests/regress.c b/tests/regress.c index 3c248d991..233d3b2bb 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -4459,6 +4459,48 @@ static void TestAgentChannelOpenWithoutRequest(void) FreeChannelOpenHarness(&harness); } +/* A poll after the peer disconnects must not open a channel or put anything + * on the wire. RFC 4253 section 11.1: the session is over. */ +static void TestAgentChannelOpenAfterDisconnect(void) +{ + ChannelOpenHarness harness; + + InitChannelOpenHarness(&harness, NULL, 0); + harness.ssh->useAgent = 1; + harness.ssh->disconnected = 1; + + AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_FATAL_ERROR); + AssertNull(harness.ssh->agent); + AssertIntEQ(harness.ssh->channelListSz, 0); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.ssh->error, WS_DISCONNECT); + + FreeChannelOpenHarness(&harness); +} + +/* An open queued before the disconnect is not flushed either: those bytes + * belong to a session that is over, the same rule wolfSSH_shutdown() applies + * to everything but its own queued disconnect. */ +static void TestAgentChannelOpenQueuedThenDisconnect(void) +{ + ChannelOpenHarness harness; + + InitChannelOpenHarness(&harness, NULL, 0); + harness.ssh->useAgent = 1; + harness.io.blockNext = 1; + + AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_WANT_WRITE); + AssertIntEQ(harness.io.outSz, 0); + + harness.ssh->disconnected = 1; + + AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_FATAL_ERROR); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.ssh->error, WS_DISCONNECT); + + FreeChannelOpenHarness(&harness); +} + /* A queued open publishes the agent, so the caller's next poll must finish * the send rather than report a success the peer never saw, and must not * open a second channel. */ @@ -13525,6 +13567,8 @@ int main(int argc, char** argv) TestAgentChannelNullAgentSendsOpenFail(); TestAgentChannelOpenWithoutRequest(); TestAgentChannelOpenFlushesQueuedOpen(); + TestAgentChannelOpenAfterDisconnect(); + TestAgentChannelOpenQueuedThenDisconnect(); TestAgentChannelOpenSendFailureCleansUp(); #ifndef NO_WOLFSSH_CLIENT TestAgentChannelOpenOnClientRefused(); diff --git a/wolfssh/agent.h b/wolfssh/agent.h index 8f57bc5df..f2bad7fb2 100644 --- a/wolfssh/agent.h +++ b/wolfssh/agent.h @@ -187,11 +187,12 @@ WOLFSSH_API int wolfSSH_AGENT_enable(WOLFSSH* ssh, byte isEnabled); * this instead. Opens one channel, then flushes what of the open is queued. * Returns WS_SUCCESS, WS_BAD_ARGUMENT before the peer asks or on a client * session, WS_WANT_READ or WS_WANT_WRITE while output is still queued, + * WS_FATAL_ERROR with WS_DISCONNECT in ssh->error once the session is over, * WS_SSH_NULL_E, WS_MEMORY_E, or whatever the send reports. WS_SUCCESS says * the open went out, not that the peer took it; a refusal reaches the * channel-open-fail callback. - * Only the send records in ssh->error, so a poll ahead of the peer's request - * leaves the session fit for wolfSSH_accept(). */ + * Only that and the send record in ssh->error, so a poll ahead of the peer's + * request leaves the session fit for wolfSSH_accept(). */ WOLFSSH_API int wolfSSH_AGENT_ChannelOpen(WOLFSSH* ssh); WOLFSSH_LOCAL int wolfSSH_AGENT_worker(WOLFSSH* ssh); WOLFSSH_API int wolfSSH_AGENT_Relay(WOLFSSH* ssh, diff --git a/wolfssh/internal.h b/wolfssh/internal.h index a8001c5a6..dabc967b9 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1643,6 +1643,12 @@ enum ChannelOpenFailReasons { OPEN_RESOURCE_SHORTAGE }; +/* A disconnect, sent or received, ends the session, so nothing further may + * go out. RFC 4253 section 11.1. Returns 1 and records WS_DISCONNECT in + * ssh->error when the session is over, 0 otherwise. 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. */ +WOLFSSH_LOCAL int SendAfterDisconnect(WOLFSSH* ssh); WOLFSSH_LOCAL int DoReceive(WOLFSSH* ssh); WOLFSSH_LOCAL int DoProtoId(WOLFSSH* ssh); WOLFSSH_LOCAL int wolfSSH_SendPacket(WOLFSSH* ssh); From fa893cfd724a4ebe3f77171292134a8c8b0179a5 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2026 09:47:15 -0700 Subject: [PATCH 3/7] ssh: add opt-in application-driven channels A server that wants to own its channels had no way to get them: accept() ran the session state machine to the end, and a shell, exec or subsystem request with no callback registered was granted regardless. - add wolfSSH_CTX_SetAppChannels() and wolfSSH_SetAppChannels(), off by default, a byte on the context copied into the session - on, accept() returns once the user is authenticated, and a session request with no callback behind it is refused: nothing is left to serve - keep the stop state out of the pending-send advance, so a re-entry with queued output cannot step over where this call is meant to stop - stop early only while the session is short of that state, so turning the mode on afterward cannot leave the loop hunting a state it went past - teach wolfSSH_SFTP_accept() that the mode parks accept() short of an established session, so it stops redoing the handshake on every poll --- src/internal.c | 12 ++++++++++- src/ssh.c | 54 +++++++++++++++++++++++++++++++++++++++++++--- src/wolfsftp.c | 8 +++++-- wolfssh/internal.h | 2 ++ wolfssh/ssh.h | 23 ++++++++++++++++++++ 5 files changed, 93 insertions(+), 6 deletions(-) diff --git a/src/internal.c b/src/internal.c index 182c43df4..3b5b453ff 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1672,6 +1672,7 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx) ssh->highwaterMark = ctx->highwaterMark; ssh->msgHighwaterMark = ctx->msgHighwaterMark; ssh->maxAuthAttempts = ctx->maxAuthAttempts; + ssh->appChannels = ctx->appChannels; ssh->highwaterCtx = (void*)ssh; ssh->reqSuccessCtx = (void*)ssh; ssh->fs = NULL; @@ -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")) { @@ -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); @@ -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); @@ -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)); diff --git a/src/ssh.c b/src/ssh.c index 36646514c..5aab671ef 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -575,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) @@ -594,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) { @@ -605,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++; } @@ -627,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: @@ -717,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: @@ -3921,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; } @@ -4915,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; diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 88cca98f8..1b7d93cf1 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -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"); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index dabc967b9..53892e3ec 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -867,6 +867,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 */ @@ -1136,6 +1137,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 */ diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 631669b74..ea6d4ef60 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -452,6 +452,29 @@ 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 once accept() has established + * the session has no effect on that session. + * + * The mode drives the session channels itself, so it does not combine with + * the built-in wolfSSH_SFTP_accept() and WS_SCP_INIT entry points; an + * application using those leaves this off. */ +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); From fa43c783bdf87d0831cf568f56df5185822cb0b6 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2026 09:49:31 -0700 Subject: [PATCH 4/7] tests: cover application-driven channels wolfSSH_SetAppChannels() changes where wolfSSH_accept() stops and what becomes of a session request with no callback behind it, so both modes are exercised. - regress.c drives a server with the pivot on, one with a shell callback and one without, and checks accept() stops at ACCEPT_SERVER_USERAUTH_SENT - regress.c pins the context setter, the session's inheritance of it, and that turning it on after accept() established the session still returns - unit.c checks DoChannelRequest() refuses a shell, exec and subsystem request with no callback once the pivot is on - the untouched AssertHandshakeSucceeds() is the regression gate for a server that registers nothing --- tests/regress.c | 178 ++++++++++++++++++++++++++++++++++++++++++++++++ tests/unit.c | 62 +++++++++++++++++ 2 files changed, 240 insertions(+) diff --git a/tests/regress.c b/tests/regress.c index 233d3b2bb..11a1cab97 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -1486,6 +1486,180 @@ 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); + + AssertFalse(result.clientSuccess); + 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. */ +static void TestAppChannelsLateEnableReturns(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + + 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); + + AssertIntEQ(wolfSSH_SetAppChannels(harness.server, 1), WS_SUCCESS); + AssertIntEQ(wolfSSH_accept(harness.server), WS_SUCCESS); + AssertIntEQ(harness.server->acceptState, + ACCEPT_CLIENT_SESSION_ESTABLISHED); + + FreeKexReplyHarness(&harness); +} + static void TestKexDhReplyRejectsRsaSha2_256SigNameDowngrade(void) { AssertHandshakeSucceeds("rsa-sha2-256", REGRESS_SERVER_KEY_PATH); @@ -13729,6 +13903,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 1007afcae..a136ac166 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -9245,6 +9245,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); From f359ec47f23393d04559ab9a1ba4e38fed139ef3 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 3 Sep 2026 22:20:29 -0700 Subject: [PATCH 5/7] ssh: correct what a late app-channels enable does DoChannelRequest() reads ssh->appChannels when the request arrives, so turning the mode on after accept() established the session still refuses an uncallbacked shell, exec or subsystem request from then on. Only accept()'s stopping point is pinned, by the guard around stopState. - say the flag reaches the requests that follow, and that what it cannot do is move where accept() returns - drive a shell request over the wire in both modes from the late-enable test, pinning the behaviour the header now describes --- tests/regress.c | 25 ++++++++++++++++++++++++- wolfssh/ssh.h | 5 +++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/tests/regress.c b/tests/regress.c index 11a1cab97..529dd5091 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -1637,11 +1637,21 @@ static void TestAppChannelsCtxInherits(void) } /* 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 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); @@ -1652,11 +1662,24 @@ static void TestAppChannelsLateEnableReturns(void) 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); } diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index ea6d4ef60..0ac6743e3 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -466,8 +466,9 @@ WOLFSSH_API void* wolfSSH_GetChannelReqCtx(WOLFSSH* ssh); * 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 once accept() has established - * the session has no effect on that session. + * 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. * * The mode drives the session channels itself, so it does not combine with * the built-in wolfSSH_SFTP_accept() and WS_SCP_INIT entry points; an From cf9a26b3a2cd74a6f6618229220bcd2c1b34ae9f Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2026 11:50:13 -0700 Subject: [PATCH 6/7] internal: commit a session only once accepted A shell, exec or subsystem request changes the channel only once the callback accepts it. The session type and command are set for the callback to read and put back if it refuses, and CLIENT_DONE follows acceptance alone, so wolfSSH_accept() no longer reports an established session, or starts SFTP, on a request it answered CHANNEL_FAILURE. - DoChannelRequestSession() carries the three arms, which differed only in the type and the callback consulted - unit.c drives a refused shell, exec and subsystem request through DoChannelRequest() and checks nothing was committed - regress.c runs a server whose shell callback refuses and checks accept() stays at ACCEPT_SERVER_CHANNEL_ACCEPT_SENT Issue: F-8852 --- src/internal.c | 94 +++++++++++++++++++++++++++--------------- tests/regress.c | 53 ++++++++++++++++++++++++ tests/unit.c | 107 ++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 213 insertions(+), 41 deletions(-) diff --git a/src/internal.c b/src/internal.c index 3b5b453ff..84cb5807f 100644 --- a/src/internal.c +++ b/src/internal.c @@ -12667,6 +12667,61 @@ static void SetTerminalSize(WOLFSSH* ssh, word32 widthChar, word32 heightRows, #endif /* WOLFSSH_TERM */ +/* Answers a shell, exec, or subsystem request. The session type, and the + * command for the two that carry one, are set for the callback to read and + * kept only if it accepts; a refused request leaves the channel as it was + * and the accept loop still waiting, so nothing serves a session the + * application turned down. Without a callback the request is accepted, + * unless the application drives its own channels. */ +static int DoChannelRequestSession(WOLFSSH* ssh, WOLFSSH_CHANNEL* channel, + byte sessionType, WS_CallbackChannelReq cb, + byte* buf, word32 len, word32* idx, int* rej) +{ + char* prevCommand = NULL; + byte prevType = channel->sessionType; + byte hasCommand = (sessionType != WOLFSSH_SESSION_SHELL); + int ret = WS_SUCCESS; + + if (hasCommand) { + prevCommand = channel->command; + channel->command = NULL; + ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL, + buf, len, idx); + if (ret == WS_SUCCESS) { + WLOG(WS_LOG_DEBUG, " command = %s", channel->command); + } + } + + if (ret == WS_SUCCESS) { + channel->sessionType = sessionType; + if (cb != NULL) { + *rej = cb(channel, ssh->channelReqCtx); + } + else { + *rej = ssh->appChannels; + } + } + + if (ret == WS_SUCCESS && !*rej) { + if (prevCommand != NULL) { + WFREE(prevCommand, ssh->ctx->heap, DYNTYPE_STRING); + } + ssh->clientState = CLIENT_DONE; + } + else { + if (hasCommand) { + if (channel->command != NULL) { + WFREE(channel->command, ssh->ctx->heap, DYNTYPE_STRING); + } + channel->command = prevCommand; + } + channel->sessionType = prevType; + } + + return ret; +} + + static int DoChannelRequest(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) { @@ -12722,42 +12777,17 @@ static int DoChannelRequest(WOLFSSH* ssh, WLOG(WS_LOG_DEBUG, " %s = %s", name, value); } else if (ChannelRequestIs(type, typeSz, "shell")) { - channel->sessionType = WOLFSSH_SESSION_SHELL; - if (ssh->ctx->channelReqShellCb) { - rej = ssh->ctx->channelReqShellCb(channel, ssh->channelReqCtx); - } - else { - rej = ssh->appChannels; - } - ssh->clientState = CLIENT_DONE; + ret = DoChannelRequestSession(ssh, channel, WOLFSSH_SESSION_SHELL, + ssh->ctx->channelReqShellCb, buf, len, &begin, &rej); } else if (ChannelRequestIs(type, typeSz, "exec")) { - ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL, - buf, len, &begin); - channel->sessionType = WOLFSSH_SESSION_EXEC; - if (ssh->ctx->channelReqExecCb) { - rej = ssh->ctx->channelReqExecCb(channel, ssh->channelReqCtx); - } - else { - rej = ssh->appChannels; - } - ssh->clientState = CLIENT_DONE; - - WLOG(WS_LOG_DEBUG, " command = %s", channel->command); + ret = DoChannelRequestSession(ssh, channel, WOLFSSH_SESSION_EXEC, + ssh->ctx->channelReqExecCb, buf, len, &begin, &rej); } else if (ChannelRequestIs(type, typeSz, "subsystem")) { - ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL, - buf, len, &begin); - channel->sessionType = WOLFSSH_SESSION_SUBSYSTEM; - if (ssh->ctx->channelReqSubsysCb) { - rej = ssh->ctx->channelReqSubsysCb(channel, ssh->channelReqCtx); - } - else { - rej = ssh->appChannels; - } - ssh->clientState = CLIENT_DONE; - - WLOG(WS_LOG_DEBUG, " subsystem = %s", channel->command); + ret = DoChannelRequestSession(ssh, channel, + WOLFSSH_SESSION_SUBSYSTEM, ssh->ctx->channelReqSubsysCb, + buf, len, &begin, &rej); } #ifdef WOLFSSH_TERM else if (ChannelRequestIs(type, typeSz, "pty-req")) { diff --git a/tests/regress.c b/tests/regress.c index 529dd5091..1d1de1ab6 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -1683,6 +1683,58 @@ static void TestAppChannelsLateEnableReturns(void) FreeKexReplyHarness(&harness); } +/* Refuses the session request, and records what the channel showed. */ +static int rejectShellReqCalls; +static WS_SessionType rejectShellReqType; + +static int RejectShellReqCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + (void)ctx; + rejectShellReqCalls++; + rejectShellReqType = wolfSSH_ChannelGetSessionType(channel); + return 1; +} + +/* A shell request the callback refuses gets CHANNEL_FAILURE and nothing + * more: the channel keeps no session type, and accept() stays where it was, + * waiting on a request it can grant, rather than reporting an established + * session it just refused. */ +static void TestSessionReqRejectedKeepsAcceptWaiting(void) +{ + KexReplyHarness harness; + KexReplyRunResult result; + WOLFSSH_CHANNEL* channel; + WS_SessionType sessionType; + + rejectShellReqCalls = 0; + rejectShellReqType = WOLFSSH_SESSION_UNKNOWN; + + InitKexReplyHarness(&harness, "rsa-sha2-256", REGRESS_SERVER_KEY_PATH, + 0, NULL); + AssertIntEQ(wolfSSH_CTX_SetChannelReqShellCb(harness.serverCtx, + RejectShellReqCb), WS_SUCCESS); + + RunKexReplyHandshake(&harness, &result); + + AssertIntEQ(rejectShellReqCalls, 1); + AssertIntEQ(rejectShellReqType, WOLFSSH_SESSION_SHELL); + AssertFalse(result.clientSuccess); + AssertIntEQ(result.clientErr, WS_CHANOPEN_FAILED); + AssertFalse(result.serverSuccess); + AssertIntEQ(harness.server->acceptState, + ACCEPT_SERVER_CHANNEL_ACCEPT_SENT); + AssertTrue(harness.server->clientState < CLIENT_DONE); + sessionType = wolfSSH_GetSessionType(harness.server); + AssertIntEQ(sessionType, WOLFSSH_SESSION_UNKNOWN); + channel = wolfSSH_ChannelNext(harness.server, NULL); + AssertNotNull(channel); + AssertIntEQ(channel->sessionType, WOLFSSH_SESSION_UNKNOWN); + AssertFalse(harness.clientIo.sawDisconnect); + AssertFalse(harness.serverIo.sawDisconnect); + + FreeKexReplyHarness(&harness); +} + static void TestKexDhReplyRejectsRsaSha2_256SigNameDowngrade(void) { AssertHandshakeSucceeds("rsa-sha2-256", REGRESS_SERVER_KEY_PATH); @@ -13930,6 +13982,7 @@ int main(int argc, char** argv) TestAppChannelsAcceptStopsAtUserAuth(); TestAppChannelsNoShellCbRejects(); TestAppChannelsLateEnableReturns(); + TestSessionReqRejectedKeepsAcceptWaiting(); TestKexDhReplyRejectsRsaSha2_256SigNameDowngrade(); #endif #ifndef WOLFSSH_NO_RSA_SHA2_512 diff --git a/tests/unit.c b/tests/unit.c index a136ac166..5e905b54d 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -8584,6 +8584,23 @@ static int CaptureMsgId(const byte* buf, word32 len) * A custom IoSend callback captures the outgoing packet in plaintext * (no cipher negotiated on a fresh session). Message ID is read via * CaptureMsgId() using LENGTH_SZ + PAD_LENGTH_SZ. */ +/* A session request callback that refuses everything, and counts. The + * callback sees the session type and command of the request it is vetting; + * what it does not see is a session already committed to the channel. */ +static int s_rejectChanReqCalls; + +static int RejectChanReqCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + (void)ctx; + s_rejectChanReqCalls++; + if (channel == NULL + || wolfSSH_ChannelGetSessionType(channel) + == WOLFSSH_SESSION_UNKNOWN) { + return 0; + } + return 1; +} + static byte s_chanReqCapture[256]; static word32 s_chanReqCaptureSz = 0; @@ -8836,6 +8853,15 @@ static int test_DoChannelRequest(void) 0x00,0x00,0x00,0x02, /* cmdSz = 2 */ 0x6C,0x73 /* "ls" */ }; + 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" */ + }; static const byte payUnknown[] = { 0x00,0x00,0x00,0x00, /* channelId = 0 */ 0x00,0x00,0x00,0x0C, /* typeSz = 12 */ @@ -8962,6 +8988,78 @@ static int test_DoChannelRequest(void) } } + /* A callback that refuses a shell, exec or subsystem request must leave + * nothing behind: no session type or command on the channel, and the + * client state short of CLIENT_DONE, or wolfSSH_accept() would go on to + * serve the session it just refused. */ + { + struct { + const char* label; + const byte* payload; + word32 payloadSz; + int errBase; + } rejCases[] = { + { "shell", payShell, (word32)sizeof(payShell), -520 }, + { "exec", payExec, (word32)sizeof(payExec), -525 }, + { "subsystem", paySubsys, (word32)sizeof(paySubsys), -530 } + }; + int r; + + wolfSSH_CTX_SetChannelReqShellCb(ctx, RejectChanReqCb); + wolfSSH_CTX_SetChannelReqExecCb(ctx, RejectChanReqCb); + wolfSSH_CTX_SetChannelReqSubsysCb(ctx, RejectChanReqCb); + + for (r = 0; r < (int)(sizeof(rejCases) / sizeof(rejCases[0])); r++) { + word32 idxRej = 0; + int retRej, capMsgId; + + s_chanReqCaptureSz = 0; + WMEMSET(s_chanReqCapture, 0, sizeof(s_chanReqCapture)); + s_rejectChanReqCalls = 0; + + retRej = wolfSSH_TestDoChannelRequest(ssh, + (byte*)rejCases[r].payload, rejCases[r].payloadSz, + &idxRej); + if (retRej != WS_SUCCESS) { + printf("DoChannelRequest[rej-%s]: ret=%d, expected=%d\n", + rejCases[r].label, retRej, WS_SUCCESS); + result = rejCases[r].errBase; + goto done; + } + if (s_rejectChanReqCalls != 1) { + printf("DoChannelRequest[rej-%s]: callback ran %d times\n", + rejCases[r].label, s_rejectChanReqCalls); + result = rejCases[r].errBase - 1; + goto done; + } + capMsgId = CaptureMsgId(s_chanReqCapture, s_chanReqCaptureSz); + if (capMsgId != (int)MSGID_CHANNEL_FAILURE) { + printf("DoChannelRequest[rej-%s]: msg_id=0x%02x, " + "expected=0x%02x\n", rejCases[r].label, capMsgId, + MSGID_CHANNEL_FAILURE); + result = rejCases[r].errBase - 2; + goto done; + } + if (ch->sessionType != WOLFSSH_SESSION_UNKNOWN + || ch->command != NULL) { + printf("DoChannelRequest[rej-%s]: session committed\n", + rejCases[r].label); + result = rejCases[r].errBase - 3; + goto done; + } + if (ssh->clientState == CLIENT_DONE) { + printf("DoChannelRequest[rej-%s]: client state changed\n", + rejCases[r].label); + result = rejCases[r].errBase - 4; + goto done; + } + } + + wolfSSH_CTX_SetChannelReqShellCb(ctx, NULL); + wolfSSH_CTX_SetChannelReqExecCb(ctx, NULL); + wolfSSH_CTX_SetChannelReqSubsysCb(ctx, NULL); + } + for (i = 0; i < (int)(sizeof(cases) / sizeof(cases[0])); i++) { word32 idx = 0; int ret; @@ -9249,15 +9347,6 @@ static int test_DoChannelRequest(void) * 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; From 7bfe29893be7362e8a4b685d3a4458463940cf23 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2026 12:01:19 -0700 Subject: [PATCH 7/7] wolfsshd: refuse sessions it cannot serve A shell, exec or subsystem request is answered as it arrives, through the channel request callbacks, so a session this build cannot serve is refused with CHANNEL_FAILURE rather than accepted and then dropped once the session is up. What the daemon serves is unchanged. - SessionRequestCb() takes a shell with WOLFSSH_SHELL, an exec with WOLFSSH_SHELL or an scp command with WOLFSSH_SCP, and the sftp subsystem with WOLFSSH_SFTP; anything else is refused and logged - a request whose command did not fit is refused rather than read through a NULL - sshd_bad_subsystem_test.sh asks for an unknown subsystem with the OpenSSH client and expects the refusal --- apps/wolfsshd/test/run_all_sshd_tests.sh | 1 + apps/wolfsshd/test/sshd_bad_subsystem_test.sh | 68 ++++++++++++++++ apps/wolfsshd/wolfsshd.c | 78 +++++++++++++++++++ 3 files changed, 147 insertions(+) create mode 100755 apps/wolfsshd/test/sshd_bad_subsystem_test.sh diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index d59d69941..59b5aa5cd 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -9,6 +9,7 @@ test_cases=( "sshd_large_sftp_test.sh" "sshd_bad_sftp_test.sh" "sshd_sftp_idle_cpu_test.sh" + "sshd_bad_subsystem_test.sh" "sshd_scp_fail.sh" "sshd_term_close_test.sh" "sshd_stdin_eof_test.sh" diff --git a/apps/wolfsshd/test/sshd_bad_subsystem_test.sh b/apps/wolfsshd/test/sshd_bad_subsystem_test.sh new file mode 100755 index 000000000..b3b9c42cd --- /dev/null +++ b/apps/wolfsshd/test/sshd_bad_subsystem_test.sh @@ -0,0 +1,68 @@ +#!/bin/sh + +# sshd local test: a subsystem the daemon does not serve is refused at the +# request, so the client sees CHANNEL_FAILURE rather than a session that +# is accepted and then dropped. Uses the system OpenSSH client, since the +# in-tree clients only ask for sftp. + +# Not named PWD: the shell rewrites that variable on every cd, so a saved +# copy would not survive the cd to the repository root below. +TESTDIR=`pwd` +cd ../../.. + +USER=`whoami` +PRIVATE_KEY="./keys/hansel-key-ecc.pem" + +if [ -z "$1" ] || [ -z "$2" ]; then + echo "expecting host and port as arguments" + echo "./sshd_bad_subsystem_test.sh 127.0.0.1 22222" + exit 1 +fi + +if ! command -v ssh >/dev/null 2>&1; then + echo "OpenSSH client not found, skipping" + exit 77 +fi + +# OpenSSH refuses a key file other users can read. +KEY=`mktemp` +cat "$PRIVATE_KEY" > "$KEY" +chmod 600 "$KEY" +OUT=`mktemp` + +ssh_to_sshd() { + ssh -p "$2" -i "$KEY" -o IdentitiesOnly=yes -o StrictHostKeyChecking=no \ + -o UserKnownHostsFile=/dev/null -o PreferredAuthentications=publickey \ + -o BatchMode=yes -o ConnectTimeout=5 "$USER@$1" "$3" "$4" +} + +# Control: the same client and key can run a command. +ssh_to_sshd "$1" "$2" "echo ok" > "$OUT" 2>&1 +RESULT=$? +if [ "$RESULT" != "0" ] || ! grep -q "^ok" "$OUT"; then + echo "Control exec through OpenSSH failed ($RESULT):" + cat "$OUT" + rm -f "$KEY" "$OUT" + exit 1 +fi + +# A subsystem nothing serves: the client reports the refusal and exits +# non-zero. +ssh_to_sshd "$1" "$2" -s no-such-subsystem > "$OUT" 2>&1 +RESULT=$? +if [ "$RESULT" = "0" ]; then + echo "Expecting the unknown subsystem request to fail" + cat "$OUT" + rm -f "$KEY" "$OUT" + exit 1 +fi +if ! grep -q "subsystem request failed" "$OUT"; then + echo "Expecting the client to report the refused subsystem request:" + cat "$OUT" + rm -f "$KEY" "$OUT" + exit 1 +fi + +rm -f "$KEY" "$OUT" +cd "$TESTDIR" +exit 0 diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 6ab963265..9e5e1ee77 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -359,6 +359,80 @@ static void CleanupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, (void)conf; } +/* Answers a shell, exec or subsystem request as it arrives: a session this + * build cannot serve is refused with CHANNEL_FAILURE, rather than accepted + * and then dropped once the session is up. Returns 0 to accept and 1 to + * refuse. The command is NULL when the request carried none that fit. */ +static int SessionRequestCb(WOLFSSH_CHANNEL* channel, void* vCtx) +{ + WOLFSSHD_CONNECTION* conn = (WOLFSSHD_CONNECTION*)vCtx; + const char* cmd; + const char* reason = NULL; + int rej = 1; + + if (conn == NULL || channel == NULL) { + return 1; + } + + cmd = wolfSSH_ChannelGetSessionCommand(channel); + switch (wolfSSH_ChannelGetSessionType(channel)) { + case WOLFSSH_SESSION_SHELL: + #ifdef WOLFSSH_SHELL + rej = 0; + #else + reason = "shell support is disabled"; + #endif + break; + + case WOLFSSH_SESSION_EXEC: + if (cmd == NULL) { + reason = "exec request carried no command"; + break; + } + #ifdef WOLFSSH_SCP + if (WSTRNCMP(cmd, "scp", 3) == 0) { + rej = 0; + break; + } + #endif + #ifdef WOLFSSH_SHELL + rej = 0; + #else + reason = "exec support is disabled"; + #endif + break; + + case WOLFSSH_SESSION_SUBSYSTEM: + if (cmd == NULL) { + reason = "subsystem request carried no name"; + } + #ifdef WOLFSSH_SFTP + else if (WSTRCMP(cmd, "sftp") == 0) { + rej = 0; + } + #endif + else { + reason = "unknown or unsupported subsystem"; + } + break; + + case WOLFSSH_SESSION_UNKNOWN: + case WOLFSSH_SESSION_TERMINAL: + default: + reason = "unsupported session type"; + break; + } + + if (rej) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Refusing session request from %s: %s [%s]", + conn->ip, reason, cmd != NULL ? cmd : ""); + } + + return rej; +} + + /* Initializes and sets up the WOLFSSH_CTX struct based on the configure options * return WS_SUCCESS on success */ @@ -386,6 +460,9 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, if (ret == WS_SUCCESS) { wolfSSH_SetUserAuth(*ctx, DefaultUserAuth); wolfSSH_SetUserAuthResult(*ctx, UserAuthResult); + wolfSSH_CTX_SetChannelReqShellCb(*ctx, SessionRequestCb); + wolfSSH_CTX_SetChannelReqExecCb(*ctx, SessionRequestCb); + wolfSSH_CTX_SetChannelReqSubsysCb(*ctx, SessionRequestCb); } /* set banner to display on connection */ @@ -2545,6 +2622,7 @@ static void* HandleConnection(void* arg) /* let UserAuthResult reach this connection to cancel the grace timer * and to reach conn->auth for the cert force-command */ wolfSSH_SetUserAuthResultCtx(ssh, conn); + wolfSSH_SetChannelReqCtx(ssh, conn); #if defined(WOLFSSH_OSSH_CERTS) && !defined(_WIN32) /* Unix-only: each connection is a forked child with its own copy of the * auth struct. Windows does not enforce OpenSSH certs. */