Minor fixes re:PR870, added null checks from PR876

5 files changed, 124 insertions, 98 deletions

diff --git a/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs b/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs
index 4f2bc1b03..7817989e7 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<string> SupportedHashMethods = new List<string>();

+        private HashSet<string> 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<string>

+            SupportedHashMethods = new HashSet<string>()

             {

                "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 db359d7dd..b3d457342 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);

+                }

+            }

+

+        }
+
         /// <summary>
         /// Save a user in the repo
         /// </summary>
diff --git a/Emby.Server.Implementations/Library/DefaultAuthenticationProvider.cs b/Emby.Server.Implementations/Library/DefaultAuthenticationProvider.cs
index 255fd8252..ca6217016 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<ProviderAuthenticationResult> 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<bool> 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));
         }
 
         /// <summary>
@@ -184,8 +157,6 @@ namespace Emby.Server.Implementations.Library
         /// </summary>
         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 a139c4e73..b8777a480 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 524484b10..cd61657c1 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<byte> Bytes = new List<byte>();
-            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;

         }