internal: refuse an unsolicited INFO_RESPONSE

src/internal.c

@@ -10282,9 +10282,13 @@ static int DoUserAuthInfoResponse(WOLFSSH* ssh,
         ret = WS_FATAL_ERROR;
     }
 
+    if ((ret == WS_SUCCESS) && !ssh->kbSetupPending) {
+        WLOG(WS_LOG_DEBUG, "DoUserAuthInfoResponse with no INFO_REQUEST "
+            "outstanding");
+        ret = WS_FATAL_ERROR;
+    }
+
     if (ret == WS_SUCCESS) {
-        /* Response arrived; the exchange is answered. */
-        ssh->kbSetupPending = 0;
         WMEMSET(&authData, 0, sizeof(authData));
         begin = *idx;
         kb = &authData.sf.keyboard;
@@ -10395,6 +10399,11 @@ static int DoUserAuthInfoResponse(WOLFSSH* ssh,
         *idx = begin;
     }
 
+    if (ssh != NULL && ret != WS_AUTH_PENDING) {
+        /* Answered; a pending one is redispatched and must pass the gate. */
+        ssh->kbSetupPending = 0;
+    }
+
     if (authFailure || partialSuccess) {
         /* Charge every non-authenticating outcome: an uncounted partial
          * success replays forever, and a rejection isn't fatal on its own if
@@ -10406,6 +10415,10 @@ static int DoUserAuthInfoResponse(WOLFSSH* ssh,
     }
     else if (ret == WOLFSSH_USERAUTH_SUCCESS_ANOTHER) {
         ret = SendUserAuthKeyboardRequest(ssh, &authData);
+        if (ret == WS_AUTH_PENDING) {
+            /* Setup blocked; this response is redispatched. */
+            ssh->kbSetupPending = 1;
+        }
     }
     else if (ret == WS_SUCCESS) {
         ssh->clientState = CLIENT_USERAUTH_DONE;
@@ -20632,6 +20645,7 @@ int SendUserAuthKeyboardRequest(WOLFSSH* ssh, WS_UserAuthData* authData)
     word32 idx;
     word32 payloadSz = 0;
     int ret = WS_SUCCESS;
+    int delivered = 0;
 
     WLOG(WS_LOG_DEBUG, "Entering SendUserAuthKeyboardRequest()");
 
@@ -20741,11 +20755,15 @@ int SendUserAuthKeyboardRequest(WOLFSSH* ssh, WS_UserAuthData* authData)
     }
 
     if (ret == WS_SUCCESS) {
+        word32 flushes = ssh->txFlushCount;
+
         ret = wolfSSH_SendPacket(ssh);
+        delivered = SendPacketDelivered(ssh, flushes, ret);
     }
 
