userauth: fix auth-cap counting gaps

src/internal.c

@@ -8014,6 +8014,11 @@ static int DoUserAuthRequestNone(WOLFSSH* ssh, WS_UserAuthData* authData,
     }
 
     if (ret == WS_SUCCESS) {
+        /* The opening "none" probe every client sends to learn the method list
+         * is exempt from the auth-attempt cap, matching the invalid-method
+         * else-branch in DoUserAuthRequest() and the documented contract. */
+        int countIt = (ssh->authRequests != 1);
+
         authData->type = WOLFSSH_USERAUTH_NONE;
         if (ssh->ctx->userAuthCb != NULL) {
             WLOG(WS_LOG_DEBUG, "DUARN: Calling the userauth callback");
@@ -8029,7 +8034,7 @@ static int DoUserAuthRequestNone(WOLFSSH* ssh, WS_UserAuthData* authData,
                 #ifndef NO_FAILURE_ON_REJECTED
                 /* Count before failing: a send that blocks returns
                  * WS_WANT_WRITE, which isn't fatal on its own. */
-                (void)SendUserAuthFailureCount(ssh, 0, 1);
+                (void)SendUserAuthFailureCount(ssh, 0, countIt);
                 #endif
                 ret = WS_USER_AUTH_E;
             }
@@ -8039,12 +8044,12 @@ static int DoUserAuthRequestNone(WOLFSSH* ssh, WS_UserAuthData* authData,
             }
             else {
                 WLOG(WS_LOG_DEBUG, "DUARN: none check failed, retry");
-                ret = SendUserAuthFailureCount(ssh, 0, 1);
+                ret = SendUserAuthFailureCount(ssh, 0, countIt);
             }
         }
         else {
             WLOG(WS_LOG_DEBUG, "DUARN: No user auth callback");
-            (void)SendUserAuthFailureCount(ssh, 0, 1);
+            (void)SendUserAuthFailureCount(ssh, 0, countIt);
             ret = WS_FATAL_ERROR;
         }
     }
