fix: clear userauth state on bad service

src/internal.c

@@ -10427,7 +10427,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;
         }
@@ -23121,6 +23136,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);

tests/unit.c

@@ -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 @@ static int test_DoUserAuthRequest_serviceName(void)
 }
 
 
+#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"));

wolfssh/internal.h

@@ -1855,6 +1855,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);