New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preventing timing attacks on authentication #9456
Comments
Having thought about this issue a bit more, here is an extension/update to the original post: IssueHere are additional points. (Points 1 and 2 of the original post are still valid.)
Possible Solutions
Additional NotesThe test in the original post was not correct, in that the comparison time difference of only one character should be tested:
Which outputs following with PHP 5.0.5:
I.e., a difference of ca. 5ms per 1000000 comparisons, or ca. 5ns per comparison. And about half of that with PHP 5.6.2. Which makes this even more theoretical in the realm of web applications (especially over the Internet). |
The above solution 2.i. (time jitter "padding" of the comparison time) would not work. It would make an attack slower, but not prevent it. Because of a possibility of a statistical analysis attack. Paraphrasing https://www.reddit.com/r/netsec/comments/3zc5qu/https_bicycle_attack/cylek82: If you were to observe enough samples, you'd eventually identify a case where the random time padding length reached its minimum, which would give you a close estimate of the actual comparison time. For example, if you say "padding time is randomly selected between 5 and 50 ns, with a resolution of 5ns", after 10 observations you can assume that the actual comparison time is probably 5ns shorter than the smallest comparison time you observed. Your confidence factor only goes up with the number of observations. |
From another discussion, from 2010, Nanosecond Scale Remote Timing Attacks On PHP Applications: Time To Take Them Seriously?:
It even advocates for timing attack prevention on user name ("login") (which is, similarly to the issue point 2 above, also vulnerable on its DB look-up in https://github.com/piwik/piwik/blob/2.16.0-b1/plugins/Login/Auth.php#L63):
But, in my opinion (given its implementation complexity), that would not be needed with a timing attack-secure password handling. |
A method was added in #16696 and documented it in the checklist and security guide plus it's a public API. we'll only need to use it in the codebase eventually |
Background
See http://codahale.com/a-lesson-in-timing-attacks/. Also, Github advises using constant-time string comparison with its authentication token.
Issue
The
authenticateWithToken()
andauthenticateWithTokenOrHashToken()
functions of the Login plugin (see https://github.com/piwik/piwik/blob/2.16.0-b1/plugins/Login/Auth.php#L72-L96) currently don't use a constant-time string comparison when checking validity oftoken_auth
.Specifically:
authenticateWithTokenOrHashToken()
: Following comparisons are not constant-time:SessionInitializer::getHashTokenAuth($login, $user['token_auth']) === $token
$user['token_auth'] === $token
authenticateWithToken()
: I'm not familiar with DB access, but it seems possible that$user = $this->userModel->getUserByTokenAuth($token)
is not constant-time, referring to the comparison by MySQL of$token
to table entries on value search (requested in https://github.com/piwik/piwik/blob/2.16.0-b1/plugins/UsersManager/Model.php#L175).Note:(See next comment.)authenticateWithPassword()
does not seem to be affected, because it compares hashes of the password. Which should make such character-by-character recovery impossible, given that a hash changes entirely with a slightest change of the password.Possible Solutions:
authenticateWithTokenOrHashToken()
: See, e.g., https://github.com/phpseclib/phpseclib/blob/2.0.0/phpseclib/Crypt/RSA.php#L2201-L2213 for an example on constant-time string comparison. (Its length check should not be an issue withtoken_auth
in Piwik, since it's a (MD5) hash, whose length is constant and thus does not contain any information useful to an attacker.)Alternatively, see next point.Another secure string comparison example can be found here: https://github.com/defuse/php-encryption/blob/399fc4c4798b301681888c2608b0ddd060d477da/src/Crypto.php#L409.
2.(See next comment.)authenticateWithToken()
: A solution could be storing thetoken_auth
in hashed form, similarly to the password. And similarly, hashing the received value before searching for it in the DB.Additional Notes
It seems to be very hard to exploit this with a web application (especially over the internet). So that this is rather an "enhancement". E.g., following test:
on http://sandbox.onlinephpfunctions.com/ outputs, e.g., following with PHP 5.0.5:
I.e., a difference of ca. 20ms per 1000000 comparisons, or ca. 20ns per comparison with(See next comment.)===
.With PHP 5.6.2 it's faster, but the difference is similar, e.g.:
P.S.: I'm not an expert with any of the above, in any way shape or form :)
The text was updated successfully, but these errors were encountered: