Skip to content

Commit 12b8f53

Browse files
committed
Block every send after a disconnect
The disconnect flag gated wolfSSH_stream_read() and wolfSSH_stream_send(), which is the client-side API. wolfsshd and echoserver drive their channels through the channel-id calls, so the daemon was never gated at all. - New SendAfterDisconnect() helper, used by the six send entry points: stream_send, stream_exit, ChannelIdSend, ChannelIdSendExt, extended_data_send and global_request. - Reads stay open, since data that arrived before the disconnect is still the caller's. wolfSSH_stream_read() drains its buffer and reports WS_DISCONNECT only once it runs dry. - wolfSSH_worker() stays ungated; the shutdown paths still pump it. - ssh.h and internal.h describe the split. - regress.c: buffered data survives the disconnect, and every send call refuses without a byte leaving the session. Issue: F-8837
1 parent 76a7291 commit 12b8f53

4 files changed

Lines changed: 181 additions & 16 deletions

File tree

src/ssh.c

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,6 +1103,21 @@ int wolfSSH_connect(WOLFSSH* ssh)
11031103
#endif /* NO_WOLFSSH_CLIENT */
11041104

11051105

1106+
/* A disconnect, sent or received, ends the session, so nothing further may
1107+
* go out. RFC 4253 section 11.1. Reads are deliberately not gated on this:
1108+
* channel data that arrived before the disconnect is still the caller's.
1109+
* Call only after ssh has been checked for NULL. */
1110+
static int SendAfterDisconnect(WOLFSSH* ssh)
1111+
{
1112+
if (ssh->disconnected) {
1113+
WLOG(WS_LOG_DEBUG, "Send attempted after a disconnect");
1114+
ssh->error = WS_DISCONNECT;
1115+
return 1;
1116+
}
1117+
return 0;
1118+
}
1119+
1120+
11061121
int wolfSSH_shutdown(WOLFSSH* ssh)
11071122
{
11081123
int ret = WS_SUCCESS;
@@ -1231,13 +1246,14 @@ int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz)
12311246
if (ssh == NULL || buf == NULL || bufSz == 0)
12321247
return WS_BAD_ARGUMENT;
12331248

1234-
if (ssh->disconnected) {
1235-
ssh->error = WS_DISCONNECT;
1236-
return WS_FATAL_ERROR;
1237-
}
1238-
1239-
if (ssh->channelList == NULL)
1249+
if (ssh->channelList == NULL) {
1250+
/* No channel left to drain, so the disconnect is all there is. */
1251+
if (ssh->disconnected) {
1252+
ssh->error = WS_DISCONNECT;
1253+
return WS_FATAL_ERROR;
1254+
}
12401255
return WS_BAD_ARGUMENT;
1256+
}
12411257

12421258
if (ssh->channelList->eofRxd) {
12431259
ssh->error = WS_EOF;
@@ -1252,6 +1268,13 @@ int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz)
12521268
inputBuffer = &ssh->channelList->inputBuffer;
12531269
ssh->error = WS_SUCCESS;
12541270

