diff --git a/src/internal.c b/src/internal.c index 817a0442..93478e72 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4621,12 +4621,14 @@ static int DoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) } } - /* Skip the "for future use" length. */ + /* RFC 4253 7.1 reserved field: fixed uint32 0, not a length prefix. */ if (ret == WS_SUCCESS) { - WLOG(WS_LOG_DEBUG, "DKI: For Future Use"); + WLOG(WS_LOG_DEBUG, "DKI: Reserved"); ret = GetUint32(&skipSz, buf, len, &begin); - if (ret == WS_SUCCESS) - begin += skipSz; + if (ret == WS_SUCCESS && skipSz != 0) { + WLOG(WS_LOG_DEBUG, "DKI: non-zero reserved field"); + ret = WS_PARSE_E; + } } if (ret == WS_SUCCESS) { diff --git a/tests/regress.c b/tests/regress.c index 5547dc51..a252121d 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -2167,6 +2167,43 @@ static void TestFirstPacketFollows(void) TestFirstPacketFollowsSkipped(); } +/* RFC 4253 7.1: the trailing uint32 in KEXINIT is reserved and must be zero. + * DoKexInit used to advance begin by that value (treating it as a length); + * the current code rejects any non-zero value with WS_PARSE_E. Lock the + * strict-rejection branch in so a regression that re-relaxes the check or + * reverts to skipping skipSz bytes would fail this test. */ +static void TestKexInitReservedNonZeroRejected(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + byte payload[512]; + word32 payloadSz; + word32 idx = 0; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(wolfSSH_SetAlgoListKex(ssh, FPF_KEX_GOOD), WS_SUCCESS); + AssertIntEQ(wolfSSH_SetAlgoListKey(ssh, FPF_KEY_GOOD), WS_SUCCESS); + + payloadSz = BuildKexInitPayload(ssh, FPF_KEX_GOOD, FPF_KEY_GOOD, + 0, payload, (word32)sizeof(payload)); + + /* BuildKexInitPayload puts the reserved uint32 in the final 4 bytes. + * Overwrite them with a non-zero value to exercise the strict branch. */ + AssertTrue(payloadSz >= UINT32_SZ); + (void)AppendUint32(payload, (word32)sizeof(payload), + payloadSz - UINT32_SZ, 0xDEADBEEFu); + + AssertIntEQ(wolfSSH_TestDoKexInit(ssh, payload, payloadSz, &idx), + WS_PARSE_E); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + #if !defined(WOLFSSH_NO_AES_CBC) && !defined(WOLFSSH_NO_AES_CTR) \ && !defined(WOLFSSH_NO_HMAC_SHA1) && !defined(WOLFSSH_NO_HMAC_SHA2_256) static void TestIndependentAlgoNegotiation(void) @@ -3190,6 +3227,7 @@ int main(int argc, char** argv) && !defined(WOLFSSH_NO_CURVE25519_SHA256) \ && !defined(WOLFSSH_NO_RSA_SHA2_256) TestFirstPacketFollows(); + TestKexInitReservedNonZeroRejected(); #endif #if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) \ && !defined(WOLFSSH_NO_CURVE25519_SHA256) \