diff --git a/src/internal.c b/src/internal.c index 41168633a..8c61cc226 100644 --- a/src/internal.c +++ b/src/internal.c @@ -17373,7 +17373,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; @@ -17431,6 +17432,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; } diff --git a/tests/api.c b/tests/api.c index 58f7f5719..7f9cc47ca 100644 --- a/tests/api.c +++ b/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 */ diff --git a/tests/auth.c b/tests/auth.c index 3b45a6316..e70a2ef84 100644 --- a/tests/auth.c +++ b/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); } diff --git a/wolfssh/settings.h b/wolfssh/settings.h index 6c423402d..e31560bd9 100644 --- a/wolfssh/settings.h +++ b/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