@@ -8202,7 +8207,7 @@ static int DoUserAuthInfoResponse(WOLFSSH* ssh,
         }
     }
     else if (ret == WOLFSSH_USERAUTH_SUCCESS_ANOTHER) {
-        ret = SendUserAuthKeyboardRequest(ssh, &authData, 0);
+        ret = SendUserAuthKeyboardRequest(ssh, &authData);
     }
     else if (ret == WS_SUCCESS) {
         ssh->clientState = CLIENT_USERAUTH_DONE;
@@ -9781,25 +9786,31 @@ static int DoUserAuthRequest(WOLFSSH* ssh,
         ssh->authId = authNameId;
         ssh->authRequests++;
 
+#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
+        /* An outstanding INFO_REQUEST the peer abandons is charged as a failed
+         * attempt no matter which method it switches to next; charging only
+         * inside the keyboard branch below let a peer dodge the cap by
+         * interleaving abandoned keyboard exchanges with other requests. The
+         * new request dispatched below still counts its own failure. */
+        if (ssh->kbSetupPending) {
+            ssh->kbSetupPending = 0;
+            ret = CountUserAuthFailure(ssh);
+        }
+#endif
+
         /* Each method branch counts its own failures; it can't be centralized
          * here since handlers return WS_SUCCESS for both success and
          * failure-sent. */
-        if (authNameId == ID_USERAUTH_PASSWORD)
+        if (ret != WS_SUCCESS) {
+            /* Charging the abandoned keyboard exchange hit the cap and the
+             * disconnect is already sent; don't dispatch this request. */
+            begin = len;
+        }
+        else if (authNameId == ID_USERAUTH_PASSWORD)
             ret = DoUserAuthRequestPassword(ssh, &authData, buf, len, &begin);
 #ifdef WOLFSSH_KEYBOARD_INTERACTIVE
         else if (authNameId == ID_USERAUTH_KEYBOARD) {
-            int counted = 0;
-
-            /* Restarting before answering the prior INFO_REQUEST counts as a
-             * failure; else a peer loops here without ever responding. Clear
-             * the flag first so the send below doesn't charge it twice. */
-            if (ssh->kbSetupPending) {
-                ssh->kbSetupPending = 0;
-                counted = 1;
-                ret = CountUserAuthFailure(ssh);
-            }
-            if (ret == WS_SUCCESS)
-                ret = SendUserAuthKeyboardRequest(ssh, &authData, counted);
+            ret = SendUserAuthKeyboardRequest(ssh, &authData);
         }
 #endif
 #if !defined(WOLFSSH_NO_RSA) || !defined(WOLFSSH_NO_ECDSA) \
@@ -16242,8 +16253,7 @@ static int BuildUserAuthRequestKeyboard(WOLFSSH* ssh, byte* output, word32* idx,
     return ret;
 }
 
-int SendUserAuthKeyboardRequest(WOLFSSH* ssh, WS_UserAuthData* authData,
-        int counted)
+int SendUserAuthKeyboardRequest(WOLFSSH* ssh, WS_UserAuthData* authData)
 {
     byte* output;
     word32 idx;
@@ -16279,10 +16289,10 @@ int SendUserAuthKeyboardRequest(WOLFSSH* ssh, WS_UserAuthData* authData,
         else {
             WLOG(WS_LOG_DEBUG, "Issue with keyboard auth setup, try another "
                 "auth type");
-            /* No INFO_REQUEST sent, so charge the rejection here unless the
-             * caller already did; else a peer loops unbounded. */
+            /* No INFO_REQUEST sent, so charge the setup rejection here; else a
+             * peer loops unbounded on rejected setups. */
             ssh->kbSetupPending = 0;
-            return SendUserAuthFailureCount(ssh, 0, !counted);
+            return SendUserAuthFailureCount(ssh, 0, 1);
         }
     }
 

tests/api.c

@@ -4004,6 +4004,18 @@ static void test_wolfSSH_CheckAlgoList(void)
 #ifndef WOLFSSH_ALLOW_NONE_CIPHER
     AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, "none"), WS_INVALID_ALGO_ID);
     AssertIntEQ(wolfSSH_CTX_SetAlgoListMac(ctx, "none"), WS_INVALID_ALGO_ID);
+#else
+    /* With --enable-none-cipher, "none" is a valid cipher/MAC selection (an
+     * insecure, testing-only plaintext transport) and must round-trip through
+     * the setters. The setters store the caller's pointer, so a static literal
+     * is used and a real list restored below before listBuf goes out of scope. */
+    AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, "none"), WS_SUCCESS);
+    AssertIntEQ(wolfSSH_CTX_SetAlgoListMac(ctx, "none"), WS_SUCCESS);
+    AssertStrEQ(wolfSSH_CTX_GetAlgoListCipher(ctx), "none");
+    AssertStrEQ(wolfSSH_CTX_GetAlgoListMac(ctx), "none");
+    /* "none" mixed into a list is still accepted alongside a real cipher. */
+    WSNPRINTF(listBuf, sizeof(listBuf), "none,%s", validCipher);
+    AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf), WS_SUCCESS);
 #endif
 
     /* Restore a static list; the setters store the caller's pointer and

tests/unit.c

@@ -3783,6 +3783,48 @@ static int UnitAuthAlwaysFail(byte authType, WS_UserAuthData* authData,
     return WOLFSSH_USERAUTH_INVALID_PASSWORD;
 }
 
+#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
+/* 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,
+        void* ctx)
+{
+    (void)ctx;
+    if (authType == WOLFSSH_USERAUTH_KEYBOARD_SETUP) {
+        authData->sf.keyboard.promptCount = 0;
+        return WOLFSSH_USERAUTH_SUCCESS;
+    }
+    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. */
+static word32 BuildAuthMethodRequest(byte* buf, word32 bufSz,
+        const char* method)
+{
+    const char* fields[3];
+    word32 idx = 0, i, fieldSz;
+
+    fields[0] = "jill";
+    fields[1] = "ssh-connection";
+    fields[2] = method;
+
+    for (i = 0; i < 3; i++) {
+        fieldSz = (word32)WSTRLEN(fields[i]);
+        if (idx + UINT32_SZ + fieldSz > bufSz)
+            return 0;
+        PutU32BE(buf + idx, fieldSz);
+        idx += UINT32_SZ;
+        WMEMCPY(buf + idx, fields[i], fieldSz);
+        idx += fieldSz;
+    }
+    return idx;
+}
+#endif /* WOLFSSH_KEYBOARD_INTERACTIVE */
+
 /* Build a "password" USERAUTH_REQUEST payload, as DoUserAuthRequest() sees it
  * with the message id already consumed. Returns the payload size. */
 static word32 BuildAuthPwRequest(byte* buf, word32 bufSz)