1271+
/* Hand back whatever arrived before the disconnect, then report it once
1272+
* the buffer runs dry rather than going back to a dead transport. */
1273+
if (ssh->disconnected && inputBuffer->length - inputBuffer->idx == 0) {
1274+
ssh->error = WS_DISCONNECT;
1275+
return WS_FATAL_ERROR;
1276+
}
1277+
12551278
if (ret == WS_SUCCESS) {
12561279
WLOG(WS_LOG_DEBUG, " Stream read index of %u", inputBuffer->idx);
12571280
WLOG(WS_LOG_DEBUG, " Stream read ava data %u", inputBuffer->length);
@@ -1318,10 +1341,8 @@ int wolfSSH_stream_send(WOLFSSH* ssh, byte* buf, word32 bufSz)
13181341
if (ssh == NULL || buf == NULL)
13191342
return WS_BAD_ARGUMENT;
13201343

1321-
if (ssh->disconnected) {
1322-
ssh->error = WS_DISCONNECT;
1344+
if (SendAfterDisconnect(ssh))
13231345
return WS_FATAL_ERROR;
1324-
}
13251346

13261347
if (ssh->channelList == NULL)
13271348
return WS_BAD_ARGUMENT;
@@ -1350,6 +1371,9 @@ int wolfSSH_ChannelIdSend(WOLFSSH* ssh, word32 channelId,
13501371
if (ssh == NULL || buf == NULL)
13511372
ret = WS_BAD_ARGUMENT;
13521373

1374+
if (ret == WS_SUCCESS && SendAfterDisconnect(ssh))
1375+
ret = WS_FATAL_ERROR;
1376+
13531377
if (ret == WS_SUCCESS) {
13541378
channel = ChannelFind(ssh, channelId, WS_CHANNEL_ID_SELF);
13551379
if (channel == NULL) {
@@ -1386,6 +1410,9 @@ int wolfSSH_ChannelIdSendExt(WOLFSSH* ssh, word32 channelId,
13861410
if (ssh == NULL || buf == NULL)
13871411
ret = WS_BAD_ARGUMENT;
13881412

1413+
if (ret == WS_SUCCESS && SendAfterDisconnect(ssh))
1414+
ret = WS_FATAL_ERROR;
1415+
13891416
if (ret == WS_SUCCESS) {
13901417
channel = ChannelFind(ssh, channelId, WS_CHANNEL_ID_SELF);
13911418
if (channel == NULL) {
@@ -1419,6 +1446,9 @@ int wolfSSH_stream_exit(WOLFSSH* ssh, int status)
14191446
if (ssh == NULL || ssh->channelList == NULL)
14201447
ret = WS_BAD_ARGUMENT;
14211448

1449+
if (ret == WS_SUCCESS && SendAfterDisconnect(ssh))
1450+
ret = WS_FATAL_ERROR;
1451+
14221452
if (ret == WS_SUCCESS)
14231453
ret = SendChannelExit(ssh, ssh->channelList->peerChannel, status);
14241454

@@ -1442,6 +1472,8 @@ int wolfSSH_global_request(WOLFSSH *ssh, const unsigned char* data, word32 dataS
14421472
return WS_BAD_ARGUMENT;
14431473
if (reply != 0 && reply != 1)
14441474
return WS_BAD_ARGUMENT;
1475+
if (SendAfterDisconnect(ssh))
1476+
return WS_FATAL_ERROR;
14451477
return SendGlobalRequest(ssh, data, dataSz, reply);
14461478
}
14471479

@@ -1452,7 +1484,13 @@ int wolfSSH_extended_data_send(WOLFSSH* ssh, byte* buf, word32 bufSz)
14521484

14531485
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_extended_data_send()");
14541486

1455-
if (ssh == NULL || buf == NULL || ssh->channelList == NULL)
1487+
if (ssh == NULL || buf == NULL)
1488+
return WS_BAD_ARGUMENT;
1489+
1490+
if (SendAfterDisconnect(ssh))
1491+
return WS_FATAL_ERROR;
1492+
1493+
if (ssh->channelList == NULL)
14561494
return WS_BAD_ARGUMENT;
14571495

14581496
if (ssh->isKeying) {

tests/regress.c

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2679,6 +2679,128 @@ static void TestDisconnectTerminalWithChannel(void)
26792679
}
26802680

26812681

2682+
/* The disconnect stops sends, not reads. Channel data that arrived before
2683+
* it is still the caller's, and only once that runs dry does the read
2684+
* report the disconnect. */
2685+
static void TestDisconnectDrainsBufferedData(void)
2686+
{
2687+
WOLFSSH_CTX* ctx;
2688+
WOLFSSH* ssh;
2689+
MemIo io;
2690+
byte in[128];
2691+
byte out[128];
2692+
byte data[16];
2693+
byte payload[] = { 'h', 'e', 'l', 'l', 'o' };
2694+
word32 inSz;
2695+
int ret;
2696+
2697+
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
2698+
AssertNotNull(ctx);
2699+
2700+
wolfSSH_SetIORecv(ctx, MemRecv);
2701+
wolfSSH_SetIOSend(ctx, MemSend);
2702+
2703+
ssh = wolfSSH_new(ctx);
2704+
AssertNotNull(ssh);
2705+
AddSessionChannel(ssh);
2706+
2707+
AssertIntEQ(ChannelPutData(ssh->channelList, payload, sizeof(payload)),
2708+
WS_SUCCESS);
2709+
2710+
inSz = BuildDisconnectPacket(WOLFSSH_DISCONNECT_BY_APPLICATION,
2711+
in, sizeof(in));
2712+
MemIoInit(&io, in, inSz, out, sizeof(out));
2713+
wolfSSH_SetIOReadCtx(ssh, &io);
2714+
wolfSSH_SetIOWriteCtx(ssh, &io);
2715+
2716+
ret = DoReceive(ssh);
2717+
AssertIntEQ(ret, WS_FATAL_ERROR);
2718+
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
2719+
AssertTrue(ssh->disconnected);
2720+
2721+
WMEMSET(data, 0, sizeof(data));
2722+
ret = wolfSSH_stream_read(ssh, data, sizeof(data));
2723+
AssertIntEQ(ret, (int)sizeof(payload));
2724+
AssertIntEQ(WMEMCMP(data, payload, sizeof(payload)), 0);
2725+
2726+
/* Buffer is dry now, so the disconnect is what is left to report. */
2727+
ret = wolfSSH_stream_read(ssh, data, sizeof(data));
2728+
AssertIntEQ(ret, WS_FATAL_ERROR);
2729+
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
2730+
2731+
wolfSSH_free(ssh);
2732+
wolfSSH_CTX_free(ctx);
2733+
}
2734+
2735+
2736+
/* Every send entry point refuses after a disconnect, not just the stream
2737+
* calls. wolfsshd and echoserver drive their channels through the
2738+
* channel-id and extended-data calls and never touch wolfSSH_stream_send(). */
2739+
static void TestDisconnectBlocksEverySend(void)
2740+
{
2741+
WOLFSSH_CTX* ctx;
2742+
WOLFSSH* ssh;
2743+
MemIo io;
2744+
byte out[256];
2745+
byte data[8];
2746+
word32 quietSz;
2747+
word32 channelId;
2748+
int ret;
2749+
2750+
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
2751+
AssertNotNull(ctx);
2752+
2753+
wolfSSH_SetIORecv(ctx, MemRecv);
2754+
wolfSSH_SetIOSend(ctx, MemSend);
2755+
2756+
ssh = wolfSSH_new(ctx);
2757+
AssertNotNull(ssh);
2758+
AddSessionChannel(ssh);
2759+
channelId = ssh->channelList->channel;
2760+
2761+
MemIoInit(&io, NULL, 0, out, sizeof(out));
2762+
wolfSSH_SetIOReadCtx(ssh, &io);
2763+
wolfSSH_SetIOWriteCtx(ssh, &io);
2764+
2765+
AssertIntEQ(wolfSSH_SendDisconnect(ssh, WOLFSSH_DISCONNECT_BY_APPLICATION),
2766+
WS_SUCCESS);
2767+
AssertTrue(ssh->disconnected);
2768+
quietSz = io.outSz;
2769+
2770+
WMEMSET(data, 0, sizeof(data));
2771+
2772+
ret = wolfSSH_stream_send(ssh, data, sizeof(data));
2773+
AssertIntEQ(ret, WS_FATAL_ERROR);
2774+
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
2775+
2776+
ret = wolfSSH_ChannelIdSend(ssh, channelId, data, sizeof(data));
2777+
AssertIntEQ(ret, WS_FATAL_ERROR);
2778+
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
2779+
2780+
ret = wolfSSH_ChannelIdSendExt(ssh, channelId, data, sizeof(data));
2781+
AssertIntEQ(ret, WS_FATAL_ERROR);
2782+
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
2783+
2784+
ret = wolfSSH_extended_data_send(ssh, data, sizeof(data));
2785+
AssertIntEQ(ret, WS_FATAL_ERROR);
2786+
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
2787+
2788+
ret = wolfSSH_global_request(ssh, data, sizeof(data), 0);
2789+
AssertIntEQ(ret, WS_FATAL_ERROR);
2790+
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
2791+
2792+
ret = wolfSSH_stream_exit(ssh, 0);
2793+
AssertIntEQ(ret, WS_FATAL_ERROR);
2794+
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
2795+
2796+
/* Not one byte left the session after the disconnect. */
2797+
AssertIntEQ(io.outSz, quietSz);
2798+
2799+
wolfSSH_free(ssh);
2800+
wolfSSH_CTX_free(ctx);
2801+
}
2802+
2803+
26822804
/* Sending SSH_MSG_DISCONNECT ends the session the same way receiving one
26832805
* does: RFC 4253 section 11.1 says the connection is over once the message
26842806
* goes out, so the stream calls must refuse afterwards. */
@@ -6454,6 +6576,8 @@ int main(int argc, char** argv)
64546576
#endif
64556577
TestDisconnectSetsDisconnectError();
64566578
TestDisconnectTerminalWithChannel();
6579+
TestDisconnectDrainsBufferedData();
6580+
TestDisconnectBlocksEverySend();
64576581
TestSendDisconnectIsTerminal();
64586582
#if !(defined(WOLFSSH_NO_RSA) && defined(WOLFSSH_NO_ECDSA_SHA2_NISTP256))
64596583
TestClientBuffersIdempotent();

wolfssh/internal.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,9 +1073,10 @@ struct WOLFSSH {
10731073
#endif
10741074
byte connReset;
10751075
byte isClosed;
1076-
/* Set when a DISCONNECT is sent or received. Only wolfSSH_stream_read()
1077-
* and wolfSSH_stream_send() are gated on it; the channel-id calls and
1078-
* wolfSSH_worker() are not, since the shutdown paths still pump them. */
1076+
/* Set when a DISCONNECT is sent or received. Gates every send call, so
1077+
* nothing more goes out. The read calls are not gated: data that
1078+
* arrived before the disconnect can still be drained. wolfSSH_worker()
1079+
* is not gated either, since the shutdown paths still pump it. */
10791080
byte disconnected;
10801081
byte clientOpenSSH;
10811082

wolfssh/ssh.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -558,9 +558,11 @@ WOLFSSH_API int wolfSSH_CTX_SetWindowPacketSize(WOLFSSH_CTX* ctx,
558558
WOLFSSH_API int wolfSSH_accept(WOLFSSH* ssh);
559559
WOLFSSH_API int wolfSSH_connect(WOLFSSH* ssh);
560560
WOLFSSH_API int wolfSSH_shutdown(WOLFSSH* ssh);
561-
/* A disconnect, sent or received, ends the session: wolfSSH_stream_read()
562-
* and wolfSSH_stream_send() report WS_DISCONNECT from then on, and channel
563-
* data that arrived before it but was never drained is dropped. */
561+
/* A disconnect, sent or received, ends the session. Nothing more goes out:
562+
* every send call below reports WS_DISCONNECT from then on. Reads are not
563+
* gated, so channel data that arrived before the disconnect can still be
564+
* drained; wolfSSH_stream_read() reports WS_DISCONNECT once its buffer
565+
* runs dry. RFC 4253 section 11.1. */
564566
WOLFSSH_API int wolfSSH_stream_peek(WOLFSSH* ssh, byte* buf, word32 bufSz);
565567
WOLFSSH_API int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz);
566568
WOLFSSH_API int wolfSSH_stream_send(WOLFSSH* ssh, byte* buf, word32 bufSz);

0 commit comments

Comments
 (0)