diff --git a/docs/reviews/2026-10-21/gates.md b/docs/reviews/2026-10-21/gates.md index 7efe7868b..ec7b178a2 100644 --- a/docs/reviews/2026-10-21/gates.md +++ b/docs/reviews/2026-10-21/gates.md @@ -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 diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/IMeshPskProvider.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/IMeshPskProvider.cs index bad1956ce..a217d1d63 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/IMeshPskProvider.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Communication/IMeshPskProvider.cs @@ -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(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(StringComparer.OrdinalIgnoreCase); // Bounded fan-out; each read bounded like the synchronous path. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshPskProviderRefreshTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshPskProviderRefreshTests.cs index 34129235d..259fa42f4 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshPskProviderRefreshTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Communication/MeshPskProviderRefreshTests.cs @@ -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(); } + /// + /// lmxopcua#784 — the docker-dev centrals supply every cluster's key through + /// MeshTransport:ClusterPsks 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. + /// + [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 + { + public List<(LogLevel Level, string Message)> Entries { get; } = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + lock (Entries) Entries.Add((logLevel, formatter(state, exception))); + } + } + private sealed class ConcurrencyTrackingResolver : ISecretResolver { private int _inFlight;