@@ -3874,6 +3916,160 @@ static int test_MaxAuthAttempts(void)
         wolfSSH_CTX_free(ctx);
     }
 
+    /* The opening "none" probe clients send to learn the method list is exempt
+     * from the cap. With the limit at 1: the first "none" must leave the
+     * connection up (not counted), and the second must be counted and trip the
+     * cap. This covers the invalid-method else-branch, and DoUserAuthRequestNone
+     * under WOLFSSH_ALLOW_USERAUTH_NONE, both of which share the exemption. */
+    if (result == 0) {
+        WOLFSSH_CTX* ctx = NULL;
+        WOLFSSH* ssh = NULL;
+        byte noneReq[64];
+        word32 noneReqSz = 0;
+        const char* fields[3];
+        word32 idx = 0, f, fieldSz;
+
+        fields[0] = "jill"; fields[1] = "ssh-connection"; fields[2] = "none";
+        for (f = 0; f < 3; f++) {
+            fieldSz = (word32)WSTRLEN(fields[f]);
+            PutU32BE(noneReq + idx, fieldSz);
+            idx += UINT32_SZ;
+            WMEMCPY(noneReq + idx, fields[f], fieldSz);
+            idx += fieldSz;
+        }
+        noneReqSz = idx;
+
+        ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
+        if (ctx == NULL) { result = -740; }
+        else {
+            wolfSSH_SetUserAuth(ctx, UnitAuthAlwaysFail);
+            wolfSSH_SetIOSend(ctx, UnitIoSendSink);
+            if (wolfSSH_CTX_SetMaxAuthAttempts(ctx, 1) != WS_SUCCESS)
+                result = -741;
+            ssh = (result == 0) ? wolfSSH_new(ctx) : NULL;
+            if (result == 0 && ssh == NULL)
+                result = -742;
+            if (result == 0) {
+                word32 di = 0;
+                int ret = wolfSSH_TestDoUserAuthRequest(ssh, noneReq,
+                        noneReqSz, &di);
+                if (ret != WS_SUCCESS) {
+                    printf("MaxAuthAttempts[none]: probe ret=%d expected %d\n",
+                           ret, WS_SUCCESS);
+                    result = -743;
+                }
+            }
+            if (result == 0) {
+                word32 di = 0;
+                int ret = wolfSSH_TestDoUserAuthRequest(ssh, noneReq,
+                        noneReqSz, &di);
+                if (ret != WS_USER_AUTH_E) {
+                    printf("MaxAuthAttempts[none]: 2nd ret=%d expected %d\n",
+                           ret, WS_USER_AUTH_E);
+                    result = -744;
+                }
+            }
+            wolfSSH_free(ssh);
+            wolfSSH_CTX_free(ctx);
+        }
+    }
+
+#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
+    /* An abandoned keyboard-interactive exchange is charged no matter which
+     * method the peer switches to next. With the limit at 2: an accepted
+     * keyboard request leaves an INFO_REQUEST outstanding (uncounted), then a
+     * failed password request charges both the abandoned exchange and itself,
+     * tripping the cap on that single password request. Before the fix the
+     * abandoned exchange escaped counting on a non-keyboard follow-up and the
+     * password returned WS_SUCCESS. */
+    if (result == 0) {
+        WOLFSSH_CTX* ctx = NULL;
+        WOLFSSH* ssh = NULL;
+        byte kbReq[64];
+        word32 kbReqSz = BuildAuthMethodRequest(kbReq, (word32)sizeof(kbReq),
+                "keyboard-interactive");
+
+        if (kbReqSz == 0)
+            result = -750;
+
+        ctx = (result == 0) ? wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL)
+                            : NULL;
+        if (result == 0 && ctx == NULL) result = -751;
+        if (result == 0) {
+            wolfSSH_SetUserAuth(ctx, UnitAuthKbSetupOk);
+            wolfSSH_SetIOSend(ctx, UnitIoSendSink);
+            if (wolfSSH_CTX_SetMaxAuthAttempts(ctx, 2) != WS_SUCCESS)
+                result = -752;
+            ssh = (result == 0) ? wolfSSH_new(ctx) : NULL;
+            if (result == 0 && ssh == NULL) result = -753;
+
+            if (result == 0) {
+                word32 di = 0;
+                int ret = wolfSSH_TestDoUserAuthRequest(ssh, kbReq, kbReqSz,
+                        &di);
+                if (ret != WS_SUCCESS) {
+                    printf("MaxAuthAttempts[kb]: setup ret=%d expected %d\n",
+                           ret, WS_SUCCESS);
+                    result = -754;
+                }
+            }
+            if (result == 0) {
+                word32 di = 0;
+                int ret = wolfSSH_TestDoUserAuthRequest(ssh, request,
+                        requestSz, &di);
+                if (ret != WS_USER_AUTH_E) {
+                    printf("MaxAuthAttempts[kb]: pw-after-abandon ret=%d "
+                           "expected %d\n", ret, WS_USER_AUTH_E);
+                    result = -755;
+                }
+            }
+            wolfSSH_free(ssh);
+            wolfSSH_CTX_free(ctx);
+        }
+    }
+
+    /* A rejected keyboard setup (no INFO_REQUEST sent) is charged. With the
+     * limit at 2: the first rejected request leaves the connection up, the
+     * second trips the cap. */
+    if (result == 0) {
+        WOLFSSH_CTX* ctx = NULL;
+        WOLFSSH* ssh = NULL;
+        byte kbReq[64];
+        word32 kbReqSz = BuildAuthMethodRequest(kbReq, (word32)sizeof(kbReq),
+                "keyboard-interactive");
+        int attempt;
+
+        if (kbReqSz == 0)
+            result = -760;
+
+        ctx = (result == 0) ? wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL)
+                            : NULL;
+        if (result == 0 && ctx == NULL) result = -761;
+        if (result == 0) {
+            wolfSSH_SetUserAuth(ctx, UnitAuthAlwaysFail); /* setup rejects */
+            wolfSSH_SetIOSend(ctx, UnitIoSendSink);
+            if (wolfSSH_CTX_SetMaxAuthAttempts(ctx, 2) != WS_SUCCESS)
+                result = -762;
+            ssh = (result == 0) ? wolfSSH_new(ctx) : NULL;
+            if (result == 0 && ssh == NULL) result = -763;
+
+            for (attempt = 1; attempt <= 2 && result == 0; attempt++) {
+                word32 di = 0;
+                int expect = (attempt < 2) ? WS_SUCCESS : WS_USER_AUTH_E;
+                int ret = wolfSSH_TestDoUserAuthRequest(ssh, kbReq, kbReqSz,
+                        &di);
+                if (ret != expect) {
+                    printf("MaxAuthAttempts[kb-reject]: attempt %d ret=%d "
+                           "expected %d\n", attempt, ret, expect);
+                    result = -764 - attempt;
+                }
+            }
+            wolfSSH_free(ssh);
+            wolfSSH_CTX_free(ctx);
+        }
+    }
+#endif /* WOLFSSH_KEYBOARD_INTERACTIVE */
+
     return result;
 }
 

wolfssh/internal.h

@@ -1322,7 +1322,7 @@ WOLFSSH_LOCAL int SendUserAuthKeyboardResponse(WOLFSSH* ssh);
  * caller already charged a failure for this message, so a declined setup
  * callback doesn't charge it twice. */
 WOLFSSH_LOCAL int SendUserAuthKeyboardRequest(WOLFSSH* ssh,
-        WS_UserAuthData* authData, int counted);
+        WS_UserAuthData* authData);
 #endif
 WOLFSSH_LOCAL int SendUserAuthSuccess(WOLFSSH* ssh);
 WOLFSSH_LOCAL int SendUserAuthFailure(WOLFSSH* ssh, byte partialSuccess);