From 970cd42699d3733097c7755ab4b6ceaf515b7e88 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 7 Aug 2026 16:24:37 -0700 Subject: [PATCH] fix(accounts): derive the password phrase from the target algorithm SetPassword salted the phrase with the username according to the OUTGOING algorithm but stored the result under the INCOMING one. SHA1 and SHA2 prepend the username; Argon2 and PBKDF2 do not. Two live lockouts followed: - A ServUO-imported SHA2 account logging in with the default Argon2 config was 'upgraded' to argon2(username + password) and tagged Argon2. The next login rebuilt the phrase as bare password and could never match. One successful login, then permanent lockout. - SetPassword also runs from the Account constructor, before _passwordAlgorithm is assigned, so it is still None. Creating an account under SHA1 or SHA2 hashed the bare password and tagged it with an algorithm that re-adds the username -- a brand-new account that could never log in. Derive the phrase from the algorithm being written. Also configures Accounts persistence in the UOContent test fixture so an Account can be constructed. Co-Authored-By: Claude Opus 5 (1M context) --- .../Fixtures/TestServerInitializer.cs | 4 ++ .../Tests/Accounting/AccountPasswordTests.cs | 47 +++++++++++++++++++ Projects/UOContent/Accounting/Account.cs | 26 ++++++---- 3 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs diff --git a/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs b/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs index 640c05837..c98fe65d6 100644 --- a/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs +++ b/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs @@ -100,6 +100,10 @@ internal static class TestServerInitializer } World.Configure(); + // Registers the Accounts entity persistence. Production reaches this through + // AssemblyHandler.Invoke("Configure"); the curated subset here must call it so that + // Accounts.NewAccount resolves and tests can construct an Account. + Server.Accounting.Accounts.Configure(); RaceDefinitions.Configure(); MovementImpl.Configure(); PathFollower.Configure(); diff --git a/Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs b/Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs new file mode 100644 index 000000000..b1659fa56 --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs @@ -0,0 +1,47 @@ +using Server.Accounting; +using Server.Accounting.Security; +using Xunit; + +namespace Server.Tests.Accounting; + +[Collection("Sequential UOContent Tests")] +public class AccountPasswordTests +{ + private const string Password = "hunter2"; + + [Theory] + [InlineData(PasswordProtectionAlgorithm.SHA1)] + [InlineData(PasswordProtectionAlgorithm.SHA2)] + [InlineData(PasswordProtectionAlgorithm.PBKDF2)] + [InlineData(PasswordProtectionAlgorithm.Argon2)] + public void NewAccount_CanLogIn(PasswordProtectionAlgorithm algorithm) + { + AccountSecurity.CurrentAlgorithm = algorithm; + var account = new Account($"new-{algorithm}-user", Password); + + Assert.Equal(algorithm, account.PasswordAlgorithm); + Assert.True(account.CheckPassword(Password)); + Assert.False(account.CheckPassword("wrong-password")); + } + + [Theory] + [InlineData(PasswordProtectionAlgorithm.SHA1)] + [InlineData(PasswordProtectionAlgorithm.SHA2)] + [InlineData(PasswordProtectionAlgorithm.PBKDF2)] + public void UpgradingAlgorithm_DoesNotLockTheAccountOut(PasswordProtectionAlgorithm from) + { + AccountSecurity.CurrentAlgorithm = from; + var account = new Account($"upgrade-{from}-user", Password); + Assert.True(account.CheckPassword(Password)); + + AccountSecurity.CurrentAlgorithm = PasswordProtectionAlgorithm.Argon2; + + // First login verifies under the old algorithm and rehashes under the new one. + Assert.True(account.CheckPassword(Password)); + Assert.Equal(PasswordProtectionAlgorithm.Argon2, account.PasswordAlgorithm); + + // Second login must verify against what the first one wrote. + Assert.True(account.CheckPassword(Password)); + Assert.False(account.CheckPassword("wrong-password")); + } +} diff --git a/Projects/UOContent/Accounting/Account.cs b/Projects/UOContent/Accounting/Account.cs index d79d70b1a..03ce9a2af 100644 --- a/Projects/UOContent/Accounting/Account.cs +++ b/Projects/UOContent/Accounting/Account.cs @@ -376,21 +376,31 @@ public partial class Account : IAccount, IComparable return true; } + /// + /// SHA1 and SHA2 are the ServUO-compatible algorithms; they salt the password with the + /// username. Argon2 and PBKDF2 carry their own salt and do not. + /// + private static bool UsesUsernamePhrase(PasswordProtectionAlgorithm algorithm) => + algorithm is PasswordProtectionAlgorithm.SHA1 or PasswordProtectionAlgorithm.SHA2; + public void SetPassword(string plainPassword) { - var phrase = _passwordAlgorithm is PasswordProtectionAlgorithm.SHA1 or PasswordProtectionAlgorithm.SHA2 - ? $"{_username}{plainPassword}" - : plainPassword; + // The phrase must match how CheckPassword will rebuild it *after* the algorithm changes, + // so it is derived from the target algorithm rather than the outgoing one. Deriving it + // from _passwordAlgorithm stored a username-salted hash under an algorithm that never + // re-adds the username, locking the account out on its next login -- and, because this + // runs from the constructor before _passwordAlgorithm is assigned, it also produced + // brand-new SHA1/SHA2 accounts that could never log in at all. + var algorithm = AccountSecurity.CurrentAlgorithm; + var phrase = UsesUsernamePhrase(algorithm) ? $"{_username}{plainPassword}" : plainPassword; - Password = AccountSecurity.CurrentPasswordProtection.EncryptPassword(phrase); - PasswordAlgorithm = AccountSecurity.CurrentAlgorithm; + Password = AccountSecurity.GetPasswordProtection(algorithm).EncryptPassword(phrase); + PasswordAlgorithm = algorithm; } public bool CheckPassword(string plainPassword) { - var phrase = _passwordAlgorithm is PasswordProtectionAlgorithm.SHA1 or PasswordProtectionAlgorithm.SHA2 - ? $"{_username}{plainPassword}" - : plainPassword; + var phrase = UsesUsernamePhrase(_passwordAlgorithm) ? $"{_username}{plainPassword}" : plainPassword; var ok = AccountSecurity.GetPasswordProtection(_passwordAlgorithm).ValidatePassword(Password, phrase); if (!ok)