diff --git a/README.md b/README.md index 7d62f65..dbfa47e 100644 --- a/README.md +++ b/README.md @@ -154,8 +154,9 @@ $otp = new OTP( digitCount: 6, validUpto: 60, retry: 3, - hashAlgorithm: 'xxh128', + hashAlgorithm: 'sha256', cacheAdapter: $cachePool, + hashKey: $applicationOtpKey, ); $code = $otp->generate('signup:alice@example.com'); @@ -169,6 +170,8 @@ Notes: - Codes are strings, not integers - Leading zeroes are preserved - Digit count must be between `4` and `10` +- Stored OTP digests use SHA-256 or SHA-512; non-cryptographic hashes are rejected +- For production, use a purpose-specific `hashKey` of at least 16 random bytes and keep it outside the OTP cache ### OCRA @@ -257,8 +260,11 @@ $parsed->ocraSuite; The package ships with contracts plus an in-memory store for testing and lightweight use: - `Infocyph\OTP\Contracts\ReplayStoreInterface` +- `Infocyph\OTP\Contracts\AtomicReplayStoreInterface` - `Infocyph\OTP\Stores\InMemoryReplayStore` +Use `AtomicReplayStoreInterface` for production stores so consume-once and monotonic-counter updates remain safe under concurrent requests. Legacy `ReplayStoreInterface` implementations remain supported but cannot provide that atomic guarantee. + Recommended usage: - TOTP: store accepted timesteps per user/device binding @@ -299,6 +305,8 @@ $result->totalGenerated; $result->lastUsedAt; ``` +For production, pass a purpose-specific secret `hashKey` of at least 16 random bytes and store it outside the recovery-code database. + Notes: - Recovery codes are stored hashed diff --git a/benchmarks/OtpBench.php b/benchmarks/OtpBench.php index 679db98..9b02c23 100644 --- a/benchmarks/OtpBench.php +++ b/benchmarks/OtpBench.php @@ -11,6 +11,7 @@ use Infocyph\OTP\Tests\Support\InMemoryCacheItemPool; use Infocyph\OTP\TOTP; use PhpBench\Attributes\BeforeMethods; +use PhpBench\Attributes\Revs; #[BeforeMethods('setUp')] final class OtpBench @@ -33,6 +34,8 @@ final class OtpBench private string $totpCode; + private InMemoryReplayStore $totpReplayStore; + public function setUp(): void { $this->totp = (new TOTP( @@ -53,7 +56,7 @@ public function setUp(): void digitCount: 6, validUpto: 60, retry: 3, - hashAlgorithm: 'xxh128', + hashAlgorithm: 'sha256', cacheAdapter: new InMemoryCacheItemPool(), ); @@ -61,6 +64,13 @@ public function setUp(): void $this->hotpCode = $this->hotp->getOTP(5); $this->ocraCode = $this->ocra->generate('12345678', 0); $this->genericCode = $this->genericOtp->generate($this->signature); + $this->totpReplayStore = new InMemoryReplayStore(); + $this->totpReplayStore->consumeOnce( + 'totp:step', + 'bench-user', + (string) $this->totp->getCurrentTimeStep(1716532624), + 90, + ); } public function benchGenericOtpGenerate(): void @@ -69,18 +79,24 @@ public function benchGenericOtpGenerate(): void digitCount: 6, validUpto: 60, retry: 3, - hashAlgorithm: 'xxh128', + hashAlgorithm: 'sha256', cacheAdapter: new InMemoryCacheItemPool(), ); $otp->generate('bench:another@example.com'); } + #[Revs(1)] public function benchGenericOtpVerify(): void { $this->genericOtp->verify($this->signature, $this->genericCode); } + public function benchGenericOtpVerifyMalformed(): void + { + $this->genericOtp->verify($this->signature, 'invalid'); + } + public function benchHotpGenerate(): void { $this->hotp->getOTP(5); @@ -91,6 +107,11 @@ public function benchHotpVerify(): void $this->hotp->verify($this->hotpCode, 5, 3); } + public function benchHotpVerifyInvalid(): void + { + $this->hotp->verify('000000', 5, 3); + } + public function benchOcraGenerate(): void { $this->ocra->generate('12345678', 0); @@ -101,6 +122,11 @@ public function benchOcraVerify(): void $this->ocra->verify($this->ocraCode, '12345678', 0); } + public function benchOcraVerifyInvalid(): void + { + $this->ocra->verify('00000000', '12345678', 0); + } + public function benchTotpGenerate(): void { $this->totp->getOTP(1716532624); @@ -111,7 +137,17 @@ public function benchTotpVerify(): void $this->totp->verify($this->totpCode, 1716532624, 1, 1); } - public function benchTotpVerifyWithReplayStore(): void + public function benchTotpVerifyInvalid(): void + { + $this->totp->verify('000000', 1716532624, 1, 1); + } + + public function benchTotpVerifyMalformed(): void + { + $this->totp->verify('invalid', 1716532624, 1, 1); + } + + public function benchTotpVerifyReplayAccepted(): void { $store = new InMemoryReplayStore(); $this->totp->verifyWithWindow( @@ -121,4 +157,14 @@ public function benchTotpVerifyWithReplayStore(): void binding: 'bench-user', ); } + + public function benchTotpVerifyReplayRejected(): void + { + $this->totp->verifyWithWindow( + $this->totpCode, + 1716532624, + replayStore: $this->totpReplayStore, + binding: 'bench-user', + ); + } } diff --git a/docs/api/contracts.rst b/docs/api/contracts.rst index 1457cfa..4101e02 100644 --- a/docs/api/contracts.rst +++ b/docs/api/contracts.rst @@ -6,6 +6,11 @@ ReplayStoreInterface Use this to persist replay state for TOTP, HOTP, or OCRA verification flows. +AtomicReplayStoreInterface +-------------------------- + +Use this extension for production replay stores. Its ``consumeOnce()`` and ``advance()`` operations must be implemented atomically so concurrent requests cannot accept the same TOTP/OCRA token or move an HOTP counter backwards. + RecoveryCodeStoreInterface -------------------------- diff --git a/docs/getting-started/migration.rst b/docs/getting-started/migration.rst index 1e109ea..a4777c6 100644 --- a/docs/getting-started/migration.rst +++ b/docs/getting-started/migration.rst @@ -10,6 +10,10 @@ Generic OTP - Leading zeroes are preserved. - The generic OTP class now expects a caller-provided PSR-6 cache pool. - Digit count is validated as OTP digits, not PHP integer size. +- Generic OTP now accepts only ``sha256`` and ``sha512`` for stored-code digests. +- Signature cache keys now use SHA-256. Outstanding cache entries created with the earlier ``xxh3`` key format are intentionally not reused. +- Validity is bounded to 86400 seconds, retries to 100, and signatures to 4096 bytes. +- An optional final ``hashKey`` constructor argument enables keyed HMAC storage without changing existing positional arguments. TOTP ---- @@ -17,12 +21,15 @@ TOTP - The old boolean leeway model has been replaced by configurable past/future windows. - A simple boolean API remains available through ``verify()``. - Richer inspection is available through ``verifyWithWindow()`` and ``VerificationResult``. +- Verification windows are bounded to 100 total drift steps and TOTP periods to 86400 seconds. +- Malformed submitted codes return a non-matching result instead of using exceptions for expected verification flow. HOTP ---- - ``verify()`` supports a look-ahead window. - ``verifyWithResult()`` returns matched counter and drift information. +- HOTP look-ahead is bounded to 100 counters. Provisioning ------------ @@ -30,9 +37,12 @@ Provisioning - ``otpauth://`` parsing is now available. - Enrollment payload helpers expose URI, QR payload, and optional SVG. - Label and issuer handling is stricter and centralized. +- Parsing rejects duplicate parameters, conflicting label/query issuers, invalid Base32 encodings, and oversized URIs. Recovery codes and replay protection ------------------------------------ - Recovery code generation and consumption are now first-class features. - Replay protection is pluggable through interfaces and in-memory stores. +- ``AtomicReplayStoreInterface`` adds atomic consume-once and monotonic advance operations for concurrency-safe replay protection. +- OCRA generated Base32 secrets should be constructed with ``OCRA::fromBase32()``; the constructor continues to accept raw key bytes for RFC test-vector and binary-key compatibility. diff --git a/docs/getting-started/quickstart.rst b/docs/getting-started/quickstart.rst index 1019cf8..739b931 100644 --- a/docs/getting-started/quickstart.rst +++ b/docs/getting-started/quickstart.rst @@ -68,7 +68,7 @@ Generic OTP digitCount: 6, validUpto: 60, retry: 3, - hashAlgorithm: 'xxh128', + hashAlgorithm: 'sha256', cacheAdapter: $cachePool, ); diff --git a/docs/guides/custom-stores.rst b/docs/guides/custom-stores.rst index 2330cac..a2aebf3 100644 --- a/docs/guides/custom-stores.rst +++ b/docs/guides/custom-stores.rst @@ -15,6 +15,7 @@ The relevant contracts are: - ``Infocyph\OTP\Contracts\RecoveryCodeStoreInterface`` - ``Infocyph\OTP\Contracts\ReplayStoreInterface`` +- ``Infocyph\OTP\Contracts\AtomicReplayStoreInterface`` - ``Infocyph\OTP\Contracts\SecretStoreInterface`` Secret storage guidance @@ -84,6 +85,8 @@ That means your system can answer, day to day: Example PDO store ~~~~~~~~~~~~~~~~~ +The following class illustrates the legacy read/write contract. It is suitable for understanding the data model, but separate ``hasConsumed()``/``markConsumed()`` and ``getState()``/``setState()`` calls are not atomic. + .. code-block:: php setPin('1234'); $code = $ocra->generate('12345678', 0); @@ -258,7 +267,7 @@ Notes on optional inputs: - if the suite includes ``C``, you should provide a counter - if the suite includes ``PSHA1`` / ``PSHA256`` / ``PSHA512``, you should call ``setPin()`` -- if the suite includes ``Snnn``, you should call ``setSession()`` +- if the suite includes ``Snnn``, you should call ``setSession()`` with an even-length hexadecimal representation no longer than the configured byte length - if the suite includes ``T...``, you can optionally call ``setTime()`` to verify or generate for a specific moment Parsed suite details @@ -311,6 +320,8 @@ For many OCRA use cases, you should reject: - reused counter values where counters are present - previously accepted challenge and counter combinations +For suites containing ``C``, atomic replay stores enforce a strictly increasing accepted counter. For suites without ``C``, the challenge/counter token is consumed once. + Example: .. code-block:: php diff --git a/docs/guides/provisioning.rst b/docs/guides/provisioning.rst index d0ec176..2e9a499 100644 --- a/docs/guides/provisioning.rst +++ b/docs/guides/provisioning.rst @@ -27,7 +27,11 @@ For TOTP, HOTP, and OCRA, you can generate a new Base32 secret before building p $totpSecret = TOTP::generateSecret(); $hotpSecret = HOTP::generateSecret(); - $ocraSharedKey = OCRA::generateSecret(); + $ocraSecret = OCRA::generateSecret(); + $ocra = OCRA::fromBase32( + 'OCRA-1:HOTP-SHA256-8:C-QN08-PSHA1', + $ocraSecret, + ); Provisioning URIs ----------------- @@ -101,7 +105,10 @@ OCRA QR example: getProvisioningUriQR( 'alice@example.com', 'Example App', @@ -176,4 +183,6 @@ The provisioning layer: - normalizes issuers - safely formats labels +- rejects duplicate query parameters and conflicting issuer identities while parsing +- validates canonical Base32 secrets and bounds URI/query sizes - separates URI generation from QR rendering diff --git a/docs/guides/recovery-codes.rst b/docs/guides/recovery-codes.rst index 88e9f8a..da0f8cc 100644 --- a/docs/guides/recovery-codes.rst +++ b/docs/guides/recovery-codes.rst @@ -23,6 +23,19 @@ Generating codes $generated->totalGenerated; $generated->remainingCount; +For production, provide a purpose-specific HMAC key kept outside the recovery-code store: + +.. code-block:: php + + planSecretRotation( - 'abcdefghijklmnopqrstuvwxyz123456', + $nextSecret, label: 'alice@example.com', issuer: 'Example App', gracePeriodInSeconds: 900, diff --git a/docs/guides/storage.rst b/docs/guides/storage.rst index 711be6c..0c38cc8 100644 --- a/docs/guides/storage.rst +++ b/docs/guides/storage.rst @@ -51,6 +51,7 @@ Contracts The package includes contracts you can implement in your own infrastructure: - ``Infocyph\OTP\Contracts\ReplayStoreInterface`` +- ``Infocyph\OTP\Contracts\AtomicReplayStoreInterface`` - ``Infocyph\OTP\Contracts\RecoveryCodeStoreInterface`` - ``Infocyph\OTP\Contracts\SecretStoreInterface`` diff --git a/docs/guides/totp.rst b/docs/guides/totp.rst index 39184b3..5ffe539 100644 --- a/docs/guides/totp.rst +++ b/docs/guides/totp.rst @@ -87,6 +87,8 @@ Windowed verification Real-world authenticators can drift slightly. RFC6238 deployments commonly allow a small validation window around the current time-step. +The current timestep is checked first for the common path. The combined past/future window is bounded to 100 steps, and configured periods are bounded to 86400 seconds. + This library supports: - previous windows only diff --git a/src/AbstractOtpAuthenticator.php b/src/AbstractOtpAuthenticator.php index eef9627..03131f4 100644 --- a/src/AbstractOtpAuthenticator.php +++ b/src/AbstractOtpAuthenticator.php @@ -4,6 +4,7 @@ namespace Infocyph\OTP; +use Infocyph\OTP\Contracts\ReplayStoreInterface; use Infocyph\OTP\Support\ProvisioningUriBuilder; use Infocyph\OTP\Support\ProvisioningUriParser; use Infocyph\OTP\Support\SvgQrRenderer; @@ -19,11 +20,23 @@ public static function parseProvisioningUri(string $uri): ParsedOtpAuthUri final protected function assertOtp(string $otp, int $digitCount): void { - if (!preg_match('/^\d+$/', $otp) || strlen($otp) !== $digitCount) { + if (!$this->isValidOtp($otp, $digitCount)) { throw new \InvalidArgumentException('OTP must be a numeric string matching the configured digit count.'); } } + final protected function assertReplayBinding( + ?ReplayStoreInterface $replayStore, + ?string $binding, + ): void { + if (($replayStore === null) !== ($binding === null)) { + throw new \InvalidArgumentException('Replay store and binding must be provided together.'); + } + if ($binding !== null && (trim($binding) === '' || strlen($binding) > 512)) { + throw new \InvalidArgumentException('Replay binding must contain between 1 and 512 bytes.'); + } + } + /** * @param $otpType OTP type (`totp`, `hotp`, or `ocra`). * @param $secret Normalized Base32 secret. @@ -38,6 +51,7 @@ final protected function assertOtp(string $otp, int $digitCount): void * @param $withQrSvg Whether to render QR SVG. * @param $imageSize QR image size. * @param $uri Provisioning URI. + * * @phpstan-param list $include * @phpstan-param array $additionalParameters */ @@ -83,6 +97,7 @@ final protected function buildEnrollmentPayload( * @param $digitCount OTP digit length. * @param $period TOTP period in seconds. * @param $counter HOTP counter value. + * * @phpstan-param list $include * @phpstan-param array $additionalParameters */ @@ -113,7 +128,7 @@ final protected function buildProvisioningUri( } /** - * @param $include Optional provisioning flags. + * @param array $include Optional provisioning flags. * @return array Include flags keyed by name. * @phpstan-param list $include * @phpstan-return array @@ -122,4 +137,9 @@ final protected function includeFlags(array $include): array { return array_fill_keys($include, true); } + + final protected function isValidOtp(string $otp, int $digitCount): bool + { + return strlen($otp) === $digitCount && ctype_digit($otp); + } } diff --git a/src/Contracts/AtomicReplayStoreInterface.php b/src/Contracts/AtomicReplayStoreInterface.php new file mode 100644 index 0000000..5d290d5 --- /dev/null +++ b/src/Contracts/AtomicReplayStoreInterface.php @@ -0,0 +1,12 @@ +secret = SecretUtility::normalizeBase32($secret); + $this->binarySecret = SecretUtility::decodeBase32($this->secret); } /** @@ -81,7 +88,7 @@ public function getEnrollmentPayload( public function getOTP(int $counter): string { - return OtpMath::hotp($this->secret, $counter, $this->digitCount, $this->algorithm); + return OtpMath::hotpFromBinary($this->binarySecret, $counter, $this->digitCount, $this->algorithm); } /** @@ -158,19 +165,15 @@ public function planSecretRotation( bool $withQrSvg = false, int $imageSize = 200, ): SecretRotation { - if ($gracePeriodInSeconds !== null && $gracePeriodInSeconds < 0) { - throw new \InvalidArgumentException('Grace period must be non-negative.'); - } - - $normalizedSecret = SecretUtility::normalizeBase32($newSecret); - $next = new self($normalizedSecret, $this->digitCount); + $rotation = SecretRotationPlanner::prepare($this->secret, $newSecret, $gracePeriodInSeconds, $now); + $next = new self($rotation['nextSecret'], $this->digitCount); $next->setAlgorithm($this->algorithm); $next->setCounter($this->counter); return new SecretRotation( $this->secret, - $normalizedSecret, - $gracePeriodInSeconds !== null ? new \DateTimeImmutable()->setTimestamp(($now ?? time()) + $gracePeriodInSeconds) : null, + $rotation['nextSecret'], + $rotation['overlapUntil'] !== null ? new \DateTimeImmutable()->setTimestamp($rotation['overlapUntil']) : null, $next->getEnrollmentPayload($label, $issuer, $include, $additionalParameters, $withQrSvg, $imageSize), ); } @@ -205,34 +208,63 @@ public function verifyWithResult( ?ReplayStoreInterface $replayStore = null, ?string $binding = null, ): VerificationResult { - $this->assertOtp($otp, $this->digitCount); - if ($counter < 0 || $lookAhead < 0) { - throw new \InvalidArgumentException('Counter and look-ahead window must be non-negative.'); + if (!$this->isValidOtp($otp, $this->digitCount)) { + return new VerificationResult(false, 'malformed'); + } + self::assertVerificationRange($counter, $lookAhead); + $this->assertReplayBinding($replayStore, $binding); + + $matchedCounter = $this->findMatchingCounter($otp, $counter, $lookAhead); + if ($matchedCounter === null) { + return new VerificationResult(false, 'mismatch'); } + if ( + $replayStore !== null + && $binding !== null + && $this->isReplay($replayStore, $binding, $matchedCounter) + ) { + return new VerificationResult(false, 'replay', matchedCounter: $matchedCounter, replayDetected: true); + } + + $offset = $matchedCounter - $counter; + + return new VerificationResult( + true, + $offset === 0 ? 'matched' : 'resynchronized', + matchedCounter: $matchedCounter, + driftOffset: $offset, + verifiedAt: new \DateTimeImmutable(), + ); + } + + private static function assertVerificationRange(int $counter, int $lookAhead): void + { + if ($counter < 0 || $lookAhead < 0 || $lookAhead > self::MAX_LOOK_AHEAD) { + throw new \InvalidArgumentException('Counter must be non-negative and look-ahead may not exceed 100.'); + } + if ($lookAhead > PHP_INT_MAX - $counter) { + throw new \InvalidArgumentException('Counter and look-ahead window exceed the supported integer range.'); + } + } + + private function findMatchingCounter(string $otp, int $counter, int $lookAhead): ?int + { for ($offset = 0; $offset <= $lookAhead; $offset++) { $matchedCounter = $counter + $offset; - if (!hash_equals($otp, $this->getOTP($matchedCounter))) { - continue; + if (hash_equals($this->getOTP($matchedCounter), $otp)) { + return $matchedCounter; } - - if ($replayStore !== null && $binding !== null) { - $lastCounter = $replayStore->getState('hotp:last_counter', $binding); - if (is_int($lastCounter) && $matchedCounter <= $lastCounter) { - return new VerificationResult(false, 'replay', matchedCounter: $matchedCounter, replayDetected: true); - } - $replayStore->setState('hotp:last_counter', $binding, $matchedCounter); - } - - return new VerificationResult( - true, - $offset === 0 ? 'matched' : 'resynchronized', - matchedCounter: $matchedCounter, - driftOffset: $offset, - verifiedAt: new \DateTimeImmutable(), - ); } - return new VerificationResult(false, 'mismatch'); + return null; + } + + private function isReplay( + ReplayStoreInterface $replayStore, + string $binding, + int $matchedCounter, + ): bool { + return !ReplayProtection::advance($replayStore, 'hotp:last_counter', $binding, $matchedCounter); } } diff --git a/src/OCRA.php b/src/OCRA.php index f8cd86e..803a7b8 100644 --- a/src/OCRA.php +++ b/src/OCRA.php @@ -6,22 +6,28 @@ use DateTimeInterface; use Exception; +use Infocyph\OTP\Contracts\AtomicReplayStoreInterface; use Infocyph\OTP\Contracts\ReplayStoreInterface; use Infocyph\OTP\Exceptions\OCRAException; use Infocyph\OTP\Result\VerificationResult; use Infocyph\OTP\Support\AlgorithmValidator; +use Infocyph\OTP\Support\OcraSuiteValidator; use Infocyph\OTP\Support\ProvisioningUriBuilder; use Infocyph\OTP\Support\ProvisioningUriParser; +use Infocyph\OTP\Support\ReplayProtection; +use Infocyph\OTP\Support\SecretRotationPlanner; use Infocyph\OTP\Support\SecretUtility; use Infocyph\OTP\Support\SvgQrRenderer; use Infocyph\OTP\ValueObjects\EnrollmentPayload; use Infocyph\OTP\ValueObjects\OcraSuite; use Infocyph\OTP\ValueObjects\ParsedOtpAuthUri; use Infocyph\OTP\ValueObjects\SecretRotation; +use InvalidArgumentException; +use ParagonIE\ConstantTime\Base32; final class OCRA { - private const string OCRA_REGEX = '/^OCRA-1:HOTP-SHA(1|256|512)-(0|[4-9]|10):(C-)?Q([ANH])(0[4-9]|[1-5]\d|6[0-4])(-(P(SHA1|SHA256|SHA512)|S\d{3}|(T((\d|[1-3]\d|4[0-8])H|(([1-9]|[1-5]\d)([SM]))))))*$/'; + private readonly string $base32Secret; /** * @var array{suite:string,algo:string,length:int,c:bool,q:array{format:string,value:int},optionals:array} @@ -36,9 +42,19 @@ final class OCRA public function __construct(string $ocraSuite, private readonly string $sharedKey) { + if (strlen($sharedKey) < 16 || strlen($sharedKey) > 1024) { + throw new OCRAException('OCRA shared keys must contain between 16 and 1024 bytes.'); + } + + $this->base32Secret = rtrim(Base32::encodeUpper($sharedKey), '='); $this->validateAndParse($ocraSuite); } + public static function fromBase32(string $ocraSuite, string $secret): self + { + return new self($ocraSuite, SecretUtility::decodeBase32($secret)); + } + /** * @param $bytes Secret byte length. * @throws Exception @@ -118,7 +134,7 @@ public function getEnrollmentPayload( return ProvisioningUriBuilder::enrollmentPayload( 'ocra', - $this->sharedKey, + $this->base32Secret, $label, $issuer, array_fill_keys($include, true), @@ -148,7 +164,7 @@ public function getProvisioningUri( ): string { return ProvisioningUriBuilder::build( 'ocra', - $this->sharedKey, + $this->base32Secret, $label, $issuer, array_fill_keys($include, true), @@ -220,24 +236,31 @@ public function planSecretRotation( bool $withQrSvg = false, int $imageSize = 200, ): SecretRotation { - if ($gracePeriodInSeconds !== null && $gracePeriodInSeconds < 0) { - throw new OCRAException('Grace period must be non-negative.'); + try { + $rotation = SecretRotationPlanner::prepare( + $this->base32Secret, + $newSecret, + $gracePeriodInSeconds, + $now, + ); + } catch (InvalidArgumentException $exception) { + throw new OCRAException($exception->getMessage(), previous: $exception); } - $next = new self($this->ocraSuite['suite'], $newSecret); + $next = self::fromBase32($this->ocraSuite['suite'], $rotation['nextSecret']); return new SecretRotation( - $this->sharedKey, - $newSecret, - $gracePeriodInSeconds !== null ? new \DateTimeImmutable()->setTimestamp(($now ?? time()) + $gracePeriodInSeconds) : null, + $this->base32Secret, + $rotation['nextSecret'], + $rotation['overlapUntil'] !== null ? new \DateTimeImmutable()->setTimestamp($rotation['overlapUntil']) : null, $next->getEnrollmentPayload($label, $issuer, $include, $additionalParameters, $withQrSvg, $imageSize), ); } public function setPin(string $pin): self { - if ($pin === '') { - throw new OCRAException('PIN cannot be empty.'); + if ($pin === '' || strlen($pin) > 1024) { + throw new OCRAException('PIN must contain between 1 and 1024 bytes.'); } $this->pin = $pin; @@ -246,8 +269,13 @@ public function setPin(string $pin): self public function setSession(string $session): self { - if ($session === '') { - throw new OCRAException('Session cannot be empty.'); + if ($session === '' || strlen($session) % 2 !== 0 || !ctype_xdigit($session)) { + throw new OCRAException('Session must be a non-empty, even-length hexadecimal string.'); + } + foreach ($this->ocraSuite['optionals'] as $optional) { + if ($optional['format'] === 's' && strlen($session) > ((int) $optional['value'] * 2)) { + throw new OCRAException('Session exceeds the byte length configured by the OCRA suite.'); + } } $this->session = $session; @@ -256,6 +284,9 @@ public function setSession(string $session): self public function setTime(DateTimeInterface $dateTime): self { + if ($dateTime->getTimestamp() < 0) { + throw new OCRAException('OCRA timestamps must be non-negative.'); + } $this->time = $dateTime->format('U'); return $this; @@ -273,37 +304,78 @@ public function verifyWithResult( ?ReplayStoreInterface $replayStore = null, ?string $binding = null, ): VerificationResult { + if ( + $this->ocraSuite['length'] > 0 + && (strlen($otp) !== $this->ocraSuite['length'] || !ctype_digit($otp)) + ) { + return new VerificationResult(false, 'malformed'); + } + $this->assertReplayBinding($replayStore, $binding); $expected = $this->generate($challenge, $counter); if (!hash_equals($expected, $otp)) { return new VerificationResult(false, 'mismatch'); } - if ($replayStore !== null && $binding !== null) { - $token = $challenge . '|' . $counter; - if ($replayStore->hasConsumed('ocra:challenge', $binding, $token)) { - return new VerificationResult(false, 'replay', matchedCounter: $counter, replayDetected: true); - } + if ( + $replayStore !== null + && $binding !== null + && $this->isReplay($replayStore, $binding, $challenge, $counter) + ) { + return new VerificationResult(false, 'replay', matchedCounter: $counter, replayDetected: true); + } + + return new VerificationResult(true, 'matched', matchedCounter: $counter, verifiedAt: new \DateTimeImmutable()); + } + + private static function decimalToBinary(string $decimal): string + { + $decimal = ltrim($decimal, '0'); + if ($decimal === '') { + return "\0"; + } - $replayStore->markConsumed('ocra:challenge', $binding, $token); - if ($this->ocraSuite['c']) { - $replayStore->setState('ocra:last_counter', $binding, $counter); + $binary = ''; + while ($decimal !== '') { + $quotient = ''; + $remainder = 0; + $length = strlen($decimal); + for ($index = 0; $index < $length; $index++) { + $value = ($remainder * 10) + (ord($decimal[$index]) - 48); + $digit = intdiv($value, 16); + if ($quotient !== '' || $digit !== 0) { + $quotient .= (string) $digit; + } + $remainder = $value % 16; } + + $binary = dechex($remainder) . $binary; + $decimal = $quotient; } - return new VerificationResult(true, 'matched', matchedCounter: $counter, verifiedAt: new \DateTimeImmutable()); + return pack('H*', $binary); } private function assertChallenge(string $challenge): void { $length = $this->ocraSuite['q']['value']; match ($this->ocraSuite['q']['format']) { - 'n' => preg_match('/^\d{' . $length . '}$/', $challenge) === 1 || throw new OCRAException('Challenge must be a numeric string of the expected length.'), + 'n' => preg_match('/^\d{1,' . $length . '}$/', $challenge) === 1 || throw new OCRAException('Challenge must be a numeric string within the configured length.'), 'a' => preg_match('/^[A-Za-z0-9]{1,128}$/', $challenge) === 1 || throw new OCRAException('Challenge must be alphanumeric and at most 128 characters.'), - 'h' => preg_match('/^[A-Fa-f0-9]{1,' . ($length * 2) . '}$/', $challenge) === 1 || throw new OCRAException('Challenge must be hexadecimal.'), + 'h' => preg_match('/^[A-Fa-f0-9]{1,' . $length . '}$/', $challenge) === 1 || throw new OCRAException('Challenge must be hexadecimal.'), default => throw new OCRAException('Invalid challenge format'), }; } + private function assertReplayBinding(?ReplayStoreInterface $replayStore, ?string $binding): void + { + if (($replayStore === null) !== ($binding === null)) { + throw new OCRAException('Replay store and binding must be provided together.'); + } + if ($binding !== null && (trim($binding) === '' || strlen($binding) > 512)) { + throw new OCRAException('Replay binding must contain between 1 and 512 bytes.'); + } + } + private function calculateOptionals(): string { $optionals = ''; @@ -325,13 +397,48 @@ private function calculateOptionals(): string private function calculateQ(string $input): string { return match ($this->ocraSuite['q']['format']) { - 'n' => str_pad(pack('H*', dechex((int) $input)), 128, "\0"), + 'n' => str_pad(self::decimalToBinary($input), 128, "\0"), 'a' => str_pad(substr($input, 0, 128), 128, "\0"), 'h' => str_pad(pack('H*', substr($input, 0, 256)), 128, "\0"), default => throw new OCRAException('Unsupported challenge format.'), }; } + private function isCounterReplay( + ReplayStoreInterface $replayStore, + string $binding, + int $counter, + ): bool { + return !ReplayProtection::advance($replayStore, 'ocra:last_counter', $binding, $counter); + } + + private function isReplay( + ReplayStoreInterface $replayStore, + string $binding, + string $challenge, + int $counter, + ): bool { + if ($this->ocraSuite['c']) { + return $this->isCounterReplay($replayStore, $binding, $counter); + } + + $token = $challenge . '|' . $counter; + if ($replayStore instanceof AtomicReplayStoreInterface) { + $consumed = $replayStore->consumeOnce('ocra:challenge', $binding, $token); + } else { + $consumed = !$replayStore->hasConsumed('ocra:challenge', $binding, $token); + if ($consumed) { + $replayStore->markConsumed('ocra:challenge', $binding, $token); + } + } + + if (!$consumed) { + return true; + } + + return false; + } + /** * @param $parts Parsed OCRA suite parts. * @return array Parsed conditional suite data. @@ -373,7 +480,7 @@ private function prepareConditionalParts(array $parts): array private function validateAndParse(string $ocraSuite): void { - if (!preg_match(self::OCRA_REGEX, $ocraSuite, $matches) || $matches[0] !== $ocraSuite) { + if (!OcraSuiteValidator::isValid($ocraSuite)) { throw new OCRAException('Invalid OCRA Suite.'); } @@ -384,5 +491,11 @@ private function validateAndParse(string $ocraSuite): void 'algo' => AlgorithmValidator::normalize($parts[3]), 'length' => (int) $parts[4], ] + $conditionalParts; + + foreach ($this->ocraSuite['optionals'] as $optional) { + if ($optional['format'] === 's' && ((int) $optional['value'] < 1 || (int) $optional['value'] > 512)) { + throw new OCRAException('OCRA session length must be between 1 and 512 bytes.'); + } + } } } diff --git a/src/OTP.php b/src/OTP.php index b9b5922..ad01764 100644 --- a/src/OTP.php +++ b/src/OTP.php @@ -5,28 +5,73 @@ namespace Infocyph\OTP; use Exception; +use InvalidArgumentException as NativeInvalidArgumentException; use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; use Psr\Cache\InvalidArgumentException; +use RuntimeException; final readonly class OTP { + private const string CACHE_KEY_PREFIX = 'ao-otp_'; + + private const int MAX_RETRIES = 100; + + private const int MAX_SIGNATURE_LENGTH = 4096; + + private const int MAX_VALIDITY_SECONDS = 86400; + + private int $digitCount; + + private string $hashAlgorithm; + + private ?string $hashKey; + + private int $retry; + + private int $validUpto; + /** * Constructor for the class. * * @param int $digitCount The number of digits. * @param int $validUpto The number of seconds until the code expires. - * @param $retry The number of allowed retries. - * @param $hashAlgorithm Hashing algorithm used for stored OTPs. - * @param $cacheAdapter PSR-6 cache adapter. + * @param int $retry The number of allowed retries. + * @param string $hashAlgorithm Hashing algorithm used for stored OTPs. + * @param CacheItemPoolInterface|null $cacheAdapter PSR-6 cache adapter. + * @param string|null $hashKey Optional application HMAC key. */ public function __construct( - private int $digitCount = 6, - private int $validUpto = 30, - private int $retry = 3, - private string $hashAlgorithm = 'xxh128', + int $digitCount = 6, + int $validUpto = 30, + int $retry = 3, + string $hashAlgorithm = 'sha256', private ?CacheItemPoolInterface $cacheAdapter = null, - ) {} + ?string $hashKey = null, + ) { + if ($digitCount < 4 || $digitCount > 10) { + throw new NativeInvalidArgumentException('The number of digits must be between 4 and 10.'); + } + if ($retry < 0 || $retry > self::MAX_RETRIES) { + throw new NativeInvalidArgumentException('The number of retries must be between 0 and 100.'); + } + if ($validUpto < 1 || $validUpto > self::MAX_VALIDITY_SECONDS) { + throw new NativeInvalidArgumentException('Validity duration must be between 1 and 86400 seconds.'); + } + + $this->digitCount = $digitCount; + $this->validUpto = $validUpto; + $this->retry = $retry; + $this->hashAlgorithm = match (strtolower(trim($hashAlgorithm))) { + 'sha256' => 'sha256', + 'sha512' => 'sha512', + default => throw new NativeInvalidArgumentException('Generic OTP storage requires SHA-256 or SHA-512.'), + }; + if ($hashKey !== null && strlen($hashKey) < 16) { + throw new NativeInvalidArgumentException('Generic OTP HMAC keys must contain at least 16 bytes.'); + } + $this->hashKey = $hashKey; + } /** * Deletes an OTP based on the given signature. @@ -38,7 +83,7 @@ public function __construct( */ public function delete(string $signature): bool { - return $this->getCacheAdapter()->deleteItem('ao-otp_' . hash('xxh3', $signature)); + return $this->getCacheAdapter()->deleteItem($this->cacheKey($signature)); } /** @@ -59,10 +104,9 @@ public function flush(): bool */ public function generate(string $signature): string { - $this->validateRequirements(); - $otpAdapter = $this->getCacheAdapter()->getItem('ao-otp_' . hash('xxh3', $signature)); + $otpAdapter = $this->getCacheAdapter()->getItem($this->cacheKey($signature)); $otp = $this->number($this->digitCount); - $this->storeData($otpAdapter, hash($this->hashAlgorithm, $otp), $this->retry, $this->validUpto); + $this->storeData($otpAdapter, $this->hash($otp), $this->retry, $this->validUpto); return $otp; } @@ -79,10 +123,10 @@ public function generate(string $signature): string */ public function verify(string $signature, string $otp, bool $deleteIfFound = true): bool { - if (!preg_match('/^\d+$/', $otp) || strlen($otp) !== $this->digitCount) { + if (strlen($otp) !== $this->digitCount || !ctype_digit($otp)) { return false; } - $signature = 'ao-otp_' . hash('xxh3', $signature); + $signature = $this->cacheKey($signature); $cacheAdapter = $this->getCacheAdapter(); $otpAdapter = $cacheAdapter->getItem($signature); if (!$otpAdapter->isHit()) { @@ -96,21 +140,39 @@ public function verify(string $signature, string $otp, bool $deleteIfFound = tru || !is_int($payload['retry']) || !is_int($payload['expiresAt']) ) { + $cacheAdapter->deleteItem($signature); + return false; } $secret = $payload['secret']; $retry = $payload['retry']; $expiresAt = $payload['expiresAt']; - $isVerified = hash_equals($secret, hash($this->hashAlgorithm, $otp)); + $now = time(); + if ($expiresAt <= $now) { + $cacheAdapter->deleteItem($signature); + + return false; + } + + $isVerified = hash_equals($secret, $this->hash($otp)); match (true) { $deleteIfFound || $isVerified || $retry < 1 => $cacheAdapter->deleteItem($signature), - default => $this->storeData($otpAdapter, $secret, --$retry, $expiresAt - time()), + default => $this->storeData($otpAdapter, $secret, $retry - 1, $expiresAt - $now), }; return $isVerified; } + private function cacheKey(string $signature): string + { + if ($signature === '' || strlen($signature) > self::MAX_SIGNATURE_LENGTH) { + throw new NativeInvalidArgumentException('Generic OTP signatures must contain between 1 and 4096 bytes.'); + } + + return self::CACHE_KEY_PREFIX . hash('sha256', $signature); + } + /** * @throws Exception */ @@ -121,17 +183,36 @@ private function getCacheAdapter(): CacheItemPoolInterface ); } + private function hash(string $otp): string + { + if ($this->hashKey === null) { + return hash($this->hashAlgorithm, $otp); + } + + return hash_hmac($this->hashAlgorithm, $otp, $this->hashKey); + } + /** * Generate Secure random number of given length * * @param $length Number of digits to generate. + * * @throws Exception */ private function number(int $length): string { $number = ''; - for ($i = 0; $i < $length; $i++) { - $number .= (string) random_int(0, 9); + while (strlen($number) < $length) { + $bytes = random_bytes(max(1, $length - strlen($number))); + $byteCount = strlen($bytes); + for ($index = 0; $index < $byteCount; $index++) { + $value = ord($bytes[$index]); + if ($value >= 250) { + continue; + } + + $number .= chr(48 + ($value % 10)); + } } return $number; @@ -144,35 +225,23 @@ private function number(int $length): string * @param string $secret The secret. * @param int $retry The number of retries. * @param int $ttl The time to live in seconds. + * + * @throws Exception */ private function storeData(CacheItemInterface $otpAdapter, string $secret, int $retry, int $ttl): void { if ($ttl < 1) { return; } - $this->getCacheAdapter()->save( + $saved = $this->getCacheAdapter()->save( $otpAdapter->set([ 'secret' => $secret, 'retry' => $retry, 'expiresAt' => time() + $ttl, ])->expiresAfter($ttl), ); - } - - /** - * Validates the requirement for the PHP function. - * - * @throws Exception The number of digits must be between 4 and 10. - * @throws Exception The number of retries must be at least 0. - * @throws Exception Validity duration is invalid. - */ - private function validateRequirements(): void - { - match (true) { - $this->digitCount < 4 || $this->digitCount > 10 => throw new Exception('The number of digits must be between 4 and 10.'), - $this->retry < 0 => throw new Exception('The number of retries must be at least 0.'), - $this->validUpto < 1 => throw new Exception('Validity duration is invalid.'), - default => null, - }; + if (!$saved) { + throw new RuntimeException('Unable to persist generic OTP state.'); + } } } diff --git a/src/RecoveryCodes.php b/src/RecoveryCodes.php index bc64ba4..619705f 100644 --- a/src/RecoveryCodes.php +++ b/src/RecoveryCodes.php @@ -12,14 +12,33 @@ final readonly class RecoveryCodes { + private const int MAX_CODE_COUNT = 100; + + private const int MAX_CODE_LENGTH = 128; + + private string $hashAlgorithm; + + private ?string $hashKey; + public function __construct( private RecoveryCodeStoreInterface $store, - private string $hashAlgorithm = 'sha256', - private ?string $hashKey = null, - ) {} + string $hashAlgorithm = 'sha256', + ?string $hashKey = null, + ) { + $this->hashAlgorithm = match (strtolower(trim($hashAlgorithm))) { + 'sha256' => 'sha256', + 'sha512' => 'sha512', + default => throw new InvalidArgumentException('Recovery code hashing requires SHA-256 or SHA-512.'), + }; + if ($hashKey !== null && strlen($hashKey) < 16) { + throw new InvalidArgumentException('Recovery code HMAC keys must contain at least 16 bytes.'); + } + $this->hashKey = $hashKey; + } public function consume(string $binding, string $code): RecoveryCodeConsumptionResult { + self::assertBinding($binding); $usedAt = new DateTimeImmutable(); $normalizedCode = strtoupper(str_replace([' ', '-'], '', trim($code))); $consumed = $this->store->consume($binding, $this->hash($normalizedCode), $usedAt); @@ -41,7 +60,15 @@ public function generate( int $groupSize = 4, string $characterSet = 'ABCDEFGHJKLMNPQRSTUVWXYZ23456789', ): RecoveryCodeGenerationResult { - if ($count < 1 || $length < 6 || $groupSize < 0) { + self::assertBinding($binding); + if ( + $count < 1 + || $count > self::MAX_CODE_COUNT + || $length < 6 + || $length > self::MAX_CODE_LENGTH + || $groupSize < 0 + || $groupSize > $length + ) { throw new InvalidArgumentException('Invalid recovery code configuration.'); } @@ -71,18 +98,30 @@ public function generate( return new RecoveryCodeGenerationResult($plainCodes, $metadata['total'], $metadata['remaining'], $metadata['lastUsedAt']); } + private static function assertBinding(string $binding): void + { + if (trim($binding) === '' || strlen($binding) > 512) { + throw new InvalidArgumentException('Recovery code binding must contain between 1 and 512 bytes.'); + } + } + private function canGenerateUniqueCodes(int $count, int $length, int $characterCount): bool { + if ($characterCount < 2) { + return false; + } + + $requiredCapacity = $count * 2; $capacity = 1; for ($i = 0; $i < $length; $i++) { - if ($capacity >= $count || $capacity > intdiv($count - 1, $characterCount)) { + if ($capacity >= $requiredCapacity || $capacity > intdiv($requiredCapacity - 1, $characterCount)) { return true; } $capacity *= $characterCount; } - return $capacity >= $count; + return $capacity >= $requiredCapacity; } /** @@ -92,20 +131,26 @@ private function canGenerateUniqueCodes(int $count, int $length, int $characterC */ private function characterSet(string $characterSet): array { + $characterSet = strtoupper($characterSet); + if ($characterSet === '' || preg_match('/^[A-Z0-9]+$/', $characterSet) !== 1) { + throw new InvalidArgumentException('Recovery code character set must contain only ASCII letters and digits.'); + } + $characters = []; foreach (str_split($characterSet) as $character) { $characters[$character] = true; } - if ($characters === []) { - throw new InvalidArgumentException('Recovery code character set cannot be empty.'); - } return array_keys($characters); } private function hash(string $code): string { - return hash_hmac($this->hashAlgorithm, $code, $this->hashKey ?? 'otp-recovery-codes'); + if ($this->hashKey === null) { + return hash($this->hashAlgorithm, $code); + } + + return hash_hmac($this->hashAlgorithm, $code, $this->hashKey); } /** diff --git a/src/Stores/InMemoryReplayStore.php b/src/Stores/InMemoryReplayStore.php index d2c7ec8..c3cf8b7 100644 --- a/src/Stores/InMemoryReplayStore.php +++ b/src/Stores/InMemoryReplayStore.php @@ -4,9 +4,10 @@ namespace Infocyph\OTP\Stores; -use Infocyph\OTP\Contracts\ReplayStoreInterface; +use Infocyph\OTP\Contracts\AtomicReplayStoreInterface; +use InvalidArgumentException; -final class InMemoryReplayStore implements ReplayStoreInterface +final class InMemoryReplayStore implements AtomicReplayStoreInterface { /** * @var array>> @@ -14,13 +15,49 @@ final class InMemoryReplayStore implements ReplayStoreInterface private array $consumed = []; /** - * @var array> + * @var array> */ private array $state = []; + public function advance(string $namespace, string $binding, int $value, ?int $ttl = null): bool + { + self::assertTtl($ttl); + $current = $this->getState($namespace, $binding); + if (is_int($current) && $value <= $current) { + return false; + } + + $this->setState($namespace, $binding, $value, $ttl); + + return true; + } + + public function consumeOnce(string $namespace, string $binding, string $token, ?int $ttl = null): bool + { + self::assertTtl($ttl); + if ($this->hasConsumed($namespace, $binding, $token)) { + return false; + } + + $this->markConsumed($namespace, $binding, $token, $ttl); + + return true; + } + public function getState(string $namespace, string $binding): int|string|null { - return $this->state[$namespace][$binding] ?? null; + if (!isset($this->state[$namespace][$binding])) { + return null; + } + + $entry = $this->state[$namespace][$binding]; + if ($entry['expiresAt'] !== null && $entry['expiresAt'] <= time()) { + unset($this->state[$namespace][$binding]); + + return null; + } + + return $entry['value']; } public function hasConsumed(string $namespace, string $binding, string $token): bool @@ -41,12 +78,26 @@ public function hasConsumed(string $namespace, string $binding, string $token): public function markConsumed(string $namespace, string $binding, string $token, ?int $ttl = null): void { + self::assertTtl($ttl); $this->consumed[$namespace][$binding][$token] = $ttl !== null ? time() + $ttl : null; } - // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.FoundInImplementedInterfaceAfterLastUsed - public function setState(string $namespace, string $binding, int|string|null $value, ?int $_ttl = null): void + public function setState(string $namespace, string $binding, int|string|null $value, ?int $ttl = null): void + { + self::assertTtl($ttl); + $this->state[$namespace][$binding] = [ + 'value' => $value, + 'expiresAt' => $ttl !== null ? time() + $ttl : null, + ]; + } + + private static function assertTtl(?int $ttl): void { - $this->state[$namespace][$binding] = $value; + if ($ttl !== null && $ttl < 1) { + throw new InvalidArgumentException('Replay state TTL must be greater than zero.'); + } + if ($ttl !== null && time() > PHP_INT_MAX - $ttl) { + throw new InvalidArgumentException('Replay state TTL exceeds the supported timestamp range.'); + } } } diff --git a/src/Support/LabelHelper.php b/src/Support/LabelHelper.php index be79bda..0e93c05 100644 --- a/src/Support/LabelHelper.php +++ b/src/Support/LabelHelper.php @@ -8,12 +8,12 @@ final class LabelHelper { + private const int MAX_LABEL_LENGTH = 255; + public static function formatLabel(string $label, ?string $issuer = null): string { $label = trim($label); - if ($label === '') { - throw new InvalidArgumentException('Label cannot be empty.'); - } + self::assertText($label, 'Label'); if ($issuer === null || $issuer === '') { return $label; @@ -36,9 +36,7 @@ public static function formatLabel(string $label, ?string $issuer = null): strin public static function normalizeIssuer(string $issuer): string { $issuer = trim($issuer); - if ($issuer === '') { - throw new InvalidArgumentException('Issuer cannot be empty.'); - } + self::assertText($issuer, 'Issuer'); return preg_replace('/\s+/', ' ', $issuer) ?? $issuer; } @@ -51,22 +49,64 @@ public static function normalizeIssuer(string $issuer): string */ public static function parseLabel(string $label, ?string $issuer = null): array { + if (preg_match('/%(?![A-Fa-f0-9]{2})/', $label) === 1) { + throw new InvalidArgumentException('Provisioning label contains invalid percent encoding.'); + } + $decoded = rawurldecode($label); + self::assertText($decoded, 'Provisioning label'); $queryIssuer = $issuer !== null && $issuer !== '' ? self::normalizeIssuer($issuer) : null; if (str_contains($decoded, ':')) { - [$labelIssuer, $account] = explode(':', $decoded, 2); - $labelIssuer = trim($labelIssuer); + return self::parseIssuerLabel($decoded, $queryIssuer); + } - return [ - 'issuer' => $queryIssuer ?? ($labelIssuer !== '' ? $labelIssuer : null), - 'label' => trim($account), - ]; + $decoded = trim($decoded); + if ($decoded === '') { + throw new InvalidArgumentException('Provisioning account label cannot be empty.'); } return [ 'issuer' => $queryIssuer, - 'label' => trim($decoded), + 'label' => $decoded, + ]; + } + + private static function assertText(string $value, string $field): void + { + if ( + $value === '' + || strlen($value) > self::MAX_LABEL_LENGTH + || preg_match('/[\x00-\x1F\x7F]/', $value) === 1 + || preg_match('//u', $value) !== 1 + ) { + throw new InvalidArgumentException(sprintf('%s must contain between 1 and 255 valid UTF-8 bytes without control characters.', $field)); + } + } + + /** + * @param $decoded Decoded provisioning label. + * @param $queryIssuer Normalized query-string issuer. + * @return array Parsed issuer/label parts. + * @phpstan-return array{issuer:?string,label:string} + */ + private static function parseIssuerLabel(string $decoded, ?string $queryIssuer): array + { + [$labelIssuer, $account] = explode(':', $decoded, 2); + $labelIssuer = trim($labelIssuer); + $account = trim($account); + if ($account === '') { + throw new InvalidArgumentException('Provisioning account label cannot be empty.'); + } + + $labelIssuer = $labelIssuer !== '' ? self::normalizeIssuer($labelIssuer) : ''; + if ($labelIssuer !== '' && $queryIssuer !== null && $labelIssuer !== $queryIssuer) { + throw new InvalidArgumentException('Provisioning label issuer must match the issuer query parameter.'); + } + + return [ + 'issuer' => $queryIssuer ?? ($labelIssuer !== '' ? $labelIssuer : null), + 'label' => $account, ]; } } diff --git a/src/Support/OcraSuiteValidator.php b/src/Support/OcraSuiteValidator.php new file mode 100644 index 0000000..a5f9dd7 --- /dev/null +++ b/src/Support/OcraSuiteValidator.php @@ -0,0 +1,28 @@ +> 32) & 0xFFFFFFFF, $counter & 0xFFFFFFFF); - $hash = hash_hmac($algorithm, $binaryCounter, Base32::decodeUpper($secret), true); - $offset = ord(substr($hash, -1)) & 0x0F; + $hash = hash_hmac($algorithm, $binaryCounter, $binarySecret, true); + $offset = ord($hash[-1]) & 0x0F; $unpacked = unpack('Nvalue', substr($hash, $offset, 4)); if ($unpacked === false) { throw new InvalidArgumentException('Unable to unpack HOTP hash fragment.'); diff --git a/src/Support/ProvisioningUriBuilder.php b/src/Support/ProvisioningUriBuilder.php index 2798379..50dbf9d 100644 --- a/src/Support/ProvisioningUriBuilder.php +++ b/src/Support/ProvisioningUriBuilder.php @@ -5,6 +5,7 @@ namespace Infocyph\OTP\Support; use Infocyph\OTP\ValueObjects\EnrollmentPayload; +use InvalidArgumentException; final class ProvisioningUriBuilder { @@ -36,8 +37,18 @@ public static function build( ?int $counter = null, ?string $ocraSuite = null, ): string { + self::assertType($type); + self::assertAdditionalParameters($additionalParameters); + $algorithm = AlgorithmValidator::normalize($algorithm); + self::assertDigits($type, $digits); + $period = self::normalizePeriod($type, $period); + self::assertCounter($type, $counter); + self::assertOcraSuite($type, $ocraSuite); + $secret = SecretUtility::normalizeBase32($secret); + SecretUtility::decodeBase32($secret); + $query = [ - 'secret' => SecretUtility::normalizeBase32($secret), + 'secret' => $secret, 'issuer' => LabelHelper::normalizeIssuer($issuer), 'algorithm' => $include['algorithm'] ?? false ? strtoupper($algorithm) : null, 'digits' => $include['digits'] ?? false ? $digits : null, @@ -48,12 +59,17 @@ public static function build( $label = rawurlencode(LabelHelper::formatLabel($label, $issuer)); - return sprintf( + $uri = sprintf( 'otpauth://%s/%s?%s', $type, $label, http_build_query(array_filter($query, static fn($value) => $value !== null), '', '&', PHP_QUERY_RFC3986), ); + if (strlen($uri) > 4096) { + throw new InvalidArgumentException('Provisioning URI cannot exceed 4096 bytes.'); + } + + return $uri; } /** @@ -86,6 +102,10 @@ public static function enrollmentPayload( ?string $ocraSuite = null, ?string $qrSvg = null, ): EnrollmentPayload { + $secret = SecretUtility::normalizeBase32($secret); + SecretUtility::decodeBase32($secret); + $label = LabelHelper::formatLabel($label); + $issuer = LabelHelper::normalizeIssuer($issuer); $uri = self::build( $type, $secret, @@ -102,4 +122,93 @@ public static function enrollmentPayload( return new EnrollmentPayload($secret, $uri, $uri, $issuer, $label, $qrSvg); } + + /** + * @param $additionalParameters Additional query parameters. + * @phpstan-param array $additionalParameters + */ + private static function assertAdditionalParameters(array $additionalParameters): void + { + if (count($additionalParameters) > 24) { + throw new InvalidArgumentException('Provisioning URIs may contain at most 24 additional parameters.'); + } + + $reservedParameters = array_fill_keys( + ['secret', 'issuer', 'algorithm', 'digits', 'period', 'counter', 'ocraSuite'], + true, + ); + foreach ($additionalParameters as $key => $value) { + if ( + trim($key) === '' + || strlen($key) > 64 + || preg_match('/[\x00-\x1F\x7F]/', $key) === 1 + ) { + throw new InvalidArgumentException('Provisioning query parameter names must contain between 1 and 64 printable bytes.'); + } + if (isset($reservedParameters[$key])) { + throw new InvalidArgumentException(sprintf('Provisioning query parameter "%s" is reserved.', $key)); + } + if (is_string($value) && strlen($value) > 1024) { + throw new InvalidArgumentException('Provisioning query parameter values cannot exceed 1024 bytes.'); + } + } + } + + private static function assertCounter(string $type, ?int $counter): void + { + if ($type === 'hotp' && ($counter === null || $counter < 0)) { + throw new InvalidArgumentException('HOTP counter must be non-negative.'); + } + if ($type !== 'hotp' && $counter !== null) { + throw new InvalidArgumentException('Only HOTP provisioning may contain a counter.'); + } + } + + private static function assertDigits(string $type, int $digits): void + { + if (($type === 'hotp' || $type === 'totp') && ($digits < 4 || $digits > 10)) { + throw new InvalidArgumentException('HOTP and TOTP digit counts must be between 4 and 10.'); + } + if ($type === 'ocra' && $digits !== 0 && ($digits < 4 || $digits > 10)) { + throw new InvalidArgumentException('OCRA digit count must be zero or between 4 and 10.'); + } + } + + private static function assertOcraSuite(string $type, ?string $ocraSuite): void + { + if ($type === 'ocra' && ($ocraSuite === null || $ocraSuite === '')) { + throw new InvalidArgumentException('OCRA provisioning requires an OCRA suite.'); + } + if ($type === 'ocra' && !OcraSuiteValidator::isValid($ocraSuite)) { + throw new InvalidArgumentException('OCRA provisioning requires a valid OCRA suite.'); + } + if ($type !== 'ocra' && $ocraSuite !== null) { + throw new InvalidArgumentException('Only OCRA provisioning may contain an OCRA suite.'); + } + } + + private static function assertType(string $type): void + { + if (!in_array($type, ['hotp', 'totp', 'ocra'], true)) { + throw new InvalidArgumentException('Unsupported OTP provisioning type.'); + } + } + + private static function normalizePeriod(string $type, ?int $period): ?int + { + if ($type !== 'totp') { + if ($period !== null) { + throw new InvalidArgumentException('Only TOTP provisioning may contain a period.'); + } + + return null; + } + + $period ??= 30; + if ($period < 1 || $period > 86400) { + throw new InvalidArgumentException('TOTP period must be between 1 and 86400 seconds.'); + } + + return $period; + } } diff --git a/src/Support/ProvisioningUriParser.php b/src/Support/ProvisioningUriParser.php index bf334ae..6088665 100644 --- a/src/Support/ProvisioningUriParser.php +++ b/src/Support/ProvisioningUriParser.php @@ -9,32 +9,41 @@ final class ProvisioningUriParser { + private const int MAX_QUERY_PARAMETERS = 32; + + private const int MAX_URI_LENGTH = 4096; + public static function parse(string $uri): ParsedOtpAuthUri { - $parts = parse_url($uri); - if (!is_array($parts) || ($parts['scheme'] ?? null) !== 'otpauth') { - throw new InvalidArgumentException('Invalid otpauth URI.'); + if ($uri === '' || strlen($uri) > self::MAX_URI_LENGTH) { + throw new InvalidArgumentException('Invalid otpauth URI length.'); } - $type = strtolower($parts['host'] ?? ''); + $parts = self::parseUriParts($uri); + + $type = strtolower($parts['host']); if (!in_array($type, ['hotp', 'totp', 'ocra'], true)) { throw new InvalidArgumentException('Unsupported otpauth type.'); } - parse_str($parts['query'] ?? '', $query); + $query = self::parseQuery($parts['query']); $secret = SecretUtility::normalizeBase32(self::stringQueryValue($query, 'secret')); + SecretUtility::decodeBase32($secret); $issuerValue = self::optionalStringQueryValue($query, 'issuer'); $issuer = $issuerValue !== null ? LabelHelper::normalizeIssuer($issuerValue) : null; - $labelParts = LabelHelper::parseLabel(ltrim((string) ($parts['path'] ?? ''), '/'), $issuer); + $labelParts = LabelHelper::parseLabel(ltrim($parts['path'], '/'), $issuer); $algorithmValue = self::optionalStringQueryValue($query, 'algorithm'); $algorithm = $algorithmValue !== null ? AlgorithmValidator::normalize($algorithmValue) : 'sha1'; - $digits = self::optionalNonNegativeIntQueryValue($query, 'digits') ?? 6; + $digitsValue = self::optionalNonNegativeIntQueryValue($query, 'digits'); + $digits = $digitsValue ?? 6; if (($type === 'hotp' || $type === 'totp') && ($digits < 4 || $digits > 10)) { throw new InvalidArgumentException('HOTP and TOTP digit counts must be between 4 and 10.'); } $period = self::optionalPositiveIntQueryValue($query, 'period'); $counter = self::optionalNonNegativeIntQueryValue($query, 'counter'); + $ocraSuite = self::optionalStringQueryValue($query, 'ocraSuite'); + $digits = self::assertTypeParameters($type, $digits, $digitsValue !== null, $period, $counter, $ocraSuite); return new ParsedOtpAuthUri( $type, @@ -45,14 +54,90 @@ public static function parse(string $uri): ParsedOtpAuthUri $digits, $period, $counter, - self::optionalStringQueryValue($query, 'ocraSuite'), + $ocraSuite, ); } + private static function assertHotpParameters( + int $digits, + ?int $period, + ?int $counter, + ?string $ocraSuite, + ): int { + if ($counter === null) { + throw new InvalidArgumentException('HOTP provisioning URIs require a counter.'); + } + if ($period !== null) { + throw new InvalidArgumentException('Only TOTP provisioning URIs may contain a period.'); + } + if ($ocraSuite !== null) { + throw new InvalidArgumentException('Only OCRA provisioning URIs may contain an OCRA suite.'); + } + + return $digits; + } + + private static function assertOcraParameters( + int $digits, + bool $digitsProvided, + ?int $period, + ?int $counter, + ?string $ocraSuite, + ): int { + if ($counter !== null) { + throw new InvalidArgumentException('Only HOTP provisioning URIs may contain a counter.'); + } + if ($period !== null) { + throw new InvalidArgumentException('Only TOTP provisioning URIs may contain a period.'); + } + if ($ocraSuite === null || $ocraSuite === '') { + throw new InvalidArgumentException('OCRA provisioning URIs require an OCRA suite.'); + } + + if (!OcraSuiteValidator::isValid($ocraSuite)) { + throw new InvalidArgumentException('Invalid OCRA suite in provisioning URI.'); + } + + $suiteDigits = OcraSuiteValidator::digitCount($ocraSuite); + if ($digitsProvided && $digits !== $suiteDigits) { + throw new InvalidArgumentException('OCRA digit count must match the provisioning suite.'); + } + + return $suiteDigits; + } + + private static function assertTotpParameters(int $digits, ?int $counter, ?string $ocraSuite): int + { + if ($counter !== null) { + throw new InvalidArgumentException('Only HOTP provisioning URIs may contain a counter.'); + } + if ($ocraSuite !== null) { + throw new InvalidArgumentException('Only OCRA provisioning URIs may contain an OCRA suite.'); + } + + return $digits; + } + + private static function assertTypeParameters( + string $type, + int $digits, + bool $digitsProvided, + ?int $period, + ?int $counter, + ?string $ocraSuite, + ): int { + return match ($type) { + 'hotp' => self::assertHotpParameters($digits, $period, $counter, $ocraSuite), + 'totp' => self::assertTotpParameters($digits, $counter, $ocraSuite), + 'ocra' => self::assertOcraParameters($digits, $digitsProvided, $period, $counter, $ocraSuite), + default => throw new InvalidArgumentException('Unsupported otpauth type.'), + }; + } + /** * @param $query Parsed URI query values. * @param $key Query parameter name. - * @phpstan-param array $query + * @phpstan-param array $query */ private static function optionalNonNegativeIntQueryValue(array $query, string $key): ?int { @@ -63,6 +148,11 @@ private static function optionalNonNegativeIntQueryValue(array $query, string $k if (!ctype_digit($value)) { throw new InvalidArgumentException(sprintf('Invalid non-negative integer otpauth query parameter "%s".', $key)); } + $canonical = ltrim($value, '0'); + $canonical = $canonical === '' ? '0' : $canonical; + if ((string) (int) $canonical !== $canonical) { + throw new InvalidArgumentException(sprintf('Otpauth query parameter "%s" exceeds the supported integer range.', $key)); + } return (int) $value; } @@ -70,13 +160,13 @@ private static function optionalNonNegativeIntQueryValue(array $query, string $k /** * @param $query Parsed URI query values. * @param $key Query parameter name. - * @phpstan-param array $query + * @phpstan-param array $query */ private static function optionalPositiveIntQueryValue(array $query, string $key): ?int { $value = self::optionalNonNegativeIntQueryValue($query, $key); - if ($value !== null && $value < 1) { - throw new InvalidArgumentException(sprintf('Otpauth query parameter "%s" must be greater than zero.', $key)); + if ($value !== null && ($value < 1 || $value > 86400)) { + throw new InvalidArgumentException(sprintf('Otpauth query parameter "%s" must be between 1 and 86400.', $key)); } return $value; @@ -85,26 +175,99 @@ private static function optionalPositiveIntQueryValue(array $query, string $key) /** * @param $query Parsed URI query values. * @param $key Query parameter name. - * @phpstan-param array $query + * @phpstan-param array $query */ private static function optionalStringQueryValue(array $query, string $key): ?string { - if (!array_key_exists($key, $query)) { + if (!isset($query[$key])) { return null; } - $value = $query[$key]; - if (!is_string($value)) { - throw new InvalidArgumentException(sprintf('Invalid otpauth query parameter "%s".', $key)); + return $query[$key]; + } + + /** + * @param $queryString Encoded URI query. + * @return array Parsed query values. + * @phpstan-return array + */ + private static function parseQuery(string $queryString): array + { + if ($queryString === '') { + return []; + } + + $parameters = explode('&', $queryString); + if (count($parameters) > self::MAX_QUERY_PARAMETERS) { + throw new InvalidArgumentException('Too many otpauth query parameters.'); } - return $value; + $query = []; + foreach ($parameters as $parameter) { + if ($parameter === '') { + throw new InvalidArgumentException('Invalid empty otpauth query parameter.'); + } + + [$encodedKey, $encodedValue] = array_pad(explode('=', $parameter, 2), 2, ''); + if ( + $encodedKey === '' + || preg_match('/%(?![A-Fa-f0-9]{2})/', $encodedKey . $encodedValue) === 1 + ) { + throw new InvalidArgumentException('Invalid otpauth query encoding.'); + } + + $key = rawurldecode($encodedKey); + if (trim($key) === '' || preg_match('/[\x00-\x1F\x7F]/', $key) === 1) { + throw new InvalidArgumentException('Invalid otpauth query parameter name.'); + } + if (isset($query[$key])) { + throw new InvalidArgumentException(sprintf('Duplicate otpauth query parameter "%s".', $key)); + } + + $query[$key] = rawurldecode($encodedValue); + } + + return $query; + } + + /** + * @param $uri Provisioning URI. + * @return array Parsed URI components. + * @phpstan-return array{host:string,path:string,query:string} + */ + private static function parseUriParts(string $uri): array + { + $parts = parse_url($uri); + $scheme = is_array($parts) ? ($parts['scheme'] ?? null) : null; + $host = is_array($parts) ? ($parts['host'] ?? null) : null; + if ( + !is_array($parts) + || !is_string($scheme) + || strtolower($scheme) !== 'otpauth' + || !is_string($host) + || $host === '' + ) { + throw new InvalidArgumentException('Invalid otpauth URI.'); + } + if ( + isset($parts['user']) + || isset($parts['pass']) + || isset($parts['port']) + || isset($parts['fragment']) + ) { + throw new InvalidArgumentException('Invalid otpauth URI authority or fragment.'); + } + + $path = $parts['path'] ?? ''; + $query = $parts['query'] ?? ''; + + return ['host' => $host, 'path' => $path, 'query' => $query]; } /** * @param $query Parsed URI query values. * @param $key Query parameter name. - * @phpstan-param array $query + * @phpstan-param array $query */ private static function stringQueryValue(array $query, string $key): string { diff --git a/src/Support/ReplayProtection.php b/src/Support/ReplayProtection.php new file mode 100644 index 0000000..7d7ae4a --- /dev/null +++ b/src/Support/ReplayProtection.php @@ -0,0 +1,31 @@ +advance($namespace, $binding, $value); + } + + $current = $store->getState($namespace, $binding); + if (is_int($current) && $value <= $current) { + return false; + } + + $store->setState($namespace, $binding, $value); + + return true; + } +} diff --git a/src/Support/SecretRotationPlanner.php b/src/Support/SecretRotationPlanner.php new file mode 100644 index 0000000..c74455f --- /dev/null +++ b/src/Support/SecretRotationPlanner.php @@ -0,0 +1,49 @@ + PHP_INT_MAX - $gracePeriodInSeconds) { + throw new InvalidArgumentException('Grace period exceeds the supported timestamp range.'); + } + + $normalizedSecret = SecretUtility::normalizeBase32($newSecret); + if (hash_equals($currentSecret, $normalizedSecret)) { + throw new InvalidArgumentException('Replacement secret must differ from the current secret.'); + } + + return [ + 'nextSecret' => $normalizedSecret, + 'overlapUntil' => $gracePeriodInSeconds !== null + ? $rotationTimestamp + $gracePeriodInSeconds + : null, + ]; + } +} diff --git a/src/Support/SecretUtility.php b/src/Support/SecretUtility.php index 66500c5..34e9d1e 100644 --- a/src/Support/SecretUtility.php +++ b/src/Support/SecretUtility.php @@ -7,17 +7,35 @@ use Exception; use InvalidArgumentException; use ParagonIE\ConstantTime\Base32; +use Throwable; final class SecretUtility { + public static function decodeBase32(string $secret): string + { + $normalized = self::normalizeBase32($secret); + + try { + $decoded = Base32::decodeUpper($normalized); + } catch (Throwable $exception) { + throw new InvalidArgumentException('Secret must be a valid Base32 string.', previous: $exception); + } + + if (rtrim(Base32::encodeUpper($decoded), '=') !== $normalized) { + throw new InvalidArgumentException('Secret must use canonical Base32 encoding.'); + } + + return $decoded; + } + /** * @param $bytes Secret byte length. * @throws Exception */ public static function generate(int $bytes = 64): string { - if ($bytes < 10) { - throw new InvalidArgumentException('Secret byte length must be at least 10.'); + if ($bytes < 10 || $bytes > 1024) { + throw new InvalidArgumentException('Secret byte length must be between 10 and 1024.'); } return rtrim(Base32::encodeUpper(random_bytes($bytes)), '='); @@ -26,7 +44,7 @@ public static function generate(int $bytes = 64): string public static function isValidBase32(string $secret): bool { try { - self::normalizeBase32($secret); + self::decodeBase32($secret); return true; } catch (InvalidArgumentException) { @@ -44,6 +62,9 @@ public static function normalizeBase32(string $secret): string if (!preg_match('/^[A-Z2-7]+$/', $secret)) { throw new InvalidArgumentException('Secret must be a valid Base32 string.'); } + if (in_array(strlen($secret) % 8, [1, 3, 6], true)) { + throw new InvalidArgumentException('Secret must be a valid Base32 string.'); + } return $secret; } diff --git a/src/Support/SvgQrRenderer.php b/src/Support/SvgQrRenderer.php index 7dfe123..e54b57a 100644 --- a/src/Support/SvgQrRenderer.php +++ b/src/Support/SvgQrRenderer.php @@ -8,11 +8,19 @@ use BaconQrCode\Renderer\ImageRenderer; use BaconQrCode\Renderer\RendererStyle\RendererStyle; use BaconQrCode\Writer; +use InvalidArgumentException; final class SvgQrRenderer { public static function render(string $payload, int $imageSize = 200): string { + if ($payload === '' || strlen($payload) > 4096) { + throw new InvalidArgumentException('QR payload must contain between 1 and 4096 bytes.'); + } + if ($imageSize < 64 || $imageSize > 4096) { + throw new InvalidArgumentException('QR image size must be between 64 and 4096 pixels.'); + } + $writer = new Writer( new ImageRenderer( new RendererStyle($imageSize), diff --git a/src/TOTP.php b/src/TOTP.php index b106b86..d5ddb4c 100644 --- a/src/TOTP.php +++ b/src/TOTP.php @@ -5,10 +5,12 @@ namespace Infocyph\OTP; use Exception; +use Infocyph\OTP\Contracts\AtomicReplayStoreInterface; use Infocyph\OTP\Contracts\ReplayStoreInterface; use Infocyph\OTP\Result\VerificationResult; use Infocyph\OTP\Support\AlgorithmValidator; use Infocyph\OTP\Support\OtpMath; +use Infocyph\OTP\Support\SecretRotationPlanner; use Infocyph\OTP\Support\SecretUtility; use Infocyph\OTP\Support\SvgQrRenderer; use Infocyph\OTP\ValueObjects\EnrollmentPayload; @@ -17,6 +19,10 @@ final class TOTP extends AbstractOtpAuthenticator { + private const int MAX_PERIOD = 86400; + + private readonly string $binarySecret; + private readonly string $secret; private string $algorithm = 'sha1'; @@ -29,11 +35,12 @@ public function __construct( if ($digitCount < 4 || $digitCount > 10) { throw new \InvalidArgumentException('Digit count must be between 4 and 10.'); } - if ($period < 1) { - throw new \InvalidArgumentException('Period must be greater than zero.'); + if ($period < 1 || $period > self::MAX_PERIOD) { + throw new \InvalidArgumentException('Period must be between 1 and 86400 seconds.'); } $this->secret = SecretUtility::normalizeBase32($secret); + $this->binarySecret = SecretUtility::decodeBase32($this->secret); } /** @@ -89,8 +96,8 @@ public function getEnrollmentPayload( public function getOTP(?int $timestamp = null): string { - return OtpMath::hotp( - $this->secret, + return OtpMath::hotpFromBinary( + $this->binarySecret, $this->getTimeStepFromTimestamp($timestamp ?? time()), $this->digitCount, $this->algorithm, @@ -150,6 +157,9 @@ public function getProvisioningUriQR( public function getRemainingSeconds(?int $timestamp = null): int { $timestamp ??= time(); + if ($timestamp < 0) { + throw new \InvalidArgumentException('Timestamp must be non-negative.'); + } return $this->period - ($timestamp % $this->period); } @@ -211,16 +221,12 @@ public function rotateSecret( ?int $gracePeriodInSeconds = null, ?int $now = null, ): array { - if ($gracePeriodInSeconds !== null && $gracePeriodInSeconds < 0) { - throw new \InvalidArgumentException('Grace period must be non-negative.'); - } - - $now ??= time(); + $rotation = SecretRotationPlanner::prepare($this->secret, $newSecret, $gracePeriodInSeconds, $now); return [ 'current' => $this->secret, - 'next' => SecretUtility::normalizeBase32($newSecret), - 'overlapUntil' => $gracePeriodInSeconds !== null ? $now + $gracePeriodInSeconds : null, + 'next' => $rotation['nextSecret'], + 'overlapUntil' => $rotation['overlapUntil'], ]; } @@ -252,39 +258,95 @@ public function verifyWithWindow( ?string $binding = null, bool $singleUse = true, ): VerificationResult { - $this->assertOtp($otp, $this->digitCount); + if (!$this->isValidOtp($otp, $this->digitCount)) { + return new VerificationResult(false, 'malformed'); + } $window ??= new VerificationWindow(); + $this->assertReplayBinding($replayStore, $binding); $baseTimestamp = $timestamp ?? time(); $currentStep = $this->getTimeStepFromTimestamp($baseTimestamp); - for ($offset = -$window->past; $offset <= $window->future; $offset++) { - $matchedStep = $currentStep + $offset; - if ($matchedStep < 0) { - continue; - } + $match = $this->findMatch($otp, $currentStep, $window); + if ($match === null) { + return new VerificationResult(false, 'mismatch'); + } + + if ( + $replayStore !== null + && $binding !== null + && $singleUse + && $this->isReplay($replayStore, $binding, $match['step'], $window) + ) { + return new VerificationResult(false, 'replay', matchedTimestep: $match['step'], driftOffset: $match['offset'], replayDetected: true); + } - if (!hash_equals($otp, OtpMath::hotp($this->secret, $matchedStep, $this->digitCount, $this->algorithm))) { - continue; + return new VerificationResult( + true, + $match['offset'] === 0 ? 'matched' : 'drifted', + matchedTimestep: $match['step'], + driftOffset: $match['offset'], + verifiedAt: new \DateTimeImmutable(), + ); + } + + /** + * @param $otp Submitted OTP. + * @param $currentStep Current TOTP step. + * @param $window Allowed verification window. + * @return array|null Matched step and drift offset. + * @phpstan-return array{step:int,offset:int}|null + */ + private function findMatch(string $otp, int $currentStep, VerificationWindow $window): ?array + { + if ($this->matches($otp, $currentStep)) { + return ['step' => $currentStep, 'offset' => 0]; + } + + $maximumDrift = max($window->past, $window->future); + for ($distance = 1; $distance <= $maximumDrift; $distance++) { + $pastStep = $currentStep - $distance; + if ($distance <= $window->past && $pastStep >= 0 && $this->matches($otp, $pastStep)) { + return ['step' => $pastStep, 'offset' => -$distance]; } + if ($distance <= $window->future && $currentStep <= PHP_INT_MAX - $distance && $this->matches($otp, $currentStep + $distance)) { + return ['step' => $currentStep + $distance, 'offset' => $distance]; + } + } + + return null; + } - if ($replayStore !== null && $binding !== null && $singleUse) { - $token = (string) $matchedStep; - if ($replayStore->hasConsumed('totp:step', $binding, $token)) { - return new VerificationResult(false, 'replay', matchedTimestep: $matchedStep, driftOffset: $offset, replayDetected: true); - } - $replayStore->markConsumed('totp:step', $binding, $token, $this->period * max(1, $window->past + $window->future + 1)); - $replayStore->setState('totp:last_timestep', $binding, $matchedStep); + private function isReplay( + ReplayStoreInterface $replayStore, + string $binding, + int $matchedStep, + VerificationWindow $window, + ): bool { + $token = (string) $matchedStep; + $ttl = $this->period * ($window->past + $window->future + 1); + if ($replayStore instanceof AtomicReplayStoreInterface) { + $consumed = $replayStore->consumeOnce('totp:step', $binding, $token, $ttl); + } else { + $consumed = !$replayStore->hasConsumed('totp:step', $binding, $token); + if ($consumed) { + $replayStore->markConsumed('totp:step', $binding, $token, $ttl); } + } - return new VerificationResult( - true, - $offset === 0 ? 'matched' : 'drifted', - matchedTimestep: $matchedStep, - driftOffset: $offset, - verifiedAt: new \DateTimeImmutable(), - ); + if (!$consumed) { + return true; } - return new VerificationResult(false, 'mismatch'); + $replayStore->setState('totp:last_timestep', $binding, $matchedStep); + + return false; + } + + private function matches(string $otp, int $step): bool + { + return hash_equals( + OtpMath::hotpFromBinary($this->binarySecret, $step, $this->digitCount, $this->algorithm), + $otp, + ); } } diff --git a/src/ValueObjects/DeviceEnrollment.php b/src/ValueObjects/DeviceEnrollment.php index 8774449..38d74cb 100644 --- a/src/ValueObjects/DeviceEnrollment.php +++ b/src/ValueObjects/DeviceEnrollment.php @@ -20,6 +20,15 @@ public function __construct( self::assertNonEmpty('deviceId', $deviceId); self::assertNonEmpty('label', $label); self::assertNonEmpty('secretReference', $secretReference); + if ($activatedAt !== null && $activatedAt < $createdAt) { + throw new InvalidArgumentException('Enrollment activation cannot precede creation.'); + } + if ($revokedAt !== null && $revokedAt < $createdAt) { + throw new InvalidArgumentException('Enrollment revocation cannot precede creation.'); + } + if ($activatedAt !== null && $revokedAt !== null && $revokedAt < $activatedAt) { + throw new InvalidArgumentException('Enrollment revocation cannot precede activation.'); + } } public static function create( @@ -36,6 +45,9 @@ public function activate(?DateTimeImmutable $activatedAt = null): self if ($this->isRevoked()) { throw new InvalidArgumentException('Revoked enrollments cannot be activated.'); } + if ($this->isActive()) { + throw new InvalidArgumentException('Active enrollments cannot be activated again.'); + } return new self( $this->deviceId, @@ -78,6 +90,10 @@ public function rename(string $label): self public function revoke(?DateTimeImmutable $revokedAt = null): self { + if ($this->isRevoked()) { + throw new InvalidArgumentException('Revoked enrollments cannot be revoked again.'); + } + return new self( $this->deviceId, $this->label, diff --git a/src/ValueObjects/SecretRotation.php b/src/ValueObjects/SecretRotation.php index 33ace6e..210bff3 100644 --- a/src/ValueObjects/SecretRotation.php +++ b/src/ValueObjects/SecretRotation.php @@ -5,6 +5,7 @@ namespace Infocyph\OTP\ValueObjects; use DateTimeImmutable; +use InvalidArgumentException; final readonly class SecretRotation { @@ -13,7 +14,14 @@ public function __construct( public string $nextSecret, public ?DateTimeImmutable $overlapUntil = null, public ?EnrollmentPayload $nextEnrollment = null, - ) {} + ) { + if (trim($currentSecret) === '' || trim($nextSecret) === '') { + throw new InvalidArgumentException('Rotation secrets cannot be empty.'); + } + if (hash_equals($currentSecret, $nextSecret)) { + throw new InvalidArgumentException('Replacement secret must differ from the current secret.'); + } + } public function hasGracePeriod(): bool { @@ -26,7 +34,7 @@ public function isDualSecretActive(?DateTimeImmutable $at = null): bool return false; } - return ($at ?? new DateTimeImmutable()) <= $this->overlapUntil; + return ($at ?? new DateTimeImmutable()) < $this->overlapUntil; } public function requiresImmediateCutover(): bool diff --git a/src/ValueObjects/VerificationWindow.php b/src/ValueObjects/VerificationWindow.php index 064ebb2..4cd76f3 100644 --- a/src/ValueObjects/VerificationWindow.php +++ b/src/ValueObjects/VerificationWindow.php @@ -8,6 +8,8 @@ final readonly class VerificationWindow { + private const int MAX_TOTAL_WINDOWS = 100; + public function __construct( public int $past = 0, public int $future = 0, @@ -15,6 +17,9 @@ public function __construct( if ($past < 0 || $future < 0) { throw new InvalidArgumentException('Verification windows must be non-negative.'); } + if ($past > self::MAX_TOTAL_WINDOWS - $future) { + throw new InvalidArgumentException('Verification windows may include at most 100 drift steps.'); + } } public static function symmetric(int $window): self diff --git a/tests/AdvancedOTPTest.php b/tests/AdvancedOTPTest.php index f876cf7..e21ae19 100644 --- a/tests/AdvancedOTPTest.php +++ b/tests/AdvancedOTPTest.php @@ -1,10 +1,13 @@ new VerificationWindow(-1, 0)) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => new VerificationWindow(51, 50)) + ->toThrow(InvalidArgumentException::class); +}); + +test('HOTP and TOTP reject malformed codes without exception control flow', function () { + $secret = TOTP::generateSecret(); + $totp = new TOTP($secret); + $hotp = new HOTP($secret); + + expect($totp->verify('invalid'))->toBeFalse() + ->and($totp->verifyWithWindow('123')->reason)->toBe('malformed') + ->and($hotp->verify('invalid', 0))->toBeFalse() + ->and($hotp->verifyWithResult('123', 0)->reason)->toBe('malformed'); +}); + +test('verification work and replay configuration are bounded', function () { + $secret = TOTP::generateSecret(); + $totp = new TOTP($secret); + $hotp = new HOTP($secret); + $store = new InMemoryReplayStore(); + + expect(fn () => $hotp->verify(str_repeat('0', 6), 0, 101)) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => $totp->verifyWithWindow($totp->getOTP(), replayStore: $store)) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => new TOTP($secret, period: 86401)) + ->toThrow(InvalidArgumentException::class); +}); + +test('Base32 secrets must be decodable and canonical', function () { + expect(SecretUtility::isValidBase32('A'))->toBeFalse() + ->and(SecretUtility::isValidBase32('MZ'))->toBeFalse() + ->and(SecretUtility::isValidBase32('MY'))->toBeTrue(); +}); + +test('recovery code configuration is bounded and normalizes custom alphabets', function () { + $codes = new RecoveryCodes(new InMemoryRecoveryCodeStore(), hashKey: str_repeat('k', 32)); + $generated = $codes->generate('user-1', count: 2, length: 6, characterSet: 'ab'); + + expect($generated->plainCodes)->each->toMatch('/^[AB-]+$/') + ->and($codes->consume('user-1', strtolower($generated->plainCodes[0]))->consumed)->toBeTrue() + ->and(fn () => new RecoveryCodes(new InMemoryRecoveryCodeStore(), 'xxh128')) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => $codes->generate('user-1', count: 101)) + ->toThrow(InvalidArgumentException::class); +}); + +test('otpauth parser rejects ambiguous identities and duplicate parameters', function () { + $secret = 'JBSWY3DPEHPK3PXP'; + + expect(fn () => ProvisioningUriParser::parse( + 'otpauth://totp/IssuerA:user?secret=' . $secret . '&issuer=IssuerB', + ))->toThrow(InvalidArgumentException::class) + ->and(fn () => ProvisioningUriParser::parse( + 'otpauth://totp/Issuer:user?secret=' . $secret . '&secret=' . $secret, + ))->toThrow(InvalidArgumentException::class) + ->and(fn () => ProvisioningUriParser::parse( + 'otpauth://hotp/Issuer:user?secret=' . $secret, + ))->toThrow(InvalidArgumentException::class); +}); + +test('otpauth provisioning rejects type-conflicting and reserved parameters', function () { + $secret = TOTP::generateSecret(); + $totp = new TOTP($secret); + + expect(fn () => ProvisioningUriParser::parse( + 'otpauth://totp/Issuer:user?secret=' . $secret . '&counter=0', + ))->toThrow(InvalidArgumentException::class) + ->and(fn () => ProvisioningUriParser::parse( + 'otpauth://hotp/Issuer:user?secret=' . $secret . '&counter=0&period=30', + ))->toThrow(InvalidArgumentException::class) + ->and(fn () => ProvisioningUriParser::parse( + 'otpauth://ocra/Issuer:user?secret=' . $secret . '&ocraSuite=OCRA-1:HOTP-SHA256-8:QN08-invalid', + ))->toThrow(InvalidArgumentException::class) + ->and(fn () => $totp->getProvisioningUri( + 'user', + 'Issuer', + additionalParameters: ['secret' => $secret], + ))->toThrow(InvalidArgumentException::class); +}); + +test('atomic in-memory replay state advances monotonically and validates TTLs', function () { + $store = new InMemoryReplayStore(); + + expect($store->advance('hotp:last_counter', 'device-1', 5))->toBeTrue() + ->and($store->advance('hotp:last_counter', 'device-1', 5))->toBeFalse() + ->and($store->advance('hotp:last_counter', 'device-1', 4))->toBeFalse() + ->and($store->advance('hotp:last_counter', 'device-1', 6))->toBeTrue() + ->and(fn () => $store->markConsumed('totp:step', 'user-1', '1', 0)) ->toThrow(InvalidArgumentException::class); }); diff --git a/tests/ArchTest.php b/tests/ArchTest.php index 609b6b6..19fc508 100644 --- a/tests/ArchTest.php +++ b/tests/ArchTest.php @@ -1,5 +1,7 @@ each->not()->toBeUsed(); }); diff --git a/tests/GenericOTPTest.php b/tests/GenericOTPTest.php index 5ef6b7d..a3aae01 100644 --- a/tests/GenericOTPTest.php +++ b/tests/GenericOTPTest.php @@ -1,5 +1,7 @@ generate($signature); expect($otp)->toBeString()->toHaveLength(4); expect($otpInstance->verify($signature, $otp))->toBeTrue(); @@ -17,16 +19,16 @@ test('Duration', function () { $signature = random_bytes(3); $cachePool = new InMemoryCacheItemPool(); - $otpInstance = new OTP(4, 2, 3, 'xxh128', $cachePool); + $otpInstance = new OTP(4, 2, 3, 'sha256', $cachePool); $otp = $otpInstance->generate($signature); - $cachePool->expire('ao-otp_'.hash('xxh3', $signature)); + $cachePool->expire('ao-otp_'.hash('sha256', $signature)); expect($otpInstance->verify($signature, $otp))->toBeFalse(); $otpInstance->delete($signature); }); test('Retry with persistent key', function () { $signature = random_bytes(3); - $otpInstance = new OTP(4, 60, 2, 'xxh128', new InMemoryCacheItemPool()); + $otpInstance = new OTP(4, 60, 2, 'sha256', new InMemoryCacheItemPool()); $otp = $otpInstance->generate($signature); $invalidOtp = str_pad((string) ((((int) $otp) + 1) % 10000), 4, '0', STR_PAD_LEFT); expect($otpInstance->verify($signature, $invalidOtp, false))->toBeFalse(); @@ -36,7 +38,7 @@ test('Retry with non-persistent key (delete key if key name matches)', function () { $signature = random_bytes(3); - $otpInstance = new OTP(4, 60, 2, 'xxh128', new InMemoryCacheItemPool()); + $otpInstance = new OTP(4, 60, 2, 'sha256', new InMemoryCacheItemPool()); $otp = $otpInstance->generate($signature); $invalidOtp = str_pad((string) ((((int) $otp) + 1) % 10000), 4, '0', STR_PAD_LEFT); expect($otpInstance->verify($signature, $invalidOtp))->toBeFalse(); @@ -46,7 +48,7 @@ test('Delete', function () { $signature = random_bytes(3); - $otpInstance = new OTP(6, 30, 3, 'xxh128', new InMemoryCacheItemPool()); + $otpInstance = new OTP(6, 30, 3, 'sha256', new InMemoryCacheItemPool()); $otp = $otpInstance->generate($signature); $otpInstance->delete($signature); expect($otpInstance->verify($signature, $otp))->toBeFalse(); @@ -54,8 +56,45 @@ test('Flash', function () { $signature = random_bytes(3); - $otpInstance = new OTP(6, 30, 3, 'xxh128', new InMemoryCacheItemPool()); + $otpInstance = new OTP(6, 30, 3, 'sha256', new InMemoryCacheItemPool()); $otp = $otpInstance->generate($signature); $otpInstance->flush(); expect($otpInstance->verify($signature, $otp))->toBeFalse(); }); + +test('Configuration rejects unsafe hashes and invalid bounds', function () { + $cache = new InMemoryCacheItemPool(); + + expect(fn () => new OTP(hashAlgorithm: 'xxh128', cacheAdapter: $cache)) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => new OTP(digitCount: 3, cacheAdapter: $cache)) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => new OTP(validUpto: 0, cacheAdapter: $cache)) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => new OTP(validUpto: 86401, cacheAdapter: $cache)) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => new OTP(retry: 101, cacheAdapter: $cache)) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => new OTP(cacheAdapter: $cache, hashKey: 'short')) + ->toThrow(InvalidArgumentException::class); +}); + +test('Generic OTP supports a purpose-specific HMAC key', function () { + $signature = random_bytes(16); + $otp = new OTP( + cacheAdapter: new InMemoryCacheItemPool(), + hashKey: str_repeat('k', 32), + ); + $code = $otp->generate($signature); + + expect($otp->verify($signature, $code))->toBeTrue(); +}); + +test('Malformed generic OTP values fail without consuming valid state', function () { + $signature = random_bytes(3); + $otpInstance = new OTP(cacheAdapter: new InMemoryCacheItemPool()); + $otp = $otpInstance->generate($signature); + + expect($otpInstance->verify($signature, 'invalid'))->toBeFalse() + ->and($otpInstance->verify($signature, $otp))->toBeTrue(); +}); diff --git a/tests/HOTPTest.php b/tests/HOTPTest.php index d0f37b9..3220593 100644 --- a/tests/HOTPTest.php +++ b/tests/HOTPTest.php @@ -1,5 +1,7 @@ setPin('1234'); $store = new InMemoryReplayStore(); $otp = $ocra->generate('12345678', 4); + $differentChallengeOtp = $ocra->generate('87654321', 4); $first = $ocra->verifyWithResult($otp, '12345678', 4, $store, 'user-42'); $second = $ocra->verifyWithResult($otp, '12345678', 4, $store, 'user-42'); + $counterReplay = $ocra->verifyWithResult($differentChallengeOtp, '87654321', 4, $store, 'user-42'); expect($first->matched)->toBeTrue() ->and($second->matched)->toBeFalse() - ->and($second->replayDetected)->toBeTrue(); + ->and($second->replayDetected)->toBeTrue() + ->and($counterReplay->matched)->toBeFalse() + ->and($counterReplay->replayDetected)->toBeTrue(); }); test('OCRA exposes parsed suite details', function () { @@ -180,3 +187,39 @@ ->and(fn () => $alpha->generate(str_repeat('A', 129)))->toThrow(OCRAException::class) ->and(fn () => $hex->generate('XYZ'))->toThrow(OCRAException::class); }); + +test('OCRA handles numeric challenges beyond the platform integer range', function () { + $ocra = new OCRA('OCRA-1:HOTP-SHA256-8:QN64', KEY_32); + $first = $ocra->generate(str_repeat('8', 64)); + $second = $ocra->generate(str_repeat('9', 64)); + + expect($first)->toHaveLength(8) + ->and($second)->toHaveLength(8) + ->and($first)->not->toBe($second); +}); + +test('OCRA Base32 construction and provisioning use the same key bytes', function () { + $secret = OCRA::generateSecret(32); + $fromBase32 = OCRA::fromBase32('OCRA-1:HOTP-SHA256-8:QN08', $secret); + $fromRaw = new OCRA( + 'OCRA-1:HOTP-SHA256-8:QN08', + SecretUtility::decodeBase32($secret), + ); + $uri = $fromBase32->getProvisioningUri('user@example.com', 'Example'); + $parsed = OCRA::parseProvisioningUri($uri); + + expect($fromBase32->generate('12345678'))->toBe($fromRaw->generate('12345678')) + ->and($parsed->secret)->toBe($secret); +}); + +test('OCRA validates optional and replay inputs at the boundary', function () { + $ocra = new OCRA('OCRA-1:HOTP-SHA256-8:QN08-S064', KEY_32); + $store = new InMemoryReplayStore(); + + expect(fn () => $ocra->setSession('not-hex')) + ->toThrow(OCRAException::class) + ->and(fn () => $ocra->verifyWithResult('00000000', '12345678', replayStore: $store)) + ->toThrow(OCRAException::class) + ->and(fn () => new OCRA('OCRA-1:HOTP-SHA256-8:QN08', 'short')) + ->toThrow(OCRAException::class); +}); diff --git a/tests/Support/InMemoryCacheItemPool.php b/tests/Support/InMemoryCacheItemPool.php index 9744281..4faeb17 100644 --- a/tests/Support/InMemoryCacheItemPool.php +++ b/tests/Support/InMemoryCacheItemPool.php @@ -1,5 +1,7 @@ activate(new \DateTimeImmutable('2026-04-20 10:00:00')); $renamed = $active->rename('Primary phone'); @@ -85,3 +92,53 @@ ->and($ocraRotation->hasGracePeriod())->toBeTrue() ->and($ocraRotation->nextEnrollment?->uri)->toContain('ocraSuite='); }); + +test('device enrollment enforces temporal state invariants', function () { + $createdAt = new \DateTimeImmutable('2026-04-20 10:00:00'); + + expect(fn () => new DeviceEnrollment( + 'device-1', + 'Phone', + 'secret-ref', + $createdAt, + new \DateTimeImmutable('2026-04-20 09:00:00'), + ))->toThrow(InvalidArgumentException::class); + + $active = DeviceEnrollment::create('device-1', 'Phone', 'secret-ref', $createdAt) + ->activate(new \DateTimeImmutable('2026-04-20 11:00:00')); + + expect(fn () => $active->activate()) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => $active->revoke(new \DateTimeImmutable('2026-04-20 10:30:00'))) + ->toThrow(InvalidArgumentException::class); + + $revoked = $active->revoke(new \DateTimeImmutable('2026-04-20 12:00:00')); + expect(fn () => $revoked->revoke()) + ->toThrow(InvalidArgumentException::class); +}); + +test('secret rotation grace period ends at its expiration instant', function () { + $totp = new TOTP(TOTP::generateSecret()); + $rotation = $totp->planSecretRotation( + TOTP::generateSecret(), + 'alice@example.com', + 'Example', + gracePeriodInSeconds: 60, + now: 1000, + ); + + expect($rotation->isDualSecretActive(new \DateTimeImmutable('@1059')))->toBeTrue() + ->and($rotation->isDualSecretActive(new \DateTimeImmutable('@1060')))->toBeFalse(); +}); + +test('time and rotation helpers reject negative timestamps and unchanged secrets', function () { + $secret = TOTP::generateSecret(); + $totp = new TOTP($secret); + + expect(fn () => $totp->getRemainingSeconds(-1)) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => $totp->rotateSecret(TOTP::generateSecret(), now: -1)) + ->toThrow(InvalidArgumentException::class) + ->and(fn () => $totp->rotateSecret($secret)) + ->toThrow(InvalidArgumentException::class); +});