keycloak / keycloak

Open Source Identity and Access Management For Modern Applications and Services
https://www.keycloak.org
Apache License 2.0
22.89k stars 6.7k forks source link

Sign the AUTH_SESSION_ID cookie value #34027

Open mposolda opened 3 hours ago

mposolda commented 3 hours ago

Description

Can we sign the value of AUTH_SESSION_ID cookie to make sure that it is integrity checked?

I am not sure about the performance impact of this, but hopefully can be acceptable as we can sign with symmetric key (possibly something like HMAC SHA256), which doesn't have so big overhead. This may require some changes in the cookie creation and validation and we should make sure that the cookie is validated, but it is validated just once per request (hopefully value can be cached as an attribute in KeycloakContext or eventually AuthenticationSession itself, which is already there, but it can be nice to doublecheck the existing codebase to make sure that we don't validate signature multiple times per request, but exactly once). Anyway, we should probably test the performance of this before/after this change...

Also we use the authSessionId in some endpoints (like LoginActionsService), so this might be affected too.

NOTE: This change could be a concern for backwards compatibility for the cluster-like deployments, which would use the new servers (with this change) combined with the old servers (without this change). That is due the format of the cookie would be incompatible. So if we want to support zero-downtime upgrade for cluster deployments, then this change probably should not go to the micro-release, but only to the minor releases.

Discussion

No response

Motivation

No response

Details

No response

ahus1 commented 2 hours ago

I would also like to understand if this would affect only authentication sessions, or also also already logged in users with their user session. If this change would log-out all currently logged in users, we should try to avoid it.