spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.81k stars 5.9k forks source link

CSRF protection should not create a HTTP session if session creation policy is set to STATELESS #5299

Open mvitz opened 6 years ago

mvitz commented 6 years ago

Summary

By default CSRF protection creates a HTTP session even if session creation policy is configured to be STATELESS. This should not be the case as an user expects no session to occur (within Spring Security) after specifying STATELESS.

Actual Behavior

A HTTP session is created when visiting the login page.

Expected Behavior

No HTTP session should be created.

Configuration

@Bean
public WebSecurityConfigurerAdapter webSecurityConfigurerAdapter() {
    return new WebSecurityConfigurerAdapter() {
        @Override
        protected void configure(HttpSecurity http) throws Exception {
             http
                 .sessionManagement().sessionCreationPolicy(STATELESS)
             .and()
                 .authorizeRequests().anyRequest().authenticated()
             .and()
                 .formLogin()
        }
    };
}

Version

5.0.4.RELEASE

Sample

A sample application (using Spring Boot 2.x) can be found at https://github.com/mvitz/spring-security-csrf-issues/tree/master/stateless-session-creation-policy

streetpoet commented 6 years ago

Since the CsrfFilter is in the security chain, if user hope to disable session creation completely, they need disable csrf functionality (filter) explicitely by 'http.csrf().disable()'. Base on decouple design, CsrfFilter should know nothing about SessionManagementFilter.

mvitz commented 6 years ago

I'm not suggesting any coupling between these two filters.

The JavaDoc for STATELESS and NEVER in SessionCreationPolicy both start with Spring Security will never create an {@link HttpSession}.

Unfortunately this does not hold true in this scenario.

One way to solve this issue would be to use an CookieCsrfTokenRepository if the user chooses STATELESS or to inject a boolean (allowSessionCreation) into the HttpSessionCsrfTokenRepository like it's done within HttpSessionSecurityContextRepository. The CsrfConfigurer already has a dependency to SessionManagementConfigurer and could therefore easily detect if session creation is allowed or not.

streetpoet commented 6 years ago

I got you, I totally agree with you that the document should use accuracy statement and note any exceptions. for the reality it's weird that we use CSRF filter as well as we don't use session. because it's only meaningful when request is stateful, (from the definition of CSRF). if the calling is stateless, it also no need to check CSRF, right? so the question is converted to how to ensure these two things keep the same logic. using CookieCsrfTokenRepository can't resolve this issue gracefully, since it will still create cookie on client's browser, right? from reading source code of CsrfConfigurer, from its intent this configurer should be used to config CsrfFilter, which has responsibility to do right logic based on all the dependencies it needs. So we can make this CsrfConfigurer more smart to handle this situation.

eg: we can add adjustment like this in the CsrfConfigurer.java#configure if (sessionConfigurer.getSessionCreationPolicy() == SessionCreationPolicy.STATELESS) { disable(); return; }

rwinch commented 6 years ago

The configuration is only around how authentication is resolved. It does not impact csrf or saving of requests when authentication is required. You must configure those separate.

mvitz commented 6 years ago

Maybe updating the Javadoc of STATELESS would be an option then?

rwinch commented 6 years ago

That's a good idea. Would you be interested in submitting a PR?

streetpoet commented 6 years ago

that means the end users should be responsible to make no conflict when they use both session and csrf, but I thought the most end users won't realize this point 🙂, is there any good way to solve it in code level?

rwinch commented 6 years ago

@streetpoet Unfortunately, I don't see a good solution to this. Having a switch that disables sessions entirely means that we implicitly disable security features like CSRF protection that users might not even be aware of. This would most certainly cause security bypasses in applications. I much prefer that the configuration must be explicit.

One thing we might do deprecate the method and name it something more explicit. As pointed out by @mvitz we could also improve the documentation.

samiconductor commented 6 years ago

I'm running into this issue as well. I set session creation to STATELESS and a CSRF token is generated on every request that has a valid auth token. New CSRF tokens on each request quickly leads to invalid CSRF token errors with concurrent requests/responses in the client.

Here are the lines that lead to a new CSRF token on every request:

A possible work around is removing the CsrfAuthenticationStrategy from SessionManagementConfigurer. Then make a request to an endpoint with CSRF ignored and let the CsrfFilter generate the token when it's missing (the request is ignored after the token is generated). Unfortunately, SessionManagementConfigurer configurer is very intentional about preventing any session auth strategies from being modified which rules out this approach.

I don't see any other workarounds that don't involve a code change.

I think a solution would be to skip adding the CsrfAuthenticationStrategy when the session creation policy is STATELESS. Similar to @streetpoet's suggestion but don't disable CSRF entirely, just the CsrfAuthenticationStrategy.

SessionManagementConfigurer<H> sessionConfigurer = http
        .getConfigurer(SessionManagementConfigurer.class);
if (sessionConfigurer != null && sessionConfigurer.getSessionCreationPolicy() != SessionCreationPolicy.STATELESS) {
    sessionConfigurer.addSessionAuthenticationStrategy(
            new CsrfAuthenticationStrategy(this.csrfTokenRepository));
}

Thanks!

mvitz commented 6 years ago

@samiconductor besides my described stuff I stumbled over your problem, too. Maybe #5300 is related to you as well.

tpredzymyrskyi commented 6 years ago

@samiconductor Have you found decision how to solve this problem? I have sometimes errors when client sends two requests at the same time, cuz spring generates new csrf for each.

samiconductor commented 6 years ago

@tararas124 nope. I've left CSRF disabled. I don't think it's possible with STATELESS sessions without a code change. Waiting on @mvitz PR in #5300.

tpredzymyrskyi commented 6 years ago

@samiconductor man, what do you think about this approach of csrf protection stateless api https://blog.jdriven.com/2014/10/stateless-spring-security-part-1-stateless-csrf-protection/ ???

mvitz commented 6 years ago