-    if ((ret == WS_WANT_WRITE) || (ret == WS_SUCCESS)) {
-        /* INFO_REQUEST sent or buffered; the exchange is now outstanding. */
+    if (delivered) {
+        /* INFO_REQUEST sent or buffered, whatever the highwater check made
+         * of it; the exchange is now outstanding. */
         ssh->kbSetupPending = 1;
     }
     else {

tests/regress.c

@@ -590,6 +590,7 @@ static void InitKbInfoResponseHarness(ChannelOpenHarness* harness,
     harness->ssh->acceptState = ACCEPT_CLIENT_USERAUTH_REQUEST_DONE;
     harness->ssh->authId = ID_USERAUTH_KEYBOARD;
     harness->ssh->kbAuth.promptCount = promptCount;
+    harness->ssh->kbSetupPending = 1;
 }
 
 /* Post-condition for a rejected INFO_RESPONSE: the transport survives and the
@@ -646,6 +647,8 @@ static void TestKbInfoResponseMismatchKeepsFraming(void)
             MSGID_USERAUTH_FAILURE);
     firstOutSz = harness.io.outSz;
 
+    /* As if a second INFO_REQUEST went out. */
+    harness.ssh->kbSetupPending = 1;
     ret = DoReceive(harness.ssh);
     AssertIntEQ(ret, WS_SUCCESS);
     AssertIntEQ(harness.io.inOff, harness.io.inSz);

tests/unit.c

@@ -13528,6 +13528,279 @@ static int test_DoUserAuthRequest_badServiceClearsKb(void)
 #endif /* WOLFSSH_KEYBOARD_INTERACTIVE */
 
 
+#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
+static int s_kbStaleSetupRet = WOLFSSH_USERAUTH_SUCCESS;
+static int s_kbStaleRespRet = WOLFSSH_USERAUTH_INVALID_PASSWORD;
+static int s_kbStaleRespCalls = 0;
+static word32 s_kbStalePromptLen = 9;
+static byte s_kbStalePromptEcho = 0;
+static byte* s_kbStalePrompts[1] = { (byte*)"Password:" };
+
+/* Setup returns s_kbStaleSetupRet with one prompt; each response is counted
+ * and answered with s_kbStaleRespRet. */
+static int UnitAuthKbStale(byte authType, WS_UserAuthData* authData,
+        void* ctx)
+{
+    (void)ctx;
+    if (authType == WOLFSSH_USERAUTH_KEYBOARD_SETUP) {
+        authData->sf.keyboard.promptCount = 1;
+        authData->sf.keyboard.promptLengths = &s_kbStalePromptLen;
+        authData->sf.keyboard.prompts = s_kbStalePrompts;
+        authData->sf.keyboard.promptEcho = &s_kbStalePromptEcho;
+        return s_kbStaleSetupRet;
+    }
+    if (authType == WOLFSSH_USERAUTH_KEYBOARD) {
+        s_kbStaleRespCalls++;
+        return s_kbStaleRespRet;
+    }
+    return WOLFSSH_USERAUTH_FAILURE;
+}
+
+/* A server session with UnitAuthKbStale. When kbRequest is set, it has
+ * taken a keyboard-interactive USERAUTH_REQUEST. */
+static WOLFSSH* NewKbStaleSession(WOLFSSH_CTX** ctx, int kbRequest,
+        int setupRet, int respRet)
+{
+    WOLFSSH* ssh;
+    byte kbReq[64];
+    word32 kbReqSz, idx = 0;
+
+    s_kbStaleSetupRet = setupRet;
+    s_kbStaleRespRet = respRet;
+    s_kbStaleRespCalls = 0;
+
+    kbReqSz = BuildAuthMethodRequest(kbReq, (word32)sizeof(kbReq),
+            "ssh-connection", "keyboard-interactive");
+    if (kbReqSz == 0)
+        return NULL;
+    *ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
+    if (*ctx == NULL)
+        return NULL;
+    wolfSSH_SetUserAuth(*ctx, UnitAuthKbStale);
+    wolfSSH_SetIOSend(*ctx, UnitIoSendSink);
+    ssh = wolfSSH_new(*ctx);
+    if (ssh != NULL && kbRequest &&
+            wolfSSH_TestDoUserAuthRequest(ssh, kbReq, kbReqSz, &idx)
+                != WS_SUCCESS) {
+        wolfSSH_free(ssh);
+        ssh = NULL;
+    }
+    if (ssh == NULL) {
+        wolfSSH_CTX_free(*ctx);
+        *ctx = NULL;
+    }
+    return ssh;
+}
+
+/* Feed one INFO_RESPONSE with responseCount responses of "x". */
+static int KbStaleInfoResponse(WOLFSSH* ssh, word32 responseCount)
+{
+    byte infoRsp[UINT32_SZ * 2 + 1];
+    word32 infoRspSz = UINT32_SZ, idx = 0;
+
+    PutU32BE(infoRsp, responseCount);
+    if (responseCount == 1) {
+        PutU32BE(infoRsp + UINT32_SZ, 1);
+        infoRsp[UINT32_SZ * 2] = 'x';
+        infoRspSz = (word32)sizeof(infoRsp);
+    }
+    return wolfSSH_TestDoUserAuthInfoResponse(ssh, infoRsp, infoRspSz, &idx);
+}
+
+/* An INFO_RESPONSE with no INFO_REQUEST outstanding is refused before the
+ * callback. kbRequest sends a keyboard-interactive request first;
+ * answerFirst answers its INFO_REQUEST once. Returns 0 or a negative code. */
+static int RunStaleInfoResponse(const char* label, int kbRequest,
+        int answerFirst, int setupRet, int respRet, word32 staleCount)
+{
+    WOLFSSH_CTX* ctx = NULL;
+    WOLFSSH* ssh;
+    int result = 0;
+    int ret;
+
+    ssh = NewKbStaleSession(&ctx, kbRequest, setupRet, respRet);
+    if (ssh == NULL)
+        return -900;
+
+    if (answerFirst) {
+        (void)KbStaleInfoResponse(ssh, 1);
+        if (s_kbStaleRespCalls != 1) {
+            printf("StaleInfoResponse(%s): solicited response calls=%d "
+                   "expected 1\n", label, s_kbStaleRespCalls);
+            result = -901;
+        }
+    }
+
+    if (result == 0) {
+        s_kbStaleRespCalls = 0;
+        ret = KbStaleInfoResponse(ssh, staleCount);
+        if (ret != WS_FATAL_ERROR) {
+            printf("StaleInfoResponse(%s): ret=%d expected %d\n",
+                   label, ret, WS_FATAL_ERROR);
+            result = -902;
+        }
+        else if (s_kbStaleRespCalls != 0) {
+            printf("StaleInfoResponse(%s): userauth callback reached\n",
+                   label);
+            result = -903;
+        }
+        else if (ssh->clientState == CLIENT_USERAUTH_DONE) {
+            printf("StaleInfoResponse(%s): session authenticated\n", label);
+            result = -904;
+        }
+    }
+
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+    return result;
+}
+
+/* A response kept for redispatch on WS_AUTH_PENDING still passes the gate:
+ * a blocked check (anotherRound 0), or a blocked second-round setup
+ * (anotherRound 1). Returns 0 or a negative code. */
+static int RunPendingInfoResponse(const char* label, int anotherRound)
+{
+    WOLFSSH_CTX* ctx = NULL;
+    WOLFSSH* ssh;
+    int result = 0;
+    int ret;
+
+    ssh = NewKbStaleSession(&ctx, 1, WOLFSSH_USERAUTH_SUCCESS,
+            anotherRound ? WOLFSSH_USERAUTH_SUCCESS_ANOTHER
+                         : WOLFSSH_USERAUTH_WOULD_BLOCK);
+    if (ssh == NULL)
+        return -910;
+    if (anotherRound)
+        s_kbStaleSetupRet = WOLFSSH_USERAUTH_WOULD_BLOCK;
+
+    ret = KbStaleInfoResponse(ssh, 1);
+    if (ret != WS_AUTH_PENDING || !ssh->kbSetupPending) {
+        printf("PendingInfoResponse(%s): ret=%d pending=%d expected %d, 1\n",
+               label, ret, ssh->kbSetupPending, WS_AUTH_PENDING);
+        result = -911;
+    }
+
+    if (result == 0) {
+        /* Redispatch of the kept packet. */
+        s_kbStaleSetupRet = WOLFSSH_USERAUTH_SUCCESS;
+        if (!anotherRound)
+            s_kbStaleRespRet = WOLFSSH_USERAUTH_SUCCESS;
+        ret = KbStaleInfoResponse(ssh, 1);
+        if (ret != WS_SUCCESS) {
+            printf("PendingInfoResponse(%s): retry ret=%d expected %d\n",
+                   label, ret, WS_SUCCESS);
+            result = -912;
+        }
+    }
+
+    if (result == 0 && anotherRound) {
+        /* The retry sent the second INFO_REQUEST; answer it. */
+        if (!ssh->kbSetupPending) {
+            printf("PendingInfoResponse(%s): second round not outstanding\n",
+                   label);
+            result = -913;
+        }
+        else {
+            s_kbStaleRespRet = WOLFSSH_USERAUTH_SUCCESS;
+            ret = KbStaleInfoResponse(ssh, 1);
+            if (ret != WS_SUCCESS) {
+                printf("PendingInfoResponse(%s): second round ret=%d "
+                       "expected %d\n", label, ret, WS_SUCCESS);
+                result = -914;
+            }
+        }
+    }
+
+    if (result == 0 && (ssh->clientState != CLIENT_USERAUTH_DONE
+                        || ssh->kbSetupPending)) {
+        printf("PendingInfoResponse(%s): clientState=%d pending=%d after "
+               "success\n", label, ssh->clientState, ssh->kbSetupPending);
+        result = -915;
+    }
+
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+    return result;
+}
+
+/* The highwater callback runs once INFO_REQUEST is out, so its error leaves
+ * the exchange outstanding and the response is still taken. Returns 0 or a
+ * negative code. */
+static int RunHighwaterInfoRequest(void)
+{
+    WOLFSSH_CTX* ctx = NULL;
+    WOLFSSH* ssh;
+    byte kbReq[64];
+    word32 kbReqSz, idx = 0;
+    int result = 0;
+    int ret;
+
+    ssh = NewKbStaleSession(&ctx, 0, WOLFSSH_USERAUTH_SUCCESS,
+            WOLFSSH_USERAUTH_SUCCESS);
+    if (ssh == NULL)
+        return -920;
+    /* Level 0 leaves the byte count off; the first packet trips the
+     * message count. */
+    wolfSSH_SetHighwaterCb(ctx, 0, FailHighwater);
+    ssh->msgHighwaterMark = 1;
+
+    kbReqSz = BuildAuthMethodRequest(kbReq, (word32)sizeof(kbReq),
+            "ssh-connection", "keyboard-interactive");
+    ret = wolfSSH_TestDoUserAuthRequest(ssh, kbReq, kbReqSz, &idx);
+    if (ret != WS_FATAL_ERROR || !ssh->kbSetupPending) {
+        printf("HighwaterInfoRequest: ret=%d pending=%d expected %d, 1\n",
+               ret, ssh->kbSetupPending, WS_FATAL_ERROR);
+        result = -921;
+    }
+
+    if (result == 0) {
+        ret = KbStaleInfoResponse(ssh, 1);
+        if (ret != WS_SUCCESS || s_kbStaleRespCalls != 1) {
+            printf("HighwaterInfoRequest: response ret=%d calls=%d "
+                   "expected %d, 1\n", ret, s_kbStaleRespCalls, WS_SUCCESS);
+            result = -922;
+        }
+    }
+
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+    return result;
+}
+
+static int test_DoUserAuthInfoResponse_unsolicited(void)
+{
+    int result = 0, r;
+
+    /* No keyboard-interactive request at all. */
+    r = RunStaleInfoResponse("no request", 0, 0,
+            WOLFSSH_USERAUTH_SUCCESS, WOLFSSH_USERAUTH_SUCCESS, 0);
+    if (result == 0) result = r;
+    /* Setup declined, so no INFO_REQUEST went out. A callback that accepts
+     * would authenticate the session. */
+    r = RunStaleInfoResponse("setup declined", 1, 0,
+            WOLFSSH_USERAUTH_FAILURE, WOLFSSH_USERAUTH_SUCCESS, 0);
+    if (result == 0) result = r;
+    /* A round answered and failed; a second response replays it. */
+    r = RunStaleInfoResponse("after failed round", 1, 1,
+            WOLFSSH_USERAUTH_SUCCESS, WOLFSSH_USERAUTH_INVALID_PASSWORD, 1);
+    if (result == 0) result = r;
+    /* A round answered with partial success; a second response is checked
+     * again without a new INFO_REQUEST. */
+    r = RunStaleInfoResponse("after partial success", 1, 1,
+            WOLFSSH_USERAUTH_SUCCESS, WOLFSSH_USERAUTH_PARTIAL_SUCCESS, 1);
+    if (result == 0) result = r;
+    r = RunPendingInfoResponse("check would block", 0);
+    if (result == 0) result = r;
+    r = RunPendingInfoResponse("setup would block", 1);
+    if (result == 0) result = r;
+    r = RunHighwaterInfoRequest();
+    if (result == 0) result = r;
+
+    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. */
@@ -26750,6 +27023,11 @@ int wolfSSH_UnitTest(int argc, char** argv)
     printf("DoUserAuthRequest_badServiceClearsKb: %s\n",
            (unitResult == 0 ? "SUCCESS" : "FAILED"));
     testResult = testResult || unitResult;
+
+    unitResult = test_DoUserAuthInfoResponse_unsolicited();
+    printf("DoUserAuthInfoResponse_unsolicited: %s\n",
+           (unitResult == 0 ? "SUCCESS" : "FAILED"));
+    testResult = testResult || unitResult;
 #endif
 
     unitResult = test_DoUserAuthRequest_rejectsPasswordChange();