From 8f76a3ac313dc0f5f18fddae843a2d5ebd8b5f49 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Sat, 8 Aug 2026 10:50:25 -0700 Subject: [PATCH] perf(login): verify Argon2 passwords on a parked worker thread An Argon2 verify is ~8.9 ms of frozen world per login attempt, successful or not, so a credential flood is a full-cost stall per packet without needing valid credentials. Measurement puts the on-loop saving at 3.5-8.9 ms: the hand-off costs ~220 ns, and the only real residue is the loop's own work slowing while a memory-hard KDF evicts shared L3. One worker, not a pool. The per-login contention tax falls with concurrency while total loop damage rises, so one hasher harms the loop least; and a single hasher cannot cost the loop more than the inline verify under any scheduling regime, because at worst it takes an equal share of one core. That bound is what lets the measurement extrapolate to hardware we cannot inspect, and a pool breaks it. It also caps live Argon2 arenas at one, which answers memory exhaustion without a separate mechanism. Scoped to Argon2-stored accounts. SHA and MD5 protections share a HashAlgorithm instance whose ComputeHash is not thread safe, and they cost microseconds anyway. Their one-time rehash into Argon2 stays on the loop too: it costs a migrating account a single 8.9 ms login exactly as today. The worker parks on an AutoResetEvent and never spins. The spin in SerializationThreadWorker exists to wait on a producer mid-drain; there is no such race here, so this is strictly cheaper at idle. It yields while the world is in PendingSave or Saving -- PendingSave included, because the serialization threads are already awake and spinning on an empty queue by then. Phrase derivation moves to AccountSecurity.DerivePhrase so verification and rehash cannot disagree about the rule, which is the shape of the lockout fixed in #2562. ApplyPasswordUpgrade refuses to write when the stored hash changed while the verify ran, so a password set mid-flight is not replaced by a rehash of the one it superseded. --- Projects/Server/Events/AccountLoginEvent.cs | 7 + .../Accounting/PasswordVerificationTests.cs | 154 +++++++++++ Projects/UOContent/Accounting/Account.cs | 55 +++- .../UOContent/Accounting/AccountHandler.cs | 132 ++++++++- .../Accounting/Security/AccountSecurity.cs | 13 + .../Security/Argon2PasswordProtection.cs | 8 + .../Security/PasswordVerificationWorker.cs | 259 ++++++++++++++++++ .../Network/Packets/IncomingAccountPackets.cs | 19 +- 8 files changed, 624 insertions(+), 23 deletions(-) create mode 100644 Projects/UOContent.Tests/Tests/Accounting/PasswordVerificationTests.cs create mode 100644 Projects/UOContent/Accounting/Security/PasswordVerificationWorker.cs diff --git a/Projects/Server/Events/AccountLoginEvent.cs b/Projects/Server/Events/AccountLoginEvent.cs index 5c81af000..b631ac68a 100644 --- a/Projects/Server/Events/AccountLoginEvent.cs +++ b/Projects/Server/Events/AccountLoginEvent.cs @@ -37,6 +37,13 @@ public class AccountLoginEventArgs public bool Accepted { get; set; } public ALRReason RejectReason { get; set; } + + /// + /// No verdict yet: a subscriber moved the password check off the game loop and will reply + /// itself once it lands. The packet handler must not send an accept or a reject when this is + /// set, or the client receives two answers to one login. + /// + public bool Deferred { get; set; } } public static partial class EventSink diff --git a/Projects/UOContent.Tests/Tests/Accounting/PasswordVerificationTests.cs b/Projects/UOContent.Tests/Tests/Accounting/PasswordVerificationTests.cs new file mode 100644 index 000000000..efd506bb3 --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Accounting/PasswordVerificationTests.cs @@ -0,0 +1,154 @@ +using System; +using Server.Accounting; +using Server.Accounting.Security; +using Xunit; + +namespace Server.Tests.Accounting; + +[Collection("Sequential UOContent Tests")] +public class PasswordVerificationTests : IDisposable +{ + private const string Password = "hunter2"; + + private readonly PasswordProtectionAlgorithm _originalAlgorithm = AccountSecurity.CurrentAlgorithm; + + public PasswordVerificationTests() => AccountSecurity.CurrentAlgorithm = PasswordProtectionAlgorithm.Argon2; + + public void Dispose() => AccountSecurity.CurrentAlgorithm = _originalAlgorithm; + + private static Account CreateAccount(string username) => + Accounts.GetAccount(username) as Account ?? new Account(username, Password); + + private static PasswordVerificationJob JobFor(Account account, string submitted) => + new() + { + Account = account, + StoredHash = account.Password, + VerifyPhrase = account.GetVerifyPhrase(submitted), + RehashPhrase = account.NeedsPasswordUpgrade() ? account.GetRehashPhrase(submitted) : null, + TargetAlgorithm = AccountSecurity.CurrentAlgorithm + }; + + [Fact] + public void VerifiesTheCorrectPassword() + { + var account = CreateAccount("offloop-correct-user"); + + var outcome = PasswordVerificationWorker.ComputeInline(JobFor(account, Password)); + + Assert.True(outcome.Verified); + } + + [Fact] + public void RejectsTheWrongPassword() + { + var account = CreateAccount("offloop-wrong-user"); + + var outcome = PasswordVerificationWorker.ComputeInline(JobFor(account, "not-the-password")); + + Assert.False(outcome.Verified); + Assert.Null(outcome.UpgradedPassword); + } + + [Fact] + public void ProducesNoUpgradeWhenParametersAreCurrent() + { + var account = CreateAccount("offloop-current-user"); + + var outcome = PasswordVerificationWorker.ComputeInline(JobFor(account, Password)); + + Assert.True(outcome.Verified); + Assert.Null(outcome.UpgradedPassword); + } + + [Fact] + public void ProducesAnUpgradeWhenParametersAreStale() + { + var account = CreateAccount("offloop-stale-user"); + + // The shipping default before #2562: Argon2i, m=8192, t=3, p=1. + account.Password = + "$argon2i$v=19$m=8192,t=3,p=1$LD1XJz7P3wQmIJ+Tu6ScgA$NO5hBABsHQ172C5nDO2X4gWnB4jDef3x6WhLdVE2LFw"; + + var outcome = PasswordVerificationWorker.ComputeInline(JobFor(account, Password)); + + Assert.True(outcome.Verified); + Assert.StartsWith("$argon2id$v=19$m=16384,t=1,p=1$", outcome.UpgradedPassword); + } + + [Fact] + public void ProducesNoUpgradeWhenThePasswordIsWrong() + { + var account = CreateAccount("offloop-wrong-stale-user"); + account.Password = + "$argon2i$v=19$m=8192,t=3,p=1$LD1XJz7P3wQmIJ+Tu6ScgA$NO5hBABsHQ172C5nDO2X4gWnB4jDef3x6WhLdVE2LFw"; + + var outcome = PasswordVerificationWorker.ComputeInline(JobFor(account, "not-the-password")); + + Assert.False(outcome.Verified); + Assert.Null(outcome.UpgradedPassword); + } + + [Fact] + public void AppliesAnUpgradeWhenThePasswordIsUnchanged() + { + var account = CreateAccount("offloop-apply-user"); + var stored = account.Password; + + var upgraded = Argon2PasswordProtection.Instance.EncryptPassword(Password); + account.ApplyPasswordUpgrade(stored, upgraded, PasswordProtectionAlgorithm.Argon2); + + Assert.Equal(upgraded, account.Password); + Assert.True(account.CheckPassword(Password)); + } + + /// + /// The verify runs off-loop for ~9 ms. A password changed in that window was already written + /// with current parameters; applying the stale upgrade would replace it with a hash of the + /// previous password and lock the account out. + /// + [Fact] + public void DropsAnUpgradeWhenThePasswordChangedMeanwhile() + { + var account = CreateAccount("offloop-stale-apply-user"); + var storedAtDispatch = account.Password; + + // Derived from the old password, as the worker would have. + var upgraded = Argon2PasswordProtection.Instance.EncryptPassword(Password); + + // ...and the password changes before the verdict lands. + account.SetPassword("a-brand-new-password"); + var afterChange = account.Password; + + account.ApplyPasswordUpgrade(storedAtDispatch, upgraded, PasswordProtectionAlgorithm.Argon2); + + Assert.Equal(afterChange, account.Password); + Assert.True(account.CheckPassword("a-brand-new-password")); + Assert.False(account.CheckPassword(Password)); + } + + [Theory] + [InlineData(PasswordProtectionAlgorithm.SHA1)] + [InlineData(PasswordProtectionAlgorithm.SHA2)] + public void UsesTheUsernameSaltedPhraseForShaAccounts(PasswordProtectionAlgorithm algorithm) + { + AccountSecurity.CurrentAlgorithm = algorithm; + var account = CreateAccount($"offloop-phrase-{algorithm}-user"); + + // Verification must use the algorithm the hash was stored under... + Assert.Equal($"{account.Username}{Password}", account.GetVerifyPhrase(Password)); + + // ...and a rehash the one it is moving to. Swapping these is the #2562 lockout. + AccountSecurity.CurrentAlgorithm = PasswordProtectionAlgorithm.Argon2; + Assert.Equal(Password, account.GetRehashPhrase(Password)); + } + + [Fact] + public void UsesTheBarePasswordForArgon2Accounts() + { + var account = CreateAccount("offloop-phrase-argon2-user"); + + Assert.Equal(Password, account.GetVerifyPhrase(Password)); + Assert.Equal(Password, account.GetRehashPhrase(Password)); + } +} diff --git a/Projects/UOContent/Accounting/Account.cs b/Projects/UOContent/Accounting/Account.cs index 45ede29bb..4a99cfaee 100644 --- a/Projects/UOContent/Accounting/Account.cs +++ b/Projects/UOContent/Accounting/Account.cs @@ -379,28 +379,63 @@ public partial class Account : IAccount, IComparable public void SetPassword(string plainPassword) { PasswordAlgorithm = AccountSecurity.CurrentAlgorithm; - var phrase = PasswordAlgorithm is PasswordProtectionAlgorithm.SHA1 or PasswordProtectionAlgorithm.SHA2 - ? $"{_username}{plainPassword}" - : plainPassword; + Password = AccountSecurity.CurrentPasswordProtection.EncryptPassword( + AccountSecurity.DerivePhrase(PasswordAlgorithm, _username, plainPassword) + ); + } - Password = AccountSecurity.CurrentPasswordProtection.EncryptPassword(phrase); + /// The phrase that verifies against the currently stored hash. + internal string GetVerifyPhrase(string plainPassword) => + AccountSecurity.DerivePhrase(_passwordAlgorithm, _username, plainPassword); + + /// The phrase a rehash to the configured algorithm would be derived from. + internal string GetRehashPhrase(string plainPassword) => + AccountSecurity.DerivePhrase(AccountSecurity.CurrentAlgorithm, _username, plainPassword); + + /// + /// Whether a successful login should rewrite the stored hash, because the algorithm changed or + /// its cost parameters moved. + /// + internal bool NeedsPasswordUpgrade() => + _passwordAlgorithm != AccountSecurity.CurrentAlgorithm || + AccountSecurity.CurrentPasswordProtection.NeedsRehash(Password); + + /// + /// Applies a rehash derived off the game loop. Not to be confused with the private + /// UpgradePassword below, which adopts a legacy hash wholesale during RunUO/ServUO + /// import. + /// + /// + /// The hash the verification started from. Milliseconds passed while it ran, and the password + /// may have been changed in that window by an admin or by the player. The newer value was + /// already written with current parameters and needs no upgrade, so this drops the stale one + /// rather than replacing a live credential with a hash of the previous password. + /// + internal void ApplyPasswordUpgrade( + string expectedCurrent, string newEncrypted, PasswordProtectionAlgorithm algorithm + ) + { + if (!string.Equals(Password, expectedCurrent, StringComparison.Ordinal)) + { + return; + } + + PasswordAlgorithm = algorithm; + Password = newEncrypted; } public bool CheckPassword(string plainPassword) { - var phrase = _passwordAlgorithm is PasswordProtectionAlgorithm.SHA1 or PasswordProtectionAlgorithm.SHA2 - ? $"{_username}{plainPassword}" - : plainPassword; + var ok = AccountSecurity.GetPasswordProtection(_passwordAlgorithm) + .ValidatePassword(Password, GetVerifyPhrase(plainPassword)); - var ok = AccountSecurity.GetPasswordProtection(_passwordAlgorithm).ValidatePassword(Password, phrase); if (!ok) { return false; } // Upgrade the password protection in case we change the algorithm - if (_passwordAlgorithm != AccountSecurity.CurrentAlgorithm || - AccountSecurity.CurrentPasswordProtection.NeedsRehash(Password)) + if (NeedsPasswordUpgrade()) { SetPassword(plainPassword); } diff --git a/Projects/UOContent/Accounting/AccountHandler.cs b/Projects/UOContent/Accounting/AccountHandler.cs index 542e16a40..8aea6cbe2 100644 --- a/Projects/UOContent/Accounting/AccountHandler.cs +++ b/Projects/UOContent/Accounting/AccountHandler.cs @@ -5,6 +5,7 @@ using System.Net; using System.Runtime.CompilerServices; using ModernUO.CodeGeneratedEvents; using Server.Accounting; +using Server.Accounting.Security; using Server.Engines.CharacterCreation; using Server.Engines.Help; using Server.Logging; @@ -307,25 +308,134 @@ public static class AccountHandler logger.Information("Login: {NetState} Access denied for '{Username}'", e.State, un); e.RejectReason = LockdownLevel > AccessLevel.Player ? ALRReason.BadComm : ALRReason.BadPass; } - else if (!acct.CheckPassword(pw)) + else { - logger.Information("Login: {NetState} Invalid password for '{Username}'", e.State, un); - e.RejectReason = ALRReason.BadPass; + HandlePasswordCheck(e, acct, pw); } - else if (acct.Banned) + } + + /// + /// Decides where the password check runs, and produces the verdict when it runs here. An + /// else-if chain cannot express this: the off-loop path yields no verdict at all, and the + /// saturated path is a rejection rather than a reason to fall through and verify. + /// + private static void HandlePasswordCheck(AccountLoginEventArgs e, Account acct, string pw) + { + switch (DispatchPasswordCheck(e, acct, pw)) { - logger.Information("Login: {NetState} Banned account '{Username}'", e.State, un); + case PasswordCheckDispatch.Deferred: + { + e.Deferred = true; + return; + } + case PasswordCheckDispatch.Saturated: + { + // Reject rather than verify inline: steering the work back onto the loop is + // what a flood would be trying to achieve. + logger.Warning( + "Login: {NetState} Password verification queue full, rejecting '{Username}'", + e.State, + acct.Username + ); + + e.RejectReason = ALRReason.BadComm; + return; + } + } + + if (!acct.CheckPassword(pw)) + { + logger.Information("Login: {NetState} Invalid password for '{Username}'", e.State, acct.Username); + e.RejectReason = ALRReason.BadPass; + return; + } + + ApplyVerifiedLogin(e, acct); + } + + /// + /// Everything after the password is known good. Shared so an off-loop verdict lands in exactly + /// the same state as an inline one. + /// + private static void ApplyVerifiedLogin(AccountLoginEventArgs e, Account acct) + { + if (acct.Banned) + { + logger.Information("Login: {NetState} Banned account '{Username}'", e.State, acct.Username); e.RejectReason = ALRReason.Blocked; + return; + } + + logger.Information("Login: {NetState} Valid credentials for '{Username}'", e.State, acct.Username); + e.State.Account = acct; + e.Accepted = true; + + acct.LogAccess(e.State); + LoginAllowlist.RecordLogin(e.State?.Address); + } + + private enum PasswordCheckDispatch + { + /// Verify on the loop, as before. + Inline, + + /// Handed to the verification thread; no verdict yet. + Deferred, + + /// The queue is full. + Saturated + } + + /// + /// Hands an Argon2 verify to the verification thread. + /// + /// Argon2-stored accounts only. A SHA/MD5 hash is verified in microseconds and its protection + /// holds a shared HashAlgorithm whose ComputeHash is not thread safe, so those + /// stay here. Their one-time rehash into Argon2 stays here too: it costs a migrating account a + /// single 8.9 ms login, exactly as it does today, and moving it would mean either making the + /// player wait on an upgrade that does not gate their verdict, or applying account state from a + /// callback with no login left to attach it to. + /// + private static PasswordCheckDispatch DispatchPasswordCheck(AccountLoginEventArgs e, Account acct, string pw) + { + if (!PasswordVerificationWorker.Enabled || + AccountSecurity.CurrentAlgorithm != PasswordProtectionAlgorithm.Argon2 || + acct.PasswordAlgorithm != PasswordProtectionAlgorithm.Argon2) + { + return PasswordCheckDispatch.Inline; + } + + var job = new PasswordVerificationJob + { + Account = acct, + State = e.State, + StoredHash = acct.Password, + VerifyPhrase = acct.GetVerifyPhrase(pw), + RehashPhrase = acct.NeedsPasswordUpgrade() ? acct.GetRehashPhrase(pw) : null, + TargetAlgorithm = AccountSecurity.CurrentAlgorithm + }; + + return PasswordVerificationWorker.TryEnqueue(job) + ? PasswordCheckDispatch.Deferred + : PasswordCheckDispatch.Saturated; + } + + /// Resumes a login whose password check ran on the verification thread. + internal static void CompleteDeferredAccountLogin(NetState state, Account acct, bool verified) + { + var e = new AccountLoginEventArgs(state, acct.Username, null); + + if (verified) + { + ApplyVerifiedLogin(e, acct); } else { - logger.Information("Login: {NetState} Valid credentials for '{Username}'", e.State, un); - e.State.Account = acct; - e.Accepted = true; - - acct.LogAccess(e.State); - LoginAllowlist.RecordLogin(e.State?.Address); + logger.Information("Login: {NetState} Invalid password for '{Username}'", state, acct.Username); + e.RejectReason = ALRReason.BadPass; } + + IncomingAccountPackets.CompleteAccountLogin(state, e.Accepted, e.RejectReason); } [OnEvent(nameof(GameServer.GameServerLoginEvent))] diff --git a/Projects/UOContent/Accounting/Security/AccountSecurity.cs b/Projects/UOContent/Accounting/Security/AccountSecurity.cs index 9f7faeafc..2a4385043 100644 --- a/Projects/UOContent/Accounting/Security/AccountSecurity.cs +++ b/Projects/UOContent/Accounting/Security/AccountSecurity.cs @@ -51,6 +51,19 @@ public static class AccountSecurity } } + /// + /// The string actually fed to the KDF. SHA1 and SHA2 salt by username; everything else hashes + /// the password alone. Verification must derive with the algorithm the stored hash was made + /// with, and a rehash with the one it is moving to -- deriving with the wrong one produces a + /// hash that verifies once and never again. + /// + public static string DerivePhrase( + PasswordProtectionAlgorithm algorithm, string username, string plainPassword + ) => + algorithm is PasswordProtectionAlgorithm.SHA1 or PasswordProtectionAlgorithm.SHA2 + ? $"{username}{plainPassword}" + : plainPassword; + public static IPasswordProtection GetPasswordProtection(PasswordProtectionAlgorithm algorithm) { var passwordProtection = algorithm switch diff --git a/Projects/UOContent/Accounting/Security/Argon2PasswordProtection.cs b/Projects/UOContent/Accounting/Security/Argon2PasswordProtection.cs index 0a952117d..3676dba7d 100644 --- a/Projects/UOContent/Accounting/Security/Argon2PasswordProtection.cs +++ b/Projects/UOContent/Accounting/Security/Argon2PasswordProtection.cs @@ -21,6 +21,14 @@ public class Argon2PasswordProtection : IPasswordProtection { public static IPasswordProtection Instance = new Argon2PasswordProtection(); + /// + /// An instance sharing no state with . Verification is static-backed and + /// safe to call from anywhere, but hashing draws its salt from a per-instance + /// , so a thread that hashes off the game loop takes its own + /// rather than racing the loop for that one field. + /// + public static IPasswordProtection CreateIsolated() => new Argon2PasswordProtection(); + // 16 MiB at t=1 is cheaper than 8 MiB at t=3 (8.5 ms vs 10.1 ms) and twice as memory-hard, which // is what resists GPU and ASIC cracking. p=1: native argon2 spawns a thread per lane. private readonly Argon2PasswordHasher _passwordHasher = new( diff --git a/Projects/UOContent/Accounting/Security/PasswordVerificationWorker.cs b/Projects/UOContent/Accounting/Security/PasswordVerificationWorker.cs new file mode 100644 index 000000000..ea08cc3dc --- /dev/null +++ b/Projects/UOContent/Accounting/Security/PasswordVerificationWorker.cs @@ -0,0 +1,259 @@ +/************************************************************************* + * ModernUO * + * Copyright 2019-2026 - ModernUO Development Team * + * Email: hi@modernuo.com * + * File: PasswordVerificationWorker.cs * + * * + * This program is free software: you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation, either version 3 of the License, or * + * (at your option) any later version. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program. If not, see . * + *************************************************************************/ + +using System; +using System.Collections.Concurrent; +using System.Threading; +using Server.Logging; +using Server.Misc; +using Server.Network; + +namespace Server.Accounting.Security; + +/// +/// Work handed to the verification thread. Everything here is either an immutable string or a +/// reference the worker only carries -- the worker reads no game state and writes none. +/// +internal sealed class PasswordVerificationJob +{ + public Account Account; + public NetState State; + + /// The stored hash at dispatch, used to verify and to guard the upgrade against a + /// password change that lands while this runs. + public string StoredHash; + + /// Phrase to verify against . + public string VerifyPhrase; + + /// Phrase to rehash from, or null when no upgrade is due. + public string RehashPhrase; + + public PasswordProtectionAlgorithm TargetAlgorithm; +} + +internal readonly struct PasswordVerificationOutcome +{ + public readonly bool Verified; + + /// The new hash, or null when the password did not verify or needed no upgrade. + public readonly string UpgradedPassword; + + public PasswordVerificationOutcome(bool verified, string upgradedPassword) + { + Verified = verified; + UpgradedPassword = upgradedPassword; + } +} + +/// +/// Runs Argon2 off the game loop. +/// +/// An Argon2 verify is ~8.9 ms, which is more than half a frame of frozen world for every login +/// attempt, successful or not. Measurement (docs/handoffs/2026-08-07-off-loop-argon2-hashing.md) +/// puts the on-loop saving at 3.5-8.9 ms per login: the hand-off costs ~220 ns, and the only real +/// residue is the loop's own work slowing while a memory-hard KDF evicts shared L3. +/// +/// One worker, deliberately, for three reasons that agree: +/// - the per-login contention tax falls with concurrency but total loop damage rises, so one +/// hasher does the least harm to the loop; +/// - a single background hasher cannot cost the loop more than the inline verify under any +/// scheduling regime, because at worst it takes an equal share of one core -- which is what +/// makes the measurement extrapolate to hardware we cannot inspect. A pool breaks that bound; +/// - exactly one 16 MiB Argon2 arena is live at a time whatever the login volume, which answers +/// memory-exhaustion without a separate cap. +/// +/// Throughput is ~110 verifies/sec. Wall-clock login latency is explicitly not a concern, so +/// head-of-line blocking during a rush costs nothing. +/// +internal sealed class PasswordVerificationWorker +{ + private static readonly ILogger logger = LogFactory.GetLogger(typeof(PasswordVerificationWorker)); + + /// + /// Pending cap. Overflow rejects the login rather than verifying it inline: falling back to the + /// loop would let anyone who fills the queue steer the work back onto the thread this exists to + /// protect. + /// + internal const int MaxPending = 128; + + // Nothing signals the worker when a save freeze ends, so it re-checks on this interval -- but + // only while a save is in progress, never in steady state. + private const int SaveGatePollMs = 50; + + private static PasswordVerificationWorker _instance; + + /// + /// Off-loop verification needs a spare core to move work to, which a 1-2 core host does not + /// have, and is pointless on a dev box or test shard where logins are rare and the simpler + /// path is easier to reason about. + /// + internal static bool Enabled { get; } = +#if DEBUG + false; +#else + Environment.ProcessorCount >= 4; +#endif + + private readonly Thread _thread; + private readonly AutoResetEvent _work = new(false); + private readonly ConcurrentQueue _queue = new(); + + // Its own Argon2, sharing no RNG with the loop's. Verification is static-backed and would be + // safe either way; hashing is not. + private readonly IPasswordProtection _argon2 = Argon2PasswordProtection.CreateIsolated(); + + private int _pending; + private volatile bool _exit; + + private PasswordVerificationWorker() + { + _thread = new Thread(Execute) + { + IsBackground = true, + Name = "Password Verification" + }; + + _thread.Start(); + } + + // Created on first use, so a shard that never takes the off-loop path never allocates a thread. + private static PasswordVerificationWorker Instance => _instance ??= new PasswordVerificationWorker(); + + internal static int Pending => _instance?._pending ?? 0; + + /// + /// Queues a job. False when the queue is full, in which case the caller must reject the login + /// without verifying. + /// + internal static bool TryEnqueue(PasswordVerificationJob job) => Instance.TryEnqueueCore(job); + + private bool TryEnqueueCore(PasswordVerificationJob job) + { + if (Volatile.Read(ref _pending) >= MaxPending) + { + return false; + } + + Interlocked.Increment(ref _pending); + _queue.Enqueue(job); + _work.Set(); + + return true; + } + + /// + /// Argon2 only, and only outside the save freeze. The freeze runs on the loop, so nothing new + /// can be queued while it holds; checking before each job bounds the overlap to whichever hash + /// was already in flight. PendingSave counts too -- the serialization threads are awake and + /// spinning on an empty queue by then, which is the worst moment to add a competitor. + /// + private static bool CanRunNow() => + World.WorldState is WorldState.Running or WorldState.WritingSave; + + private void Execute() + { + while (!_exit) + { + if (_queue.IsEmpty) + { + // A kernel block at zero CPU. Set() during a hash leaves the event signalled, so a + // wake arriving mid-job is not lost. + _work.WaitOne(); + continue; + } + + if (!CanRunNow()) + { + _work.WaitOne(SaveGatePollMs); + continue; + } + + if (!_queue.TryDequeue(out var job)) + { + continue; + } + + Interlocked.Decrement(ref _pending); + + PasswordVerificationOutcome outcome; + + try + { + outcome = Compute(job); + } + catch (Exception ex) + { + // A verdict must still come back, or the connection waits forever for a reply. + logger.Error(ex, "Password verification failed for {Username}", job.Account?.Username); + outcome = new PasswordVerificationOutcome(false, null); + } + + Core.LoopContext.Post(() => Apply(job, outcome)); + } + } + + private PasswordVerificationOutcome Compute(PasswordVerificationJob job) + { + if (!_argon2.ValidatePassword(job.StoredHash, job.VerifyPhrase)) + { + return new PasswordVerificationOutcome(false, null); + } + + return new PasswordVerificationOutcome( + true, + job.RehashPhrase == null ? null : _argon2.EncryptPassword(job.RehashPhrase) + ); + } + + private static void Apply(PasswordVerificationJob job, PasswordVerificationOutcome outcome) + { + var state = job.State; + + // The connection may have gone while the hash ran. A dead NetState must not be revived, and + // nothing may be written on its behalf. + if (state?.Running != true) + { + return; + } + + if (outcome.Verified && outcome.UpgradedPassword != null) + { + job.Account.ApplyPasswordUpgrade(job.StoredHash, outcome.UpgradedPassword, job.TargetAlgorithm); + } + + AccountHandler.CompleteDeferredAccountLogin(state, job.Account, outcome.Verified); + } + + /// Runs a job on the calling thread. The seam the tests drive, and the path taken when + /// off-loop verification is gated off. + internal static PasswordVerificationOutcome ComputeInline(PasswordVerificationJob job) => + Instance.Compute(job); + + internal static void Exit() + { + var instance = _instance; + + if (instance == null) + { + return; + } + + instance._exit = true; + instance._work.Set(); + instance._thread.Join(TimeSpan.FromSeconds(5)); + _instance = null; + } +} diff --git a/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs b/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs index 888eeb6d7..792ed880b 100644 --- a/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs +++ b/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs @@ -564,7 +564,22 @@ public static class IncomingAccountPackets EventSink.InvokeAccountLogin(accountLoginEventArgs); - if (accountLoginEventArgs.Accepted) + // The password check moved off the loop; whoever took it replies when the verdict lands. + if (accountLoginEventArgs.Deferred) + { + return; + } + + CompleteAccountLogin(state, accountLoginEventArgs.Accepted, accountLoginEventArgs.RejectReason); + } + + /// + /// Replies to an account login. Split out so a verdict produced off the loop reaches the client + /// through exactly the same path as one produced inline. + /// + internal static void CompleteAccountLogin(NetState state, bool accepted, ALRReason rejectReason) + { + if (accepted) { var serverListEventArgs = new GatewayServer.ServerListEventArgs(state, state.Account); @@ -584,7 +599,7 @@ public static class IncomingAccountPackets else { state.Account = null; - AccountLogin_ReplyRej(state, accountLoginEventArgs.RejectReason); + AccountLogin_ReplyRej(state, rejectReason); } }