Cover and document the client KEX role check

TestClientOnlyKexMsgsBlocked now asserts that 31 and 33, the ids a
client does receive, stay allowed where the handshake expects them, so
widening the role check into a 30-34 range fails the suite. The
IsMessageAllowed() comment records the receive-only policy and the
per-KEX-method id namespace.

- assert id 33 is allowed once expectMsgId is MSGID_KEXDH_GEX_REPLY
- assert expectMsgId starts at MSGID_NONE, so the role check is what
  rejects the blocked ids
- recheck all three blocked ids during a rekey on an established session

Author: ejohnstown

src/internal.c

@@ -950,7 +950,13 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
 
 
 /* 'state' argument is for if trying to send a message or receive one.
- * Returns 1 if allowed 0 if not allowed. */
+ * Returns 1 if allowed 0 if not allowed.
+ *
+ * The side helpers implement the receive policy only; 'state' is unused,
+ * and both WS_MSG_SEND callers ask about channel messages that no role
+ * list names. IDs 30 to 49 are per KEX method, so those lists hold for
+ * the methods in cannedKexAlgoNames, not the ids; RFC 4432 and RFC 4462
+ * send some of those ids the other way. */
 INLINE static int IsMessageAllowed(WOLFSSH *ssh, byte msg, byte state)
 {
 #ifndef NO_WOLFSSH_SERVER

tests/regress.c

@@ -2274,6 +2274,9 @@ static void TestClientOnlyKexMsgsBlocked(WOLFSSH* ssh)
     ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
     ssh->handshake = AllocHandshake(ssh);
     ssh->handshake->kexId = ID_DH_GEX_SHA256;
+    /* The client expects no particular message yet, so the expectMsgId
+     * check cannot catch these, the role check has to. */
+    AssertIntEQ(ssh->handshake->expectMsgId, MSGID_NONE);
 
     allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_KEXDH_INIT,
             WS_MSG_RECV);
@@ -2300,11 +2303,35 @@ static void TestClientOnlyKexMsgsBlocked(WOLFSSH* ssh)
             WS_MSG_RECV);
     AssertTrue(allowed);
     AssertIntEQ(ssh->handshake->expectMsgId, MSGID_NONE);
+    AssertIntEQ(ssh->error, WS_SUCCESS);
+
+    /* 33 sits between the two blocked ids and has to stay allowed. Assert
+     * it where the client actually expects it, once it has sent its GEX
+     * init. */
+    ssh->error = 0;
+    ssh->handshake->expectMsgId = MSGID_KEXDH_GEX_REPLY;
+    allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_KEXDH_GEX_REPLY,
+            WS_MSG_RECV);
+    AssertTrue(allowed);
+    AssertIntEQ(ssh->handshake->expectMsgId, MSGID_NONE);
+    AssertIntEQ(ssh->error, WS_SUCCESS);
 
     /* Same answer during a rekey on an established session. */
     ssh->error = 0;
     ssh->connectState = CONNECT_DONE;
 
+    allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_KEXDH_INIT,
+            WS_MSG_RECV);
+    AssertFalse(allowed);
+    AssertIntEQ(ssh->error, WS_MSGID_NOT_ALLOWED_E);
+
+    ssh->error = 0;
+    allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_KEXDH_GEX_INIT,
+            WS_MSG_RECV);
+    AssertFalse(allowed);
+    AssertIntEQ(ssh->error, WS_MSGID_NOT_ALLOWED_E);
+
+    ssh->error = 0;
     allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_KEXDH_GEX_REQUEST,
             WS_MSG_RECV);
     AssertFalse(allowed);