Cookie session ID cannot be regenerated when using a custom ticket store
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 5h
- Merged PRs (30d)
- 276
Description
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Describe the bug
[OWASP's session management cheat sheet requires that a session id is regenerated after any privilege level change](https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html#renew-the-session-id-after-any-privilege-level-change). This is required to prevent session fixation attacks.
If an application wants to change the privilege level of a user (e.g. switching from user role to admin role), currently there is no way to implement this without introducing a session fixation vulnerability when an application is configured for cookie authentication with a [SessionStore](https://github.com/dotnet/aspnetcore/blob/c0acf059eddd7e70498804dcc99a7c7b33732417/src/Security/Authentication/Cookies/src/CookieAuthenticationOptions.cs#L122).
When trying to use `SignInAsync` with the updated principal that has elevated claims, the previous session id is reused.
Attempted workarounds by first calling `SignOutAsync` does remove the session from the ticket store, but the session id is still cached in `CookieAuthenticationHandler`. So, if `SignInAsync` is called later in the same request, the previously cached session id is reused.
The behavior was introduced as part of #22135 (MR #22732) that renewed the session id rather than storing a new one.
### Expected Behavior
Calling `SignInAsync` should not make creating session-fixation bugs so easy.
### Steps To Reproduce
Repro xunit test is here: https://github.com/ccurrens/session-fixation-repro
The same code runs twice for cookie authentication, once with a custom SessionStore (failure) and once without (succeeds).
### Exceptions (if any)
_No response_
### .NET Version
.NET 5+
### Anything else?
At the very least, `SignOutAsync` should remove clear the cached `sessionKey` so that subsequent calls in the same request to `SignInAsync` can switch users roles and store with a separate session id.
It would be great if `SignInAsync` would always issue a new session id. Of course, this causes the issue in #22135, which happens when the session is being refreshed. If there was a way to pass along state perhaps in `AuthenticationProperties` to tell the handler to generate a new session key, that could also work.
I think ideally `SignInAsync` should only reuse the session id if its coming from the authentication handler (e.g., the session is being refreshed). If the call is coming from user code, it should generate a new session so that developers don't have to explicitly opt-in to behavior that would prevent session fixation (it should behave that way by default).
Contributor guide
Assessment
This issue has not been assessed yet.