Skip to content

Replay tracking is per instance, and a complete guarantee needs a shared record #51

Description

@shreemaan-abhishek

What

replay_dict remembers every assertion an instance accepts, so it refuses the same one twice. An lua_shared_dict is shared between the workers of one gateway and nowhere else, so the guarantee is really at most once per replica.

Behind a load balancer, a captured assertion replayed a second time lands on a replica that has never seen it and is accepted. With N replicas and no affinity, a blind replay has an (N-1)/N chance of picking a fresh one, so on a multi-replica deployment this is closer to a speed bump than a defence.

This is written down in the README today rather than fixed, under "Remembering assertions".

Why it is not urgent

Across replicas the work is done by the request binding from #43: SubjectConfirmationData/@InResponseTo has to name the AuthnRequest this SP issued, and that ID lives in the user's own session cookie, which travels with them to whichever replica they land on. A replayed assertion is refused on any replica, because the attacker's own session names a different request.

replay_dict is the defence for what that leaves uncovered: an IdP that sends no InResponseTo, which the README also states plainly. Those deployments are exactly the ones where the per-instance limit bites.

What it should do

A shared record with an atomic add, so the first presentation wins across the whole deployment:

  • an interface the caller supplies rather than a hardcoded backend, since APISIX, the EE gateway and a bare OpenResty deployment do not agree on what they have
  • Redis is the obvious first one, through SET key value NX EX ttl, which is the same one-shot claim safe_add gives locally
  • the local dict stays the default, so nothing new is required of a deployment that does not want it
  • failure of the shared store needs the same answer the full dict got: leave the login untracked and log it, rather than refusing every login when the store is unreachable

Notes

Raised by Copilot and by @jarvis9443 reviewing #50, on the README claim rather than on the code. Depends on nothing; #50 can merge as it stands.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions