feat: Round out RFC 6238 support and validate time inputs - #26
Merged
Conversation
generateByTime and generateByTimeWindow now reject a non-positive $window (previously an uncaught DivisionByZeroError) and a negative $timestamp (previously silently produced an unintended counter), throwing InvalidArgumentException instead.
generateByCounter, generateByTime and generateByTimeWindow now accept an optional $algorithm (default sha1), validated against hash_hmac_algos(). HOTPResult::toDec previously hard-coded the dynamic-truncation offset byte to index 19, the final byte of a SHA-1 digest, which truncated wider SHA-256/512 digests at the wrong position. It now derives the offset from the digest's actual final byte per RFC 4226. Verified against the RFC 6238 Appendix B SHA-1, SHA-256 and SHA-512 vectors.
generateByTime and generateByTimeWindow accept an optional $startTime (RFC 6238 "T0", default 0) that is subtracted from the timestamp before the time step is computed, per RFC 6238 section 4.1. Documents the 2.2.0 release in the changelog.
A $startTime greater than $timestamp yields a negative effective time step and an unintended counter. generateByTime and generateByTimeWindow now reject it with InvalidArgumentException.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This brings the time-based methods (and
generateByCounter) closer to a full RFC 6238 implementation and tightens up how we handle bad input. Four small, self-contained commits, all backwards compatible. Every new parameter is optional with a default, so existing callers don't change.What's in here
Input validation. We were feeding whatever came in straight into
intval($timestamp / $window). A$windowof0threw an uncaughtDivisionByZeroError, and a negative$windowor$timestampquietly produced a nonsense counter. Both now throw anInvalidArgumentExceptionup front.Configurable HMAC algorithm.
generateByCounter/generateByTime/generateByTimeWindowtake an optional$algorithm(defaults tosha1), so you can generate HMAC-SHA-256 and HMAC-SHA-512 TOTPs. RFC 6238 allows these, and plenty of authenticators use them.Truncation fix for wider digests.
HOTPResulthard-coded the RFC 4226 dynamic-truncation offset to byte 19, the last byte of a SHA-1 digest. For SHA-256/512 that byte sits further along, so those digests were getting truncated at the wrong spot. We now read it from the digest's actual last byte. SHA-1 is unaffected.T0 support. The time methods take an optional
$startTime(RFC 6238's "T0", defaults to0) to shift where time-step counting starts. There's a guard so a$startTimepast the timestamp can't produce a negative step.Testing
Added coverage for all of it, including the RFC 6238 Appendix B vectors for SHA-1, SHA-256, and SHA-512. The per-algorithm keys come from the Appendix A reference code. Full suite is green at 33 tests, and
php-cs-fixeris clean.