From 599e0a76ff87235fe062afc7c8a0548fb946362d Mon Sep 17 00:00:00 2001 From: Febin K Dominic Date: Mon, 29 Jun 2026 19:38:12 +0530 Subject: [PATCH 1/3] fix: Lock while generating the Security Key --- .../DefaultStore/DataProtectionStore.cs | 4 +- .../DefaultStore/InMemoryStore.cs | 4 +- .../Interfaces/IJsonWebKeyStore.cs | 2 +- .../Jwt/JwtService.cs | 29 ++++++++++--- .../DatabaseJsonWebKeyStore.cs | 42 +++++++++++++++++-- .../FileSystemStore.cs | 3 +- 6 files changed, 69 insertions(+), 15 deletions(-) diff --git a/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs b/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs index fcb1ad2..e6bdf55 100644 --- a/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs +++ b/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs @@ -54,7 +54,7 @@ public DataProtectionStore( _memoryCache = memoryCache; _dataProtector = provider.CreateProtector(nameof(KeyMaterial)); ; } - public Task Store(KeyMaterial securityParameters) + public Task Store(KeyMaterial securityParameters) { var possiblyEncryptedKeyElement = _dataProtector.Protect(JsonSerializer.Serialize(securityParameters)); @@ -74,7 +74,7 @@ public Task Store(KeyMaterial securityParameters) KeyRepository.StoreElement(keyElement, friendlyName); ClearCache(); - return Task.CompletedTask; + return Task.FromResult(securityParameters); } diff --git a/src/NetDevPack.Security.Jwt.Core/DefaultStore/InMemoryStore.cs b/src/NetDevPack.Security.Jwt.Core/DefaultStore/InMemoryStore.cs index 9bd2511..5bcd41e 100644 --- a/src/NetDevPack.Security.Jwt.Core/DefaultStore/InMemoryStore.cs +++ b/src/NetDevPack.Security.Jwt.Core/DefaultStore/InMemoryStore.cs @@ -11,7 +11,7 @@ internal class InMemoryStore : IJsonWebKeyStore private readonly SemaphoreSlim _slim = new(1); internal const string DefaultRevocationReason = "Revoked"; - public Task Store(KeyMaterial keyMaterial) + public Task Store(KeyMaterial keyMaterial) { if (keyMaterial is null) throw new InvalidOperationException("Can't store empty value."); @@ -19,7 +19,7 @@ public Task Store(KeyMaterial keyMaterial) _store.Add(keyMaterial); _slim.Release(); - return Task.CompletedTask; + return Task.FromResult(keyMaterial); } public Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws) diff --git a/src/NetDevPack.Security.Jwt.Core/Interfaces/IJsonWebKeyStore.cs b/src/NetDevPack.Security.Jwt.Core/Interfaces/IJsonWebKeyStore.cs index 23bd3d6..5a73b10 100644 --- a/src/NetDevPack.Security.Jwt.Core/Interfaces/IJsonWebKeyStore.cs +++ b/src/NetDevPack.Security.Jwt.Core/Interfaces/IJsonWebKeyStore.cs @@ -7,7 +7,7 @@ namespace NetDevPack.Security.Jwt.Core.Interfaces; public interface IJsonWebKeyStore { - Task Store(KeyMaterial keyMaterial); + Task Store(KeyMaterial keyMaterial); Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws); Task Revoke(KeyMaterial keyMaterial, string reason=default); Task> GetLastKeys(int quantity, JwtKeyType? jwtKeyType = null); diff --git a/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs b/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs index 56c89c2..1fba6ad 100644 --- a/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs +++ b/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs @@ -11,6 +11,8 @@ internal class JwtService : IJwtService { private readonly IJsonWebKeyStore _store; private readonly IOptions _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 options) { @@ -22,9 +24,11 @@ public async Task GenerateKey(JwtKeyType jwtKeyType = JwtKeyType.Jw var key = new CryptographicKey(jwtKeyType == JwtKeyType.Jws ? _options.Value.Jws : _options.Value.Jwe); var model = new KeyMaterial(key); - await _store.Store(model); + // Store returns the persisted key: itself when it wins the insert, + // or the key another replica already stored for this slot — so we never sign with a key that isn't published. + var persisted = await _store.Store(model); - return model.GetSecurityKey(); + return persisted.GetSecurityKey(); } public async Task GetCurrentSecurityKey(JwtKeyType jwtKeyType = JwtKeyType.Jws) @@ -33,10 +37,23 @@ public async Task GetCurrentSecurityKey(JwtKeyType jwtKeyType = Jwt 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 under the lock. Store/Revoke clear the cache, so a key created + // while we were waiting is visible on this read. + 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); + return await GenerateKey(jwtKeyType); + } + } + finally + { + RotationLock.Release(); + } } // options has change. Change current key diff --git a/src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs b/src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs index d9c7f02..dd871eb 100644 --- a/src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs +++ b/src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs @@ -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; @@ -32,13 +34,47 @@ public DatabaseJsonWebKeyStore(TContext context, ILogger Store(KeyMaterial securityParamteres) { - await _context.SecurityKeys.AddAsync(securityParamteres); + // Deterministic Id per key type + rotation window: every replica computes the same Id, + // so concurrent first-start/rotation inserts collide on the primary key and only one wins. + // Uses PK/id uniqueness (no secondary unique index) + securityParamteres.Id = DeterministicId(securityParamteres.Use, CurrentSlot()); _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; + var winner = await _context.SecurityKeys.AsNoTracking() + .FirstOrDefaultAsync(k => k.Id == securityParamteres.Id); + if (winner == null) + throw; + + // Return the persisted winner so the caller signs with the published key, not our orphan. + ClearCache(); + return winner; + } ClearCache(); + return securityParamteres; + } + + // slot = rotation window index. Same window => same Id on every replica. + private long CurrentSlot() + => DateTime.UtcNow.Ticks / TimeSpan.FromDays(Math.Max(1, _options.Value.DaysUntilExpire)).Ticks; + + private static Guid DeterministicId(string use, long slot) + { + using var sha = SHA256.Create(); + var hash = sha.ComputeHash(Encoding.UTF8.GetBytes($"{use}:{slot}")); + var guidBytes = new byte[16]; + Array.Copy(hash, guidBytes, 16); + return new Guid(guidBytes); } public async Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws) diff --git a/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs b/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs index f3375bf..b8eedf4 100644 --- a/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs +++ b/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs @@ -35,7 +35,7 @@ private string GetCurrentFile(JwtKeyType jwtKeyType) return Path.Combine(KeysPath.FullName, $"{_options.Value.KeyPrefix}current.{jwtKeyType}.key"); } - public async Task Store(KeyMaterial securityParamteres) + public async Task Store(KeyMaterial securityParamteres) { if (!KeysPath.Exists) KeysPath.Create(); @@ -48,6 +48,7 @@ public async Task Store(KeyMaterial securityParamteres) await File.WriteAllTextAsync(Path.Combine(KeysPath.FullName, $"{_options.Value.KeyPrefix}current-{securityParamteres.KeyId}.{keyType}.key"), JsonSerializer.Serialize(securityParamteres, new JsonSerializerOptions() { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull })); ClearCache(); + return securityParamteres; } public bool NeedsUpdate(KeyMaterial current) From 2fc27702e07c9e994b30fde835caec6c9f87cae8 Mon Sep 17 00:00:00 2001 From: Febin K Dominic Date: Tue, 30 Jun 2026 17:39:38 +0530 Subject: [PATCH 2/3] fix: move away from time-based epochs --- .../DefaultStore/DataProtectionStore.cs | 4 +-- .../DefaultStore/InMemoryStore.cs | 2 +- .../Interfaces/IJsonWebKeyStore.cs | 2 +- .../Jwt/JwtService.cs | 26 ++++++++++++------- src/NetDevPack.Security.Jwt.Core/Model/Key.cs | 1 + .../DatabaseJsonWebKeyStore.cs | 20 ++++++-------- .../FileSystemStore.cs | 4 +-- 7 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs b/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs index e6bdf55..70454b9 100644 --- a/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs +++ b/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs @@ -79,11 +79,11 @@ public Task Store(KeyMaterial securityParameters) - public async Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws) + public async Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws, bool bypassCache = false) { var cacheKey = JwkContants.CurrentJwkCache + jwtKeyType; - if (!_memoryCache.TryGetValue(cacheKey, out KeyMaterial keyMaterial)) + if (bypassCache || !_memoryCache.TryGetValue(cacheKey, out KeyMaterial keyMaterial)) { var keys = await GetLastKeys(1, jwtKeyType); keyMaterial = keys.FirstOrDefault(); diff --git a/src/NetDevPack.Security.Jwt.Core/DefaultStore/InMemoryStore.cs b/src/NetDevPack.Security.Jwt.Core/DefaultStore/InMemoryStore.cs index 5bcd41e..49c021e 100644 --- a/src/NetDevPack.Security.Jwt.Core/DefaultStore/InMemoryStore.cs +++ b/src/NetDevPack.Security.Jwt.Core/DefaultStore/InMemoryStore.cs @@ -22,7 +22,7 @@ public Task Store(KeyMaterial keyMaterial) return Task.FromResult(keyMaterial); } - public Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws) + public Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws, bool bypassCache = false) { return Task.FromResult(_store.Where(s => s.Use == (jwtKeyType == JwtKeyType.Jws ? "sig" : "enc")).OrderByDescending(s => s.CreationDate).FirstOrDefault()); } diff --git a/src/NetDevPack.Security.Jwt.Core/Interfaces/IJsonWebKeyStore.cs b/src/NetDevPack.Security.Jwt.Core/Interfaces/IJsonWebKeyStore.cs index 5a73b10..740213d 100644 --- a/src/NetDevPack.Security.Jwt.Core/Interfaces/IJsonWebKeyStore.cs +++ b/src/NetDevPack.Security.Jwt.Core/Interfaces/IJsonWebKeyStore.cs @@ -8,7 +8,7 @@ namespace NetDevPack.Security.Jwt.Core.Interfaces; public interface IJsonWebKeyStore { Task Store(KeyMaterial keyMaterial); - Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws); + Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws, bool bypassCache = false); Task Revoke(KeyMaterial keyMaterial, string reason=default); Task> GetLastKeys(int quantity, JwtKeyType? jwtKeyType = null); Task Get(string keyId); diff --git a/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs b/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs index 1fba6ad..5a91924 100644 --- a/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs +++ b/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs @@ -20,12 +20,20 @@ public JwtService(IJsonWebKeyStore store, IOptions options) _options = options; } public async Task GenerateKey(JwtKeyType jwtKeyType = JwtKeyType.Jws) + { + var current = await _store.GetCurrent(jwtKeyType, bypassCache: true); + return await GenerateKey(jwtKeyType, current); + } + + private async Task GenerateKey(JwtKeyType jwtKeyType, KeyMaterial previous) { var key = new CryptographicKey(jwtKeyType == JwtKeyType.Jws ? _options.Value.Jws : _options.Value.Jwe); var model = new KeyMaterial(key); - // Store returns the persisted key: itself when it wins the insert, - // or the key another replica already stored for this slot — so we never sign with a key that isn't published. + // Next rotation version. Seeded at 1 on cold start and for pre-column rows (Version defaults to 0). + model.Version = (previous?.Version ?? 0) + 1; + // Store returns the persisted key when it wins the insert, or the key another replica already stored + // for this version, so we never sign with a key that isn't published. var persisted = await _store.Store(model); return persisted.GetSecurityKey(); @@ -40,14 +48,13 @@ public async Task GetCurrentSecurityKey(JwtKeyType jwtKeyType = Jwt await RotationLock.WaitAsync(); try { - // Re-check under the lock. Store/Revoke clear the cache, so a key created - // while we were waiting is visible on this read. - current = await _store.GetCurrent(jwtKeyType); + // Re-check under the lock, bypassing the cache + current = await _store.GetCurrent(jwtKeyType, bypassCache: true); 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); - return await GenerateKey(jwtKeyType); + return await GenerateKey(jwtKeyType, current); } } finally @@ -91,7 +98,7 @@ private async Task CheckCompatibility(KeyMaterial currentKey, JwtKeyType j if (jwtKeyType == JwtKeyType.Jws && currentKey.Type != _options.Value.Jws.Kty() || jwtKeyType == JwtKeyType.Jwe && currentKey.Type != _options.Value.Jwe.Kty()) { - await GenerateKey(jwtKeyType); + await GenerateKey(jwtKeyType, currentKey); return false; } return true; @@ -106,10 +113,9 @@ public async Task RevokeKey(string keyId, string reason = null) public async Task GenerateNewKey(JwtKeyType jwtKeyType = JwtKeyType.Jws) { - var oldCurrent = await _store.GetCurrent(jwtKeyType); + var oldCurrent = await _store.GetCurrent(jwtKeyType, bypassCache: true); await _store.Revoke(oldCurrent); - return await GenerateKey(jwtKeyType); - + return await GenerateKey(jwtKeyType, oldCurrent); } private bool NeedsUpdate(KeyMaterial current) diff --git a/src/NetDevPack.Security.Jwt.Core/Model/Key.cs b/src/NetDevPack.Security.Jwt.Core/Model/Key.cs index c8e0973..d7b03de 100644 --- a/src/NetDevPack.Security.Jwt.Core/Model/Key.cs +++ b/src/NetDevPack.Security.Jwt.Core/Model/Key.cs @@ -19,6 +19,7 @@ public KeyMaterial(CryptographicKey cryptographicKey) } public Guid Id { get; set; } = Guid.NewGuid(); + public long Version { get; set; } public string KeyId { get; set; } public string Type { get; set; } public string Use { get; set; } diff --git a/src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs b/src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs index dd871eb..dfb3c5b 100644 --- a/src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs +++ b/src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs @@ -36,10 +36,10 @@ public DatabaseJsonWebKeyStore(TContext context, ILogger Store(KeyMaterial securityParamteres) { - // Deterministic Id per key type + rotation window: every replica computes the same Id, - // so concurrent first-start/rotation inserts collide on the primary key and only one wins. - // Uses PK/id uniqueness (no secondary unique index) - securityParamteres.Id = DeterministicId(securityParamteres.Use, CurrentSlot()); + // Deterministic Id per use + kty + rotation version + // every replica replacing the same key computes the same Id + // Concurrent inserts collide on the primary key + securityParamteres.Id = DeterministicId(securityParamteres.Use, securityParamteres.Type, securityParamteres.Version); _logger.LogInformation($"Saving new SecurityKeyWithPrivate {securityParamteres.Id}", typeof(TContext).Name); try @@ -64,24 +64,20 @@ public async Task Store(KeyMaterial securityParamteres) return securityParamteres; } - // slot = rotation window index. Same window => same Id on every replica. - private long CurrentSlot() - => DateTime.UtcNow.Ticks / TimeSpan.FromDays(Math.Max(1, _options.Value.DaysUntilExpire)).Ticks; - - private static Guid DeterministicId(string use, long slot) + private static Guid DeterministicId(string use, string kty, long version) { using var sha = SHA256.Create(); - var hash = sha.ComputeHash(Encoding.UTF8.GetBytes($"{use}:{slot}")); + var hash = sha.ComputeHash(Encoding.UTF8.GetBytes($"{use}:{kty}:{version}")); var guidBytes = new byte[16]; Array.Copy(hash, guidBytes, 16); return new Guid(guidBytes); } - public async Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws) + public async Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws, bool bypassCache = false) { var cacheKey = JwkContants.CurrentJwkCache + jwtKeyType; - if (!_memoryCache.TryGetValue(cacheKey, out KeyMaterial credentials)) + if (bypassCache || !_memoryCache.TryGetValue(cacheKey, out KeyMaterial credentials)) { var keyType = (jwtKeyType == JwtKeyType.Jws ? "sig" : "enc"); #if NET5_0_OR_GREATER diff --git a/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs b/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs index b8eedf4..1b6a687 100644 --- a/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs +++ b/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs @@ -75,11 +75,11 @@ public async Task Revoke(KeyMaterial securityKeyWithPrivate, string reason = nul } - public Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws) + public Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws, bool bypassCache = false) { var cacheKey = JwkContants.CurrentJwkCache + jwtKeyType; - if (!_memoryCache.TryGetValue(cacheKey, out KeyMaterial credentials)) + if (bypassCache || !_memoryCache.TryGetValue(cacheKey, out KeyMaterial credentials)) { credentials = GetKey(GetCurrentFile(jwtKeyType)); // Set cache options. From 46d91434fc160ef54460bbb05b8c88935123538f Mon Sep 17 00:00:00 2001 From: Febin K Dominic Date: Wed, 1 Jul 2026 14:11:13 +0530 Subject: [PATCH 3/3] fix: manually revoking tokens cause collision --- src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs b/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs index 5a91924..decc7df 100644 --- a/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs +++ b/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs @@ -22,6 +22,8 @@ public JwtService(IJsonWebKeyStore store, IOptions options) public async Task GenerateKey(JwtKeyType jwtKeyType = JwtKeyType.Jws) { var current = await _store.GetCurrent(jwtKeyType, bypassCache: true); + // if current is null, get the highest version ever created (manually revoked/first-run) + current ??= (await _store.GetLastKeys(1, jwtKeyType)).FirstOrDefault(); return await GenerateKey(jwtKeyType, current); } @@ -50,6 +52,8 @@ public async Task GetCurrentSecurityKey(JwtKeyType jwtKeyType = Jwt { // Re-check under the lock, bypassing the cache current = await _store.GetCurrent(jwtKeyType, bypassCache: true); + // No active key: fall back to the newest key including revoked. + current ??= (await _store.GetLastKeys(1, jwtKeyType)).FirstOrDefault(); 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 @@ -114,6 +118,8 @@ public async Task RevokeKey(string keyId, string reason = null) public async Task GenerateNewKey(JwtKeyType jwtKeyType = JwtKeyType.Jws) { var oldCurrent = await _store.GetCurrent(jwtKeyType, bypassCache: true); + // if current is null, get the highest version ever created (manually revoked/first-run) + oldCurrent ??= (await _store.GetLastKeys(1, jwtKeyType)).FirstOrDefault(); await _store.Revoke(oldCurrent); return await GenerateKey(jwtKeyType, oldCurrent); }