diff --git a/src/modules/Elsa.Identity/Services/DefaultApplicationCredentialsValidator.cs b/src/modules/Elsa.Identity/Services/DefaultApplicationCredentialsValidator.cs index f70c28c3c..f30671229 100644 --- a/src/modules/Elsa.Identity/Services/DefaultApplicationCredentialsValidator.cs +++ b/src/modules/Elsa.Identity/Services/DefaultApplicationCredentialsValidator.cs @@ -62,6 +62,8 @@ public class DefaultApplicationCredentialsValidator : IApplicationCredentialsVal if (needsRehash && _applicationStore != null) { + var previousHashedApiKey = application.HashedApiKey; + var previousHashedApiKeySalt = application.HashedApiKeySalt; var hashedApiKey = _secretHasher.HashSecret(apiKey); application.HashedApiKey = hashedApiKey.EncodeSecret(); application.HashedApiKeySalt = hashedApiKey.EncodeSalt(); @@ -69,8 +71,16 @@ public class DefaultApplicationCredentialsValidator : IApplicationCredentialsVal { await _applicationStore.SaveAsync(application, cancellationToken); } - catch (Exception e) + catch (OperationCanceledException) { + application.HashedApiKey = previousHashedApiKey; + application.HashedApiKeySalt = previousHashedApiKeySalt; + throw; + } + catch (InvalidOperationException e) + { + application.HashedApiKey = previousHashedApiKey; + application.HashedApiKeySalt = previousHashedApiKeySalt; _logger.LogWarning(e, "Failed to save upgraded API key hash for application {ApplicationId}.", application.Id); } } diff --git a/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs b/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs index f727ec35e..696ba255a 100644 --- a/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs +++ b/src/modules/Elsa.Identity/Services/DefaultSecretHasher.cs @@ -65,6 +65,12 @@ public class DefaultSecretHasher : ISecretHasher } var legacyHash = HashLegacySha256(passwordBytes, salt); + if (password.Length != legacyHash.Length) + { + needsRehash = false; + return false; + } + var isLegacyMatch = CryptographicOperations.FixedTimeEquals(legacyHash, password); needsRehash = isLegacyMatch; return isLegacyMatch; @@ -88,7 +94,10 @@ public class DefaultSecretHasher : ISecretHasher private static byte[] HashLegacySha256(byte[] secret, byte[] salt) { - return SHA256.HashData(secret.Concat(salt).ToArray()); + using var sha256 = IncrementalHash.CreateHash(HashAlgorithmName.SHA256); + sha256.AppendData(secret); + sha256.AppendData(salt); + return sha256.GetHashAndReset(); } private static bool TryReadPbkdf2Hash(byte[] secret, out int iterationCount, out byte[] hash) diff --git a/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs b/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs index 6f2cc8cf9..b0e2b2dcb 100644 --- a/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs +++ b/src/modules/Elsa.Identity/Services/DefaultUserCredentialsValidator.cs @@ -56,6 +56,8 @@ public class DefaultUserCredentialsValidator : IUserCredentialsValidator if (needsRehash && _userStore != null) { + var previousHashedPassword = user.HashedPassword; + var previousHashedPasswordSalt = user.HashedPasswordSalt; var hashedPassword = _secretHasher.HashSecret(password); user.HashedPassword = hashedPassword.EncodeSecret(); user.HashedPasswordSalt = hashedPassword.EncodeSalt(); @@ -63,8 +65,16 @@ public class DefaultUserCredentialsValidator : IUserCredentialsValidator { await _userStore.SaveAsync(user, cancellationToken); } + catch (OperationCanceledException) + { + user.HashedPassword = previousHashedPassword; + user.HashedPasswordSalt = previousHashedPasswordSalt; + throw; + } catch (Exception e) { + user.HashedPassword = previousHashedPassword; + user.HashedPasswordSalt = previousHashedPasswordSalt; _logger.LogWarning(e, "Failed to save upgraded password hash for user {UserId}.", user.Id); } } diff --git a/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs b/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs index 7f51601af..db76a5d6b 100644 --- a/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs +++ b/test/unit/Elsa.Identity.UnitTests/Services/DefaultSecretHasherTests.cs @@ -55,6 +55,17 @@ public class DefaultSecretHasherTests Assert.False(needsRehash); } + [Fact] + public void VerifySecret_WithCorrectPasswordAndLowerIterationCount_RequestsRehash() + { + var hashedSecret = CreatePbkdf2Hash("secret", 1); + + var verified = _hasher.VerifySecret("secret", hashedSecret, out var needsRehash); + + Assert.True(verified); + Assert.True(needsRehash); + } + [Fact] public void VerifySecret_RejectsPbkdf2HashWithExcessiveIterations() { @@ -81,6 +92,17 @@ public class DefaultSecretHasherTests Assert.False(needsRehash); } + [Fact] + public void VerifySecret_RejectsMalformedLegacyHashWithInvalidKeyLength() + { + var hashedSecret = HashedSecret.FromBytes(RandomNumberGenerator.GetBytes(16), _hasher.GenerateSalt()); + + var verified = _hasher.VerifySecret("secret", hashedSecret, out var needsRehash); + + Assert.False(verified); + Assert.False(needsRehash); + } + [Fact] public async Task ValidateAsync_RehashesLegacyUserPassword() { @@ -148,6 +170,8 @@ public class DefaultSecretHasherTests var validatedUser = await validator.ValidateAsync("alice", "secret"); Assert.Same(user, validatedUser); + Assert.Equal(legacyHash.EncodeSecret(), user.HashedPassword); + Assert.Equal(legacyHash.EncodeSalt(), user.HashedPasswordSalt); } [Fact] @@ -193,6 +217,8 @@ public class DefaultSecretHasherTests var validatedApplication = await validator.ValidateAsync(apiKey); Assert.Same(application, validatedApplication); + Assert.Equal(legacyHash.EncodeSecret(), application.HashedApiKey); + Assert.Equal(legacyHash.EncodeSalt(), application.HashedApiKeySalt); } [Fact]