Repository navigation
Tie each pending request ID to the session that started it #450
Description
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.
- 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.
- When
- They present the signed response at the ACS. Under
"always", it consumes the other user's ID and signs the sender in as themselves. - The other user's genuine response is then rejected with
SubjectInResponseTo is not validorInResponseTo 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"
- Someone starts a login at the service provider, so their own request ID is recorded.
- They authenticate at the IdP as themselves and keep the resulting
SAMLResponserather than submitting it. - 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
possibleRequestIDsfrom 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).
- Caller supplies the expected request IDs. Return the ID when a request is generated. Today a caller can get it only by supplying
generateUniqueIdor by decoding the generated request. The caller keeps it in its session and passes it tovalidatePostResponseAsyncandvalidateRedirectAsync, 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. - Caller supplies a session identifier. Generation and validation both take one, and cache keys combine it with the request ID. This keeps
cacheProvideras the store, but every call site needs the identifier. - 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 followsAGENTS.md, "Make the caller choose", and matches RequirevalidateInResponseToto be set #399 and Reject replayed assertions by remembering their IDs (SAML profiles §4.1.4.5) #449. - Logout.
LogoutRequestIDs are recorded and consumed the same way, so a logout response should be bound to its session too.
Related
- A failed validation drops the pending request ID named by unauthenticated input #438 and Stop unsigned input from retiring a pending request ID #448: unsigned input no longer retires pending IDs; this is the signed remainder.
- Reject replayed assertions by remembering their IDs (SAML profiles §4.1.4.5) #449: the assertion-ID replay cache (§4.1.4.5). That stops a response being accepted twice. This issue is about who may present it at all, which is a separate question.
- Require
validateInResponseToto be set #399 and RequireconsumeAsynconCacheProvider#431: other v6 requirements onvalidateInResponseToandcacheProvider. If this changes thecacheProvidercontract, it should land with them.
🤖 Generated with Claude Code
Activity