fix: validate keyboard-interactive prompts

RFC 4256 section 3.3 forbids an empty prompt, and RFC 4251 section 5
allows only 0 or 1 in a boolean field. Validation runs before the
payload is sized, so the sizing and building passes agree.

- reject a zero-length prompt or one with a NULL buffer
- reject a prompt count with the arrays unset
- reject a prompt over WOLFSSH_MAX_PROMPT_SZ, which the sizing pass
  would otherwise sum into a wrapped payload size
- normalize the outgoing echo byte to 0 or 1
- cover the rejections in tests/api.c, the echo byte in tests/auth.c

Issue: F-10583, F-10584

Author: ejohnstown

src/internal.c

@@ -17383,7 +17383,8 @@ static int BuildUserAuthRequestKeyboard(WOLFSSH* ssh, byte* output, word32* idx,
             WMEMCPY(output + begin, authData->sf.keyboard.prompts[entry],
                     authData->sf.keyboard.promptLengths[entry]);
             begin += authData->sf.keyboard.promptLengths[entry];
-            output[begin] = authData->sf.keyboard.promptEcho[entry];
+            /* RFC 4251 booleans are 0 or 1. */
+            output[begin] = (authData->sf.keyboard.promptEcho[entry] != 0);
             begin++;
         }
         *idx = begin;
@@ -17441,6 +17442,35 @@ int SendUserAuthKeyboardRequest(WOLFSSH* ssh, WS_UserAuthData* authData)
         }
     }
 
+    if (ret == WS_SUCCESS && authData->sf.keyboard.promptCount > 0) {
+        /* RFC 4256 section 3.3 forbids an empty prompt. Check before
+         * sizing, so both passes see the same entries. */
+        word32 entry;
+
+        if (authData->sf.keyboard.promptLengths == NULL ||
+                authData->sf.keyboard.prompts == NULL ||
+                authData->sf.keyboard.promptEcho == NULL) {
+            WLOG(WS_LOG_DEBUG, "SUAKR: keyboard setup left a prompt array "
+                "unset");
+            ret = WS_BAD_USAGE;
+        }
+
+        for (entry = 0; ret == WS_SUCCESS &&
+                entry < authData->sf.keyboard.promptCount; entry++) {
+            if (authData->sf.keyboard.promptLengths[entry] == 0 ||
+                    authData->sf.keyboard.prompts[entry] == NULL) {
+                WLOG(WS_LOG_DEBUG, "SUAKR: prompt %u is empty", entry);
+                ret = WS_BAD_USAGE;
+            }
+            /* The sizing pass sums these into a word32. */
+            else if (authData->sf.keyboard.promptLengths[entry] >
+                    WOLFSSH_MAX_PROMPT_SZ) {
+                WLOG(WS_LOG_DEBUG, "SUAKR: prompt %u too long", entry);
+                ret = WS_BAD_USAGE;
+            }
+        }
+    }
+
     if (ret == WS_SUCCESS) {
         ssh->kbAuth.promptCount = authData->sf.keyboard.promptCount;
     }

tests/api.c

@@ -7792,6 +7792,84 @@ static void test_wolfSSH_KeyboardInteractive(void)
 #else /* WOLFSSH_SFTP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED */
 static void test_wolfSSH_KeyboardInteractive(void) { ; }
 #endif /* WOLFSSH_SFTP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED */
