ChristianRiesen/otp

How old can a opt code be?

Closed this issue · 10 comments

I've tried setting the time limit to 5 minutes (300 sec) as the timecounter argument in totp(), but it doesn't seem to work.
The time window to enter in the code seems to be pretty short.

Is there a way to adjust this?

Code I use to create the key:
$key = $otp->totp(Encoding::base32DecodeUpper($secret, 300));

and then checking validity:
$otp->checkTotp(Encoding::base32DecodeUpper($secret), $key)

There are a couple errors in there, I hope I can clear that up :)

The second parameter in totp is null by default as you should not need it. It's there to manually input the counter so the function and code can be tested or if you want to provide that (for some reason) by yourself, and just want to use the algorythm.

What you are looking for is the following function:

otp/src/Otp.php

Line 205 in aab865a

public function setPeriod($period)

That will set the time how long a OTP can be valid, but then of course your app, should you use one, also needs to use that same time. By default it's 30 seconds.
However, the checkTotp function has a third parameter for time drift, allowing you to enter a code if it is valid within that window of time drift, given in numbers of codes. So if you set the period to 300, then the currently valid code, and the code in the 300 seconds before that is valid by default (trimedrift is 1 per default). So if you want to check strictly within that 300 second window, then you wnat to also set the timedrift to 0, however that is a bit of a bad experience for users if they call up their OTP app or whatever they use for that, and it shows them they have 10 seconds to enter it. Do they try and maybe fail, or just wait? With the 30 seconds and 1 timedrift window, the key they see first displayed will be valid no matter what, for the next 30 seconds, due to the default timedrift.

And you have an error in the usage of the first code line, as you add 300 to the base32DecodeUpper function, not the totp one.

What you probably want, from what I can tell, is this:

$otp->setPeriod(300);
$key = $otp->totp(Encoding::base32DecodeUpper($secret));
$otp->checkTotp(Encoding::base32DecodeUpper($secret), $key);

Let me know if that works for you or if you run into any more questions :)

And as always with OTPs: Once it's used it should be not useable again for at least the period time multiplied by the allowed timedrift, but that's up to you to make sure it's not happening (some memory store or something like that offers itself nicely for that).

Thanks much; not clear on the trimedrift concept; but something I could lookup if needed.

Timedrift is used on checking only. So say you have code 1 valid for 30 seconds, and then it switches to the next one wich is code 2 now. With timedrift 1, you can now use both code 1 and 2 during the 30 second window of code 2. Once that windows is up and code 3 is generated, now you can use code 2 and code 3, etc. If you want to be strict, you set timedrift to 0.
And that of course is if your period is 30 seconds (the default).
Did this clear your issue?

Seems my implementation has a problem still.
I added
$otp->setPeriod(300);
over the
$key = $otp->totp(Encoding::base32DecodeUpper($secret));

and made $otp a global variable with new $otp; at the top of the file with global $otp; in my functions.
I have 2 functions that use $otp
generateAndEmailOtp($userId, $conn, $encryption_key), where the $key is created, secret is encrypted and stored in the user database, and the key is emailed to the user.
and then verifyOtpAndHandle($key, $mysqli, $encryption_key, $rememberMe) where otp checking takes place, with secret retrieved and decrypted from the database before use with the key.

Sending the key via email and then input by user is a requirement.

Still the $otp->setPeriod(300); doesn't seem to extend the time out for me.
How does it work?
is the period encoded in the $secret so the otp function knows how long to honor the key?
What times it out?

I take it you mean you have something like $otp = new Otp(); and not new $otp; correct? That would be an issue right there otherwise :)

Since I can't know what your code does, I can only talk about what this library does. I'm trying to understand how you are trying to use it, so please correct me if I'm wrong here with my assumptions.

All of this assumes that $otp is an instance of the Otp class of course.
You set the period to 300 with $otp->setPeriod(300);
You call $otp->totp($secret); and I think thats what your $encryption_key is named outside the library, correct?
Assuming that the $encryption_key remains the same, that first call gives you a one time password back into $key
That key (one time password) is 6 digits and you take that key and email it to the user. The request ends.

Now the user gets the email and has to enter it. This is a new request to the web server.
You check the given key with $otp->checkTotp($secret, $key); and it doesn't work.

There is an important bit: Did you call $otp->setPeriod(300); on both requests or only one? Think of this as part of the setup of the class, like if it was a db you talk to you have to give it the user name and password each time. If it's not done each time and each time the same, the algorythm can't create the same key and so the check will fail.

Also important to note: The time, those 300 seconds, they don't start ticking on creation of the key, they constantly tick down. So if you create one now, it could be that we are already at 299 seconds of the 300 seconds window. What happens is that it takes the time in seconds since 1970 and then divides it by the period (and rounds down). So these "windows" keep on moving, no matter what you do. So if you check with a timedrift of 0 that may lead often to a key not working.

Otp is meant for repeated use, so they have to enter a password say every time they log in or so and where they have an app that they can generate the one time password (2 factor authentication). If you only need a singluar one time verification key, I would suggest using a different solution, like writing something into a memory store (like redis, memcache etc) or even into a database field, depending on your use.

Yes;
$otp = new Otp();

and I went back and added
$otp->setPeriod(300);
before the other use as well.

Goal:
Each time a user logs in the key gets generated and emailed to them to complete the login.
(unless a remember cookie is set after a successful validation the first time for as long as the cookie is valid)

For the key generation function:

global $otp;
    $otp->setPeriod(300);
    $key = $otp->totp(Encoding::base32DecodeUpper($secret));

From the function that validates the user submitted key:

  global $otp;
    $otp->setPeriod(300);
    if ($otp->checkTotp(Encoding::base32DecodeUpper($secret), $key)) {

Still having access issues; I may need to revisit how I do this.
Alternatively, I'm thinking I could just generate the key, encrypt it to store, then just compare with the one submitted after decrypting, not worrying about the timing or otp, as the key is one time use, because if the key is wrong a new one is generated and sent.
And there is a table for tracking the previous key within a certain time period.

Thank you for your assistance; I'll just have to more deeply explore or think through things, unless I find a flaw that is causing the failure in my implementation.

I thought your use is something along those lines, but you can make your life easier if you want.

Have part of your sign up, or if you enable this function at will then in your options, run trhough a process where they get the secret as a QR code. They can then use an app that is free like freeotp, aegis, etc to scan that code once and their phone will generate the current code for them, even show how long a code is valid for still. In your login you have a field for this code and you are done. When they login they enter their stuff, look at the app and from there they see their code and can enter it immediately.

Or is there another reason why you do it this way that I'm unaware of?

Was trying to do things securely and not using a phone but email is a hard requirement from client.

Thank you for your assistance; I learned something :)
Thanks much.

Was trying to do things securely and not using a phone but email is a hard requirement from client.

The phone is part of the 2FA way as in "something you have", versus a password that is "something you know". For example, if someone has your users password, they might also gain access to their email since many people use the same password for both (they shouldn't but not everyone follows good practices). So your security system is circumvented in such a case.

Use this otp class when you want to use otp methods with the phone apps, as there it takes all the guess work out of that portion for you, and since the codes only live for 30 seconds and you should avoid accepting the same valid code twice in a rwo from the same user (mitigating replay attacks) then you have one of the most secure ways that you can make with very little effort and still a high user acceptance. You can use it for other things, including your use case here, but it wont work as easy as you might want.

If you want to do that thing with the email, I would advise you to add two fields to your user in the database and create a random number and record in the second field the time it was generated, so you can then check on reading if it expired (I wouldn't set the expired date in the db, because then you can alter the time on the fly in the checking step).

And feel free to just grab pieces of the code as well, that's what it's here for :)