Merge pull request #870 from LogicalPhallacy/betterauth

Better default authentication

Emby.Server.Implementations/Cryptography/CryptographyProvider.cs

@@ -1,13 +1,49 @@
 using System;
+using System.Collections.Generic;
+using System.Globalization;
 using System.IO;
 using System.Security.Cryptography;
 using System.Text;
+using System.Linq;
 using MediaBrowser.Model.Cryptography;
 
 namespace Emby.Server.Implementations.Cryptography
 {
     public class CryptographyProvider : ICryptoProvider
     {
+        private static readonly HashSet<string> _supportedHashMethods = new HashSet<string>()
+            {
+                "MD5",
+                "System.Security.Cryptography.MD5",
+                "SHA",
+                "SHA1",
+                "System.Security.Cryptography.SHA1",
+                "SHA256",
+                "SHA-256",
+                "System.Security.Cryptography.SHA256",
+                "SHA384",
+                "SHA-384",
+                "System.Security.Cryptography.SHA384",
+                "SHA512",
+                "SHA-512",
+                "System.Security.Cryptography.SHA512"
+            };
+
+        public string DefaultHashMethod => "PBKDF2";
+
+        private RandomNumberGenerator _randomNumberGenerator;
+
+        private const int _defaultIterations = 1000;
+
+        public CryptographyProvider()
+        {
+            //FIXME: When we get DotNet Standard 2.1 we need to revisit how we do the crypto
+            //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
+            //Please note the default method of PBKDF2 is not included, it cannot be used to generate hashes cleanly as it is actually a pbkdf with sha1
+            _randomNumberGenerator = RandomNumberGenerator.Create();
+        }
+
         public Guid GetMD5(string str)
         {
             return new Guid(ComputeMD5(Encoding.Unicode.GetBytes(str)));
@@ -36,5 +72,98 @@ namespace Emby.Server.Implementations.Cryptography
                 return provider.ComputeHash(bytes);
             }
         }
+
+        public IEnumerable<string> GetSupportedHashMethods()
+        {
+            return _supportedHashMethods;
+        }
+
+        private byte[] PBKDF2(string method, byte[] bytes, byte[] salt, int iterations)
+        {
+            //downgrading for now as we need this library to be dotnetstandard compliant
+            //with this downgrade we'll add a check to make sure we're on the downgrade method at the moment
+            if (method == DefaultHashMethod)
+            {
+                using (var r = new Rfc2898DeriveBytes(bytes, salt, iterations))
+                {
+                    return r.GetBytes(32);
+                }
+            }
+
+            throw new CryptographicException($"Cannot currently use PBKDF2 with requested hash method: {method}");
+        }
+
+        public byte[] ComputeHash(string hashMethod, byte[] bytes)
+        {
+            return ComputeHash(hashMethod, bytes, Array.Empty<byte>());
+        }
+
+        public byte[] ComputeHashWithDefaultMethod(byte[] bytes)
+        {
+            return ComputeHash(DefaultHashMethod, bytes);
+        }
+
+        public byte[] ComputeHash(string hashMethod, byte[] bytes, byte[] salt)
+        {
+            if (hashMethod == DefaultHashMethod)
+            {
+                return PBKDF2(hashMethod, bytes, salt, _defaultIterations);
+            }
+            else if (_supportedHashMethods.Contains(hashMethod))
+            {
+                using (var h = HashAlgorithm.Create(hashMethod))
+                {
+                    if (salt.Length == 0)
+                    {
+                        return h.ComputeHash(bytes);
+                    }
+                    else
+                    {
+                        byte[] salted = new byte[bytes.Length + salt.Length];
+                        Array.Copy(bytes, salted, bytes.Length);
+                        Array.Copy(salt, 0, salted, bytes.Length, salt.Length);
+                        return h.ComputeHash(salted);
+                    }
+                }
+            }
+            else
+            {
+                throw new CryptographicException($"Requested hash method is not supported: {hashMethod}");
+            }
+        }
+
+        public byte[] ComputeHashWithDefaultMethod(byte[] bytes, byte[] salt)
+        {
+            return PBKDF2(DefaultHashMethod, bytes, salt, _defaultIterations);
+        }
+        
+        public byte[] ComputeHash(PasswordHash hash)
+        {
+            int iterations = _defaultIterations;
+            if (!hash.Parameters.ContainsKey("iterations"))
+            {
+                hash.Parameters.Add("iterations", _defaultIterations.ToString(CultureInfo.InvariantCulture));
+            }
+            else
+            {
+                try
+                {
+                    iterations = int.Parse(hash.Parameters["iterations"]);
+                }
+                catch (Exception e)
+                {
+                    throw new InvalidDataException($"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[64];
+            _randomNumberGenerator.GetBytes(salt);
+            return salt;
+        }
     }
 }

Emby.Server.Implementations/Data/SqliteUserRepository.cs

@@ -55,6 +55,8 @@ namespace Emby.Server.Implementations.Data
                 {
                     TryMigrateToLocalUsersTable(connection);
                 }
+
+                RemoveEmptyPasswordHashes();
             }
         }
 
