Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
saefty opened this issue Feb 6, 2024 · 4 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: bug A general bug
Milestone

Comments

@saefty
Copy link

saefty commented Feb 6, 2024

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

@saefty saefty added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Feb 6, 2024
@Akireken
Copy link

Akireken commented Feb 9, 2024

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 jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 12, 2024
@jzheaux jzheaux added this to the 6.2.2 milestone Feb 12, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Feb 12, 2024

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
Copy link

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
Copy link

How would one store this in Redis?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants