diff --git a/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs b/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs index 1325700d5..a38af9c3d 100644 --- a/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs +++ b/Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs @@ -100,8 +100,7 @@ internal static class TestServerInitializer } World.Configure(); - // Registers the Accounts entity persistence, without which Accounts.NewAccount cannot - // resolve and no test can construct an Account. + // Registers the Accounts entity persistence; without it no test can construct an Account. Server.Accounting.Accounts.Configure(); RaceDefinitions.Configure(); MovementImpl.Configure(); diff --git a/Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs b/Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs index 14e6089cd..29a1b818f 100644 --- a/Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs +++ b/Projects/UOContent.Tests/Tests/Accounting/AccountPasswordTests.cs @@ -30,9 +30,9 @@ public class AccountPasswordTests : IDisposable Assert.False(account.CheckPassword("wrong-password")); } - // SetPassword assigns PasswordAlgorithm before deriving the phrase from it. Reversing those two - // lines salts the hash by the outgoing algorithm's rule and stores it under the incoming one, - // which verifies once and then never again. + // SetPassword assigns PasswordAlgorithm before deriving the phrase from it. Reversed, the hash + // is salted by the outgoing algorithm's rule but stored under the incoming one, which verifies + // once and then never again. [Theory] [InlineData(PasswordProtectionAlgorithm.SHA1)] [InlineData(PasswordProtectionAlgorithm.SHA2)] diff --git a/Projects/UOContent.Tests/Tests/Accounting/Security/PasswordProtectionTest.cs b/Projects/UOContent.Tests/Tests/Accounting/Security/PasswordProtectionTest.cs index 333c35692..b86df418d 100644 --- a/Projects/UOContent.Tests/Tests/Accounting/Security/PasswordProtectionTest.cs +++ b/Projects/UOContent.Tests/Tests/Accounting/Security/PasswordProtectionTest.cs @@ -75,7 +75,7 @@ public class PasswordProtectionTest Assert.False(passwordProtection.ValidatePassword(encryptedPassword, "Not the same password")); } - // The shipping default before this change. A literal, so it cannot drift with the configured + // The shipping default before this change, as a literal so it cannot drift with the configured // defaults. Password: "hunter2". private const string LegacyArgon2iHash = "$argon2i$v=19$m=8192,t=3,p=1$LD1XJz7P3wQmIJ+Tu6ScgA$NO5hBABsHQ172C5nDO2X4gWnB4jDef3x6WhLdVE2LFw"; @@ -105,9 +105,8 @@ public class PasswordProtectionTest Assert.Equal(expected, Argon2PasswordProtection.Instance.NeedsRehash(hash)); } - // Digest and salt lengths are the decoded sizes of the base64 segments, not parameter-list - // entries, so they need their own literals. Current type and cost throughout; only a length - // differs from the defaults. The theory above is the negative control at default lengths. + // Digest and salt lengths are decoded base64 sizes rather than parameter-list entries, so they + // need their own literals. Current type and cost throughout; only a length differs. [Theory] // 16-byte digest: 22 base64 chars instead of the 43 a 32-byte digest encodes to. [InlineData("$argon2id$v=19$m=16384,t=1,p=1$LD1XJz7P3wQmIJ+Tu6ScgA$NO5hBABsHQ172C5nDO2X4g")] diff --git a/Projects/UOContent.Tests/Tests/Network/Packets/AuthIdTests.cs b/Projects/UOContent.Tests/Tests/Network/Packets/AuthIdTests.cs new file mode 100644 index 000000000..456eb174a --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Network/Packets/AuthIdTests.cs @@ -0,0 +1,382 @@ +using System; +using System.Net; +using Server.Accounting; +using Server.Accounting.Security; +using Server.Network; +using Server.Tests.Network; +using Xunit; + +namespace Server.Tests.Network.Packets; + +[Collection("Sequential UOContent Tests")] +public class AuthIdTests : IDisposable +{ + private static readonly IPAddress AddressX = IPAddress.Parse("203.0.113.10"); + private static readonly IPAddress AddressY = IPAddress.Parse("203.0.113.11"); + + private readonly PasswordProtectionAlgorithm _originalAlgorithm = AccountSecurity.CurrentAlgorithm; + + public AuthIdTests() + { + AccountSecurity.CurrentAlgorithm = PasswordProtectionAlgorithm.Argon2; + IncomingAccountPackets.ClearAuthIdWindow(); + } + + public void Dispose() + { + IncomingAccountPackets.ClearAuthIdWindow(); + AccountSecurity.CurrentAlgorithm = _originalAlgorithm; + } + + private static IAccount CreateAccount(string username) => + Accounts.GetAccount(username) ?? new Account(username, "hunter2"); + + private static int Register(IAccount account, IPAddress address) => + IncomingAccountPackets.RegisterAuthId(account, address, new ClientVersion(7, 0, 0, 0)); + + [Fact] + public void VouchesForTheAccountAndAddressItWasIssuedTo() + { + var account = CreateAccount("authid-match-user"); + var authId = Register(account, AddressX); + + var result = IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressX, out var entry); + + Assert.Equal(IncomingAccountPackets.AuthIdResult.Vouched, result); + Assert.Same(account, entry.Account); + } + + [Fact] + public void RejectsADifferentAccount() + { + var issued = CreateAccount("authid-owner-user"); + var other = CreateAccount("authid-other-user"); + var authId = Register(issued, AddressX); + + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Rejected, + IncomingAccountPackets.ConsumeAuthId(authId, other.Username, AddressX, out _) + ); + } + + [Fact] + public void RejectsADifferentAddress() + { + var account = CreateAccount("authid-switch-user"); + var authId = Register(account, AddressX); + + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Rejected, + IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressY, out _) + ); + } + + [Fact] + public void MatchesTheUsernameCaseInsensitively() + { + var account = CreateAccount("AuthId-Case-User"); + var authId = Register(account, AddressX); + + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Vouched, + IncomingAccountPackets.ConsumeAuthId(authId, "authid-case-user", AddressX, out _) + ); + } + + [Fact] + public void MatchesAnIPv4MappedIPv6Address() + { + var account = CreateAccount("authid-mapped-user"); + var authId = Register(account, AddressX); + + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Vouched, + IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressX.MapToIPv6(), out _) + ); + } + + [Fact] + public void RejectsAnUnknownAuthId() + { + var account = CreateAccount("authid-unknown-user"); + var authId = Register(account, AddressX); + + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Rejected, + IncomingAccountPackets.ConsumeAuthId(authId + 1, account.Username, AddressX, out _) + ); + } + + [Fact] + public void IsSingleUseAfterASuccess() + { + var account = CreateAccount("authid-once-user"); + var authId = Register(account, AddressX); + + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Vouched, + IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressX, out _) + ); + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Rejected, + IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressX, out _) + ); + } + + // A rejected attempt must not consume the id, or anyone landing on a live one could burn it and + // force its owner to log in again. + [Fact] + public void SurvivesAnAttemptFromTheWrongAddress() + { + var account = CreateAccount("authid-not-burned-address-user"); + var authId = Register(account, AddressX); + + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Rejected, + IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressY, out _) + ); + + Assert.Equal(1, IncomingAccountPackets.AuthIdWindowCount); + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Vouched, + IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressX, out _) + ); + } + + [Fact] + public void SurvivesAnAttemptForTheWrongAccount() + { + var account = CreateAccount("authid-not-burned-account-user"); + var authId = Register(account, AddressX); + + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Rejected, + IncomingAccountPackets.ConsumeAuthId(authId, "not-the-owner", AddressX, out _) + ); + + Assert.Equal(1, IncomingAccountPackets.AuthIdWindowCount); + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Vouched, + IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressX, out _) + ); + } + + [Fact] + public void ARejectedAttemptYieldsNoEntry() + { + var account = CreateAccount("authid-no-leak-user"); + var authId = Register(account, AddressX); + + IncomingAccountPackets.ConsumeAuthId(authId, "not-the-owner", AddressX, out var entry); + + Assert.Null(entry.Account); + } + + [Fact] + public void AnExpiredIdIsSpentByItsOwner() + { + var account = CreateAccount("authid-expired-spent-user"); + var authId = Register(account, AddressX); + + var now = Core._now; + + try + { + Core._now = now + TimeSpan.FromMinutes(30.0); + + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Expired, + IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressX, out _) + ); + Assert.Equal(0, IncomingAccountPackets.AuthIdWindowCount); + } + finally + { + Core._now = now; + } + } + + // Expiry is not a lockout. The game login always verified the password before any of this + // existed, so falling back to that verify is the behaviour we started from. + [Fact] + public void ExpiresIntoAPasswordVerifyRatherThanARejection() + { + var account = CreateAccount("authid-expired-user"); + var authId = Register(account, AddressX); + + var now = Core._now; + + try + { + Core._now = now + TimeSpan.FromMinutes(30.0); + + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Expired, + IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressX, out var entry) + ); + + // Still carries the client version the game login needs. + Assert.Equal(new ClientVersion(7, 0, 0, 0), entry.Version); + } + finally + { + Core._now = now; + } + } + + [Fact] + public void AnExpiredIdFromAnotherAddressIsStillRejected() + { + var account = CreateAccount("authid-expired-elsewhere-user"); + var authId = Register(account, AddressX); + + var now = Core._now; + + try + { + Core._now = now + TimeSpan.FromMinutes(30.0); + + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Rejected, + IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressY, out _) + ); + } + finally + { + Core._now = now; + } + } + + private static int Ensure(int existingAuthId, IAccount account, IPAddress address) => + IncomingAccountPackets.EnsureAuthId( + existingAuthId, + account, + address, + new ClientVersion(7, 0, 0, 0) + ); + + [Fact] + public void IssuesAnIdWhenTheConnectionHasNone() + { + var account = CreateAccount("authid-first-select-user"); + + var authId = Ensure(0, account, AddressX); + + Assert.NotEqual(0, authId); + Assert.Equal(1, IncomingAccountPackets.AuthIdWindowCount); + } + + // Handing the same id back rather than minting another is what makes an orphan impossible, + // instead of something to clean up afterwards. + [Fact] + public void ReSelectingReturnsTheSameIdAndAddsNothingToTheWindow() + { + var account = CreateAccount("authid-reselect-user"); + var first = Ensure(0, account, AddressX); + + for (var i = 0; i < 10; i++) + { + Assert.Equal(first, Ensure(first, account, AddressX)); + } + + Assert.Equal(1, IncomingAccountPackets.AuthIdWindowCount); + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Vouched, + IncomingAccountPackets.ConsumeAuthId(first, account.Username, AddressX, out _) + ); + } + + [Fact] + public void AbandonedIdsAreSweptWhenNewOnesAreIssued() + { + var abandoned = CreateAccount("authid-abandoned-user"); + var live = CreateAccount("authid-live-user"); + + var now = Core._now; + + try + { + for (var i = 0; i < 128; i++) + { + Register(abandoned, AddressX); + } + + Assert.Equal(128, IncomingAccountPackets.AuthIdWindowCount); + + Core._now = now + TimeSpan.FromMinutes(30.0); + + var liveId = Register(live, AddressX); + + Assert.Equal(1, IncomingAccountPackets.AuthIdWindowCount); + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Vouched, + IncomingAccountPackets.ConsumeAuthId(liveId, live.Username, AddressX, out _) + ); + } + finally + { + Core._now = now; + } + } + + // A login rush is not a backlog. Every id belongs to a client on its way to redeem it, so none + // may be discarded to hold the window at some arbitrary size. + [Fact] + public void ALoginRushDoesNotEvictAnyonesAuthId() + { + var account = CreateAccount("authid-rush-user"); + var ids = new int[800]; + + for (var i = 0; i < ids.Length; i++) + { + ids[i] = Register(account, AddressX); + } + + Assert.Equal(ids.Length, IncomingAccountPackets.AuthIdWindowCount); + + // Every id issued during the rush is still redeemable, including the first one. + for (var i = 0; i < ids.Length; i++) + { + Assert.Equal( + IncomingAccountPackets.AuthIdResult.Vouched, + IncomingAccountPackets.ConsumeAuthId(ids[i], account.Username, AddressX, out _) + ); + } + } + + [Fact] + public void PreAuthenticatedGameLogin_SkipsThePasswordCheck() + { + var account = CreateAccount("authid-preauth-user"); + using var ns = PacketTestUtilities.CreateTestNetState(); + + // A wrong password is accepted only because the auth id already vouched for the account. + var e = new GameServer.GameLoginEventArgs(ns, account.Username, "wrong-password", true); + GameServer.GameServerLoginEvent(e); + + Assert.True(e.Accepted); + } + + [Fact] + public void GameLoginWithoutPreAuthentication_StillChecksThePassword() + { + var account = CreateAccount("authid-nopreauth-user"); + using var ns = PacketTestUtilities.CreateTestNetState(); + + var wrong = new GameServer.GameLoginEventArgs(ns, account.Username, "wrong-password", false); + GameServer.GameServerLoginEvent(wrong); + Assert.False(wrong.Accepted); + + var right = new GameServer.GameLoginEventArgs(ns, account.Username, "hunter2", false); + GameServer.GameServerLoginEvent(right); + Assert.True(right.Accepted); + } + + [Fact] + public void GeneratesDistinctAuthIds() + { + var account = CreateAccount("authid-distinct-user"); + + Assert.NotEqual(Register(account, AddressX), Register(account, AddressX)); + } +} diff --git a/Projects/UOContent/Accounting/AccountHandler.cs b/Projects/UOContent/Accounting/AccountHandler.cs index ea0c4b9e0..542e16a40 100644 --- a/Projects/UOContent/Accounting/AccountHandler.cs +++ b/Projects/UOContent/Accounting/AccountHandler.cs @@ -343,7 +343,9 @@ public static class AccountHandler logger.Information("Login: {NetState} Access denied for '{Username}'", e.State, un); e.Accepted = false; } - else if (!acct.CheckPassword(pw)) + // The auth id was only issued after the account login packet verified this password, so + // re-deriving the hash costs a second Argon2 verify to answer the same question. + else if (!e.PreAuthenticated && !acct.CheckPassword(pw)) { logger.Information("Login: {NetState} Invalid password for '{Username}'", e.State, un); e.Accepted = false; diff --git a/Projects/UOContent/Accounting/Security/Argon2PasswordProtection.cs b/Projects/UOContent/Accounting/Security/Argon2PasswordProtection.cs index 3b509e099..0a952117d 100644 --- a/Projects/UOContent/Accounting/Security/Argon2PasswordProtection.cs +++ b/Projects/UOContent/Accounting/Security/Argon2PasswordProtection.cs @@ -37,9 +37,8 @@ public class Argon2PasswordProtection : IPasswordProtection public bool ValidatePassword(string encryptedPassword, string plainPassword) => _passwordHasher.Verify(encryptedPassword, plainPassword); - // The PHC string carries the parameters it was hashed with, so verification uses those rather - // than the configured ones. Comparing them is what lets a parameter change reach existing - // accounts. + // Verification uses the parameters embedded in the PHC string, not the configured ones, so + // comparing them is what lets a parameter change reach existing accounts. public bool NeedsRehash(string encryptedPassword) { // Unparseable but verified: a format this build does not understand, so rewrite it. diff --git a/Projects/UOContent/Network/GameServer.cs b/Projects/UOContent/Network/GameServer.cs index 9fc170d69..b9de09515 100644 --- a/Projects/UOContent/Network/GameServer.cs +++ b/Projects/UOContent/Network/GameServer.cs @@ -6,13 +6,21 @@ public static partial class GameServer { public class GameLoginEventArgs { - public GameLoginEventArgs(NetState state, string un, string pw) + public GameLoginEventArgs(NetState state, string un, string pw, bool preAuthenticated) { State = state; Username = un; Password = pw; + PreAuthenticated = preAuthenticated; } + /// + /// The auth id presented on this game login was issued to this account, from this address, + /// after the account login packet verified the password. Read-only so a subscriber cannot + /// grant itself the skip. + /// + public bool PreAuthenticated { get; } + public NetState State { get; } public string Username { get; } diff --git a/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs b/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs index 07a444d03..888eeb6d7 100644 --- a/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs +++ b/Projects/UOContent/Network/Packets/IncomingAccountPackets.cs @@ -17,6 +17,9 @@ using System; using System.Buffers; using System.Collections.Generic; using System.IO; +using System.Net; +using System.Security.Cryptography; +using Server.Accounting; using Server.Engines.CharacterCreation; using Server.Misc; using Server.Mobiles; @@ -25,7 +28,16 @@ namespace Server.Network; public static class IncomingAccountPackets { + // Initial capacity and the point at which issuing sweeps expired ids. Not a cap; the window + // grows rather than evicting a live id. private const int _authIDWindowSize = 128; + + private static int _authIdPurgeThreshold = _authIDWindowSize; + + // The gap between PlayServerAck and the game login is seconds. Bounds how long a stolen id + // stays usable. + private static readonly TimeSpan _authIDLifetime = TimeSpan.FromMinutes(2.0); + private static readonly Dictionary _authIDWindow = new(_authIDWindowSize); @@ -34,13 +46,33 @@ public static class IncomingAccountPackets public DateTime Age; public readonly ClientVersion Version; - public AuthIDPersistence(ClientVersion v) + // GameLogin skips its password verify when both match, so the id is a bearer token and has + // to be bound to whatever earned it. + public readonly IAccount Account; + public readonly IPAddress Address; + + public AuthIDPersistence(ClientVersion v, IAccount account, IPAddress address) { Age = Core.Now; Version = v; + Account = account; + Address = Utility.Intern(address); } } + internal enum AuthIdResult + { + // No such id, or it was issued for a different account or address. + Rejected, + + // Right account and address, too old to stand in for the verify. Idling on the server list + // is normal, so this falls back to the password check rather than becoming a lockout. + Expired, + + // Issued to this account, from this address, recently. Stands in for the password verify. + Vouched + } + public static unsafe void Configure() { IncomingPackets.Register(0x00, &CreateCharacter, 104, outgameOnly: true); @@ -312,42 +344,92 @@ public static class IncomingAccountPackets } } - private static int GenerateAuthID(this NetState state) + private static int GenerateAuthID(this NetState state) => + EnsureAuthId(state.AuthId, state.Account, state.Address, state.Version); + + /// + /// One id per connection, by construction. Choosing a server queues a disconnect that is not + /// drained until the next slice, so a client pipelining another select into the same buffer + /// arrives here again; handing back the id it already holds cannot orphan one. + /// + internal static int EnsureAuthId(int existingAuthId, IAccount account, IPAddress address, ClientVersion version) + => existingAuthId != 0 ? existingAuthId : RegisterAuthId(account, address, version); + + internal static int RegisterAuthId(IAccount account, IPAddress address, ClientVersion version) { - if (_authIDWindow.Count == _authIDWindowSize) + // Sweep the ids left behind by clients that picked a server and never arrived, but never + // evict a live one to make room -- the client holding it is on its way to redeem it. If all + // are live the window grows, which is a login rush, not a backlog. Each entry costs a + // successful password verify, so the size is self-limiting. + if (_authIDWindow.Count >= _authIdPurgeThreshold) { - var oldestID = 0; - var oldest = DateTime.MaxValue; - - foreach (var (key, authId) in _authIDWindow) - { - if (authId.Age < oldest) - { - oldestID = key; - oldest = authId.Age; - } - } - - _authIDWindow.Remove(oldestID); + PurgeExpiredAuthIds(); + _authIdPurgeThreshold = Math.Max(_authIDWindowSize, _authIDWindow.Count * 2); } int authID; + // The id stands in for a password verify, so it has to be unguessable. Zero is reserved: + // GameLogin reads state.AuthId == 0 as "no auth id was issued". do { - authID = Utility.Random(1, int.MaxValue - 1); + authID = RandomNumberGenerator.GetInt32(int.MinValue, int.MaxValue); + } while (authID == 0 || _authIDWindow.ContainsKey(authID)); - if (Utility.RandomBool()) - { - authID |= 1 << 31; - } - } while (_authIDWindow.ContainsKey(authID)); - - _authIDWindow[authID] = new AuthIDPersistence(state.Version); + _authIDWindow[authID] = new AuthIDPersistence(version, account, address); return authID; } + /// + /// Spends an auth id, but only for the account and address it was issued to. An address + /// mismatch is rather than a fallback: network switching + /// mid-login is not supported. + /// + internal static AuthIdResult ConsumeAuthId(int authId, string username, IPAddress address, out AuthIDPersistence entry) + { + if (!_authIDWindow.TryGetValue(authId, out entry)) + { + return AuthIdResult.Rejected; + } + + // Look, then take: removing before ownership is proven would let anyone landing on a live id + // burn it, leaving its owner to log in again. Address before username, so a remote guesser + // never learns whether a username matched. + if (!Utility.Intern(address).Equals(entry.Address) + || entry.Account == null || !username.InsensitiveEquals(entry.Account.Username)) + { + entry = default; + return AuthIdResult.Rejected; + } + + // Theirs, so spend it. Expired counts as spent; it has done all it is ever going to do. + _authIDWindow.Remove(authId); + + return Core.Now - entry.Age > _authIDLifetime ? AuthIdResult.Expired : AuthIdResult.Vouched; + } + + private static void PurgeExpiredAuthIds() + { + var now = Core.Now; + + foreach (var (key, entry) in _authIDWindow) + { + if (now - entry.Age > _authIDLifetime) + { + _authIDWindow.Remove(key); + } + } + } + + internal static void ClearAuthIdWindow() + { + _authIDWindow.Clear(); + _authIdPurgeThreshold = _authIDWindowSize; + } + + internal static int AuthIdWindowCount => _authIDWindow.Count; + public static void GameLogin(NetState state, SpanReader reader) { if (state.SentFirstPacket) @@ -360,12 +442,6 @@ public static class IncomingAccountPackets var authId = reader.ReadInt32(); - if (!_authIDWindow.TryGetValue(authId, out var ap)) - { - state.LogInfo("Invalid client detected, disconnecting..."); - state.Disconnect("Unable to find auth id."); - } - if (state.AuthId != 0 && authId != state.AuthId || state.AuthId == 0 && authId != state.Seed) { state.LogInfo("Invalid client detected, disconnecting..."); @@ -373,14 +449,28 @@ public static class IncomingAccountPackets return; } - _authIDWindow.Remove(authId); - state.Version = ap.Version; - state.Seeded = true; - var username = reader.ReadLatin1Safe(30); var password = reader.ReadLatin1Safe(30); - var e = new GameServer.GameLoginEventArgs(state, username, password); + var authResult = ConsumeAuthId(authId, username, state.Address, out var ap); + + if (authResult == AuthIdResult.Rejected) + { + state.LogInfo("Invalid client detected, disconnecting..."); + state.Disconnect("Unable to find auth id."); + return; + } + + state.Version = ap.Version; + state.Seeded = true; + + // Expired carries a usable entry; only the password verify skip is withheld. + var e = new GameServer.GameLoginEventArgs( + state, + username, + password, + authResult == AuthIdResult.Vouched + ); GameServer.GameServerLoginEvent(e); @@ -402,6 +492,14 @@ public static class IncomingAccountPackets public static void PlayServer(NetState state, SpanReader reader) { + // A server is picked once per connection. Picking again hands back an id this connection may + // already have spent on a game login, which the client could never redeem. + if (state.AuthId != 0) + { + state.Disconnect("Duplicate play server packet sent."); + return; + } + int index = reader.ReadInt16(); var info = state.ServerInfo; var a = state.Account; @@ -414,7 +512,7 @@ public static class IncomingAccountPackets { var si = info[index]; - state.AuthId = GenerateAuthID(state); + state.AuthId = state.GenerateAuthID(); state.SentFirstPacket = false; state.SendPlayServerAck(si, state.AuthId); @@ -423,6 +521,14 @@ public static class IncomingAccountPackets public static void LoginServerSeed(NetState state, SpanReader reader) { + // Seeding happens once per connection. A second one restarts a handshake this connection + // already completed, which no real client does. + if (state.Seeded) + { + state.Disconnect("Duplicate login server seed packet sent."); + return; + } + state.Seed = reader.ReadInt32(); state.Seeded = true;