Skip to content

Tie each pending request ID to the session that started it #450

Open

Description

@cjbarth

Summary

node-saml records every generated request ID in one cacheProvider that every user shares, keyed by the ID alone. A response is accepted for whichever pending ID it names, whoever presents it and from whichever browser. #448 stops unsigned input from retiring a pending ID. But a signature only shows that the IdP answered a request. It does not show that the person presenting the response is the one who started it. Two problems follow, and binding each request ID to the session that started it fixes both.

1. Retiring someone else's pending request with a signed response

This is the remainder of #438 after #448.

  1. Someone with an account at the IdP gets it to answer a request that carries another user's pending ID:
    • When AuthnRequests are unsigned, which many IdPs accept by default, knowing the ID is enough to build such a request.
    • When they are signed, by replaying that user's own AuthnRequest, to an IdP that doesn't reject replayed requests. The signed request travels in the same browser URL through which the ID leaks.
  2. They present the signed response at the ACS. Under "always", it consumes the other user's ID and signs the sender in as themselves.
  3. The other user's genuine response is then rejected with SubjectInResponseTo is not valid or InResponseTo is not valid.

The effect is the same as in #438: one failed login. The preconditions are knowing the pending ID and holding an IdP account, plus either unsigned AuthnRequests or an IdP that accepts replayed ones. The README (updated in #448) describes this and the partial mitigation of signing AuthnRequests.

2. Login CSRF under "always"

  1. Someone starts a login at the service provider, so their own request ID is recorded.
  2. They authenticate at the IdP as themselves and keep the resulting SAMLResponse rather than submitting it.
  3. They make a victim's browser POST it to the ACS, for example with an auto-submitting form.

The ID is pending and the response is genuine, so node-saml accepts it. The victim's browser is now signed in to the attacker's account, and anything the victim enters or uploads goes there.

"always" is documented as accepting "only a response that answers a request Node-SAML recorded". It does not check that the request was made by this browser, which is what an integrator choosing "always" would reasonably assume. Under "ifPresent" and "never", unsolicited responses are accepted by design, so IdP-initiated login is open to login CSRF in any library. This issue is about solicited responses.

What other libraries do

Every library surveyed in #448 keeps the pending request in the user's own session or cookie, so neither problem arises there:

  • python3-saml and ruby-saml: the application stores the request ID in its session and passes it to validation (request_id, matches_request_id).
  • crewjam/saml: each request is tracked in a signed cookie, and validation builds possibleRequestIDs from the cookies the browser sent.
  • Spring Security: the request is kept in the HttpSession (HttpSessionSaml2AuthenticationRequestRepository).
  • SimpleSAMLphp: request state is kept in the session and looked up by InResponseTo.

Design questions

node-saml never sees the HTTP request or a session; passport-saml does. So the binding has to be supplied by the caller, and passport-saml has to plumb it from req.session (a companion change there).

  1. Caller supplies the expected request IDs. Return the ID when a request is generated. Today a caller can get it only by supplying generateUniqueId or by decoding the generated request. The caller keeps it in its session and passes it to validatePostResponseAsync and validateRedirectAsync, which accept a response only for one of those IDs. This is the python3-saml and crewjam model. The shared cache can stay, for one-time consumption across processes, or become unnecessary.
  2. Caller supplies a session identifier. Generation and validation both take one, and cache keys combine it with the request ID. This keeps cacheProvider as the store, but every call site needs the identifier.
  3. What to do when the binding is absent. An opt-in could ship in 5.x as a new optional parameter. Requiring it under "always" rejects every login that doesn't supply it, so that waits for a major. This follows AGENTS.md, "Make the caller choose", and matches Require validateInResponseTo to be set #399 and Reject replayed assertions by remembering their IDs (SAML profiles §4.1.4.5) #449.
  4. Logout. LogoutRequest IDs are recorded and consumed the same way, so a logout response should be bound to its session too.

Related

🤖 Generated with Claude Code

Activity

  1. added this to the v6 milestone on Sep 25, 2026
  2. added
    enhancementAdds a feature, option or export without breaking existing callers
    securityHas security impact, such as tampered input being accepted
    on Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementAdds a feature, option or export without breaking existing callerssecurityHas security impact, such as tampered input being accepted

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions