search

PHPGangsta / GoogleAuthenticator

PHP class to generate and verify Google Authenticator 2-factor authentication
http://phpgangsta.de/4376
BSD 2-Clause "Simplified" License
2.26k stars 698 forks source link

Avoid TOTP code replay attacks #9

Closed greggles closed 9 years ago

greggles commented 10 years ago

As far as I can tell, GoogleAuthenticator library doesn't prevent a replay attack.

For this to be relevant we have to assume that either the victim is using an insecure network or has malware on their computer - both of which are not ideal, but happen more often than we'd like.

Would it be possible for GoogleAuthenticator library itself to prevent a replay? My guess is that the library doesn't have any way to persist data so it can't really prevent this kind of attack, but maybe you have ideas on that.

If it's not possible to solve in the library, then I suggest adding a note to the README to encourage people who use the library to add a feature to their code to prevent replay.

greggles commented 10 years ago

I just saw "For a secret installation you have to make sure that used codes cannot be reused (replay-attack)." So I guess the second part of the request is done.

PHPGangsta commented 10 years ago

Hi Greg,

you are right, replay-attacks are a problem that have to be taken care of.

I'm not sure if I should add that to the library, as you said we have to persist the used codes somehow. Maybe a small sqlite database would be sufficient? But that would add a dependency. What should we do if sqlite-support is not available in PHP? What should we do if we don't have write access to the file system (like in many modern PaaS-environments)? Should we add PDO-support, so the user can configure the database he wants?

What would you need, and what do you think other people need?

greggles commented 10 years ago

There's a part of me that thinks doing it in sqlite would be great, and making it possible to skip that if the code calling the library takes care of it directly or if the php-sqlite module is not available. That way code that relies on the library would be "secure by default."

That said, there is documentation and this seems like something best handled outside of the library so I'd be fine if this issue is just closed as "works as designed."

lva commented 10 years ago

Here is a possible solution for your problem. You don't have to store the used codes; you only have to store the timeslice of the last successful authentication.

Instead of always just passing the secret between the application and your library, an array should be exchanged BY REFERENCE. This array then contains the secret and the timeslice of the last successful authentication. You can then also use this array to track the number of successful and failed authentications (which you can then ultimately use to lock a token after X failed authentications).

In the for-loop in verifyCode you should check that ($currentTimeSlice + $i) is bigger than the stored timeslice in the array. If the verification fails, update the failed counter in the array. If the verification is successful, update the timeslice moment in the array and possibly update the nr of successful authentications (if you want to track this).

I mentioned that you should accept the array by reference since the application will have to store the array again in its database (possibly serialized).

PHPGangsta commented 9 years ago

As greggles said, it should be done outside of this library. The application should use its favorite storage for remembering used codes, and reject them if they are tried again. So it "works as designed".