dohertj2/ScadaBridge
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

feat(grpc): PSK-authenticate the site gRPC control plane; drop the vestigial management receptionist registration

Browse Source 
Phase 0 of the ClusterClient→gRPC migration
(docs/plans/2026-07-22-clusterclient-to-grpc-plan.md). Standalone hardening: it
closes a gap that exists today and is a precondition for moving command/control
onto gRPC in later phases.

T0.1 — delete the ManagementActor ClusterClientReceptionist registration.
It was built for an out-of-cluster CLI that was never written: the shipped CLI
speaks HTTP Basic to /management, which asks the actor in-process through
ManagementActorHolder. Nothing in the repo ever sent to /user/management. The
actor still runs there; only the cross-boundary advertisement is gone. Six
documents claimed the CLI used ClusterClient — including the CLI's own README
"Architecture Notes" — and are corrected here rather than left to rot.

T0.2 — record, do not port, the dead integration-routing path.
IntegrationCallRequest is unwired at BOTH ends: RouteIntegrationCallAsync has
zero callers anywhere, and RegisterLocalHandler(Integration, …) appears only in
a test, so production always answers "Integration handler not available". It is
excluded from the gRPC contract (28 of 29 commands migrate) rather than
enshrined on an additive-only wire format, and deleting it during a
transport migration would mix a behavioural change into a change whose whole
value is that behaviour is identical. See
docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md.

T0.3 — preshared-key authentication on SiteStreamService.
The service shipped with no auth at all: plaintext h2c, no interceptor, so
anything that could reach a site node's :8083 could open a live data stream or
read audit rows back via PullAuditEvents/PullSiteCalls. ControlPlaneAuthInterceptor
now gates /sitestream.SiteStreamService/ — modeled on LocalDbSyncAuthInterceptor
(constant-time compare, fail-closed, PermissionDenied) but gating a SET of
service prefixes so phases 1A/1B add services rather than interceptors. LocalDb
sync keeps its own separate key: it authenticates the pair partner, not central,
and collapsing the two would make a site's central-facing key also admit writes
into its database.

Keys are per site (SB-GRPC-PSK-<siteId>), never fleet-wide, so a compromised
site yields only its own. Central attaches them through ControlPlaneCredentials,
which binds CallCredentials to the channel — covering unary and streaming
uniformly, and letting the key resolve asynchronously, which a client
interceptor could not do without blocking. All three central→site channel
creation sites go through it (SiteStreamGrpcClient and both audit pull invokers);
the pull invokers' channel caches are re-keyed by (site, endpoint) because
credentials are per-site and bound to the channel.

Two decisions beyond the plan:

  * StartupValidator now requires GrpcPsk on Site nodes. The plan specified only
    the runtime gate, but fail-closed with no boot check produces a node that
    joins, answers heartbeats and reports healthy while refusing every stream,
    audit pull and telemetry ingest — silent and total. Same reasoning as the
    existing inbound API-key pepper rule.

  * Added Communication:SitePsks as a central-side key map. The plan assumed
    central would read the store, seeded via a dev KEK; the docker rig
    deliberately boots with no master key, so store-only resolution would leave
    it unable to dial its own sites. The store stays primary — it is the only
    source that can serve a site added at runtime — with the map covering
    key-less hosts and one-off pins. Neither source falling back to
    "unauthenticated" is the invariant.

T0.4 — dev keys on both rigs and tests.
34 tests. The seven that matter most exercise a real in-process gRPC stack over
TestServer: the unit tests on either side of the wire would both stay green if
the halves disagreed, and gRPC refuses call credentials on a plaintext channel
by default — the UnsafeUseInsecureChannelCallCredentials opt-in is only provable
by making a real call. They confirm correct key passes on unary AND streaming,
wrong key and no-credentials both get PermissionDenied, and an unresolvable key
fails the call with nothing reaching the service.

