keyboard-interactive: send USERAUTH_FAILURE on response-count mismatch

[yosuke-wolfssl]

Description

DoUserAuthInfoResponse() tore down the transport instead of sending an
SSH_MSG_USERAUTH_FAILURE when a keyboard-interactive INFO_RESPONSE carried a
responseCount that did not match the server's pending promptCount, or a count
greater than WOLFSSH_MAX_PROMPTS.

Both guards set ret = WS_USER_AUTH_E. The failure-response gate only calls
SendUserAuthFailure() when authFailure || partialSuccess is set, so no SSH
message was emitted; DoReceive() converts WS_USER_AUTH_E to WS_FATAL_ERROR
and the connection is torn down. RFC 4256 requires the server to answer every
INFO_RESPONSE with SUCCESS, FAILURE, or another INFO_REQUEST. Impact is
limited to malformed/malicious clients, but the response is non-conformant.

Addressed by f_6514.

Fix

In both guards, set authFailure so the existing gate sends
SSH_MSG_USERAUTH_FAILURE and the connection survives (the client may retry).

ret is kept non-success in these branches so the response allocation, parse,
and user-auth callback are skipped:

• a wire-controlled responseCount (up to 2^32-1) would otherwise drive a
  large allocation, and
• the auth callback must not run on a protocol-violating message.

The failure path also reports the whole payload consumed (*idx = len),
consistent with the success path's contract.

Testing

Added tests/regress.c coverage using the in-memory packet harness, which feeds
a crafted INFO_RESPONSE straight into DoReceive() and inspects the bytes the
server writes back:

• TestKbInfoResponseCountMismatchSendsFailure — responseCount (2) disagrees
  with promptCount (1).
• TestKbInfoResponseTooManySendsFailure — responseCount > WOLFSSH_MAX_PROMPTS.

Both assert DoReceive() returns WS_SUCCESS and the reply is
MSGID_USERAUTH_FAILURE (via the shared AssertKbInfoResponseSentFailure
helper).

$ ./tests/regress.test
regress: PASS
$ ./tests/auth.test    # exit 0

• Negative control: reverting the authFailure change makes the new tests
  abort at the WS_SUCCESS assertion with ret = -1001 (WS_FATAL_ERROR),
  confirming they catch the regression.
• Sanitizers: built with -fsanitize=address,undefined (leak detection off
  on macOS); regress.test is clean.

Notes

• No public API or signature changes.
• The *idx = len line is a contract-consistency safeguard. In the current
  receive path it is not behaviorally observable (DoReceive force-shrinks the
  input buffer after every packet), but it keeps the handler consistent with its
  siblings and robust to future buffer-management changes.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1070

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

[aidangarske]

🐺 Skoll Code Review

Overall recommendation: COMMENT
Findings: 1 total — 1 posted, 0 skipped

Posted findings

• [Medium] Malformed keyboard response failure leaves challenge state active — src/internal.c:7278-7385

Review generated by Skoll.

[aidangarske]

🟡 [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.