Clear disconnectTxd once the buffer drains

The flag was set in SendDisconnect() and never cleared, so it meant "a
disconnect was sent" rather than "a flush is owed". Once ours had gone
out, the next teardown call still pushed whatever the internal senders
had queued behind it: measured, a CHANNEL_EOF from DoChannelEof() went
on the wire after the disconnect. wolfSSH_SendPacket() now clears it.

Issue: F-8837

Author: ejohnstown

src/internal.c

@@ -4327,6 +4327,11 @@ int wolfSSH_SendPacket(WOLFSSH* ssh)
 
     ssh->outputBuffer.plainSz = 0;
 
+    /* The buffer is empty, so our disconnect, if one was in it, has gone
+     * out and no flush is owed. Leaving this set hands the next teardown
+     * call a licence to push whatever gets queued next. */
+    ssh->disconnectTxd = 0;
+
     WLOG(WS_LOG_DEBUG, "SB: Shrinking output buffer");
     ShrinkBuffer(&ssh->outputBuffer, 0);
     return HighwaterCheck(ssh, WOLFSSH_HWSIDE_TRANSMIT);

tests/regress.c

@@ -3811,6 +3811,70 @@ static void TestDisconnectOutranksRekey(void)
 }
 
 
+/* disconnectTxd means "a flush is owed", not "a disconnect was sent". Once
+ * ours has gone out, a teardown call must not push whatever the internal
+ * senders queued behind it. */
+static void TestDisconnectTxdClearsOnFlush(void)
+{
+    WOLFSSH_CTX* ctx;
+    WOLFSSH* ssh;
+    WOLFSSH_CHANNEL* channel;
+    MemIo io;
+    byte out[512];
+    int ret;
+
+    ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
+    AssertNotNull(ctx);
+
+    wolfSSH_SetIORecv(ctx, MemRecv);
+    wolfSSH_SetIOSend(ctx, MemSendWantWrite);
+
+    ssh = wolfSSH_new(ctx);
+    AssertNotNull(ssh);
+    AddSessionChannel(ssh);
+    channel = ssh->channelList;
+    ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE;
+
+    MemIoInit(&io, NULL, 0, out, sizeof(out));
+    wolfSSH_SetIOReadCtx(ssh, &io);
+    wolfSSH_SetIOWriteCtx(ssh, &io);
+
+    /* Our disconnect short-sends, so a flush is owed. */
+    MemSendWantWriteCount = 1;
+    AssertIntEQ(wolfSSH_SendDisconnect(ssh, WOLFSSH_DISCONNECT_BY_APPLICATION),
+            WS_WANT_WRITE);
+    AssertTrue(ssh->disconnectTxd);
+    AssertTrue(wolfSSH_OutputPending(ssh));
+    AssertIntEQ(io.outSz, 0);
+
+    /* The caller's retry loop pumps it out in full, so nothing is owed. */
+    AssertIntEQ(wolfSSH_SendPacket(ssh), WS_SUCCESS);
+    AssertFalse(wolfSSH_OutputPending(ssh));
+    AssertFalse(ssh->disconnectTxd);
+    AssertIntEQ(out[LENGTH_SZ + 1], MSGID_DISCONNECT);
+    io.outSz = 0;
+
+    /* An in-flight CHANNEL_EOF draws a reply out of DoChannelEof(); the
+     * internal senders are not behind the disconnect gate. It short-sends,
+     * so it sits in the output buffer. */
+    MemSendWantWriteCount = 1;
+    AssertIntEQ(SendChannelEof(ssh, channel->peerChannel), WS_WANT_WRITE);
+    AssertTrue(wolfSSH_OutputPending(ssh));
+    AssertIntEQ(io.outSz, 0);
+
+    /* Teardown must leave it there: the disconnect is already gone, so this
+     * would be traffic after it. RFC 4253 section 11.1. */
+    ret = wolfSSH_shutdown(ssh);
+    AssertIntEQ(ret, WS_SUCCESS);
+    AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
+    AssertIntEQ(io.outSz, 0);
+    AssertTrue(wolfSSH_OutputPending(ssh));
+
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+}
+
+
 /* A flush that is itself short owns ssh->error. Callers gate their retry on
  * WS_WANT_WRITE, so the disconnect gate must not overwrite it. */
 static void TestShutdownKeepsFlushWantWrite(void)
@@ -7644,6 +7708,7 @@ int main(int argc, char** argv)
     TestQueuedDisconnectFlushes();
     TestShutdownFlushesQueuedDisconnect();
     TestShutdownKeepsFlushWantWrite();
+    TestDisconnectTxdClearsOnFlush();
     TestDisconnectOutranksRekey();
 #if defined(WOLFSSH_TERM) && !defined(NO_FILESYSTEM)
     TestTerminalResizeBlockedAfterDisconnect();

wolfssh/internal.h

@@ -1110,7 +1110,8 @@ struct WOLFSSH {
      * paths pump the worker. */
     byte disconnected;
     /* Set once SendDisconnect() has bundled our own DISCONNECT into the
-     * output buffer, so a short send can still be flushed. The flag above
+     * output buffer, and cleared once wolfSSH_SendPacket() drains it, so it
+     * means "a flush is owed" rather than "one was sent". The flag above
      * cannot stand in for this: it does not say whose disconnect it was,
      * and a peer's leaves only unrelated traffic queued. */
     byte disconnectTxd;