+
+#ifndef NO_WOLFSSH_SERVER
+
+/* Supplies the prompt set the test installed as the userAuth context. */
+static int emptyPromptUserAuth(byte authType, WS_UserAuthData* authData,
+        void* ctx)
+{
+    if (authType == WOLFSSH_USERAUTH_KEYBOARD_SETUP) {
+        WMEMCPY(&authData->sf.keyboard, (WS_UserAuthData_Keyboard*)ctx,
+                sizeof(WS_UserAuthData_Keyboard));
+        return WOLFSSH_USERAUTH_SUCCESS;
+    }
+    return WOLFSSH_USERAUTH_FAILURE;
+}
+
+
+/* The sender must refuse a setup callback that supplies an empty prompt.
+ * Refused before sizing, so no keyed session is needed. */
+static void test_wolfSSH_KeyboardInteractive_emptyPrompt(void)
+{
+    WOLFSSH_CTX* ctx = NULL;
+    WOLFSSH* ssh = NULL;
+    WS_UserAuthData authData;
+    WS_UserAuthData_Keyboard prompts;
+    byte* promptText[1];
+    word32 promptLengths[1];
+    byte promptEcho[1];
+
+    promptText[0]    = (byte*)"Password: ";
+    promptLengths[0] = 10;
+    promptEcho[0]    = 0;
+    WMEMSET(&prompts, 0, sizeof(prompts));
+    prompts.promptCount   = 1;
+    prompts.prompts       = promptText;
+    prompts.promptLengths = promptLengths;
+    prompts.promptEcho    = promptEcho;
+
+    AssertNotNull(ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL));
+    wolfSSH_SetUserAuth(ctx, emptyPromptUserAuth);
+    AssertNotNull(ssh = wolfSSH_new(ctx));
+    wolfSSH_SetUserAuthCtx(ssh, &prompts);
+
+    /* Control: the intact prompt set clears validation. It fails later, on
+     * an unkeyed session, but not as bad usage. */
+    WMEMSET(&authData, 0, sizeof(authData));
+    AssertIntNE(SendUserAuthKeyboardRequest(ssh, &authData), WS_BAD_USAGE);
+
+    /* Zero-length prompt. */
+    promptLengths[0] = 0;
+    WMEMSET(&authData, 0, sizeof(authData));
+    AssertIntEQ(SendUserAuthKeyboardRequest(ssh, &authData), WS_BAD_USAGE);
+
+    /* Non-zero length, no buffer. */
+    promptLengths[0] = 10;
+    promptText[0] = NULL;
+    WMEMSET(&authData, 0, sizeof(authData));
+    AssertIntEQ(SendUserAuthKeyboardRequest(ssh, &authData), WS_BAD_USAGE);
+
+    /* A prompt longer than the payload bound. */
+    promptLengths[0] = WOLFSSH_MAX_PROMPT_SZ + 1;
+    promptText[0] = (byte*)"Password: ";
+    WMEMSET(&authData, 0, sizeof(authData));
+    AssertIntEQ(SendUserAuthKeyboardRequest(ssh, &authData), WS_BAD_USAGE);
+
+    /* Prompt count with the arrays unset. */
+    prompts.prompts = NULL;
+    prompts.promptLengths = NULL;
+    prompts.promptEcho = NULL;
+    WMEMSET(&authData, 0, sizeof(authData));
+    AssertIntEQ(SendUserAuthKeyboardRequest(ssh, &authData), WS_BAD_USAGE);
+
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+}
+
+#else /* NO_WOLFSSH_SERVER */
+static void test_wolfSSH_KeyboardInteractive_emptyPrompt(void) { ; }
+#endif /* NO_WOLFSSH_SERVER */
 #endif /* WOLFSSH_KEYBOARD_INTERACTIVE */
 
 #endif /* WOLFSSH_TEST_BLOCK */
@@ -7897,6 +7975,7 @@ int wolfSSH_ApiTest(int argc, char** argv)
 #endif
 #ifdef WOLFSSH_KEYBOARD_INTERACTIVE
     test_wolfSSH_KeyboardInteractive();
+    test_wolfSSH_KeyboardInteractive_emptyPrompt();
 #endif
 
     /* SCP tests */

tests/auth.c

@@ -1736,6 +1736,8 @@ word32 kbResponseCount;
 byte kbMultiRound = 0;
 byte currentRound = 0;
 byte unbalanced = 0;
+/* What the client must see for each prompt's echo byte. */
+byte kbExpectedEcho = 1;
 
 WS_UserAuthData_Keyboard promptData;
 
@@ -1832,7 +1834,9 @@ static THREAD_RETURN WOLFSSH_THREAD server_thread(void* args)
         for (word32 prompt = 0; prompt < kbResponseCount; prompt++) {
             promptData.prompts[prompt] = (byte*)"Password: ";
             promptData.promptLengths[prompt] = 10;
-            promptData.promptEcho[prompt] = 0;
+            /* Not 0 or 1, so the client sees whether the sender canonicalized
+             * the boolean. */
+            promptData.promptEcho[prompt] = 0x42;
         }
     }
     else {
@@ -1915,6 +1919,10 @@ static int keyboardUserAuth(byte authType, WS_UserAuthData* authData, void* ctx)
         AssertIntEQ(kbResponseCount, authData->sf.keyboard.promptCount);
         for (word32 prompt = 0; prompt < kbResponseCount; prompt++) {
             AssertStrEQ("Password: ", authData->sf.keyboard.prompts[prompt]);
+            /* RFC 4251 section 5: whatever the server stored, the wire
+             * carries the canonical 0 or 1. */
+            AssertIntEQ(authData->sf.keyboard.promptEcho[prompt],
+                    kbExpectedEcho);
         }
 
         authData->sf.keyboard.responseCount = kbResponseCount;
@@ -2235,6 +2243,8 @@ static void test_invalid_cb_keyboard(void)
     kbResponses[0]       = (byte*)testText1;
     kbResponseLengths[0] = 4;
     kbResponseCount      = 1;
+    /* This server stores 0, which must arrive as 0. */
+    kbExpectedEcho       = 0;
 
     serverArgs.signal          = &ready;
     serverArgs.pubkeyServerCtx = NULL;
@@ -2272,6 +2282,7 @@ static void test_invalid_cb_keyboard(void)
     ThreadJoin(serThread);
     AssertIntNE(serverArgs.return_code, WS_SUCCESS); /* auth must NOT be granted */
 
+    kbExpectedEcho = 1;
     FreeTcpReady(&ready);
 }
 

wolfssh/settings.h

@@ -86,6 +86,14 @@ extern "C" {
     #define WOLFSSH_MAX_PROMPTS 64
 #endif
 
+/* Maximum length of one prompt string. Bounds the request payload, which is
+ * sized by summing the prompt lengths.
+ */
+
+#if defined(WOLFSSH_KEYBOARD_INTERACTIVE) && !defined(WOLFSSH_MAX_PROMPT_SZ)
+    #define WOLFSSH_MAX_PROMPT_SZ 1024
+#endif
+
 #ifdef __cplusplus
 }
 #endif