Replace PBKDF2-SHA1 with PBKDF2-SHA512

This also migrates already created passwords on login

Source for the number of iterations:
https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2

9 files changed, 88 insertions, 113 deletions

diff --git a/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs b/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs
index 673810c49..e9c005cea 100644
--- a/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs
+++ b/Emby.Server.Implementations/Cryptography/CryptographyProvider.cs
@@ -1,9 +1,11 @@
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Security.Cryptography;
+using System.Text;
 using MediaBrowser.Common.Extensions;
 using MediaBrowser.Model.Cryptography;
-using static MediaBrowser.Common.Cryptography.Constants;
+using static MediaBrowser.Model.Cryptography.Constants;
 
 namespace Emby.Server.Implementations.Cryptography
 {
@@ -12,10 +14,7 @@ namespace Emby.Server.Implementations.Cryptography
     /// </summary>
     public class CryptographyProvider : ICryptoProvider
     {
-        // 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
+        // TODO: remove when not needed for backwards compat
         private static readonly HashSet<string> _supportedHashMethods = new HashSet<string>()
             {
                 "MD5",
@@ -35,60 +34,81 @@ namespace Emby.Server.Implementations.Cryptography
             };
 
         /// <inheritdoc />
-        public string DefaultHashMethod => "PBKDF2";
+        public string DefaultHashMethod => "PBKDF2-SHA512";
 
         /// <inheritdoc />
-        public IEnumerable<string> GetSupportedHashMethods()
-            => _supportedHashMethods;
-
-        private byte[] PBKDF2(string method, byte[] bytes, byte[] salt, int iterations)
+        public PasswordHash CreatePasswordHash(ReadOnlySpan<char> password)
         {
-            // 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)
-            {
-                throw new CryptographicException($"Cannot currently use PBKDF2 with requested hash method: {method}");
-            }
-
-            using var r = new Rfc2898DeriveBytes(bytes, salt, iterations);
-            return r.GetBytes(32);
+            byte[] salt = GenerateSalt();
+            return new PasswordHash(
+                DefaultHashMethod,
+                Rfc2898DeriveBytes.Pbkdf2(
+                    password,
+                    salt,
+                    DefaultIterations,
+                    HashAlgorithmName.SHA512,
+                    DefaultOutputLength),
+                salt,
+                new Dictionary<string, string>
+                {
+                    { "iterations", DefaultIterations.ToString(CultureInfo.InvariantCulture) }
+                });
         }
 
         /// <inheritdoc />
-        public byte[] ComputeHash(string hashMethod, byte[] bytes, byte[] salt)
+        public bool Verify(PasswordHash hash, ReadOnlySpan<char> password)
         {
-            if (hashMethod == DefaultHashMethod)
+            if (string.Equals(hash.Id, "PBKDF2", StringComparison.Ordinal))
             {
-                return PBKDF2(hashMethod, bytes, salt, DefaultIterations);
+                return hash.Hash.SequenceEqual(
+                    Rfc2898DeriveBytes.Pbkdf2(
+                        password,
+                        hash.Salt,
+                        int.Parse(hash.Parameters["iterations"], CultureInfo.InvariantCulture),
+                        HashAlgorithmName.SHA1,
+                        32));
             }
 
-            if (!_supportedHashMethods.Contains(hashMethod))
+            if (string.Equals(hash.Id, "PBKDF2-SHA512", StringComparison.Ordinal))
             {
-                throw new CryptographicException($"Requested hash method is not supported: {hashMethod}");
+                return hash.Hash.SequenceEqual(
+                    Rfc2898DeriveBytes.Pbkdf2(
+                        password,
+                        hash.Salt,
+                        int.Parse(hash.Parameters["iterations"], CultureInfo.InvariantCulture),
+                        HashAlgorithmName.SHA512,
+                        DefaultOutputLength));
             }
 
-            using var h = HashAlgorithm.Create(hashMethod) ?? throw new ResourceNotFoundException($"Unknown hash method: {hashMethod}.");
-            if (salt.Length == 0)
+            if (!_supportedHashMethods.Contains(hash.Id))
             {
-                return h.ComputeHash(bytes);
+                throw new CryptographicException($"Requested hash method is not supported: {hash.Id}");
             }
 
-            byte[] salted = new byte[bytes.Length + salt.Length];
+            using var h = HashAlgorithm.Create(hash.Id) ?? throw new ResourceNotFoundException($"Unknown hash method: {hash.Id}.");
+            var bytes = Encoding.UTF8.GetBytes(password.ToArray());
+            if (hash.Salt.Length == 0)
+            {
+                return hash.Hash.SequenceEqual(h.ComputeHash(bytes));
+            }
+
+            byte[] salted = new byte[bytes.Length + hash.Salt.Length];
             Array.Copy(bytes, salted, bytes.Length);
-            Array.Copy(salt, 0, salted, bytes.Length, salt.Length);
-            return h.ComputeHash(salted);
+            hash.Salt.CopyTo(salted.AsSpan(bytes.Length));
+            return hash.Hash.SequenceEqual(h.ComputeHash(salted));
         }
 
         /// <inheritdoc />
-        public byte[] ComputeHashWithDefaultMethod(byte[] bytes, byte[] salt)
-            => PBKDF2(DefaultHashMethod, bytes, salt, DefaultIterations);
-
-        /// <inheritdoc />
         public byte[] GenerateSalt()
             => GenerateSalt(DefaultSaltLength);
 
         /// <inheritdoc />
         public byte[] GenerateSalt(int length)
-            => RandomNumberGenerator.GetBytes(length);
+        {
+            var salt = new byte[length];
+            using var rng = RandomNumberGenerator.Create();
+            rng.GetNonZeroBytes(salt);
+            return salt;
+        }
     }
 }
