mirror of https://github.com/wolfSSL/wolfssh.git
Make a disconnect end the session
SSH_MSG_DISCONNECT left nothing behind but ssh->error, which wolfSSH_stream_read() clears on entry. An application looping on the stream calls lost the code and went back to a connection already over. - Add WOLFSSH.disconnected, set by DoDisconnect() and SendDisconnect(). - DoDisconnect() sets it before decoding the payload, so a malformed message still ends the session. RFC 4253 section 11.1. - wolfSSH_stream_read() and wolfSSH_stream_send() report WS_DISCONNECT from the flag instead of reaching for the transport again. - Both guards run ahead of the channelList NULL test, so a torn-down session reports the disconnect rather than WS_BAD_ARGUMENT. - ssh.h states that undrained channel data goes with the session; internal.h states which calls the flag gates and which it does not. - regress.c: the receive side, the send side, and both of those again on a session with an open channel. Issue: F-8837 The test channel credits the peer's window too. Left at 0, SendChannelData() bails with WS_WINDOW_FULL before the wire, and the "nothing went out" checks would hold with the gate removed.pull/1192/head
parent
5303681b91
commit
e6b324d14f
|
|
@ -8174,6 +8174,10 @@ static int DoDisconnect(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
|
|||
|
||||
WOLFSSH_UNUSED(reasonStr);
|
||||
|
||||
/* RFC 4253 section 11.1, the peer is gone whether or not the rest of
|
||||
* the message decodes. */
|
||||
ssh->disconnected = 1;
|
||||
|
||||
ret = GetUint32(&reason, buf, len, &begin);
|
||||
if (ret == WS_SUCCESS) {
|
||||
/* Skip the description text. */
|
||||
|
|
@ -16782,6 +16786,11 @@ int SendDisconnect(WOLFSSH* ssh, word32 reason)
|
|||
if (ssh == NULL)
|
||||
ret = WS_BAD_ARGUMENT;
|
||||
|
||||
/* Mark the session over before the send. A partial or failed send
|
||||
* still ends it. */
|
||||
if (ret == WS_SUCCESS)
|
||||
ssh->disconnected = 1;
|
||||
|
||||
if (ret == WS_SUCCESS)
|
||||
ret = PreparePacket(ssh, MSG_ID_SZ + UINT32_SZ + (LENGTH_SZ * 2));
|
||||
|
||||
|
|
|
|||
20
src/ssh.c
20
src/ssh.c
|
|
@ -1228,7 +1228,15 @@ int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz)
|
|||
|
||||
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_stream_read()");
|
||||
|
||||
if (ssh == NULL || buf == NULL || bufSz == 0 || ssh->channelList == NULL)
|
||||
if (ssh == NULL || buf == NULL || bufSz == 0)
|
||||
return WS_BAD_ARGUMENT;
|
||||
|
||||
if (ssh->disconnected) {
|
||||
ssh->error = WS_DISCONNECT;
|
||||
return WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
if (ssh->channelList == NULL)
|
||||
return WS_BAD_ARGUMENT;
|
||||
|
||||
if (ssh->channelList->eofRxd) {
|
||||
|
|
@ -1307,7 +1315,15 @@ int wolfSSH_stream_send(WOLFSSH* ssh, byte* buf, word32 bufSz)
|
|||
|
||||
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_stream_send()");
|
||||
|
||||
if (ssh == NULL || buf == NULL || ssh->channelList == NULL)
|
||||
if (ssh == NULL || buf == NULL)
|
||||
return WS_BAD_ARGUMENT;
|
||||
|
||||
if (ssh->disconnected) {
|
||||
ssh->error = WS_DISCONNECT;
|
||||
return WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
if (ssh->channelList == NULL)
|
||||
return WS_BAD_ARGUMENT;
|
||||
|
||||
if (ssh->isKeying) {
|
||||
|
|
|
|||
136
tests/regress.c
136
tests/regress.c
|
|
@ -88,6 +88,7 @@ static void ResetSession(WOLFSSH* ssh)
|
|||
ssh->connectState = CONNECT_BEGIN;
|
||||
ssh->acceptState = ACCEPT_BEGIN;
|
||||
ssh->error = 0;
|
||||
ssh->disconnected = 0;
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -2850,6 +2851,7 @@ static void TestDisconnectSetsDisconnectError(void)
|
|||
MemIo io;
|
||||
byte in[128];
|
||||
byte out[32];
|
||||
byte data[8];
|
||||
word32 inSz;
|
||||
int ret;
|
||||
|
||||
|
|
@ -2873,6 +2875,138 @@ static void TestDisconnectSetsDisconnectError(void)
|
|||
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
|
||||
AssertIntEQ(io.inOff, io.inSz);
|
||||
|
||||
/* The disconnect is terminal, not just this call's error. Later stream
|
||||
* calls must report it rather than clearing the error and reading or
|
||||
* writing more. */
|
||||
AssertTrue(ssh->disconnected);
|
||||
|
||||
WMEMSET(data, 0, sizeof(data));
|
||||
ret = wolfSSH_stream_read(ssh, data, sizeof(data));
|
||||
AssertIntEQ(ret, WS_FATAL_ERROR);
|
||||
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
|
||||
|
||||
ret = wolfSSH_stream_send(ssh, data, sizeof(data));
|
||||
AssertIntEQ(ret, WS_FATAL_ERROR);
|
||||
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
|
||||
|
||||
wolfSSH_free(ssh);
|
||||
wolfSSH_CTX_free(ctx);
|
||||
}
|
||||
|
||||
|
||||
/* Append a bare session channel so the stream calls have a channel to work
|
||||
* on, the state a disconnect actually arrives in. */
|
||||
static void AddSessionChannel(WOLFSSH* ssh)
|
||||
{
|
||||
WOLFSSH_CHANNEL* ch;
|
||||
|
||||
ch = ChannelNew(ssh, ID_CHANTYPE_SESSION, 1024, 1024);
|
||||
AssertNotNull(ch);
|
||||
AssertIntEQ(ChannelAppend(ssh, ch), WS_SUCCESS);
|
||||
ch->openConfirmed = 1;
|
||||
/* Credit the peer's window too. Left at 0, SendChannelData() bails with
|
||||
* WS_WINDOW_FULL before the wire, and the "nothing went out" checks
|
||||
* would hold with the gates removed. */
|
||||
ch->peerWindowSz = 1024;
|
||||
ch->peerMaxPacketSz = 1024;
|
||||
}
|
||||
|
||||
|
||||
/* The same received disconnect on an established session. Without a channel
|
||||
* the stream calls bail out on the NULL channel list before they reach
|
||||
* anything, so this is the case that shows the gate doing work. */
|
||||
static void TestDisconnectTerminalWithChannel(void)
|
||||
{
|
||||
WOLFSSH_CTX* ctx;
|
||||
WOLFSSH* ssh;
|
||||
MemIo io;
|
||||
byte in[128];
|
||||
byte out[128];
|
||||
byte data[8];
|
||||
word32 inSz;
|
||||
int ret;
|
||||
|
||||
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
|
||||
AssertNotNull(ctx);
|
||||
|
||||
wolfSSH_SetIORecv(ctx, MemRecv);
|
||||
wolfSSH_SetIOSend(ctx, MemSend);
|
||||
|
||||
ssh = wolfSSH_new(ctx);
|
||||
AssertNotNull(ssh);
|
||||
AddSessionChannel(ssh);
|
||||
/* Past userauth, or the message filter blocks the sends on its own. */
|
||||
ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE;
|
||||
|
||||
inSz = BuildDisconnectPacket(WOLFSSH_DISCONNECT_BY_APPLICATION,
|
||||
in, sizeof(in));
|
||||
MemIoInit(&io, in, inSz, out, sizeof(out));
|
||||
wolfSSH_SetIOReadCtx(ssh, &io);
|
||||
wolfSSH_SetIOWriteCtx(ssh, &io);
|
||||
|
||||
ret = DoReceive(ssh);
|
||||
AssertIntEQ(ret, WS_FATAL_ERROR);
|
||||
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
|
||||
AssertTrue(ssh->disconnected);
|
||||
|
||||
WMEMSET(data, 0, sizeof(data));
|
||||
ret = wolfSSH_stream_read(ssh, data, sizeof(data));
|
||||
AssertIntEQ(ret, WS_FATAL_ERROR);
|
||||
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
|
||||
|
||||
/* Nothing may go out on the channel either. */
|
||||
ret = wolfSSH_stream_send(ssh, data, sizeof(data));
|
||||
AssertIntEQ(ret, WS_FATAL_ERROR);
|
||||
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
|
||||
AssertIntEQ(io.outSz, 0);
|
||||
|
||||
wolfSSH_free(ssh);
|
||||
wolfSSH_CTX_free(ctx);
|
||||
}
|
||||
|
||||
|
||||
/* Sending SSH_MSG_DISCONNECT ends the session the same way receiving one
|
||||
* does: RFC 4253 section 11.1 says the connection is over once the message
|
||||
* goes out, so the stream calls must refuse afterwards. */
|
||||
static void TestSendDisconnectIsTerminal(void)
|
||||
{
|
||||
WOLFSSH_CTX* ctx;
|
||||
WOLFSSH* ssh;
|
||||
MemIo io;
|
||||
byte out[128];
|
||||
byte data[8];
|
||||
int ret;
|
||||
|
||||
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
|
||||
AssertNotNull(ctx);
|
||||
|
||||
wolfSSH_SetIORecv(ctx, MemRecv);
|
||||
wolfSSH_SetIOSend(ctx, MemSend);
|
||||
|
||||
ssh = wolfSSH_new(ctx);
|
||||
AssertNotNull(ssh);
|
||||
AddSessionChannel(ssh);
|
||||
/* Past userauth, or the message filter blocks the sends on its own. */
|
||||
ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE;
|
||||
|
||||
MemIoInit(&io, NULL, 0, out, sizeof(out));
|
||||
wolfSSH_SetIOReadCtx(ssh, &io);
|
||||
wolfSSH_SetIOWriteCtx(ssh, &io);
|
||||
|
||||
ret = wolfSSH_SendDisconnect(ssh, WOLFSSH_DISCONNECT_BY_APPLICATION);
|
||||
AssertIntEQ(ret, WS_SUCCESS);
|
||||
AssertTrue(ssh->disconnected);
|
||||
AssertTrue(io.outSz > 0);
|
||||
|
||||
WMEMSET(data, 0, sizeof(data));
|
||||
ret = wolfSSH_stream_send(ssh, data, sizeof(data));
|
||||
AssertIntEQ(ret, WS_FATAL_ERROR);
|
||||
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
|
||||
|
||||
ret = wolfSSH_stream_read(ssh, data, sizeof(data));
|
||||
AssertIntEQ(ret, WS_FATAL_ERROR);
|
||||
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
|
||||
|
||||
wolfSSH_free(ssh);
|
||||
wolfSSH_CTX_free(ctx);
|
||||
}
|
||||
|
|
@ -6603,6 +6737,8 @@ int main(int argc, char** argv)
|
|||
TestDoNewKeys();
|
||||
#endif
|
||||
TestDisconnectSetsDisconnectError();
|
||||
TestDisconnectTerminalWithChannel();
|
||||
TestSendDisconnectIsTerminal();
|
||||
#if !(defined(WOLFSSH_NO_RSA) && defined(WOLFSSH_NO_ECDSA_SHA2_NISTP256))
|
||||
TestClientBuffersIdempotent();
|
||||
#endif
|
||||
|
|
|
|||
|
|
@ -1102,6 +1102,10 @@ struct WOLFSSH {
|
|||
#endif
|
||||
byte connReset;
|
||||
byte isClosed;
|
||||
/* Set when a DISCONNECT is sent or received. Only wolfSSH_stream_read()
|
||||
* and wolfSSH_stream_send() are gated on it; the channel-id calls and
|
||||
* wolfSSH_worker() are not, since the shutdown paths still pump them. */
|
||||
byte disconnected;
|
||||
byte clientOpenSSH;
|
||||
|
||||
byte kexId;
|
||||
|
|
|
|||
|
|
@ -558,6 +558,9 @@ WOLFSSH_API int wolfSSH_CTX_SetWindowPacketSize(WOLFSSH_CTX* ctx,
|
|||
WOLFSSH_API int wolfSSH_accept(WOLFSSH* ssh);
|
||||
WOLFSSH_API int wolfSSH_connect(WOLFSSH* ssh);
|
||||
WOLFSSH_API int wolfSSH_shutdown(WOLFSSH* ssh);
|
||||
/* A disconnect, sent or received, ends the session: wolfSSH_stream_read()
|
||||
* and wolfSSH_stream_send() report WS_DISCONNECT from then on, and channel
|
||||
* data that arrived before it but was never drained is dropped. */
|
||||
WOLFSSH_API int wolfSSH_stream_peek(WOLFSSH* ssh, byte* buf, word32 bufSz);
|
||||
WOLFSSH_API int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz);
|
||||
WOLFSSH_API int wolfSSH_stream_send(WOLFSSH* ssh, byte* buf, word32 bufSz);
|
||||
|
|
|
|||
Loading…
Reference in New Issue