warrentyexpired/ModernUO
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(network): expire auth ids into a verify, and reclaim abandoned ones

Browse source 
Two problems with the first cut.

An expired id disconnected. Before any of this existed the game login always
verified the password, so that turned a slow player into a lockout for no
gain. A player can sit on the server list as long as they like; expiry now
falls back to the password verify that was always there. An id for the wrong
account or the wrong address is a different matter -- that client is not ours,
and there is nothing there worth checking a password against.

Ids are only consumed by a game login, so anyone who reaches the server list
and never picks a server abandons theirs. Nothing reclaimed those: the lifetime
was only consulted on lookup. 128 abandoned logins filled the window and the
eviction path then started discarding ids that live clients were still on their
way to redeem. Issuing now reclaims expired entries before evicting a live one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Kamron Batman 2026-08-08 00:35:36 -07:00
parent dd64e972b6
commit e5b7161a71
No known key found for this signature in database
GPG key ID: 7D81DF26D9A5D94A
2 changed files with 142 additions and 15 deletions
Download patch file Download diff file Expand all files Collapse all files

105
Projects/UOContent.Tests/Tests/Network/Packets/AuthIdTests.cs
View file

@ -47,16 +47,14 @@ public class AuthIdTests : IDisposable
}
[Fact]
public void DoesNotVouchForADifferentAccount()
public void RejectsADifferentAccount()
{
var issued = CreateAccount("authid-owner-user");
var other = CreateAccount("authid-other-user");
var authId = Register(issued, AddressX);
// AccountMismatch, not Rejected: the caller must still be able to accept this login by
// verifying the password, exactly as it did before pre-authentication existed.
Assert.Equal(
IncomingAccountPackets.AuthIdResult.AccountMismatch,
IncomingAccountPackets.AuthIdResult.Rejected,
IncomingAccountPackets.ConsumeAuthId(authId, other.Username, AddressX, out _)
);
}
@ -132,17 +130,112 @@ public class AuthIdTests : IDisposable
var authId = Register(account, AddressX);
Assert.Equal(
IncomingAccountPackets.AuthIdResult.AccountMismatch,
IncomingAccountPackets.AuthIdResult.Rejected,
IncomingAccountPackets.ConsumeAuthId(authId, "not-the-owner", AddressX, out _)
);
// Spent regardless, so a guessed id cannot be reused to enumerate usernames.
// Spent regardless, so a guessed id cannot be reused to probe for its owner.
Assert.Equal(
IncomingAccountPackets.AuthIdResult.Rejected,
IncomingAccountPackets.ConsumeAuthId(authId, account.Username, AddressX, out _)
);
}
/// <summary>
/// An expired id is not a lockout. A player can sit on the server list, and before any of this
/// existed the game login always verified the password anyway -- so falling back to that verify
/// is the behaviour we started from, not a regression.
/// </summary>
[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;
}
}
/// <summary>
/// Ids are only consumed by a game login, so a player who reaches the server list and never
/// picks a server abandons theirs. Enough of those must not push out ids that live clients are
/// still on their way to redeem.
/// </summary>
[Fact]
public void AbandonedIdsAreReclaimedBeforeALiveOneIsEvicted()
{
var abandoned = CreateAccount("authid-abandoned-user");
var live = CreateAccount("authid-live-user");
var now = Core._now;
try
{
// Fill the window with ids nobody ever redeems.
for (var i = 0; i < 128; i++)
{
Register(abandoned, AddressX);
}
Assert.Equal(128, IncomingAccountPackets.AuthIdWindowCount);
Core._now = now + TimeSpan.FromMinutes(30.0);
// The next issue reclaims the dead entries instead of evicting, so the id it hands out
// is the only one left and is immediately redeemable.
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;
}
}
[Fact]
public void PreAuthenticatedGameLogin_SkipsThePasswordCheck()
{

52
Projects/UOContent/Network/Packets/IncomingAccountPackets.cs
View file

@ -58,14 +58,16 @@ public static class IncomingAccountPackets
internal enum AuthIdResult
{
// No such id, or it expired, or it came from another address. Indistinguishable from forged.
// No such id, or it was issued for a different account or address. A client that presents
// one of those is not one of ours; nothing here is worth a password check.
Rejected,
// Live id from the right address, but for a different account than the one being logged
// into. Proves nothing, so the caller falls back to verifying the password.
AccountMismatch,
// Right account, right address, but too old to stand in for the verify. A player can idle
// on the server list, so this is a normal thing to do -- fall back to checking the password
// rather than turning it into a lockout.
Expired,
// Issued to this account, from this address. Stands in for the password verify.
// Issued to this account, from this address, recently. Stands in for the password verify.
Vouched
}
@ -345,6 +347,15 @@ public static class IncomingAccountPackets
internal static int RegisterAuthId(IAccount account, IPAddress address, ClientVersion version)
{
// Ids are only consumed by a game login, so anyone who reaches the server list and never
// picks a server leaves theirs behind. Reclaim the dead ones before evicting a live one --
// otherwise enough abandoned logins fill the window and start pushing out ids that clients
// are still on their way to redeem.
if (_authIDWindow.Count >= _authIDWindowSize)
{
PurgeExpiredAuthIds();
}
if (_authIDWindow.Count >= _authIDWindowSize)
{
var oldestID = 0;
@ -392,18 +403,39 @@ public static class IncomingAccountPackets
return AuthIdResult.Rejected;
}
if (Core.Now - entry.Age > _authIDLifetime || !Utility.Intern(address).Equals(entry.Address))
// Address before age: a different address is a different client, however fresh the id is.
if (!Utility.Intern(address).Equals(entry.Address))
{
return AuthIdResult.Rejected;
}
return entry.Account != null && username.InsensitiveEquals(entry.Account.Username)
? AuthIdResult.Vouched
: AuthIdResult.AccountMismatch;
if (entry.Account == null || !username.InsensitiveEquals(entry.Account.Username))
{
return AuthIdResult.Rejected;
}
return Core.Now - entry.Age > _authIDLifetime ? AuthIdResult.Expired : AuthIdResult.Vouched;
}
private static void PurgeExpiredAuthIds()
{
var now = Core.Now;
// Removing during enumeration is supported on Dictionary since .NET Core 3.0, so this
// reclaims in one pass with no scratch list.
foreach (var (key, entry) in _authIDWindow)
{
if (now - entry.Age > _authIDLifetime)
{
_authIDWindow.Remove(key);
}
}
}
internal static void ClearAuthIdWindow() => _authIDWindow.Clear();
internal static int AuthIdWindowCount => _authIDWindow.Count;
public static void GameLogin(NetState state, SpanReader reader)
{
if (state.SentFirstPacket)
@ -439,6 +471,8 @@ public static class IncomingAccountPackets
state.Version = ap.Version;
state.Seeded = true;
// Expired still carries a usable entry, and the client just pays the password verify it
// would have paid before any of this existed.
var e = new GameServer.GameLoginEventArgs(
state,
username,