Spring Security
  1. Spring Security
  2. SEC-1396

Race condition between HttpSessionContextIntegrationFilter and SessionFixationProtectionFilter

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.5
    • Fix Version/s: 3.0.2
    • Component/s: Web
    • Labels:
      None
    • Environment:
      Linux, Jetty 6.1.21, Firefox 3.5/3.6 (possibly IE7/8)

      Description

      There seems to be a race condition between processing in the HttpSessionContextIntegrationFilter and the SessionFixationProtectionFilter.

      Here is our problem.

      We have a web application with a fairly heavy home page that can be served quite slowly depending on the size of certain attributes associated with the user. We are currently using Jetty 6.1.21 which supports HTTP 1.1 chunked responses.

      If a user opens their browser and goes directly to the home page URL using a RememberMe token, this initial web request executes down through the filter stack and into the Spring MVC framework. Steps 1a - 1d describe the processing of this initial request:

      1a. The HttpSessionContextIntegration filter creates a new HttpSession and puts a default SecurityContext into the ThreadLocal handler.

      1b. The RememberMeProcessingFilter authenticates the token and sets the user's Authentication object into the ThreadLocal handler.

      1c. The SessionFixationProtectionFilter detects a new authentication, invalidates the existing HttpSession, and creates a new HttpSession (migrating the attributes). It does not set the user's Authentication into the HttpSession – this is supposed to be done by the HttpSessionContextIntegration after all request processing is complete.

      1d. The request goes into the Spring MVC framework, causing a response to start to be sent to the client using HTTP chunking. This response includes the session id of the newly created session (created by the SessionFixationProtectionFilter).

      The client begins receiving the markup, which includes various <script> elements to script files served by the web app. Steps 2a - 2c describe the processing of this second request. This processing takes place while the initial request is still being processed. This second request sends over the session id created by the SessionFixationProtectionFilter for the first request.

      2a. The HttpSessionContextIntegration puts a default SecurityContext into the ThreadLocal handler (since one has not yet been set into the HttpSession).

      2b. The RememberMeProcessingFilter authenticates the token and sets the user's Authentication object into the ThreadLocal handler.

      2c. The SessionFixationProtectionFilter detects another new authentication, invalidates the existing HttpSession, and creates a new HttpSession (migrating the attributes).

      Processing of the initial web request is continuing through the Spring MVC framework. We happen to use JSP for our view technology.

      1e. Continued processing of a JSP page (or import) for the initial request causes an attempt to deference a variable, causing a IllegalStateException in Jetty's AbstractSessionManager.Session class.

      I can "fix" this by extending the SessionFixationProtectionFilter and overriding

      protected void startNewSessionIfRequired(
      HttpServletRequest request, HttpServletResponse response)
      {
      super.startNewSessionIfRequired(request, response);
      request.getSession().setAttribute(
      HttpSessionContextIntegrationFilter.SPRING_SECURITY_CONTEXT_KEY,
      SecurityContextHolder.getContext().getAuthentication());
      }

      so that the user's Authentication object is stored into the HttpSession before processing enters the Spring MVC framework, causing the execution of filters on the second request to not be considered a new authentication, thus preserving the validity of the session created during the initial request.

      Is this the right way to do this? It seems to keep session fixation protection working the way it is supposed to, but I'm a little worried about what will happen with the OnRedirectUpdateSessionResponseWrapper created by the HttpSessionContextIntegrationFilter if the request processing results in a sendError() or sendRedirect(). My casual reading of HttpSessionContextIntegrationFilter seems to indicate it would still work correctly.

      Any thoughts or advice would be appreciated.

        Activity

        Hide
        Luke Taylor added a comment -

        I don't see any immediate problem with eagerly storing the security context in the session and its probably something we should consider in Spring Security 3 in order to prevent recreating the session multiple times.

        Could you explain the details of 1e? What is the Jetty exception and do you consider it to be a Jetty bug? Do you see the same behaviour in a later version?

        Other things you might try include:

        1. Disabling session-fixation protection and recreate the session yourself, at a point in your app when you know it is safe to do so.
        2. Omit supplementary files like .js and images from the filter chain
        3. Adjust the logic in your app so that authenticated users end up at a standard welcome page in which you can standardize the loading behaviour, so that a single session and security context are always established before proceeding to parts ofthe app which may submit multiple concurrent requests. This may or may not be possible depending on the nature of the app.

        Show
        Luke Taylor added a comment - I don't see any immediate problem with eagerly storing the security context in the session and its probably something we should consider in Spring Security 3 in order to prevent recreating the session multiple times. Could you explain the details of 1e? What is the Jetty exception and do you consider it to be a Jetty bug? Do you see the same behaviour in a later version? Other things you might try include: 1. Disabling session-fixation protection and recreate the session yourself, at a point in your app when you know it is safe to do so. 2. Omit supplementary files like .js and images from the filter chain 3. Adjust the logic in your app so that authenticated users end up at a standard welcome page in which you can standardize the loading behaviour, so that a single session and security context are always established before proceeding to parts ofthe app which may submit multiple concurrent requests. This may or may not be possible depending on the nature of the app.
        Hide
        Luke Taylor added a comment -

        I've added the eager saving of the request as I don't see a problem with this.

        Show
        Luke Taylor added a comment - I've added the eager saving of the request as I don't see a problem with this.
        Hide
        Brien Wheeler added a comment -

        Luke, thanks for the fix. I love Spring Security – it's hugely valuable to me.

        To answer your other questions – the Jetty error was an exception in PageContextImpl, I believe (I don't have stack trace immediately available to me). I don't know that it should be considered a Jetty (Jasper?) error, although not having dug into it enough I'm really not sure. It depends on what you think should happen when a variable is trying to be dereferenced and the pages HttpSession has been invalidated underneath page processing.

        We probably should trim our spring security filter mapping down to not include static content such as javascript, et al, but that might simply kick the can down the road a little bit, with the underlying issue waiting to haunt us again later.

        BTW, I recently had another interesting Spring Security configuration scenario in which I had to copy some Spring Security source code into my own classes and refactor a little to get the configuration I wanted – if you're interested in hearing details about that to accomodate a little more flexibility in a future design email me at brien.wheeler@currensee.com. (Of course, you might have already improved this area – I'm running 2.0.5.RELEASE.)

        Cheers,
        Brien

        Show
        Brien Wheeler added a comment - Luke, thanks for the fix. I love Spring Security – it's hugely valuable to me. To answer your other questions – the Jetty error was an exception in PageContextImpl, I believe (I don't have stack trace immediately available to me). I don't know that it should be considered a Jetty (Jasper?) error, although not having dug into it enough I'm really not sure. It depends on what you think should happen when a variable is trying to be dereferenced and the pages HttpSession has been invalidated underneath page processing. We probably should trim our spring security filter mapping down to not include static content such as javascript, et al, but that might simply kick the can down the road a little bit, with the underlying issue waiting to haunt us again later. BTW, I recently had another interesting Spring Security configuration scenario in which I had to copy some Spring Security source code into my own classes and refactor a little to get the configuration I wanted – if you're interested in hearing details about that to accomodate a little more flexibility in a future design email me at brien.wheeler@currensee.com. (Of course, you might have already improved this area – I'm running 2.0.5.RELEASE.) Cheers, Brien

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Brien Wheeler
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: