From ad42c78b541908292eab87b5f496861334e9c07c Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2026 11:45:18 -0700 Subject: [PATCH] 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 --- src/internal.c | 8 +++++++- tests/regress.c | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index c0143d0a2..0cecd9d4f 100644 --- a/src/internal.c +++ b/src/internal.c @@ -945,7 +945,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 diff --git a/tests/regress.c b/tests/regress.c index 31db3009d..4afb62ac3 100644 --- a/tests/regress.c +++ b/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);