fix(af): bound the anti-forensic stripe count (third fuzz-found cost axis) - #6
Merged
Conversation
The nightly `unlock` fuzz target still times out after the PBKDF2 and Argon2
bounds landed, with a new artifact. Those bounds were necessary and not
sufficient: key derivation is not the only cost the header chooses.
`KeySlot::stripes` is a `u32` read straight from the header -- `be_u32` at
offset 44 for LUKS1, the `af.stripes` JSON field for LUKS2 -- and nothing
validates it. It reaches two unbounded places:
* `af::material_len` multiplies `block_size * stripes` and the caller
immediately does `vec![0u8; material_len]`. 64 bytes times u32::MAX
stripes is roughly 275 GB, allocated on the container's say-so.
* `af_merge` loops `stripes - 1` times, hashing each round.
Real containers use 4000 -- LUKS1 fixes it there and cryptsetup writes the same
for LUKS2 -- so a bound far above that costs nothing real.
Three tests: `material_len` must refuse an absurd count and name it; `merge`
must refuse independently, because it is re-exported as the public `af_merge`
and so cannot assume a caller went through `material_len` first; and a real
4000-stripe split must still round-trip, since a bound that rejects genuine
containers would be worse than the bug.
RED: `material_len` returns `usize` with no ceiling, and
`LuksError::ImplausibleStripes` does not exist, so this does not compile.
`MAX_STRIPES` caps the count at 1 << 20. LUKS1 fixes it at 4000 and cryptsetup
writes the same for LUKS2, so that is two orders of magnitude of headroom over
anything genuine while keeping both costs bounded.
Enforced in two places, because there are two ways in:
* `material_len` is now fallible. It computes the size of a buffer the caller
immediately allocates, so an implausible count has to be refused here rather
than handed back as an allocation request. The multiply is checked too --
`block_size * stripes` can wrap, and a wrapped product is a *small*
allocation followed by out-of-bounds reads, which is worse than a large one.
* `merge` validates independently. It is re-exported as the public
`af_merge`, so it cannot assume a caller came through `material_len`: its
loop runs `stripes - 1` times whatever the material length says.
The two call sites in `volume.rs` propagate with `?`. Both already sat directly
in front of `vec![0u8; material_len]`, which was the allocation being sized by
the header.
Refusal names the count and the buffer size it would have produced, so an
examiner sees what the container asked for rather than a bare "invalid".
This is the third cost axis in the unlock path that the header chooses; the
PBKDF2 iteration count and the Argon2 time/memory costs were the first two. The
fuzz timeout that survived those bounds is what pointed here.
Full workspace: 7 suites, 0 failures; fmt, clippy and the coverage gate clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why there was still a timeout
The nightly
unlockfuzz target kept timing out after the PBKDF2 and Argon2bounds landed, with a new artifact. Those bounds were necessary and not
sufficient — key derivation isn't the only cost the header chooses.
KeySlot::stripesis au32read straight from the header (be_u32at offset44 for LUKS1, the
af.stripesJSON field for LUKS2) and nothing validatedit. It reached two unbounded places:
64 bytes ×
u32::MAXstripes is roughly 275 GB, requested on thecontainer's say-so.
The bound
MAX_STRIPES = 1 << 20. LUKS1 fixes the count at 4000 and cryptsetup writes thesame for LUKS2, so that's two orders of magnitude of headroom over anything
genuine.
Enforced in two places, because there are two ways in:
material_lenis now fallible — it computes a size the caller immediatelyallocates, so an implausible count is refused here rather than returned as an
allocation request. The multiply is checked too:
block_size * stripescanwrap, and a wrapped product means a small allocation followed by
out-of-bounds reads — worse than a large one.
mergevalidates independently — it's re-exported as the publicaf_merge, so it can't assume a caller came throughmaterial_len; its loopruns
stripes - 1times regardless of the material length.The refusal names the count and the buffer size it would have produced, so an
examiner sees what the container asked for rather than a bare "invalid".
Third axis, same class
This is the third header-chosen cost in the unlock path: PBKDF2 iterations,
Argon2 time/memory, now AF stripes. I reported the earlier PR as fixing the DoS
when it had fixed two of three — the surviving fuzz timeout is what pointed
here.
Verification
cargo fmt --all -- --check— cleancargo clippy --workspace --all-targets -- -D warnings— cleancargo test --workspace— 7 suites, 0 failuresgenuine containers