From 9a90a873ed96fb25c9829fce8c8fe1a0478ef539 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2026 10:39:47 -0700 Subject: [PATCH 1/3] tests: cover the channel open response callbacks The confirmation and failure hooks reach an application only through DoChannelOpenConf() and DoChannelOpenFail(), and nothing in tests/ or apps/ registered either, so both arms shipped unexercised. - assert the confirm callback runs with the peer's channel id, window and packet size already recorded - assert the failure callback runs while the channel is still findable, before DoChannelOpenFail() removes it - count each hook separately, so a test can tell which one ran - a rejecting confirm hook fails the receive and leaves the open unfinished; a rejecting failure hook fails it and leaves the channel on the list - seed the unconfirmed channel through ChannelNew() and ChannelAppend(), the state an outstanding open leaves behind --- tests/regress.c | 250 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 250 insertions(+) diff --git a/tests/regress.c b/tests/regress.c index b1209675b..6c004fc33 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -3006,6 +3006,252 @@ static void RepointHarnessInput(ChannelOpenHarness* harness, harness->io.outSz = 0; } +/* Builds a plaintext SSH_MSG_CHANNEL_OPEN_CONFIRMATION. */ +static word32 BuildChannelOpenConfPacket(word32 recipientChannelId, + word32 senderChannelId, word32 initialWindowSz, word32 maxPacketSz, + byte* out, word32 outSz) +{ + byte payload[32]; + word32 idx = 0; + + idx = AppendUint32(payload, sizeof(payload), idx, recipientChannelId); + idx = AppendUint32(payload, sizeof(payload), idx, senderChannelId); + idx = AppendUint32(payload, sizeof(payload), idx, initialWindowSz); + idx = AppendUint32(payload, sizeof(payload), idx, maxPacketSz); + + return WrapPacket(MSGID_CHANNEL_OPEN_CONF, payload, idx, out, outSz); +} + +/* Builds a plaintext SSH_MSG_CHANNEL_OPEN_FAILURE with an empty language + * tag, which is what every sender in the tree emits. */ +static word32 BuildChannelOpenFailPacket(word32 recipientChannelId, + word32 reasonId, const char* desc, byte* out, word32 outSz) +{ + byte payload[128]; + word32 idx = 0; + + idx = AppendUint32(payload, sizeof(payload), idx, recipientChannelId); + idx = AppendUint32(payload, sizeof(payload), idx, reasonId); + idx = AppendString(payload, sizeof(payload), idx, desc); + idx = AppendString(payload, sizeof(payload), idx, ""); + + return WrapPacket(MSGID_CHANNEL_OPEN_FAIL, payload, idx, out, outSz); +} + +/* What the open-response callbacks saw. File scope rather than reached + * through the callback ctx, so a zero reading means the hook did not run and + * cannot instead mean the ctx stopped being delivered. Each hook has its own + * count, so a test can tell which one ran. The harness's first channel has + * id 0, so the recorded id starts at a value no channel can have. */ +#define REGRESS_NO_CHANNEL ((word32)-1) +static int openConfCbCalls; +static int openFailCbCalls; +static word32 openRespCbChannel; +static void* openRespCbCtx; +static word32 openConfCbPeerChannel; +static word32 openConfCbPeerWindowSz; +static word32 openConfCbPeerMaxPacketSz; +static word32 openFailCbListSz; +static int openRespCbReturn; + +static int RecordingChannelOpenConfCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + AssertNotNull(channel); + openConfCbCalls++; + openRespCbChannel = channel->channel; + openRespCbCtx = ctx; + openConfCbPeerChannel = channel->peerChannel; + openConfCbPeerWindowSz = channel->peerWindowSz; + openConfCbPeerMaxPacketSz = channel->peerMaxPacketSz; + + return openRespCbReturn; +} + +static int RecordingChannelOpenFailCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + AssertNotNull(channel); + AssertNotNull(channel->ssh); + openFailCbCalls++; + openRespCbChannel = channel->channel; + openRespCbCtx = ctx; + openFailCbListSz = channel->ssh->channelListSz; + + return openRespCbReturn; +} + +/* Seeds an unconfirmed channel of our own, the state a channel is in while + * its open is outstanding and the only state the open responses apply to. */ +static WOLFSSH_CHANNEL* SeedUnconfirmedChannel(ChannelOpenHarness* harness) +{ + WOLFSSH_CHANNEL* channel; + + channel = ChannelNew(harness->ssh, ID_CHANTYPE_SESSION, 1024, 1024); + AssertNotNull(channel); + AssertIntEQ(ChannelAppend(harness->ssh, channel), WS_SUCCESS); + + return channel; +} + +/* Registers both open-response hooks, set to return cbReturn, and seeds the + * outstanding channel. */ +static WOLFSSH_CHANNEL* SeedOpenRespHarness(ChannelOpenHarness* harness, + void* cbCtx, int cbReturn) +{ + openConfCbCalls = 0; + openFailCbCalls = 0; + openRespCbChannel = REGRESS_NO_CHANNEL; + openRespCbCtx = NULL; + openConfCbPeerChannel = 0; + openConfCbPeerWindowSz = 0; + openConfCbPeerMaxPacketSz = 0; + openFailCbListSz = 0; + openRespCbReturn = cbReturn; + + InitChannelOpenHarness(harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelOpenRespCb(harness->ctx, + RecordingChannelOpenConfCb, RecordingChannelOpenFailCb), + WS_SUCCESS); + AssertIntEQ(wolfSSH_SetChannelOpenCtx(harness->ssh, cbCtx), WS_SUCCESS); + + return SeedUnconfirmedChannel(harness); +} + +/* The confirmation callback is the application's only notice that a channel + * it opened is usable, and it runs with the peer's parameters already + * recorded. */ +static void TestChannelOpenConfCallbackRuns(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[64]; + word32 inSz; + word32 selfChannelId; + int cbCtx = 0; + int ret; + + channel = SeedOpenRespHarness(&harness, &cbCtx, WS_SUCCESS); + selfChannelId = channel->channel; + + inSz = BuildChannelOpenConfPacket(selfChannelId, 7, 0x4000, 0x8000, + in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + ret = DoReceive(harness.ssh); + AssertIntEQ(ret, WS_SUCCESS); + AssertIntEQ(harness.io.inOff, harness.io.inSz); + AssertIntEQ(openConfCbCalls, 1); + AssertIntEQ(openFailCbCalls, 0); + AssertIntEQ(openRespCbChannel, selfChannelId); + AssertTrue(openRespCbCtx == &cbCtx); + + /* Read in the callback: the peer's numbers are already in place when it + * runs, not merely by the time DoReceive() returns. */ + AssertIntEQ(openConfCbPeerChannel, 7); + AssertIntEQ(openConfCbPeerWindowSz, 0x4000); + AssertIntEQ(openConfCbPeerMaxPacketSz, 0x8000); + + FreeChannelOpenHarness(&harness); +} + +/* The failure callback runs while the channel is still findable, since it is + * removed immediately afterward and the application would otherwise have no + * way to tell which open was refused. */ +static void TestChannelOpenFailCallbackRuns(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[128]; + word32 inSz; + word32 selfChannelId; + int cbCtx = 0; + int ret; + + channel = SeedOpenRespHarness(&harness, &cbCtx, WS_SUCCESS); + selfChannelId = channel->channel; + + inSz = BuildChannelOpenFailPacket(selfChannelId, + OPEN_ADMINISTRATIVELY_PROHIBITED, "no", in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + /* DoReceive() reports WS_CHANOPEN_FAILED through ssh->error; only the + * statuses it can resume from come back as themselves. */ + ret = DoReceive(harness.ssh); + AssertIntEQ(ret, WS_FATAL_ERROR); + AssertIntEQ(wolfSSH_get_error(harness.ssh), WS_CHANOPEN_FAILED); + AssertIntEQ(harness.io.inOff, harness.io.inSz); + AssertIntEQ(openFailCbCalls, 1); + AssertIntEQ(openConfCbCalls, 0); + AssertIntEQ(openRespCbChannel, selfChannelId); + AssertTrue(openRespCbCtx == &cbCtx); + /* Still on the list when the callback ran, gone by the time the caller + * is told. */ + AssertIntEQ(openFailCbListSz, 1); + AssertIntEQ(harness.ssh->channelListSz, 0); + + FreeChannelOpenHarness(&harness); +} + +/* A confirm callback that returns an error fails the receive with that + * error, and the open is not marked done: the session state and default + * peer channel stay as they were. */ +static void TestChannelOpenConfCallbackRejects(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[64]; + word32 inSz; + word32 defaultPeerChannelId; + int serverState; + int cbCtx = 0; + int ret; + + channel = SeedOpenRespHarness(&harness, &cbCtx, WS_BAD_ARGUMENT); + serverState = harness.ssh->serverState; + defaultPeerChannelId = harness.ssh->defaultPeerChannelId; + + inSz = BuildChannelOpenConfPacket(channel->channel, 7, 0x4000, 0x8000, + 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); + + 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); +} + /* A username change after the first userauth request must end the session. */ static void TestUsernameChangeDisconnects(void) { @@ -12204,6 +12450,10 @@ int main(int argc, char** argv) TestServerServiceRequestRejectedDuringKeying(); TestFailedSendClearsPendingPlaintext(); TestChannelOpenCallbackRejectSendsOpenFail(); + TestChannelOpenConfCallbackRuns(); + TestChannelOpenFailCallbackRuns(); + TestChannelOpenConfCallbackRejects(); + TestChannelOpenFailCallbackRejects(); TestSecondSessionChannelRejected(); TestUsernameChangeDisconnects(); TestSameUserRetryAllowed(); From 5f9752fc67edacab12f3e0972290563e3418ce08 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2026 10:39:48 -0700 Subject: [PATCH 2/3] tests: cover the channel close callback wolfSSH_CTX_SetChannelCloseCb() had no caller in tests/, examples/ or apps/, so nothing held DoChannelClose() to running the hook before it retires the channel. - assert the callback runs with the closing channel's id and the ctx set on the session - assert the channel is still on the list inside the callback and gone by the time the caller is told - a rejecting callback changes nothing: the return is discarded and the close completes --- tests/regress.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/tests/regress.c b/tests/regress.c index 6c004fc33..ce5702f22 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -3252,6 +3252,104 @@ static void TestChannelOpenFailCallbackRejects(void) 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); +} + /* A username change after the first userauth request must end the session. */ static void TestUsernameChangeDisconnects(void) { @@ -12454,6 +12552,8 @@ int main(int argc, char** argv) TestChannelOpenFailCallbackRuns(); TestChannelOpenConfCallbackRejects(); TestChannelOpenFailCallbackRejects(); + TestChannelCloseCallbackRuns(); + TestChannelCloseCallbackReturnIgnored(); TestSecondSessionChannelRejected(); TestUsernameChangeDisconnects(); TestSameUserRetryAllowed(); From 37573ce7f717f9e8dbb268bf3f9a9e3deb9f674f Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2026 10:48:49 -0700 Subject: [PATCH 3/3] tests: cover the exec and subsystem req callbacks Only the shell hook had coverage, so nothing held DoChannelRequest() to handing the command and session type to the exec and subsystem callbacks or to answering with what they return. - assert each callback sees the session type and the command string the request carried, and the ctx set on the session - assert an accepting callback draws CHANNEL_SUCCESS and a rejecting one CHANNEL_FAILURE, whether it rejects with a WS_ error or a bare nonzero --- tests/regress.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/tests/regress.c b/tests/regress.c index ce5702f22..9425ae85a 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -3350,6 +3350,127 @@ static void TestChannelCloseCallbackReturnIgnored(void) 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) +{ + 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); + 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); +} + /* A username change after the first userauth request must end the session. */ static void TestUsernameChangeDisconnects(void) { @@ -12554,6 +12675,8 @@ int main(int argc, char** argv) TestChannelOpenFailCallbackRejects(); TestChannelCloseCallbackRuns(); TestChannelCloseCallbackReturnIgnored(); + TestChannelReqExecCallbackRuns(); + TestChannelReqSubsysCallbackRuns(); TestSecondSessionChannelRejected(); TestUsernameChangeDisconnects(); TestSameUserRetryAllowed();