Skip to content

Commit 4166b8c

Browse files
ejohnstownejohnstown
committed
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
1 parent 9731cfe commit 4166b8c

4 files changed

Lines changed: 124 additions & 2 deletions

File tree

‎src/internal.c‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17373,7 +17373,8 @@ static int BuildUserAuthRequestKeyboard(WOLFSSH* ssh, byte* output, word32* idx,
1737317373
WMEMCPY(output + begin, authData->sf.keyboard.prompts[entry],
1737417374
authData->sf.keyboard.promptLengths[entry]);
1737517375
begin += authData->sf.keyboard.promptLengths[entry];
17376-
output[begin] = authData->sf.keyboard.promptEcho[entry];
17376+
/* RFC 4251 booleans are 0 or 1. */
17377+
output[begin] = (authData->sf.keyboard.promptEcho[entry] != 0);
1737717378
begin++;
1737817379
}
1737917380
*idx = begin;
@@ -17431,6 +17432,35 @@ int SendUserAuthKeyboardRequest(WOLFSSH* ssh, WS_UserAuthData* authData)
1743117432
}
1743217433
}
1743317434

17435+
if (ret == WS_SUCCESS && authData->sf.keyboard.promptCount > 0) {
17436+
/* RFC 4256 section 3.3 forbids an empty prompt. Check before
17437+
* sizing, so both passes see the same entries. */
17438+
word32 entry;
17439+
17440+
if (authData->sf.keyboard.promptLengths == NULL ||
17441+
authData->sf.keyboard.prompts == NULL ||
17442+
authData->sf.keyboard.promptEcho == NULL) {
17443+
WLOG(WS_LOG_DEBUG, "SUAKR: keyboard setup left a prompt array "
17444+
"unset");
17445+
ret = WS_BAD_USAGE;
17446+
}
17447+
17448+
for (entry = 0; ret == WS_SUCCESS &&
17449+
entry < authData->sf.keyboard.promptCount; entry++) {
17450+
if (authData->sf.keyboard.promptLengths[entry] == 0 ||
17451+
authData->sf.keyboard.prompts[entry] == NULL) {
17452+
WLOG(WS_LOG_DEBUG, "SUAKR: prompt %u is empty", entry);
17453+
ret = WS_BAD_USAGE;
17454+
}
17455+
/* The sizing pass sums these into a word32. */
17456+
else if (authData->sf.keyboard.promptLengths[entry] >
17457+
WOLFSSH_MAX_PROMPT_SZ) {
17458+
WLOG(WS_LOG_DEBUG, "SUAKR: prompt %u too long", entry);
17459+
ret = WS_BAD_USAGE;
17460+
}
17461+
}
17462+
}
17463+
1743417464
if (ret == WS_SUCCESS) {
1743517465
ssh->kbAuth.promptCount = authData->sf.keyboard.promptCount;
1743617466
}