@@ -73,6 +75,38 @@ 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", StringComparison.Ordinal)
+                    || !string.Equals(user.Password, "$SHA1$DA39A3EE5E6B4B0D3255BFEF95601890AFD80709", StringComparison.Ordinal))
+                {
+                    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>

Emby.Server.Implementations/Library/DefaultAuthenticationProvider.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Linq;
 using System.Text;
 using System.Threading.Tasks;
 using MediaBrowser.Controller.Authentication;
@@ -18,20 +19,64 @@ namespace Emby.Server.Implementations.Library
         public string Name => "Default";
 
         public bool IsEnabled => true;
-
+        
+        // This is dumb and an artifact of the backwards way auth providers were designed.
+        // This version of authenticate was never meant to be called, but needs to be here for interface compat
+        // Only the providers that don't provide local user support use this
         public Task<ProviderAuthenticationResult> Authenticate(string username, string password)
         {
             throw new NotImplementedException();
         }
-
+        
+        // This is the verson that we need to use for local users. Because reasons.
         public Task<ProviderAuthenticationResult> Authenticate(string username, string password, User resolvedUser)
         {
+            bool success = false;
             if (resolvedUser == null)
             {
                 throw new Exception("Invalid username or password");
             }
 
-            var success = string.Equals(GetPasswordHash(resolvedUser), GetHashedString(resolvedUser, password), StringComparison.OrdinalIgnoreCase);
+            // As long as jellyfin supports passwordless users, we need this little block here to accomodate
+            if (IsPasswordEmpty(resolvedUser, password))
+            {
+                return Task.FromResult(new ProviderAuthenticationResult
+                {
+                    Username = username
+                });
+            }
+
+            ConvertPasswordFormat(resolvedUser);
+            byte[] passwordbytes = Encoding.UTF8.GetBytes(password);
+
+            PasswordHash readyHash = new PasswordHash(resolvedUser.Password);
+            byte[] calculatedHash;
+            string calculatedHashString;
+            if (_cryptographyProvider.GetSupportedHashMethods().Contains(readyHash.Id))
+            {
+                if (string.IsNullOrEmpty(readyHash.Salt))
+                {
+                    calculatedHash = _cryptographyProvider.ComputeHash(readyHash.Id, passwordbytes);
+                    calculatedHashString = BitConverter.ToString(calculatedHash).Replace("-", string.Empty);
+                }
+                else
+                {
+                    calculatedHash = _cryptographyProvider.ComputeHash(readyHash.Id, passwordbytes, readyHash.SaltBytes);
+                    calculatedHashString = BitConverter.ToString(calculatedHash).Replace("-", string.Empty);
+                }
+
+                if (calculatedHashString == readyHash.Hash)
+                {
+                    success = true;
+                    // throw new Exception("Invalid username or password");
+                }
+            }
+            else
+            {
+                throw new Exception(string.Format($"Requested crypto method not available in provider: {readyHash.Id}"));
+            }
+
+            // var success = string.Equals(GetPasswordHash(resolvedUser), GetHashedString(resolvedUser, password), StringComparison.OrdinalIgnoreCase);
 
             if (!success)
             {
@@ -44,46 +89,86 @@ namespace Emby.Server.Implementations.Library
             });
         }
 
+        // This allows us to move passwords forward to the newformat without breaking. They are still insecure, unsalted, and dumb before a password change
+        // but at least they are in the new format.
+        private void ConvertPasswordFormat(User user)
+        {
+            if (string.IsNullOrEmpty(user.Password))
+            {
+                return;
+            }
+
+            if (!user.Password.Contains("$"))
+            {
+                string hash = user.Password;
+                user.Password = string.Format("$SHA1${0}", hash);
+            }
+            
+            if (user.EasyPassword != null && !user.EasyPassword.Contains("$"))
+            {
+                string hash = user.EasyPassword;
+                user.EasyPassword = string.Format("$SHA1${0}", hash);
+            }
+        }
+
         public Task<bool> HasPassword(User user)
         {
             var hasConfiguredPassword = !IsPasswordEmpty(user, GetPasswordHash(user));
             return Task.FromResult(hasConfiguredPassword);
         }
 
