Skip to content

Commit 00448b1

Browse files
committed
agent: refuse a channel open after a disconnect
wolfSSH_AGENT_ChannelOpen() answers a poll on a session that is over with WS_DISCONNECT: 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
1 parent 670ee14 commit 00448b1

5 files changed

Lines changed: 63 additions & 13 deletions

File tree

src/agent.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1746,6 +1746,12 @@ int wolfSSH_AGENT_ChannelOpen(WOLFSSH* ssh)
17461746

17471747
if (ssh == NULL)
17481748
ret = WS_SSH_NULL_E;
1749+
else if (SendAfterDisconnect(ssh)) {
1750+
/* The session is over, so neither a new open nor the flush of one
1751+
* queued before the disconnect may go out. RFC 4253 section 11.1.
1752+
* SendAfterDisconnect() has already recorded WS_DISCONNECT. */
1753+
ret = WS_DISCONNECT;
1754+
}
17491755
else if (!ssh->useAgent) {
17501756
/* Nothing asked for agent forwarding on this session. */
17511757
ret = WS_BAD_ARGUMENT;

src/ssh.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -567,10 +567,6 @@ static int DoReceiveHandshake(WOLFSSH* ssh)
567567
#endif /* !NO_WOLFSSH_SERVER || !NO_WOLFSSH_CLIENT */
568568

569569

570-
/* Defined below, ahead of both drivers; either can be the only one built. */
571-
static int SendAfterDisconnect(WOLFSSH* ssh);
572-
573-
574570
#ifndef NO_WOLFSSH_SERVER
575571

576572
const char acceptError[] = "accept error: %s, %d";
@@ -1094,11 +1090,8 @@ int wolfSSH_connect(WOLFSSH* ssh)
10941090
#endif /* NO_WOLFSSH_CLIENT */
10951091

10961092

1097-
/* A disconnect, sent or received, ends the session, so nothing further may
1098-
* go out. RFC 4253 section 11.1. Reads are deliberately not gated on this:
1099-
* channel data that arrived before the disconnect is still the caller's.
1100-
* Call only after ssh has been checked for NULL. */
1101-
static int SendAfterDisconnect(WOLFSSH* ssh)
1093+
/* See wolfssh/internal.h for the contract. */
1094+
int SendAfterDisconnect(WOLFSSH* ssh)
11021095
{
11031096
if (ssh->disconnected) {
11041097
WLOG(WS_LOG_DEBUG, "Send attempted after a disconnect");

tests/regress.c

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4459,6 +4459,48 @@ static void TestAgentChannelOpenWithoutRequest(void)
44594459
FreeChannelOpenHarness(&harness);
44604460
}
44614461

4462+
/* A poll after the peer disconnects must not open a channel or put anything
4463+
* on the wire. RFC 4253 section 11.1: the session is over. */
4464+
static void TestAgentChannelOpenAfterDisconnect(void)
4465+
{
4466+
ChannelOpenHarness harness;
4467+
4468+
InitChannelOpenHarness(&harness, NULL, 0);
4469+
harness.ssh->useAgent = 1;
4470+
harness.ssh->disconnected = 1;
4471+
4472+
AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_DISCONNECT);
4473+
AssertNull(harness.ssh->agent);
4474+
AssertIntEQ(harness.ssh->channelListSz, 0);
4475+
AssertIntEQ(harness.io.outSz, 0);
4476+
AssertIntEQ(harness.ssh->error, WS_DISCONNECT);
4477+
4478+
FreeChannelOpenHarness(&harness);
4479+
}
4480+
4481+
/* An open queued before the disconnect is not flushed either: those bytes
4482+
* belong to a session that is over, the same rule wolfSSH_shutdown() applies
4483+
* to everything but its own queued disconnect. */
4484+
static void TestAgentChannelOpenQueuedThenDisconnect(void)
4485+
{
4486+
ChannelOpenHarness harness;
4487+
4488+
InitChannelOpenHarness(&harness, NULL, 0);
4489+
harness.ssh->useAgent = 1;
4490+
harness.io.blockNext = 1;
4491+
4492+
AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_WANT_WRITE);
4493+
AssertIntEQ(harness.io.outSz, 0);
4494+
4495+
harness.ssh->disconnected = 1;
4496+
4497+
AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_DISCONNECT);
4498+
AssertIntEQ(harness.io.outSz, 0);
4499+
AssertIntEQ(harness.ssh->error, WS_DISCONNECT);
4500+
4501+
FreeChannelOpenHarness(&harness);
4502+
}
4503+
44624504
/* A queued open publishes the agent, so the caller's next poll must finish
44634505
* the send rather than report a success the peer never saw, and must not
44644506
* open a second channel. */
@@ -13475,6 +13517,8 @@ int main(int argc, char** argv)
1347513517
TestAgentChannelNullAgentSendsOpenFail();
1347613518
TestAgentChannelOpenWithoutRequest();
1347713519
TestAgentChannelOpenFlushesQueuedOpen();
13520+
TestAgentChannelOpenAfterDisconnect();
13521+
TestAgentChannelOpenQueuedThenDisconnect();
1347813522
#endif
1347913523
#endif /* NO_WOLFSSH_SERVER */
1348013524
#if defined(WOLFSSH_AGENT) && !defined(WOLFSSH_NO_ED25519) \

