wang-haha / memcached-session-manager

Automatically exported from code.google.com/p/memcached-session-manager
0 stars 0 forks source link

Re-using session ids breaks Spring Security #143

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. m-s-m master branch with Spring Security
2. Re-using the session id after log-in causes a redirect to login page after 
login. Possibly some kind of race condition caused by SS invalidating the 
unauthenticated session during login, not sure.

What is the expected output? What do you see instead?

Creating a new session should lead to a new JSESSIONID. That session should be 
valid.

What version of the product are you using? On what operating system?

Current master (f907715023830a853039b8747231b946a450e08c) on Ubuntu 10.04, 
tomcat6.

Please provide any additional information below.

The attached patch fixes this issue by never re-using the sessionId.

Original issue reported on code.google.com by one...@energyhub.net on 13 Jun 2012 at 9:05

Attachments:

GoogleCodeExporter commented 8 years ago
To use the specified (requested) sessionId is the behaviour of all tomcat 
session manager implementations (implemented in 
ManagerBase.createSession(String sessionId)). To change the behaviour for msm 
I'd check very carefully if this is really needed.

Can you please provide a working sample that I can use to reproduce your issue?
Alternatively provide request/response headers and debug logs of msm for the 
problematic work flow so that I can see what's happening.

Original comment by martin.grotzke on 14 Jun 2012 at 8:06

GoogleCodeExporter commented 8 years ago
Here's two MSM logs with and without the patch. In each case 

Without patch (re-use session id and getting logged out after login)
https://gist.github.com/2931870

With patch (ignore old session id, use a new one)
https://gist.github.com/2931745

Original comment by one...@energyhub.net on 14 Jun 2012 at 6:09

GoogleCodeExporter commented 8 years ago
Ah. I have disabled the Spring Security 'session fixation strategy' and it now 
works without the patch. SS must have been invalidating the authenticated 
session because the session id was unchanged. As such this is not a bug in 
m-s-m.

Perhaps there's a way to specify changing the ID for new sessions without 
patching m-s-m. I will investigate. Changing session IDs on login is desirable: 
consider a user on a public terminal. Attacker loads the login page for the app 
and steals the cookie. User comes along and logs in again, (re-using the 
session id). Now the attacker is also logged in using the old cookie.

Original comment by one...@energyhub.net on 14 Jun 2012 at 6:34

GoogleCodeExporter commented 8 years ago
Great you solved it! To prevent session fixation is the job of auth frameworks 
as spring security provides it and also tomcat container managed security (form 
based auth) does it by default.

Still, I'd also be interested why this sessionId reuse is done in tomcat. Maybe 
I'll ask this on the tomcat-dev list.

In any case, can you please share logs again with the activated session 
fixation in spring security? In the former logs there seemed to be s.th. not 
optimal with session locking mode 'auto', so I'd like to review again.

Original comment by martin.grotzke on 14 Jun 2012 at 6:50

GoogleCodeExporter commented 8 years ago
I haven't discovered a way to change the sessionId on login apart from
a) Have the 'SessionFixationStrategy' enabled in Spring Security
AND b) Have this patch in m-s-m

However...
- With these two enabled the login is slow, due to 
"WARNING: Reached timeout when trying to aquire lock for session 
FBF32ED3B90BCDF1D4E5ABB9922C9F64-n1. Will use this session without this lock."

I'm planning to leave the patch applied to my build of m-s-m, and then in my 
'admin' app enable the SS session-fixation-prevention. In my 
lower-security-requirement apps I will disable the session-fixation-prevention 
to enable faster logins. 

Later versions of tomcat have a session fixation setting, perhaps m-s-m could 
also take a setting to enable this patch's behaviour.

Original comment by one...@energyhub.net on 14 Jun 2012 at 7:19

GoogleCodeExporter commented 8 years ago
If you're using tomcat6 you can set emptySessionPath property on the connector 
to false to prevent sessionId reuse: 
http://tomcat.apache.org/tomcat-6.0-doc/config/http.html

> - With these two enabled the login is slow, due to 
> "WARNING: Reached timeout when trying to aquire lock for session 
FBF32ED3B90BCDF1D4E5ABB9922C9F64-n1. Will use this session without this lock."

What do you mean with "With these two enabled"?
Can you pleaes share logs with your desired scenario, I'd like to see what 
might be the reason for this.

> Later versions of tomcat have a session fixation setting

This hold only true for tomcat authenticators, for anything else (see e.g. 
http://www.tomcatexpert.com/blog/2011/04/25/session-fixation-protection). Or is 
there anything I missed?

Btw, here's my request on the tomcat-dev list: 
http://tomcat.10.n6.nabble.com/Why-does-Manager-createSession-String-take-a-sess
ionId-td4982635.html

Original comment by martin.grotzke on 14 Jun 2012 at 7:53

GoogleCodeExporter commented 8 years ago
I already saw in your previously shared logs (https://gist.github.com/2931870) 
that on session invalidation the session lock was not freed.
I submitted this as issue #144 and just fixed it.

Though, this shouldn't be the explanation for the lock-acquiry-timeout-warning 
with session-fixation-protection as in this case the sessionId should be 
changed and the still existing lock doesn't cause any harm.

So it would be good if you could share logs again so that I can have a look.

Original comment by martin.grotzke on 14 Jun 2012 at 8:21

GoogleCodeExporter commented 8 years ago
Anyway, please can you checkout the attached jars with the fix for #144?

Original comment by martin.grotzke on 14 Jun 2012 at 8:27

Attachments:

GoogleCodeExporter commented 8 years ago
I will post the logs and test the fix for #144 as soon as I get a chance.

Yes, we are using emptySessionPath cookies, thanks for that tidbit. We did it 
on purpose because we serve the same app via several different paths and proxy 
to the real path in Apache. We could probably re-write the cookie in Apache 
instead but it's a bit messy.

By 'with these two enabled' I mean 1) Spring Security 
session-fixation-prevention and 2) My patch to ignore the sessionId in m-s-m.

Original comment by one...@energyhub.net on 14 Jun 2012 at 8:33

GoogleCodeExporter commented 8 years ago
An update:

We have moved away from empty session path cookies* and the original issue of 
re-using session IDs is not a problem in that configuration, so we've dropped 
the patch from above that forces new session ids. This is more a bug in tomcat 
than m-s-m.

We have also re-enabled Spring Security's session fixation protection which now 
works just fine.

Thanks again for your help Martin.

* We are re-writing the cookie path in apache using mod_headers (in order to 
serve one tomcat app on multiple paths via apache reverse proxy).

Original comment by one...@energyhub.net on 27 Jun 2012 at 9:18

GoogleCodeExporter commented 8 years ago
Which version of msm were you using now?

Original comment by martin.grotzke on 28 Jun 2012 at 8:40

GoogleCodeExporter commented 8 years ago
Using master as of yesterday.

Original comment by one...@energyhub.net on 28 Jun 2012 at 11:28

GoogleCodeExporter commented 8 years ago
Ok, good to know the new version is fine. If you want you can send logs so that 
I can double check that auto locking behaves as it should.

Original comment by martin.grotzke on 28 Jun 2012 at 5:05