From 1799950a1e152667431ef7f6f1179f092767d51a Mon Sep 17 00:00:00 2001
From: Florent Morselli <contact@spomky-labs.com>
Date: Fri, 18 Feb 2022 08:04:19 +0100
Subject: [PATCH] Leeway (#152)
* Leeway
* Doc
---
doc/Window.md | 28 ++++++++++++-----
src/TOTP.php | 43 ++++++++------------------
tests/TOTPTest.php | 77 ++++++++++++++++++++++++++++++++++++----------
3 files changed, 94 insertions(+), 54 deletions(-)
diff --git a/doc/Window.md b/doc/Window.md
index 0613984..45fcb23 100644
--- a/doc/Window.md
+++ b/doc/Window.md
@@ -15,13 +15,27 @@ $hotp->verify('123456', 999, 10); // Will return true (1000 is tested)
## Window and TOTP
-The window of timestamps goes from `timestamp - window * period` to `timestamp + window * period`.
-For example, if the `window` is `5`, the period `30` and the timestamp `1476822000`, the OTP tested are within `1476821850` (`1476822000 - 5 * 30`) and `1476822150` (`1476822000 + 5 * 30`).
+The window of TOTP acts as time drift.
+If the `window` is `10`, the period `30` and the timestamp `147682209`,
+the OTP tested are within `1476821999` (`147682209 - 10`), `147682209` and `1476822219` (`147682209 + 10`).
+This includes the previous OTP, but not the next one.
+
+As an example, at the timestamp `147682209` the correct TOTP is `123456`.
+But the user device timestamp `1476821995` and the proposed input is the previous one `654321`.
+
+**It is mandatory to have a window lower than the period**.
```php
-$totp->at(1000); // e.g. will return '123456'
-$totp->verify('123456'); // Will return true
-// 30 seconds later
-$totp->verify('123456'); // Will return false
-$totp->verify('123456', null, 1); // Will return true during the next period
+// Without the window feature, this will fail
+$totp->verify('654321'); // returns false
+
+// With a window 5 seconds, this will fail
+// because the input is tested with 147682209-5 and 147682209+5 seconds.
+// and this does not allow the previous OTP to be used
+$totp->verify('654321', null, 5); // returns false
+
+// With a window 10 seconds, this will succeed
+// because the input is tested with 147682209-10 and 147682209+10 seconds.
+// and the previous OTP is tested
+$totp->verify('654321', null, 10); // returns true
```
diff --git a/src/TOTP.php b/src/TOTP.php
index ea9c0af..2ac2250 100644
--- a/src/TOTP.php
+++ b/src/TOTP.php
@@ -60,17 +60,24 @@ final class TOTP extends OTP implements TOTPInterface
}
/**
- * If no timestamp is provided, the OTP is verified at the actual timestamp.
+ * If no timestamp is provided, the OTP is verified at the actual timestamp. When used, the leeway parameter will
+ * allow time drift. The passed value is in seconds.
*/
- public function verify(string $otp, null|int $timestamp = null, null|int $window = null): bool
+ public function verify(string $otp, null|int $timestamp = null, null|int $leeway = null): bool
{
- $timestamp = $this->getTimestamp($timestamp);
+ $timestamp = $timestamp ?? time();
+ Assertion::greaterOrEqualThan($timestamp, 0, 'Timestamp must be at least 0.');
- if ($window === null) {
+ if ($leeway === null) {
return $this->compareOTP($this->at($timestamp), $otp);
}
- return $this->verifyOtpWithWindow($otp, $timestamp, $window);
+ $leeway = abs($leeway);
+ Assertion::lessThan($leeway, $this->getPeriod(), 'The leeway must be lower than the TOTP period');
+
+ return $this->compareOTP($this->at($timestamp - $leeway), $otp)
+ || $this->compareOTP($this->at($timestamp), $otp)
+ || $this->compareOTP($this->at($timestamp + $leeway), $otp);
}
public function getProvisioningUri(): string
@@ -133,32 +140,6 @@ final class TOTP extends OTP implements TOTPInterface
$this->setParameter('epoch', $epoch);
}
- private function verifyOtpWithWindow(string $otp, int $timestamp, int $window): bool
- {
- $window = abs($window);
-
- for ($i = 0; $i <= $window; ++$i) {
- $next = $i * $this->getPeriod() + $timestamp;
- $previous = -$i * $this->getPeriod() + $timestamp;
- $valid = $this->compareOTP($this->at($next), $otp) ||
- $this->compareOTP($this->at($previous), $otp);
-
- if ($valid) {
- return true;
- }
- }
-
- return false;
- }
-
- private function getTimestamp(null|int $timestamp): int
- {
- $timestamp = $timestamp ?? time();
- Assertion::greaterOrEqualThan($timestamp, 0, 'Timestamp must be at least 0.');
-
- return $timestamp;
- }
-
private function timecode(int $timestamp): int
{
return (int) floor(($timestamp - $this->getEpoch()) / $this->getPeriod());
diff --git a/tests/TOTPTest.php b/tests/TOTPTest.php
index d97d0ef..497105e 100644
--- a/tests/TOTPTest.php
+++ b/tests/TOTPTest.php
@@ -276,33 +276,58 @@ final class TOTPTest extends TestCase
/**
* @test
*/
- public function verifyOtpInWindow(): void
+ public function invalidOtpWindow(): void
{
+ $this->expectException(InvalidArgumentException::class);
+ $this->expectExceptionMessage('The leeway must be lower than the TOTP period');
+ $otp = $this->createTOTP(6, 'sha1', 30);
+ $otp->verify('123456', null, 31);
+ }
+
+ /**
+ * @test
+ * @dataProvider dataLeeway
+ */
+ public function verifyOtpInWindow(int $timestamp, string $input, int $leeway, bool $expectedResult): void
+ {
+ ClockMock::register(TOTP::class);
+ ClockMock::withClockMock($timestamp);
$otp = $this->createTOTP(6, 'sha1', 30);
- static::assertFalse($otp->verify('054409', 319690800, 10)); // -11 periods
- static::assertTrue($otp->verify('808167', 319690800, 10)); // -10 periods
- static::assertTrue($otp->verify('364393', 319690800, 10)); // -9 periods
- static::assertTrue($otp->verify('762124', 319690800, 10)); // 0 periods
- static::assertTrue($otp->verify('988451', 319690800, 10)); // +9 periods
- static::assertTrue($otp->verify('789387', 319690800, 10)); // +10 periods
- static::assertFalse($otp->verify('465009', 319690800, 10)); // +11 periods
+ static::assertSame($expectedResult, $otp->verify($input, null, $leeway));
}
/**
* @test
+ * @dataProvider dataLeewayWithEpoch
*/
- public function verifyOtpWithEpochInWindow(): void
+ public function verifyOtpWithEpochInWindow(int $timestamp, string $input, int $leeway, bool $expectedResult): void
{
+ ClockMock::register(TOTP::class);
+ ClockMock::withClockMock($timestamp);
$otp = $this->createTOTP(6, 'sha1', 30, 'JDDK4U6G3BJLEZ7Y', 'alice@foo.bar', 'My Project', 100);
- static::assertFalse($otp->verify('054409', 319690900, 10)); // -11 periods
- static::assertTrue($otp->verify('808167', 319690900, 10)); // -10 periods
- static::assertTrue($otp->verify('364393', 319690900, 10)); // -9 periods
- static::assertTrue($otp->verify('762124', 319690900, 10)); // 0 periods
- static::assertTrue($otp->verify('988451', 319690900, 10)); // +9 periods
- static::assertTrue($otp->verify('789387', 319690900, 10)); // +10 periods
- static::assertFalse($otp->verify('465009', 319690900, 10)); // +11 periods
+ static::assertSame($expectedResult, $otp->verify($input, null, $leeway));
+ }
+
+ /**
+ * @return array<int, int|string|bool>[]
+ */
+ public function dataLeewayWithEpoch(): array
+ {
+ return [
+ [319690889, '762124', 10, false], //Leeway of 10 seconds, **out** the period of 11sec
+ [319690890, '762124', 10, true], //Leeway of 10 seconds, **out** the period of 10sec
+ [319690899, '762124', 10, true], //Leeway of 10 seconds, **out** the period of 1sec
+ [319690899, '762124', 0, false], //No leeway, **out** the period
+ [319690900, '762124', 0, true], //No leeway, in the period
+ [319690920, '762124', 0, true], //No leeway, in the period
+ [319690929, '762124', 0, true], //No leeway, in the period
+ [319690930, '762124', 0, false], //No leeway, **out** the period
+ [319690930, '762124', 10, true], //Leeway of 10 seconds, **out** the period of 1sec
+ [319690939, '762124', 10, true], //Leeway of 10 seconds, **out** the period of 10sec
+ [319690940, '762124', 10, false], //Leeway of 10 seconds, **out** the period of 11sec
+ ];
}
/**
@@ -349,6 +374,26 @@ final class TOTPTest extends TestCase
];
}
+ /**
+ * @return array<int, int|string|bool>[]
+ */
+ public function dataLeeway(): array
+ {
+ return [
+ [319690789, '762124', 10, false], //Leeway of 10 seconds, **out** the period of 11sec
+ [319690790, '762124', 10, true], //Leeway of 10 seconds, **out** the period of 10sec
+ [319690799, '762124', 10, true], //Leeway of 10 seconds, **out** the period of 1sec
+ [319690799, '762124', 0, false], //No leeway, **out** the period
+ [319690800, '762124', 0, true], //No leeway, in the period
+ [319690820, '762124', 0, true], //No leeway, in the period
+ [319690829, '762124', 0, true], //No leeway, in the period
+ [319690830, '762124', 0, false], //No leeway, **out** the period
+ [319690830, '762124', 10, true], //Leeway of 10 seconds, **out** the period of 1sec
+ [319690839, '762124', 10, true], //Leeway of 10 seconds, **out** the period of 10sec
+ [319690840, '762124', 10, false], //Leeway of 10 seconds, **out** the period of 11sec
+ ];
+ }
+
private function createTOTP(
int $digits,
string $digest,
--
GitLab