Skip to content

SessionRegistry is not informed when using session-fixation-protection=changeSessionId #10242

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

Open
ghost opened this issue Sep 8, 2021 · 6 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) status: feedback-provided Feedback has been provided type: bug A general bug

Comments

@ghost
Copy link

ghost commented Sep 8, 2021

Describe the bug
I use org.springframework.security.core.session.SessionRegistry to store the sessions of 2 tomcat instances in a database.

I used version 3.1.2 of spring and spring-security in the past, which doesn't had session-fixation-protection set, so according to the docs migrateSession should be used. After upgrading all libraries (to switch from Java 8 to OpenJDK 11), it happened, that very often (or always) the session were not removed from the SessionRegistry. According to the docs the new default-value changeSessionId was used. I set session-fixation-protection="migrateSession" and everything worked again without a problem.

Versions:
Apache Tomcat 9
Spring: 5.3.6
Spring-Security: 5.4.6

This issue seems to be related, since my customer gave me the same steps to reproduce my problem:
#3704

According to this
#5439
the SessionRegistry should work with changeSessionId

To Reproduce

Expected behavior
SessionRegistry should work for every session-fixation-protection.

Sample
`


<form-login login-page="/login.jsf" 
	login-processing-url="/security_check" 
	authentication-success-handler-ref="loginSuccessHandler" />
<logout invalidate-session="false" logout-url="/logout" success-handler-ref="logoutSuccessHandler" />

<session-management 
	session-fixation-protection="migrateSession"
	invalid-session-url="/login.jsf">
	<concurrency-control max-sessions="1" 
		error-if-maximum-exceeded="true" 
		expired-url="/login.jsf" session-registry-ref="sessionRegistry" />
</session-management>

`

@ghost ghost added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Sep 8, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Feb 10, 2022

Thanks, @netty-jawn. In your description you say

I set session-fixation-protection="migrateSession" and everything worked again without a problem.

And in your sample, you use migrateSession. Given that, it's not clear to me what the problematic configuration is.

Can you please provide a reproducing sample that demonstrates the issue you are seeing?

@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 10, 2022
@jzheaux jzheaux assigned eleftherias and jzheaux and unassigned rwinch and eleftherias Feb 10, 2022
@jzheaux jzheaux added the status: waiting-for-feedback We need additional information before we can continue label Feb 10, 2022
@ghost
Copy link
Author

ghost commented Feb 14, 2022

My sample shows the working xml. If I remove the

session-fixation-protection="migrateSession"

I got my errors back.

Since I never wrote a spring-application, I can't provide a working sample. The above problem appeared in a very big project, which I took over. If I can download such a sample-application somewhere, I can probably create a working example, which shows the problem.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 14, 2022
@jzheaux
Copy link
Contributor

jzheaux commented Feb 16, 2022

@netty-jawn, you can begin from one of the samples in Spring Security Samples if you like.

@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Feb 16, 2022
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Feb 23, 2022
@ghost
Copy link
Author

ghost commented Feb 23, 2022

The only working example at the moment is the code of my customer and this code is not open-source, hence I am not allowed to share it. My customer has very limited resources, so there is no money/time for me to create another minimal example. Don't have time for this in my free time at the moment.

But I give you a short summarize of my fix, as it appears in our GitLab:

Old version:

<session-management
	invalid-session-url="/login.jsf">
	<concurrency-control max-sessions="1"
		error-if-maximum-exceeded="true" expired-url="/login.jsf"
		session-registry-ref="sessionRegistry" />
</session-management>

New version:

<session-management
	session-fixation-protection="migrateSession"
	invalid-session-url="/login.jsf">
	<concurrency-control max-sessions="1"
		error-if-maximum-exceeded="true" expired-url="/login.jsf"
		session-registry-ref="sessionRegistry" />
</session-management>

Using the code of old-version we had, despite the max-sessions="1", more than 1 session of a user in the database. For the new-version I simply added the "migrateSession"-line and suddenly there were only 1 session per user in the database. I verified this by adding logging-statements in my implementation of SessionRegistry.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Feb 23, 2022
@ghost
Copy link
Author

ghost commented Jun 22, 2022

@JP95Git

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) status: feedback-provided Feedback has been provided type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants