From 3d40c249a46cd81621d7c60edc2d37418028fb4a Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Tue, 3 Dec 2024 11:29:27 -0800 Subject: [PATCH 1/3] Full RFC6238 Compatibility The full TOTP specification supports not only SHA1, but also SHA256 and SHA512. Up til now, the TOTP provider in this plugin (and most other PHP implementations) claims to support a specified hash type, but doesn't actually work with anything other than SHA1. This is due to key lengths and hash lengths being _different_ for the three hash variants. This change introcudes support for both SHA256 and SHA512, porting the implementation directly from https://github.com/ericmann/totp. See https://tools.ietf.org/html/rfc6238#section-1.2 for more information on the `MAY USE` notation for SHA256 and SHA512. See https://tools.ietf.org/html/rfc6238#appendix-A for a fully compliant reference implementation in Java. See https://tools.ietf.org/html/rfc6238#appendix-B for test vectors showing TOTPs generated for specific time values and the three hash variants. See https://github.com/ericmann/totp/blob/master/test/phpunit/ReferenceTest.php for example unit tests verifying this particular implementation before it was ported to the plugin. --- providers/class-two-factor-totp.php | 56 ++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 5 deletions(-) diff --git a/providers/class-two-factor-totp.php b/providers/class-two-factor-totp.php index 003a9774..c96d096a 100644 --- a/providers/class-two-factor-totp.php +++ b/providers/class-two-factor-totp.php @@ -529,11 +529,13 @@ public function validate_code_for_user( $user, $code ) { * * @param string $key The share secret key to use. * @param string $authcode The code to test. + * @param string $hash The hash used to calculate the code. + * @param int $time_step The size of the time step. * * @return bool Whether the code is valid within the time frame. */ - public static function is_valid_authcode( $key, $authcode ) { - return (bool) self::get_authcode_valid_ticktime( $key, $authcode ); + public static function is_valid_authcode( $key, $authcode, $hash = self::DEFAULT_CRYPTO, $time_step = self::DEFAULT_TIME_STEP_SEC ) { + return (bool) self::get_authcode_valid_ticktime( $key, $authcode, $hash, $time_step ); } /** @@ -541,10 +543,12 @@ public static function is_valid_authcode( $key, $authcode ) { * * @param string $key The share secret key to use. * @param string $authcode The code to test. + * @param string $hash The hash used to calculate the code. + * @param int $time_step The size of the time step. * * @return false|int Returns the timestamp of the authcode on success, False otherwise. */ - public static function get_authcode_valid_ticktime( $key, $authcode ) { + public static function get_authcode_valid_ticktime( $key, $authcode, $hash = self::DEFAULT_CRYPTO, $time_step = self::DEFAULT_TIME_STEP_SEC ) { /** * Filter the maximum ticks to allow when checking valid codes. * @@ -564,9 +568,11 @@ public static function get_authcode_valid_ticktime( $key, $authcode ) { $time = floor( time() / self::DEFAULT_TIME_STEP_SEC ); + $digits = strlen( $authcode ); + foreach ( $ticks as $offset ) { $log_time = $time + $offset; - if ( hash_equals( self::calc_totp( $key, $log_time ), $authcode ) ) { + if ( hash_equals( self::calc_totp( $key, $log_time, $digits, $hash, $time_step ), $authcode ) ) { // Return the tick timestamp. return $log_time * self::DEFAULT_TIME_STEP_SEC; } @@ -619,6 +625,30 @@ public static function pack64( $value ) { return pack( 'NN', $higher, $lower ); } + /** + * Pad a short secret with bytes from the same until it's the correct length + * for hashing. + * + * @param string $secret Secret key to pad. + * @param int $length Byte length of the desired padded secret. + * + * @throws InvalidArgumentException If the secret or length are invalid. + * + * @return string + */ + protected static function pad_secret( $secret, $length ) { + if ( empty( $secret ) ) { + throw new InvalidArgumentException( 'Secret must be non-empty!' ); + } + + $length = intval( $length ); + if ( $length <= 0 ) { + throw new InvalidArgumentException( 'Padding length must be non-zero' ); + } + + return str_pad( $secret, $length, $secret, STR_PAD_RIGHT ); + } + /** * Calculate a valid code given the shared secret key * @@ -627,12 +657,28 @@ public static function pack64( $value ) { * @param int $digits The number of digits in the returned code. * @param string $hash The hash used to calculate the code. * @param int $time_step The size of the time step. + * + * @throws InvalidArgumentException If the hash type is invalid. * * @return string The totp code */ public static function calc_totp( $key, $step_count = false, $digits = self::DEFAULT_DIGIT_COUNT, $hash = self::DEFAULT_CRYPTO, $time_step = self::DEFAULT_TIME_STEP_SEC ) { $secret = self::base32_decode( $key ); + switch ( $hash ) { + case 'sha1': + $secret = self::pad_secret( $secret, 20 ); + break; + case 'sha256': + $secret = self::pad_secret( $secret, 32 ); + break; + case 'sha512': + $secret = self::pad_secret( $secret, 64 ); + break; + default: + throw new InvalidArgumentException( 'Invalid hash type specified!' ); + } + if ( false === $step_count ) { $step_count = floor( time() / $time_step ); } @@ -641,7 +687,7 @@ public static function calc_totp( $key, $step_count = false, $digits = self::DEF $hash = hash_hmac( $hash, $timestamp, $secret, true ); - $offset = ord( $hash[19] ) & 0xf; + $offset = ord( $hash[ strlen( $hash ) - 1 ] ) & 0xf; $code = ( ( ( ord( $hash[ $offset + 0 ] ) & 0x7f ) << 24 ) | From 93438bec0b682d80dfba72dd4752584ff3490ba0 Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Tue, 3 Dec 2024 11:32:50 -0800 Subject: [PATCH 2/3] Port over the RFC reference tests --- providers/class-two-factor-totp.php | 29 ++++- tests/providers/class-two-factor-totp.php | 130 ++++++++++++++++++++++ 2 files changed, 157 insertions(+), 2 deletions(-) diff --git a/providers/class-two-factor-totp.php b/providers/class-two-factor-totp.php index c96d096a..1a418121 100644 --- a/providers/class-two-factor-totp.php +++ b/providers/class-two-factor-totp.php @@ -51,6 +51,31 @@ protected function __construct() { parent::__construct(); } + /** + * Timestamp returned by time() + * + * @var int $now + */ + private static $now; + + /** + * Override time() in the current object for testing. + * + * @return int + */ + private static function time() { + return self::$now ?: time(); + } + + /** + * Set up the internal state of time() invocations for deterministic generation. + * + * @param int $now Timestamp to use when overriding time(). + */ + public static function __set_time( $now ) { + self::$now = $now; + } + /** * Register the rest-api endpoints required for this provider. * @@ -566,7 +591,7 @@ public static function get_authcode_valid_ticktime( $key, $authcode, $hash = sel $ticks = range( - $max_ticks, $max_ticks ); usort( $ticks, array( __CLASS__, 'abssort' ) ); - $time = floor( time() / self::DEFAULT_TIME_STEP_SEC ); + $time = floor( self::time() / self::DEFAULT_TIME_STEP_SEC ); $digits = strlen( $authcode ); @@ -680,7 +705,7 @@ public static function calc_totp( $key, $step_count = false, $digits = self::DEF } if ( false === $step_count ) { - $step_count = floor( time() / $time_step ); + $step_count = floor( self::time() / $time_step ); } $timestamp = self::pack64( $step_count ); diff --git a/tests/providers/class-two-factor-totp.php b/tests/providers/class-two-factor-totp.php index 52aa6090..202f5e35 100644 --- a/tests/providers/class-two-factor-totp.php +++ b/tests/providers/class-two-factor-totp.php @@ -14,6 +14,18 @@ */ class Tests_Two_Factor_Totp extends WP_UnitTestCase { + private static $token = '12345678901234567890'; + private static $step = 30; + + private static $vectors = [ + 59 => ['94287082', '46119246', '90693936'], + 1111111109 => ['07081804', '68084774', '25091201'], + 1111111111 => ['14050471', '67062674', '99943326'], + 1234567890 => ['89005924', '91819424', '93441116'], + 2000000000 => ['69279037', '90698825', '38618901'], + 20000000000 => ['65353130', '77737706', '47863826'] + ]; + /** * Instance of our provider class. * @@ -317,4 +329,122 @@ function test_get_authcode_valid_ticktime() { $this->assertFalse( Two_Factor_Totp::get_authcode_valid_ticktime( $key, '000000' ) ); } + + /** + * @covers Two_Factor_Totp::calc_totp + */ + public function test_sha1_generate() { + if ( PHP_INT_SIZE === 4 ) { + $this->markTestSkipped( 'calc_totp requires 64-bit PHP' ); + } + + $provider = $this->provider; + $hash = 'sha1'; + $token = $provider->base32_encode( self::$token ); + + foreach (self::$vectors as $time => $vector) { + $provider::__set_time( (int) $time ); + $this->assertEquals( $vector[0], $provider::calc_totp( $token, false, 8, $hash, self::$step ) ); + $this->assertEquals( substr( $vector[0], 2 ), $provider::calc_totp( $token, false, 6, $hash, self::$step ) ); + } + } + + /** + * @covers Two_Factor_Totp::is_valid_authcode + * @covers Two_Factor_Totp::calc_totp + */ + public function test_sha1_authenticate() { + if ( PHP_INT_SIZE === 4 ) { + $this->markTestSkipped( 'calc_totp requires 64-bit PHP' ); + } + + $provider = $this->provider; + $hash = 'sha1'; + $token = $provider->base32_encode( self::$token ); + + foreach ( self::$vectors as $time => $vector ) { + $provider::__set_time( (int) $time ); + $this->assertTrue( $provider::is_valid_authcode( $token, $vector[0], $hash ) ); + $this->assertTrue( $provider::is_valid_authcode( $token, substr( $vector[0], 2 ), $hash ) ); + } + } + + /** + * @covers Two_Factor_Totp::calc_totp + */ + public function test_sha256_generate() { + if (PHP_INT_SIZE === 4) { + $this->markTestSkipped( 'calc_totp requires 64-bit PHP' ); + } + + $provider = $this->provider; + $hash = 'sha256'; + $token = $provider->base32_encode( self::$token ); + + foreach ( self::$vectors as $time => $vector ) { + $provider::__set_time( (int) $time ); + $this->assertEquals( $vector[1], $provider::calc_totp( $token, false, 8, $hash, self::$step ) ); + $this->assertEquals( substr( $vector[1], 2 ), $provider::calc_totp( $token, false, 6, $hash, self::$step ) ); + } + } + + /** + * @covers Two_Factor_Totp::is_valid_authcode + * @covers Two_Factor_Totp::calc_totp + */ + public function test_sha256_authenticate() { + if ( PHP_INT_SIZE === 4 ) { + $this->markTestSkipped( 'calc_totp requires 64-bit PHP' ); + } + + $provider = $this->provider; + $hash = 'sha256'; + $token = $provider->base32_encode( self::$token ); + + foreach ( self::$vectors as $time => $vector ) { + $provider::__set_time( (int) $time ); + $this->assertTrue( $provider::is_valid_authcode( $token, $vector[1], $hash ) ); + $this->assertTrue( $provider::is_valid_authcode( $token, substr( $vector[1], 2 ), $hash ) ); + } + } + + /** + * @covers Two_Factor_Totp::calc_totp + */ + public function test_sha512_generate() { + if ( PHP_INT_SIZE === 4 ) { + $this->markTestSkipped('calc_totp requires 64-bit PHP'); + } + + $provider = $this->provider; + $hash = 'sha512'; + $token = $provider->base32_encode( self::$token ); + + foreach ( self::$vectors as $time => $vector ) { + $provider::__set_time( (int) $time ); + $this->assertEquals( $vector[2], $provider::calc_totp( $token, false, 8, $hash, self::$step ) ); + $this->assertEquals( substr($vector[2], 2 ), $provider::calc_totp( $token, false, 6, $hash, self::$step ) ); + } + } + + /** + * @covers Two_Factor_Totp::is_valid_authcode + * @covers Two_Factor_Totp::calc_totp + */ + public function test_sha512_authenticate() { + if ( PHP_INT_SIZE === 4 ) { + $this->markTestSkipped( 'calc_totp requires 64-bit PHP' ); + } + + $provider = $this->provider; + $hash = 'sha512'; + $token = $provider->base32_encode( self::$token ); + + foreach ( self::$vectors as $time => $vector ) { + $provider::__set_time( (int) $time ); + $this->assertTrue( $provider::is_valid_authcode( $token, $vector[2], $hash ) ); + $this->assertTrue( $provider::is_valid_authcode( $token, substr( $vector[2], 2 ), $hash ) ); + } + + } } From 80c681951c49b233d6ac7b697a2495852451e37f Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Tue, 3 Dec 2024 11:47:01 -0800 Subject: [PATCH 3/3] Fix Linting Error with ```set_time()` According to the linter: > Method name "Two_Factor_Totp::__set_time" is discouraged; PHP has reserved all method names with a double underscore prefix for future use. Rename the function to make the linter happy. --- providers/class-two-factor-totp.php | 2 +- tests/providers/class-two-factor-totp.php | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/providers/class-two-factor-totp.php b/providers/class-two-factor-totp.php index 1a418121..c40b8dc5 100644 --- a/providers/class-two-factor-totp.php +++ b/providers/class-two-factor-totp.php @@ -72,7 +72,7 @@ private static function time() { * * @param int $now Timestamp to use when overriding time(). */ - public static function __set_time( $now ) { + public static function set_time( $now ) { self::$now = $now; } diff --git a/tests/providers/class-two-factor-totp.php b/tests/providers/class-two-factor-totp.php index 202f5e35..909e52c7 100644 --- a/tests/providers/class-two-factor-totp.php +++ b/tests/providers/class-two-factor-totp.php @@ -343,7 +343,7 @@ public function test_sha1_generate() { $token = $provider->base32_encode( self::$token ); foreach (self::$vectors as $time => $vector) { - $provider::__set_time( (int) $time ); + $provider::set_time( (int) $time ); $this->assertEquals( $vector[0], $provider::calc_totp( $token, false, 8, $hash, self::$step ) ); $this->assertEquals( substr( $vector[0], 2 ), $provider::calc_totp( $token, false, 6, $hash, self::$step ) ); } @@ -363,7 +363,7 @@ public function test_sha1_authenticate() { $token = $provider->base32_encode( self::$token ); foreach ( self::$vectors as $time => $vector ) { - $provider::__set_time( (int) $time ); + $provider::set_time( (int) $time ); $this->assertTrue( $provider::is_valid_authcode( $token, $vector[0], $hash ) ); $this->assertTrue( $provider::is_valid_authcode( $token, substr( $vector[0], 2 ), $hash ) ); } @@ -382,7 +382,7 @@ public function test_sha256_generate() { $token = $provider->base32_encode( self::$token ); foreach ( self::$vectors as $time => $vector ) { - $provider::__set_time( (int) $time ); + $provider::set_time( (int) $time ); $this->assertEquals( $vector[1], $provider::calc_totp( $token, false, 8, $hash, self::$step ) ); $this->assertEquals( substr( $vector[1], 2 ), $provider::calc_totp( $token, false, 6, $hash, self::$step ) ); } @@ -402,7 +402,7 @@ public function test_sha256_authenticate() { $token = $provider->base32_encode( self::$token ); foreach ( self::$vectors as $time => $vector ) { - $provider::__set_time( (int) $time ); + $provider::set_time( (int) $time ); $this->assertTrue( $provider::is_valid_authcode( $token, $vector[1], $hash ) ); $this->assertTrue( $provider::is_valid_authcode( $token, substr( $vector[1], 2 ), $hash ) ); } @@ -421,7 +421,7 @@ public function test_sha512_generate() { $token = $provider->base32_encode( self::$token ); foreach ( self::$vectors as $time => $vector ) { - $provider::__set_time( (int) $time ); + $provider::set_time( (int) $time ); $this->assertEquals( $vector[2], $provider::calc_totp( $token, false, 8, $hash, self::$step ) ); $this->assertEquals( substr($vector[2], 2 ), $provider::calc_totp( $token, false, 6, $hash, self::$step ) ); } @@ -441,7 +441,7 @@ public function test_sha512_authenticate() { $token = $provider->base32_encode( self::$token ); foreach ( self::$vectors as $time => $vector ) { - $provider::__set_time( (int) $time ); + $provider::set_time( (int) $time ); $this->assertTrue( $provider::is_valid_authcode( $token, $vector[2], $hash ) ); $this->assertTrue( $provider::is_valid_authcode( $token, substr( $vector[2], 2 ), $hash ) ); }