Skip to content

fix(api-gateway): enforce wallet challenge expiry server-side - #581

Open
Andreschuks101 wants to merge 1 commit into
Betta-Pay:mainfrom
Andreschuks101:api-gateway/554-wallet-challenge-expiry
Open

fix(api-gateway): enforce wallet challenge expiry server-side#581
Andreschuks101 wants to merge 1 commit into
Betta-Pay:mainfrom
Andreschuks101:api-gateway/554-wallet-challenge-expiry

Conversation

@Andreschuks101

Copy link
Copy Markdown
Contributor

closes #554

Problem

wallet-auth-challenge.test.ts describes a Redis-backed challenge flow with a TTL, but the server did not implement one. Challenges lived in a per-process Map:

const walletChallenges = new Map<string, { challenge: string; expiresAt: number }>();

Three things followed from that:

  1. No expiry store. Nothing evicted an entry that was issued and never verified. The Map grew for the lifetime of the process, one entry per address that ever asked for a challenge, and the only deadline lived in memory that a restart discarded and that no peer instance could see. A challenge issued by instance A was simply unknown to instance B.
  2. No single-use guarantee. The challenge was deleted only after a successful verification. A failed signature left it in place, so one outstanding challenge could be guessed at as many times as the attacker liked until the deadline passed.
  3. Expiry enforced against local state only. Date.now() > challengeInfo.expiresAt is the right check, but it ran against a Map that was neither durable nor shared, so in a multi-instance deployment it was routinely bypassed by simply not being reachable.

Solution

A new module, services/api-gateway/src/wallet-challenge-store.ts, backs challenges with Redis:

  • issue(address) mints a 32-byte challenge and writes it with PX set to the same TTL as the recorded expiresAt. Redis reaps unclaimed challenges on its own, so nothing accumulates and a stale challenge disappears even if nobody comes back for it. Re-issuing replaces the outstanding challenge for that address.
  • consume(address) reads and deletes in one atomic GETDEL. The challenge is gone whatever happens next — valid signature, invalid signature, malformed request — so it is single-use and there is nothing left to guess against. Two concurrent verifications cannot both claim it.
  • The recorded deadline is re-checked against the server clock, independently of the Redis TTL. A record that outlives its TTL — clock skew, a key not yet reaped, a snapshot restored into a live instance — returns expired rather than being accepted. Defence in depth: Redis expiry is the primary mechanism, the deadline check is the one that cannot be defeated by the store misbehaving.
  • Corrupt or shape-invalid records are treated as absent, never trusted.

In index.ts, POST /api/auth/challenge and POST /api/auth/verify use the store. Both return 503 Authentication service unavailable when Redis is unreachable, so a store failure can never be mistaken for a valid challenge (fail-open) or for an invalid one (a confusing 400). The verify route's response codes and messages are otherwise unchanged: 400 Challenge not found or expired, 400 Challenge expired, 401 Invalid signature.

The clock and the Redis surface are both injectable, so expiry is tested without waiting five minutes and without a live Redis.

Expired-challenge rejection

Two distinct expiry paths, both proven. First, the deadline check — the record is deliberately left in place while the clock moves past its deadline:

# a challenge that outlives its TTL is still rejected as expired
ok 9  the recorded deadline is enforced by the server
ok 10 the expired challenge is removed rather than left to be retried

# a challenge one millisecond inside its deadline is still valid
ok 11 the deadline itself is inclusive

Second, the Redis TTL:

# a challenge reaped by its Redis TTL is not found
ok 8 the key is gone once its TTL elapses

And through the real routes:

# POST /api/auth/verify rejects an expired challenge
ok 23 an expired challenge is refused                         400
ok 24 the caller is told the challenge expired                "Challenge expired"
ok 25 the expired challenge is cleared

# POST /api/auth/verify rejects a challenge Redis has already reaped
ok 26 a reaped challenge is refused                           400
ok 27 the caller is told no challenge is outstanding          "Challenge not found or expired"

Single use

# a challenge can be consumed exactly once
ok 12 the first attempt gets the challenge
ok 13 the second attempt finds nothing

# consuming is atomic, so concurrent attempts cannot both win
ok 14 exactly one of two concurrent verifications gets the challenge

# POST /api/auth/verify consumes the challenge even when the signature is wrong
ok 28 an invalid signature is rejected                        401
ok 29 the challenge is consumed by the attempt
ok 30 the challenge cannot be retried                         400
ok 31 a second signature guess has nothing to guess against

Assertions 28-31 are the behaviour change that closes the guessing window: previously a wrong signature left the challenge sitting there for the next attempt.

TTL storage, and store failures

# issuing a challenge stores it under a TTL
ok 1 a 32-byte challenge is generated
ok 2 the deadline is recorded on the challenge
ok 3 the Redis key carries a matching TTL
ok 4 the deadline is persisted, not just returned

# the challenge routes report 503 when Redis is unreachable
ok 33 issuing reports the store as unavailable
ok 34 verifying reports the store as unavailable
ok 35 the failure is not mistaken for an invalid challenge

Verification

$ pnpm --filter api-gateway build
$ tsc --project tsconfig.json
(clean)

$ pnpm --filter api-gateway test
# tests 3    # pass 3    health-all
# tests 20   # pass 20   param-sanitization
# tests 1    # pass 0    # fail 0   payment-to-settlement (integration, skipped without a DB)
# tests 40   # pass 40   warmup-downstream
# tests 35   # pass 35   wallet-challenge-expiry (new)

Also re-ran the suites that touch this area: wallet-auth-challenge (7/7), auth-security (45/45), rate-limit-headers (9/9), error-response (7/7).

The new test file is wired into the api-gateway test script so it runs in CI.

Notes for reviewers

  • GETDEL needs Redis 6.2 or newer. If older servers are still in the fleet, the same atomicity can be had with a two-command MULTI or a one-line Lua script — say the word and I will swap it.
  • The existing wallet-auth-challenge.test.ts builds its own miniature app rather than exercising the gateway's routes, so it keeps passing unchanged. Its premise — a Redis-backed, TTL-bearing store shared across instances — is now what the server actually does, so it could be folded into the new file in a follow-up if you would like the duplication gone.
  • The separate nonce-based /api/auth/wallet/verify route already has its own replay protection (used_nonce:*) and is untouched by this change.

Wallet auth challenges lived in a per-process Map. Nothing evicted an
entry that was never verified, no other gateway instance could see it,
and a challenge survived every failed verification attempt — so one
outstanding challenge could be guessed at indefinitely, and the only
expiry check ran against state that a restarted or peer instance did
not share.

- Add src/wallet-challenge-store.ts: challenges are stored in Redis
  under a TTL matching their recorded deadline, so an unclaimed
  challenge is reaped instead of accumulating forever.
- consume() reads and deletes in one atomic GETDEL, so a challenge is
  usable exactly once whether or not the signature turns out to be
  valid, and two concurrent verifications cannot both claim it.
- Re-check the recorded expiresAt against the server clock, so a record
  that outlives its TTL (clock skew, an unreaped key, a restored
  snapshot) is rejected rather than accepted.
- POST /api/auth/challenge and POST /api/auth/verify use the store and
  report 503 when it is unreachable, so a store failure is never
  mistaken for a valid or an invalid challenge.
- Add src/wallet-challenge-expiry.test.ts: TTL storage, expiry by TTL
  and by deadline, deadline boundary, single use, atomic consumption,
  corrupt records, and the routes end to end.
@therealjhay

Copy link
Copy Markdown
Contributor

kindly resolve conflict

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.

api-gateway wallet auth does not verify challenge expiry on the server

2 participants