agent: let the application open the agent channel

The one server-side site that opens auth-agent@openssh.com sits inside
wolfSSH_accept(), so an application driving its own channels cannot
reach it: the session records the request and no channel follows.

- add wolfSSH_AGENT_ChannelOpen(), the same open lifted out of accept(),
  which still calls it
- it reports WS_BAD_ARGUMENT until the peer asks and is idempotent
  afterward, so an application can poll it
- publish the agent on a queued open too, so a retry after WS_WANT_WRITE
  finds it rather than opening a second channel and leaking the first
- flush what is left of a queued open on the next call, rather than
  reporting a success the peer never saw

Author: ejohnstown

src/agent.c

@@ -1731,6 +1731,76 @@ int wolfSSH_AGENT_enable(WOLFSSH* ssh, byte isEnabled)
 }
 
 
+int wolfSSH_AGENT_ChannelOpen(WOLFSSH* ssh)
+{
+    WOLFSSH_AGENT_CTX* newAgent = NULL;
+    WOLFSSH_CHANNEL* newChannel = NULL;
+    int ret = WS_SUCCESS;
+
+    WLOG_ENTER();
+
+    if (ssh == NULL)
+        ret = WS_SSH_NULL_E;
+    else if (!ssh->useAgent) {
+        /* Nothing asked for agent forwarding on this session. */
+        ret = WS_BAD_ARGUMENT;
+    }
+    else if (ssh->agent == NULL) {
+        /* Server side sets ssh->agent here and nowhere else, so a NULL one
+         * is the "not opened yet" test. Idempotent so a caller polling for
+         * the peer's request cannot end up with two agent channels. */
+        WLOG(WS_LOG_AGENT, "Starting agent channel");
+
+        newAgent = wolfSSH_AGENT_new(ssh->ctx->heap);
+        if (newAgent == NULL)
+            ret = WS_MEMORY_E;
+
+        if (ret == WS_SUCCESS) {
+            newChannel = ChannelNew(ssh, ID_CHANTYPE_AUTH_AGENT,
+                    ssh->ctx->windowSz, ssh->ctx->maxPacketSz);
+            if (newChannel == NULL)
+                ret = WS_MEMORY_E;
+        }
+
+        if (ret == WS_SUCCESS) {
+            ret = SendChannelOpenSession(ssh, newChannel);
+
+            if (ret < WS_SUCCESS
+                    && ret != WS_WANT_WRITE && ret != WS_WANT_READ) {
+                ChannelDelete(newChannel, ssh->ctx->heap);
+            }
+            else {
+                /* Publish the agent even when the open is only queued, so
+                 * a retry takes the already-open path above rather than
+                 * opening a second channel. */
+                ChannelAppend(ssh, newChannel);
+                newAgent->channel = newChannel->channel;
+                ssh->agent = newAgent;
+                newAgent = NULL;
+                if (ssh->ctx->agentCb) {
+                    ssh->ctx->agentCb(WOLFSSH_AGENT_LOCAL_SETUP,
+                            ssh->agentCbCtx);
+                }
+            }
+        }
+
+        if (newAgent != NULL)
+            wolfSSH_AGENT_free(newAgent);
+    }
+    else if (wolfSSH_OutputPending(ssh)) {
+        /* The open is framed but not on the wire. Flush it rather than
+         * report a success the peer hasn't seen. */
+        ret = wolfSSH_SendPacket(ssh);
+    }
+
+    if (ssh != NULL)
+        ssh->error = ret;
+
+    WLOG_LEAVE(ret);
+    return ret;
+}
+
+
 int wolfSSH_AGENT_worker(WOLFSSH* ssh)
 {
     int ret = WS_SUCCESS;

src/ssh.c

@@ -764,52 +764,12 @@ int wolfSSH_accept(WOLFSSH* ssh)
 #endif /* WOLFSSH_SFTP and !NO_WOLFSSH_SERVER */
 #ifdef WOLFSSH_AGENT
                 if (ssh->useAgent) {
-                    WOLFSSH_AGENT_CTX* newAgent;
-                    WOLFSSH_CHANNEL* newChannel;
-
-                    WLOG(WS_LOG_AGENT, "Starting agent channel");
-
-                    newAgent = wolfSSH_AGENT_new(ssh->ctx->heap);
-                    if (newAgent == NULL) {
-                        ssh->error = WS_MEMORY_E;
-                        WLOG(WS_LOG_DEBUG, acceptError,
-                            "SERVER_USERAUTH_ACCEPT_DONE", ssh->error);
-                        return WS_ERROR;
-                    }
-
-                    newChannel = ChannelNew(ssh, ID_CHANTYPE_AUTH_AGENT,
-                            ssh->ctx->windowSz, ssh->ctx->maxPacketSz);
-                    if (newChannel == NULL) {
-                        wolfSSH_AGENT_free(newAgent);
-                        ssh->error = WS_MEMORY_E;
-                        WLOG(WS_LOG_DEBUG, acceptError,
-                            "SERVER_USERAUTH_ACCEPT_DONE", ssh->error);
-                        return WS_FATAL_ERROR;
-                    }
-
-                    ssh->error = SendChannelOpenSession(ssh, newChannel);
+                    ssh->error = wolfSSH_AGENT_ChannelOpen(ssh);
                     if (ssh->error < WS_SUCCESS) {
-                        if (ssh->error == WS_WANT_WRITE ||
-                                ssh->error == WS_WANT_READ) {
-                            ChannelAppend(ssh, newChannel);
-                        }
-                        else {
-                            ChannelDelete(newChannel, ssh->ctx->heap);
-                            wolfSSH_AGENT_free(newAgent);
-                        }
                         WLOG(WS_LOG_DEBUG, acceptError,
                             "SERVER_USERAUTH_ACCEPT_DONE", ssh->error);
                         return WS_FATAL_ERROR;
                     }
-                    ChannelAppend(ssh, newChannel);
-                    newAgent->channel = newChannel->channel;
-                    if (ssh->ctx->agentCb) {
-                        ssh->ctx->agentCb(WOLFSSH_AGENT_LOCAL_SETUP,
-                                ssh->agentCbCtx);
-                    }
-                    if (ssh->agent != NULL)
-                        wolfSSH_AGENT_free(ssh->agent);
-                    ssh->agent = newAgent;
                 }
 #endif /* WOLFSSH_AGENT */
                 ssh->acceptState = ACCEPT_CLIENT_SESSION_ESTABLISHED;

tests/regress.c

@@ -4170,6 +4170,45 @@ static void TestAgentChannelNullAgentSendsOpenFail(void)
 
     FreeChannelOpenHarness(&harness);
 }