-        private bool IsPasswordEmpty(User user, string passwordHash)
+        private bool IsPasswordEmpty(User user, string password)
         {
-            return string.Equals(passwordHash, GetEmptyHashedString(user), StringComparison.OrdinalIgnoreCase);
+            return (string.IsNullOrEmpty(user.Password) && string.IsNullOrEmpty(password));
         }
 
         public Task ChangePassword(User user, string newPassword)
         {
-            string newPasswordHash = null;
-
-            if (newPassword != null)
+            ConvertPasswordFormat(user);
+            // This is needed to support changing a no password user to a password user
+            if (string.IsNullOrEmpty(user.Password))
             {
-                newPasswordHash = GetHashedString(user, newPassword);
+                PasswordHash newPasswordHash = new PasswordHash(_cryptographyProvider);
+                newPasswordHash.SaltBytes = _cryptographyProvider.GenerateSalt();
+                newPasswordHash.Salt = PasswordHash.ConvertToByteString(newPasswordHash.SaltBytes);
+                newPasswordHash.Id = _cryptographyProvider.DefaultHashMethod;
+                newPasswordHash.Hash = GetHashedStringChangeAuth(newPassword, newPasswordHash);
+                user.Password = newPasswordHash.ToString();
+                return Task.CompletedTask;
             }
 
-            if (string.IsNullOrWhiteSpace(newPasswordHash))
+            PasswordHash passwordHash = new PasswordHash(user.Password);
+            if (passwordHash.Id == "SHA1" && string.IsNullOrEmpty(passwordHash.Salt))
             {
-                throw new ArgumentNullException(nameof(newPasswordHash));
+                passwordHash.SaltBytes = _cryptographyProvider.GenerateSalt();
+                passwordHash.Salt = PasswordHash.ConvertToByteString(passwordHash.SaltBytes);
+                passwordHash.Id = _cryptographyProvider.DefaultHashMethod;
+                passwordHash.Hash = GetHashedStringChangeAuth(newPassword, passwordHash);
+            }
+            else if (newPassword != null)
+            {
+                passwordHash.Hash = GetHashedString(user, newPassword);
             }
 
-            user.Password = newPasswordHash;
+            if (string.IsNullOrWhiteSpace(passwordHash.Hash))
+            {
+                throw new ArgumentNullException(nameof(passwordHash.Hash));
+            }
+
+            user.Password = passwordHash.ToString();
 
             return Task.CompletedTask;
         }
 
         public string GetPasswordHash(User user)
         {
-            return string.IsNullOrEmpty(user.Password)
-                ? GetEmptyHashedString(user)
-                : user.Password;
+            return user.Password;
         }
 
-        public string GetEmptyHashedString(User user)
+        public string GetHashedStringChangeAuth(string newPassword, PasswordHash passwordHash)
         {
-            return GetHashedString(user, string.Empty);
+            passwordHash.HashBytes = Encoding.UTF8.GetBytes(newPassword);
+            return PasswordHash.ConvertToByteString(_cryptographyProvider.ComputeHash(passwordHash));
         }
 
         /// <summary>
@@ -91,14 +176,28 @@ namespace Emby.Server.Implementations.Library
         /// </summary>
         public string GetHashedString(User user, string str)
         {
-            var salt = user.Salt;
-            if (salt != null)
+            PasswordHash passwordHash;
+            if (string.IsNullOrEmpty(user.Password))
             {
-                // return BCrypt.HashPassword(str, salt);
+                passwordHash = new PasswordHash(_cryptographyProvider);
+            }
+            else
+            {
+                ConvertPasswordFormat(user);
+                passwordHash = new PasswordHash(user.Password);
             }
 
-            // legacy
-            return BitConverter.ToString(_cryptographyProvider.ComputeSHA1(Encoding.UTF8.GetBytes(str))).Replace("-", string.Empty);
+            if (passwordHash.SaltBytes != null)
+            {
+                // 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
+            {
+                // 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)));
+            }
         }
     }
 }

Emby.Server.Implementations/Library/UserManager.cs

@@ -4,6 +4,7 @@ using System.Globalization;
 using System.IO;
 using System.Linq;
 using System.Text;
+using System.Text.RegularExpressions;
 using System.Threading;
 using System.Threading.Tasks;
 using MediaBrowser.Common.Events;
