From 83742922dda6af9844fb7e0e850a300e5a75527a Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Fri, 22 Dec 2017 09:10:08 -0800 Subject: [PATCH 1/6] 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 | 48 +++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/providers/class.two-factor-totp.php b/providers/class.two-factor-totp.php index f6400410..a3ecd259 100644 --- a/providers/class.two-factor-totp.php +++ b/providers/class.two-factor-totp.php @@ -176,10 +176,12 @@ public function validate_authentication( $user ) { * * @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 ) { + public static function is_valid_authcode( $key, $authcode, $hash = 'sha1', $time_step = 30 ) { /** * Filter the maximum ticks to allow when checking valid codes. * @@ -196,9 +198,11 @@ public static function is_valid_authcode( $key, $authcode ) { $time = time() / self::DEFAULT_TIME_STEP_SEC; + $digits = strlen( $authcode ); + foreach ( $ticks as $offset ) { $log_time = $time + $offset; - if ( self::calc_totp( $key, $log_time ) === $authcode ) { + if ( self::calc_totp( $key, $log_time, $digits, $hash, $time_step ) === $authcode ) { return true; } } @@ -249,6 +253,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 * @@ -263,6 +291,20 @@ public static function pack64( $value ) { 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 ); } @@ -271,7 +313,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 665b11f5eec9e970b6a83c281c6ef82332ce627d Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Thu, 1 Mar 2018 20:36:34 -0800 Subject: [PATCH 2/6] Catch potential exceptions in `calc_totp` --- providers/class.two-factor-totp.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/providers/class.two-factor-totp.php b/providers/class.two-factor-totp.php index a3ecd259..eb4c1dfd 100644 --- a/providers/class.two-factor-totp.php +++ b/providers/class.two-factor-totp.php @@ -202,8 +202,12 @@ public static function is_valid_authcode( $key, $authcode, $hash = 'sha1', $time foreach ( $ticks as $offset ) { $log_time = $time + $offset; - if ( self::calc_totp( $key, $log_time, $digits, $hash, $time_step ) === $authcode ) { - return true; + try { + if ( self::calc_totp( $key, $log_time, $digits, $hash, $time_step ) === $authcode ) { + return true; + } + } catch ( InvalidArgumentException $e ) { + return false; } } return false; @@ -286,6 +290,8 @@ protected static function pad_secret( $secret, $length ) { * @param string $hash The hash used to calculate the code. * @param int $time_step The size of the time step. * + * @throws InvalidArgumentException If an invalid hash type is specified. + * * @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 ) { From 9c7d057f88d5a28b5ed871da3e4e844e39765d9e Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Thu, 1 Mar 2018 21:13:48 -0800 Subject: [PATCH 3/6] Port over the RFC reference tests --- providers/class.two-factor-totp.php | 29 ++++- tests/providers/class.two-factor-totp.php | 132 ++++++++++++++++++++++ 2 files changed, 159 insertions(+), 2 deletions(-) diff --git a/providers/class.two-factor-totp.php b/providers/class.two-factor-totp.php index eb4c1dfd..61f8c899 100644 --- a/providers/class.two-factor-totp.php +++ b/providers/class.two-factor-totp.php @@ -41,6 +41,31 @@ protected function __construct() { return 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; + } + /** * Ensures only one instance of this class exists in memory at any one time. */ @@ -196,7 +221,7 @@ public static function is_valid_authcode( $key, $authcode, $hash = 'sha1', $time $ticks = range( - $max_ticks, $max_ticks ); usort( $ticks, array( __CLASS__, 'abssort' ) ); - $time = time() / self::DEFAULT_TIME_STEP_SEC; + $time = self::time() / self::DEFAULT_TIME_STEP_SEC; $digits = strlen( $authcode ); @@ -312,7 +337,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 6477b499..baf3ccd1 100644 --- a/tests/providers/class.two-factor-totp.php +++ b/tests/providers/class.two-factor-totp.php @@ -5,6 +5,21 @@ 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'] + ]; + + /** + * @var Two_Factor_Totp + */ protected $provider; /** @@ -192,4 +207,121 @@ public function test_is_valid_authcode() { $this->assertTrue( $this->provider->is_valid_authcode( $key, $authcode ) ); } + /** + * @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 71c2cf5e50e6af736588681acce37dfa71812d9e Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Tue, 18 May 2021 11:40:35 -0700 Subject: [PATCH 4/6] Use class defaults. Props @kasparsd --- class-two-factor-core.php | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index b9d1b7b4..e70a5dbf 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -680,27 +680,27 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg - From b2f0fb0e4471a9a0beeede630d2ea5977274ceff Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Tue, 18 May 2021 11:43:24 -0700 Subject: [PATCH 5/6] Revert "Use class defaults. Props @kasparsd" This reverts commit 71c2cf5e50e6af736588681acce37dfa71812d9e. --- class-two-factor-core.php | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index e70a5dbf..b9d1b7b4 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -680,27 +680,27 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg - From 11cf5727ffedb22b4c5ea0fd45bced4d8b0bd737 Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Tue, 18 May 2021 11:43:45 -0700 Subject: [PATCH 6/6] Use class defaults. Props @kasparsd --- providers/class-two-factor-totp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/class-two-factor-totp.php b/providers/class-two-factor-totp.php index d06ed294..f454e308 100644 --- a/providers/class-two-factor-totp.php +++ b/providers/class-two-factor-totp.php @@ -337,7 +337,7 @@ public function validate_authentication( $user ) { * * @return bool Whether the code is valid within the time frame */ - public static function is_valid_authcode( $key, $authcode, $hash = 'sha1', $time_step = 30 ) { + public static function is_valid_authcode( $key, $authcode, $hash = self::DEFAULT_CRYPTO, $time_step = self::DEFAULT_TIME_STEP_SEC ) { /** * Filter the maximum ticks to allow when checking valid codes. *