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 | ```
This commit is contained in:
parent
fdf8c5cf23
commit
0ce2a62a76
3 changed files with 105 additions and 87 deletions
|
|
@ -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<Char", false)] // Contains forbidden character
|
||||
[InlineData("EndsWithSpace ", false)] // Ends with space
|
||||
[InlineData("EndsWithPeriod.", false)] // Ends with period
|
||||
[InlineData(" StartsWithSpace", false)] // Starts with space
|
||||
[InlineData("ValidUser123", true)] // Standard Username
|
||||
[InlineData("Valid.User", true)] // Valid with period
|
||||
[InlineData("ValidUser!@#", true)] // Contains valid special characters
|
||||
public void IsValidUsername_ValidatesCorrectly(string username, bool expected)
|
||||
{
|
||||
Assert.Equal(expected, AccountHandler.IsValidUsername(username));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("", false)] // Empty password
|
||||
[InlineData("Invalid\x01Char", false)] // Contains invalid ASCII character
|
||||
[InlineData("ValidPass123!", true)] // Standard Password
|
||||
[InlineData(" ", true)] // Single space
|
||||
[InlineData("ValidPass!@#", true)] // Valid special characters
|
||||
[InlineData("ValidPassWithLength1234567890", true)] // Long valid password
|
||||
public void IsValidPassword_ValidatesCorrectly(string password, bool expected)
|
||||
{
|
||||
Assert.Equal(expected, AccountHandler.IsValidPassword(password));
|
||||
}
|
||||
}
|
||||
|
|
@ -17,30 +17,30 @@ public class PasswordProtectionTest
|
|||
[InlineData(typeof(HashAlgorithmPasswordProtection), "SHA2")]
|
||||
public void TestValidates(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.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,
|
||||
};
|
||||
}
|
||||
}
|
||||
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"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<IPAddress, int> m_IPTable;
|
||||
|
||||
private static char[] m_ForbiddenChars = { '<', '>', ':', '"', '/', '\\', '|', '?', '*' };
|
||||
private static readonly SearchValues<char> 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<char> 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<char> 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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue