cc @zerkms as this bug was reintroduced for us in #47488 - I would like to discuss the rationale behind this fix. I agree it would allow an attacker that has stolen a remember me cookie to permanently do requests to try and steal a new remember me cookie after a user reauthenticates. However, if you stole a remember me cookie you can just use it to open a session and then keep using that session for "ever" as long as you keep sending requests to keep it alive. So all in all I am unsure what the security benefits are, and it definitely causes problems in our real world usage.

Especially as the fix closed the window of the first 60 seconds after the login. But when the cache is longer lived a later authentication via RememberMe will return the new token. So the change is only a "security feature" (huge quotes) when the caching backend also expires the old tokens after 60 seconds. Which is not a given as the cache backend can be built custom-sided.

So the change is only a "security feature"

@heiglandreas I didn't try to sell my PR as a "security feature" - it was fixing a real typical use case scenario that was breaking users' experience.

The problem happens when the request that wins the race doesn't actually return to the client

It's not a problem of my PR, that's how rememberme protocol is designed to work: you exchange one token with the other. You have one chance for that. A lot of protocols work the same way (eg: oauth)

Always return the current RememberMe token when logging in via cached Rememberme token.

This breaks the security of the idea of series-based rememberme. This solution should come with removal of series-based logic, as it would become just unnecessary complexity and then detailed explanation of the new protocol. Also, it will reintroduce #42343 which hits happy path users.

So the change is only a "security feature" (huge quotes) when the caching backend also expires the old tokens after 60 seconds

If your caching layer leaks authentication credentials - it's not a problem of rememberme 🤷

What sounds like a really exotic edge-case causes about 3000 CookieTheftExceptions per day in our application. As the root cause can be any network interruption that causes the response of a RememberMe authentication to not reach the client it might actually trigger an immediate reload of the resource so that the second login actually happens rather often within the 60 second timeframe.

This looks suspicious that you have 3000 requests with remember me that fail. How much is it in the total value of remember calls? 0.5%? Or more?

@Seldaek

So all in all I am unsure what the security benefits are

My PR didn't fix a security issue, it fixed a logic issue originally explained in #42343 (security improvement is just a byproduct)

and it definitely causes problems in our real world usage.

My PR fixed the happy path: when all requests are successful. And you get it "broken" when some requests fail. I guess, the main priority should be to work "as designed" first, then handle exceptions: to me 2 concurrent successful requests is a primary case, and "a request that failed due to network connectivity issues" is secondary.

Okay, I have the third suggestion then:

Symfony stop throwing the CookieTheftExceptions exception: it's useless, and cannot really be handled. Instead - it emits an event and if you care about it - do whatever you want in a listener. And then after an event is emmitted rememberme handler clears rememberme cookie (as it's proven to be "wrong") and lets symfony firewall do the rest of its job.

So the unlucky users that lost their opportunity to exchange the rememberme with a new valid one, will next time just have to authenticate normally, not a big deal. And everybody happy, right?

I like the idea of triggering an event!

But that is not the problem here.

Currently we have a situation where multiple requests to a resource result in different responses based on the amount of time between the first and the second request.

So currently when someone sends a request with a RememberMe-Token and no session-ID they get assigned a new token. Which is fine.
And when - after some time they send a request again with that same Rememberme-Token and no sessison-ID they get

• either the previous token assigned (request happens more than 60 seconds after the previous one)
• or no token assigned at all (request happens less than 60 seconds after the previous one)

This is ... odd.

I would expect the same behaviour in both cases, especially as the 60 seconds is just any arbitrary hardcoded number.

I see two possible options that will not interfere with the happy path:

• Always return the new token, not only after 60 seconds or
• Make the time-interval in which no cookie is set configurable. (whether via an event or via a config value is perhaps a separate question

If we allow cached RememberMe tokens I would expect them to always work the same way when they are valid. I see this similar to 2FA where one can also configure to accept a certain number of "old" OTPs. As long as the old OTP is cached, I can use that as well as the current one to verify myself.

So the change is only a "security feature" (huge quotes) when the caching backend also expires the old tokens after 60 seconds
If your caching layer leaks authentication credentials - it's not a problem of rememberme 🤷

The caching layer is not leaking credentials. If at all it's the arbitrary number of 60 seconds that is leaking the credentials.

This looks suspicious that you have 3000 requests with remember me that fail. How much is it in the total value of remember calls? 0.5%? Or more?

Good point actually. Had to check that. It was more than 0.5%! Before modifying the cache-TTL we were at about 3%. Now we are down to 1%

So the unlucky users that lost their opportunity to exchange the rememberme with a new valid one, will next time just have to authenticate normally, not a big deal. And everybody happy, right?

Apart from the user on a flaky internet connection that constantly drops and that constantly - for no to the user apparent reason - has to enter username and password despite having activated the RememberMe feasture.

Okay, I have refreshed my memories about the original problems with the code, and current state. And also gave it a weekend of thinking. So, more weighted responses:

Currently we have a situation where multiple requests to a resource result in different responses based on the amount of time between the first and the second request.

It's fine, the rememberme protocol is stateful and is not idempotent.

either the previous token assigned (request happens more than 60 seconds after the previous one)

I cannot see that from the code: if it's more than 60 seconds, and the request contains the valid rememberme value: then the new token will be issued. In current code the token is never to be exposed after the first request. If it is exposed - it's a bug (but I don't see where it could come from).

If at all it's the arbitrary number of 60 seconds that is leaking the credentials.

https://github.com/symfony/symfony/blob/7.1/src/Symfony/Component/Security/Http/RememberMe/PersistentRememberMeHandler.php#L92 - nothing leaks anything here 🤷

Good point actually. Had to check that. It was more than 0.5%! Before modifying the cache-TTL we were at about 3%. Now we are down to 1%

I still am not sure what "cache" is being referred here, in rememberme implementation there is no any cache anywhere (or I possibly am looking at the wrong place). But 3% is a huge number for the discussed problem. To me it should happen extremely rarely.

Btw, for your particular use case (when you have incredibly high number of users with unsuccessful first request) - you possibly could tag your custom handler with security.remember_me_handler and override the response to always return the current rememberme token to every request, which makes the whole idea of series useless, but still - at least you wouldn't be having thousands of sentry reports a day?

This breaks the security of the idea of series-based rememberme. This solution should come with removal of series-based logic, as it would become just unnecessary complexity and then detailed explanation of the new protocol. Also, it will reintroduce #42343 which hits happy path users.

We can for sure argue about the series-based rememberme. Yes it is slightly less secure as it allows two persons to get the new value in the series, if both request with the same old rememberme token, AND both do it within 60 seconds.

Regarding the problem in #42343, I see the issue and I think it might be mitigated by re-fetching the token from DB before returning it again in the cookie. I'd need to look at the code again more closely to make sure.

Anyway I think we need to evaluate a bit more the problem on our end. I am not yet fully convinced this will resolve it, nor about the cause of the failed requests which seem to be way too high. So I'd say let's put this on hold, we'll first try to patch symfony on our end and see if it helps or not.

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

Just a quick reminder to make a comment on this. If I don't hear anything I'll close this.

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!