From 9416e8f1e29f1321f5733f097c79ec4542f4e418 Mon Sep 17 00:00:00 2001 From: Lucas Nicodemus Date: Mon, 22 Nov 2021 10:26:57 -0800 Subject: [PATCH] Remove DIY password hashing crypto The old system for hashing passwords and permitting users to select their algorithm has been deprecated and phased out since 2015. This removes the remaining functions for hashing passwords to clear the way for .NET5/6 and for OTAPI 3. In 211b70ca3771fb42c5284d5a8565e7a2bad1ac3a, I allowed blank passwords to upgrade to bcrypt hashes. However, the minimum password length has been 4 historically for a long time. So I don't actually assume a lot of users have blank passwords, so I think there are very few, if any of the old hashes laying around. So therefore, I think this is pretty much safe to merge. --- CHANGELOG.md | 1 + TShockAPI/Configuration/TShockConfig.cs | 5 -- TShockAPI/DB/UserManager.cs | 72 +------------------------ 3 files changed, 3 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdacb30e..9dad9c7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This is the rolling changelog for TShock for Terraria. Use past tense when addin ## Upcoming changes * Fixed the `/respawn` command to permit respawning players from the console. (@hakusaro, @Kojirremer) +* Removed the old password hashing system, which predated `bcrypt` hashes and allowed specifying the hash algorithm in the config file. This also removes the config option for setting the hash algorithm (`HashAlgorithm`). This is because it helps clear the way for .NET5/6 and OTAPI 3, and because `bcrypt` has been the default since TShock 4.3 in 2015. (@hakusaro) ## TShock 4.5.6 * Updated Linux guide. (@NezbednikSK) diff --git a/TShockAPI/Configuration/TShockConfig.cs b/TShockAPI/Configuration/TShockConfig.cs index 18499f87..eeb658c1 100644 --- a/TShockAPI/Configuration/TShockConfig.cs +++ b/TShockAPI/Configuration/TShockConfig.cs @@ -368,11 +368,6 @@ namespace TShockAPI.Configuration [Description("The minimum password length for new user accounts. Can never be lower than 4.")] public int MinimumPasswordLength = 4; - /// The hash algorithm used to encrypt user passwords. - /// Valid types: "sha512", "sha256" and "md5". Append with "-xp" for the xp supported algorithms. - [Description("The hash algorithm used to encrypt user passwords. Valid types: \"sha512\", \"sha256\" and \"md5\". Append with \"-xp\" for the xp supported algorithms.")] - public string HashAlgorithm = "sha512"; - /// Determines the BCrypt work factor to use. If increased, all passwords will be upgraded to new work-factor on verify. /// The number of computational rounds is 2^n. Increase with caution. Range: 5-31. [Description("Determines the BCrypt work factor to use. If increased, all passwords will be upgraded to new work-factor on verify. The number of computational rounds is 2^n. Increase with caution. Range: 5-31.")] diff --git a/TShockAPI/DB/UserManager.cs b/TShockAPI/DB/UserManager.cs index 49f6b73c..e2af578b 100644 --- a/TShockAPI/DB/UserManager.cs +++ b/TShockAPI/DB/UserManager.cs @@ -438,7 +438,7 @@ namespace TShockAPI.DB { try { - if (BCrypt.Net.BCrypt.Verify(password, Password)) + if (BCrypt.Net.BCrypt.Verify(password, Password)) { // If necessary, perform an upgrade to the highest work factor. UpgradePasswordWorkFactor(password); @@ -447,35 +447,12 @@ namespace TShockAPI.DB } catch (SaltParseException) { - if (String.Equals(HashPassword(password), Password, StringComparison.InvariantCultureIgnoreCase)) - { - // The password is not stored using BCrypt; upgrade it to BCrypt immediately - UpgradePasswordToBCrypt(password); - return true; - } + TShock.Log.ConsoleError("Error: Unable to verify the password hash for user {0} ({1})", Name, ID); return false; } return false; } - /// Upgrades a password to BCrypt, from an insecure hashing algorithm. - /// The raw user account password (unhashed) to upgrade - protected void UpgradePasswordToBCrypt(string password) - { - // Save the old password, in the event that we have to revert changes. - string oldpassword = Password; - - try - { - TShock.UserAccounts.SetUserAccountPassword(this, password); - } - catch (UserAccountManagerException e) - { - TShock.Log.ConsoleError(e.ToString()); - Password = oldpassword; // Revert changes - } - } - /// Upgrades a password to the highest work factor available in the config. /// The raw user account password (unhashed) to upgrade protected void UpgradePasswordWorkFactor(string password) @@ -536,51 +513,6 @@ namespace TShockAPI.DB Password = BCrypt.Net.BCrypt.HashPassword(password.Trim(), workFactor); } - /// - /// A dictionary of hashing algorithms and an implementation object. - /// - protected readonly Dictionary> HashTypes = new Dictionary> - { - {"sha512", () => new SHA512Managed()}, - {"sha256", () => new SHA256Managed()}, - {"md5", () => new MD5Cng()}, - {"sha512-xp", () => SHA512.Create()}, - {"sha256-xp", () => SHA256.Create()}, - {"md5-xp", () => MD5.Create()}, - }; - - /// - /// Returns a hashed string for a given string based on the config file's hash algo - /// - /// bytes to hash - /// string hash - protected string HashPassword(byte[] bytes) - { - if (bytes == null) - throw new NullReferenceException("bytes"); - Func func; - if (!HashTypes.TryGetValue(TShock.Config.Settings.HashAlgorithm.ToLower(), out func)) - throw new NotSupportedException("Hashing algorithm {0} is not supported".SFormat(TShock.Config.Settings.HashAlgorithm.ToLower())); - - using (var hash = func()) - { - var ret = hash.ComputeHash(bytes); - return ret.Aggregate("", (s, b) => s + b.ToString("X2")); - } - } - - /// - /// Returns a hashed string for a given string based on the config file's hash algo - /// - /// string to hash - /// string hash - protected string HashPassword(string password) - { - if (string.IsNullOrEmpty(password) && Password == "non-existant password") - return "non-existant password"; - return HashPassword(Encoding.UTF8.GetBytes(password)); - } - #region IEquatable /// Indicates whether the current is equal to another .