+
+/* Nothing asked for forwarding, so the open is refused rather than started. */
+static void TestAgentChannelOpenWithoutRequest(void)
+{
+    ChannelOpenHarness harness;
+
+    InitChannelOpenHarness(&harness, NULL, 0);
+
+    AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_BAD_ARGUMENT);
+    AssertNull(harness.ssh->agent);
+    AssertIntEQ(harness.io.outSz, 0);
+
+    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. */
+static void TestAgentChannelOpenFlushesQueuedOpen(void)
+{
+    ChannelOpenHarness harness;
+
+    InitChannelOpenHarness(&harness, NULL, 0);
+    harness.ssh->useAgent = 1;
+    harness.io.blockNext = 1;
+
+    AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_WANT_WRITE);
+    AssertNotNull(harness.ssh->agent);
+    AssertIntEQ(harness.ssh->channelListSz, 1);
+    AssertIntEQ(harness.io.outSz, 0);
+
+    AssertIntEQ(wolfSSH_AGENT_ChannelOpen(harness.ssh), WS_SUCCESS);
+    AssertIntEQ(harness.ssh->channelListSz, 1);
+    AssertTrue(harness.io.outSz > 0);
+    AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz),
+            MSGID_CHANNEL_OPEN);
+
+    FreeChannelOpenHarness(&harness);
+}
 #endif
 
 
@@ -13153,6 +13192,8 @@ int main(int argc, char** argv)
 #endif
 #ifdef WOLFSSH_AGENT
     TestAgentChannelNullAgentSendsOpenFail();
+    TestAgentChannelOpenWithoutRequest();
+    TestAgentChannelOpenFlushesQueuedOpen();
 #endif
 #endif /* NO_WOLFSSH_SERVER */
 #if defined(WOLFSSH_AGENT) && !defined(WOLFSSH_NO_ED25519) \

wolfssh/agent.h

@@ -181,6 +181,13 @@ WOLFSSH_API int wolfSSH_CTX_set_agent_cb(WOLFSSH_CTX* ctx,
 WOLFSSH_API int wolfSSH_set_agent_cb_ctx(WOLFSSH* ssh, void* ctx);
 WOLFSSH_API int wolfSSH_CTX_AGENT_enable(WOLFSSH_CTX* ctx, byte isEnabled);
 WOLFSSH_API int wolfSSH_AGENT_enable(WOLFSSH* ssh, byte isEnabled);
+/* Server side. Opens the auth-agent@openssh.com channel to the client once
+ * the peer's auth-agent-req@openssh.com asks for forwarding. wolfSSH_accept()
+ * does it on the default path; an application driving its own channels polls
+ * this instead. Opens one channel, then flushes what of the open is queued.
+ * Returns WS_SUCCESS, WS_BAD_ARGUMENT before the peer asks, WS_WANT_READ or
+ * WS_WANT_WRITE with the open queued, WS_SSH_NULL_E, or WS_MEMORY_E. */
+WOLFSSH_API int wolfSSH_AGENT_ChannelOpen(WOLFSSH* ssh);
 WOLFSSH_LOCAL int wolfSSH_AGENT_worker(WOLFSSH* ssh);
 WOLFSSH_API int wolfSSH_AGENT_Relay(WOLFSSH* ssh,
         const byte* msg, word32* msgSz, byte* rsp, word32* rspSz);