Public Followup for 8.x-1.3 security release.

120 day mark reached,
Adding tests in new MR as they are necessary to reduce regression risk.
Starting to open followups and finalizing final writeup.

On July 24th it was reported that TFA was vulnerable to multiple replay attack vectors which should be prevented. These vulnerability consisted of flaws both in the root API.

The first vulnerability was the alreadyAcceptedCode() method used by plugins to validate a one time use token/code has not been used in in the past was determined to be subject to a possible replay attack if the site hash_salt is rotated.

This vulnerability may, be considered a failure to meet NIST SP.800-63 5.2.8 (replay resistance) or PCI/DSS 4.0 8.5.1 (The MFA system is not susceptible to replay attacks) among other technical standards.

Obtaining a first factor is generally seen as a simple/trivial task (readily available database may be obtained that could contain this information) leaving the obtaining of a previously valid second factor to pose the higher workload. Examples of how a previously valid second factors could be obtained by an attacker range from as simple as looking over the shoulder of a victim to sophisticated out of band captures.

The site hash_salt is encouraged to be rotated in the case of a suspected breach of a site meaning that when a second factor token should provide its best replay resistant is when it was unable to do so.

A social engineering attack may be sufficient to cause a site to rotate the hash_salt.

Generally the more significant the site protected by TFA the more likely to an attacker is to use more complicated attack vectors.

The Drupal Security Team determined that they had never published a vulnerability that required “this much compromise” and as such declined a maintainer requests that previous releases be marked insecure, that the 8.x-3.3 release be tagged as as security release, and that an Security Advisory be published.

The included HOTP and TOTP plugins both utilized the vulnerable API, however the HOTP module included tracking an always incrementing counter which already mitigated against this replay attack preventing older codes from being used.

The TOTP plugin was vulnerable, however its impacted time was limited to only the administrator configured time window which by default is approximately 2 minutes, though this time could be significantly longer if default configuration was changed.

Plugins not provided by the TFA package may have a range of vulnerability from none to indefinite depending upon how they used the API and if they have other protection that mitigates the impact.

Historical Context:
This vulnerability tracks it history back to when the related code was part of the tfa_basic module. The tfa_basic modules maintainers were made aware of this issue shortly after the TFA maintainers in order to allow them time to resolve the issue prior to publication.

A bounty was held by the Drupal Association in 2014, see https://groups.drupal.org/node/439868. the TOTP plugin being vulnerable to a replay attack was discovered at that time and the related fix was the source of this vulnerability.

The issue was fixed in public as tfa_basic did not have a stable release a the time to qualify for DST security coverage. No discussion appears to have been had regarding the hash_salt posing a risk of invaliding codes meaning this was likely an oversight at the time.

A comment was made in an issue in the upstream library

Here is a possible solution for your problem. You don't have to store the used codes; you only have to store the timeslice of the last successful authentication.

(https://github.com/PHPGangsta/GoogleAuthenticator/issues/9#issuecomment-...)

At the time this suggestion was not adopted.

Long term fixes applied in 8.x-1.3 to combat this vulnerably:
For alreadyAcceptedCode()/storeAcceptedCode() we no longer utilize the site hash. All new logins after 8.x.1.3 is deployed will be safe from changes to the site hash_salt. A configuration option was added to allow sites to input previously used hashes to prevent replay attacks using older codes as a migration method. This allows all plugins a method to continue operating without significant rewriting of their code.

The TOTP plugin now uses an always incrementing counter by storing the times slice preventing the use of ‘stale’ codes, that are the same age or older than the most recently accepted code, even if it is a code that has never been submitted.

Followups:
📌 Remove usage of alreadyAcceptedCode()/storeAccepedCode() in the TOTP,HOTP, Recovery Code plugins. Active

The second vulnerability involves a race condition that could allow for a replay attack.

Prior to 8.x-1.3 here was a short time (absolute maximum is PHP’s MAX_EXECUTION_TIME) where a code could be accepted by TFA twice due to there being no locking between the time a code was validated the time a code was stored as used. In order for this to occur an attacker would be required to intercept a valid login (with token), cause the server to delay processing of the request, and submit a competing request before the initial request reached the stage to store the previously submitted code. In general terms the timeline for this occurring is extremely short and the chance for success extremely low. This would require an attack to be explicitly targeting a site.

The TFA TOTP, HOTP, and Recovery Code plugins were all subject to this exploit.

To combat this in 8.x.1.3 TFA now maintains a lock prior to calling the plugins in order to ensure that a plugin does not possibly process the same code before it has been stored by storeAcceptedCode() is called. In 2.x this global lock will be removed and all plugins will be expected to obtain a lock where necessary.

Followups:
📌 Require plugins to be responsible for locking Active

Committed the tests to the 8.x-1.x repo. The remaining followups can stand on their own.

The equivalent tests for the 2.x branch were handled in 📌 Switch to spomky-labs/otphp Fixed as part of converting the OTP library.

Automatically closed - issue fixed for 2 weeks with no activity.