diff --git a/Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs b/Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs
index 42389cbef..5253d24de 100644
--- a/Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs
+++ b/Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs
@@ -73,4 +73,66 @@ public class AccountPasswordTests : IDisposable
Assert.True(account.CheckPassword("hunter2"));
Assert.Equal(afterFirst, account.Password);
}
+
+ ///
+ /// 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.
+ ///
+ 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;
+ }
+ }
}
diff --git a/Projects/UOContent/Accounting/Account.cs b/Projects/UOContent/Accounting/Account.cs
index a4246784f..d60f387d0 100644
--- a/Projects/UOContent/Accounting/Account.cs
+++ b/Projects/UOContent/Accounting/Account.cs
@@ -403,18 +403,36 @@ public partial class Account : IAccount, IComparable
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);
diff --git a/Projects/UOContent/Accounting/Security/AccountSecurity.cs b/Projects/UOContent/Accounting/Security/AccountSecurity.cs
index 9f7faeafc..c2c495c2f 100644
--- a/Projects/UOContent/Accounting/Security/AccountSecurity.cs
+++ b/Projects/UOContent/Accounting/Security/AccountSecurity.cs
@@ -35,6 +35,15 @@ public static class AccountSecurity
{
public static PasswordProtectionAlgorithm CurrentAlgorithm { get; set; }
+ ///
+ /// 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.
+ ///
+ 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.");