Tobias Frauenschlaeger 960551cee0 Address review findings on the sniffer Encrypt-Then-MAC fix ...

Hand the corrected plaintext length to the WOLFSSL_SNIFFER_STORE_DATA_CB
callback. The previous commit fixed the length only for the branch that
copies into the caller's buffer; the callback branch still passed the raw
record size, which spans the explicit IV, the tag or MAC and any padding.
The callback pointer already points at the start of the plaintext, so the
extra bytes run off the end of the decrypt output buffer, which leaves only
a record header of slack. AddressSanitizer reports a heap-buffer-overflow
read of 68 bytes on a TLS 1.2 CBC capture before this change and is clean
after it.

Reject a non-empty encrypt_then_mac extension in the ServerHello. RFC 7366
section 3.1 specifies empty extension_data and TLSX_EncryptThenMac_Parse
enforces it, as do the neighbouring length-constrained cases in the same
switch.

Clear the Encrypt-Then-MAC decision at the start of every ServerHello.
Nothing reset it, so a renegotiation that dropped the extension kept
stripping a MAC that was no longer there, and one that added it started
stripping while the previous cipher was still active.

Reset the decrypted flag at doMessage and gate the padSz subtraction on it.
The flag is set inside the decrypt block and was never cleared, so it
described whichever earlier record in the packet had last been decrypted.
The handshake case already guards the same value with it.

Set the decoded-data flag on the WOLFSSL_ASYNC_CRYPT drain path too. A
record that goes pending is completed inside SnifferAsyncPollQueue rather
than DecodePacket, so an async build could decrypt a capture correctly and
still report that nothing was decrypted. Send that diagnostic to stderr
rather than into the decoded output, and note the exit status in the usage
text so a wrapper is not surprised by it.

Add a TLS 1.2 CBC Encrypt-Then-MAC capture and the sniffer-gen.sh recipe
that produces it, so the fix has a regression test that can be regenerated.
The existing static RSA and IPv6 captures exercise the same path but no
script in the tree can rebuild them. The new capture covers both failure
modes: with an HMAC-SHA1 suite the old code fails the decrypt outright,
and with HMAC-SHA256, whose MAC is a multiple of the block size, it
silently decrypts the wrong byte range and emits 48 bytes where 14 are
correct.

Warn when the Encrypt-Then-MAC gate drops the TLS 1.2 CBC captures, so a
build configuration that loses coverage says so, and regenerate the keylog
reference output with a binary that carries the new feature token.

2026-09-01 09:33:17 +02:00

8.5 KiB

Raw Permalink History

View Raw