From 0ce2a62a76c1d9f511f3e6531b0b27e9c00f5e82 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Fri, 11 Apr 2025 22:40:06 -0700 Subject: [PATCH] fix: Use vectorized search for username-password validation. (#2152) ### Summary Optimizes account validation: ```cs | Method | Mean | Error | StdDev | |-------------------- |-----------:|----------:|----------:| | ForLoopUsernameSafe | 158.536 ns | 1.6247 ns | 1.5198 ns | | SVUsernameSafe | 2.859 ns | 0.0581 ns | 0.0796 ns | ``` --- .../Tests/Accounting/AccountHandlerTests.cs | 35 ++++++++ .../Security/PasswordProtectionTest.cs | 90 +++++++++---------- .../UOContent/Accounting/AccountHandler.cs | 67 ++++++-------- 3 files changed, 105 insertions(+), 87 deletions(-) create mode 100644 Projects/UOContent.Tests/Tests/Accounting/AccountHandlerTests.cs diff --git a/Projects/UOContent.Tests/Tests/Accounting/AccountHandlerTests.cs b/Projects/UOContent.Tests/Tests/Accounting/AccountHandlerTests.cs new file mode 100644 index 000000000..429d252eb --- /dev/null +++ b/Projects/UOContent.Tests/Tests/Accounting/AccountHandlerTests.cs @@ -0,0 +1,35 @@ +using Server.Misc; +using Xunit; + +namespace Server.Tests.Accounting; + +public class AccountHandlerTests +{ + [Theory] + [InlineData("", false)] // Empty username + [InlineData(" ", false)] // Single space + [InlineData(".", false)] // Single period + [InlineData("Invalid HashAlgorithmPasswordProtection.SHA1Instance, - "SHA2" => HashAlgorithmPasswordProtection.SHA2Instance, - _ => HashAlgorithmPasswordProtection.MD5Instance, - }; - } - else - { - passwordProtection = Activator.CreateInstance(protectionType) as IPasswordProtection; - } - - if (passwordProtection == null) - { - Assert.Fail($"{protectionType.Name} is not an IPasswordProtection."); - } - - var encryptedPassword = passwordProtection.EncryptPassword(plainPassword); - - Assert.True(passwordProtection.ValidatePassword(encryptedPassword, plainPassword)); + "SHA1" => HashAlgorithmPasswordProtection.SHA1Instance, + "SHA2" => HashAlgorithmPasswordProtection.SHA2Instance, + _ => HashAlgorithmPasswordProtection.MD5Instance, + }; } + else + { + passwordProtection = Activator.CreateInstance(protectionType) as IPasswordProtection; + } + + if (passwordProtection == null) + { + Assert.Fail($"{protectionType.Name} is not an IPasswordProtection."); + } + + var encryptedPassword = passwordProtection.EncryptPassword(plainPassword); + + Assert.True(passwordProtection.ValidatePassword(encryptedPassword, plainPassword)); + } [Theory] [InlineData(typeof(Argon2PasswordProtection), null)] @@ -50,28 +50,28 @@ public class PasswordProtectionTest [InlineData(typeof(HashAlgorithmPasswordProtection), "SHA2")] public void TestPasswordDoesNotValidate(Type protectionType, string algorithmType) { - IPasswordProtection passwordProtection; - if (protectionType == typeof(HashAlgorithmPasswordProtection)) + IPasswordProtection passwordProtection; + if (protectionType == typeof(HashAlgorithmPasswordProtection)) + { + passwordProtection = algorithmType switch { - passwordProtection = algorithmType switch - { - "SHA1" => HashAlgorithmPasswordProtection.SHA1Instance, - "SHA2" => HashAlgorithmPasswordProtection.SHA2Instance, - _ => HashAlgorithmPasswordProtection.MD5Instance, - }; - } - else - { - passwordProtection = Activator.CreateInstance(protectionType) as IPasswordProtection; - } - - if (passwordProtection == null) - { - Assert.Fail($"{protectionType.Name} is not an IPasswordProtection."); - } - - var encryptedPassword = passwordProtection.EncryptPassword(plainPassword); - - Assert.False(passwordProtection.ValidatePassword(encryptedPassword, "Not the same password")); + "SHA1" => HashAlgorithmPasswordProtection.SHA1Instance, + "SHA2" => HashAlgorithmPasswordProtection.SHA2Instance, + _ => HashAlgorithmPasswordProtection.MD5Instance, + }; } -} \ No newline at end of file + else + { + passwordProtection = Activator.CreateInstance(protectionType) as IPasswordProtection; + } + + if (passwordProtection == null) + { + Assert.Fail($"{protectionType.Name} is not an IPasswordProtection."); + } + + var encryptedPassword = passwordProtection.EncryptPassword(plainPassword); + + Assert.False(passwordProtection.ValidatePassword(encryptedPassword, "Not the same password")); + } +} diff --git a/Projects/UOContent/Accounting/AccountHandler.cs b/Projects/UOContent/Accounting/AccountHandler.cs index fce134355..50ee90e81 100644 --- a/Projects/UOContent/Accounting/AccountHandler.cs +++ b/Projects/UOContent/Accounting/AccountHandler.cs @@ -1,6 +1,8 @@ using System; +using System.Buffers; using System.Collections.Generic; using System.Net; +using System.Runtime.CompilerServices; using ModernUO.CodeGeneratedEvents; using Server.Accounting; using Server.Engines.CharacterCreation; @@ -17,13 +19,13 @@ public static class AccountHandler private static int MaxAccountsPerIP; private static bool AutoAccountCreation; - private static bool RestrictDeletion = !TestCenter.Enabled; - private static TimeSpan DeleteDelay = TimeSpan.FromDays(7.0); + private static readonly bool RestrictDeletion = !TestCenter.Enabled; + private static readonly TimeSpan DeleteDelay = TimeSpan.FromDays(7.0); private static bool PasswordCommandEnabled; private static Dictionary m_IPTable; - private static char[] m_ForbiddenChars = { '<', '>', ':', '"', '/', '\\', '|', '?', '*' }; + private static readonly SearchValues ForbiddenChars = SearchValues.Create("<>:\"/\\|?*"); public static AccessLevel LockdownLevel { get; set; } @@ -240,41 +242,24 @@ public static class AccountHandler public static bool CanCreate(IPAddress ip) => !IPTable.TryGetValue(ip, out var result) || result < MaxAccountsPerIP; - private static bool IsForbiddenChar(char c) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool IsValidUsername(ReadOnlySpan username) => + username.Length > 0 && + // Usernames must not start with a space, end with a space, or end with a period + !username.StartsWith(' ') && !username.EndsWith(' ') && !username.EndsWith('.') && + // Usernames must only contain characters [0x20 -> 0x7E], and not contain any forbidden characters + !username.ContainsAnyExceptInRange((char)0x20, (char)0x7E) && + !username.ContainsAny(ForbiddenChars); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool IsValidPassword(ReadOnlySpan password) => + password.Length > 0 && + // Passwords must have characters [0x20 -> 0x7E] + !password.ContainsAnyExceptInRange((char)0x20, (char)0x7E); + + private static Account CreateAccount(NetState state, string username, string password) { - for (var i = 0; i < m_ForbiddenChars.Length; ++i) - { - if (c == m_ForbiddenChars[i]) - { - return true; - } - } - - return false; - } - - private static Account CreateAccount(NetState state, string un, string pw) - { - if (un.Length == 0 || pw.Length == 0) - { - return null; - } - - var isSafe = !(un.StartsWithOrdinal(" ") || - un.EndsWithOrdinal(" ") || - un.EndsWithOrdinal(".")); - - for (var i = 0; isSafe && i < un.Length; ++i) - { - isSafe = un[i] >= 0x20 && un[i] < 0x7F && !IsForbiddenChar(un[i]); - } - - for (var i = 0; isSafe && i < pw.Length; ++i) - { - isSafe = pw[i] >= 0x20 && pw[i] < 0x7F; - } - - if (!isSafe) + if (!IsValidUsername(username) || !IsValidPassword(password)) { return null; } @@ -284,18 +269,16 @@ public static class AccountHandler logger.Information( $"Login: {{NetState}} Account '{{Username}}' not created, ip already has {{AccountCount}} account{(MaxAccountsPerIP == 1 ? "" : "s")}.", state, - un, + username, MaxAccountsPerIP ); return null; } - logger.Information("Login: {NetState}: Creating new account '{Username}'", state, un); + logger.Information("Login: {NetState}: Creating new account '{Username}'", state, username); - var a = new Account(un, pw); - - return a; + return new Account(username, password); } public static void EventSink_AccountLogin(AccountLoginEventArgs e)