From 77602aff889e605f8178ecf95592c0d75102e59f Mon Sep 17 00:00:00 2001 From: Phallacy Date: Wed, 13 Feb 2019 00:33:00 -0800 Subject: [PATCH] Minor fixes re:PR870, added null checks from PR876 --- .../Cryptography/CryptographyProvider.cs | 38 ++++++---- .../Data/SqliteUserRepository.cs | 32 ++++++++ .../Library/DefaultAuthenticationProvider.cs | 71 ++++++------------ .../Library/UserManager.cs | 6 +- .../Cryptography/PasswordHash.cs | 75 +++++++++++-------- 5 files changed, 124 insertions(+), 98 deletions(-) diff --git a/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs b/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs index 4f2bc1b030..7817989e7f 100644 --- a/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs +++ b/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs @@ -9,7 +9,7 @@ namespace Emby.Server.Implementations.Cryptography { public class CryptographyProvider : ICryptoProvider { - private List SupportedHashMethods = new List(); + private HashSet SupportedHashMethods; public string DefaultHashMethod => "SHA256"; private RandomNumberGenerator rng; private int defaultiterations = 1000; @@ -17,7 +17,7 @@ namespace Emby.Server.Implementations.Cryptography { //Currently supported hash methods from https://docs.microsoft.com/en-us/dotnet/api/system.security.cryptography.cryptoconfig?view=netcore-2.1 //there might be a better way to autogenerate this list as dotnet updates, but I couldn't find one - SupportedHashMethods = new List + SupportedHashMethods = new HashSet() { "MD5" ,"System.Security.Cryptography.MD5" @@ -71,9 +71,9 @@ namespace Emby.Server.Implementations.Cryptography return SupportedHashMethods; } - private byte[] PBKDF2(string method, byte[] bytes, byte[] salt) - { - using (var r = new Rfc2898DeriveBytes(bytes, salt, defaultiterations, new HashAlgorithmName(method))) + private byte[] PBKDF2(string method, byte[] bytes, byte[] salt, int iterations) + { + using (var r = new Rfc2898DeriveBytes(bytes, salt, iterations, new HashAlgorithmName(method))) { return r.GetBytes(32); } @@ -102,30 +102,40 @@ namespace Emby.Server.Implementations.Cryptography } else { - return PBKDF2(HashMethod, bytes, salt); + return PBKDF2(HashMethod, bytes, salt,defaultiterations); } } else { throw new CryptographicException(String.Format("Requested hash method is not supported: {0}", HashMethod)); } - } + } public byte[] ComputeHashWithDefaultMethod(byte[] bytes, byte[] salt) { - return PBKDF2(DefaultHashMethod, bytes, salt); + return PBKDF2(DefaultHashMethod, bytes, salt, defaultiterations); } public byte[] ComputeHash(PasswordHash hash) - { - return ComputeHash(hash.Id, hash.HashBytes, hash.SaltBytes); - } - + { + int iterations = defaultiterations; + if (!hash.Parameters.ContainsKey("iterations")) + { + hash.Parameters.Add("iterations", defaultiterations.ToString()); + } + else + { + try { iterations = int.Parse(hash.Parameters["iterations"]); } + catch (Exception e) { iterations = defaultiterations; throw new Exception($"Couldn't successfully parse iterations value from string:{hash.Parameters["iterations"]}", e); } + } + return PBKDF2(hash.Id, hash.HashBytes, hash.SaltBytes,iterations); + } + public byte[] GenerateSalt() { - byte[] salt = new byte[8]; + byte[] salt = new byte[64]; rng.GetBytes(salt); return salt; - } + } } } diff --git a/Emby.Server.Implementations/Data/SqliteUserRepository.cs b/Emby.Server.Implementations/Data/SqliteUserRepository.cs index db359d7ddc..b3d4573427 100644 --- a/Emby.Server.Implementations/Data/SqliteUserRepository.cs +++ b/Emby.Server.Implementations/Data/SqliteUserRepository.cs @@ -55,6 +55,7 @@ namespace Emby.Server.Implementations.Data { TryMigrateToLocalUsersTable(connection); } + RemoveEmptyPasswordHashes(); } } @@ -73,6 +74,37 @@ namespace Emby.Server.Implementations.Data } } + private void RemoveEmptyPasswordHashes() + { + foreach (var user in RetrieveAllUsers()) + { + // If the user password is the sha1 hash of the empty string, remove it + if (!string.Equals(user.Password, "DA39A3EE5E6B4B0D3255BFEF95601890AFD80709") || !string.Equals(user.Password, "$SHA1$DA39A3EE5E6B4B0D3255BFEF95601890AFD80709")) + { + continue; + } + + user.Password = null; + var serialized = _jsonSerializer.SerializeToBytes(user); + + using (WriteLock.Write()) + using (var connection = CreateConnection()) + { + connection.RunInTransaction(db => + { + using (var statement = db.PrepareStatement("update LocalUsersv2 set data=@data where Id=@InternalId")) + { + statement.TryBind("@InternalId", user.InternalId); + statement.TryBind("@data", serialized); + statement.MoveNext(); + } + + }, TransactionMode); + } + } + + } + /// /// Save a user in the repo /// diff --git a/Emby.Server.Implementations/Library/DefaultAuthenticationProvider.cs b/Emby.Server.Implementations/Library/DefaultAuthenticationProvider.cs index 255fd82527..ca6217016b 100644 --- a/Emby.Server.Implementations/Library/DefaultAuthenticationProvider.cs +++ b/Emby.Server.Implementations/Library/DefaultAuthenticationProvider.cs @@ -36,32 +36,27 @@ namespace Emby.Server.Implementations.Library bool success = false; if (resolvedUser == null) { - success = false; throw new Exception("Invalid username or password"); } ConvertPasswordFormat(resolvedUser); byte[] passwordbytes = Encoding.UTF8.GetBytes(password); - - if (!resolvedUser.Password.Contains("$")) - { - ConvertPasswordFormat(resolvedUser); - } - PasswordHash ReadyHash = new PasswordHash(resolvedUser.Password); + + PasswordHash readyHash = new PasswordHash(resolvedUser.Password); byte[] CalculatedHash; string CalculatedHashString; - if (_cryptographyProvider.GetSupportedHashMethods().Any(i => i == ReadyHash.Id)) + if (_cryptographyProvider.GetSupportedHashMethods().Any(i => i == readyHash.Id)) { - if (String.IsNullOrEmpty(ReadyHash.Salt)) + if (String.IsNullOrEmpty(readyHash.Salt)) { - CalculatedHash = _cryptographyProvider.ComputeHash(ReadyHash.Id, passwordbytes); + CalculatedHash = _cryptographyProvider.ComputeHash(readyHash.Id, passwordbytes); CalculatedHashString = BitConverter.ToString(CalculatedHash).Replace("-", string.Empty); } else { - CalculatedHash = _cryptographyProvider.ComputeHash(ReadyHash.Id, passwordbytes, ReadyHash.SaltBytes); + CalculatedHash = _cryptographyProvider.ComputeHash(readyHash.Id, passwordbytes, readyHash.SaltBytes); CalculatedHashString = BitConverter.ToString(CalculatedHash).Replace("-", string.Empty); } - if (CalculatedHashString == ReadyHash.Hash) + if (CalculatedHashString == readyHash.Hash) { success = true; //throw new Exception("Invalid username or password"); @@ -69,8 +64,7 @@ namespace Emby.Server.Implementations.Library } else { - success = false; - throw new Exception(String.Format("Requested crypto method not available in provider: {0}", ReadyHash.Id)); + throw new Exception(String.Format("Requested crypto method not available in provider: {0}", readyHash.Id)); } //var success = string.Equals(GetPasswordHash(resolvedUser), GetHashedString(resolvedUser, password), StringComparison.OrdinalIgnoreCase); @@ -105,26 +99,6 @@ namespace Emby.Server.Implementations.Library } } - // OLD VERSION //public Task Authenticate(string username, string password, User resolvedUser) - // OLD VERSION //{ - // OLD VERSION // if (resolvedUser == null) - // OLD VERSION // { - // OLD VERSION // throw new Exception("Invalid username or password"); - // OLD VERSION // } - // OLD VERSION // - // OLD VERSION // var success = string.Equals(GetPasswordHash(resolvedUser), GetHashedString(resolvedUser, password), StringComparison.OrdinalIgnoreCase); - // OLD VERSION // - // OLD VERSION // if (!success) - // OLD VERSION // { - // OLD VERSION // throw new Exception("Invalid username or password"); - // OLD VERSION // } - // OLD VERSION // - // OLD VERSION // return Task.FromResult(new ProviderAuthenticationResult - // OLD VERSION // { - // OLD VERSION // Username = username - // OLD VERSION // }); - // OLD VERSION //} - public Task HasPassword(User user) { var hasConfiguredPassword = !IsPasswordEmpty(user, GetPasswordHash(user)); @@ -133,7 +107,7 @@ namespace Emby.Server.Implementations.Library private bool IsPasswordEmpty(User user, string passwordHash) { - return string.Equals(passwordHash, GetEmptyHashedString(user), StringComparison.OrdinalIgnoreCase); + return string.IsNullOrEmpty(passwordHash); } public Task ChangePassword(User user, string newPassword) @@ -144,7 +118,7 @@ namespace Emby.Server.Implementations.Library if(passwordHash.Id == "SHA1" && string.IsNullOrEmpty(passwordHash.Salt)) { passwordHash.SaltBytes = _cryptographyProvider.GenerateSalt(); - passwordHash.Salt = BitConverter.ToString(passwordHash.SaltBytes).Replace("-",""); + passwordHash.Salt = PasswordHash.ConvertToByteString(passwordHash.SaltBytes); passwordHash.Id = _cryptographyProvider.DefaultHashMethod; passwordHash.Hash = GetHashedStringChangeAuth(newPassword, passwordHash); }else if (newPassword != null) @@ -164,19 +138,18 @@ namespace Emby.Server.Implementations.Library public string GetPasswordHash(User user) { - return string.IsNullOrEmpty(user.Password) - ? GetEmptyHashedString(user) - : user.Password; + return user.Password; } public string GetEmptyHashedString(User user) { - return GetHashedString(user, string.Empty); + return null; } - public string GetHashedStringChangeAuth(string NewPassword, PasswordHash passwordHash) + public string GetHashedStringChangeAuth(string newPassword, PasswordHash passwordHash) { - return BitConverter.ToString(_cryptographyProvider.ComputeHash(passwordHash.Id, Encoding.UTF8.GetBytes(NewPassword), passwordHash.SaltBytes)).Replace("-", string.Empty); + passwordHash.HashBytes = Encoding.UTF8.GetBytes(newPassword); + return PasswordHash.ConvertToByteString(_cryptographyProvider.ComputeHash(passwordHash)); } /// @@ -184,8 +157,6 @@ namespace Emby.Server.Implementations.Library /// public string GetHashedString(User user, string str) { - //This is legacy. Deprecated in the auth method. - //return BitConverter.ToString(_cryptoProvider2.ComputeSHA1(Encoding.UTF8.GetBytes(str))).Replace("-", string.Empty); PasswordHash passwordHash; if (String.IsNullOrEmpty(user.Password)) { @@ -197,13 +168,15 @@ namespace Emby.Server.Implementations.Library passwordHash = new PasswordHash(user.Password); } if (passwordHash.SaltBytes != null) - { - return BitConverter.ToString(_cryptographyProvider.ComputeHash(passwordHash.Id, Encoding.UTF8.GetBytes(str), passwordHash.SaltBytes)).Replace("-",string.Empty); + { + //the password is modern format with PBKDF and we should take advantage of that + passwordHash.HashBytes = Encoding.UTF8.GetBytes(str); + return PasswordHash.ConvertToByteString(_cryptographyProvider.ComputeHash(passwordHash)); } else - { - return BitConverter.ToString(_cryptographyProvider.ComputeHash(passwordHash.Id, Encoding.UTF8.GetBytes(str))).Replace("-", string.Empty); - //throw new Exception("User does not have a hash, this should not be possible"); + { + //the password has no salt and should be called with the older method for safety + return PasswordHash.ConvertToByteString(_cryptographyProvider.ComputeHash(passwordHash.Id, Encoding.UTF8.GetBytes(str))); } diff --git a/Emby.Server.Implementations/Library/UserManager.cs b/Emby.Server.Implementations/Library/UserManager.cs index a139c4e736..b8777a4802 100644 --- a/Emby.Server.Implementations/Library/UserManager.cs +++ b/Emby.Server.Implementations/Library/UserManager.cs @@ -217,9 +217,8 @@ namespace Emby.Server.Implementations.Library } } - public bool IsValidUsername(string username) + public static bool IsValidUsername(string username) { - //The old way was dumb, we should make it less dumb, lets do so. //This is some regex that matches only on unicode "word" characters, as well as -, _ and @ //In theory this will cut out most if not all 'control' characters which should help minimize any weirdness string UserNameRegex = "^[\\w-'._@]*$"; @@ -229,8 +228,7 @@ namespace Emby.Server.Implementations.Library private static bool IsValidUsernameCharacter(char i) { - string UserNameRegex = "^[\\w-'._@]*$"; - return Regex.IsMatch(i.ToString(), UserNameRegex); + return IsValidUsername(i.ToString()); } public string MakeValidUsername(string username) diff --git a/MediaBrowser.Model/Cryptography/PasswordHash.cs b/MediaBrowser.Model/Cryptography/PasswordHash.cs index 524484b107..cd61657c18 100644 --- a/MediaBrowser.Model/Cryptography/PasswordHash.cs +++ b/MediaBrowser.Model/Cryptography/PasswordHash.cs @@ -16,70 +16,78 @@ namespace MediaBrowser.Model.Cryptography public byte[] SaltBytes; public string Hash; public byte[] HashBytes; - public PasswordHash(string StorageString) + public PasswordHash(string storageString) { - string[] a = StorageString.Split('$'); - Id = a[1]; - if (a[2].Contains("=")) + string[] SplitStorageString = storageString.Split('$'); + Id = SplitStorageString[1]; + if (SplitStorageString[2].Contains("=")) { - foreach (string paramset in (a[2].Split(','))) + foreach (string paramset in (SplitStorageString[2].Split(','))) { if (!String.IsNullOrEmpty(paramset)) { - string[] fields = paramset.Split('='); - Parameters.Add(fields[0], fields[1]); + string[] fields = paramset.Split('='); + if(fields.Length == 2) + { + Parameters.Add(fields[0], fields[1]); + } } } - if (a.Length == 4) + if (SplitStorageString.Length == 5) { - Salt = a[2]; - SaltBytes = FromByteString(Salt); - Hash = a[3]; - HashBytes = FromByteString(Hash); + Salt = SplitStorageString[3]; + SaltBytes = ConvertFromByteString(Salt); + Hash = SplitStorageString[4]; + HashBytes = ConvertFromByteString(Hash); } else { Salt = string.Empty; - Hash = a[3]; - HashBytes = FromByteString(Hash); + Hash = SplitStorageString[3]; + HashBytes = ConvertFromByteString(Hash); } } else { - if (a.Length == 4) + if (SplitStorageString.Length == 4) { - Salt = a[2]; - SaltBytes = FromByteString(Salt); - Hash = a[3]; - HashBytes = FromByteString(Hash); + Salt = SplitStorageString[2]; + SaltBytes = ConvertFromByteString(Salt); + Hash = SplitStorageString[3]; + HashBytes = ConvertFromByteString(Hash); } else { Salt = string.Empty; - Hash = a[2]; - HashBytes = FromByteString(Hash); + Hash = SplitStorageString[2]; + HashBytes = ConvertFromByteString(Hash); } } } - public PasswordHash(ICryptoProvider cryptoProvider2) + public PasswordHash(ICryptoProvider cryptoProvider) { - Id = "SHA256"; - SaltBytes = cryptoProvider2.GenerateSalt(); - Salt = BitConverter.ToString(SaltBytes).Replace("-", ""); + Id = cryptoProvider.DefaultHashMethod; + SaltBytes = cryptoProvider.GenerateSalt(); + Salt = ConvertToByteString(SaltBytes); } - private byte[] FromByteString(string ByteString) + public static byte[] ConvertFromByteString(string byteString) { List Bytes = new List(); - for (int i = 0; i < ByteString.Length; i += 2) + for (int i = 0; i < byteString.Length; i += 2) { - Bytes.Add(Convert.ToByte(ByteString.Substring(i, 2),16)); + Bytes.Add(Convert.ToByte(byteString.Substring(i, 2),16)); } return Bytes.ToArray(); - } + } + public static string ConvertToByteString(byte[] bytes) + { + return BitConverter.ToString(bytes).Replace("-", ""); + } + private string SerializeParameters() { string ReturnString = String.Empty; @@ -98,10 +106,15 @@ namespace MediaBrowser.Model.Cryptography { string OutString = "$"; OutString += Id; - if (!string.IsNullOrEmpty(SerializeParameters())) - OutString += $"${SerializeParameters()}"; + string paramstring = SerializeParameters(); + if (!string.IsNullOrEmpty(paramstring)) + { + OutString += $"${paramstring}"; + } if (!string.IsNullOrEmpty(Salt)) + { OutString += $"${Salt}"; + } OutString += $"${Hash}"; return OutString; }