From 29467a3ed92ee967c870763a7e73d526ba8d3bdd Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Thu, 30 Apr 2020 02:33:14 -0700 Subject: [PATCH] Adds PBKDF2 protection for account passwords. (#122) --- .../PBKDF2PasswordProtectionTest.cs | 30 +++ .../Misc/HexStringConverterTest.cs | 20 ++ Projects/Scripts.Tests/Scripts.Tests.csproj | 4 + Projects/Scripts/Accounting/Account.cs | 184 ++---------------- Projects/Scripts/Accounting/AccountHandler.cs | 9 - .../Scripts/Accounting/AccountSecurity.cs | 26 +++ .../Scripts/Accounting/IPasswordProtection.cs | 8 + .../Accounting/PBKDF2PasswordProtection.cs | 46 +++++ Projects/Scripts/Misc/HexStringConverter.cs | 54 +++++ 9 files changed, 209 insertions(+), 172 deletions(-) create mode 100644 Projects/Scripts.Tests/Accounting/PBKDF2PasswordProtectionTest.cs create mode 100644 Projects/Scripts.Tests/Misc/HexStringConverterTest.cs create mode 100644 Projects/Scripts/Accounting/AccountSecurity.cs create mode 100644 Projects/Scripts/Accounting/IPasswordProtection.cs create mode 100644 Projects/Scripts/Accounting/PBKDF2PasswordProtection.cs create mode 100644 Projects/Scripts/Misc/HexStringConverter.cs diff --git a/Projects/Scripts.Tests/Accounting/PBKDF2PasswordProtectionTest.cs b/Projects/Scripts.Tests/Accounting/PBKDF2PasswordProtectionTest.cs new file mode 100644 index 000000000..1b1ec46bf --- /dev/null +++ b/Projects/Scripts.Tests/Accounting/PBKDF2PasswordProtectionTest.cs @@ -0,0 +1,30 @@ +using Xunit; +using Server.Accounting; + +namespace Server.Tests.Accounting +{ + public class PBKDF2PasswordProtectionTest + { + private const string plainPassword = "hello-good-sir"; + + [Fact] + public void TestValidates() + { + var passwordProtection = new PBKDF2PasswordProtection(); + + string encryptedPassword = passwordProtection.EncryptPassword(plainPassword); + + Assert.True(passwordProtection.ValidatePassword(encryptedPassword, plainPassword)); + } + + [Fact] + public void TestPasswordDoesNotValidate() + { + var passwordProtection = new PBKDF2PasswordProtection(); + + string encryptedPassword = passwordProtection.EncryptPassword(plainPassword); + + Assert.False(passwordProtection.ValidatePassword(encryptedPassword, "Not the same password")); + } + } +} diff --git a/Projects/Scripts.Tests/Misc/HexStringConverterTest.cs b/Projects/Scripts.Tests/Misc/HexStringConverterTest.cs new file mode 100644 index 000000000..9ed3b73b1 --- /dev/null +++ b/Projects/Scripts.Tests/Misc/HexStringConverterTest.cs @@ -0,0 +1,20 @@ +using System; +using Xunit; +using Server.Misc; + +namespace Server.Tests.Accounting +{ + public class HexStringConverterTest + { + [Theory] + [InlineData("ABCDEF1234", new byte[]{ 0xAB, 0xCD, 0xEF, 0x12, 0x34 })] + public void ConvertsProperly(string input, byte[] bytes) + { + Span outputBytes = stackalloc byte[input.Length / 2]; + HexStringConverter.GetBytes(input, outputBytes); + + Assert.Equal(bytes, outputBytes.ToArray()); + Assert.Equal(input, HexStringConverter.GetString(bytes)); + } + } +} diff --git a/Projects/Scripts.Tests/Scripts.Tests.csproj b/Projects/Scripts.Tests/Scripts.Tests.csproj index 7b6f91feb..d67207c07 100644 --- a/Projects/Scripts.Tests/Scripts.Tests.csproj +++ b/Projects/Scripts.Tests/Scripts.Tests.csproj @@ -9,4 +9,8 @@ + + + + diff --git a/Projects/Scripts/Accounting/Account.cs b/Projects/Scripts/Accounting/Account.cs index 622d8d3cc..1252c8855 100644 --- a/Projects/Scripts/Accounting/Account.cs +++ b/Projects/Scripts/Accounting/Account.cs @@ -1,8 +1,6 @@ using System; using System.Collections.Generic; using System.Net; -using System.Security.Cryptography; -using System.Text; using System.Xml; using Server.Misc; using Server.Mobiles; @@ -14,9 +12,7 @@ namespace Server.Accounting public class Account : IAccount, IComparable { public static readonly TimeSpan YoungDuration = TimeSpan.FromHours(40.0); - public static readonly TimeSpan InactiveDuration = TimeSpan.FromDays(180.0); - public static readonly TimeSpan EmptyInactiveDuration = TimeSpan.FromDays(30.0); private AccessLevel m_AccessLevel; @@ -24,6 +20,7 @@ namespace Server.Accounting private List m_Comments; private List m_Tags; private readonly Mobile[] m_Mobiles; + private PasswordProtectionAlgorithm m_PasswordAlgorithm; /// /// Deletes the account, all characters of the account, and all houses of those characters @@ -90,19 +87,9 @@ namespace Server.Accounting public string Email { get; set; } /// - /// Account password. Plain text. Case sensitive validation. May be null. + /// Account username and password. May be null. /// - public string PlainPassword { get; set; } - - /// - /// Account password. Hashed with MD5. May be null. - /// - public string CryptPassword { get; set; } - - /// - /// Account username and password hashed with SHA1. May be null. - /// - public string NewCryptPassword { get; set; } + public string Password { get; set; } /// /// Initial AccessLevel for new characters created on this account. @@ -328,94 +315,23 @@ namespace Server.Accounting return banTime != DateTime.MinValue && banDuration != TimeSpan.Zero; } - private static MD5CryptoServiceProvider m_MD5HashProvider; - private static SHA1CryptoServiceProvider m_SHA1HashProvider; - private static byte[] m_HashBuffer; - - public static string HashMD5(string phrase) - { - if (m_MD5HashProvider == null) - m_MD5HashProvider = new MD5CryptoServiceProvider(); - - if (m_HashBuffer == null) - m_HashBuffer = new byte[256]; - - int length = Encoding.ASCII.GetBytes(phrase, 0, phrase.Length > 256 ? 256 : phrase.Length, m_HashBuffer, 0); - byte[] hashed = m_MD5HashProvider.ComputeHash(m_HashBuffer, 0, length); - - return BitConverter.ToString(hashed); - } - - public static string HashSHA1(string phrase) - { - if (m_SHA1HashProvider == null) - m_SHA1HashProvider = new SHA1CryptoServiceProvider(); - - if (m_HashBuffer == null) - m_HashBuffer = new byte[256]; - - int length = Encoding.ASCII.GetBytes(phrase, 0, phrase.Length > 256 ? 256 : phrase.Length, m_HashBuffer, 0); - byte[] hashed = m_SHA1HashProvider.ComputeHash(m_HashBuffer, 0, length); - - return BitConverter.ToString(hashed); - } - public void SetPassword(string plainPassword) { - switch (AccountHandler.ProtectPasswords) - { - case PasswordProtection.None: - { - PlainPassword = plainPassword; - CryptPassword = null; - NewCryptPassword = null; - - break; - } - case PasswordProtection.Crypt: - { - PlainPassword = null; - CryptPassword = HashMD5(plainPassword); - NewCryptPassword = null; - - break; - } - default: // PasswordProtection.NewCrypt - { - PlainPassword = null; - CryptPassword = null; - NewCryptPassword = HashSHA1(Username + plainPassword); - - break; - } - } + Password = AccountSecurity.CurrentPasswordProtection.EncryptPassword(plainPassword); + m_PasswordAlgorithm = AccountSecurity.AlgorithmName; } public bool CheckPassword(string plainPassword) { - bool ok; - PasswordProtection curProt; + bool ok = AccountSecurity.GetPasswordProtection(m_PasswordAlgorithm).ValidatePassword(Password, plainPassword); + if (!ok) + return false; - if (PlainPassword != null) - { - ok = PlainPassword == plainPassword; - curProt = PasswordProtection.None; - } - else if (CryptPassword != null) - { - ok = CryptPassword == HashMD5(plainPassword); - curProt = PasswordProtection.Crypt; - } - else - { - ok = NewCryptPassword == HashSHA1(Username + plainPassword); - curProt = PasswordProtection.NewCrypt; - } - - if (ok && curProt != AccountHandler.ProtectPasswords) + // Upgrade the password protection in case we change the algorithm + if (m_PasswordAlgorithm != AccountSecurity.AlgorithmName) SetPassword(plainPassword); - return ok; + return true; } private Timer m_YoungTimer; @@ -539,53 +455,10 @@ namespace Server.Accounting { Username = Utility.GetText(node["username"], "empty"); - string plainPassword = Utility.GetText(node["password"], null); - string cryptPassword = Utility.GetText(node["cryptPassword"], null); - string newCryptPassword = Utility.GetText(node["newCryptPassword"], null); - - switch (AccountHandler.ProtectPasswords) - { - case PasswordProtection.None: - { - if (plainPassword != null) - SetPassword(plainPassword); - else if (newCryptPassword != null) - NewCryptPassword = newCryptPassword; - else if (cryptPassword != null) - CryptPassword = cryptPassword; - else - SetPassword("empty"); - - break; - } - case PasswordProtection.Crypt: - { - if (cryptPassword != null) - CryptPassword = cryptPassword; - else if (plainPassword != null) - SetPassword(plainPassword); - else if (newCryptPassword != null) - NewCryptPassword = newCryptPassword; - else - SetPassword("empty"); - - break; - } - default: // PasswordProtection.NewCrypt - { - if (newCryptPassword != null) - NewCryptPassword = newCryptPassword; - else if (plainPassword != null) - SetPassword(plainPassword); - else if (cryptPassword != null) - CryptPassword = cryptPassword; - else - SetPassword("empty"); - - break; - } - } - + // Note: ModernUO doesn't support plain passwords, MD5, or SHA1. + // TODO: Offload passwords to its own module so it can be easily written/upgraded + Password = Utility.GetText(node["password"], null); + Enum.TryParse(Utility.GetText(node["passwordAlgorithm"], null), true, out m_PasswordAlgorithm); Enum.TryParse(Utility.GetText(node["accessLevel"], "Player"), true, out m_AccessLevel); Flags = Utility.GetXMLInt32(Utility.GetText(node["flags"], "0"), 0); Created = Utility.GetXMLDateTime(Utility.GetText(node["created"], null), DateTime.UtcNow); @@ -883,26 +756,13 @@ namespace Server.Accounting xml.WriteString(Username); xml.WriteEndElement(); - if (PlainPassword != null) - { - xml.WriteStartElement("password"); - xml.WriteString(PlainPassword); - xml.WriteEndElement(); - } + xml.WriteStartElement("passwordAlgorithm"); + xml.WriteString(m_PasswordAlgorithm.ToString()); + xml.WriteEndElement(); - if (CryptPassword != null) - { - xml.WriteStartElement("cryptPassword"); - xml.WriteString(CryptPassword); - xml.WriteEndElement(); - } - - if (NewCryptPassword != null) - { - xml.WriteStartElement("newCryptPassword"); - xml.WriteString(NewCryptPassword); - xml.WriteEndElement(); - } + xml.WriteStartElement("password"); + xml.WriteString(Password); + xml.WriteEndElement(); if (m_AccessLevel != AccessLevel.Player) { @@ -932,8 +792,6 @@ namespace Server.Accounting xml.WriteStartElement("chars"); - // xml.WriteAttributeString( "length", m_Mobiles.Length.ToString() ); //Legacy, Not used anymore - for (int i = 0; i < m_Mobiles.Length; ++i) { Mobile m = m_Mobiles[i]; diff --git a/Projects/Scripts/Accounting/AccountHandler.cs b/Projects/Scripts/Accounting/AccountHandler.cs index 24896a617..1e9412f17 100644 --- a/Projects/Scripts/Accounting/AccountHandler.cs +++ b/Projects/Scripts/Accounting/AccountHandler.cs @@ -9,13 +9,6 @@ using Server.Regions; namespace Server.Misc { - public enum PasswordProtection - { - None, - Crypt, - NewCrypt - } - public class AccountHandler { private static readonly int MaxAccountsPerIP = 1; @@ -23,8 +16,6 @@ namespace Server.Misc private static readonly bool RestrictDeletion = !TestCenter.Enabled; private static readonly TimeSpan DeleteDelay = TimeSpan.FromDays(7.0); - public static PasswordProtection ProtectPasswords = PasswordProtection.NewCrypt; - private static readonly CityInfo[] StartingCities = { new CityInfo("New Haven", "New Haven Bank", 1150168, 3667, 2625, 0), diff --git a/Projects/Scripts/Accounting/AccountSecurity.cs b/Projects/Scripts/Accounting/AccountSecurity.cs new file mode 100644 index 000000000..0e14425ea --- /dev/null +++ b/Projects/Scripts/Accounting/AccountSecurity.cs @@ -0,0 +1,26 @@ +namespace Server.Accounting +{ + public enum PasswordProtectionAlgorithm + { + PBKDF2 + } + + public static class AccountSecurity + { + // TODO: Put it in a configuration + public const PasswordProtectionAlgorithm AlgorithmName = PasswordProtectionAlgorithm.PBKDF2; + + public static readonly IPasswordProtection CurrentPasswordProtection = GetPasswordProtection(AlgorithmName); + + public static IPasswordProtection GetPasswordProtection(PasswordProtectionAlgorithm algorithm) + { + var passwordProtection = algorithm switch + { + PasswordProtectionAlgorithm.PBKDF2 => PBKDF2PasswordProtection.Instance, + _ => null + }; + + return passwordProtection; + } + } +} diff --git a/Projects/Scripts/Accounting/IPasswordProtection.cs b/Projects/Scripts/Accounting/IPasswordProtection.cs new file mode 100644 index 000000000..20c25fe2a --- /dev/null +++ b/Projects/Scripts/Accounting/IPasswordProtection.cs @@ -0,0 +1,8 @@ +namespace Server.Accounting +{ + public interface IPasswordProtection + { + string EncryptPassword(string plainPassword); + bool ValidatePassword(string encryptedPassword, string plainPassword); + } +} diff --git a/Projects/Scripts/Accounting/PBKDF2PasswordProtection.cs b/Projects/Scripts/Accounting/PBKDF2PasswordProtection.cs new file mode 100644 index 000000000..da0944e15 --- /dev/null +++ b/Projects/Scripts/Accounting/PBKDF2PasswordProtection.cs @@ -0,0 +1,46 @@ +using System; +using System.Buffers.Binary; +using System.Security.Cryptography; +using Server.Misc; + +namespace Server.Accounting +{ + public class PBKDF2PasswordProtection : IPasswordProtection + { + public static PBKDF2PasswordProtection Instance = new PBKDF2PasswordProtection(); + + private const ushort m_MinIterations = 1024; + private const ushort m_MaxIterations = 1536; + private static readonly HashAlgorithmName m_Algorithm = HashAlgorithmName.SHA256; + private const int m_SaltSize = 8; + private const int m_HashSize = 32; + private const int m_OutputSize = 2 + m_SaltSize + m_HashSize; + + public string EncryptPassword(string plainPassword) + { + Span output = stackalloc byte[m_OutputSize]; + int iterations = Utility.RandomMinMax(m_MinIterations, m_MaxIterations); + BinaryPrimitives.WriteUInt16LittleEndian(output.Slice(0, 2), (ushort)iterations); + + var rfc2898 = new Rfc2898DeriveBytes(plainPassword, m_SaltSize, iterations, m_Algorithm); + rfc2898.Salt.CopyTo(output.Slice(2, m_SaltSize)); + rfc2898.GetBytes(m_HashSize).CopyTo(output.Slice(m_SaltSize + 2)); + + return HexStringConverter.GetString(output); + } + + public bool ValidatePassword(string encryptedPassword, string plainPassword) + { + Span encryptedBytes = stackalloc byte[m_OutputSize]; + HexStringConverter.GetBytes(encryptedPassword, encryptedBytes); + + ushort iterations = BinaryPrimitives.ReadUInt16LittleEndian(encryptedBytes.Slice(0, 2)); + Span salt = encryptedBytes.Slice(2, m_SaltSize); + + ReadOnlySpan hash = + new Rfc2898DeriveBytes(plainPassword, salt.ToArray(), iterations, m_Algorithm).GetBytes(m_HashSize); + + return hash.SequenceEqual(encryptedBytes.Slice(m_SaltSize + 2)); + } + } +} diff --git a/Projects/Scripts/Misc/HexStringConverter.cs b/Projects/Scripts/Misc/HexStringConverter.cs new file mode 100644 index 000000000..1bf58847e --- /dev/null +++ b/Projects/Scripts/Misc/HexStringConverter.cs @@ -0,0 +1,54 @@ +using System; + +namespace Server.Misc +{ + public class HexStringConverter + { + public static readonly uint[] m_Lookup32Chars = CreateLookup32Chars(); + + private static uint[] CreateLookup32Chars() + { + var result = new uint[256]; + for (int i = 0; i < 256; i++) + { + string s = i.ToString("X2"); + if (BitConverter.IsLittleEndian) + result[i] = s[0] + ((uint)s[1] << 16); + else + result[i] = s[1] + ((uint)s[0] << 16); + } + + return result; + } + + public static unsafe string GetString(ReadOnlySpan bytes) + { + var result = new string((char)0, bytes.Length * 2); + fixed (char* resultP = result) + { + uint* resultP2 = (uint*)resultP; + for (int i = 0; i < bytes.Length; i++) + resultP2[i] = m_Lookup32Chars[bytes[i]]; + } + return result; + } + + public static unsafe void GetBytes(string str, Span bytes) + { + fixed (char* strP = str) + { + int i = 0; + int j = 0; + while (i < str.Length) + { + int chr1 = strP[i++]; + int chr2 = strP[i++]; + if (BitConverter.IsLittleEndian) + bytes[j++] = (byte)((chr1 - (chr1 >= 65 ? 55 : 48)) << 4 | (chr2 - (chr2 >= 65 ? 55 : 48))); + else + bytes[j++] = (byte)((chr1 - (chr1 >= 65 ? 55 : 48)) | ((chr2 - (chr2 >= 65 ? 55 : 48)) << 4)); + } + } + } + } +}