Skip to content

security: fix bearer-token length oracle in constant_time_eq (#162)#164

Merged
WaylandYang merged 1 commit into
mainfrom
fix/162-token-timing
May 25, 2026
Merged

security: fix bearer-token length oracle in constant_time_eq (#162)#164
WaylandYang merged 1 commit into
mainfrom
fix/162-token-timing

Conversation

@WaylandYang
Copy link
Copy Markdown
Contributor

Closes #162.

The hand-rolled constant_time_eq in crates/forkd-controller/src/auth.rs was advertised as constant-time but leaked the presented token's length via two distinct paths, both reported precisely by @m-dmupba:

  1. Dead-code optimization: the length-mismatch branch's "fake work" loop used x.wrapping_mul(0) == 0, which the compiler is allowed to (and LLVM does) eliminate. Response time was therefore monotonic in max(len(presented), len(expected)).

  2. Branch-shape oracle: even if the loop had been preserved, taking different branches for length-equal vs length-mismatch is itself a timing oracle — equal-length ran a real XOR loop, mismatch ran a constant-byte loop.

For a deployment with a fixed-length token (e.g. 32-byte hex from forkd doctor), the length oracle collapses security to a single attempted length, after which the equal-length branch is genuinely constant-time.

Fix

Replaced with subtle::ConstantTimeEq against a zero-padded copy of the presented token. Same code path regardless of input length; length difference is folded into the result via a non-short-circuiting bitwise AND so the function's total time doesn't depend on bytes_match either.

subtle was already an indirect dep via the rustls / aws-lc-rs subtree — confirmed by inspecting Cargo.lock (no new transitive entry added).

Tests

Three regression tests added in addition to the original two:

  • presented_prefix_of_expected_rejected — strict prefix attack
  • presented_longer_than_expected_rejected — truncating attack
  • all_zero_padding_doesnt_accidentally_match — zero-tail collision
cargo test -p forkd-controller --lib auth::
test auth::tests::all_zero_padding_doesnt_accidentally_match ... ok
test auth::tests::matching_tokens_eq ... ok
test auth::tests::presented_longer_than_expected_rejected ... ok
test auth::tests::presented_prefix_of_expected_rejected ... ok
test auth::tests::different_tokens_not_eq ... ok
test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 34 filtered out

Built + tested on dev box (Ubuntu 24.04 / Linux 6.14 / rust stable).

Test plan

  • cargo check -p forkd-controller passes
  • cargo test -p forkd-controller --lib auth:: — 5 passed
  • Cargo.lock unchanged (subtle already transitive)
  • CHANGELOG.md entry under "Unreleased"

🤖 Generated with Claude Code

…_eq (#162)

The previous hand-rolled constant_time_eq in
crates/forkd-controller/src/auth.rs was advertised as constant-time
but leaked the presented token's length via two distinct paths:

1. The length-mismatch branch's "fake work" loop used
   `x.wrapping_mul(0)` — the compiler is allowed to (and LLVM does)
   delete the loop as dead code, so response time was monotonic in
   the longer slice's length.
2. Even if the loop had been preserved, taking different branches
   for length-equal vs length-mismatch was itself a timing oracle.

For a deployment with a fixed-length token (e.g. 32-byte hex from
`forkd doctor`), the length oracle collapses security to a single
attempted length, after which the equal-length branch is real
constant-time.

Fix: replace with `subtle::ConstantTimeEq` against a zero-padded
copy of the presented token. Same code path regardless of input
length; length difference is folded in via a non-short-circuiting
bitwise AND so the function's total time doesn't depend on the
byte-comparison's result either. `subtle` was already an indirect
dep via the rustls / aws-lc-rs subtree, so the patch is lockfile-
neutral.

Adds three regression tests:
- presented being a strict prefix of expected → reject
- presented longer than expected → reject (padded view truncates,
  so an attacker can't bypass by appending)
- zero-tail accident: presented shorter than expected, where
  expected's tail happens to be 0x00 → still reject

cargo test -p forkd-controller --lib auth:: → 5 passed.

Thanks to @m-dmupba for the precise report.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@WaylandYang WaylandYang merged commit 622f9a7 into main May 25, 2026
2 checks passed
@WaylandYang WaylandYang deleted the fix/162-token-timing branch May 25, 2026 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

constant_time_eq is not constant-time when the lengths differ

1 participant