Change the default implementation of Saml2AuthenticationRequestRepository to store and load AuthnRequests based on the ID instead of the session

[kailash-everlaw]

Expected Behavior
Saml2AuthenticationRequestRepository is implemented in a way that allows setting SameSite to Lax or Strict on the session cookie when using spring-security’s SAML implementation and spring-session.

Current Behavior
HttpSessionSaml2AuthenticationRequestRepository is the current default implementation of Saml2AuthenticationRequestRepository and uses HttpSession to store and load Requests, which means that the session cookie must have SameSite set to None since it must be included in the POST request sent by the user’s IdP.

Context
We currently have to set SameSite to None on the session cookie, which is not ideal since it weakens our defense against CSRF attacks. Ideally we would be able to set SameSite to Lax or Strict, but that is not currently possible due to how HttpSessionSaml2AuthenticationRequestRepository is implemented.

One possible workaround is to create a custom implementation of Saml2AuthenticationRequestRepository that stores and loads the Requests based on the ID instead of the session (as described in #10828 (comment) and #11468). However, I think it would be useful to make this the default implementation so that anyone who is using spring-security’s SAML implementation and spring-session can set SameSite to Lax or Strict on the session cookie without needing to create a custom implementation of Saml2AuthenticationRequestRepository. This new default behavior would be more secure than the existing default implementation, which requires SameSite to be set to None.

This is pretty similar to #11468, but I thought it made sense to open a new issue since that issue is closed and was focused on enabling this workaround, while this issue is about making the workaround enabled by that issue the default behavior.

I think it also might be possible to create an implementation of Saml2AuthenticationRequestRepository that loads and stores Requests using a custom cookie (as opposed to the session cookie), which would still allow SameSite to be set to Lax or Strict on that actual session cookie since it wouldn't need to be included in the POST from the user's IdP. I am not sure which approach is preferable though.

[ryanrupp]

Stumbled upon this because we're actually using the legacy Spring Security SAML library still and I was curious how this is handled in new versions (appears to still be an issue essentially). FWIW this "half works" currently with SameSite=Lax (or omitted which in modern browsers since ~2019/2020 is now defaulting to Lax) because most browsers I believe implement the Lax + POST mitigation so if the session is established within the last 2 minutes the IDP is able to POST back and have that POST request leverage the session cookie which allows linking up with persistent session data. That said, this breaks if:

1. The user takes more than 2 minutes i.e. they weren't authenticated with the IDP already and took a while to flow through or have it configured for "force authentication" on every SAML flow
2. The user starts the SAML flow (which establishes the session at least for us) but then never completes it and comes back later and tries again (it doesn't re-create the session for us at least) which then you have an old session cookie that the browser won't allow to passthrough.

We were sort of exploring the last option in your paragraph which is a different cookie just for this purpose that could then be set to SameSite=None. I'm less familiar with the internals of Spring Security SAMLs 2.x to comment on which approach is more appropriate.

[jzheaux]

Thanks for the suggestion, @kailash-everlaw. It may make sense to provide an implementation that does not retrieve the authentication request from the session.

That said, it's important to remember that there are security benefits to storing it in the session. One such benefit is the natural login fixation defense it provides. For example, if an application looks the authentication request up from the session, then even if an attacker provides their own SAML response to a victim, the login will fail.

On the other hand, if we trust the InResponseTo or RelayState to retrieve the authentication request, then there's no way to know if the SAML response was requested by that handshake. I'm not certain how to address this without creating another cookie, making me think that I might as well use the session cookie after all.

Given that, I'd argue that using SameSite=None, at least during login, is more secure, and that the session cookie created post-authentication should be SameSite=Strict which seems to be what the POST + Lax mitigation alludes to. I've opened #14297 to take a closer look at this and see if there is something we can provide to make that easier.

As for this ticket, I think for Spring Security to provide a by-id implementation is too early due to what I outlined above. That said, I don't mind taking a look at this ticket again once #14297 is resolved.

[kailash-everlaw]

@jzheaux Thanks for the response!

That said, it's important to remember that there are security benefits to storing it in the session. One such benefit is the natural login fixation defense it provides. For example, if an application looks the authentication request up from the session, then even if an attacker provides their own SAML response to a victim, the login will fail.

On the other hand, if we trust the InResponseTo or RelayState to retrieve the authentication request, then there's no way to know if the SAML response was requested by that handshake. I'm not certain how to address this without creating another cookie, making me think that I might as well use the session cookie after all.

Just to confirm I'm understanding correctly, are you saying that after the SAML request is sent but before the legitimate SAML response is received an attacker could send a spoofed SAML response and use that to login?

Given that, I'd argue that using SameSite=None, at least during login, is more secure, and that the session cookie created post-authentication should be SameSite=Strict which seems to be what the POST + Lax mitigation alludes to.

Assuming I'm understanding the security implications correctly, this makes sense to me. This is kind of what I had in mind when I mentioned using two different cookies - it seemed like a simple way for the pre and post-authentication cookies to have different SameSite values. However, I'm not very familiar with the internals of the SAML implementation so I'd definitely defer to your judgement about which implementation is best.

I've opened #14297 to take a closer look at this and see if there is something we can provide to make that easier.

As for this ticket, I think for Spring Security to provide a by-id implementation is too early due to what I outlined above. That said, I don't mind taking a look at this ticket again once #14297 is resolved.

FWIW my main motivation behind opening this was to try to find a way for the post-authentication cookie to be SameSite=Strict or SameSite=Lax without breaking SAML. I'm not familiar enough with SAML to know if there are other benefits to a by-id implementation, but I think the issue you opened describes the underlying problem I had in mind quite well so I would be fine with closing this issue.

[jzheaux]

an attacker could send a spoofed SAML response and use that to login?

Not quite. I'm saying that an attacker could begin a legitimate login of their own, intercept the redirect, and use that to force a victim's browser to log in as the attacker. It is a traditional fixation attack, similar to session fixation attacks where an attacker can log in legitimately and then provide a URL to a victim that contains the attacker's session id.

I think the issue you opened describes the underlying problem I had in mind quite well so I would be fine with closing this issue.

Sounds good, I'll close this issue for now. We can always reopen if we need to take another look.

[kailash-everlaw]

Not quite. I'm saying that an attacker could begin a legitimate login of their own, intercept the redirect, and use that to force a victim's browser to log in as the attacker. It is a traditional fixation attack, similar to session fixation attacks where an attacker can log in legitimately and then provide a URL to a victim that contains the attacker's session id.

Interesting, thanks for explaining that!