DoNewKeys: reject NEWKEYS with non-empty payload

- Reject SSH_MSG_NEWKEYS when len != 0 per RFC 4253 7.3.
- Update wolfSSH_TestDoNewKeys to take buf/len/idx instead
  of assuming NULL/0, and add a non-zero len test case.

Issue: F-2079

Author: ejohnstown

src/internal.c

@@ -6309,10 +6309,10 @@ static int DoNewKeys(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
     int ret = WS_SUCCESS;
 
     WOLFSSH_UNUSED(buf);
-    WOLFSSH_UNUSED(len);
     WOLFSSH_UNUSED(idx);
 
-    if (ssh == NULL || ssh->handshake == NULL)
+    /* RFC 4253 7.3: SSH_MSG_NEWKEYS has no payload. */
+    if (ssh == NULL || ssh->handshake == NULL || len != 0)
         ret = WS_BAD_ARGUMENT;
 
     if (ret == WS_SUCCESS) {
@@ -18135,10 +18135,9 @@ int wolfSSH_TestDoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
     return DoKexInit(ssh, buf, len, idx);
 }
 
-int wolfSSH_TestDoNewKeys(WOLFSSH* ssh)
+int wolfSSH_TestDoNewKeys(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
 {
-    /* DoNewKeys ignores buf/len/idx (marked WOLFSSH_UNUSED internally). */
-    return DoNewKeys(ssh, NULL, 0, NULL);
+    return DoNewKeys(ssh, buf, len, idx);
 }
 
 void wolfSSH_TestFreeHandshake(WOLFSSH* ssh)

tests/regress.c

@@ -3000,7 +3000,7 @@ static void TestDoNewKeys(void)
 
     /* Peer has sent NewKeys; self has already sent its own (not keying). */
     ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
-    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh), WS_SUCCESS);
+    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh, NULL, 0, NULL), WS_SUCCESS);
 
     /* handshake freed by DoNewKeys. */
     AssertTrue(ssh->handshake == NULL);
@@ -3055,7 +3055,7 @@ static void TestDoNewKeys(void)
     WMEMCPY(&savedPeerKeys, &ssh->handshake->peerKeys, sizeof(Keys));
 
     ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
-    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh), WS_SUCCESS);
+    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh, NULL, 0, NULL), WS_SUCCESS);
 
     AssertTrue(ssh->handshake == NULL);
 
@@ -3109,7 +3109,7 @@ static void TestDoNewKeys(void)
     WMEMCPY(&savedPeerKeys, &ssh->handshake->peerKeys, sizeof(Keys));
 
     ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
-    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh), WS_SUCCESS);
+    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh, NULL, 0, NULL), WS_SUCCESS);
 
     AssertTrue(ssh->handshake == NULL);
 
@@ -3163,7 +3163,11 @@ static void TestDoNewKeys(void)
     WMEMCPY(&savedPeerKeys, &ssh->handshake->peerKeys, sizeof(Keys));
 
     ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
-    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh), WS_SUCCESS);
+    /* Exercise the len != 0 rejection while handshake is still allocated,
+     * so the guard is reached and not short-circuited by handshake == NULL. */
+    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh, NULL, 1, NULL), WS_BAD_ARGUMENT);
+    AssertNotNull(ssh->handshake);
+    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh, NULL, 0, NULL), WS_SUCCESS);
 
     AssertTrue(ssh->handshake == NULL);
 

wolfssh/internal.h

@@ -1366,8 +1366,9 @@ enum WS_MessageIdLimits {
             word32 len, word32* idx);
     WOLFSSH_API int wolfSSH_TestDoKexInit(WOLFSSH* ssh, byte* buf,
             word32 len, word32* idx);
+    WOLFSSH_API int wolfSSH_TestDoNewKeys(WOLFSSH* ssh, byte* buf,
+            word32 len, word32* idx);
     WOLFSSH_API int wolfSSH_TestGenerateKeys(WOLFSSH* ssh, byte hashId);
-    WOLFSSH_API int wolfSSH_TestDoNewKeys(WOLFSSH* ssh);
     WOLFSSH_API void wolfSSH_TestFreeHandshake(WOLFSSH* ssh);
     WOLFSSH_API int wolfSSH_TestDoKexDhInit(WOLFSSH* ssh, byte* buf,
             word32 len, word32* idx);