spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.75k stars 5.88k forks source link

Spring Security 6.2 defaults to InMemoryOidcSessionRegistry causing memory leaks in distributed systems with external session storage #14558

Closed saefty closed 7 months ago

saefty commented 8 months ago

Describe the bug

Default configuration of InMemoryOidcSessionRegistry causes memory leak in cloud environments using external session storage systems such as org.springframework.session:spring-session-data-redis. This default caused JVM OOMs in our production system for multiple days until we boiled it down using heap dumps from production containers.

image

This was introduced with spring security 6.2 or might happen if you update from spring boot starter 3.1.6 to 3.2.x while using spring session with an external storage.

To Reproduce

  1. Use Spring Security >= 6.2, Spring Data Session Redis or a spring boot starter app with version >= 3.2 Excerpt from the build.gradle

        id 'org.springframework.boot' version '3.2.2'
        // [...]
        implementation 'org.springframework.boot:spring-boot-starter-web'
        implementation 'org.springframework.boot:spring-boot-starter-oauth2-client'
        implementation 'org.springframework.boot:spring-boot-starter-actuator'
        implementation 'org.springframework.boot:spring-boot-starter-data-redis'
        implementation 'org.springframework.session:spring-session-data-redis'
    
        implementation 'redis.clients:jedis:5.1.0'
        implementation 'io.lettuce:lettuce-core:6.3.1.RELEASE'
  2. Configure app to use redis as a leading session storage system.
  3. Set break points in the InMemoryOidcSessionRegistry.saveSessionInformation
  4. Create multiple sessions with different users
  5. The Number of objects in the ConcurrentHashMap will increase constantly.

Expected behavior

This might be a communication issue in the documentation of the change logs. Setting the default to InMemoryOidcSessionRegistry is a breaking change for applications that are running in a cloud environment with non-sticky sessions.

I would either recommend removing the default or clearly communicating this as a breaking change in the behavior of spring security. This change requires configuration changes to stay functional.

Related Documentation

Our mitigation

We would mitigate this issue by providing a custom OidcSessionRegistry implementation that does not store the session information in memory. Spring Data Redis is already configured to store the session information in Redis. We also implemented a custom back-channel logout endpoint to remove the session information from the Redis store by wiping the http session. In this particular case, we would use the NoopSpringDataOidcSessionsStrategy to avoid the memory leak until further support for external data storage is added.

public class NoopSpringDataOidcSessionsStrategy implements OidcSessionRegistry {
  @Override
  public void saveSessionInformation(OidcSessionInformation info) {
  }

  @Override
  public OidcSessionInformation removeSessionInformation(String clientSessionId) {
    return null;
  }

  @Override
  public Iterable<OidcSessionInformation> removeSessionInformation(OidcLogoutToken logoutToken) {
    return null;
  }
}

  public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
    http
        .oauth2Login(oauth2Login -> oauth2Login.oidcSessionRegistry(new NoopSpringDataOidcSessionsStrategy()));
  }

Sample

Creating a sample costs a lot of time and is not feasible for us at the moment. We are happy to provide a sample if it is required.

Related issues

Akireken commented 8 months ago

I had the exactly same problem since I bump Spring Security from 6.1.5 to 6.2.0.

Thanks for the work around @saefty .

jzheaux commented 7 months ago

Thanks for reporting this, @saefty! Also thank you to @jheizmann who reported this through a separate channel.

This is fixed now and should go out in the next release. If you are able, please test with 6.2.2-SNAPSHOT to confirm the fix.

dreamstar-enterprises commented 1 month ago

The documentation is still out of date: https://docs.spring.io/spring-security/reference/reactive/oauth2/login/logout.html#_customizing_the_oidc_provider_session_strategy

dreamstar-enterprises commented 1 month ago

How would one store this in Redis?