@@ -213,22 +214,17 @@ namespace Emby.Server.Implementations.Library
             }
         }
 
-        public bool IsValidUsername(string username)
+        public static bool IsValidUsername(string username)
         {
-            // Usernames can contain letters (a-z), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)
-            foreach (var currentChar in username)
-            {
-                if (!IsValidUsernameCharacter(currentChar))
-                {
-                    return false;
-                }
-            }
-            return true;
+            //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
+            // Usernames can contain letters (a-z + whatever else unicode is cool with), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)
+            return Regex.IsMatch(username, "^[\\w-'._@]*$");
         }
 
         private static bool IsValidUsernameCharacter(char i)
         {
-            return !char.Equals(i, '<') && !char.Equals(i, '>');
+            return IsValidUsername(i.ToString());
         }
 
         public string MakeValidUsername(string username)
@@ -475,15 +471,10 @@ namespace Emby.Server.Implementations.Library
         private string GetLocalPasswordHash(User user)
         {
             return string.IsNullOrEmpty(user.EasyPassword)
-                ? _defaultAuthenticationProvider.GetEmptyHashedString(user)
+                ? null
                 : user.EasyPassword;
         }
 
-        private bool IsPasswordEmpty(User user, string passwordHash)
-        {
-            return string.Equals(passwordHash, _defaultAuthenticationProvider.GetEmptyHashedString(user), StringComparison.OrdinalIgnoreCase);
-        }
-
         /// <summary>
         /// Loads the users from the repository
         /// </summary>
@@ -526,14 +517,14 @@ namespace Emby.Server.Implementations.Library
                 throw new ArgumentNullException(nameof(user));
             }
 
-            var hasConfiguredPassword = GetAuthenticationProvider(user).HasPassword(user).Result;
-            var hasConfiguredEasyPassword = !IsPasswordEmpty(user, GetLocalPasswordHash(user));
+            bool hasConfiguredPassword = GetAuthenticationProvider(user).HasPassword(user).Result;
+            bool hasConfiguredEasyPassword = string.IsNullOrEmpty(GetLocalPasswordHash(user));
 
-            var hasPassword = user.Configuration.EnableLocalPassword && !string.IsNullOrEmpty(remoteEndPoint) && _networkManager.IsInLocalNetwork(remoteEndPoint) ?
+            bool hasPassword = user.Configuration.EnableLocalPassword && !string.IsNullOrEmpty(remoteEndPoint) && _networkManager.IsInLocalNetwork(remoteEndPoint) ?
                 hasConfiguredEasyPassword :
                 hasConfiguredPassword;
 
-            var dto = new UserDto
+            UserDto dto = new UserDto
             {
                 Id = user.Id,
                 Name = user.Name,
@@ -552,7 +543,7 @@ namespace Emby.Server.Implementations.Library
                 dto.EnableAutoLogin = true;
             }
 
-            var image = user.GetImageInfo(ImageType.Primary, 0);
+            ItemImageInfo image = user.GetImageInfo(ImageType.Primary, 0);
 
             if (image != null)
             {
@@ -688,7 +679,7 @@ namespace Emby.Server.Implementations.Library
 
             if (!IsValidUsername(name))
             {
-                throw new ArgumentException("Usernames can contain letters (a-z), numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)");
+                throw new ArgumentException("Usernames can contain unicode symbols, numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)");
             }
 
             if (Users.Any(u => u.Name.Equals(name, StringComparison.OrdinalIgnoreCase)))

MediaBrowser.Model/Cryptography/ICryptoProvider.cs

@@ -1,5 +1,6 @@
 using System;
 using System.IO;
+using System.Collections.Generic;
 
 namespace MediaBrowser.Model.Cryptography
 {
@@ -9,5 +10,13 @@ namespace MediaBrowser.Model.Cryptography
         byte[] ComputeMD5(Stream str);
         byte[] ComputeMD5(byte[] bytes);
         byte[] ComputeSHA1(byte[] bytes);
+        IEnumerable<string> GetSupportedHashMethods();
+        byte[] ComputeHash(string HashMethod, byte[] bytes);
+        byte[] ComputeHashWithDefaultMethod(byte[] bytes);
+        byte[] ComputeHash(string HashMethod, byte[] bytes, byte[] salt);
+        byte[] ComputeHashWithDefaultMethod(byte[] bytes, byte[] salt);
+        byte[] ComputeHash(PasswordHash hash);
+        byte[] GenerateSalt();
+        string DefaultHashMethod { get; }
     }
 }

