Skip to content

Commit b9dcbe8

Browse files
committed
Gate both send-EOF calls on confirm and disconnect
wolfSSH_stream_send_eof() went straight to SendChannelEof(), which resolves its argument by peer id. An unconfirmed head has peerChannel 0, so the lookup lands on whichever channel holds peer id 0 -- itself, in the single-channel case -- and an EOF naming recipient channel 0 goes out while eofTxd latches on the live channel, silencing its send direction. - Mirror the openConfirmed guard from wolfSSH_ChannelSendEof() into wolfSSH_stream_send_eof(), returning WS_CHANNEL_NOT_CONF. - Add the SendAfterDisconnect() gate to both send-EOF entry points; they were the only public senders without it, against what ssh.h promises. - Order it ahead of the channel-list test, as the other stream calls do, so a torn-down session reports the disconnect and not a bad argument. - Name WS_DISCONNECT in the wolfSSH_ChannelSendEof() header contract. - Cover the stream variant in test_SendEofUnconfirmedChannel() and add test_SendEofAfterDisconnect(); both fail without the fix.
1 parent 41d1e52 commit b9dcbe8

3 files changed

Lines changed: 100 additions & 2 deletions

File tree

src/ssh.c

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1580,7 +1580,15 @@ int wolfSSH_stream_send_eof(WOLFSSH* ssh)
15801580

15811581
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_stream_send_eof()");
15821582

1583-
if (ssh == NULL || ssh->channelList == NULL)
1583+
if (ssh == NULL)
1584+
ret = WS_BAD_ARGUMENT;
1585+
1586+
/* Ahead of the channel-list test, like the other stream calls, so a
1587+
* torn-down session reports the disconnect and not a bad argument. */
1588+
if (ret == WS_SUCCESS && SendAfterDisconnect(ssh))
1589+
ret = WS_FATAL_ERROR;
1590+
1591+
if (ret == WS_SUCCESS && ssh->channelList == NULL)
15841592
ret = WS_BAD_ARGUMENT;
15851593

15861594
/* Only KEX traffic may go out mid-rekey, RFC 4253 section 7.1. */
@@ -1589,6 +1597,14 @@ int wolfSSH_stream_send_eof(WOLFSSH* ssh)
15891597
ret = WS_REKEYING;
15901598
}
15911599

1600+
/* Same peer-id lookup as wolfSSH_ChannelSendEof(), so the same guard.
1601+
* An unconfirmed head has peerChannel 0 and resolves to whichever
1602+
* channel holds peer id 0, itself included. */
1603+
if (ret == WS_SUCCESS && !ssh->channelList->openConfirmed) {
1604+
WLOG(WS_LOG_DEBUG, "Channel not confirmed yet.");
1605+
ret = WS_CHANNEL_NOT_CONF;
1606+
}
1607+
15921608
if (ret == WS_SUCCESS)
15931609
ret = SendChannelEof(ssh, ssh->channelList->peerChannel);
15941610

@@ -4379,6 +4395,10 @@ int wolfSSH_ChannelSendEof(WOLFSSH_CHANNEL* channel)
43794395
if (channel == NULL)
43804396
ret = WS_BAD_ARGUMENT;
43814397

4398+
if (ret == WS_SUCCESS && channel->ssh != NULL &&
4399+
SendAfterDisconnect(channel->ssh))
4400+
ret = WS_FATAL_ERROR;
4401+
43824402
/* Only KEX traffic may go out mid-rekey, RFC 4253 section 7.1. */
43834403
if (ret == WS_SUCCESS && channel->ssh->isKeying) {
43844404
channel->ssh->error = WS_REKEYING;

tests/unit.c

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7245,6 +7245,64 @@ static int test_SendEofApi(void)
72457245
return result;
72467246
}
72477247

