Address identity secret hashing review feedback

This commit is contained in:
Sipke Schoorstra 2026-05-21 00:04:21 +02:00
parent 65caa73d6f
commit 6a7ff8d48a
No known key found for this signature in database
GPG key ID: 5C10502B28A4268F
3 changed files with 53 additions and 7 deletions

View file

@ -1,3 +1,4 @@
using System.Globalization;
using System.Security.Cryptography;
using System.Text;
using Elsa.Identity.Contracts;
@ -81,7 +82,7 @@ public class DefaultSecretHasher : ISecretHasher
{
var hash = HashSecret(secret, salt, DefaultIterationCount);
var encodedHash = Convert.ToBase64String(hash);
return Encoding.UTF8.GetBytes($"{Algorithm}{Separator}{DefaultIterationCount}{Separator}{encodedHash}");
return Encoding.UTF8.GetBytes($"{Algorithm}{Separator}{DefaultIterationCount.ToString(CultureInfo.InvariantCulture)}{Separator}{encodedHash}");
}
/// <inheritdoc />
@ -110,7 +111,7 @@ public class DefaultSecretHasher : ISecretHasher
if (segments.Length != 3 || !string.Equals(segments[0], Algorithm, StringComparison.Ordinal))
return false;
if (!int.TryParse(segments[1], out iterationCount) || iterationCount <= 0 || iterationCount > MaxIterationCount)
if (!int.TryParse(segments[1], NumberStyles.None, CultureInfo.InvariantCulture, out iterationCount) || iterationCount <= 0 || iterationCount > MaxIterationCount)
return false;
try

View file

@ -71,7 +71,7 @@ public class DefaultUserCredentialsValidator : IUserCredentialsValidator
user.HashedPasswordSalt = previousHashedPasswordSalt;
throw;
}
catch (InvalidOperationException e)
catch (Exception e)
{
user.HashedPassword = previousHashedPassword;
user.HashedPasswordSalt = previousHashedPasswordSalt;

View file

@ -1,3 +1,4 @@
using System.Globalization;
using System.Security.Cryptography;
using System.Text;
using Elsa.Common.Services;
@ -23,6 +24,30 @@ public class DefaultSecretHasherTests
Assert.False(needsRehash);
}
[Fact]
public void HashSecret_FormatsPbkdf2EnvelopeUsingInvariantCulture()
{
using var _ = new CultureScope("ar-SA");
var hashedSecret = _hasher.HashSecret("secret");
Assert.StartsWith("pbkdf2-sha256$600000$", Encoding.UTF8.GetString(hashedSecret.Secret));
Assert.True(_hasher.VerifySecret("secret", hashedSecret, out var needsRehash));
Assert.False(needsRehash);
}
[Fact]
public void VerifySecret_ParsesPbkdf2EnvelopeUsingInvariantCulture()
{
var hashedSecret = CreatePbkdf2Hash("secret", 1);
using var _ = new CultureScope("ar-SA");
var verified = _hasher.VerifySecret("secret", hashedSecret, out var needsRehash);
Assert.True(verified);
Assert.True(needsRehash);
}
[Fact]
public void HashSecret_UsesUniqueSalts()
{
@ -164,7 +189,7 @@ public class DefaultSecretHasherTests
HashedPassword = legacyHash.EncodeSecret(),
HashedPasswordSalt = legacyHash.EncodeSalt()
};
var userStore = new FailingUserStore(user);
var userStore = new FailingUserStore(user, new TimeoutException("Save timed out."));
var validator = new DefaultUserCredentialsValidator(new StoreBasedUserProvider(userStore), userStore, _hasher);
var validatedUser = await validator.ValidateAsync("alice", "secret");
@ -260,15 +285,16 @@ public class DefaultSecretHasherTests
var salt = RandomNumberGenerator.GetBytes(32);
var secretBytes = Encoding.UTF8.GetBytes(secret);
var hash = Rfc2898DeriveBytes.Pbkdf2(secretBytes, salt, iterationCount, HashAlgorithmName.SHA256, 32);
var envelope = Encoding.UTF8.GetBytes($"pbkdf2-sha256${iterationCount}${Convert.ToBase64String(hash)}");
var envelope = Encoding.UTF8.GetBytes($"pbkdf2-sha256${iterationCount.ToString(CultureInfo.InvariantCulture)}${Convert.ToBase64String(hash)}");
return HashedSecret.FromBytes(envelope, salt);
}
private sealed class FailingUserStore(User user) : IUserStore
private sealed class FailingUserStore(User user, Exception? exception = null) : IUserStore
{
private readonly User _user = user;
private readonly Exception _exception = exception ?? new InvalidOperationException("Save failed.");
public Task SaveAsync(User user, CancellationToken cancellationToken = default) => throw new InvalidOperationException("Save failed.");
public Task SaveAsync(User user, CancellationToken cancellationToken = default) => throw _exception;
public Task DeleteAsync(UserFilter filter, CancellationToken cancellationToken = default) => Task.CompletedTask;
@ -321,4 +347,23 @@ public class DefaultSecretHasherTests
return Task.FromResult(application);
}
}
private sealed class CultureScope : IDisposable
{
private readonly CultureInfo _currentCulture = CultureInfo.CurrentCulture;
private readonly CultureInfo _currentUICulture = CultureInfo.CurrentUICulture;
public CultureScope(string cultureName)
{
var culture = CultureInfo.GetCultureInfo(cultureName);
CultureInfo.CurrentCulture = culture;
CultureInfo.CurrentUICulture = culture;
}
public void Dispose()
{
CultureInfo.CurrentCulture = _currentCulture;
CultureInfo.CurrentUICulture = _currentUICulture;
}
}
}