fix: clear userauth state on bad service

A USERAUTH_REQUEST naming a service other than "ssh-connection" is
refused before method dispatch, so it drops the per-method state there
too. A following USERAUTH_INFO_RESPONSE is no longer run against a
keyboard-interactive exchange the server has already failed.

- reset ssh->authId and clear ssh->kbSetupPending on the refusal
- charge the abandoned exchange, as the dispatch path does
- leave ssh->kbAuth alone; clients free those buffers by promptCount
- add wolfSSH_TestDoUserAuthInfoResponse() for the tests
- cover the refusal and the stale INFO_RESPONSE in tests/unit.c

Issue: F-10575
pull/1202/head
John Safranek 2026-08-27 11:21:05 -07:00 committed by philljj
parent 89b8055a0a
commit a98fbddb05
3 changed files with 164 additions and 12 deletions

View File

@ -10445,7 +10445,22 @@ static int DoUserAuthRequest(WOLFSSH* ssh,
!= ID_SERVICE_CONNECTION) {
WLOG(WS_LOG_DEBUG, "DUAR: Invalid service name");
serviceValid = 0;
ret = SendUserAuthFailureCount(ssh, 0, 1);
/* Refused before method dispatch, which normally updates this
* state. Drop it here too, else a later INFO_RESPONSE runs
* against the outstanding exchange. */
ssh->authId = ID_NONE;
#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
/* Charge the exchange this refusal abandons, as the dispatch
* path does; else a peer stretches the cap by alternating
* keyboard requests with bad service names. */
if (ssh->kbSetupPending) {
ssh->kbSetupPending = 0;
ret = CountUserAuthFailure(ssh);
}
#endif
if (ret == WS_SUCCESS) {
ret = SendUserAuthFailureCount(ssh, 0, 1);
}
/* Consume all remaining data */
*idx = len;
}
@ -23179,6 +23194,14 @@ int wolfSSH_TestDoUserAuthRequest(WOLFSSH* ssh, byte* buf, word32 len,
return DoUserAuthRequest(ssh, buf, len, idx);
}
#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
int wolfSSH_TestDoUserAuthInfoResponse(WOLFSSH* ssh, byte* buf, word32 len,
word32* idx)
{
return DoUserAuthInfoResponse(ssh, buf, len, idx);
}
#endif
int wolfSSH_TestSendUserAuthFailure(WOLFSSH* ssh, byte partialSuccess)
{
return SendUserAuthFailure(ssh, partialSuccess);

View File

@ -7654,6 +7654,10 @@ static int UnitAuthAlwaysFail(byte authType, WS_UserAuthData* authData,
}
#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
/* Set when the callback sees a response, so a processed INFO_RESPONSE is
* visible even when rejected. */
static int s_kbRespCbCalled = 0;
/* Keyboard setup that accepts, sending an INFO_REQUEST with no prompts so the
* exchange goes outstanding (kbSetupPending). Every other outcome fails. */
static int UnitAuthKbSetupOk(byte authType, WS_UserAuthData* authData,
@ -7664,25 +7668,31 @@ static int UnitAuthKbSetupOk(byte authType, WS_UserAuthData* authData,
authData->sf.keyboard.promptCount = 0;
return WOLFSSH_USERAUTH_SUCCESS;
}
if (authType == WOLFSSH_USERAUTH_KEYBOARD) {
s_kbRespCbCalled = 1;
}
return WOLFSSH_USERAUTH_INVALID_PASSWORD;
}
/* Build a USERAUTH_REQUEST payload for auth method `method` with no method-
* specific fields, as DoUserAuthRequest() sees it with the message id already
* consumed. The keyboard and none dispatch paths don't parse trailing fields,
* so three strings (user, service, method) suffice. Returns the payload size,
* 0 on overflow. */
/* Build a USERAUTH_REQUEST payload for `service` and auth method `method`
* with no method-specific fields, as DoUserAuthRequest() sees it with the
* message id already consumed. The keyboard and none dispatch paths don't
* parse trailing fields, so three strings (user, service, method) suffice. A
* NULL method omits that field, which a bad service name never reaches.
* Returns the payload size, 0 on overflow. */
static word32 BuildAuthMethodRequest(byte* buf, word32 bufSz,
const char* method)
const char* service, const char* method)
{
const char* fields[3];
word32 idx = 0, i, fieldSz;
word32 idx = 0, i, fieldCount;
word32 fieldSz;
fields[0] = "jill";
fields[1] = "ssh-connection";
fields[1] = service;
fields[2] = method;
fieldCount = (method == NULL) ? 2 : 3;
for (i = 0; i < 3; i++) {
for (i = 0; i < fieldCount; i++) {
fieldSz = (word32)WSTRLEN(fields[i]);
if (idx + UINT32_SZ + fieldSz > bufSz)
return 0;
@ -7857,7 +7867,7 @@ static int test_MaxAuthAttempts(void)
WOLFSSH* ssh = NULL;
byte kbReq[64];
word32 kbReqSz = BuildAuthMethodRequest(kbReq, (word32)sizeof(kbReq),
"keyboard-interactive");
"ssh-connection", "keyboard-interactive");
if (kbReqSz == 0)
result = -750;
@ -7906,7 +7916,7 @@ static int test_MaxAuthAttempts(void)
WOLFSSH* ssh = NULL;
byte kbReq[64];
word32 kbReqSz = BuildAuthMethodRequest(kbReq, (word32)sizeof(kbReq),
"keyboard-interactive");
"ssh-connection", "keyboard-interactive");
int attempt;
if (kbReqSz == 0)
@ -8124,6 +8134,114 @@ done:
}
#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
/* Counts KEYBOARD (response) callbacks, so a processed INFO_RESPONSE is
* visible even when rejected. */
/* A USERAUTH_REQUEST naming a service other than "ssh-connection" must not
* leave a keyboard-interactive exchange outstanding.
*
* Asserts, in order: an accepted keyboard request leaves the exchange
* outstanding; a bad-service request clears authId and kbSetupPending; a
* following INFO_RESPONSE is rejected without reaching the callback. */
static int test_DoUserAuthRequest_badServiceClearsKb(void)
{
WOLFSSH_CTX* ctx = NULL;
WOLFSSH* ssh = NULL;
byte kbReq[64], badReq[64], infoRsp[UINT32_SZ];
word32 kbReqSz, badReqSz, idx;
int result = 0;
int ret;
kbReqSz = BuildAuthMethodRequest(kbReq, (word32)sizeof(kbReq),
"ssh-connection", "keyboard-interactive");
badReqSz = BuildAuthMethodRequest(badReq, (word32)sizeof(badReq),
"ssh-userauth", NULL);
if (kbReqSz == 0 || badReqSz == 0)
return -800;
/* INFO_RESPONSE body: response count of 0, matching promptCount. */
PutU32BE(infoRsp, 0);
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
if (ctx == NULL)
return -801;
wolfSSH_SetUserAuth(ctx, UnitAuthKbSetupOk);
wolfSSH_SetIOSend(ctx, UnitIoSendSink);
ssh = wolfSSH_new(ctx);
if (ssh == NULL) {
wolfSSH_CTX_free(ctx);
return -802;
}
s_kbRespCbCalled = 0;
idx = 0;
ret = wolfSSH_TestDoUserAuthRequest(ssh, kbReq, kbReqSz, &idx);
if (ret != WS_SUCCESS) {
printf("badServiceClearsKb: kb request ret=%d expected %d\n",
ret, WS_SUCCESS);
result = -803;
}
if (result == 0 && (ssh->authId != ID_USERAUTH_KEYBOARD
|| !ssh->kbSetupPending)) {
printf("badServiceClearsKb: kb exchange not outstanding "
"(authId=%d pending=%d)\n", ssh->authId, ssh->kbSetupPending);
result = -804;
}
if (result == 0) {
idx = 0;
ret = wolfSSH_TestDoUserAuthRequest(ssh, badReq, badReqSz, &idx);
if (ret != WS_SUCCESS) {
printf("badServiceClearsKb: bad service ret=%d expected %d\n",
ret, WS_SUCCESS);
result = -805;
}
}
if (result == 0 && ssh->authId == ID_USERAUTH_KEYBOARD) {
printf("badServiceClearsKb: authId still ID_USERAUTH_KEYBOARD after "
"bad service name\n");
result = -806;
}
if (result == 0 && ssh->kbSetupPending) {
printf("badServiceClearsKb: kbSetupPending still set after bad "
"service name\n");
result = -807;
}
/* One charge for the refusal, one for the exchange it abandoned. */
if (result == 0 && ssh->authFailures != 2) {
printf("badServiceClearsKb: authFailures=%u expected 2\n",
ssh->authFailures);
result = -810;
}
if (result == 0) {
idx = 0;
ret = wolfSSH_TestDoUserAuthInfoResponse(ssh, infoRsp,
(word32)sizeof(infoRsp), &idx);
if (ret != WS_FATAL_ERROR) {
printf("badServiceClearsKb: stale INFO_RESPONSE ret=%d "
"expected %d\n", ret, WS_FATAL_ERROR);
result = -808;
}
}
if (result == 0 && s_kbRespCbCalled) {
printf("badServiceClearsKb: userauth callback reached by stale "
"INFO_RESPONSE\n");
result = -809;
}
wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
return result;
}
#endif /* WOLFSSH_KEYBOARD_INTERACTIVE */
/* userauth callback that records whether it was invoked. Returns SUCCESS so
* that, if it were ever reached for a password-change request, the request
* would be (incorrectly) authenticated - making a missed rejection visible. */
@ -17705,6 +17823,13 @@ int wolfSSH_UnitTest(int argc, char** argv)
(unitResult == 0 ? "SUCCESS" : "FAILED"));
testResult = testResult || unitResult;
#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
unitResult = test_DoUserAuthRequest_badServiceClearsKb();
printf("DoUserAuthRequest_badServiceClearsKb: %s\n",
(unitResult == 0 ? "SUCCESS" : "FAILED"));
testResult = testResult || unitResult;
#endif
unitResult = test_DoUserAuthRequest_rejectsPasswordChange();
printf("DoUserAuthRequest_rejectsPasswordChange: %s\n",
(unitResult == 0 ? "SUCCESS" : "FAILED"));

View File

@ -1858,6 +1858,10 @@ enum WS_MessageIdLimits {
const byte* src, word32 srcSz);
WOLFSSH_API int wolfSSH_TestDoUserAuthRequest(WOLFSSH* ssh, byte* buf,
word32 len, word32* idx);
#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
WOLFSSH_API int wolfSSH_TestDoUserAuthInfoResponse(WOLFSSH* ssh, byte* buf,
word32 len, word32* idx);
#endif
WOLFSSH_API int wolfSSH_TestSendUserAuthFailure(WOLFSSH* ssh,
byte partialSuccess);
WOLFSSH_API int wolfSSH_TestHighwaterCheck(WOLFSSH* ssh, byte side);