From 6bc89ac2f4510f5a526731d84321416f1c0891f6 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Mon, 29 Jun 2026 13:55:07 +0900 Subject: [PATCH] keyboard-interactive: send USERAUTH_FAILURE on response-count mismatch --- src/internal.c | 10 ++--- tests/regress.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 6 deletions(-) diff --git a/src/internal.c b/src/internal.c index 35b5e73df..7b1a15718 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8223,12 +8223,10 @@ static int DoUserAuthInfoResponse(WOLFSSH* ssh, if ((ret == WS_SUCCESS) && (ssh->kbAuth.promptCount != kb->responseCount)) { WLOG(WS_LOG_DEBUG, "DUARKB: Invalid number of responses received"); - ret = WS_USER_AUTH_E; - } - - if (ret == WS_SUCCESS && kb->responseCount > WOLFSSH_MAX_PROMPTS) { - WLOG(WS_LOG_DEBUG, "DUARKB: Received too many responses (%d), max: %d", - kb->responseCount, WOLFSSH_MAX_PROMPTS); + /* Answer with USERAUTH_FAILURE instead of tearing down the transport. + * Keeping ret non-success skips the allocation, parse, and callback + * below; past here responseCount equals the capped promptCount. */ + authFailure = 1; ret = WS_USER_AUTH_E; } diff --git a/tests/regress.c b/tests/regress.c index 27a89f7d8..5f02e01c0 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -340,6 +340,103 @@ static void FreeChannelOpenHarness(ChannelOpenHarness* harness) wolfSSH_CTX_free(harness->ctx); } +#ifdef WOLFSSH_KEYBOARD_INTERACTIVE +/* Build a plaintext SSH_MSG_USERAUTH_INFO_RESPONSE. The wire response count is + * "responseCount"; "stringCount" "x" response strings are actually appended. + * The count-mismatch guard rejects before parsing the body, so a short body is + * sufficient to drive it. */ +static word32 BuildInfoResponsePacket(word32 responseCount, word32 stringCount, + byte* out, word32 outSz) +{ + byte payload[128]; + word32 idx = 0; + word32 i; + + idx = AppendUint32(payload, sizeof(payload), idx, responseCount); + for (i = 0; i < stringCount; i++) { + idx = AppendString(payload, sizeof(payload), idx, "x"); + } + + return WrapPacket(MSGID_USERAUTH_INFO_RESPONSE, payload, idx, out, outSz); +} + +/* Place the server mid keyboard-interactive auth, expecting an INFO_RESPONSE + * for "promptCount" prompts. */ +static void InitKbInfoResponseHarness(ChannelOpenHarness* harness, + byte* in, word32 inSz, word32 promptCount) +{ + InitChannelOpenHarness(harness, in, inSz); + harness->ssh->acceptState = ACCEPT_CLIENT_USERAUTH_REQUEST_DONE; + harness->ssh->authId = ID_USERAUTH_KEYBOARD; + harness->ssh->kbAuth.promptCount = promptCount; +} + +/* Post-condition for a rejected INFO_RESPONSE: the transport survives and the + * server's reply is a USERAUTH_FAILURE rather than a teardown. */ +static void AssertKbInfoResponseSentFailure(const ChannelOpenHarness* harness, + int ret) +{ + AssertIntEQ(ret, WS_SUCCESS); + AssertIntEQ(harness->io.inOff, harness->io.inSz); + AssertTrue(harness->io.outSz > 0); + AssertIntEQ(ParseMsgId(harness->io.out, harness->io.outSz), + MSGID_USERAUTH_FAILURE); +} + +/* A response count that disagrees with the server's prompt count must yield a + * USERAUTH_FAILURE, not a transport teardown (RFC 4256). */ +static void TestKbInfoResponseCountMismatchSendsFailure(void) +{ + ChannelOpenHarness harness; + byte in[128]; + word32 inSz; + int ret; + + /* Server expects 1 response; client sends 2. */ + inSz = BuildInfoResponsePacket(2, 2, in, sizeof(in)); + InitKbInfoResponseHarness(&harness, in, inSz, 1); + + ret = DoReceive(harness.ssh); + + AssertKbInfoResponseSentFailure(&harness, ret); + + FreeChannelOpenHarness(&harness); +} + +/* A rejected INFO_RESPONSE must leave packet framing intact: the next packet on + * the surviving connection still parses and draws its own failure. */ +static void TestKbInfoResponseMismatchKeepsFraming(void) +{ + ChannelOpenHarness harness; + byte in[256]; + word32 inSz, nextSz, firstOutSz; + int ret; + + /* Two back-to-back responses, both disagreeing with a prompt count of 1. */ + inSz = BuildInfoResponsePacket(2, 2, in, sizeof(in)); + nextSz = BuildInfoResponsePacket(3, 3, in + inSz, + (word32)sizeof(in) - inSz); + InitKbInfoResponseHarness(&harness, in, inSz + nextSz, 1); + + ret = DoReceive(harness.ssh); + AssertIntEQ(ret, WS_SUCCESS); + AssertTrue(harness.io.outSz > 0); + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + MSGID_USERAUTH_FAILURE); + firstOutSz = harness.io.outSz; + + ret = DoReceive(harness.ssh); + AssertIntEQ(ret, WS_SUCCESS); + AssertIntEQ(harness.io.inOff, harness.io.inSz); + AssertTrue(harness.io.outSz > firstOutSz); + AssertIntEQ(ParseMsgId(harness.io.out + firstOutSz, + harness.io.outSz - firstOutSz), MSGID_USERAUTH_FAILURE); + + FreeChannelOpenHarness(&harness); +} + +#endif /* WOLFSSH_KEYBOARD_INTERACTIVE */ + /* Needs server, client, key files, and one covered host-key algorithm. */ #if !defined(NO_WOLFSSH_SERVER) && !defined(NO_WOLFSSH_CLIENT) && \ !defined(NO_FILESYSTEM) && \ @@ -5097,6 +5194,10 @@ int main(int argc, char** argv) TestSecondSessionChannelRejected(); TestUsernameChangeDisconnects(); TestSameUserRetryAllowed(); +#ifdef WOLFSSH_KEYBOARD_INTERACTIVE + TestKbInfoResponseCountMismatchSendsFailure(); + TestKbInfoResponseMismatchKeepsFraming(); +#endif #ifdef WOLFSSH_FWD TestDirectTcpipRejectSendsOpenFail(); TestDirectTcpipNoFwdCbSendsOpenFail();