Skip to content

Commit 59a1ef8

Browse files
ejohnstownphilljj
authored andcommitted
echoserver: close the agent socket on reset and handle a rekey
The worker drops back to APP_STATE_LISTEN when an agent connection ends, but never closes the socket. The next accept() overwrites agentFd, so every agent connection after the first leaks the previous descriptor. The forward path has the same gap on its connection-reset arm, where the socket is closed but fwdFd keeps the closed number. A rekey was treated as a read failure and ended the session. It cannot just be skipped either: wolfSSH_worker() reports WS_REKEYING in place of WS_CHAN_RXD while keying, and nothing raises the data report again, so ignoring it strands whatever arrived in that call and the peer waits on an answer that never comes. This is the hazard the library already calls out for WS_EXTDATA, which is exempted from the same override. - close agentFd and clear it on both the read-zero and the ECONNRESET/ECONNABORTED arms - clear fwdFd on the forward reset arm, matching the read-zero arm - clear agentCtx.appFd and fwdCtx.appFd wherever the worker closes the socket, so the stored copy cannot outlive the descriptor - drain the channel on WS_REKEYING as well as WS_CHAN_RXD, and take an empty read as "nothing buffered" rather than a failure on that path. wolfSSH_ChannelIdRead() has no isKeying gate and the window credit it owes is parked until the rekey completes
1 parent 26ccac3 commit 59a1ef8

1 file changed

Lines changed: 35 additions & 4 deletions

File tree

examples/echoserver/echoserver.c

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -999,14 +999,25 @@ static int ssh_worker(thread_ctx_t* threadCtx)
999999
cnt_r = wolfSSH_worker(ssh, &lastChannel);
10001000
if (cnt_r < 0) {
10011001
rc = wolfSSH_get_error(ssh);
1002-
if (rc == WS_CHAN_RXD) {
1002+
/* wolfSSH_worker() reports WS_REKEYING in place of
1003+
* WS_CHAN_RXD while a rekey is in flight, and the data
1004+
* report is never raised again, so drain on both or the
1005+
* buffered bytes sit there and the peer waits forever.
1006+
* wolfSSH_ChannelIdRead() has no isKeying gate; the window
1007+
* credit it owes is parked until the rekey finishes. */
1008+
if (rc == WS_CHAN_RXD || rc == WS_REKEYING) {
10031009
if (lastChannel == threadCtx->shellCtx.channelId) {
10041010
cnt_r = wolfSSH_ChannelIdRead(ssh,
10051011
threadCtx->shellCtx.channelId,
10061012
threadCtx->channelBuffer,
10071013
sizeof threadCtx->channelBuffer);
1008-
if (cnt_r <= 0)
1014+
if (cnt_r <= 0) {
1015+
/* Nothing was buffered. Only an actual data
1016+
* report makes that a failure. */
1017+
if (rc == WS_REKEYING)
1018+
continue;
10091019
break;
1020+
}
10101021
#ifdef SHELL_DEBUG
10111022
buf_dump(threadCtx->channelBuffer, cnt_r);
10121023
#endif
@@ -1044,8 +1055,13 @@ static int ssh_worker(thread_ctx_t* threadCtx)
10441055
cnt_r = wolfSSH_ChannelIdRead(ssh, agentChannelId,
10451056
threadCtx->channelBuffer,
10461057
sizeof threadCtx->channelBuffer);
1047-
if (cnt_r <= 0)
1058+
if (cnt_r <= 0) {
1059+
/* Nothing was buffered. Only an actual data
1060+
* report makes that a failure. */
1061+
if (rc == WS_REKEYING)
1062+
continue;
10481063
break;
1064+
}
10491065
#ifdef SHELL_DEBUG
10501066
buf_dump(threadCtx->channelBuffer, cnt_r);
10511067
#endif
@@ -1063,8 +1079,13 @@ static int ssh_worker(thread_ctx_t* threadCtx)
10631079
threadCtx->fwdCtx.channelId,
10641080
threadCtx->channelBuffer,
10651081
sizeof threadCtx->channelBuffer);
1066-
if (cnt_r <= 0)
1082+
if (cnt_r <= 0) {
1083+
/* Nothing was buffered. Only an actual data
1084+
* report makes that a failure. */
1085+
if (rc == WS_REKEYING)
1086+
continue;
10671087
break;
1088+
}
10681089
#ifdef SHELL_DEBUG
10691090
buf_dump(threadCtx->channelBuffer, cnt_r);
10701091
#endif
@@ -1084,6 +1105,7 @@ static int ssh_worker(thread_ctx_t* threadCtx)
10841105
if (fwdFd != -1) {
10851106
WCLOSESOCKET(fwdFd);
10861107
fwdFd = -1;
1108+
threadCtx->fwdCtx.appFd = -1;
10871109
}
10881110
if (threadCtx->fwdCbCtx.originName != NULL) {
10891111
WFREE(threadCtx->fwdCbCtx.originName,
@@ -1151,6 +1173,9 @@ static int ssh_worker(thread_ctx_t* threadCtx)
11511173
if (cnt_r == 0) {
11521174
/* Read zero-returned. Socket is closed. Go back
11531175
to listening. */
1176+
WCLOSESOCKET(agentFd);
1177+
agentFd = -1;
1178+
threadCtx->agentCtx.appFd = -1;
11541179
threadCtx->agentCtx.state = APP_STATE_LISTEN;
11551180
continue;
11561181
}
@@ -1164,6 +1189,9 @@ static int ssh_worker(thread_ctx_t* threadCtx)
11641189
err == SOCKET_ECONNABORTED) {
11651190
/* Connection reset. Socket is closed.
11661191
* Go back to listening. */
1192+
WCLOSESOCKET(agentFd);
1193+
agentFd = -1;
1194+
threadCtx->agentCtx.appFd = -1;
11671195
threadCtx->agentCtx.state = APP_STATE_LISTEN;
11681196
continue;
11691197
}
@@ -1215,6 +1243,7 @@ static int ssh_worker(thread_ctx_t* threadCtx)
12151243
to listening. */
12161244
WCLOSESOCKET(fwdFd);
12171245
fwdFd = -1;
1246+
threadCtx->fwdCtx.appFd = -1;
12181247
if (threadCtx->fwdCbCtx.hostName != NULL) {
12191248
WFREE(threadCtx->fwdCbCtx.hostName,
12201249
NULL, 0);
@@ -1235,6 +1264,8 @@ static int ssh_worker(thread_ctx_t* threadCtx)
12351264
/* Connection reset. Socket is closed.
12361265
* Go back to listening. */
12371266
WCLOSESOCKET(fwdFd);
1267+
fwdFd = -1;
1268+
threadCtx->fwdCtx.appFd = -1;
12381269
threadCtx->fwdCtx.state = APP_STATE_LISTEN;
12391270
continue;
12401271
}

0 commit comments

Comments
 (0)