fix(mesh): stop reporting override-keyed clusters as REVOKED (lmxopcua#784)
The docker-dev centrals logged "Mesh PSK secret ot-mesh-psk-<cluster> ... is absent (or blank) in the store; the key is REVOKED" for every cluster on every contact refresh, while every gRPC dispatch and ack authenticated. Investigated: not an auth bypass. The rig supplies each cluster's key through MeshTransport:ClusterPsks (central side, equal to that cluster's nodes' MeshTransport:GrpcPsk) and seeds nothing in the store. TryGetPsk/Matches consult the override first on every call, so enforcement was on; only RefreshAsync read the store for those clusters regardless and logged a revocation that never took effect. RefreshAsync now skips clusters covered by a non-blank override, so the REVOKED warning is only logged for a cluster that genuinely has no key. Test pins it with a positive control (a cluster with no override is still read, warned about and refused) and enforcement on the override key (wrong token rejected). Mutation-checked: removing the skip fails the test. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
@@ -190,6 +190,12 @@ hold.
|
||||
absent (or blank) in the store; the key is REVOKED — commands to that cluster and acks from it are refused`. Yet
|
||||
every gRPC dispatch and ack in these runs succeeded. Either the rig's PSK secrets are not seeded and enforcement is
|
||||
off, or the warning is stale. Not investigated.
|
||||
**Resolved (lmxopcua#784): the warning was false, not a bypass.** The rig's centrals supply all three keys
|
||||
through `MeshTransport__ClusterPsks__{MAIN,SITE-A,SITE-B}` (each equal to that cluster's nodes'
|
||||
`MeshTransport__GrpcPsk`) and seed nothing in the store. `MeshPskProvider.TryGetPsk` consults that override
|
||||
first on every lookup, so enforcement was on and every call authenticated against the override key; only
|
||||
the background `RefreshAsync` read the store for those clusters anyway and reported them revoked. The
|
||||
refresh now skips override-covered clusters, so `REVOKED` is logged only for a cluster with no key at all.
|
||||
3. **A surgical raw-tag add respawns that driver.** The driver's spawn spec carries its tag set, so adding one tag
|
||||
reconnects the whole driver (the accepted cost noted in `DriverSpawnPlanner`). On a live subscriber this shows as
|
||||
one `BadNoCommunication` sample (< 1 ms) on every value of that driver, including UNS references and virtual tags
|
||||
|
||||
@@ -345,7 +345,31 @@ public sealed class MeshPskProvider : IMeshPskProvider
|
||||
ArgumentNullException.ThrowIfNull(clusterIds);
|
||||
|
||||
var previous = _refreshed;
|
||||
var ids = clusterIds.Where(id => !string.IsNullOrWhiteSpace(id)).Distinct(StringComparer.OrdinalIgnoreCase).ToList();
|
||||
|
||||
// lmxopcua#784: a cluster whose key comes from the MeshTransport:ClusterPsks override is never
|
||||
// served from the store (TryGetPsk consults the override first, on every call), so reading the
|
||||
// store for it is pointless — and reporting "absent in the store; the key is REVOKED" for it is
|
||||
// false: its commands and acks ARE authenticated, against the override key. The docker-dev rig
|
||||
// supplies every cluster's key this way and logged that warning for all three on every contact
|
||||
// refresh while every dispatch and ack authenticated. Skip those clusters here so the REVOKED
|
||||
// warning is only ever logged for a cluster that genuinely has no key.
|
||||
var overrides = _options.Value.ClusterPsks;
|
||||
var all = clusterIds.Where(id => !string.IsNullOrWhiteSpace(id)).Distinct(StringComparer.OrdinalIgnoreCase).ToList();
|
||||
var ids = new List<string>(all.Count);
|
||||
foreach (var id in all)
|
||||
{
|
||||
if (overrides.TryGetValue(id, out var configured) && !string.IsNullOrWhiteSpace(configured))
|
||||
{
|
||||
_logger.LogDebug(
|
||||
"Mesh PSK for cluster {ClusterId} is supplied by MeshTransport:ClusterPsks; the store is not "
|
||||
+ "consulted and commands to / acks from that cluster are authenticated against that key.",
|
||||
id);
|
||||
continue;
|
||||
}
|
||||
|
||||
ids.Add(id);
|
||||
}
|
||||
|
||||
var results = new ConcurrentDictionary<string, StoreRead>(StringComparer.OrdinalIgnoreCase);
|
||||
|
||||
// Bounded fan-out; each read bounded like the synchronous path.
|
||||
|
||||
+47
@@ -1,3 +1,4 @@
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using Shouldly;
|
||||
@@ -129,6 +130,52 @@ public sealed class MeshPskProviderRefreshTests
|
||||
foreach (var c in clusters) provider.TryGetPsk(c, out _).ShouldBeTrue();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// lmxopcua#784 — the docker-dev centrals supply every cluster's key through
|
||||
/// <c>MeshTransport:ClusterPsks</c> and seed nothing in the store, and logged "absent in the store; the
|
||||
/// key is REVOKED — commands … refused" for all three clusters on every contact refresh while every
|
||||
/// dispatch and ack authenticated. The override wins on every lookup, so the key was never revoked: the
|
||||
/// warning was false. The refresh now leaves override-covered clusters alone. SITE-B (no override) is the
|
||||
/// positive control — a cluster with no key anywhere still gets the REVOKED warning and is refused.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task A_cluster_keyed_by_the_config_override_is_not_reported_revoked_and_stays_authenticated()
|
||||
{
|
||||
var resolver = new GatedResolver { Value = null }; // the store holds nothing, as on the rig
|
||||
var options = new MeshTransportOptions();
|
||||
options.ClusterPsks["SITE-A"] = "rig-psk-site-a";
|
||||
var logger = new RecordingLogger();
|
||||
var provider = new MeshPskProvider(Options.Create(options), resolver, logger);
|
||||
|
||||
await provider.RefreshAsync(new[] { "site-a", "SITE-B" }, CancellationToken.None);
|
||||
|
||||
resolver.Calls.ShouldBe(1, "only SITE-B, which has no override, is read from the store");
|
||||
var revoked = logger.Entries.Where(e => e.Level == LogLevel.Warning && e.Message.Contains("REVOKED")).ToList();
|
||||
revoked.Count.ShouldBe(1);
|
||||
revoked[0].Message.ShouldContain("SITE-B");
|
||||
|
||||
// Enforcement is real for both: SITE-A authenticates against the override key and nothing else,
|
||||
// SITE-B (no key anywhere) is refused.
|
||||
provider.Matches("SITE-A", "rig-psk-site-a").ShouldBeTrue();
|
||||
provider.Matches("SITE-A", "wrong").ShouldBeFalse();
|
||||
provider.TryGetPsk("SITE-B", out _).ShouldBeFalse();
|
||||
}
|
||||
|
||||
private sealed class RecordingLogger : ILogger<MeshPskProvider>
|
||||
{
|
||||
public List<(LogLevel Level, string Message)> Entries { get; } = new();
|
||||
|
||||
public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
|
||||
|
||||
public bool IsEnabled(LogLevel logLevel) => true;
|
||||
|
||||
public void Log<TState>(
|
||||
LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
|
||||
{
|
||||
lock (Entries) Entries.Add((logLevel, formatter(state, exception)));
|
||||
}
|
||||
}
|
||||
|
||||
private sealed class ConcurrencyTrackingResolver : ISecretResolver
|
||||
{
|
||||
private int _inFlight;
|
||||
|
||||
Reference in New Issue
Block a user