fix: refuse a legacy account's password at the same cost as any other - #898
Merged
blaipr merged 1 commit intoSep 2, 2026
Merged
Conversation
A row with `isMigrate` holds a sha1, md5 or crypt digest rather than a bcrypt hash, so `checkMigrateUser()`'s three comparisons take microseconds and its fourth hands that digest to password_verify(), which rejects it on sight. A wrong password for such an account refused in 0.3ms where a migrated account and an unknown login both paid 220ms for a real verify — measured by the test, which was written first and failed on it. That is the enumeration oracle the catch block below already deals with, reopened for a subset: a fast refusal said 'this login exists and is one of the old ones', naming both a real account and the ones whose stored hashes are weakest, to a caller who never guessed a password. The migrate branch now answers for itself and spends a verify against ABSENT_USER_HASH on the way to the same refusal. Falling through would not have spent it: that check is the same one checkMigrateUser() just made against the same non-bcrypt value, so the answer was false either way and short-circuiting is behaviour-preserving.
blaipr
deleted the
fix/a-legacy-account-refuses-a-password-at-the-same-cost
branch
September 2, 2026 21:42
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
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.
A wrong password for an account that has not been migrated refused in 0.3ms, where a wrong
password for a migrated account and a login naming nobody both cost 220ms — measured on this
installation by the test below, which was written before the fix and failed on it.
A row with
isMigrateholds a sha1, md5 or crypt digest rather than a bcrypt hash.checkMigrateUser()compares against all three — microseconds — and then hands that same digest toHash::checkHashKey(), wherepassword_verify()rejects it on sight because it is not a bcrypthash at all. Falling through to the check below it did not spend the time either: that is the same
check against the same value, so it fast-fails identically.
This is the enumeration oracle
authUser()'s catch block already deals with, reopened for asubset. A fast refusal says this login exists and is one of the old ones — a real account
name confirmed to an unauthenticated caller who never guessed a password, and specifically the
accounts whose stored hashes are the weakest, which is the set an attacker most wants named.
The change
The migrate branch now answers for itself, and spends a verify against
ABSENT_USER_HASHon itsway to the same refusal — the same fixed hash, and the same reasoning, as the missing-user path
directly below it.
Nothing else moves:
checkMigrateUser()returning false already meant the fall-through check wouldreturn false too, so short-circuiting there is behaviour-preserving. A migrated account with a
bcrypt hash and a wrong password paid for two verifies before and pays for two now.
Tests
testALegacyAccountRefusesAPasswordAtTheSameCostAsAMigratedOnecompares the two paths againsteach other rather than against a number of milliseconds, so it calibrates itself to whatever
machine it runs on — the same technique as
testALoginForAMissingUserCostsTheSameAsOneForAnExistingUser, which it sits beside.Written first and watched fail:
(migrated: 220.3ms, legacy: 0.3ms). The four existing migrationtests — SHA1, MD5, crypt and bcrypt — still pass, so a legacy account that supplies the right
password still authenticates and still gets migrated.