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

Author: ejohnstown

src/internal.c

@@ -8166,6 +8166,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. */
@@ -16731,6 +16735,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));
 

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) {

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;
 }
 
 
@@ -2571,6 +2572,7 @@ static void TestDisconnectSetsDisconnectError(void)
     MemIo io;
     byte in[128];
     byte out[32];
+    byte data[8];
     word32 inSz;
     int ret;
 
@@ -2594,6 +2596,129 @@ 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;
+}
+
+
+/* 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);
+
+    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);
+
+    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);
 }
@@ -6328,6 +6453,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

wolfssh/internal.h

@@ -1073,6 +1073,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;

wolfssh/ssh.h

@@ -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);