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

Changing session id upon login with Spring (session fixation protection) #6

Closed
stojsavljevic opened this issue May 10, 2016 · 4 comments
Assignees

Comments

@stojsavljevic
Copy link

Hi,

I'm trying to set up session clustering with my Spring Security application.
I configured SpringAwareWebFilter, SessionListener and SessionRegistry.

Session clustering works but I am not able to set up proper session fixation protection. I want to force changing session id on login but my Hazelcast's session id never change with Hazelcast 3.6.2.
I managed to get desired behavior with version 3.4.1.

After some debugging, it seems to me that there is no appropriate code in WebFilter 3.6.2 - after successful login old session got invalidated but new session is created with old id that is cached in SessionRegistry - clusteredSessionId attribute. With 3.4.1 version there is a code that calls creation of new session by passing null as session id (so new one is generated).

I tried this on different environments: Tomcat 7 and 8, embedded Tomcat (using Spring Boot), XML config, Java config.. In general, I want to use latest stable versions of tools like Spring, Spring Boot etc.

@mesutcelik
Copy link
Contributor

@stojsavljevic
Copy link
Author

Hi @mesutcelik ,

As far as I can see, SpringAwareWebFilterTest does not test changing session id upon login so I wrote my test and put it in SpringAwareWebFilterTest. Here is my test case:

@Test
public void test_issue_change_session_id()
        throws Exception {

    SpringSecuritySession sss = new SpringSecuritySession();
    request(RequestType.POST_REQUEST,
            SPRING_SECURITY_LOGIN_URL,
            serverPort1, sss.cookieStore);

    String hzSessionIdBeforeLogin = sss.getHazelcastSessionId();
    String jsessionIdBeforeLogin = sss.getSessionId();

    sss = login(sss, false);

    org.junit.Assert.assertNotEquals(jsessionIdBeforeLogin, sss.getSessionId());
    org.junit.Assert.assertNotEquals(hzSessionIdBeforeLogin, sss.getHazelcastSessionId());
}

and test fails in last line (when comparing Hazelcast session ids) on 3.6.2 tag version of Hazelcast.
And test passes under 3.4.1 tag.

Also, I checked Hazelcast's sample for integration with Spring Security (https://github.com/hazelcast/hazelcast-code-samples/tree/master/hazelcast-integration/spring-security). And I confirmed the issue.
With that sample, 3.6.2 version of Hazelcast and Tomcat 8.0.33 (changing session id is default session fixation protection for that Servlet 3.1) - Hazelcast's SID doesn't change and I'm getting warning:
WARN ChangeSessionIdAuthenticationStrategy : Your servlet container did not change the session ID when a new session was created. You will not be adequately protected against session-fixation attacks
Same thing happens when I set session-fixation-protection="newSession". Also, same thing with Tomcat 7.0.56.
But when I use Hazelcast 3.4.1 and session-fixation-protection="newSession" Hazelcast's SID do change on both Tomcat versions.

@bilalyasar bilalyasar self-assigned this May 25, 2016
@mesutcelik mesutcelik assigned emre-aydin and unassigned bilalyasar Sep 7, 2016
@bchupika
Copy link

bchupika commented Apr 25, 2019

Hi guys,
I found that Hazelcast removes sessions from the originalSessions map, but does not remove from sessions map.
This may cause a memory leak

private final ConcurrentMap<String, String> originalSessions = new ConcurrentHashMap(1000); //<---- OK

private final ConcurrentMap<String, HazelcastHttpSession> sessions = new ConcurrentHashMap(1000); // <------- NOT OK
3.8.3

spring security conf:
<session-management> <concurrency-control/> </session-management>

@emre-aydin
Copy link
Contributor

Hi @bchupika, it might be a good idea to create a new issue for this and link this one if it's relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants