fix: reject service messages while keying

RFC 4253 section 7.1 bars SERVICE_REQUEST and SERVICE_ACCEPT between a
KEXINIT and the matching NEWKEYS. The admission check now consults
ssh->isKeying, which acceptState and connectState do not track. It tests
WOLFSSH_PEER_IS_KEYING, as the KEXINIT gate below it does: a peer that
has not seen our KEXINIT yet may still legally send.

- gate MSGID_SERVICE_REQUEST on isKeying in IsMessageAllowedServer()
- gate MSGID_SERVICE_ACCEPT on isKeying in IsMessageAllowedClient()
- record the refusals as WS_MSGID_NOT_ALLOWED_E, the code DoReceive()
  stores anyway, rather than WS_REKEYING, a soft retry-later status
- add unit and receive-path coverage in tests/regress.c

Issue: F-10569

Author: ejohnstown

src/internal.c

@@ -710,6 +710,15 @@ INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg)
     }
 
     if (msg == MSGID_SERVICE_REQUEST) {
+        /* RFC 4253 section 7.1: no service request once the peer's KEXINIT
+         * lands. acceptState doesn't track a rekey. */
+        if (ssh->isKeying & WOLFSSH_PEER_IS_KEYING) {
+            WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+                    msg, "server", "when keying");
+            ssh->error = WS_MSGID_NOT_ALLOWED_E;
+            return 0;
+        }
+
         if (ssh->acceptState == ACCEPT_KEYED) {
             return 1;
         }
@@ -732,7 +741,7 @@ INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg)
             if (msg == MSGID_KEXINIT) {
                 WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
                         msg, "server", "when keying");
-                ssh->error = WS_REKEYING;
+                ssh->error = WS_MSGID_NOT_ALLOWED_E;
                 return 0;
             }
 
@@ -744,7 +753,7 @@ INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg)
                     WLOG(WS_LOG_DEBUG,
                             "Message ID %u not the expected message %u",
                             msg, ssh->handshake->expectMsgId);
-                    ssh->error = WS_REKEYING;
+                    ssh->error = WS_MSGID_NOT_ALLOWED_E;
                     return 0;
                 }
                 else {
@@ -823,6 +832,15 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
     }
 
     if (msg == MSGID_SERVICE_ACCEPT) {
+        /* RFC 4253 section 7.1: no service accept once the peer's KEXINIT
+         * lands. connectState doesn't track a rekey. */
+        if (ssh->isKeying & WOLFSSH_PEER_IS_KEYING) {
+            WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+                    msg, "client", "when keying");
+            ssh->error = WS_MSGID_NOT_ALLOWED_E;
+            return 0;
+        }
+
         if (ssh->connectState == CONNECT_CLIENT_USERAUTH_REQUEST_SENT) {
             return 1;
         }
@@ -845,7 +863,7 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
             if (msg == MSGID_KEXINIT) {
                 WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
                         msg, "client", "when keying");
-                ssh->error = WS_REKEYING;
+                ssh->error = WS_MSGID_NOT_ALLOWED_E;
                 return 0;
             }
 
@@ -857,7 +875,7 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
                     WLOG(WS_LOG_DEBUG,
                             "Message ID %u not the expected message %u",
                             msg, ssh->handshake->expectMsgId);
-                    ssh->error = WS_REKEYING;
+                    ssh->error = WS_MSGID_NOT_ALLOWED_E;
                     return 0;
                 }
                 else {

tests/regress.c

@@ -2167,6 +2167,39 @@ static void TestChannelAllowedAfterAuth(WOLFSSH* ssh)
 }
 
 
+/* A service accept is allowed only when no key exchange is in flight.
+ * connectState does not see a rekey, so isKeying is checked too. */
+static void TestClientServiceAcceptBlockedDuringKeying(WOLFSSH* ssh)
+{
+    int allowed;
+
+    ResetSession(ssh);
+    ssh->connectState = CONNECT_CLIENT_USERAUTH_REQUEST_SENT;
+
+    allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_SERVICE_ACCEPT,
+            WS_MSG_RECV);
+    AssertTrue(allowed);
+
+    /* Same connectState, peer's rekey KEXINIT arrived first. */
+    ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
+
+    allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_SERVICE_ACCEPT,
+            WS_MSG_RECV);
+    AssertFalse(allowed);
+    AssertIntEQ(ssh->error, WS_MSGID_NOT_ALLOWED_E);
+
+    /* Allowed when only this side has started a rekey: RFC 4253 section 7.1
+     * requires tolerating what the peer sent before it saw our KEXINIT. */
+    ResetSession(ssh);
+    ssh->connectState = CONNECT_CLIENT_USERAUTH_REQUEST_SENT;
+    ssh->isKeying = WOLFSSH_SELF_IS_KEYING;
+
+    allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_SERVICE_ACCEPT,
+            WS_MSG_RECV);
+    AssertTrue(allowed);
+}
+
+
 /* Drive the whole receive path with a CHANNEL_OPEN sent from a pre-auth
  * connectState: no channel created, no reply emitted. The connectState
  * gate does the rejecting; isKeying below is scene-setting only. */
@@ -2325,6 +2358,64 @@ static void TestServerServiceRequestStateGated(WOLFSSH* ssh)
 }
 
 
+/* Drive the receive path with a SERVICE_REQUEST arriving mid-rekey.
+ * acceptState sits at ACCEPT_KEYED throughout, so only isKeying catches
+ * it. */
+static void TestServerServiceRequestRejectedDuringKeying(void)
+{
+    WOLFSSH_CTX* ctx;
+    WOLFSSH* ssh;
+    MemIo io;
+    byte payload[64];
+    byte pkt[128];
+    byte out[128];
+    word32 pktSz;
+    word32 idx = 0;
+
+    idx = AppendString(payload, sizeof(payload), idx, "ssh-userauth");
+    pktSz = WrapPacket(MSGID_SERVICE_REQUEST, payload, idx, pkt, sizeof(pkt));
+
+    ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
+    AssertNotNull(ctx);
+    wolfSSH_SetIORecv(ctx, MemRecv);
+    wolfSSH_SetIOSend(ctx, MemSend);
+
+    ssh = wolfSSH_new(ctx);
+    AssertNotNull(ssh);
+
+    MemIoInit(&io, pkt, pktSz, out, sizeof(out));
+    wolfSSH_SetIOReadCtx(ssh, &io);
+    wolfSSH_SetIOWriteCtx(ssh, &io);
+
+    ssh->acceptState = ACCEPT_KEYED;
+    /* The peer's rekey KEXINIT arrived, its NEWKEYS has not. */
+    ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
+
+    AssertIntEQ(wolfSSH_TestDoReceive(ssh), WS_FATAL_ERROR);
+    AssertIntEQ(ssh->error, WS_MSGID_NOT_ALLOWED_E);
+    /* Not dispatched, so acceptState did not advance. */
+    AssertIntEQ(ssh->clientState, CLIENT_BEGIN);
+    AssertIntEQ(ssh->acceptState, ACCEPT_KEYED);
+    /* Nothing emitted in reply. */
+    AssertIntEQ(io.outSz, 0);
+
+    /* Allowed when only this side has started a rekey. */
+    ssh->error = 0;
+    ssh->isKeying = WOLFSSH_SELF_IS_KEYING;
+    AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, MSGID_SERVICE_REQUEST,
+            WS_MSG_RECV));
+
+    /* Allowed once the key exchange is done. */
+    ssh->error = 0;
+    ssh->isKeying = 0;
+    AssertTrue(wolfSSH_TestIsMessageAllowed(ssh, MSGID_SERVICE_REQUEST,
+            WS_MSG_RECV));
+
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+}
+
+
 static void TestChannelOpenCallbackRejectSendsOpenFail(void)
 {
     ChannelOpenHarness harness;
@@ -8501,6 +8592,7 @@ int main(int argc, char** argv)
     TestChannelBlockedBeforeAuth(ssh);
     TestChannelBlockedEveryPreAuthState(ssh);
     TestChannelAllowedAfterAuth(ssh);
+    TestClientServiceAcceptBlockedDuringKeying(ssh);
     TestChannelOpenRejectedBeforeKex(CONNECT_CLIENT_KEXINIT_SENT);
     TestChannelOpenRejectedBeforeKex(CONNECT_CLIENT_KEXDH_INIT_SENT);
 #ifndef NO_WOLFSSH_SERVER
@@ -8509,6 +8601,7 @@ int main(int argc, char** argv)
     TestServerUserauthBlockedBeforeKeyed(serverSsh);
     TestServerOnlyUserauthMsgsBlocked(serverSsh);
     TestServerServiceRequestStateGated(serverSsh);
+    TestServerServiceRequestRejectedDuringKeying();
     TestChannelOpenCallbackRejectSendsOpenFail();
     TestSecondSessionChannelRejected();
     TestUsernameChangeDisconnects();