Allocate channelName dynamically to shrink WOLFSSH struct by ~4KB

src/internal.c

@@ -1565,6 +1565,11 @@ void SshResourceFree(WOLFSSH* ssh, void* heap)
     if (ssh->userName) {
         WFREE(ssh->userName, heap, DYNTYPE_STRING);
     }
+    if (ssh->channelName != NULL) {
+        WFREE(ssh->channelName, heap, DYNTYPE_STRING);
+        ssh->channelName = NULL;
+        ssh->channelNameSz = 0;
+    }
     if (ssh->peerProtoId) {
         WFREE(ssh->peerProtoId, heap, DYNTYPE_STRING);
     }
@@ -19015,6 +19020,9 @@ int SendChannelRequest(WOLFSSH* ssh, byte* name, word32 nameSz)
     if (ssh == NULL)
         ret = WS_BAD_ARGUMENT;
 
+    if (ret == WS_SUCCESS && nameSz > 0 && name == NULL)
+        ret = WS_BAD_ARGUMENT;
+
     if (ret == WS_SUCCESS) {
         channel = ChannelFind(ssh,
                 ssh->defaultPeerChannelId, WS_CHANNEL_ID_PEER);
@@ -19079,17 +19087,19 @@ int SendChannelRequest(WOLFSSH* ssh, byte* name, word32 nameSz)
 
     #ifdef DEBUG_WOLFSSH
         /* only compile in code for checks on type if in debug mode */
-        switch (ssh->connectChannelId) {
-            case WOLFSSH_SESSION_EXEC:
-                WLOG(WS_LOG_INFO, "  command = %s", name);
-                break;
+        if (name != NULL) {
+            switch (ssh->connectChannelId) {
+                case WOLFSSH_SESSION_EXEC:
+                    WLOG(WS_LOG_INFO, "  command = %.*s", (int)nameSz, name);
+                    break;
 
-            case WOLFSSH_SESSION_SUBSYSTEM:
-                WLOG(WS_LOG_INFO, "  subsystem = %s", name);
-                break;
+                case WOLFSSH_SESSION_SUBSYSTEM:
+                    WLOG(WS_LOG_INFO, "  subsystem = %.*s", (int)nameSz, name);
+                    break;
 
-            default:
-                break;
+                default:
+                    break;
+            }
         }
     #endif
 

src/ssh.c

@@ -1614,6 +1614,12 @@ int wolfSSH_SetChannelType(WOLFSSH* ssh, byte type, byte* name, word32 nameSz)
 
     switch (type) {
         case WOLFSSH_SESSION_SHELL:
+            /* shell has no name; drop any name left by a prior subsystem/exec */
+            if (ssh->channelName != NULL) {
+                WFREE(ssh->channelName, ssh->ctx->heap, DYNTYPE_STRING);
+                ssh->channelName = NULL;
+            }
+            ssh->channelNameSz = 0;
             ssh->connectChannelId = type;
             break;
 
@@ -1624,21 +1630,45 @@ int wolfSSH_SetChannelType(WOLFSSH* ssh, byte type, byte* name, word32 nameSz)
             }
             FALL_THROUGH;
 
-        case WOLFSSH_SESSION_SUBSYSTEM:
-            ssh->connectChannelId = type;
-            if (name != NULL && nameSz < WOLFSSH_MAX_CHN_NAMESZ) {
-                WMEMCPY(ssh->channelName, name, nameSz);
-                ssh->channelNameSz = nameSz;
+        case WOLFSSH_SESSION_SUBSYSTEM: {
+            byte* newName;
+
+            if (name != NULL && nameSz > 0 && nameSz < WOLFSSH_MAX_CHN_NAMESZ) {
+                /* only (re)allocate when the name changed; SFTP/SCP retry
+                 * loops re-set the same name on every poll */
+                if (ssh->channelName == NULL || ssh->channelNameSz != nameSz ||
+                        WMEMCMP(ssh->channelName, name, nameSz) != 0) {
+                    newName = (byte*)WMALLOC(nameSz + 1, ssh->ctx->heap,
+                            DYNTYPE_STRING);
+                    if (newName == NULL) {
+                        return WS_MEMORY_E;
+                    }
+                    WMEMCPY(newName, name, nameSz);
+                    newName[nameSz] = 0;
+                    if (ssh->channelName != NULL) {
+                        WFREE(ssh->channelName, ssh->ctx->heap, DYNTYPE_STRING);
+                    }
+                    ssh->channelName = newName;
+                    ssh->channelNameSz = nameSz;
+                }
             }
             else {
+                /* invalid name ignored; type set but WS_SUCCESS returned */
                 WLOG(WS_LOG_DEBUG, "No subsystem name or name was too large");
             }
+            ssh->connectChannelId = type;
             break;
+        }
 
 #ifdef WOLFSSH_TERM
         case WOLFSSH_SESSION_TERMINAL:
             /* send a pseudo-terminal request and shell channel */
             ssh->sendTerminalRequest = 1;
+            if (ssh->channelName != NULL) {
+                WFREE(ssh->channelName, ssh->ctx->heap, DYNTYPE_STRING);
+                ssh->channelName = NULL;
+            }
+            ssh->channelNameSz = 0;
             ssh->connectChannelId = WOLFSSH_SESSION_SHELL;
             break;
 #endif

tests/api.c

@@ -285,6 +285,102 @@ static void test_wolfSSH_SetUsername(void)
 }
 
 
+static void test_wolfSSH_SetChannelType(void)
+{
+#ifndef NO_WOLFSSH_CLIENT
+    WOLFSSH_CTX* ctx;
+    WOLFSSH* ssh;
+    const byte sub1[] = "sftp";
+    const byte sub2[] = "a-longer-subsystem-name";
+    byte* prevName;
+
+    AssertIntNE(WS_SUCCESS, wolfSSH_SetChannelType(NULL,
+                WOLFSSH_SESSION_SHELL, NULL, 0));
+
+    AssertNotNull(ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL));
+    AssertNotNull(ssh = wolfSSH_new(ctx));
+
+    AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_SHELL, NULL, 0));
+    AssertNull(ssh->channelName);
+    AssertIntEQ(0, ssh->channelNameSz);
+
+    AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_SUBSYSTEM, NULL, 0));
+    AssertNull(ssh->channelName);
+
+    AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1, WOLFSSH_MAX_CHN_NAMESZ));
+    AssertNull(ssh->channelName);
+
+    AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1,
+                (word32)(sizeof(sub1) - 1)));
+    AssertNotNull(ssh->channelName);
+    AssertIntEQ((int)(sizeof(sub1) - 1), (int)ssh->channelNameSz);
+    AssertIntEQ(0, strcmp((const char*)ssh->channelName, (const char*)sub1));
+    AssertIntEQ(0, ssh->channelName[ssh->channelNameSz]); /* NUL terminated */
+
+    /* re-setting the same name must not reallocate (no churn) */
+    prevName = ssh->channelName;
+    AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1,
+                (word32)(sizeof(sub1) - 1)));
+    AssertIntEQ(1, ssh->channelName == prevName);
+
+    /* a rejected (oversize) name must leave the previous buffer intact */
+    AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1, WOLFSSH_MAX_CHN_NAMESZ));
+    AssertIntEQ(1, ssh->channelName == prevName);
+    AssertIntEQ((int)(sizeof(sub1) - 1), (int)ssh->channelNameSz);
+    AssertIntEQ(0, strcmp((const char*)ssh->channelName, (const char*)sub1));
+
+    /* a zero-length name is ignored and preserves the stored name */
+    AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1, 0));
+    AssertIntEQ(1, ssh->channelName == prevName);
+    AssertIntEQ((int)(sizeof(sub1) - 1), (int)ssh->channelNameSz);
+
+    /* repeated set frees the previous buffer before replacing it */
+    AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub2,
+                (word32)(sizeof(sub2) - 1)));
+    AssertNotNull(ssh->channelName);
+    AssertIntEQ((int)(sizeof(sub2) - 1), (int)ssh->channelNameSz);
+    AssertIntEQ(0, strcmp((const char*)ssh->channelName, (const char*)sub2));
+
+    /* EXEC reaches the same alloc path via fallthrough on the client side */
+    AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_EXEC, (byte*)sub1,
+                (word32)(sizeof(sub1) - 1)));
+    AssertNotNull(ssh->channelName);
+    AssertIntEQ((int)(sizeof(sub1) - 1), (int)ssh->channelNameSz);
+    AssertIntEQ(0, strcmp((const char*)ssh->channelName, (const char*)sub1));
+
+    /* switching to SHELL frees and clears a prior subsystem/exec name */
+    AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_SHELL, NULL, 0));
+    AssertNull(ssh->channelName);
+    AssertIntEQ(0, ssh->channelNameSz);
+
+    /* unknown channel type is rejected */
+    AssertIntNE(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_UNKNOWN, NULL, 0));
+
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+
+    /* server-side EXEC is rejected */
+    AssertNotNull(ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL));
+    AssertNotNull(ssh = wolfSSH_new(ctx));
+    AssertIntNE(WS_SUCCESS, wolfSSH_SetChannelType(ssh,
+                WOLFSSH_SESSION_EXEC, (byte*)sub1, (word32)(sizeof(sub1) - 1)));
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+#endif /* NO_WOLFSSH_CLIENT */
+}
+
+
 enum WS_TestFormatTypes {
     TEST_GOOD_FORMAT_ASN1 = WOLFSSH_FORMAT_ASN1,
     TEST_GOOD_FORMAT_PEM = WOLFSSH_FORMAT_PEM,
@@ -3837,6 +3933,7 @@ int wolfSSH_ApiTest(int argc, char** argv)
     test_client_wolfSSH_new();
     test_wolfSSH_set_fd();
     test_wolfSSH_SetUsername();
+    test_wolfSSH_SetChannelType();
     test_wolfSSH_ConvertConsole();
     test_wolfSSH_CTX_UsePrivateKey_buffer();
     test_wolfSSH_CTX_UseCert_buffer();

wolfssh/internal.h

@@ -915,7 +915,7 @@ struct WOLFSSH {
     word32 channelListSz;
     word32 defaultPeerChannelId;
     word32 connectChannelId;
-    byte channelName[WOLFSSH_MAX_CHN_NAMESZ];
+    byte* channelName;
     word32 channelNameSz;
     word32 lastRxId;