benlucchesi / grails-cookie-session

cookie sessions for grails applications
28 stars 32 forks source link

Session Bleeding in plugin. #65

Open rubindersingh opened 8 years ago

rubindersingh commented 8 years ago

Hi,

We are using cookie session plugin cookie-session-2.0.18 to maintain session and we have configured it in spring security compatibility mode. So, we are saving security context in session before serialization and reading security context again after deserilizing session in next request. We are using KRYO as serializer. We have encountered few incidents of session bleeding in which users are getting switched to some other user's session while interacting with app. This happens only when app is under load and apdex score is falling leading to app crash. We have tried to debug problem and found out that code in SecurityContextSessionPersistenceListener in which security context is set from SCH to session variable SPRING_SECURITY_CONTEXT. We have also put logs in this part and tried to replicate problem thinking of this snippet as our culprit. We did not find any problem with this module. Digging further, I came to know that KRYO serializer is not thread safe and we are using kryo serializer in our configuration to serialize session. I found out that KRYO serializer is initialized in a way in KryoSessionSerializer.groovy's getConfiguredKryoSerializer method which is not thread safe. I am adding few first lines of this method below:

` private def getConfiguredKryoSerializer(){

log.trace "configuring kryo serializer"
def kryo = new Kryo()
kryo.setAsmEnabled(true)
`

I think this could be leading to session bleeding in our case. Please share your opinion on the same. It would be really appreciated if you could fix this asap.

Regards, Rubinder Singh

benlucchesi commented 8 years ago

Hi Rubinder,

I've been thinking about this problem all day and I can't see how the cookie-session plugin could be responsible for sessions bleeding between users. You are correct that the kryo serializer (like most serializers) isn't thread safe - this means that a single instance of the serializer can't be shared between multiple threads. A new instance of the serializer must to be constructed for each request, which is exactly what the KryoSessionSerializer does. getConfiguredKryoSerializer constructs and configures an instance of the serializer, returns it to the methods that is either serializing or deserializing the session, it gets used, then its disregarded.

Thinking through possible scenarios... here's a couple of places I'd start looking for the issue:

1) is there anywhere in your code that manually log a user in by calling springSecuriryService.reauthenticate() or manually assign an authentication token to the thread's security context? If so, is it possible that a bad user id is used to look up the user? I'd fgrep the code base to search for EVERY POSSIBLE use of the springSecurityService and security context and put them under a magnifying glass.

2) are you encrypting the cookie sessions? If not, do it. this will prevent rogue users from modifying the cookie and trying to hijack another users account.

3) are you load balancing the app between servers? if so, is it possible that your load balancer may be getting overwhelmed and allowing cross talk between connections to occur? This would be EXTREMELY unlikely, but then again, this entire situation is extremely unlikely.

4) are you using HTTPS? if not, you should be - it will help ensure that content can't be replayed on connection.

5) do you know anything about the users who are experiencing this problem? If so, can you determine if they're connecting through a proxy server or NAT server? If so, that device/software may be screwing up. Again, another reason to use HTTPS from end-to-end. It virtually guarantees that if cross talk does occur, then the connection will be lost because the host won't be able to decrypt the packets its receiving from a different connection.

6) is your application running out of memory?

Something to keep in mind about the kryo serializer is that its extremely fragile. It will break in spectacular and unimaginable ways at the sign of the slightest error in the data being deserialized. This is both a good and a bad thing - on the deserialization side, this works in our favor because at the first sign of an error, the session is completely lost because it can't be deserialized. This makes it virtually impossible for a serialized session to be inadvertently created that maps to a different user.

If you are in a position to share more about the architecture of your application or possibly code, I could look through it if you're interested. Otherwise, trying to solve this problem on my end would be like throwing darts blindfolded. I have no way to reproduce the error myself and no way to quality whether the solution is working. If on the other hand, you implement code in a fork of the cookie-session that proves to fix the issue, I'd be interested in seeing it.

-ben

On Tue, May 17, 2016 at 12:01 AM, rubindersingh notifications@github.com wrote:

Hi,

We are using cookie session plugin to maintain session and we have configured it in spring security compatibility mode. So, we are saving security context in session before serialization and reading security context again after deserilizing session in next request. We are using KRYO as serializer. We have encountered few incidents of session bleeding in which users are getting switched to some other user's session while interacting with app. This happens only when app is under load and apdex score is falling leading to app crash. We have tried to debug problem and found out that code in SecurityContextSessionPersistenceListener in which security context is set from SCH to session variable _SPRING_SECURITYCONTEXT. We have also put logs in this part and tried to replicate problem thinking of this snippet as our culprit. We did not find any problem with this module. Digging further, I came to know that KRYO serializer is not thread s afe and we are using kryo serializer in our configuration to serialize session. I found out that KRYO serializer is initialized in a way in KryoSessionSerializer.groovy's getConfiguredKryoSerializer method which is not thread safe. I am adding few first lines of this method below:

` private def getConfiguredKryoSerializer(){

log.trace "configuring kryo serializer" def kryo = new Kryo() kryo.setAsmEnabled(true) `

I think this could be leading to session bleeding in our case. Please share your opinion on the same. It would be really appreciated if you could fix this asap.

Regards, Rubinder Singh

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/benlucchesi/grails-cookie-session/issues/65