diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/CentralChannelProvider.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/CentralChannelProvider.cs index 26409601..907cad7f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/CentralChannelProvider.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/CentralChannelProvider.cs @@ -117,7 +117,7 @@ public sealed class CentralChannelProvider : IDisposable KeepAlivePingPolicy = HttpKeepAlivePingPolicy.Always, EnableMultipleHttp2Connections = true, }, - }.WithMessageLimits().WithSiteCredentials(pskProvider, siteId); + }.WithMessageLimits().WithSiteCredentials(pskProvider, siteId, endpoints[i]); _channels[i] = GrpcChannel.ForAddress(endpoints[i], channelOptions); _clients[i] = new CentralControlService.CentralControlServiceClient(_channels[i]); diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/ISitePskProvider.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/ISitePskProvider.cs index 9c515fee..bdefcd28 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/ISitePskProvider.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/ISitePskProvider.cs @@ -65,8 +65,16 @@ public interface ISitePskProvider /// credentials to a plaintext channel by default, precisely because a bearer token on h2c is /// readable and replayable by anyone on the path. That is a real and accepted limitation here: /// these listeners are h2c today and the boundary assumes a trusted network. The PSK raises the -/// bar from "anyone who can reach the port" to "anyone who can read the traffic"; TLS on these -/// listeners is the follow-on hardening and requires no change to this code. +/// bar from "anyone who can reach the port" to "anyone who can read the traffic". +/// +/// +/// TLS (ScadaBridge#64). An earlier version of this comment claimed TLS would need no change +/// here. It did: Grpc.Net.Client refuses an https:// address on a channel whose credentials +/// are composed over ("insecure channel credentials and +/// can't use a HttpClient with a 'https' scheme"), so every https endpoint threw at channel +/// construction. now takes the endpoint and composes over +/// for https — the PSK still rides every call, +/// now inside TLS — and keeps the insecure composition, byte for byte, for http. /// /// public static class ControlPlaneCredentials @@ -119,9 +127,15 @@ public static class ControlPlaneCredentials /// Channel options being built. /// Key provider, or null to leave the channel unauthenticated. /// The site this channel talks to. + /// + /// The address the channel will dial. An https address composes the call credentials over + /// TLS channel credentials (ScadaBridge#64); anything else — including null, for callers that + /// predate it — keeps the h2c composition. Must be the same address passed to + /// GrpcChannel.ForAddress, or an https channel throws at construction. + /// /// The same options instance, for chaining. public static GrpcChannelOptions WithSiteCredentials( - this GrpcChannelOptions options, ISitePskProvider? provider, string? siteId) + this GrpcChannelOptions options, ISitePskProvider? provider, string? siteId, string? endpoint = null) { ArgumentNullException.ThrowIfNull(options); @@ -130,9 +144,20 @@ public static class ControlPlaneCredentials return options; } - options.Credentials = ChannelCredentials.Create( - ChannelCredentials.Insecure, ForSite(provider, siteId)); - options.UnsafeUseInsecureChannelCallCredentials = true; + if (IsTlsEndpoint(endpoint)) + { + // TLS: the server certificate is validated by the HTTP handler (OS trust store by + // default, or the handler's own SslOptions), and the call credentials need no opt-in. + options.Credentials = ChannelCredentials.Create( + ChannelCredentials.SecureSsl, ForSite(provider, siteId)); + options.UnsafeUseInsecureChannelCallCredentials = false; + } + else + { + options.Credentials = ChannelCredentials.Create( + ChannelCredentials.Insecure, ForSite(provider, siteId)); + options.UnsafeUseInsecureChannelCallCredentials = true; + } if (options.HttpHandler is not PskRejectionRefreshHandler) { // Null means "let GrpcChannel build its default", which is this same handler shape. @@ -144,6 +169,13 @@ public static class ControlPlaneCredentials return options; } + + /// True when is an absolute https URI. + /// The channel address, or null. + /// Whether the channel will run over TLS. + internal static bool IsTlsEndpoint(string? endpoint) => + Uri.TryCreate(endpoint, UriKind.Absolute, out var uri) + && string.Equals(uri.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase); } /// diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SitePairChannelProvider.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SitePairChannelProvider.cs index d22f94f0..2e8a5e0b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SitePairChannelProvider.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SitePairChannelProvider.cs @@ -584,7 +584,7 @@ public sealed class SitePairChannelProvider : IDisposable KeepAlivePingPolicy = HttpKeepAlivePingPolicy.Always, EnableMultipleHttp2Connections = true } - }.WithMessageLimits().WithSiteCredentials(_pskProvider, siteId); + }.WithMessageLimits().WithSiteCredentials(_pskProvider, siteId, endpoint); return GrpcChannel.ForAddress(endpoint, channelOptions); } diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SiteStreamGrpcClient.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SiteStreamGrpcClient.cs index 67d9a7cd..cd617608 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SiteStreamGrpcClient.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SiteStreamGrpcClient.cs @@ -94,7 +94,7 @@ public class SiteStreamGrpcClient : IAsyncDisposable, IDisposable KeepAlivePingTimeout = options.GrpcKeepAlivePingTimeout, KeepAlivePingPolicy = HttpKeepAlivePingPolicy.Always } - }.WithMessageLimits().WithSiteCredentials(pskProvider, siteIdentifier)); + }.WithMessageLimits().WithSiteCredentials(pskProvider, siteIdentifier, endpoint)); _client = new SiteStreamService.SiteStreamServiceClient(_channel); _logger = logger; } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/Grpc/HttpsEndpointChannelTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/Grpc/HttpsEndpointChannelTests.cs new file mode 100644 index 00000000..71bd047c --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/Grpc/HttpsEndpointChannelTests.cs @@ -0,0 +1,88 @@ +using Grpc.Net.Client; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using ZB.MOM.WW.ScadaBridge.Communication.Grpc; + +namespace ZB.MOM.WW.ScadaBridge.Communication.Tests.Grpc; + +/// +/// Every production builder of a PSK-credentialed control-plane channel must accept an +/// https:// endpoint (ScadaBridge#64). Before the endpoint was threaded into +/// , each composed its credentials over +/// insecure channel credentials and Grpc.Net.Client threw at construction for any https address — +/// in that throw was caught and the site address logged as +/// malformed, so a TLS site was silently undialable. +/// +public class HttpsEndpointChannelTests +{ + private const string Site = "site-a"; + + /// Verifies central's per-site pair provider builds a channel for an https site address. + /// A task that represents the asynchronous operation. + [Fact] + public async Task SitePairChannelProvider_builds_an_https_site_channel() + { + using var provider = new SitePairChannelProvider( + new FixedKeyProvider(), + Options.Create(new CommunicationOptions()), + NullLogger.Instance, + handlerFactory: null, + reachabilityProbe: (_, _) => Task.FromResult(false)); + + provider.UpdateSite(Site, "https://site-a-node-a:8083", nodeBEndpoint: null); + + var target = await provider.ExecuteAsync( + Site, (channel, _) => Task.FromResult(channel.Target), CancellationToken.None); + Assert.Equal("site-a-node-a:8083", target); + } + + /// Verifies a site's central channel provider builds https channels for both centrals. + [Fact] + public void CentralChannelProvider_builds_https_central_channels() + { + using var provider = new CentralChannelProvider( + new[] { "https://central-a:8083", "https://central-b:8083" }, + new FixedKeyProvider(), + Site, + new CommunicationOptions(), + NullLogger.Instance); + } + + /// Verifies the site stream client builds an https channel with credentials. + [Fact] + public void SiteStreamGrpcClient_builds_an_https_channel() + { + using var client = new SiteStreamGrpcClient( + "https://site-a-node-a:8083", + NullLogger.Instance, + new CommunicationOptions(), + new FixedKeyProvider(), + Site); + + Assert.Equal("https://site-a-node-a:8083", client.Endpoint); + } + + /// Verifies the scheme check behind the composition choice. + /// The endpoint. + /// Whether it is TLS. + [Theory] + [InlineData("https://host:8083", true)] + [InlineData("HTTPS://host:8083", true)] + [InlineData("http://host:8083", false)] + [InlineData(null, false)] + [InlineData("", false)] + [InlineData("not a url", false)] + public void IsTlsEndpoint_matches_only_absolute_https(string? endpoint, bool expected) + { + Assert.Equal(expected, ControlPlaneCredentials.IsTlsEndpoint(endpoint)); + } + + private sealed class FixedKeyProvider : ISitePskProvider + { + /// + public ValueTask GetAsync(string siteId, CancellationToken ct) => new("k"); + + /// + public void Invalidate(string siteId) { } + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ControlPlaneTlsEndToEndTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ControlPlaneTlsEndToEndTests.cs new file mode 100644 index 00000000..af138267 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ControlPlaneTlsEndToEndTests.cs @@ -0,0 +1,229 @@ +using System.Net; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Grpc.Core; +using Grpc.Net.Client; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Hosting.Server; +using Microsoft.AspNetCore.Hosting.Server.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; +using ZB.MOM.WW.ScadaBridge.Communication; +using ZB.MOM.WW.ScadaBridge.Communication.Grpc; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// The control-plane PSK over a real TLS listener (ScadaBridge#64): a Kestrel HTTP/2 endpoint with +/// a certificate (h2 negotiated by ALPN), the real , and a +/// client channel built by exactly as the +/// production channel builders do. +/// +/// +/// +/// (what ControlPlaneAuthEndToEndTests +/// uses) has no TLS, so this binds real Kestrel on a loopback ephemeral port. The client trusts the +/// generated certificate by pinning its thumbprint in the handler's validation callback — the test +/// stand-in for "the node trusts the issuing CA"; production uses the OS trust store. +/// +/// +/// The first test is the reason the endpoint argument exists: before it, every channel composed its +/// PSK over insecure channel credentials, and Grpc.Net.Client refuses that on an https address. +/// +/// +public sealed class ControlPlaneTlsEndToEndTests : IAsyncLifetime +{ + private const string SiteKey = "the-site-a-preshared-key"; + + private X509Certificate2 _certificate = null!; + private IHost _host = null!; + private string _httpsAddress = null!; + + /// Generates a localhost certificate and starts a TLS gRPC host with the real interceptor. + /// A task that represents the asynchronous operation. + public async Task InitializeAsync() + { + using (var rsa = RSA.Create(2048)) + { + var request = new CertificateRequest("CN=localhost", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + var san = new SubjectAlternativeNameBuilder(); + san.AddDnsName("localhost"); + request.CertificateExtensions.Add(san.Build()); + request.CertificateExtensions.Add(new X509EnhancedKeyUsageExtension( + new OidCollection { new Oid("1.3.6.1.5.5.7.3.1") }, critical: false)); // serverAuth + using var ephemeral = request.CreateSelfSigned(DateTimeOffset.UtcNow.AddMinutes(-5), DateTimeOffset.UtcNow.AddDays(1)); + // Round-trip through PFX so the private key is usable by SslStream on every platform. + _certificate = X509CertificateLoader.LoadPkcs12(ephemeral.Export(X509ContentType.Pfx), password: null); + } + + _host = await new HostBuilder() + .ConfigureWebHost(web => web + .UseKestrel(kestrel => kestrel.Listen(IPAddress.Loopback, 0, listen => + { + listen.Protocols = HttpProtocols.Http2; + listen.UseHttps(_certificate); + })) + .ConfigureServices(services => + { + // By TYPE, interceptor not in DI — exactly as Program.cs registers it. + services.AddGrpc(o => o.Interceptors.Add()); + services.AddSingleton(Options.Create(new CommunicationOptions { GrpcPsk = SiteKey })); + }) + .Configure(app => + { + app.UseRouting(); + app.UseEndpoints(e => e.MapGrpcService()); + })) + .StartAsync(); + + var bound = _host.Services.GetRequiredService().Features.Get()!.Addresses.Single(); + _httpsAddress = $"https://localhost:{new Uri(bound).Port}"; + } + + /// Stops the host and releases the certificate. + /// A task that represents the asynchronous operation. + public async Task DisposeAsync() + { + await _host.StopAsync(); + _host.Dispose(); + _certificate.Dispose(); + } + + private SocketsHttpHandler PinnedHandler() => new() + { + EnableMultipleHttp2Connections = true, + SslOptions = + { + RemoteCertificateValidationCallback = (_, cert, _, _) => + cert is not null && string.Equals( + cert.GetCertHashString(), _certificate.Thumbprint, StringComparison.OrdinalIgnoreCase), + }, + }; + + private GrpcChannel Channel(string key, string? endpointForCredentials) => + GrpcChannel.ForAddress(_httpsAddress, new GrpcChannelOptions { HttpHandler = PinnedHandler() } + .WithMessageLimits() + .WithSiteCredentials(new FixedPskProvider(key), "site-a", endpointForCredentials)); + + /// + /// Verifies the pre-#64 composition (no endpoint → insecure credentials) cannot even build an + /// https channel. This is the defect the endpoint argument fixes; if Grpc.Net.Client ever stops + /// refusing it, this test says so. + /// + [Fact] + public void Https_channel_with_insecure_composition_throws_at_construction() + { + var ex = Assert.Throws(() => Channel(SiteKey, endpointForCredentials: null)); + + Assert.Contains("insecure channel credentials", ex.Message); + } + + /// Verifies a correctly keyed call succeeds over TLS and carries the site header. + /// A task that represents the asynchronous operation. + [Fact] + public async Task Correct_key_is_accepted_over_tls() + { + using var channel = Channel(SiteKey, _httpsAddress); + var client = new SiteStreamService.SiteStreamServiceClient(channel); + + var reply = await client.PullAuditEventsAsync(new PullAuditEventsRequest { BatchSize = 1 }); + + Assert.False(reply.MoreAvailable); + } + + /// + /// Verifies TLS does not replace the PSK: a wrong key over a perfectly good TLS channel is still + /// refused by the interceptor. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task Wrong_key_is_still_rejected_over_tls() + { + using var channel = Channel("some-other-sites-key", _httpsAddress); + var client = new SiteStreamService.SiteStreamServiceClient(channel); + + var ex = await Assert.ThrowsAsync( + async () => await client.PullAuditEventsAsync(new PullAuditEventsRequest())); + + Assert.Equal(StatusCode.PermissionDenied, ex.StatusCode); + } + + /// + /// Verifies the client validates the server certificate: a handler that does not trust it fails + /// the call rather than sending the PSK to an unverified peer. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task Untrusted_server_certificate_fails_the_call() + { + var distrusting = new SocketsHttpHandler + { + SslOptions = { RemoteCertificateValidationCallback = (_, _, _, _) => false }, + }; + using var channel = GrpcChannel.ForAddress(_httpsAddress, new GrpcChannelOptions { HttpHandler = distrusting } + .WithSiteCredentials(new FixedPskProvider(SiteKey), "site-a", _httpsAddress)); + var client = new SiteStreamService.SiteStreamServiceClient(channel); + + var ex = await Assert.ThrowsAsync( + async () => await client.PullAuditEventsAsync(new PullAuditEventsRequest())); + + // Grpc.Net.Client surfaces a failed handshake as a transport error; what matters is that the + // cause is the certificate check, i.e. the call never reached the server. + Assert.NotEqual(StatusCode.OK, ex.StatusCode); + Assert.Contains(Causes(ex.Status.DebugException), e => e is System.Security.Authentication.AuthenticationException); + } + + private static IEnumerable Causes(Exception? ex) + { + for (; ex is not null; ex = ex.InnerException) + { + yield return ex; + } + } + + /// + /// Verifies the h2c composition is unchanged for an http endpoint: insecure channel credentials + /// with the explicit opt-in, as before #64. + /// + [Fact] + public void Http_endpoint_keeps_the_h2c_composition() + { + var options = new GrpcChannelOptions() + .WithSiteCredentials(new FixedPskProvider(SiteKey), "site-a", "http://site-a:8083"); + + Assert.True(options.UnsafeUseInsecureChannelCallCredentials); + // Building the channel on the http address must still succeed. + using var channel = GrpcChannel.ForAddress("http://site-a:8083", options); + } + + /// Verifies the https endpoint drops the insecure opt-in. + [Fact] + public void Https_endpoint_drops_the_insecure_opt_in() + { + var options = new GrpcChannelOptions() + .WithSiteCredentials(new FixedPskProvider(SiteKey), "site-a", "https://site-a:8083"); + + Assert.False(options.UnsafeUseInsecureChannelCallCredentials); + } + + private sealed class FixedPskProvider(string key) : ISitePskProvider + { + /// + public ValueTask GetAsync(string siteId, CancellationToken ct) => new(key); + + /// + public void Invalidate(string siteId) { } + } + + /// Stub service: the interceptor is under test, not the service. + private sealed class EchoSiteStreamService : SiteStreamService.SiteStreamServiceBase + { + /// + public override Task PullAuditEvents( + PullAuditEventsRequest request, ServerCallContext context) => + Task.FromResult(new PullAuditEventsResponse { MoreAvailable = false }); + } +}