GoogleAuthenticator
GoogleAuthenticator copied to clipboard
verifyCode() method could be slightly improved
I've noticed that you use a timing safe equals comparison method in verifyCode()
, yet this method itself is not time safe. That seems to be a bit of a contradiction. It has to do with the return
inside the for
loop:
for ($i = -$discrepancy; $i <= $discrepancy; ++$i) {
$calculatedCode = $this->getCode($secret, $currentTimeSlice + $i);
if ($this->timingSafeEquals($calculatedCode, $code)) {
return true;
}
}
Depending on the time slice in which a match is found the verifyCode()
method will take longer or shorter to execute. To be honest, I don't see how this can be exploited in a timing attack, but still, why not simply get rid of this problem? Something like:
$success = false;
for ($i = -$discrepancy; $i <= $discrepancy; ++$i) {
$calculatedCode = $this->getCode($secret, $currentTimeSlice + $i);
if ($this->timingSafeEquals($calculatedCode, $code)) {
$success = true;
}
}
return $success;
Now the time this piece of code takes to execute does no longer depend on when a match is found.
Is there a reason you didn't open a pull request for this? Just wondering.
I have no idea how that works. I tried but can't get very far with that. All Github allows me is to read the documentation, nothing more.
Here's a link to how to propose/request a change to a github repository, such as what you are proposing:
https://www.earthdatascience.org/courses/intro-to-earth-data-science/git-github/github-collaboration/how-to-submit-pull-requests-on-github/
Hope that helps, and looking forward to see your code implemented into the project.