christianriesen/otp

One Time Passwords

otp's Issues

use hash_equals instead of custom safe string compare

https://secure.php.net/hash_equals

This is available from symfony/polyfill-php56 in PHP versions < 5.6.

OTP code lasts for 60s even if I set period to 30

Please help!

My 'verify' function has like this:

    public function verify($secret, $code, $period = 30, $digest = "sha1", $digits = 6)
    {
        $otp = new Otp();
        $otp->setPeriod($period);
        $otp->setDigits($digits);
        $otp->setAlgorithm($digest);
        return $otp->checkTotp(Encoding::base32DecodeUpper($secret), $code);
    }

The $secret is derived from your generateSecret() function, but with length of 64 (but I get the same with 16 so I know it's not that)

        $keys = array_merge(range('A','Z'), range(2,7)); // No padding char
        $secret = '';
        for ($i = 0; $i < 64; $i++) {
            $secret .= $keys[random_int(0, 31)];
        }
        return $secret;

When I get the OTP code in my authenticator, I run a script that calls the verify function with the $secret and the $code passed in (not overriding any other values), and it lasts for 60s:

$ while true; do date; ~/otp.sh 542566; echo ""; sleep 1; done
Thu Sep 21 06:35:33 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:35:35 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:35:37 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:35:40 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:35:42 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:35:44 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:35:46 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:35:49 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:35:51 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:35:53 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:35:56 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:35:58 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:00 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:03 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:05 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:07 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:10 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:12 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:14 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:16 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:19 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:21 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:23 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:26 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:28 UTC 2023
 
{"success":"1"}
Thu Sep 21 06:36:30 UTC 2023
 
{"error":{"message":"The code was invalid"}}
Thu Sep 21 06:36:33 UTC 2023

Why is this the case? What have I misunderstood?

Running Ubuntu 20.04 LTS. The server timezone is in UTC, and I was running the script on the same machine as the PHP app.

PHP 7.4.33 (cli) (built: Sep  2 2023 08:03:15) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.33, Copyright (c), by Zend Technologies
    with Xdebug v3.1.6, Copyright (c) 2002-2022, by Derick Rethans

Use random_int() instead of openssl with failover to mt_rand

People who know their stuff much better than I do are counselling against the use of openssl for random number generation, and mt_rand is woefully inadequate. PHP 7 introduces two new functions for best-practice random number generation: random_bytes() and random_int(), and (just like when password_hash() was introduced) someone's kindly created a polyfill to provide these functions on existing versions of PHP: paragonie/random_compat

What about adding a composer requirement (or maybe a suggest) for paragonie/random_compat, then replacing the getRand() private method with a call to random_int(0,31) in https://github.com/ChristianRiesen/otp/blob/master/src/Otp/GoogleAuthenticator.php#L160?

Recovery Codes

Any chance we could get a function to generate recovery codes?
Recovery codes are provided by Google also when you setup Two Factor Authentication for their apps.

README example is confusing

In the README:

// Just to create a key for display (testing)
$key = $otp->totp($secret);

This should be:

$key = $otp->totp(Base32::decode($secret));

Took me a while to figure that one out :-)

Composer now requires random_compat v99.99.99

Hey @ChristianRiesen

I haven't yet looked into this for long enough to understand why, but the change in #32 appears to end up requiring password_compat v99.99.99.

I guess it's something to do with this?

https://github.com/paragonie/random_compat#version-99999

With that in mind, is this behaviour intended?

Chris

OTP implicitly requires PHP 7

The composer.json requirement is PHP 5.4 or greater, but goes on to include either version 1 or 2 of https://github.com/paragonie/constant_time_encoding

Version 2 of constant_time_encoding requires PHP 7. This means composer can grab version 2 but the codebase can run on a lower PHP version.

I solved this for my project by explicitly requiring the 1.0 version

composer require paragonie/constant_time_encoding:^1.0

Add docs about why not to use sessions for storing the secret

The docs say to generate a secret and save it to the user's account, and then show the QR code to the user. The example does this by saving the secret to the session, but also says to not do so in production. I'm just wondering why not?

I guess the recommended flow is to

1. generate the secret
2. save it to the user's account (e.g. in a database)
3. show the QR code and request a verification key
4. check that the verification key matches, and then mark the secret as verified

Is that about right? I guess I'm confused about the data storage requirements, because it seems like it'd be neater to save the secret to the session, and then only after getting the verification key save it to the database, because at that point we know the user has set up the TOTP app with the new secret.

How old can a opt code be?

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)

Resynchronization of the HOTP Counter

The HOTP standard defines the resynchronization algorithm which seems to be missing from this library.
This is a critical functionality which should be present as the end user might generate multiple otps and hence we lose track of the counter.

4 digit OTP code

Is there any option to generate a 4 digit code only?

dependency christian-riesen/base32?

Is the "christian-riesen/base32": "^1.0" dependency still used anywhere? Maybe it can be removed from composer.json.

constant-time remainder

I don't think the remainder (%) operator is constant-time, so it leaks a bit of information about the OTP:

otp/src/Otp.php

Line 334 in f29c9eb

) % pow(10, $this->digits);

Don't use rand()

Bug is here:

otp/src/Otp/GoogleAuthenticator.php

Line 154 in 7b1c92e

$string .= $keys[rand(0,31)];

rand() isn't a CSPRNG - even the PHP team caution against treating it as such: http://php.net/rand#refsect1-function.rand-notes

use paragonie/random_compat for random_int (not optional)

https://paragonie.com/blog/2016/05/how-generate-secure-random-numbers-in-various-programming-languages#php-csprng

Missing vendors in .gitignore

Hello,

Can you add /vendor/ and composer.lock paths in .gitignore file ?

Thanks

Move to php7

With 5.6 (and even 7.0) going EOL it seems right to upgrade this lib.
It would then automatically be a lot more type safe. Which is always nice.

Deprecation of Google Chart Infographics API

The Google Chart Infographics API is deprecated since 2012 although it is still working there is no guarantee this will continue to work. Maybe you could include an alternative.

Info: https://developers.google.com/chart/infographics/docs/qr_codes

Move to GitHub Action

As far as I know, the Travis CI dot org will be deprecated soon.

And the Travis CI dot com is not friendly for open source projects on GitHub.

I think it's time to move to GitHub Action.

Once this issue is accepted by @ChristianRiesen, I will be happy to work on that.