‎tests/api.c‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7792,6 +7792,84 @@ static void test_wolfSSH_KeyboardInteractive(void)
77927792
#else /* WOLFSSH_SFTP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED */
77937793
static void test_wolfSSH_KeyboardInteractive(void) { ; }
77947794
#endif /* WOLFSSH_SFTP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED */
7795+
7796+
#ifndef NO_WOLFSSH_SERVER
7797+
7798+
/* Supplies the prompt set the test installed as the userAuth context. */
7799+
static int emptyPromptUserAuth(byte authType, WS_UserAuthData* authData,
7800+
void* ctx)
7801+
{
7802+
if (authType == WOLFSSH_USERAUTH_KEYBOARD_SETUP) {
7803+
WMEMCPY(&authData->sf.keyboard, (WS_UserAuthData_Keyboard*)ctx,
7804+
sizeof(WS_UserAuthData_Keyboard));
7805+
return WOLFSSH_USERAUTH_SUCCESS;
7806+
}
7807+
return WOLFSSH_USERAUTH_FAILURE;
7808+
}
7809+
7810+
7811+
/* The sender must refuse a setup callback that supplies an empty prompt.
7812+
* Refused before sizing, so no keyed session is needed. */
7813+
static void test_wolfSSH_KeyboardInteractive_emptyPrompt(void)
7814+
{
7815+
WOLFSSH_CTX* ctx = NULL;
7816+
WOLFSSH* ssh = NULL;
7817+
WS_UserAuthData authData;
7818+
WS_UserAuthData_Keyboard prompts;
7819+
byte* promptText[1];
7820+
word32 promptLengths[1];
7821+
byte promptEcho[1];
7822+
7823+
promptText[0] = (byte*)"Password: ";
7824+
promptLengths[0] = 10;
7825+
promptEcho[0] = 0;
7826+
WMEMSET(&prompts, 0, sizeof(prompts));
7827+
prompts.promptCount = 1;
7828+
prompts.prompts = promptText;
7829+
prompts.promptLengths = promptLengths;
7830+
prompts.promptEcho = promptEcho;
7831+
7832+
AssertNotNull(ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL));
7833+
wolfSSH_SetUserAuth(ctx, emptyPromptUserAuth);
7834+
AssertNotNull(ssh = wolfSSH_new(ctx));
7835+
wolfSSH_SetUserAuthCtx(ssh, &prompts);
7836+
7837+
/* Control: the intact prompt set clears validation. It fails later, on
7838+
* an unkeyed session, but not as bad usage. */
7839+
WMEMSET(&authData, 0, sizeof(authData));
7840+
AssertIntNE(SendUserAuthKeyboardRequest(ssh, &authData), WS_BAD_USAGE);
7841+
7842+
/* Zero-length prompt. */
7843+
promptLengths[0] = 0;
7844+
WMEMSET(&authData, 0, sizeof(authData));
7845+
AssertIntEQ(SendUserAuthKeyboardRequest(ssh, &authData), WS_BAD_USAGE);
7846+
7847+
/* Non-zero length, no buffer. */
7848+
promptLengths[0] = 10;
7849+
promptText[0] = NULL;
7850+
WMEMSET(&authData, 0, sizeof(authData));
7851+
AssertIntEQ(SendUserAuthKeyboardRequest(ssh, &authData), WS_BAD_USAGE);
7852+
7853+
/* A prompt longer than the payload bound. */
7854+
promptLengths[0] = WOLFSSH_MAX_PROMPT_SZ + 1;
7855+
promptText[0] = (byte*)"Password: ";
7856+
WMEMSET(&authData, 0, sizeof(authData));
7857+
AssertIntEQ(SendUserAuthKeyboardRequest(ssh, &authData), WS_BAD_USAGE);
7858+
7859+
/* Prompt count with the arrays unset. */
7860+
prompts.prompts = NULL;
7861+
prompts.promptLengths = NULL;
7862+
prompts.promptEcho = NULL;
7863+
WMEMSET(&authData, 0, sizeof(authData));
7864+
AssertIntEQ(SendUserAuthKeyboardRequest(ssh, &authData), WS_BAD_USAGE);
7865+
7866+
wolfSSH_free(ssh);
7867+
wolfSSH_CTX_free(ctx);
7868+
}
7869+
7870+
#else /* NO_WOLFSSH_SERVER */
7871+
static void test_wolfSSH_KeyboardInteractive_emptyPrompt(void) { ; }
7872+
#endif /* NO_WOLFSSH_SERVER */
77957873
#endif /* WOLFSSH_KEYBOARD_INTERACTIVE */
77967874

77977875
#endif /* WOLFSSH_TEST_BLOCK */
@@ -7897,6 +7975,7 @@ int wolfSSH_ApiTest(int argc, char** argv)
78977975
#endif
78987976
#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
78997977
test_wolfSSH_KeyboardInteractive();
7978+
test_wolfSSH_KeyboardInteractive_emptyPrompt();
79007979
#endif
79017980

79027981
/* SCP tests */

‎tests/auth.c‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1832,7 +1832,9 @@ static THREAD_RETURN WOLFSSH_THREAD server_thread(void* args)
18321832
for (word32 prompt = 0; prompt < kbResponseCount; prompt++) {
18331833
promptData.prompts[prompt] = (byte*)"Password: ";
18341834
promptData.promptLengths[prompt] = 10;
1835-
promptData.promptEcho[prompt] = 0;
1835+
/* Not 0 or 1, so the client sees whether the sender canonicalized
1836+
* the boolean. */
1837+
promptData.promptEcho[prompt] = 0x42;
18361838
}
18371839
}
18381840
else {
@@ -1915,6 +1917,9 @@ static int keyboardUserAuth(byte authType, WS_UserAuthData* authData, void* ctx)
19151917
AssertIntEQ(kbResponseCount, authData->sf.keyboard.promptCount);
19161918
for (word32 prompt = 0; prompt < kbResponseCount; prompt++) {
19171919
AssertStrEQ("Password: ", authData->sf.keyboard.prompts[prompt]);
1920+
/* RFC 4251 section 5: whatever the server stored, the wire
1921+
* carries 0 or 1. */
1922+
AssertIntLE(authData->sf.keyboard.promptEcho[prompt], 1);
19181923
}
19191924

19201925
authData->sf.keyboard.responseCount = kbResponseCount;

‎wolfssh/settings.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,14 @@ extern "C" {
8686
#define WOLFSSH_MAX_PROMPTS 64
8787
#endif
8888

89+
/* Maximum length of one prompt string. Bounds the request payload, which is
90+
* sized by summing the prompt lengths.
91+
*/
92+
93+
#if defined(WOLFSSH_KEYBOARD_INTERACTIVE) && !defined(WOLFSSH_MAX_PROMPT_SZ)
94+
#define WOLFSSH_MAX_PROMPT_SZ 1024
95+
#endif
96+
8997
#ifdef __cplusplus
9098
}
9199
#endif

0 commit comments

Comments
 (0)