MediaBrowser.Model/Cryptography/PasswordHash.cs

@@ -0,0 +1,153 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+namespace MediaBrowser.Model.Cryptography
+{
+    public class PasswordHash
+    {
+        // Defined from this hash storage spec
+        // https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md
+        // $<id>[$<param>=<value>(,<param>=<value>)*][$<salt>[$<hash>]]
+        // with one slight amendment to ease the transition, we're writing out the bytes in hex
+        // rather than making them a BASE64 string with stripped padding
+
+        private string _id;
+
+        private Dictionary<string, string> _parameters = new Dictionary<string, string>();
+
+        private string _salt;
+
+        private byte[] _saltBytes;
+
+        private string _hash;
+
+        private byte[] _hashBytes;
+
+        public string Id { get => _id; set => _id = value; }
+
+        public Dictionary<string, string> Parameters { get => _parameters; set => _parameters = value; }
+
+        public string Salt { get => _salt; set => _salt = value; }
+
+        public byte[] SaltBytes { get => _saltBytes; set => _saltBytes = value; }
+
+        public string Hash { get => _hash; set => _hash = value; }
+
+        public byte[] HashBytes { get => _hashBytes; set => _hashBytes = value; }
+
+        public PasswordHash(string storageString)
+        {
+            string[] splitted = storageString.Split('$');
+            _id = splitted[1];
+            if (splitted[2].Contains("="))
+            {
+                foreach (string paramset in (splitted[2].Split(',')))
+                {
+                    if (!string.IsNullOrEmpty(paramset))
+                    {
+                        string[] fields = paramset.Split('=');
+                        if (fields.Length == 2)
+                        {
+                            _parameters.Add(fields[0], fields[1]);
+                        }
+                        else
+                        {
+                            throw new Exception($"Malformed parameter in password hash string {paramset}");
+                        }
+                    }
+                }
+                if (splitted.Length == 5)
+                {
+                    _salt = splitted[3];
+                    _saltBytes = ConvertFromByteString(_salt);
+                    _hash = splitted[4];
+                    _hashBytes = ConvertFromByteString(_hash);
+                }
+                else
+                {
+                    _salt = string.Empty;
+                    _hash = splitted[3];
+                    _hashBytes = ConvertFromByteString(_hash);
+                }
+            }
+            else
+            {
+                if (splitted.Length == 4)
+                {
+                    _salt = splitted[2];
+                    _saltBytes = ConvertFromByteString(_salt);
+                    _hash = splitted[3];
+                    _hashBytes = ConvertFromByteString(_hash);
+                }
+                else
+                {
+                    _salt = string.Empty;
+                    _hash = splitted[2];
+                    _hashBytes = ConvertFromByteString(_hash);
+                }
+
+            }
+
+        }
+
+        public PasswordHash(ICryptoProvider cryptoProvider)
+        {
+            _id = cryptoProvider.DefaultHashMethod;
+            _saltBytes = cryptoProvider.GenerateSalt();
+            _salt = ConvertToByteString(SaltBytes);
+        }
+
+        public static byte[] ConvertFromByteString(string byteString)
+        {
+            byte[] bytes = new byte[byteString.Length / 2];
+            for (int i = 0; i < byteString.Length; i += 2)
+            {
+                // TODO: NetStandard2.1 switch this to use a span instead of a substring.
+                bytes[i / 2] = Convert.ToByte(byteString.Substring(i, 2), 16);
+            }
+
+            return bytes;
+        }
+
+        public static string ConvertToByteString(byte[] bytes)
+        {
+            return BitConverter.ToString(bytes).Replace("-", "");
+        }
+
+        private string SerializeParameters()
+        {
+            string returnString = string.Empty;
+            foreach (var KVP in _parameters)
+            {
+                returnString += $",{KVP.Key}={KVP.Value}";
+            }
+
+            if ((!string.IsNullOrEmpty(returnString)) && returnString[0] == ',')
+            {
+                returnString = returnString.Remove(0, 1);
+            }
+
+            return returnString;
+        }
+
+        public override string ToString()
+        {
+            string outString = "$" + _id;
+            string paramstring = SerializeParameters();
+            if (!string.IsNullOrEmpty(paramstring))
+            {
+                outString += $"${paramstring}";
+            }
+
+            if (!string.IsNullOrEmpty(_salt))
+            {
+                outString += $"${_salt}";
+            }
+
+            outString += $"${_hash}";
+            return outString;
+        }
+    }
+
+}