dohertj2/lmxopcua
1
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases Wiki Activity

feat(security): short TTL read-through cache in front of the ConfigDb key store

Browse Source
This commit is contained in:
Joseph Doherty
2026-08-20 09:40:56 -04:00
parent 3b56e5bd01
commit 37288d4df5
4 changed files with 252 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
archreview/plans/R3-05-acl-hot-path-expiry-plan.md
+14
View File
@@ -618,3 +618,17 @@ for the orchestrator.
`Anonymous_grant_resolution_is_bounded_too` (moved forward from the plan's T3 list — the deadline half
reaches `FindGrantsAsync` in the same change, so it is asserted in the same change).
- 5 tests, green.
### T3 — TTL read-through cache + the falsification control
- **RED observed:** `error CS0117: 'BoundedApiKeyStore' does not contain a definition for
'MaxCacheEntries'` — the cache and its ceiling absent.
- **DEVIATION (fail-closed refinement, 2 lines):** the plan keys `FindAsync`'s cache on `apiKeyId` alone.
Taken literally that lets the cache **resurrect a key that expired by the clock** for up to a TTL, which
is exactly the axis 03/S19 exists to close. A cached hit is therefore re-checked against the caller's
`utcNow` before it is served (`IsExpired`) — so the TTL bounds how stale an `Enabled` **revocation** may
be, and nothing else. `utcNow` is still not part of the key (it changes every call and would defeat the
cache entirely).
- The counter's `cache_hit` / `timeout` `Add` calls are deliberately NOT here — they are T4, and writing
them early would have made T4's red vacuous.
- 11 tests in the class, green.
archreview/plans/R3-05-acl-hot-path-expiry-plan.md.tasks.json
+1 -1
View File
@@ -19,7 +19,7 @@
{
"id": "T3",
"subject": "BoundedApiKeyStore: short TTL read-through cache + falsification control (a revoked key is denied once the TTL has passed)",
"status": "pending",
"status": "done",
"blockedBy": [
"T2"
]
src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/BoundedApiKeyStore.cs
+102 -4
View File
@@ -1,3 +1,4 @@
using System.Collections.Concurrent;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
@@ -24,13 +25,34 @@ namespace ZB.MOM.WW.OtOpcUa.Host.OpcUa;
/// a bad credential. Nothing here can admit a session the inner store did not authorize — the
/// decorator can only return what the inner store returned, or throw.
/// </para>
/// <para>
/// ⚠️ <b>Faults and timeouts are NEVER cached.</b> Caching a deny would be fail-closed and is
/// still wrong: it would extend a ConfigDb outage past the outage, denying valid keys for a
/// further TTL after the database came back. Only a COMPLETED lookup — a hit or an authoritative
/// miss — is stored.
/// </para>
/// </remarks>
public sealed class BoundedApiKeyStore : IApiKeyStore
{
/// <summary>
/// Entries either cache may hold before it is cleared wholesale.
/// </summary>
/// <remarks>
/// Wholesale rather than per-entry eviction, for the reason <c>SessionAclDecisionCache</c> already
/// documents: picking WHICH entry to drop needs knowledge the cache does not have, and a
/// repopulate is cheap and cannot be wrong. The ceiling exists so a key-id guessing flood is a
/// bounded memory event — negative results are cached deliberately, so that a flood of unknown ids
/// does not become a SQL flood.
/// </remarks>
public const int MaxCacheEntries = 1024;
private readonly IApiKeyStore _inner;
private readonly IOptions<ApiKeyOptions> _options;
private readonly ILogger<BoundedApiKeyStore> _log;
private readonly TimeProvider _time;
private readonly ConcurrentDictionary<string, Entry<ApiKeyLookup?>> _keys = new(StringComparer.Ordinal);
private readonly ConcurrentDictionary<string, Entry<IReadOnlyList<NodeAcl>>> _grants =
new(StringComparer.Ordinal);
/// <summary>Initializes a new instance of the <see cref="BoundedApiKeyStore"/> class.</summary>
/// <param name="inner">The store being bounded.</param>
@@ -59,12 +81,88 @@ public sealed class BoundedApiKeyStore : IApiKeyStore
}
/// <inheritdoc />
public Task<ApiKeyLookup?> FindAsync(string apiKeyId, DateTime utcNow, CancellationToken ct) =>
BoundAsync(token => _inner.FindAsync(apiKeyId, utcNow, token), ct);
/// <remarks>
/// Keyed by <paramref name="apiKeyId"/> alone. <paramref name="utcNow"/> is deliberately not part
/// of the key — it changes on every call, so including it would defeat the cache entirely — but a
/// cached hit IS re-checked against it before being served, so the TTL can never resurrect a key
/// that has expired by the clock in the meantime. The TTL therefore bounds how stale an
/// <c>Enabled</c> revocation may be, and nothing else.
/// </remarks>
public async Task<ApiKeyLookup?> FindAsync(string apiKeyId, DateTime utcNow, CancellationToken ct)
{
var ttl = Ttl;
if (ttl > TimeSpan.Zero
&& TryServe(_keys, apiKeyId, ttl, out var cached)
&& !IsExpired(cached, utcNow))
{
return cached;
}
var fresh = await BoundAsync(token => _inner.FindAsync(apiKeyId, utcNow, token), ct)
.ConfigureAwait(false);
if (ttl > TimeSpan.Zero) Store(_keys, apiKeyId, fresh);
return fresh;
}
/// <inheritdoc />
public Task<IReadOnlyList<NodeAcl>> FindGrantsAsync(string principal, CancellationToken ct) =>
BoundAsync(token => _inner.FindGrantsAsync(principal, token), ct);
/// <remarks>Keyed by principal — the anonymous activation path, which runs on every unauthenticated connect.</remarks>
public async Task<IReadOnlyList<NodeAcl>> FindGrantsAsync(string principal, CancellationToken ct)
{
var ttl = Ttl;
if (ttl > TimeSpan.Zero && TryServe(_grants, principal, ttl, out var cached)) return cached;
var fresh = await BoundAsync(token => _inner.FindGrantsAsync(principal, token), ct)
.ConfigureAwait(false);
if (ttl > TimeSpan.Zero) Store(_grants, principal, fresh);
return fresh;
}
/// <summary>The configured TTL; non-positive means the cache is off.</summary>
private TimeSpan Ttl => TimeSpan.FromSeconds(_options.Value.StoreCacheSeconds);
/// <summary>Whether a cached lookup has since expired by the clock.</summary>
/// <param name="cached">The cached lookup, possibly null (an authoritative miss).</param>
/// <param name="utcNow">The caller's current time.</param>
/// <returns><see langword="true"/> when the cached key must not be served.</returns>
private static bool IsExpired(ApiKeyLookup? cached, DateTime utcNow) =>
cached?.Key.ExpiresUtc is { } expires && expires <= utcNow;
/// <summary>Serves an entry if one is present and still inside the TTL.</summary>
/// <typeparam name="T">The cached value type.</typeparam>
/// <param name="cache">The cache to read.</param>
/// <param name="key">The cache key.</param>
/// <param name="ttl">How long an entry stays servable.</param>
/// <param name="value">The cached value, when this returns true.</param>
/// <returns><see langword="true"/> on a hit.</returns>
private bool TryServe<T>(
ConcurrentDictionary<string, Entry<T>> cache, string key, TimeSpan ttl, out T value)
{
if (cache.TryGetValue(key, out var entry) && _time.GetUtcNow() - entry.StampedUtc < ttl)
{
value = entry.Value;
return true;
}
value = default!;
return false;
}
/// <summary>Stores a COMPLETED lookup, clearing wholesale first if the ceiling has been reached.</summary>
/// <typeparam name="T">The cached value type.</typeparam>
/// <param name="cache">The cache to write.</param>
/// <param name="key">The cache key.</param>
/// <param name="value">The value to cache.</param>
private void Store<T>(ConcurrentDictionary<string, Entry<T>> cache, string key, T value)
{
if (cache.Count >= MaxCacheEntries) cache.Clear();
cache[key] = new Entry<T>(_time.GetUtcNow(), value);
}
/// <summary>One cached lookup and the instant it was taken.</summary>
/// <typeparam name="T">The cached value type.</typeparam>
/// <param name="StampedUtc">When the inner store answered.</param>
/// <param name="Value">What it answered.</param>
private readonly record struct Entry<T>(DateTimeOffset StampedUtc, T Value);
/// <summary>Runs one inner call under the configured deadline.</summary>
/// <typeparam name="T">The inner call's result type.</typeparam>
tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/BoundedApiKeyStoreTests.cs
+135
View File
@@ -5,6 +5,7 @@ using Xunit;
using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
using ZB.MOM.WW.OtOpcUa.Core.Authorization;
using ZB.MOM.WW.OtOpcUa.Host.OpcUa;
using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security;
namespace ZB.MOM.WW.OtOpcUa.Host.Tests.OpcUa;
@@ -138,6 +139,137 @@ public sealed class BoundedApiKeyStoreTests
result.Success.ShouldBeFalse();
}
// ---------- TTL read-through cache ----------
/// <summary>A second activation inside the TTL is served from the cache and never reaches the store.</summary>
[Fact]
public async Task A_second_activation_within_the_TTL_does_not_reach_the_store()
{
var (_, inner) = SetUp();
var clock = new FakeClock(new DateTimeOffset(2026, 8, 20, 12, 0, 0, TimeSpan.Zero));
var sut = Bounded(inner, timeoutMs: 0, cacheSeconds: 5, clock);
(await sut.FindAsync(inner.KeyId, clock.GetUtcNow().UtcDateTime, default)).ShouldNotBeNull();
clock.Advance(TimeSpan.FromSeconds(2));
(await sut.FindAsync(inner.KeyId, clock.GetUtcNow().UtcDateTime, default)).ShouldNotBeNull();
inner.Calls.ShouldBe(1);
}
/// <summary>
/// FALSIFICATION CONTROL. A key revoked in the store is denied once the TTL has passed — the
/// cache expires rather than merely existing.
/// </summary>
/// <remarks>
/// A cache with no expiry at all passes every other case in this class: the hit test, the
/// read-through test and the fault test are all satisfied by "store once, serve forever". This is
/// the one that makes <c>StoreCacheSeconds</c> a bound rather than a claim.
/// </remarks>
[Fact]
public async Task A_revoked_key_is_denied_once_the_TTL_has_passed()
{
var (_, inner) = SetUp();
var clock = new FakeClock(new DateTimeOffset(2026, 8, 20, 12, 0, 0, TimeSpan.Zero));
var sut = Bounded(inner, timeoutMs: 0, cacheSeconds: 5, clock);
(await sut.FindAsync(inner.KeyId, clock.GetUtcNow().UtcDateTime, default))
.ShouldNotBeNull("non-vacuity: the key really was usable before the revocation");
inner.Revoked = true;
// Still inside the TTL — the documented, accepted staleness window.
clock.Advance(TimeSpan.FromSeconds(3));
(await sut.FindAsync(inner.KeyId, clock.GetUtcNow().UtcDateTime, default)).ShouldNotBeNull();
// Past it — the store is re-read and the revocation lands.
clock.Advance(TimeSpan.FromSeconds(3));
(await sut.FindAsync(inner.KeyId, clock.GetUtcNow().UtcDateTime, default)).ShouldBeNull();
inner.Calls.ShouldBe(2);
}
/// <summary>A non-positive TTL disables the cache: every call reads through, today's behaviour.</summary>
[Fact]
public async Task TTL_zero_reads_through_every_time()
{
var (_, inner) = SetUp();
var clock = new FakeClock(new DateTimeOffset(2026, 8, 20, 12, 0, 0, TimeSpan.Zero));
var sut = Bounded(inner, timeoutMs: 0, cacheSeconds: 0, clock);
for (var i = 0; i < 3; i++)
await sut.FindAsync(inner.KeyId, clock.GetUtcNow().UtcDateTime, default);
inner.Calls.ShouldBe(3);
}
/// <summary>
/// A fault is never cached: the next call reaches the store, so an outage cannot outlive itself.
/// </summary>
/// <remarks>
/// Caching a deny would be fail-closed and is still wrong — it would go on denying valid keys for
/// a further TTL after the database came back.
/// </remarks>
[Fact]
public async Task A_faulted_store_is_not_cached()
{
var (_, inner) = SetUp();
var clock = new FakeClock(new DateTimeOffset(2026, 8, 20, 12, 0, 0, TimeSpan.Zero));
var sut = Bounded(inner, timeoutMs: 0, cacheSeconds: 60, clock);
inner.ThrowsRemaining = 1;
await Should.ThrowAsync<InvalidOperationException>(
() => sut.FindAsync(inner.KeyId, clock.GetUtcNow().UtcDateTime, default));
// Same instant — a cached fault would still be "fresh" here, so the clock cannot rescue this.
(await sut.FindAsync(inner.KeyId, clock.GetUtcNow().UtcDateTime, default)).ShouldNotBeNull();
inner.Calls.ShouldBe(2);
}
/// <summary>The anonymous grant lookup is cached on the same terms, keyed by principal.</summary>
[Fact]
public async Task Anonymous_grants_are_cached_too()
{
var (_, inner) = SetUp();
var clock = new FakeClock(new DateTimeOffset(2026, 8, 20, 12, 0, 0, TimeSpan.Zero));
var sut = Bounded(inner, timeoutMs: 0, cacheSeconds: 5, clock);
await sut.FindGrantsAsync(OpcUaPrincipals.Anonymous, default);
await sut.FindGrantsAsync(OpcUaPrincipals.Anonymous, default);
inner.Calls.ShouldBe(1);
clock.Advance(TimeSpan.FromSeconds(6));
await sut.FindGrantsAsync(OpcUaPrincipals.Anonymous, default);
inner.Calls.ShouldBe(2);
}
/// <summary>
/// Past the entry ceiling the cache clears wholesale and keeps answering — a key-id guessing flood
/// is a bounded memory event, not an unbounded one.
/// </summary>
/// <remarks>
/// Wholesale rather than per-entry eviction for the reason <c>SessionAclDecisionCache</c> already
/// documents: choosing WHICH entry to drop needs knowledge the cache does not have, and a
/// repopulate is cheap and cannot be wrong.
/// </remarks>
[Fact]
public async Task Cache_overflow_clears_wholesale_and_keeps_serving()
{
var (_, inner) = SetUp();
var clock = new FakeClock(new DateTimeOffset(2026, 8, 20, 12, 0, 0, TimeSpan.Zero));
var sut = Bounded(inner, timeoutMs: 0, cacheSeconds: 600, clock);
// Seed the real key, then flood past the ceiling with ids that do not exist.
(await sut.FindAsync(inner.KeyId, clock.GetUtcNow().UtcDateTime, default)).ShouldNotBeNull();
for (var i = 0; i < BoundedApiKeyStore.MaxCacheEntries + 1; i++)
await sut.FindAsync($"ghost-{i}", clock.GetUtcNow().UtcDateTime, default);
var callsAfterFlood = inner.Calls;
// Still correct after the clear — and the real key was evicted, so it is re-read rather than
// served from a cache the flood was supposed to have emptied.
(await sut.FindAsync(inner.KeyId, clock.GetUtcNow().UtcDateTime, default)).ShouldNotBeNull();
inner.Calls.ShouldBe(callsAfterFlood + 1);
}
// ---------- helpers ----------
private static BoundedApiKeyStore Bounded(
@@ -179,6 +311,9 @@ public sealed class BoundedApiKeyStoreTests
/// </summary>
private sealed class GatedApiKeyStore(ApiKey key) : IApiKeyStore
{
/// <summary>The id of the key this store holds.</summary>
public string KeyId => key.ApiKeyId;
/// <summary>When set, every call awaits this before answering.</summary>
public Task? Gate { get; set; }