wolfssh/agent.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,11 @@ WOLFSSH_API int wolfSSH_AGENT_enable(WOLFSSH* ssh, byte isEnabled);
186186
* does it on the default path; an application driving its own channels polls
187187
* this instead. Opens one channel, then flushes what of the open is queued.
188188
* Returns WS_SUCCESS, WS_BAD_ARGUMENT before the peer asks, WS_WANT_READ or
189-
* WS_WANT_WRITE with the open queued, WS_SSH_NULL_E, WS_MEMORY_E, or whatever
190-
* else the send of the open reports. Only a call that reached the transport
191-
* records its result in ssh->error, so a poll ahead of the peer's request
192-
* leaves the session fit for wolfSSH_accept(). */
189+
* WS_WANT_WRITE with the open queued, WS_DISCONNECT once the session is over,
190+
* WS_SSH_NULL_E, WS_MEMORY_E, or whatever else the send of the open reports.
191+
* Only a call that reached the transport records its result in ssh->error, so
192+
* a poll ahead of the peer's request leaves the session fit for
193+
* wolfSSH_accept(). */
193194
WOLFSSH_API int wolfSSH_AGENT_ChannelOpen(WOLFSSH* ssh);
194195
WOLFSSH_LOCAL int wolfSSH_AGENT_worker(WOLFSSH* ssh);
195196
WOLFSSH_API int wolfSSH_AGENT_Relay(WOLFSSH* ssh,

wolfssh/internal.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,6 +1643,12 @@ enum ChannelOpenFailReasons {
16431643
OPEN_RESOURCE_SHORTAGE
16441644
};
16451645

1646+
/* A disconnect, sent or received, ends the session, so nothing further may
1647+
* go out. RFC 4253 section 11.1. Returns 1 and records WS_DISCONNECT in
1648+
* ssh->error when the session is over, 0 otherwise. Reads are deliberately
1649+
* not gated on this: channel data that arrived before the disconnect is
1650+
* still the caller's. Call only after ssh has been checked for NULL. */
1651+
WOLFSSH_LOCAL int SendAfterDisconnect(WOLFSSH* ssh);
16461652
WOLFSSH_LOCAL int DoReceive(WOLFSSH* ssh);
16471653
WOLFSSH_LOCAL int DoProtoId(WOLFSSH* ssh);
16481654
WOLFSSH_LOCAL int wolfSSH_SendPacket(WOLFSSH* ssh);

0 commit comments

Comments
 (0)