Refine identity secret hashing review fixes

This commit is contained in:
Sipke Schoorstra 2026-05-21 01:45:17 +02:00
parent b4e6f0f57e
commit 04429e81c6
No known key found for this signature in database
GPG key ID: 5C10502B28A4268F
4 changed files with 75 additions and 33 deletions

View file

@ -13,7 +13,7 @@ public class DefaultApiKeyGeneratorAndParser : IApiKeyGenerator, IApiKeyParser
public string Generate(string clientId)
{
var hexIdentifier = Convert.ToHexString(Encoding.UTF8.GetBytes(clientId));
var id = Convert.ToHexString(RandomNumberGenerator.GetBytes(32));
var id = new Guid(RandomNumberGenerator.GetBytes(16)).ToString("D");
return $"{hexIdentifier}-{id}";
}

View file

@ -1,6 +1,5 @@
using System.Buffers;
using System.Buffers.Text;
using System.Globalization;
using System.Security.Cryptography;
using System.Text;
using Elsa.Identity.Contracts;
@ -113,8 +112,7 @@ public class DefaultSecretHasher : ISecretHasher
var hash = HashSecret(secret, salt, DefaultIterationCount);
try
{
var encodedHash = Convert.ToBase64String(hash);
return Encoding.UTF8.GetBytes($"{Algorithm}{Separator}{DefaultIterationCount.ToString(CultureInfo.InvariantCulture)}{Separator}{encodedHash}");
return FormatHashEnvelope(hash);
}
finally
{
@ -130,6 +128,30 @@ public class DefaultSecretHasher : ISecretHasher
return Rfc2898DeriveBytes.Pbkdf2(secret, salt, iterationCount, HashAlgorithmName.SHA256, KeySize);
}
private static byte[] FormatHashEnvelope(byte[] hash)
{
Span<byte> iterationBytes = stackalloc byte[16];
if (!Utf8Formatter.TryFormat(DefaultIterationCount, iterationBytes, out var iterationBytesWritten))
throw new InvalidOperationException("Failed to format the PBKDF2 iteration count.");
var encodedHashLength = ((hash.Length + 2) / 3) * 4;
var result = new byte[AlgorithmBytes.Length + 1 + iterationBytesWritten + 1 + encodedHashLength];
var resultSpan = result.AsSpan();
AlgorithmBytes.CopyTo(resultSpan);
var offset = AlgorithmBytes.Length;
resultSpan[offset++] = SeparatorByte;
iterationBytes[..iterationBytesWritten].CopyTo(resultSpan[offset..]);
offset += iterationBytesWritten;
resultSpan[offset++] = SeparatorByte;
var status = Base64.EncodeToUtf8(hash, resultSpan[offset..], out var consumed, out var written);
if (status != OperationStatus.Done || consumed != hash.Length || written != encodedHashLength)
throw new InvalidOperationException("Failed to encode the PBKDF2 hash.");
return result;
}
private static byte[] HashLegacySha256(byte[] secret, byte[] salt)
{
using var sha256 = IncrementalHash.CreateHash(HashAlgorithmName.SHA256);

View file

@ -11,9 +11,8 @@ public class DefaultApiKeyGeneratorAndParserTests
var apiKey = generator.Generate("client-1");
var suffix = apiKey.Split('-', 2)[1];
var bytes = Convert.FromHexString(suffix);
Assert.Equal(64, suffix.Length);
Assert.Equal(32, bytes.Length);
Assert.Equal(36, suffix.Length);
Assert.True(Guid.TryParseExact(suffix, "D", out _));
}
}

View file

@ -20,18 +20,8 @@ public class DefaultSecretHasherTests
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 HashSecret_FormatsPbkdf2EnvelopeUsingInvariantCulture()
{
using var _ = new CultureScope("ar-SA");
var hashedSecret = _hasher.HashSecret("secret");
Assert.StartsWith("pbkdf2-sha256$600000$", Encoding.UTF8.GetString(hashedSecret.Secret));
Assert.Equal(32, hashedSecret.Salt.Length);
Assert.Equal(44, hashedSecret.EncodeSalt().Length);
Assert.True(_hasher.VerifySecret("secret", hashedSecret, out var needsRehash));
Assert.False(needsRehash);
}
@ -49,14 +39,12 @@ public class DefaultSecretHasherTests
}
[Fact]
public void HashSecret_GeneratesExpectedSaltAndVerifiesSecret()
public void GenerateSalt_GeneratesExpectedSalt()
{
var hashedSecret = _hasher.HashSecret("secret");
var salt = _hasher.GenerateSalt();
Assert.Equal(32, hashedSecret.Salt.Length);
Assert.Equal(44, hashedSecret.EncodeSalt().Length);
Assert.True(_hasher.VerifySecret("secret", hashedSecret, out var needsRehash));
Assert.False(needsRehash);
Assert.Equal(32, salt.Length);
Assert.Equal(44, Convert.ToBase64String(salt).Length);
}
[Fact]
@ -141,14 +129,16 @@ public class DefaultSecretHasherTests
HashedPassword = legacyHash.EncodeSecret(),
HashedPasswordSalt = legacyHash.EncodeSalt()
});
var validator = new DefaultUserCredentialsValidator(new StoreBasedUserProvider(userStore), userStore, _hasher);
var rehashingHasher = new RehashingSecretHasher("secret");
var validator = new DefaultUserCredentialsValidator(new StoreBasedUserProvider(userStore), userStore, rehashingHasher);
var user = await validator.ValidateAsync("alice", "secret");
var reloadedUser = await userStore.FindAsync(new UserFilter { Name = "alice" });
Assert.NotNull(user);
Assert.NotNull(reloadedUser);
Assert.StartsWith("pbkdf2-sha256$", Encoding.UTF8.GetString(Convert.FromBase64String(reloadedUser.HashedPassword)));
Assert.Equal(rehashingHasher.UpgradedSecret.EncodeSecret(), reloadedUser.HashedPassword);
Assert.Equal(rehashingHasher.UpgradedSecret.EncodeSalt(), reloadedUser.HashedPasswordSalt);
}
[Fact]
@ -169,14 +159,16 @@ public class DefaultSecretHasherTests
HashedClientSecretSalt = ""
});
var applicationProvider = new StoreBasedApplicationProvider(applicationStore);
var validator = new DefaultApplicationCredentialsValidator(apiKeyGenerator, applicationProvider, applicationStore, _hasher);
var rehashingHasher = new RehashingSecretHasher(apiKey);
var validator = new DefaultApplicationCredentialsValidator(apiKeyGenerator, applicationProvider, applicationStore, rehashingHasher);
var application = await validator.ValidateAsync(apiKey);
var reloadedApplication = await applicationStore.FindAsync(new ApplicationFilter { ClientId = "client-1" });
Assert.NotNull(application);
Assert.NotNull(reloadedApplication);
Assert.StartsWith("pbkdf2-sha256$", Encoding.UTF8.GetString(Convert.FromBase64String(reloadedApplication.HashedApiKey)));
Assert.Equal(rehashingHasher.UpgradedSecret.EncodeSecret(), reloadedApplication.HashedApiKey);
Assert.Equal(rehashingHasher.UpgradedSecret.EncodeSalt(), reloadedApplication.HashedApiKeySalt);
}
[Fact]
@ -191,7 +183,7 @@ public class DefaultSecretHasherTests
HashedPasswordSalt = legacyHash.EncodeSalt()
};
var userStore = new FailingUserStore(user, new TimeoutException("Save timed out."));
var validator = new DefaultUserCredentialsValidator(new StoreBasedUserProvider(userStore), userStore, _hasher);
var validator = new DefaultUserCredentialsValidator(new StoreBasedUserProvider(userStore), userStore, new RehashingSecretHasher("secret"));
var validatedUser = await validator.ValidateAsync("alice", "secret");
@ -212,7 +204,7 @@ public class DefaultSecretHasherTests
HashedPassword = encodedLegacyHash,
HashedPasswordSalt = legacyHash.EncodeSalt()
};
var validator = new DefaultUserCredentialsValidator(new StaticUserProvider(user), _hasher);
var validator = new DefaultUserCredentialsValidator(new StaticUserProvider(user), new RehashingSecretHasher("secret"));
var validatedUser = await validator.ValidateAsync("alice", "secret");
@ -238,7 +230,7 @@ public class DefaultSecretHasherTests
};
var applicationStore = new FailingApplicationStore(application);
var applicationProvider = new StoreBasedApplicationProvider(applicationStore);
var validator = new DefaultApplicationCredentialsValidator(apiKeyGenerator, applicationProvider, applicationStore, _hasher);
var validator = new DefaultApplicationCredentialsValidator(apiKeyGenerator, applicationProvider, applicationStore, new RehashingSecretHasher(apiKey));
var validatedApplication = await validator.ValidateAsync(apiKey);
@ -265,7 +257,7 @@ public class DefaultSecretHasherTests
HashedClientSecretSalt = ""
};
var applicationProvider = new StaticApplicationProvider(application);
var validator = new DefaultApplicationCredentialsValidator(apiKeyGenerator, applicationProvider, _hasher);
var validator = new DefaultApplicationCredentialsValidator(apiKeyGenerator, applicationProvider, new RehashingSecretHasher(apiKey));
var validatedApplication = await validator.ValidateAsync(apiKey);
@ -312,6 +304,35 @@ public class DefaultSecretHasherTests
}
}
private sealed class RehashingSecretHasher(string expectedSecret) : ISecretHasher
{
public HashedSecret UpgradedSecret { get; } = HashedSecret.FromBytes(Encoding.UTF8.GetBytes("upgraded-secret"), Encoding.UTF8.GetBytes("upgraded-salt"));
public HashedSecret HashSecret(string secret) => UpgradedSecret;
public HashedSecret HashSecret(string secret, byte[] salt) => UpgradedSecret;
public bool VerifySecret(string clearTextSecret, string secret, string salt) => clearTextSecret == expectedSecret;
public bool VerifySecret(string clearTextSecret, string secret, string salt, out bool needsRehash)
{
needsRehash = clearTextSecret == expectedSecret;
return clearTextSecret == expectedSecret;
}
public bool VerifySecret(string clearTextSecret, HashedSecret hashedSecret) => clearTextSecret == expectedSecret;
public bool VerifySecret(string clearTextSecret, HashedSecret hashedSecret, out bool needsRehash)
{
needsRehash = clearTextSecret == expectedSecret;
return clearTextSecret == expectedSecret;
}
public byte[] HashSecret(byte[] secret, byte[] salt) => UpgradedSecret.Secret;
public byte[] GenerateSalt(int saltSize = 32) => UpgradedSecret.Salt;
}
private sealed class FailingApplicationStore(Application application) : IApplicationStore
{
private readonly Application _application = application;