Skip to content

Saturate HmacAuth token expiry and test the authenticator - #764

Open
hiroki-abe-58 wants to merge 1 commit into
h5i-dev:mainfrom
hiroki-abe-58:fix/hmac-expiry-overflow
Open

hiroki-abe-58 wants to merge 1 commit into
h5i-dev:mainfrom
hiroki-abe-58:fix/hmac-expiry-overflow

Conversation

@hiroki-abe-58

Copy link
Copy Markdown

HmacAuth::issue computes the expiry as now() + ttl_secs. A TTL greater than u64::MAX - now(), such as u64::MAX, overflows:

  • With overflow checks (debug builds, including a default cargo test), issue panics with "attempt to add with overflow".
  • Without them, the sum wraps to a time in the past, so verify refuses the token it just issued as "token expired".

This PR makes the expiry saturate at u64::MAX. Refusing instead would mean either a panic or an API change, because issue returns a String. And any TTL that overflows already exceeds about 5.8×10¹¹ years, so saturating gives the same token as the exact sum would. The TTL is chosen by the server, not read from a request.

h5i-app-http had no tests for HmacAuth, so this PR also adds a few. They use a minimal test kernel whose principal is (tenant, user) and whose other types are all ().

  • Round trip: an issued token verifies, and authenticates through Authorization: Bearer ….
  • TTL saturation: issue(…, u64::MAX) gives exp == u64::MAX and verifies. On main this test fails in both build modes: the overflow panic with overflow checks, and the exp assertion without them.
  • Refusals, each with its expected message:
    • another secret, or a tampered payload: bad signature;
    • a past expiry: token expired;
    • a token missing its last field: malformed token.
  • The header:
    • a missing header is refused;
    • a different scheme is refused unless configured with with_scheme;
    • or_anonymous applies only when the header is missing, never when it is present but invalid.

Nothing else changes: the token format, the parsing in h5i-app-token (whose proofs cover every u64 expiry), and verify.

Testing

  • cargo test --locked -p h5i-app-http: 4 passed.
  • With now() + ttl_secs restored, ttl_saturates fails.
  • cargo clippy --locked -p h5i-app-http --all-targets -- -D warnings: clean.
  • Only h5i-app-http changed. The workspace-wide build, test and clippy were not run locally.
  • Run on Linux (WSL2 Ubuntu 24.04), Rust 1.99.1.

Written with AI assistance (Claude Code, Claude Opus 5.5) and checked in a separate AI-assisted review session. No human reviewed it before submission.

🤖 Generated with Claude Code

`HmacAuth::issue` computed the expiry as `now() + ttl_secs`. A TTL above
`u64::MAX - now()` overflowed: a panic with overflow checks, otherwise an
expiry in the past, so the token was refused. The expiry now saturates at
`u64::MAX`.

Add the first tests for `HmacAuth`: issued tokens verify and
authenticate; another secret, a tampered payload, a past expiry and a
truncated token are refused with their messages; another scheme is
refused unless configured; and the anonymous fallback applies to a
missing header only, never to a bad one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant