internal: reject inbound packets that are not block aligned

[yosuke-wolfssl]

Problem

DoReceive() accepted an inbound binary packet on a maximum-length check alone. Nothing verified the RFC 4253 §6 rule that packet_length + padding_length + payload + padding be a multiple of the cipher block size, or 8, whichever is larger. RFC 5647 imposes the same rule on AES-GCM, excluding the length field.

What coverage existed was incidental. AES-CBC/CTR were aligned only as a side effect of Decrypt()'s sz % AES_BLOCK_SIZE, and only when the packet spanned more than one block. AES-GCM had no check at all. Neither did the null cipher — which every connection runs for the whole pre-NEWKEYS handshake, in every build, with no MAC to backstop it. So KEXINIT, KEXDH, and NEWKEYS were all accepted at arbitrary lengths.

Closes f-8834.

Fix (src/internal.c)

One check in DoReceive()'s PROCESS_PACKET_LENGTH case, placed before any decrypt or dispatch so it covers every cipher path:

Transport	Aligned quantity	Block size
cleartext / null cipher	UINT32_SZ + packet_length	8
AES-CBC, AES-CTR	UINT32_SZ + packet_length	16
AES-GCM	packet_length	16

peerBlockSz is floored at MIN_BLOCK_SZ. A misaligned packet sets ssh->error to WS_BUFFER_E and returns WS_FATAL_ERROR — the code DoPacket() already returns for the sibling MIN_PAD_LENGTH violation. The rule mirrors what BundlePacket() has always emitted on the send side, so no conformant peer is affected.

Tests

test_DoReceive_RejectsMisalignedPacket and test_DoReceive_RejectsMisalignedAead cover the cleartext and AES-GCM paths. Each packet is valid in every other respect, so alignment is the only thing that can reject it.

Reviewers should expect fixture churn: BuildPacket() (regress.c, 10 call sites) and BuildMacTestPacketPrefix() (unit.c) both aligned the packet body to 8 and left the total at 12, i.e. they were emitting packets RFC 4253 forbids. The new check didn't break those tests — it revealed they were testing against malformed input. Both now pad to a block-aligned total, and test_DoReceive_AeadTagFailure uses the real GCM peerBlockSz of 16 rather than 4.

Verification

• gcc-13 -Werror clean across 6 configs; lint clean.
• unit.test and regress.test pass. make check: 10 pass, 1 skip; the lone failure is scripts/sftp.test, a known parallel-run flake that passes serially.
• Negative control: with the check reverted, both new tests fail by DoReceive accepting the malformed packet.

[Copilot]

Pull request overview

This PR hardens inbound SSH packet parsing by enforcing RFC-required packet block alignment in DoReceive(), ensuring malformed packets are rejected consistently across cleartext/null, CBC/CTR, and AEAD (AES-GCM) transport paths.

Changes:

• Add a block-alignment check in src/internal.c during PROCESS_PACKET_LENGTH, before decrypt/dispatch.
• Fix test fixtures that previously emitted RFC-invalid packet sizes and add targeted negative tests for misalignment (cleartext + AES-GCM).
• Adjust existing AEAD/HMAC unit tests to use correctly aligned packet prefixes and correct AEAD block size assumptions.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/internal.c	Rejects inbound packets whose length violates RFC block-alignment rules before any decrypt/MAC/dispatch.
tests/unit.c	Updates packet-building helpers/fixtures and adds unit tests asserting misaligned packets are rejected (cleartext + AES-GCM).
tests/regress.c	Updates minimal packet builder to generate RFC-valid, block-aligned packets for clear transport fixtures.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.