@samiconductor I tried to create a PR (current state can be seen in https://github.com/mvitz/spring-security/commit/c6d9eb88565f908b07aac5cd1368cb8b2dd531bc) but I'm unable to get a proper working test.

Maybe you can point me somewhere I can get help for this?

samiconductor commented 6 years ago

@mvitz sure. I'll give it a shot this weekend.

samiconductor commented 6 years ago

If you set a breakpoint on L94 of the SessionManagementFilter and debug the test trustResolver.isAnonymous is returning true which means we never get to the next line that would call the custom session auth strategy.

How do you make the trust resolver no longer anonymous? Not sure. Anyone know?

stochmalm commented 5 years ago

How about using synchronizer token pattern by default if session creation policy is set to STATELESS? This also has some drawbacks, but stateless app would be at least a little bit safer and it wouldn't create session without developer knowledge.

serhanozbey commented 5 years ago

Had the same issue with a project with following structure:

The problem was after authentication, the page would return with initial _csrf hidden fields embedded to POST forms, but CSRF cookie token would be incremented for each of the resources loaded. (such as CSS and JS). Any of the generated POST forms were ending up with InvalidCsrfTokenException.

My solution (or hack if you may call it) was like the following:

WebSecurityConfig.java (extends WebSecurityConfigurerAdapter)

.sessionManagement()
    .sessionCreationPolicy(SessionCreationPolicy.STATELESS)
    .sessionAuthenticationStrategy(ThymeleafStatelessSessionAuthenticationStrategy
        .builder()
        .authenticationEndpointURIs(List.of("/dashboard")) //the endpoint after authentication
        .build())
.and()
    .csrf()
    .csrfTokenRepository(new CookieCsrfTokenRepository())

ThymeleafStatelessSessionAuthenticationStrategy.java

@lombok.Builder
public class ThymeleafStatelessSessionAuthenticationStrategy implements SessionAuthenticationStrategy {
    private final List<String> authenticationEndpointURIs;
    @Override
    public void onAuthentication(Authentication authentication, HttpServletRequest request, HttpServletResponse response) throws SessionAuthenticationException {
        if (!authenticationEndpointURIs.contains(request.getRequestURI()) && authentication.isAuthenticated()) {
            Optional<Cookie> token = CookieUtils.getCookie(request, "XSRF-TOKEN");
            token.ifPresent(cookie -> {
                String csrfToken = cookie.getValue();
                CookieUtils.deleteCookie(request,response,"XSRF-TOKEN");
                CookieUtils.addCookie(response,"XSRF-TOKEN",csrfToken,-1);
            });
        }
    }
    public void addEndpoint(String uri) {
        authenticationEndpointURIs.add(uri);
    }
    public void removeEndpoint(String uri) {
        authenticationEndpointURIs.remove(uri);
    }   
}

In this way, the CSRF token embedded into Thymeleaf template is similar with the CSRF cookie token. This is because we are avoiding the if (containsToken) at line 25 at the CsrfAuthenticationStrategy.java for the GET requests for additional resources. Also we still change CSRF after authentication for the given authentication URIs.

Please let me know if this would be helpful or if you see a security concern with this approach. Tagging @semiconductor, @mvitz and @streetpoet for their visibility.

chrylis commented 3 years ago

Came here to note the same; in my case, caused by NEVER. I'm not unhappy with the results of the implementation of SaveOnAccessCsrfToken, but it contradicts clear and explicit documentation on session-creation policy.

JimHosken commented 3 years ago

Had the same issue with a project with following structure:

  • Thymeleaf templates
  • JWT authentication cookies
  • CSRF cookie

The problem was after authentication, the page would return with initial _csrf hidden fields embedded to POST forms, but CSRF cookie token would be incremented for each of the resources loaded. (such as CSS and JS). Any of the generated POST forms were ending up with InvalidCsrfTokenException.

My solution (or hack if you may call it) was like the following:

WebSecurityConfig.java (extends WebSecurityConfigurerAdapter)

.sessionManagement()
  .sessionCreationPolicy(SessionCreationPolicy.STATELESS)
  .sessionAuthenticationStrategy(ThymeleafStatelessSessionAuthenticationStrategy
      .builder()
      .authenticationEndpointURIs(List.of("/dashboard")) //the endpoint after authentication
      .build())
.and()
  .csrf()
  .csrfTokenRepository(new CookieCsrfTokenRepository())

ThymeleafStatelessSessionAuthenticationStrategy.java

@lombok.Builder
public class ThymeleafStatelessSessionAuthenticationStrategy implements SessionAuthenticationStrategy {
  private final List<String> authenticationEndpointURIs;
  @Override
  public void onAuthentication(Authentication authentication, HttpServletRequest request, HttpServletResponse response) throws SessionAuthenticationException {
      if (!authenticationEndpointURIs.contains(request.getRequestURI()) && authentication.isAuthenticated()) {
          Optional<Cookie> token = CookieUtils.getCookie(request, "XSRF-TOKEN");
          token.ifPresent(cookie -> {
              String csrfToken = cookie.getValue();
              CookieUtils.deleteCookie(request,response,"XSRF-TOKEN");
              CookieUtils.addCookie(response,"XSRF-TOKEN",csrfToken,-1);
          });
      }
  }
  public void addEndpoint(String uri) {
      authenticationEndpointURIs.add(uri);
  }
  public void removeEndpoint(String uri) {
      authenticationEndpointURIs.remove(uri);
  }   
}

In this way, the CSRF token embedded into Thymeleaf template is similar with the CSRF cookie token. This is because we are avoiding the if (containsToken) at line 25 at the CsrfAuthenticationStrategy.java for the GET requests for additional resources. Also we still change CSRF after authentication for the given authentication URIs.

Please let me know if this would be helpful or if you see a security concern with this approach. Tagging @semiconductor, @mvitz and @streetpoet for their visibility.

This issue is really old, but I am in a similar position. I have spring boot applications which use Stateless session creation policy, and also use proxy authentication from an upstream service, which does hold a session. Every request to my application contains an Authorization header with a Bearer JWT token. Because of this, my applications are vulnerable to CSRF attacks. I want to use the Synchronizer Token Pattern via CookieCsrfTokenRepository- but each request to the application causes a new CSRF Token to be generated, so the one set as the _csrf hidden field on the Thymeleaf template is invalid when the form is POSTed.

@serhanozbey 's approach as well as using NullSessionAuthenticationStrategy both seem to work. Has there been an official recommendation made for what to do in this scenario, that I have missed?

schnapster commented 2 years ago

It's not just CSRF. The SimpleUrlAuthenticationFailureHandler and HttpSessionRequestCache will also, by default, create new sessions (under certain circumstances), ignoring the SessionCreationPolicys NEVER and STATELESS. Make sure to configure them appropriately as well when trying to get Spring to stop creating new sessions.