Move Timebox for Authentication and add to password resets

[valorin]

This PR fixes two related issues, and should be back-ported to all versions where security fixes are applied.

1. Authentication flow Timebox in wrong spot

The Timebox class was introduced in #44069 inside the SessionGuard::hasValidCredentials() method as a way to mask timing differences within the authentication flow that can lead to user enumeration attacks. However it was implemented in the wrong spot - it is applied after the database is queried for a user record. As such, the response time for a request where the user record doesn't exist will be slightly shorter than that where the user does exist - making it measurable and vulnerable to an enumeration attack.

To fix the issue, and maintain backwards compatibility, I've moved the Timebox from SessionGuard::hasValidCredentials() to the three methods which call hasValidCredentials: validate, attempt, and attemptWhen. This allows the Timebox to encapsulate the database queries, masking user existence and making user enumeration significantly more difficult.

I also added the auth.timebox_duration configuration option, as in my tests the default 200,000 microsecond wait time wasn't sufficient for a slow server to mask the timing differences. By making this configurable, devs can easily tweak it to optimise for their infrastructure.

I tried getting meaningful but simple benchmarks but the timing differences were too subtle. The differences are easy to observe in isolation - such as the difference in query time, but tying them together to benchmark the whole request was difficult. I can provide more details as needed.

2. Adding Timebox to the Password Reset

The Timebox was also missing from password reset requests, which would allow an attacker to easily enumerate user accounts based on that response time too. I added the Timebox to PasswordBroker::sendResetLink(), and PasswordBroker::reset(), both of which could be used to enumerate user accounts.

Notes

• Preventing timing attacks is virtually impossible, as there will always been some variations between different requests, however detecting these variations can be extremely difficult, so if enumeration is a concern then adding the Timebox can make a difference and slow down an attacker.
• The default Starter Kits leak user existence in other ways, regardless of these changes. This fix is for the devs who have implemented their own authentication system on top of the framework and expect the Timebox to prevent enumeration.

[JurianArie]

I tried getting meaningful but simple benchmarks but the timing differences were too subtle.

James Kettle has a great write-up on this topic that you might find interesting: https://portswigger.net/research/listen-to-the-whispers-web-timing-attacks-that-actually-work

[valorin]

I tried getting meaningful but simple benchmarks but the timing differences were too subtle.

James Kettle has a great write-up on this topic that you might find interesting: https://portswigger.net/research/listen-to-the-whispers-web-timing-attacks-that-actually-work

Oh nice, I'll take a look, thanks! 👍