Skip to content

fix(verify): require expires and apply clock-skew tolerance to it - #3

Merged
AmirF194 merged 1 commit into
AmirF194:mainfrom
ebarkhordar:fix/verify-request-expiry
Jul 10, 2026
Merged

fix(verify): require expires and apply clock-skew tolerance to it#3
AmirF194 merged 1 commit into
AmirF194:mainfrom
ebarkhordar:fix/verify-request-expiry

Conversation

@ebarkhordar

Copy link
Copy Markdown
Contributor

Problem

The freshness gate in verify_request has two gaps:

1. Clock-skew tolerance is applied asymmetrically. max_skew is applied to the created (future) check but not the expires check:

if parsed.expires is not None and now > parsed.expires:      # no skew allowance
    ... reject ...
...
if parsed.created is not None and parsed.created > now + max_skew:   # skew allowed
    ... reject ...

So a verifier whose clock runs a little fast rejects a signature that is only a few seconds past expires, even though it is still valid on the signer's clock. RFC 9421 §3.2.1 says the skew tolerance should apply to both created and expires.

2. A signature with no expires verifies forever. The check is skipped entirely when expires is absent, then a passing "signature not expired" mark is recorded. A Web Bot Auth signature that omits expires therefore passes the expiry gate and — if the key resolves and the crypto checks out — is accepted indefinitely, i.e. replayable. Web Bot Auth signatures are short-lived and are required to carry an expiry.

Reproduction (before this change)

now = 1_000_000
# (1) expired 60s ago, max_skew=300 -> wrongly rejected
r = verify_request(url, expired_60s_ago, resolver, now=now, max_skew=300)
assert r.ok            # FAILS: "signature expired 60s ago"

# (2) signature with no `expires`, created ages ago -> accepted forever
r = verify_request(url, no_expires_headers, resolver, now=now)
assert not r.ok        # FAILS: r.ok is True

Fix

In verify_request:

  • Require expires — reject when it is absent ("signature has no expires (web-bot-auth requires one)").
  • Tolerate max_skew past expires (now > expires + max_skew), mirroring the existing allowance on created.

verify_directory is intentionally left unchanged: directory signatures are long-lived (365d) and pre-signed offline, so request-style clock skew doesn't apply.

Tests

Added to tests/test_rfc9421.py:

  • test_expiry_tolerates_clock_skew_within_max_skew — expired 100s ago is accepted with max_skew=300.
  • test_expiry_beyond_skew_still_fails — expired 400s ago is still rejected.
  • test_signature_without_expires_is_rejected — a hand-crafted, cryptographically valid signature that omits expires is rejected.

The existing test_verify_fails_when_expired (expired ~9940s, far beyond skew) still passes. Full suite: 45 passed (42 + 3 new).

…uest

verify_request had two gaps in the freshness gate:

1. max_skew was applied to the created (future) check but not the
   expires check, so a verifier whose clock runs slightly fast would
   reject a signature only a few seconds past expiry (RFC 9421 3.2.1
   allows skew tolerance on both).

2. A signature that omitted expires entirely passed the expiry gate and
   verified forever, making it replayable indefinitely. Web Bot Auth
   signatures are short-lived and must carry an expiry.

Require expires to be present, and tolerate max_skew past it. Add tests
for skew-within-tolerance, skew-beyond-tolerance, and missing-expires.

@AmirF194 AmirF194 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both fixes are correct and well-tested. The missing-expires gate closes a real replay hole, and the now > expires + max_skew change makes the skew tolerance symmetric with the created check (RFC 9421 §3.2.1). Verified the suite passes locally across the rfc9421 tests. Thanks!

@AmirF194
AmirF194 merged commit 0599e30 into AmirF194:main Jul 10, 2026
5 checks passed
@AmirF194 AmirF194 mentioned this pull request Jul 19, 2026
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.

2 participants