diff --git a/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs b/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs index fcb1ad2..cb4cfc9 100644 --- a/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs +++ b/src/NetDevPack.Security.Jwt.Core/DefaultStore/DataProtectionStore.cs @@ -89,8 +89,7 @@ public async Task GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws 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); @@ -160,8 +159,7 @@ public Task> 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()) { diff --git a/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs b/src/NetDevPack.Security.Jwt.Core/Jwt/JwtService.cs index 56c89c2..7e5c4c9 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) { @@ -19,12 +21,21 @@ public JwtService(IJsonWebKeyStore store, IOptions options) } public async Task GenerateKey(JwtKeyType jwtKeyType = JwtKeyType.Jws) { - var key = new CryptographicKey(jwtKeyType == JwtKeyType.Jws ? _options.Value.Jws : _options.Value.Jwe); + async Task 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 GetCurrentSecurityKey(JwtKeyType jwtKeyType = JwtKeyType.Jws) @@ -33,10 +44,27 @@ 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: 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 @@ -92,7 +120,6 @@ public async Task GenerateNewKey(JwtKeyType jwtKeyType = JwtKeyType var oldCurrent = await _store.GetCurrent(jwtKeyType); await _store.Revoke(oldCurrent); return await GenerateKey(jwtKeyType); - } private bool NeedsUpdate(KeyMaterial current) diff --git a/src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs b/src/NetDevPack.Security.Jwt.Store.EntityFrameworkCore/DatabaseJsonWebKeyStore.cs index d9c7f02..c4086ce 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; @@ -34,13 +36,43 @@ public DatabaseJsonWebKeyStore(TContext context, ILogger 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 GetCurrent(JwtKeyType jwtKeyType = JwtKeyType.Jws) { var cacheKey = JwkContants.CurrentJwkCache + jwtKeyType; @@ -56,8 +88,7 @@ public async Task 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); @@ -83,8 +114,7 @@ public async Task> 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); diff --git a/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs b/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs index f3375bf..592d613 100644 --- a/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs +++ b/src/NetDevPack.Security.Jwt.Store.FileSystem/FileSystemStore.cs @@ -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); } @@ -114,8 +113,7 @@ public Task> 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);