Simplify and correct some challenge TTL lockout code
0673e79d6d96
Actions

Description

Simplify and correct some challenge TTL lockout code

Summary:
Depends on D19889. Ref T13222. Some of this logic is either not-quite-right or a little more complicated than it needs to be.

Currently, we TTL TOTP challenges after three timesteps -- once the current code could no longer be used. But we actually have to TTL it after five timesteps -- once the most-future acceptable code could no longer be used. Otherwise, you can enter the most-future code now (perhaps the attacker compromises NTP and skews the server clock back by 75 seconds) and then an attacker can re-use it in three timesteps.

Generally, simplify things a bit and trust TTLs more. This also makes the "wait" dialog friendlier since we can give users an exact number of seconds.

The overall behavior here is still a little odd because we don't actually require you to respond to the challenge you were issued (right now, we check that the response is valid whenever you submit it, not that it's a valid response to the challenge we issued), but that will change in a future diff. This is just moving us generally in the right direction, and doesn't yet lock everything down properly.

Test Plan:

Added a little snippet to the control caption to list all the valid codes to make this easier:

$key = new PhutilOpaqueEnvelope($config->getFactorSecret());
$valid = array();
foreach ($this->getAllowedTimesteps() as $step) {
  $valid[] = self::getTOTPCode($key, $step);
}

$control->setCaption(
  pht(
    'Valid Codes: '.implode(', ', $valid)));