Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 64 additions & 2 deletions src/main/java/com/eatthepath/otp/HmacOneTimePasswordGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ public HmacOneTimePasswordGenerator(final int passwordLength) {
}

/**
* <p>Creates a new HMAC-based one-time password generator using the given password length and algorithm. Note that
* Creates a new HMAC-based one-time password generator using the given password length and algorithm. Note that
* <a href="https://tools.ietf.org/html/rfc4226">RFC&nbsp;4226</a> specifies that HOTP must always use HMAC-SHA1 as
* an algorithm, but derived one-time password systems like TOTP may allow for other algorithms.</p>
* an algorithm, but derived one-time password systems like TOTP may allow for other algorithms.
*
* @param passwordLength the length, in decimal digits, of the one-time passwords to be generated; must be between
* 6 and 8, inclusive
Expand Down Expand Up @@ -199,6 +199,68 @@ public String generateOneTimePasswordString(final Key key, final long counter, f
return this.formatOneTimePassword(generateOneTimePassword(key, counter), locale);
}

/**
* Checks whether a given one-time password matches the one-time password generated for the given key and counter
* value. Note that this method simply checks equality of two one-time passwords; incrementing expected counter
* values, throttling/rate-limiting, counter resynchronization, and so one are all beyond the scope of this method.
*
* @param key the key to be used to generate the password
* @param counter the counter value for which to generate the password
* @param oneTimePassword the user-provided one-time password to check against the generated one-time password
*
* @return {@code true} if and only if the given one-time password matches the one-time password generated for the
* given key and counter value; one-time password strings match if they have the correct number of digits (see
* {@link #getPasswordLength()}), can be parsed as an integer, and that integer matches the one-time password
* generated for the given key and counter value
*
* @throws InvalidKeyException if the given key is inappropriate for initializing the {@link Mac} for this generator
* @throws NullPointerException if the given one-time password is {@code null}
*
* @see <a href="https://datatracker.ietf.org/doc/html/rfc4226#section-7">HOTP: An HMAC-Based One-Time Password Algorithm (RFC 4226) - Security Requirements</a>
*/
public boolean validateOneTimePassword(final Key key, final long counter, final String oneTimePassword) throws InvalidKeyException {
if (oneTimePassword == null) {
throw new NullPointerException("One-time password must not be null");
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're explicitly checking for null here, we should probably do it everywhere.

Copy link
Copy Markdown
Owner Author

@jchambers jchambers May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, broadly, there are two strategies we could use for checking that the provided one-time password matches the expected password:

We can do what we're doing here where we parse the given one-time password as an integer, then compare it to the integer value of the expected password
We can generate the expected one-time password as a string, then do a direct string comparison

The integer comparison approach is locale-agnostic (Integer.parseInt can parse numbers from any character set), but a little more complicated. Its execution time is also "constant-time" in the sense that none of its execution time is dependent upon any property of the expected one-time password.

EDIT: Actually, on a second read, this isn't quite true—if we bail out early with the length check, an attacker can learn the expected length of the password by observing execution time. I'll come back and iterate on this, though reasonable minds might disagree about whether the expected password length is actually a secret. (EDIT EDIT: it's constant-time now!)

The string comparison approach is much simpler and, assuming we use something like MessageDigest#isEqual, also constant-time:

return MessageDigest.isEqual(oneTimePassword.getBytes(StandardCharsets.UTF_8),
    generateOneTimePasswordString(key, counter, locale).getBytes(StandardCharsets.UTF_8));

The problem (or maybe advantage, depending on who you ask?) is that string comparison is locale-sensitive. In other words, even though "003784" and "००३७८४" represent the same number in their respective locales, they're clearly not the same string, and a caller would have to specify which locale they're expecting for validation to pass.

I think I like the locale-agnostic approach, but am very open to feedback and suggestions!

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're explicitly checking for null here, we should probably do it everywhere.

Actually, existing coverage here is solid. The main gap I was noticing was passing null for keys, but those turn into InvalidKeyExceptions, which makes sense to me.

}

// We COULD return early if the length doesn't match, but that could allow an attacker to learn the expected
// passowrd length by observing execution time. Arguably, the expected password length isn't a secret, but we
// can avoid revealing it here and choose to do so.
final boolean lengthMatches = oneTimePassword.length() == this.passwordLength;

try {
final boolean passwordMatches = validateOneTimePassword(key, counter, Integer.parseInt(oneTimePassword));

// Again, this construction may seem a little odd, but the goal is to make sure this check happens in
// constant time relative to any secret data or internal state. `&` is a constant-time operation while `&&`
// can short-circuit. This construction means we evaluate both criteria and don't return early if the length
// of the given one-time password was incorrect.
return lengthMatches & passwordMatches;
} catch (final NumberFormatException e) {
return false;
}
}

/**
* Checks whether a given one-time password matches the one-time password generated for the given key and counter
* value. Note that this method simply checks equality of two one-time passwords; incrementing expected counter
* values, throttling/rate-limiting, counter resynchronization, and so one are all beyond the scope of this method.
*
* @param key the key to be used to generate the password
* @param counter the counter value for which to generate the password
* @param oneTimePassword the user-provided one-time password to check against the generated one-time password
*
* @return {@code true} if and only if the given one-time password matches the one-time password generated for the
* given key and counter value
*
* @throws InvalidKeyException if the given key is inappropriate for initializing the {@link Mac} for this generator
*
* @see <a href="https://datatracker.ietf.org/doc/html/rfc4226#section-7">HOTP: An HMAC-Based One-Time Password Algorithm (RFC 4226) - Security Requirements</a>
*/
public boolean validateOneTimePassword(final Key key, final long counter, final int oneTimePassword) throws InvalidKeyException {
return generateOneTimePassword(key, counter) == oneTimePassword;
}

/**
* Formats a one-time password as a fixed-length string using the given locale.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public TimeBasedOneTimePasswordGenerator(final Duration timeStep, final int pass
* @throws InvalidKeyException if the given key is inappropriate for initializing the {@link Mac} for this generator
*/
public int generateOneTimePassword(final Key key, final Instant timestamp) throws InvalidKeyException {
return this.hotp.generateOneTimePassword(key, timestamp.toEpochMilli() / this.timeStep.toMillis());
return this.hotp.generateOneTimePassword(key, getCounterValue(timestamp));
}

/**
Expand Down Expand Up @@ -174,6 +174,50 @@ public String generateOneTimePasswordString(final Key key, final Instant timesta
return this.hotp.formatOneTimePassword(this.generateOneTimePassword(key, timestamp), locale);
}

/**
* Checks whether a given one-time password matches the one-time password generated for the given key and timestamp.
* Note that this method simply checks equality of two one-time passwords; compensating for clock drift,
* throttling/rate-limiting, clock resynchronization, and so one are all beyond the scope of this method.
*
* @param key the key to be used to generate the password
* @param timestamp the timestamp for which to generate the password
* @param oneTimePassword the user-provided one-time password to check against the generated one-time password
*
* @return {@code true} if and only if the given one-time password matches the one-time password generated for the
* given key and timestamp; one-time password strings match if they have the correct number of digits (see
* {@link #getPasswordLength()}), can be parsed as an integer, and that integer matches the one-time password
* generated for the given key and timestamp
*
* @throws InvalidKeyException if the given key is inappropriate for initializing the {@link Mac} for this generator
* @throws NullPointerException if the given timestamp or one-time password is {@code null}
*
* @see <a href="https://datatracker.ietf.org/doc/html/rfc6238#section-5">TOTP: Time-Based One-Time Password Algorithm (RFC 6238) - Security Considerations</a>
*/
public boolean validateOneTimePassword(final Key key, final Instant timestamp, final String oneTimePassword) throws InvalidKeyException {
return hotp.validateOneTimePassword(key, getCounterValue(timestamp), oneTimePassword);
}

/**
* Checks whether a given one-time password matches the one-time password generated for the given key and timestamp.
* Note that this method simply checks equality of two one-time passwords; compensating for clock drift,
* throttling/rate-limiting, clock resynchronization, and so one are all beyond the scope of this method.
*
* @param key the key to be used to generate the password
* @param timestamp the timestamp for which to generate the password
* @param oneTimePassword the user-provided one-time password to check against the generated one-time password
*
* @return {@code true} if and only if the given one-time password matches the one-time password generated for the
* given key and timestamp
*
* @throws InvalidKeyException if the given key is inappropriate for initializing the {@link Mac} for this generator
* @throws NullPointerException if the given timestamp is {@code null}
*
* @see <a href="https://datatracker.ietf.org/doc/html/rfc6238#section-5">TOTP: Time-Based One-Time Password Algorithm (RFC 6238) - Security Considerations</a>
*/
public boolean validateOneTimePassword(final Key key, final Instant timestamp, final int oneTimePassword) throws InvalidKeyException {
return hotp.validateOneTimePassword(key, getCounterValue(timestamp), oneTimePassword);
}

/**
* Returns the time step used by this generator.
*
Expand All @@ -200,4 +244,12 @@ public int getPasswordLength() {
public String getAlgorithm() {
return this.hotp.getAlgorithm();
}

private long getCounterValue(final Instant timestamp) {
if (timestamp == null) {
throw new NullPointerException("Timestamp must not be null");
}

return timestamp.toEpochMilli() / this.timeStep.toMillis();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.nio.charset.StandardCharsets;
import java.security.InvalidKeyException;
import java.security.Key;
import java.time.Instant;
import java.util.Arrays;
import java.util.Locale;
import java.util.concurrent.*;
Expand Down Expand Up @@ -147,6 +148,39 @@ private static Stream<Arguments> generateOneTimePasswordStringLocale() {
);
}

@Test
void validateOneTimePasswordInt() throws InvalidKeyException {
final HmacOneTimePasswordGenerator hotp = new HmacOneTimePasswordGenerator();
final long counter = ThreadLocalRandom.current().nextLong();

assertTrue(hotp.validateOneTimePassword(HOTP_KEY, counter, hotp.generateOneTimePassword(HOTP_KEY, counter)));
assertFalse(hotp.validateOneTimePassword(HOTP_KEY, counter, hotp.generateOneTimePassword(HOTP_KEY, counter + 1)));
}

@Test
void validateOneTimePasswordString() throws InvalidKeyException {
final HmacOneTimePasswordGenerator hotp = new HmacOneTimePasswordGenerator();

// A counter value of 36 with HOTP produces a one-time password of "003784" (or "००३७८४" in the hi-IN-u-nu-Deva
// locale). The leading zeros are an important edge case for string-based tests.
final long counter = 36;

assertTrue(hotp.validateOneTimePassword(HOTP_KEY, counter, "003784"));
assertFalse(hotp.validateOneTimePassword(HOTP_KEY, counter, "3784"));
assertFalse(hotp.validateOneTimePassword(HOTP_KEY, counter, "0003784"));
assertFalse(hotp.validateOneTimePassword(HOTP_KEY, counter, "0037840"));

assertTrue(hotp.validateOneTimePassword(HOTP_KEY, counter, "००३७८४"));
assertFalse(hotp.validateOneTimePassword(HOTP_KEY, counter, "३७८४"));
assertFalse(hotp.validateOneTimePassword(HOTP_KEY, counter, "०००३७८४"));
assertFalse(hotp.validateOneTimePassword(HOTP_KEY, counter, "००३७८४०"));

assertFalse(hotp.validateOneTimePassword(HOTP_KEY, counter, "cursed"));

assertThrows(NullPointerException.class, () ->
hotp.validateOneTimePassword(HOTP_KEY, counter, null));
}

@Test
void generateOneTimePasswordConcurrent() throws InterruptedException {
final int iterations = 10_000;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
import java.nio.charset.StandardCharsets;
import java.security.InvalidKeyException;
import java.security.Key;
import java.security.NoSuchAlgorithmException;
import java.time.Duration;
import java.time.Instant;
import java.util.Locale;
import java.util.stream.Stream;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
import static org.junit.jupiter.params.provider.Arguments.arguments;

Expand Down Expand Up @@ -200,6 +200,50 @@ private static Stream<Arguments> generateOneTimePasswordStringLocale() {
);
}

@Test
void validateOneTimePasswordInt() throws InvalidKeyException {
final TimeBasedOneTimePasswordGenerator totp = new TimeBasedOneTimePasswordGenerator();
final Instant timestamp = Instant.now();
final Key key =
new SecretKeySpec(HMAC_SHA1_KEY_BYTES, TimeBasedOneTimePasswordGenerator.TOTP_ALGORITHM_HMAC_SHA1);

assertTrue(totp.validateOneTimePassword(key, timestamp, totp.generateOneTimePassword(key, timestamp)));
assertFalse(totp.validateOneTimePassword(key, timestamp, totp.generateOneTimePassword(key, timestamp.plus(totp.getTimeStep()))));

assertThrows(NullPointerException.class, () ->
totp.validateOneTimePassword(key, null, totp.generateOneTimePassword(key, timestamp)));
}

@Test
void validateOneTimePasswordString() throws InvalidKeyException {
final TimeBasedOneTimePasswordGenerator totp = new TimeBasedOneTimePasswordGenerator();
final Key key =
new SecretKeySpec(HMAC_SHA1_KEY_BYTES, TimeBasedOneTimePasswordGenerator.TOTP_ALGORITHM_HMAC_SHA1);

// A timestamp of 1970-01-01T00:18:00Z with a default TOTP generator produces a one-time password of "003784"
// (or "००३७८४" in the hi-IN-u-nu-Deva locale). The leading zeros are an important edge case for string-based
// tests.
final Instant timestamp = Instant.parse("1970-01-01T00:18:00Z");

assertTrue(totp.validateOneTimePassword(key, timestamp, "003784"));
assertFalse(totp.validateOneTimePassword(key, timestamp, "3784"));
assertFalse(totp.validateOneTimePassword(key, timestamp, "0003784"));
assertFalse(totp.validateOneTimePassword(key, timestamp, "0037840"));

assertTrue(totp.validateOneTimePassword(key, timestamp, "००३७८४"));
assertFalse(totp.validateOneTimePassword(key, timestamp, "३७८४"));
assertFalse(totp.validateOneTimePassword(key, timestamp, "०००३७८४"));
assertFalse(totp.validateOneTimePassword(key, timestamp, "००३७८४०"));

assertFalse(totp.validateOneTimePassword(key, timestamp, "cursed"));

assertThrows(NullPointerException.class, () ->
totp.validateOneTimePassword(key, timestamp, null));

assertThrows(NullPointerException.class, () ->
totp.validateOneTimePassword(key, null, "003784"));
}

private static void assumeAlgorithmSupported(final String algorithm) {
boolean algorithmSupported = true;

Expand Down