OPERATIONAL: a site node upgraded to this build without a key will not boot.
That includes the gitignored deploy/wonder-app-vd03/ overlay.
This commit is contained in:
Joseph Doherty
2026-07-22 17:51:09 -04:00
parent f1ad967083
commit 2ee84af1c0
45 changed files with 1913 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ControlPlaneAuthInterceptorTests.cs
+218
View File
@@ -0,0 +1,218 @@
using Grpc.Core;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using ZB.MOM.WW.ScadaBridge.Communication;
using ZB.MOM.WW.ScadaBridge.Communication.Grpc;
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
/// <summary>
/// The site↔central gRPC control plane's inbound gate (ClusterClient→gRPC migration, T0.3).
/// </summary>
/// <remarks>
/// <para>
/// <c>SiteStreamService</c> shipped unauthenticated: plaintext h2c with no interceptor, so
/// anything that could reach a site node's gRPC port could open a live data stream or pull audit
/// rows back with <c>PullAuditEvents</c>/<c>PullSiteCalls</c>. These tests pin the gate that
/// closes it, and — just as importantly — pin that it does NOT gate LocalDb sync, which has its
/// own interceptor and its own key.
/// </para>
/// <para>
/// Sibling of <see cref="LocalDbSyncAuthInterceptorTests"/>; the two interceptors share a shape
/// deliberately, so the cases mirror each other.
/// </para>
/// </remarks>
public class ControlPlaneAuthInterceptorTests
{
// Real method paths: package `sitestream`, service `SiteStreamService` (sitestream.proto).
private const string SubscribeMethod = "/sitestream.SiteStreamService/SubscribeInstance";
private const string PullAuditMethod = "/sitestream.SiteStreamService/PullAuditEvents";
private const string LocalDbSyncMethod = "/localdb_sync.v1.LocalDbSync/Sync";
private static ControlPlaneAuthInterceptor CreateInterceptor(string? psk)
=> new(
Options.Create(new CommunicationOptions { GrpcPsk = psk ?? "" }),
NullLogger<ControlPlaneAuthInterceptor>.Instance);
private static ServerCallContext CreateContext(string method, string? authorizationHeader)
{
var headers = new Metadata();
if (authorizationHeader is not null)
headers.Add("authorization", authorizationHeader);
return new FakeServerCallContext(method, headers);
}
/// <summary>
/// Minimal <see cref="ServerCallContext"/> carrying just a method name and request headers —
/// the only two things the interceptor reads. Hand-rolled for the same reason the LocalDb
/// sibling hand-rolls one: <c>Grpc.Core.Testing.TestServerCallContext</c> lives in the
/// retired native package and does not exist on the grpc-dotnet stack.
/// </summary>
private sealed class FakeServerCallContext(string method, Metadata requestHeaders)
: ServerCallContext
{
protected override string MethodCore => method;
protected override string HostCore => "localhost";
protected override string PeerCore => "ipv4:127.0.0.1:12345";
protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1);
protected override Metadata RequestHeadersCore => requestHeaders;
protected override CancellationToken CancellationTokenCore => CancellationToken.None;
protected override Metadata ResponseTrailersCore { get; } = [];
protected override Status StatusCore { get; set; }
protected override WriteOptions? WriteOptionsCore { get; set; }
protected override AuthContext AuthContextCore { get; } =
new(null, new Dictionary<string, List<AuthProperty>>());
protected override ContextPropagationToken CreatePropagationTokenCore(
ContextPropagationOptions? options)
=> throw new NotSupportedException();
protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders)
=> Task.CompletedTask;
}
/// <summary>Invokes the interceptor's unary path with a trivial continuation.</summary>
private static Task<string> Invoke(
ControlPlaneAuthInterceptor interceptor, ServerCallContext context)
=> interceptor.UnaryServerHandler<string, string>(
"request", context, (_, _) => Task.FromResult("ok"));
[Fact]
public async Task LocalDbSyncMethod_PassesThrough_BecauseItHasItsOwnGateAndItsOwnKey()
{
// Both interceptors sit on the same site AddGrpc pipeline and see every call. If this
// one also gated sync, a site would need its central-facing key to equal its pair-replication
// key — collapsing two distinct trust relationships into one secret.
var interceptor = CreateInterceptor("the-site-key");
var context = CreateContext(LocalDbSyncMethod, authorizationHeader: null);
Assert.Equal("ok", await Invoke(interceptor, context));
}
[Fact]
public async Task GatedMethod_WithNoKeyConfigured_IsDenied_EvenWithABearerToken()
{
// Fail-closed, and this is the case that differs in consequence from LocalDb's: an
// unset key here does not disable an optional feature, it closes the site's entire
// central-facing surface. Loud refusal beats silent unauthenticated service.
var interceptor = CreateInterceptor(psk: null);
var context = CreateContext(SubscribeMethod, "Bearer anything-at-all");
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
}
[Fact]
public async Task GatedMethod_WithNoBearerToken_IsDenied()
{
var interceptor = CreateInterceptor("the-site-key");
var context = CreateContext(SubscribeMethod, authorizationHeader: null);
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
}
[Fact]
public async Task GatedMethod_WithWrongBearerToken_IsDenied()
{
var interceptor = CreateInterceptor("the-site-key");
var context = CreateContext(SubscribeMethod, "Bearer some-other-sites-key");
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
}
[Fact]
public async Task GatedMethod_WithCorrectBearerToken_PassesThrough()
{
var interceptor = CreateInterceptor("the-site-key");
var context = CreateContext(SubscribeMethod, "Bearer the-site-key");
Assert.Equal("ok", await Invoke(interceptor, context));
}
[Fact]
public async Task GatedMethod_WithCorrectKey_ButNoBearerScheme_IsDenied()
{
// A raw key with no "Bearer " prefix is not what ControlPlaneCredentials sends;
// accepting it would widen the accepted credential shape for nothing.
var interceptor = CreateInterceptor("the-site-key");
var context = CreateContext(SubscribeMethod, "the-site-key");
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
}
[Fact]
public async Task GatedMethod_TokenComparison_IsNotAPrefixMatch()
{
// A StartsWith comparison would accept a truncated key and make the secret recoverable
// one character at a time. FixedTimeEquals also rejects on length.
var interceptor = CreateInterceptor("the-site-key");
var context = CreateContext(SubscribeMethod, "Bearer the-site-ke");
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
}
[Fact]
public async Task ServerStreaming_IsGated_BecauseThatIsHowSubscriptionsActuallyRun()
{
// SubscribeInstance/SubscribeSite are server-streaming. Gating only the unary path
// would leave the live data feed wide open while every unary test still passed.
var interceptor = CreateInterceptor("the-site-key");
var context = CreateContext(SubscribeMethod, "Bearer the-wrong-key");
var ex = await Assert.ThrowsAsync<RpcException>(() =>
interceptor.ServerStreamingServerHandler<string, string>(
"request",
responseStream: null!,
context,
(_, _, _) => Task.CompletedTask));
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
}
[Fact]
public async Task PullRpcs_AreGated_BecauseTheyReturnAuditRows()
{
// The strongest reason this gate exists: PullAuditEvents/PullSiteCalls hand back audit
// content to anyone who asks. Unary, so it would be easy to miss in a streaming-focused
// reading of the service.
var interceptor = CreateInterceptor("the-site-key");
var context = CreateContext(PullAuditMethod, authorizationHeader: null);
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, context));
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
}
[Fact]
public async Task GatedPrefixes_AreConstructorProvided_SoLaterPhasesAddServicesNotInterceptors()
{
// Phases 1A/1B add CentralControlService and SiteCommandService to this same gate.
var interceptor = new ControlPlaneAuthInterceptor(
Options.Create(new CommunicationOptions { GrpcPsk = "k" }),
NullLogger<ControlPlaneAuthInterceptor>.Instance,
new[] { "/scadabridge.sitecommand.v1.SiteCommandService/" });
var gated = CreateContext("/scadabridge.sitecommand.v1.SiteCommandService/ExecuteQuery", null);
var ex = await Assert.ThrowsAsync<RpcException>(() => Invoke(interceptor, gated));
Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode);
// ...and the default set is no longer implied once an explicit set is supplied.
var notGated = CreateContext(SubscribeMethod, authorizationHeader: null);
Assert.Equal("ok", await Invoke(interceptor, notGated));
}
[Fact]
public void DefaultGatedPrefixes_MatchTheRealSiteStreamServicePath()
{
// A typo here disables the whole gate silently: every call would simply pass through.
// Pin it against a path taken from the generated service, not from the proto text.
var method = SiteStreamService.Descriptor.FullName;
Assert.Contains(
ControlPlaneAuthInterceptor.DefaultGatedPrefixes,
p => p == $"/{method}/");
}
}