diff --git a/Emby.Server.Implementations/LiveTv/Listings/SchedulesDirect.cs b/Emby.Server.Implementations/LiveTv/Listings/SchedulesDirect.cs
index 1f963e4a2..615539db3 100644
--- a/Emby.Server.Implementations/LiveTv/Listings/SchedulesDirect.cs
+++ b/Emby.Server.Implementations/LiveTv/Listings/SchedulesDirect.cs
@@ -11,6 +11,7 @@ using System.Net;
 using System.Net.Http;
 using System.Net.Http.Headers;
 using System.Net.Mime;
+using System.Security.Cryptography;
 using System.Text;
 using System.Text.Json;
 using System.Threading;
@@ -648,7 +649,7 @@ namespace Emby.Server.Implementations.LiveTv.Listings
             CancellationToken cancellationToken)
         {
             using var options = new HttpRequestMessage(HttpMethod.Post, ApiUrl + "/token");
-            var hashedPasswordBytes = _cryptoProvider.ComputeHash("SHA1", Encoding.ASCII.GetBytes(password), Array.Empty<byte>());
+            var hashedPasswordBytes = SHA1.HashData(Encoding.ASCII.GetBytes(password));
             // TODO: remove ToLower when Convert.ToHexString supports lowercase
             // Schedules Direct requires the hex to be lowercase
             string hashedPassword = Convert.ToHexString(hashedPasswordBytes).ToLowerInvariant();
diff --git a/Jellyfin.Server.Implementations/Users/DefaultAuthenticationProvider.cs b/Jellyfin.Server.Implementations/Users/DefaultAuthenticationProvider.cs
index 6a78e7ee6..7480a05c2 100644
--- a/Jellyfin.Server.Implementations/Users/DefaultAuthenticationProvider.cs
+++ b/Jellyfin.Server.Implementations/Users/DefaultAuthenticationProvider.cs
@@ -1,9 +1,6 @@
 using System;
-using System.Linq;
-using System.Text;
 using System.Threading.Tasks;
 using Jellyfin.Data.Entities;
-using MediaBrowser.Common.Cryptography;
 using MediaBrowser.Controller.Authentication;
 using MediaBrowser.Model.Cryptography;
 
@@ -61,35 +58,25 @@ namespace Jellyfin.Server.Implementations.Users
             }
 
             // Handle the case when the stored password is null, but the user tried to login with a password
