Details
Description
Looks like regression from EZP-32217 which was apparently incomplete. Reported as a security vulnerability by security company https://it-sec.de/
They report being able to see time differences for valid/invalid users with the /login_check route. I believe the fix we made last time with a random sleep only protects hash comparison. It doesn't encompass the whole login process. A valid user / wrong password attempt will lead to more data being loaded than if the username is invalid. We need to do a constant-time or random sleep fix at a higher level. (That's the theory. Still not clear why we didn't catch the issue last time, likely wrong test method.)
As an example, the loadByLogin() call here will likely return in different time if the user exists or not. So loadUserByCredentials() should return in constant time, if execution reaches this call. All other entrances must be protected in the same way. (Not saying this is the right place to apply constant-time logic, it could be elsewhere.)
https://github.com/ezsystems/ezpublish-kernel/blob/7.5/eZ/Publish/Core/Repository/UserService.php#L611
Steps to reproduce
The attached timing-attack-test.php reproduces the problem. A web login attempt with wrong password takes about the same time as a successful login, 270 msec in my test. But an attempt with an invalid username takes only 220 msec, a significant and detectable difference. This matches the report. I get similar results with API login.
API - Valid login, average time: 532.66710137 msec (100 repeats) API - Invalid username, average time: 492.87669317 msec (100 repeats) <- significantly lower API - Invalid password, average time: 558.04756152 msec (100 repeats) Web - Valid login, average time: 279.42206281 msec (100 repeats) Web - Invalid username, average time: 228.34236277 msec (100 repeats) <- significantly lower Web - Invalid password, average time: 285.82204882 msec (100 repeats)
Solution
- Configure a given constant time that authentication should take.
- Use usleep to fill the gap when actual execution time is shorter. If actual execution time is longer, log a warning that the configured constant time should be increased.
- If constant time is configured to zero, do not sleep and do not log a warning (dev mode).
- Remove existing random usleep, as it is inadequate and no longer needed.
Designs
Attachments
Issue Links
- relates to
-
IBX-2973 Remove obsolete random sleep on REST login
- Closed