Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,11 @@ The TOTP secret is stored **encrypted at rest** (the store cipher) and recovery
**argon2id-hashed**; verification uses the server clock and a constant-time compare over a **configurable
clock-skew window** (`[auth].totp_skew_steps`, **default `0` = the current 30 s step only** — strictest
replay window, ASVS 6.5.5; set `1`/`2` to restore RFC-6238 ±1 network-delay tolerance, the forward step
clamped to the current step so single-use holds). TOTP is a shared-secret factor — L3 *prefers*
clamped to the current step to avoid a self-inflicted lockout). ⚠️ **Single-use (ASVS 6.5.1) holds only at
the default `0`.** At `totp_skew_steps >= 1` the clamp records a tolerated *future* code against the
current step, leaving that code's own step unspent — so the **same code verifies a second time** once the
clock reaches it. That is the cost of the opt-out, and it is why the default is `0`. TOTP is a
shared-secret factor — L3 *prefers*
phishing-resistant factors: **WebAuthn passkeys are the built WP-14b sibling** (next section), and TOTP
stays fully supported alongside them (a non-browser client — e.g. the test harness, or CLI/API
automation — has no `navigator.credentials`, so TOTP remains its usable second factor).
Expand Down
12 changes: 11 additions & 1 deletion messagefoundry/auth/totp.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,17 @@ def verify_totp_step(
tolerated future code (``counter+1``) reports ``counter``, never advancing the single-use high-
water mark past the genuinely-current step. Otherwise burning ``counter+1`` would reject the user's
own current-step code (a non-greater step) for up to ~30 s — a self-inflicted lockout, not a
bypass. The clamp only lowers the recorded step, so single-use is preserved.
bypass.

⚠️ **At ``window >= 1`` the clamp does not preserve single-use — it is what costs it.** Because a
tolerated future code is recorded at ``counter`` rather than at its OWN step, that step is left
unspent, so the SAME code verifies again once the clock reaches ``counter+1``: two successful uses
of one code, which is what ASVS 6.5.1 forbids. Measured — ``verify_totp_step(s, totp(s, now=t+30),
now=t, window=1)`` returns ``step(t)``, and the same code at ``now=t+30`` returns ``step(t)+1``,
strictly greater, so a high-water store accepts both. At the shipped default ``window=0``
(``[auth].totp_skew_steps``) single-use DOES hold, because a future code is not accepted at all.
The second use is the price of the opt-out, not a property of the clamp. Pinned by
tests/test_totp_window.py::test_optout_lets_one_tolerated_future_code_be_used_twice.
"""
candidate = code.strip()
if len(candidate) != digits or not candidate.isdigit():
Expand Down
38 changes: 37 additions & 1 deletion tests/test_totp_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
authenticator can still log in) but the returned step is **clamped to the current step**, so consuming
a tolerated future code never advances the single-use high-water mark past ``now`` — otherwise the
user's own genuine current-step code (a non-greater step) would be rejected for up to ~30 s, a
self-inflicted lockout, not a bypass. The clamp only lowers the recorded step, so single-use holds.
self-inflicted lockout, not a bypass. ⚠️ The clamp does NOT preserve single-use here — it is what
costs it: recording a tolerated future code at ``now`` leaves that code's OWN step unspent, so the
same code verifies again one step later (two successful uses, ASVS 6.5.1). Single-use holds only at
the strict default. See test_optout_lets_one_tolerated_future_code_be_used_twice.

These call ``verify_totp_step`` directly with an EXPLICIT ``window`` so both the strict default and the
opt-out are pinned regardless of the module-level ``DEFAULT_WINDOW`` (which stays 1 for callers that
Expand Down Expand Up @@ -99,6 +102,39 @@ def test_optout_fast_clock_future_code_causes_no_self_lockout() -> None:
assert genuine is not None and genuine > consumed


def test_optout_lets_one_tolerated_future_code_be_used_twice() -> None:
"""The clamp's COST, pinned — one code, two successful uses, at ``totp_skew_steps >= 1``.

This is the gap the neighbouring tests leave. ``test_single_use_step_is_stable_for_the_same_code``
replays the same code at the SAME ``now``; ``test_optout_fast_clock...no_self_lockout`` replays a
DIFFERENT (genuine) code at a later ``now``. Nobody replayed the SAME code at a LATER ``now``, which
is where the clamp bites: recording a tolerated ``counter+1`` code at ``counter`` leaves its own step
unspent, so it verifies again when the clock arrives there. A high-water store that rejects a
non-greater step accepts BOTH, because the second resolution is strictly greater.

Three docstrings previously drew the opposite conclusion from the same true premise ("the clamp only
lowers the recorded step, so single-use is preserved"). It lowers the step, and that is precisely
why the code survives. ASVS 6.5.1 requires TOTPs be "only successfully usable once"; that holds at
the shipped default and not at the opt-out, so the opt-out carries a real cost the docs now name.
"""
t = 5_000 * PERIOD + 5.0
future = totp.totp(SECRET, now=t + PERIOD)

first = totp.verify_totp_step(SECRET, future, now=t, window=1)
second = totp.verify_totp_step(SECRET, future, now=t + PERIOD, window=1)
assert first == _step(t) # clamped down, per SEC-014
assert second == _step(t + PERIOD) # its own step, still unspent
assert second is not None and first is not None and second > first, (
"the second resolution must be STRICTLY GREATER — that is what makes a high-water store "
"accept the same code a second time"
)

# The strict default is the control: the same code cannot be used twice, because the first
# presentation is refused outright rather than clamped.
assert totp.verify_totp_step(SECRET, future, now=t, window=0) is None
assert totp.verify_totp_step(SECRET, future, now=t + PERIOD, window=0) == _step(t + PERIOD)


def test_single_use_step_is_stable_for_the_same_code_and_now() -> None:
# The same code at the same now resolves to the same step both times (a single-use store rejecting a
# non-greater step then rejects the replay). Holds under both the strict and the opt-out window.
Expand Down
Loading