Skip to content

keyboard-interactive: send USERAUTH_FAILURE on response-count mismatch #1070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yosuke-wolfssl wants to merge 1 commit into
base: master
Choose a base branch
from
Open

keyboard-interactive: send USERAUTH_FAILURE on response-count mismatch #1070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7275,12 +7275,17 @@ static int DoUserAuthInfoResponse(WOLFSSH* ssh,
if ((ret == WS_SUCCESS) &&
(ssh->kbAuth.promptCount != kb->responseCount)) {
WLOG(WS_LOG_DEBUG, "DUARKB: Invalid number of responses received");
/* Respond with USERAUTH_FAILURE rather than

@aidangarske aidangarske Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [Medium] Malformed keyboard response failure leaves challenge state active
💡 SUGGEST bug

The new count-mismatch and too-many branches now turn a malformed INFO_RESPONSE into a successful SendUserAuthFailure() path, but the pending keyboard-interactive state is left intact. ssh->authId remains ID_USERAUTH_KEYBOARD and ssh->kbAuth.promptCount remains the old challenge count, so after receiving USERAUTH_FAILURE a malformed client can send another MSGID_USERAUTH_INFO_RESPONSE without first sending a new USERAUTH_REQUEST. Before this PR these malformed count cases tore down the connection, so this retry-without-new-request path is introduced by the change. That is at least protocol-state drift from the intended retry flow described in the PR.

Recommendation: After sending USERAUTH_FAILURE for a rejected keyboard-interactive INFO_RESPONSE, clear the pending keyboard challenge state or add an explicit pending-challenge flag so a later INFO_RESPONSE is rejected unless a fresh USERAUTH_REQUEST caused a new INFO_REQUEST. Add a regression test that sends a malformed INFO_RESPONSE, observes USERAUTH_FAILURE, then sends a second INFO_RESPONSE without a new USERAUTH_REQUEST and verifies it is not accepted as an answer to the old challenge.

* tearing down the transport. Keep ret non-success so the
* allocation, parse, and callback steps below are skipped. */
authFailure = 1;
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);
authFailure = 1;
ret = WS_USER_AUTH_E;
}

Expand Down Expand Up @@ -7370,6 +7375,11 @@ static int DoUserAuthInfoResponse(WOLFSSH* ssh,
if (ret == WS_SUCCESS || ret == WOLFSSH_USERAUTH_SUCCESS_ANOTHER) {
*idx = begin;
}
else if (authFailure) {
/* On a failure response, report the whole payload consumed, matching
* the contract of the success path above. */
*idx = len;
}

if (authFailure || partialSuccess) {
ret = SendUserAuthFailure(ssh, partialSuccess);
Expand Down
89 changes: 89 additions & 0 deletions tests/regress.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,91 @@ 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 and too-many-responses guards reject before parsing the
* body, so a short body is sufficient to drive them. */
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 response count above WOLFSSH_MAX_PROMPTS must yield a USERAUTH_FAILURE, not
* a transport teardown. promptCount matches so the mismatch guard passes and
* the too-many guard fires. */
static void TestKbInfoResponseTooManySendsFailure(void)
{
ChannelOpenHarness harness;
byte in[128];
word32 tooMany = WOLFSSH_MAX_PROMPTS + 1;
word32 inSz;
int ret;

inSz = BuildInfoResponsePacket(tooMany, 0, in, sizeof(in));
InitKbInfoResponseHarness(&harness, in, inSz, tooMany);

ret = DoReceive(harness.ssh);

AssertKbInfoResponseSentFailure(&harness, ret);

FreeChannelOpenHarness(&harness);
}
#endif /* WOLFSSH_KEYBOARD_INTERACTIVE */

#if !defined(NO_WOLFSSH_SERVER) && !defined(NO_WOLFSSH_CLIENT) && \
!defined(WOLFSSH_NO_RSA) && !defined(NO_FILESYSTEM)
#if !defined(WOLFSSH_NO_DH_GROUP14_SHA256)
Expand Down Expand Up @@ -4366,6 +4451,10 @@ int main(int argc, char** argv)
TestChannelAllowedAfterAuth(ssh);
TestChannelOpenCallbackRejectSendsOpenFail();
TestSecondSessionChannelRejected();
#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
TestKbInfoResponseCountMismatchSendsFailure();
TestKbInfoResponseTooManySendsFailure();
#endif
#ifdef WOLFSSH_FWD
TestDirectTcpipRejectSendsOpenFail();
TestDirectTcpipNoFwdCbSendsOpenFail();
Expand Down
Loading