agent: refuse a channel open after a disconnect

wolfSSH_AGENT_ChannelOpen() answers a poll on a session that is over
with WS_FATAL_ERROR and WS_DISCONNECT in ssh->error, the shape every
other public sender uses: no channel opened, nothing on the wire,
RFC 4253 section 11.1. wolfSSH_accept() gates the open it drives, so
the new public entry point is the only way in.

- promote SendAfterDisconnect() to WOLFSSH_LOCAL so agent.c uses the
  same helper as every other public sender
- leave an open queued before the disconnect unflushed, the rule
  wolfSSH_shutdown() applies to all but its own disconnect
- keep WS_DISCONNECT in ssh->error at the accept() call site, which
  used to overwrite it with the status the open returns

Author: ejohnstown

src/agent.c

@@ -1749,6 +1749,13 @@ int wolfSSH_AGENT_ChannelOpen(WOLFSSH* ssh)
          * checks below would report a channel a client never opened. */
         ret = WS_BAD_ARGUMENT;
     }
+    else if (SendAfterDisconnect(ssh)) {
+        /* The session is over, so neither a new open nor the flush of one
+         * queued before the disconnect may go out. RFC 4253 section 11.1.
+         * WS_DISCONNECT is in ssh->error, where the rest of the API puts
+         * it. */
+        ret = WS_FATAL_ERROR;
+    }
     else if (!ssh->useAgent) {
         /* Nothing asked for agent forwarding on this session. */
         ret = WS_BAD_ARGUMENT;

src/ssh.c

@@ -567,10 +567,6 @@ static int DoReceiveHandshake(WOLFSSH* ssh)
 #endif /* !NO_WOLFSSH_SERVER || !NO_WOLFSSH_CLIENT */
 
 
-/* Defined below, ahead of both drivers; either can be the only one built. */
-static int SendAfterDisconnect(WOLFSSH* ssh);
-
-
 #ifndef NO_WOLFSSH_SERVER
 
 const char acceptError[] = "accept error: %s, %d";
@@ -764,8 +760,13 @@ int wolfSSH_accept(WOLFSSH* ssh)
 #endif /* WOLFSSH_SFTP and !NO_WOLFSSH_SERVER */
 #ifdef WOLFSSH_AGENT
                 if (ssh->useAgent) {
-                    ssh->error = wolfSSH_AGENT_ChannelOpen(ssh);
-                    if (ssh->error < WS_SUCCESS) {
+                    int agentRet = wolfSSH_AGENT_ChannelOpen(ssh);
+
+                    if (agentRet < WS_SUCCESS) {
+                        /* WS_FATAL_ERROR is the disconnect, which already
+                         * recorded WS_DISCONNECT; keep that. */
+                        if (agentRet != WS_FATAL_ERROR)
+                            ssh->error = agentRet;
                         WLOG(WS_LOG_DEBUG, acceptError,
                             "SERVER_USERAUTH_ACCEPT_DONE", ssh->error);
                         return WS_FATAL_ERROR;
@@ -1094,11 +1095,8 @@ int wolfSSH_connect(WOLFSSH* ssh)
 #endif /* NO_WOLFSSH_CLIENT */
 
 
-/* A disconnect, sent or received, ends the session, so nothing further may
- * go out. RFC 4253 section 11.1. Reads are deliberately not gated on this:
- * channel data that arrived before the disconnect is still the caller's.
- * Call only after ssh has been checked for NULL. */
-static int SendAfterDisconnect(WOLFSSH* ssh)
+/* See wolfssh/internal.h for the contract. */
+int SendAfterDisconnect(WOLFSSH* ssh)
 {
     if (ssh->disconnected) {
         WLOG(WS_LOG_DEBUG, "Send attempted after a disconnect");

tests/regress.c

@@ -4459,6 +4459,48 @@ static void TestAgentChannelOpenWithoutRequest(void)
     FreeChannelOpenHarness(&harness);
 }
 
+/* A poll after the peer disconnects must not open a channel or put anything
+ * on the wire. RFC 4253 section 11.1: the session is over. */
+static void TestAgentChannelOpenAfterDisconnect(void)
+{
+    ChannelOpenHarness harness;
+
+    InitChannelOpenHarness(&harness, NULL, 0);
+    harness.ssh->useAgent = 1;
+    harness.ssh->disconnected = 1;
+
+    AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_FATAL_ERROR);
+    AssertNull(harness.ssh->agent);
+    AssertIntEQ(harness.ssh->channelListSz, 0);
+    AssertIntEQ(harness.io.outSz, 0);
+    AssertIntEQ(harness.ssh->error, WS_DISCONNECT);
+
+    FreeChannelOpenHarness(&harness);
+}
+
+/* An open queued before the disconnect is not flushed either: those bytes
+ * belong to a session that is over, the same rule wolfSSH_shutdown() applies
+ * to everything but its own queued disconnect. */
+static void TestAgentChannelOpenQueuedThenDisconnect(void)
+{
+    ChannelOpenHarness harness;
+
+    InitChannelOpenHarness(&harness, NULL, 0);
+    harness.ssh->useAgent = 1;
+    harness.io.blockNext = 1;
+
+    AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_WANT_WRITE);
+    AssertIntEQ(harness.io.outSz, 0);
+
+    harness.ssh->disconnected = 1;
+
+    AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_FATAL_ERROR);
+    AssertIntEQ(harness.io.outSz, 0);
+    AssertIntEQ(harness.ssh->error, WS_DISCONNECT);
+
+    FreeChannelOpenHarness(&harness);
+}
+
 /* A queued open publishes the agent, so the caller's next poll must finish
  * the send rather than report a success the peer never saw, and must not
  * open a second channel. */
@@ -13525,6 +13567,8 @@ int main(int argc, char** argv)
     TestAgentChannelNullAgentSendsOpenFail();
     TestAgentChannelOpenWithoutRequest();
     TestAgentChannelOpenFlushesQueuedOpen();
+    TestAgentChannelOpenAfterDisconnect();
+    TestAgentChannelOpenQueuedThenDisconnect();
     TestAgentChannelOpenSendFailureCleansUp();
 #ifndef NO_WOLFSSH_CLIENT
     TestAgentChannelOpenOnClientRefused();

wolfssh/agent.h

@@ -187,11 +187,12 @@ WOLFSSH_API int wolfSSH_AGENT_enable(WOLFSSH* ssh, byte isEnabled);
  * this instead. Opens one channel, then flushes what of the open is queued.
  * Returns WS_SUCCESS, WS_BAD_ARGUMENT before the peer asks or on a client
  * session, WS_WANT_READ or WS_WANT_WRITE while output is still queued,
+ * WS_FATAL_ERROR with WS_DISCONNECT in ssh->error once the session is over,
  * WS_SSH_NULL_E, WS_MEMORY_E, or whatever the send reports. WS_SUCCESS says
  * the open went out, not that the peer took it; a refusal reaches the
  * channel-open-fail callback.
- * Only the send records in ssh->error, so a poll ahead of the peer's request
- * leaves the session fit for wolfSSH_accept(). */
+ * Only that and the send record in ssh->error, so a poll ahead of the peer's
+ * request leaves the session fit for wolfSSH_accept(). */
 WOLFSSH_API int wolfSSH_AGENT_ChannelOpen(WOLFSSH* ssh);
 WOLFSSH_LOCAL int wolfSSH_AGENT_worker(WOLFSSH* ssh);
 WOLFSSH_API int wolfSSH_AGENT_Relay(WOLFSSH* ssh,

wolfssh/internal.h

@@ -1643,6 +1643,12 @@ enum ChannelOpenFailReasons {
     OPEN_RESOURCE_SHORTAGE
 };
 
+/* A disconnect, sent or received, ends the session, so nothing further may
+ * go out. RFC 4253 section 11.1. Returns 1 and records WS_DISCONNECT in
+ * ssh->error when the session is over, 0 otherwise. Reads are deliberately
+ * not gated on this: channel data that arrived before the disconnect is
+ * still the caller's. Call only after ssh has been checked for NULL. */
+WOLFSSH_LOCAL int SendAfterDisconnect(WOLFSSH* ssh);
 WOLFSSH_LOCAL int DoReceive(WOLFSSH* ssh);
 WOLFSSH_LOCAL int DoProtoId(WOLFSSH* ssh);
 WOLFSSH_LOCAL int wolfSSH_SendPacket(WOLFSSH* ssh);