-            if (resolvedUser.Password != null)
+            if (resolvedUser.Password == null)
             {
-                byte[] passwordBytes = Encoding.UTF8.GetBytes(password);
-
-                PasswordHash readyHash = PasswordHash.Parse(resolvedUser.Password);
-                if (_cryptographyProvider.GetSupportedHashMethods().Contains(readyHash.Id)
-                    || _cryptographyProvider.DefaultHashMethod == readyHash.Id)
-                {
-                    byte[] calculatedHash = _cryptographyProvider.ComputeHash(
-                        readyHash.Id,
-                        passwordBytes,
-                        readyHash.Salt.ToArray());
-
-                    if (readyHash.Hash.SequenceEqual(calculatedHash))
-                    {
-                        success = true;
-                    }
-                }
-                else
-                {
-                    throw new AuthenticationException($"Requested crypto method not available in provider: {readyHash.Id}");
-                }
+                throw new AuthenticationException("Invalid username or password");
             }
 
+            PasswordHash readyHash = PasswordHash.Parse(resolvedUser.Password);
+            success = _cryptographyProvider.Verify(readyHash, password);
+
             if (!success)
             {
                 throw new AuthenticationException("Invalid username or password");
             }
 
+            // Migrate old hashes to the new default
+            if (!string.Equals(readyHash.Id, _cryptographyProvider.DefaultHashMethod, StringComparison.Ordinal))
+            {
+                ChangePassword(resolvedUser, password);
+            }
+
             return Task.FromResult(new ProviderAuthenticationResult
             {
                 Username = username
diff --git a/Jellyfin.Server.Implementations/Users/UserManager.cs b/Jellyfin.Server.Implementations/Users/UserManager.cs
index 8ca6e8d21..3d0a51ff6 100644
--- a/Jellyfin.Server.Implementations/Users/UserManager.cs
+++ b/Jellyfin.Server.Implementations/Users/UserManager.cs
@@ -5,7 +5,6 @@ using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
-using System.Text;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using Jellyfin.Data.Entities;
@@ -13,7 +12,6 @@ using Jellyfin.Data.Enums;
 using Jellyfin.Data.Events;
 using Jellyfin.Data.Events.Users;
 using MediaBrowser.Common;
-using MediaBrowser.Common.Cryptography;
 using MediaBrowser.Common.Extensions;
 using MediaBrowser.Common.Net;
 using MediaBrowser.Controller.Authentication;
@@ -818,11 +816,7 @@ namespace Jellyfin.Server.Implementations.Users
             {
                 // Check easy password
                 var passwordHash = PasswordHash.Parse(user.EasyPassword);
-                var hash = _cryptoProvider.ComputeHash(
-                    passwordHash.Id,
-                    Encoding.UTF8.GetBytes(password),
-                    passwordHash.Salt.ToArray());
-                success = passwordHash.Hash.SequenceEqual(hash);
+                success = _cryptoProvider.Verify(passwordHash, password);
             }
 
             return (authenticationProvider, username, success);
diff --git a/MediaBrowser.Common/Cryptography/CryptoExtensions.cs b/MediaBrowser.Common/Cryptography/CryptoExtensions.cs
deleted file mode 100644
index 157b0ed10..000000000
--- a/MediaBrowser.Common/Cryptography/CryptoExtensions.cs
+++ /dev/null
@@ -1,35 +0,0 @@
-using System.Collections.Generic;
-using System.Globalization;
-using System.Text;
-using MediaBrowser.Model.Cryptography;
-using static MediaBrowser.Common.Cryptography.Constants;
-
-namespace MediaBrowser.Common.Cryptography
-{
-    /// <summary>
-    /// Class containing extension methods for working with Jellyfin cryptography objects.
-    /// </summary>
-    public static class CryptoExtensions
-    {
-        /// <summary>
-        /// Creates a new <see cref="PasswordHash" /> instance.
-        /// </summary>
-        /// <param name="cryptoProvider">The <see cref="ICryptoProvider" /> instance used.</param>
-        /// <param name="password">The password that will be hashed.</param>
-        /// <returns>A <see cref="PasswordHash" /> instance with the hash method, hash, salt and number of iterations.</returns>
-        public static PasswordHash CreatePasswordHash(this ICryptoProvider cryptoProvider, string password)
-        {
-            byte[] salt = cryptoProvider.GenerateSalt();
-            return new PasswordHash(
-                cryptoProvider.DefaultHashMethod,
-                cryptoProvider.ComputeHashWithDefaultMethod(
-                    Encoding.UTF8.GetBytes(password),
-                    salt),
-                salt,
-                new Dictionary<string, string>
-                {
-                    { "iterations", DefaultIterations.ToString(CultureInfo.InvariantCulture) }
-                });
-        }
-    }
-}
diff --git a/MediaBrowser.Common/Cryptography/Constants.cs b/MediaBrowser.Model/Cryptography/Constants.cs
index 354114232..f2ebb5d3d 100644
--- a/MediaBrowser.Common/Cryptography/Constants.cs
+++ b/MediaBrowser.Model/Cryptography/Constants.cs
@@ -1,4 +1,4 @@
-namespace MediaBrowser.Common.Cryptography
+namespace MediaBrowser.Model.Cryptography
 {
     /// <summary>
     /// Class containing global constants for Jellyfin Cryptography.
@@ -8,11 +8,16 @@ namespace MediaBrowser.Common.Cryptography
         /// <summary>
         /// The default length for new salts.
         /// </summary>
-        public const int DefaultSaltLength = 64;
+        public const int DefaultSaltLength = 128 / 8;
+
+        /// <summary>
+        /// The default output length.
+        /// </summary>
+        public const int DefaultOutputLength = 512 / 8;
 
         /// <summary>
         /// The default amount of iterations for hashing passwords.
         /// </summary>
-        public const int DefaultIterations = 1000;
+        public const int DefaultIterations = 120000;
     }
 }
diff --git a/MediaBrowser.Model/Cryptography/ICryptoProvider.cs b/MediaBrowser.Model/Cryptography/ICryptoProvider.cs
index d8b7d848a..6c521578c 100644
--- a/MediaBrowser.Model/Cryptography/ICryptoProvider.cs
+++ b/MediaBrowser.Model/Cryptography/ICryptoProvider.cs
@@ -1,6 +1,6 @@
 #pragma warning disable CS1591
 
-using System.Collections.Generic;
+using System;
 
 namespace MediaBrowser.Model.Cryptography
 {
@@ -8,11 +8,14 @@ namespace MediaBrowser.Model.Cryptography
     {
         string DefaultHashMethod { get; }
 
-        IEnumerable<string> GetSupportedHashMethods();
+        /// <summary>
+        /// Creates a new <see cref="PasswordHash" /> instance.
+        /// </summary>
+        /// <param name="password">The password that will be hashed.</param>
+        /// <returns>A <see cref="PasswordHash" /> instance with the hash method, hash, salt and number of iterations.</returns>
+        PasswordHash CreatePasswordHash(ReadOnlySpan<char> password);
 
-        byte[] ComputeHash(string hashMethod, byte[] bytes, byte[] salt);
-
-        byte[] ComputeHashWithDefaultMethod(byte[] bytes, byte[] salt);
+        bool Verify(PasswordHash hash, ReadOnlySpan<char> password);
 
         byte[] GenerateSalt();
 
diff --git a/MediaBrowser.Common/Cryptography/PasswordHash.cs b/MediaBrowser.Model/Cryptography/PasswordHash.cs
index 0e2065302..eec541041 100644
--- a/MediaBrowser.Common/Cryptography/PasswordHash.cs
+++ b/MediaBrowser.Model/Cryptography/PasswordHash.cs
@@ -4,7 +4,7 @@ using System;
 using System.Collections.Generic;
 using System.Text;
 
-namespace MediaBrowser.Common.Cryptography
+namespace MediaBrowser.Model.Cryptography
 {
     // Defined from this hash storage spec
     // https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md
diff --git a/tests/Jellyfin.Common.Tests/Cryptography/PasswordHashTests.cs b/tests/Jellyfin.Model.Tests/Cryptography/PasswordHashTests.cs
index bfece97b6..6948280a3 100644
--- a/tests/Jellyfin.Common.Tests/Cryptography/PasswordHashTests.cs
+++ b/tests/Jellyfin.Model.Tests/Cryptography/PasswordHashTests.cs
@@ -1,9 +1,9 @@
 using System;
 using System.Collections.Generic;
-using MediaBrowser.Common.Cryptography;
+using MediaBrowser.Model.Cryptography;
 using Xunit;
 
-namespace Jellyfin.Common.Tests.Cryptography
+namespace Jellyfin.Model.Tests.Cryptography
 {
     public static class PasswordHashTests
     {