Bug 55521 - Race Condition in HttpSession#invalidate() / HttpServletRequest#getSession(boolean)
Summary: Race Condition in HttpSession#invalidate() / HttpServletRequest#getSession(bo...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Servlet & JSP API (show other bugs)
Version: 7.0.40
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-04 10:24 UTC by Christoph
Modified: 2013-09-05 16:28 UTC (History)
0 users



Attachments
code flow that exhibits the race condition (3.83 KB, text/plain)
2013-09-04 10:24 UTC, Christoph
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph 2013-09-04 10:24:05 UTC
Created attachment 30798 [details]
code flow that exhibits the race condition

For session fixation protection, we have to discard a user's session and create a new one whenever the user's login state changes. For this we rely on Spring Security's SessionFixationProtectionStrategy that, at its core, uses the following commands:

  session.invalidate();
  session = request.getSession(true);

Yesterday, we had a message in the log that indicates the latter command returned the same session that was invalidated in the line before:
"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 (catalina-exec-339, org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy, SessionFixationProtectionStrategy.java:102)"

When I investigated this issue, I found there is in fact a race condition if two threads (associated with requests from the same client) enter the session fixation protection code in parallel. I attached a TXT file that illustrates the code flow that leads to the race condition: When thread B calls session.invalidate(), the call returns immediately becuase the session is already in the "expiring" state. Since the session is not invalid yet, the call to request.getSession(true) won't create a new session, though. So in effect, thread B cannot obtain a new session.

The documentation at http://tomcat.apache.org/tomcat-7.0-doc/servletapi/ has no indication that a session may not yet be invalid when session.invalidate() returns. The session interface neither provides a way to detect "expiring" session.

The error message appears only once in the production log files that go some weeks back, so it seems to be an infrequent event. Nevertheless, it should be possible to implement session fixation without a race condition.

Regards
Christoph
Comment 1 Mark Thomas 2013-09-05 14:53:22 UTC
I've taken a look at this and there are some things we can do in Tomcat to ensure that a call to invalidate() doesn't return until the session has been invalidated.

However, there may still be an issue that needs fixing in Spring Security. Looking at SessionFixationProtectionStrategy.applySessionFixation() it is possible (although even less likely than the issue you have seen) for concurrent requests to generate a series of invalidate / create / invalidate / create etc. events. It is pretty unlikely but is possible. Since I work for Pivotal, I'll ping one of the developers.
Comment 2 Christoph 2013-09-05 15:27:21 UTC
Mark, it would be great if you'd ask one of your colleagues to take a look at the additional issues in Spring Security you observed.

As far as Tomcat is concerned, the race condition I observed would no longer exist if the early check of the expiring field before the synchronized block is entered would be removed. Of course, I don't now whether this check is merely the result of an over-eager optimization or whether it is needed in some situation I am not aware of.
Comment 3 Mark Thomas 2013-09-05 16:13:35 UTC
Thsi has been fixed in trunk and 7.0.x and will be included in 8.0.0-RC2 and 7.0.43 onwards.
Comment 4 Christoph 2013-09-05 16:28:21 UTC
Thanks for the very prompt fix!

Regards
Christoph