Skip to content

Commit 127b742

Browse files
committed
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
1 parent 162dfb8 commit 127b742

2 files changed

Lines changed: 27 additions & 3 deletions

File tree

tests/regress.c

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1637,11 +1637,21 @@ static void TestAppChannelsCtxInherits(void)
16371637
}
16381638

16391639
/* Turning the mode on after accept() established the session must not leave
1640-
* the accept loop hunting for a state it has already stepped past. */
1640+
* the accept loop hunting for a state it has already stepped past. The flag
1641+
* still reaches DoChannelRequest() from there, which is what ssh.h promises,
1642+
* so pin both halves: accept() stays put, the requests that follow flip. */
16411643
static void TestAppChannelsLateEnableReturns(void)
16421644
{
16431645
KexReplyHarness harness;
16441646
KexReplyRunResult result;
1647+
/* SSH_MSG_CHANNEL_REQUEST body: channel 0, "shell", wantReply. */
1648+
static byte payShell[] = {
1649+
0x00,0x00,0x00,0x00, /* channelId = 0 */
1650+
0x00,0x00,0x00,0x05, /* typeSz = 5 */
1651+
0x73,0x68,0x65,0x6C,0x6C, /* "shell" */
1652+
0x01 /* wantReply = 1 */
1653+
};
1654+
word32 idx;
16451655

16461656
InitKexReplyHarness(&harness, "rsa-sha2-256", REGRESS_SERVER_KEY_PATH,
16471657
0, NULL);
@@ -1652,11 +1662,24 @@ static void TestAppChannelsLateEnableReturns(void)
16521662
AssertIntEQ(harness.server->acceptState,
16531663
ACCEPT_CLIENT_SESSION_ESTABLISHED);
16541664

1665+
/* Default mode, no callback registered: the request is granted. */
1666+
idx = 0;
1667+
AssertIntEQ(wolfSSH_TestDoChannelRequest(harness.server, payShell,
1668+
(word32)sizeof(payShell), &idx), WS_SUCCESS);
1669+
AssertIntEQ(wolfSSH_worker(harness.client, NULL), WS_SUCCESS);
1670+
16551671
AssertIntEQ(wolfSSH_SetAppChannels(harness.server, 1), WS_SUCCESS);
16561672
AssertIntEQ(wolfSSH_accept(harness.server), WS_SUCCESS);
16571673
AssertIntEQ(harness.server->acceptState,
16581674
ACCEPT_CLIENT_SESSION_ESTABLISHED);
16591675

1676+
/* Same request, same session, mode now on: refused instead. */
1677+
idx = 0;
1678+
AssertIntEQ(wolfSSH_TestDoChannelRequest(harness.server, payShell,
1679+
(word32)sizeof(payShell), &idx), WS_SUCCESS);
1680+
AssertTrue(wolfSSH_worker(harness.client, NULL) < WS_SUCCESS);
1681+
AssertIntEQ(wolfSSH_get_error(harness.client), WS_CHANOPEN_FAILED);
1682+
16601683
FreeKexReplyHarness(&harness);
16611684
}
16621685

wolfssh/ssh.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -466,8 +466,9 @@ WOLFSSH_API void* wolfSSH_GetChannelReqCtx(WOLFSSH* ssh);
466466
* accept() already returned, nothing is left to service it.
467467
*
468468
* Set it on the context before wolfSSH_new(), or on a session before the
469-
* first wolfSSH_accept() call. Turning it on once accept() has established
470-
* the session has no effect on that session.
469+
* first wolfSSH_accept() call. Turning it on later still applies to the
470+
* channel requests that follow, but it cannot move where accept() returns
471+
* on a session that has already gone past the user-auth stop.
471472
*
472473
* The mode drives the session channels itself, so it does not combine with
473474
* the built-in wolfSSH_SFTP_accept() and WS_SCP_INIT entry points; an

0 commit comments

Comments
 (0)