7248+
/* A disconnect, sent or received, ends the session, so neither send-EOF
7249+
* entry point may put a CHANNEL_EOF on the wire afterwards. RFC 4253
7250+
* section 11.1. */
7251+
static int test_SendEofAfterDisconnect(void)
7252+
{
7253+
WOLFSSH_CTX* ctx = NULL;
7254+
WOLFSSH* ssh = NULL;
7255+
WOLFSSH_CHANNEL* ch = NULL;
7256+
int result = 0;
7257+
int ret;
7258+
7259+
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
7260+
if (ctx == NULL)
7261+
return -1660;
7262+
wolfSSH_SetIOSend(ctx, CountingIoSend);
7263+
7264+
ssh = wolfSSH_new(ctx);
7265+
if (ssh == NULL) { result = -1661; goto done; }
7266+
ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE;
7267+
7268+
ch = ChannelNew(ssh, ID_CHANTYPE_SESSION,
7269+
DEFAULT_WINDOW_SZ, DEFAULT_MAX_PACKET_SZ);
7270+
if (ch == NULL) { result = -1662; goto done; }
7271+
if (ChannelAppend(ssh, ch) != WS_SUCCESS) {
7272+
ChannelDelete(ch, ssh->ctx->heap);
7273+
result = -1663;
7274+
goto done;
7275+
}
7276+
ch->openConfirmed = 1;
7277+
ch->peerWindowSz = 1024;
7278+
ch->peerMaxPacketSz = 1024;
7279+
7280+
/* Our own disconnect is the last thing that may go out. */
7281+
ret = wolfSSH_SendDisconnect(ssh, WOLFSSH_DISCONNECT_BY_APPLICATION);
7282+
if (ret != WS_SUCCESS) { result = -1664; goto done; }
7283+
if (!ssh->disconnected) { result = -1665; goto done; }
7284+
7285+
s_ioSendCalls = 0;
7286+
7287+
ret = wolfSSH_stream_send_eof(ssh);
7288+
if (ret != WS_FATAL_ERROR) { result = -1666; goto done; }
7289+
if (wolfSSH_get_error(ssh) != WS_DISCONNECT) { result = -1667; goto done; }
7290+
7291+
ret = wolfSSH_ChannelSendEof(ch);
7292+
if (ret != WS_FATAL_ERROR) { result = -1668; goto done; }
7293+
if (wolfSSH_get_error(ssh) != WS_DISCONNECT) { result = -1669; goto done; }
7294+
7295+
/* Nothing reached the transport and the channel is unmarked, so a
7296+
* later drain cannot mistake it for a half-close of ours. */
7297+
if (s_ioSendCalls != 0) { result = -1670; goto done; }
7298+
if (ch->eofTxd) { result = -1671; goto done; }
7299+
7300+
done:
7301+
wolfSSH_free(ssh);
7302+
wolfSSH_CTX_free(ctx);
7303+
return result;
7304+
}
7305+
72487306
/* SendChannelEof() addresses the channel by peer id, and peerChannel is 0
72497307
* until the peer confirms the open. wolfSSH_ChannelSendEof() on a channel
72507308
* still awaiting its confirmation must refuse rather than let the lookup land
@@ -7303,6 +7361,20 @@ static int test_SendEofUnconfirmedChannel(void)
73037361
ret = wolfSSH_ChannelSend(sess, buf, (word32)sizeof(buf));
73047362
if (ret != (int)sizeof(buf)) { result = -1639; goto done; }
73057363

7364+
/* wolfSSH_stream_send_eof() runs the same lookup on the head, so it
7365+
* needs the same guard. An unconfirmed head resolves to whichever
7366+
* channel holds peer id 0, itself included, and latching eofTxd there
7367+
* would silence the real channel's send direction for good. */
7368+
sess->openConfirmed = 0;
7369+
ret = wolfSSH_stream_send_eof(ssh);
7370+
if (ret != WS_CHANNEL_NOT_CONF) { result = -1650; goto done; }
7371+
if (sess->eofTxd || pend->eofTxd) { result = -1651; goto done; }
7372+
7373+
sess->openConfirmed = 1;
7374+
ret = wolfSSH_stream_send_eof(ssh);
7375+
if (ret != WS_SUCCESS) { result = -1652; goto done; }
7376+
if (!sess->eofTxd) { result = -1653; goto done; }
7377+
73067378
done:
73077379
wolfSSH_free(ssh);
73087380
wolfSSH_CTX_free(ctx);
@@ -18374,6 +18446,11 @@ int wolfSSH_UnitTest(int argc, char** argv)
1837418446
(unitResult == 0 ? "SUCCESS" : "FAILED"));
1837518447
testResult = testResult || unitResult;
1837618448

18449+
unitResult = test_SendEofAfterDisconnect();
18450+
printf("SendEofAfterDisconnect: %s\n",
18451+
(unitResult == 0 ? "SUCCESS" : "FAILED"));
18452+
testResult = testResult || unitResult;
18453+
1837718454
unitResult = test_SendChannelEofWantWrite();
1837818455
printf("SendChannelEofWantWrite: %s\n",
1837918456
(unitResult == 0 ? "SUCCESS" : "FAILED"));

wolfssh/ssh.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,8 @@ WOLFSSH_API int wolfSSH_ChannelExit(WOLFSSH_CHANNEL* channel);
329329
*
330330
* Returns WS_SUCCESS, WS_BAD_ARGUMENT on a NULL channel,
331331
* WS_CHANNEL_NOT_CONF if the peer has not confirmed the channel open yet,
332-
* WS_REKEYING during a key exchange, or a send-path status such as
332+
* WS_REKEYING during a key exchange, WS_FATAL_ERROR with WS_DISCONNECT
333+
* latched once the session is over, or a send-path status such as
333334
* WS_WANT_WRITE. */
334335
WOLFSSH_API int wolfSSH_ChannelSendEof(WOLFSSH_CHANNEL* channel);
335336
WOLFSSH_API int wolfSSH_ChannelGetEof(WOLFSSH_CHANNEL* channel);

0 commit comments

Comments
 (0)