feat(accounts): add a config-gated repair for mis-migrated passwords
Accounts corrupted by the pre-fix SetPassword hold argon2(username + password) tagged with an algorithm whose phrase rule omits the username. Nothing in the hash distinguishes that from a forgotten password, so the only detection is to retry with the other phrase on a failed verify -- which doubles the cost of every failed login, and failed logins are the credential-stuffing surface. Hence accountSecurity.repairMigratedPasswords, default false: an operator turns it on for a migration window and off again. Repaired accounts are force-rehashed because their stored parameters may already be current, in which case NeedsRehash would decline and the account would verify once and stay broken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
fcff2578dc
commit
e7ab7e8738
3 changed files with 97 additions and 5 deletions
|
|
@ -73,4 +73,66 @@ public class AccountPasswordTests : IDisposable
|
|||
Assert.True(account.CheckPassword("hunter2"));
|
||||
Assert.Equal(afterFirst, account.Password);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds an account whose stored hash is what the pre-fix SetPassword produced when a SHA2
|
||||
/// account was upgraded to Argon2: argon2(username + password), tagged Argon2, whose phrase
|
||||
/// rule omits the username. Unrecoverable without the plaintext, hence the repair path.
|
||||
/// </summary>
|
||||
private static Account CreateMisMigratedAccount(string username)
|
||||
{
|
||||
AccountSecurity.CurrentAlgorithm = PasswordProtectionAlgorithm.Argon2;
|
||||
var account = new Account(username, Password);
|
||||
account.Password = AccountSecurity.CurrentPasswordProtection
|
||||
.EncryptPassword($"{username}{Password}");
|
||||
|
||||
return account;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void MisMigratedAccount_IsRejected_WhenRepairIsDisabled()
|
||||
{
|
||||
var account = CreateMisMigratedAccount("repair-off-user");
|
||||
AccountSecurity.RepairMigratedPasswords = false;
|
||||
|
||||
Assert.False(account.CheckPassword(Password));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void MisMigratedAccount_IsRepaired_WhenRepairIsEnabled()
|
||||
{
|
||||
var account = CreateMisMigratedAccount("repair-on-user");
|
||||
AccountSecurity.RepairMigratedPasswords = true;
|
||||
|
||||
try
|
||||
{
|
||||
Assert.True(account.CheckPassword(Password));
|
||||
}
|
||||
finally
|
||||
{
|
||||
AccountSecurity.RepairMigratedPasswords = false;
|
||||
}
|
||||
|
||||
// Repaired in place: it must now verify with the flag back off.
|
||||
Assert.True(account.CheckPassword(Password));
|
||||
Assert.False(account.CheckPassword("wrong-password"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void WrongPassword_IsStillRejected_WhenRepairIsEnabled()
|
||||
{
|
||||
AccountSecurity.CurrentAlgorithm = PasswordProtectionAlgorithm.Argon2;
|
||||
var account = new Account("repair-wrong-pass-user", Password);
|
||||
AccountSecurity.RepairMigratedPasswords = true;
|
||||
|
||||
try
|
||||
{
|
||||
Assert.False(account.CheckPassword("wrong-password"));
|
||||
Assert.True(account.CheckPassword(Password));
|
||||
}
|
||||
finally
|
||||
{
|
||||
AccountSecurity.RepairMigratedPasswords = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -403,18 +403,36 @@ public partial class Account : IAccount, IComparable<Account>
|
|||
|
||||
public bool CheckPassword(string plainPassword)
|
||||
{
|
||||
var protection = AccountSecurity.GetPasswordProtection(_passwordAlgorithm);
|
||||
var phrase = UsesUsernamePhrase(_passwordAlgorithm) ? $"{_username}{plainPassword}" : plainPassword;
|
||||
var forceRehash = false;
|
||||
|
||||
var ok = AccountSecurity.GetPasswordProtection(_passwordAlgorithm).ValidatePassword(Password, phrase);
|
||||
if (!ok)
|
||||
if (!protection.ValidatePassword(Password, phrase))
|
||||
{
|
||||
return false;
|
||||
// A pre-fix SetPassword stored username + password under an algorithm whose phrase rule
|
||||
// omits the username. Nothing in the hash distinguishes that from a forgotten password,
|
||||
// so the only way to detect it is to try the other phrase -- which costs a second
|
||||
// verify on every failed login. Off by default for exactly that reason.
|
||||
if (!AccountSecurity.RepairMigratedPasswords || UsesUsernamePhrase(_passwordAlgorithm) ||
|
||||
!protection.ValidatePassword(Password, $"{_username}{plainPassword}"))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
logger.Warning(
|
||||
"Account '{Username}' had a password mis-migrated by a pre-fix SetPassword; repairing it.",
|
||||
_username
|
||||
);
|
||||
|
||||
// The stored hash may already carry current parameters, so NeedsRehash would say no
|
||||
// and the account would verify once and stay broken.
|
||||
forceRehash = true;
|
||||
}
|
||||
|
||||
// Rehash when either the algorithm or its cost parameters have moved on. The short-circuit
|
||||
// ordering is load-bearing: NeedsRehash must never be handed a hash produced by a different
|
||||
// algorithm, and the first clause guarantees it is not.
|
||||
if (_passwordAlgorithm != AccountSecurity.CurrentAlgorithm ||
|
||||
// algorithm, and the second clause guarantees it is not.
|
||||
if (forceRehash || _passwordAlgorithm != AccountSecurity.CurrentAlgorithm ||
|
||||
AccountSecurity.CurrentPasswordProtection.NeedsRehash(Password))
|
||||
{
|
||||
logger.Debug("Rehashing the password for account '{Username}'.", _username);
|
||||
|
|
|
|||
|
|
@ -35,6 +35,15 @@ public static class AccountSecurity
|
|||
{
|
||||
public static PasswordProtectionAlgorithm CurrentAlgorithm { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// Enables a one-time repair for accounts whose password was corrupted by the pre-fix
|
||||
/// SetPassword, which hashed username + password but tagged it with an algorithm whose phrase
|
||||
/// rule omits the username. Off by default: the repair costs a second verify on every FAILED
|
||||
/// login, and failed logins are the credential-stuffing surface. Turn it on for a migration
|
||||
/// window, then off again.
|
||||
/// </summary>
|
||||
public static bool RepairMigratedPasswords { get; set; }
|
||||
|
||||
public static IPasswordProtection CurrentPasswordProtection => GetPasswordProtection(CurrentAlgorithm);
|
||||
|
||||
public static void Configure()
|
||||
|
|
@ -45,6 +54,9 @@ public static class AccountSecurity
|
|||
PasswordProtectionAlgorithm.Argon2
|
||||
);
|
||||
|
||||
RepairMigratedPasswords =
|
||||
ServerConfiguration.GetOrUpdateSetting("accountSecurity.repairMigratedPasswords", false);
|
||||
|
||||
if (CurrentAlgorithm < PasswordProtectionAlgorithm.SHA2)
|
||||
{
|
||||
throw new Exception($"Security: {CurrentAlgorithm} is obsolete and not secure. Do not use it.");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue