Skip to content

fix: Lock while generating the Security Key #73

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
febinkdominic wants to merge 4 commits into
base: master
Choose a base branch
from
+76 −23
Draft

fix: Lock while generating the Security Key #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
599e0a7
fix: Lock while generating the Security Key
febinkdominic Jun 29, 2026
2fc2770
fix: move away from time-based epochs
febinkdominic Jun 30, 2026
46d9143
fix: manually revoking tokens cause collision
febinkdominic Jul 1, 2026
ed8dc12
fix(JwtService): fix racing using another Get
febinkdominic Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@
keyMaterial = keys.FirstOrDefault();
// Set cache options.
var cacheEntryOptions = new MemoryCacheEntryOptions()
// Keep in cache for this time, reset time if accessed.
.SetSlidingExpiration(_options.Value.CacheTime);
.SetAbsoluteExpiration(_options.Value.CacheTime);

if (keyMaterial != null)
_memoryCache.Set(cacheKey, keyMaterial, cacheEntryOptions);
Expand Down Expand Up @@ -160,8 +159,7 @@

// Set cache options.
var cacheEntryOptions = new MemoryCacheEntryOptions()
// Keep in cache for this time, reset time if accessed.
.SetSlidingExpiration(_options.Value.CacheTime);
.SetAbsoluteExpiration(_options.Value.CacheTime);

if (keys.Any())
{
Expand Down Expand Up @@ -195,7 +193,7 @@
}


public async Task Revoke(KeyMaterial keyMaterial, string reason = null)

Check warning on line 196 in src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs

View workflow job for this annotation

GitHub Actions / pull-request 

Cannot convert null literal to non-nullable reference type.
{
if (keyMaterial == null)
return;
Expand Down
45 changes: 36 additions & 9 deletions src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
{
private readonly IJsonWebKeyStore _store;
private readonly IOptions<JwtOptions> _options;
// Process-wide lock so a scoped service across concurrent requests rotates once, not once per request.
private static readonly SemaphoreSlim RotationLock = new(1, 1);

public JwtService(IJsonWebKeyStore store, IOptions<JwtOptions> options)
{
Expand All @@ -19,12 +21,21 @@
}
public async Task<SecurityKey> GenerateKey(JwtKeyType jwtKeyType = JwtKeyType.Jws)
{
var key = new CryptographicKey(jwtKeyType == JwtKeyType.Jws ? _options.Value.Jws : _options.Value.Jwe);
async Task<KeyMaterial> StoreNewKey()
{
var key = new CryptographicKey(jwtKeyType == JwtKeyType.Jws ? _options.Value.Jws : _options.Value.Jwe);
await _store.Store(new KeyMaterial(key));
return await _store.GetCurrent(jwtKeyType);
}

var model = new KeyMaterial(key);
await _store.Store(model);
var current = await StoreNewKey();

return model.GetSecurityKey();
// If current is null, the last stored key was also revoked during the race
current ??= await StoreNewKey();

// A second null means keys are being revoked as fast as we mint them
return current?.GetSecurityKey()
?? throw new InvalidOperationException($"Unable to persist an active {jwtKeyType} key: keys are being revoked concurrently.");
}

public async Task<SecurityKey> GetCurrentSecurityKey(JwtKeyType jwtKeyType = JwtKeyType.Jws)
Expand All @@ -33,10 +44,27 @@

if (NeedsUpdate(current))
{
// According NIST - https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-57pt1r4.pdf - Private key should be removed when no longer needs
await _store.Revoke(current);
var newKey = await GenerateKey(jwtKeyType);
return newKey;
await RotationLock.WaitAsync();
try
{
// Re-check: if another request on this pod already rotated, its Store cleared the cache
// and this read comes back fresh.
current = await _store.GetCurrent(jwtKeyType);
if (NeedsUpdate(current))
{
// According NIST - https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-57pt1r4.pdf - Private key should be removed when no longer needs
await _store.Revoke(current);
// All stores would internally clear cache on revoke
// So this GetCurrent is from the actual source, and could possibly be updated by someone else.
current = await _store.GetCurrent(jwtKeyType);
if (NeedsUpdate(current))
return await GenerateKey(jwtKeyType);
}
}
finally
{
RotationLock.Release();
}
}

// options has change. Change current key
Expand Down Expand Up @@ -80,7 +108,7 @@
return true;
}

public async Task RevokeKey(string keyId, string reason = null)

Check warning on line 111 in src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs

View workflow job for this annotation

GitHub Actions / pull-request 

Cannot convert null literal to non-nullable reference type.
{
var key = await _store.Get(keyId);

Expand All @@ -92,7 +120,6 @@
var oldCurrent = await _store.GetCurrent(jwtKeyType);
await _store.Revoke(oldCurrent);
return await GenerateKey(jwtKeyType);

}

private bool NeedsUpdate(KeyMaterial current)
Expand Down
42 changes: 36 additions & 6 deletions src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using System.Security.Cryptography;
using System.Text;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
Expand Down Expand Up @@ -34,13 +36,43 @@ public DatabaseJsonWebKeyStore(TContext context, ILogger<DatabaseJsonWebKeyStore

public async Task Store(KeyMaterial securityParamteres)
{
await _context.SecurityKeys.AddAsync(securityParamteres);
// Deterministic Id chained off the newest row for this use: replicas replacing the same
// predecessor compute the same Id, so concurrent inserts collide on the primary key
// instead of multiplying keys.
var previousId = await _context.SecurityKeys.AsNoTracking()
.Where(k => k.Use == securityParamteres.Use)
.OrderByDescending(k => k.CreationDate)
.Select(k => (Guid?)k.Id)
.FirstOrDefaultAsync();
securityParamteres.Id = DeterministicId(securityParamteres.Use, securityParamteres.Type, previousId);

_logger.LogInformation($"Saving new SecurityKeyWithPrivate {securityParamteres.Id}", typeof(TContext).Name);
await _context.SaveChangesAsync();
try
{
await _context.SecurityKeys.AddAsync(securityParamteres);
await _context.SaveChangesAsync();
}
catch
{
// Lost the race or a transient fault.
_context.Entry(securityParamteres).State = EntityState.Detached;

// Another replica inserted this Id first: drop ours
if (!await _context.SecurityKeys.AsNoTracking().AnyAsync(k => k.Id == securityParamteres.Id))
throw;
}
ClearCache();
}

private static Guid DeterministicId(string use, string kty, Guid? previousId)
{
using var sha = SHA256.Create();
var hash = sha.ComputeHash(Encoding.UTF8.GetBytes($"{use}:{kty}:{previousId?.ToString() ?? "genesis"}"));
var guidBytes = new byte[16];
Array.Copy(hash, guidBytes, 16);
return new Guid(guidBytes);
}

public async Task<KeyMaterial> GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws)
{
var cacheKey = JwkContants.CurrentJwkCache + jwtKeyType;
Expand All @@ -56,8 +88,7 @@ public async Task<KeyMaterial> GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws

// Set cache options.
var cacheEntryOptions = new MemoryCacheEntryOptions()
// Keep in cache for this time, reset time if accessed.
.SetSlidingExpiration(_options.Value.CacheTime);
.SetAbsoluteExpiration(_options.Value.CacheTime);

if (credentials != null)
_memoryCache.Set(cacheKey, credentials, cacheEntryOptions);
Expand All @@ -83,8 +114,7 @@ public async Task<ReadOnlyCollection<KeyMaterial>> GetLastKeys(int quantity = 5,
#endif
// Set cache options.
var cacheEntryOptions = new MemoryCacheEntryOptions()
// Keep in cache for this time, reset time if accessed.
.SetSlidingExpiration(_options.Value.CacheTime);
.SetAbsoluteExpiration(_options.Value.CacheTime);

if (keys.Any())
_memoryCache.Set(cacheKey, keys, cacheEntryOptions);
Expand Down
6 changes: 2 additions & 4 deletions src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ public async Task Revoke(KeyMaterial securityKeyWithPrivate, string reason = nul
credentials = GetKey(GetCurrentFile(jwtKeyType));
// Set cache options.
var cacheEntryOptions = new MemoryCacheEntryOptions()
// Keep in cache for this time, reset time if accessed.
.SetSlidingExpiration(_options.Value.CacheTime);
.SetAbsoluteExpiration(_options.Value.CacheTime);
if (credentials != null)
_memoryCache.Set(cacheKey, credentials, cacheEntryOptions);
}
Expand Down Expand Up @@ -114,8 +113,7 @@ public Task<ReadOnlyCollection<KeyMaterial>> GetLastKeys(int quantity = 5, JwtKe

// Set cache options.
var cacheEntryOptions = new MemoryCacheEntryOptions()
// Keep in cache for this time, reset time if accessed.
.SetSlidingExpiration(_options.Value.CacheTime);
.SetAbsoluteExpiration(_options.Value.CacheTime);

if (keys.Any())
_memoryCache.Set(cacheKey, keys, cacheEntryOptions);
Expand Down
Loading