SEC-2856: Make cookie theft detection in remember-me service configurable because it's seriously broken

[spring-projects-issues]

Jean-Pierre Bergamin (Migrated from SEC-2856) said:

After enabling remember-me authentication for our SSO portal, people were complaining about errors they got while logging in. Those errors turned out to be CookieTheftExceptions.

After investigating quite intensively how these exceptions occured, we found that there are so many regular usecases how this can happen that this feature can be considered as really broken.

h5. Usecase 1

• Open two windows in your browser and login to the remember-me enabled web app in both windows
• Close the browser
• Open the browser (with the setting to re-open all previous windows)
• Both windows get re-opened and both send almost simultaneously a request with the same remember-me cookie to the web app
• The first request succeeds, where the second one fails (because the first already consumes the cookie) and the user is logged out

h5. Usecase 2

• Log in to the remember-me enabled web-app
• Close the browser
• Open the browser and visit the web-app again, which triggers a remember-me authentication
• The remember-me authentication takes a while (e.g. because the AD-Server responds very slowly) and the user closes the tab
• The user visits the web-app again after a while and gets a CookieTheftException and is logged out

The problem here is that the browser never got the response with the updated cookie back because the user closed the tab before.

h5. Usecase 3

• Open your remember-me enabled web-app in Chrome
• Close the browser
• Start entering the URL of your web-app in Chrome's address bar and hit enter
• You get a CookieTheftException and are logged out

What happens here is that Chrome already sends a request in the background while entering the URL. When hitting enter before the background request returned with a new cookie in its response, a second request with the same cookie is sent again - which leads to a CookieTheftException.

h5. Usecase 4

• The remember-me enabled web-app is an SSO (single sign-on) application where people authenticate for different other web-apps
• Open different web-apps which use the SSO in different tabs
• Close the browser
• Open the browser again (with the setting to re-load all previous tabs)
• The different web-apps in the different tabs need to re-login with the SSO app and immediately redirect to it after loading
• You get a CookieTheftException and are logged out

The problem here is that all webapps redirect to the SSO app and query it almost simultaneously which leads to the CookieTheftException.

As you can see, this CookieTheftException detection makes more harm than it tries to resolve. The PersistentTokenBasedRememberMeServices should have a way to disable the cookie theft detection on demand.

Currently we "disable" the cookie theft detection by always returning a constant token data like:

public class CustomPersistentTokenBasedRememberMeServices extends PersistentTokenBasedRememberMeServices {
    public CustomPersistentTokenBasedRememberMeServices(String key, UserDetailsService userDetailsService, PersistentTokenRepository tokenRepository) {
        super(key, userDetailsService, tokenRepository);
    }

    @Override
    protected String generateTokenData() {
        // Return a constant value for the token value to avoid CookieTheftExceptions.
        return "U1WUsKXNkM0Jzpozau/BeQ==";
    }
}

The PersistentTokenBasedRememberMeServices class should be configurable to have cookie theft detection turned on or off.

[devxzero]

The security model is an implementation of http://jaspan.com/improved_persistent_login_cookie_best_practice , which is not suitable for a multi threaded environment. Since the web is almost always multi threaded, it is simply broken.
A simple double click on a page refresh can trigger a CookieTheftException if the page doesn't respond quick enough.

See also https://www.drupal.org/node/327263#comment-3425002

[kromit]

So the issues is here for more than two years aready. Is there any new info on this?

I would like to see at least HttpServletRequest and username as parameters for generateTokenData. This would make it possible to generate a token value based on a secret, username and request IP, so concurrent requests would be possible without disabling cookie theft protection entirely.

[devxzero]

For those that want to avoid false cookieTheftExceptions, but still want to use the persistent remember me feature (but without cookie theft detection), the fix is pretty simply:
In PersistentTokenBasedRememberMeServices change:

PersistentRememberMeToken newToken = new PersistentRememberMeToken(
			token.getUsername(), token.getSeries(), generateTokenData(), new Date());

to:

PersistentRememberMeToken newToken = new PersistentRememberMeToken(
            token.getUsername(), token.getSeries(), token.getTokenValue(), new Date());

[weaselmetal]

I think #2648 is a duplicate of this issue.
I commented on the other issue. Should have seen this one first.
Here's my comment #2648 (comment)

[PrakashJetty]

Is there an update on this

[misisol]

For those that want to avoid false cookieTheftExceptions, but still want to use the persistent remember me feature (but without cookie theft detection), the fix is pretty simply:
In PersistentTokenBasedRememberMeServices change:

PersistentRememberMeToken newToken = new PersistentRememberMeToken(
			token.getUsername(), token.getSeries(), generateTokenData(), new Date());

to:

PersistentRememberMeToken newToken = new PersistentRememberMeToken(
            token.getUsername(), token.getSeries(), token.getTokenValue(), new Date());

There's problem with this solution. If the token is somehow stolen, the stolen token is always working and not event gets invalidated over a password change.

[NoUsername]

I guess a "slightly less broken" workaround would be to return a value that is somehow derived from the old value but in a way that an attacker cannot reconstruct reliably (taking some private-data into account as well - and it would even only have to return the same result for a very short amount of time.. a few seconds, a minute tops

so an easy solution would be that the generateTokenData method would get the old token as a parameter, then that could be used as a cache key for the new value (and the generated value could still be completely random)

or am i missing something? (for initial generation - when old token doesn't exist) a cache probably isn't necessary?

[kubav182]

I agree this implementation of remember me token services is not compatible with real world. The whitepaper may need some changes.

I did easy workaround:
Check if cache contains tokens.
When refresh session cookie is called, check the result for old tokens.
Cache the result for 30s and use the tokens as cache key.

The result is parallel requests pass. If someone steal token he has 30 minutes to use session with remember me token, with this solution he has 30 minutes and 30 seconds to use it. After this limit is cookie theft detection active. The cache time is configurable.

It does not solve all of the problems, but most of them.

[fedotxxl]

I agree this implementation of remember me token services is not compatible with real world. The whitepaper may need some changes.

I did easy workaround: Check if cache contains tokens. When refresh session cookie is called, check the result for old tokens. Cache the result for 30s and use the tokens as cache key.

The result is parallel requests pass. If someone steal token he has 30 minutes to use session with remember me token, with this solution he has 30 minutes and 30 seconds to use it. After this limit is cookie theft detection active. The cache time is configurable.

It does not solve all of the problems, but most of them.

can you share your code?