From 7912d793e2b911135c838b51572075bf4ec1a647 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 9 Nov 2026 17:22:57 -0500 Subject: [PATCH] fix(host): central https URLs get TLS from a node certificate or refuse to boot (ScadaBridge#108) Central re-bound its HTTP surface from ASPNETCORE_URLS keeping only the port, so https://+:443 silently served plaintext HTTP. The parser now keeps the scheme (ParseHttpBindEndpoints); an https entry binds UseHttps with the node certificate from the new ScadaBridge:Node:Tls section (CertificatePath + CertificatePassword, or CertificateThumbprint + StoreName + StoreLocation; default unset) and the node refuses to boot, with a clear error, when https is requested without a certificate. http entries bind exactly as before. Tests: scheme parsing, refuse-to-boot, http endpoint config identical to the legacy inline binding (reflected effective Kestrel listen options), and a real Kestrel HTTPS listener with a generated .pfx serving h2 and HTTP/1.1 by ALPN while plaintext to the TLS port fails. Reverting the fix turns the refuse-to-boot and real-listener tests red. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV --- .../CentralHttpBinding.cs | 111 +++++++++++ src/ZB.MOM.WW.ScadaBridge.Host/NodeOptions.cs | 34 ++++ src/ZB.MOM.WW.ScadaBridge.Host/Program.cs | 59 ++++-- .../CentralHttpsBindingTests.cs | 173 ++++++++++++++++++ 4 files changed, 357 insertions(+), 20 deletions(-) create mode 100644 src/ZB.MOM.WW.ScadaBridge.Host/CentralHttpBinding.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CentralHttpsBindingTests.cs diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/CentralHttpBinding.cs b/src/ZB.MOM.WW.ScadaBridge.Host/CentralHttpBinding.cs new file mode 100644 index 00000000..0d7f1205 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Host/CentralHttpBinding.cs @@ -0,0 +1,111 @@ +using System.Security.Cryptography.X509Certificates; +using Microsoft.AspNetCore.Server.Kestrel.Core; + +namespace ZB.MOM.WW.ScadaBridge.Host; + +/// One endpoint of central's HTTP surface, parsed from ASPNETCORE_URLS. +/// The TCP port. +/// True when the URL scheme was https. +internal readonly record struct HttpBindEndpoint(int Port, bool IsHttps); + +/// +/// Central's re-declared HTTP/1.1+HTTP/2 Kestrel listeners (ScadaBridge#108). http entries bind +/// exactly as they always did; https entries bind TLS with the node certificate, and refuse to +/// boot when none is configured. +/// +internal static class CentralHttpBinding +{ + /// + /// Loads the node certificate when any endpoint is https. Returns null when none is (the + /// certificate settings are then ignored). Throws when https is requested without a usable certificate. + /// + internal static X509Certificate2? LoadCertificateIfNeeded(IReadOnlyList endpoints, NodeTlsOptions? tls) + { + var httpsPorts = endpoints.Where(e => e.IsHttps).Select(e => e.Port).ToList(); + if (httpsPorts.Count == 0) + { + return null; + } + + if (tls is null || !tls.IsConfigured) + { + throw new InvalidOperationException( + $"ASPNETCORE_URLS requests https on port(s) {string.Join(", ", httpsPorts)} but no node certificate is " + + "configured. Set ScadaBridge:Node:Tls:CertificatePath (+ CertificatePassword) or " + + "ScadaBridge:Node:Tls:CertificateThumbprint, or use an http:// URL. Refusing to boot rather than " + + "serve plaintext on an https port."); + } + + return Load(tls); + } + + /// Loads the certificate from the configured .pfx or store thumbprint (exactly one). + internal static X509Certificate2 Load(NodeTlsOptions tls) + { + var hasPath = !string.IsNullOrWhiteSpace(tls.CertificatePath); + var hasThumb = !string.IsNullOrWhiteSpace(tls.CertificateThumbprint); + if (hasPath == hasThumb) + { + throw new InvalidOperationException( + "ScadaBridge:Node:Tls must set exactly one of CertificatePath or CertificateThumbprint."); + } + + if (hasPath) + { + if (!File.Exists(tls.CertificatePath)) + { + throw new InvalidOperationException( + $"ScadaBridge:Node:Tls:CertificatePath '{tls.CertificatePath}' does not exist."); + } + + return X509CertificateLoader.LoadPkcs12FromFile( + tls.CertificatePath!, string.IsNullOrEmpty(tls.CertificatePassword) ? null : tls.CertificatePassword); + } + + if (!Enum.TryParse(tls.StoreName, ignoreCase: true, out var storeName)) + { + throw new InvalidOperationException($"ScadaBridge:Node:Tls:StoreName '{tls.StoreName}' is not a valid store name."); + } + + var location = tls.StoreLocation?.Trim().ToLowerInvariant() switch + { + "local-machine" or "localmachine" => StoreLocation.LocalMachine, + "current-user" or "currentuser" => StoreLocation.CurrentUser, + _ => throw new InvalidOperationException( + $"ScadaBridge:Node:Tls:StoreLocation '{tls.StoreLocation}' must be 'local-machine' or 'current-user'."), + }; + + using var store = new X509Store(storeName, location); + store.Open(OpenFlags.ReadOnly); + var thumbprint = new string(tls.CertificateThumbprint!.Where(char.IsLetterOrDigit).ToArray()); + var found = store.Certificates.Find(X509FindType.FindByThumbprint, thumbprint, validOnly: false); + if (found.Count == 0 || !found[0].HasPrivateKey) + { + throw new InvalidOperationException( + $"ScadaBridge:Node:Tls certificate with thumbprint {thumbprint} not found (with a private key) in {location}\\{storeName}."); + } + + return found[0]; + } + + /// + /// Binds each endpoint with ListenAnyIP, Http1AndHttp2; https endpoints also + /// UseHttps(certificate). http endpoints are bound exactly as before ScadaBridge#108. + /// + internal static void BindHttpSurface( + KestrelServerOptions options, IReadOnlyList endpoints, X509Certificate2? certificate) + { + foreach (var endpoint in endpoints) + { + options.ListenAnyIP(endpoint.Port, listenOptions => + { + listenOptions.Protocols = HttpProtocols.Http1AndHttp2; + if (endpoint.IsHttps) + { + listenOptions.UseHttps(certificate + ?? throw new InvalidOperationException("https endpoint without a certificate.")); + } + }); + } + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/NodeOptions.cs b/src/ZB.MOM.WW.ScadaBridge.Host/NodeOptions.cs index 3c004478..54f66ec3 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/NodeOptions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/NodeOptions.cs @@ -36,4 +36,38 @@ public class NodeOptions /// with the Akka remoting port a site node binds. /// public int MetricsPort { get; set; } = 8084; + + /// + /// This node's own TLS certificate for the Kestrel HTTP surface (ScadaBridge#108, design + /// 2026-11-09-tls-production-profile stage 1). Default: nothing set. Required if (and only if) + /// ASPNETCORE_URLS contains an https:// entry; central then refuses to boot + /// without it. Applies to central's HTTP/1.1+HTTP/2 surface only; the gRPC listener is unchanged. + /// + public NodeTlsOptions Tls { get; set; } = new(); +} + +/// +/// Certificate source for : a .pfx path plus password, OR a +/// certificate-store thumbprint. Same shape as the cluster's RemotingTls certificate fields. +/// +public sealed class NodeTlsOptions +{ + /// Path to a PKCS#12 (.pfx) file with the private key. Set this OR . + public string? CertificatePath { get; set; } + + /// Password for ; write it as ${secret:NAME}, never a literal. + public string? CertificatePassword { get; set; } + + /// Thumbprint of a certificate in the OS store. Set this OR . + public string? CertificateThumbprint { get; set; } + + /// Store name for . Default My. + public string StoreName { get; set; } = "My"; + + /// Store location for : local-machine (default) or current-user. + public string StoreLocation { get; set; } = "local-machine"; + + /// Whether any certificate source field is set. + public bool IsConfigured => + !string.IsNullOrWhiteSpace(CertificatePath) || !string.IsNullOrWhiteSpace(CertificateThumbprint); } diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs index 298e4a7c..ec463aa0 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs @@ -205,7 +205,11 @@ try var httpUrls = configuration["ASPNETCORE_URLS"] ?? builder.WebHost.GetSetting("urls") ?? "http://+:5000"; - var httpPorts = ParseHttpBindPorts(httpUrls); + var httpEndpoints = ParseHttpBindEndpoints(httpUrls); + var httpPorts = httpEndpoints.Select(e => e.Port).ToList(); + // ScadaBridge#108: an https URL needs a configured node certificate; without one the + // node refuses to boot rather than silently serving plaintext on that port. + var httpsCertificate = CentralHttpBinding.LoadCertificateIfNeeded(httpEndpoints, nodeOptions.Tls); // Register row 56: central's dedicated Prometheus listener, OPT-IN. // @@ -233,13 +237,7 @@ try options.Limits.Http2.KeepAlivePingTimeout = SiteKestrelSetup.Http2KeepAlivePingTimeout; // The HTTP/1.1 (+ HTTP/2) surface from the configured URLs, re-declared so it // survives the explicit-endpoints switch above. - foreach (var httpPort in httpPorts) - { - options.ListenAnyIP(httpPort, listenOptions => - { - listenOptions.Protocols = Microsoft.AspNetCore.Server.Kestrel.Core.HttpProtocols.Http1AndHttp2; - }); - } + CentralHttpBinding.BindHttpSurface(options, httpEndpoints, httpsCertificate); // The gRPC control plane, HTTP/2 h2c only, on its own port. options.ListenAnyIP(centralGrpcPort, listenOptions => @@ -942,29 +940,41 @@ public partial class Program /// /// Extracts the distinct TCP ports from an ASP.NET Core server-URLs string (the - /// ASPNETCORE_URLS / --urls value, e.g. "http://+:5000" or a + /// ASPNETCORE_URLS / --urls value). Port-only view of + /// ; kept for callers that only need the ports. + /// + /// The server-URLs string; may be null/empty. + /// The distinct ports, in first-seen order. + internal static IReadOnlyList ParseHttpBindPorts(string? serverUrls) => + ParseHttpBindEndpoints(serverUrls).Select(e => e.Port).ToList(); + + /// + /// Extracts the distinct (port, scheme) endpoints from an ASP.NET Core server-URLs string + /// (the ASPNETCORE_URLS / --urls value, e.g. "http://+:5000" or a /// semicolon-separated list). Used to re-declare central's HTTP surface after the gRPC - /// listener switches Kestrel into explicit-endpoints mode — see the call site's warning. + /// listener switches Kestrel into explicit-endpoints mode - see the call site's warning. /// /// /// Bind hosts (+, *, 0.0.0.0, [::], a hostname) are irrelevant - /// here because the caller re-binds via ListenAnyIP; only the port matters. A URL - /// with no explicit port falls back to the scheme default (80/443). Unparseable entries - /// are skipped rather than throwing — a bad URL should not take the node down at boot. + /// here because the caller re-binds via ListenAnyIP. The scheme IS kept (ScadaBridge#108): + /// dropping it used to make https://+:443 serve plaintext. A URL with no explicit port + /// falls back to the scheme default (80/443). Unparseable entries are skipped rather than + /// throwing. The same port listed with two schemes is a configuration error and throws. /// /// The server-URLs string; may be null/empty. - /// The distinct ports, in first-seen order. - internal static IReadOnlyList ParseHttpBindPorts(string? serverUrls) + /// The distinct endpoints, in first-seen order. + internal static IReadOnlyList ParseHttpBindEndpoints(string? serverUrls) { - var ports = new List(); + var endpoints = new List(); if (string.IsNullOrWhiteSpace(serverUrls)) { - return ports; + return endpoints; } foreach (var raw in serverUrls.Split(';', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)) { int port; + var isHttps = raw.StartsWith("https://", StringComparison.OrdinalIgnoreCase); // Uri can't parse the wildcard hosts Kestrel accepts (+, *), so normalize them // to a placeholder host before parsing; the host is discarded anyway. var normalized = raw.Replace("://+", "://placeholder").Replace("://*", "://placeholder"); @@ -982,12 +992,21 @@ public partial class Program } } - if (port is > 0 and <= 65535 && !ports.Contains(port)) + if (port is > 0 and <= 65535) { - ports.Add(port); + var existing = endpoints.FindIndex(e => e.Port == port); + if (existing < 0) + { + endpoints.Add(new HttpBindEndpoint(port, isHttps)); + } + else if (endpoints[existing].IsHttps != isHttps) + { + throw new InvalidOperationException( + $"ASPNETCORE_URLS lists port {port} with both http and https schemes; a port is one or the other."); + } } } - return ports; + return endpoints; } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CentralHttpsBindingTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CentralHttpsBindingTests.cs new file mode 100644 index 00000000..a57efcee --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CentralHttpsBindingTests.cs @@ -0,0 +1,173 @@ +using System.Net; +using System.Net.Http; +using System.Net.Sockets; +using System.Reflection; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// ScadaBridge#108 (F1): central re-binds its HTTP surface from ASPNETCORE_URLS. It used to keep only +/// the port, so https://+:443 served plaintext. https must now get TLS from the configured node +/// certificate or refuse to boot; http must bind exactly as before. +/// +public sealed class CentralHttpsBindingTests : IDisposable +{ + private readonly string _dir = Directory.CreateTempSubdirectory("sb108").FullName; + + /// Removes the generated certificate files. + public void Dispose() => Directory.Delete(_dir, recursive: true); + + private (string pfx, X509Certificate2 cert) MakePfx(string password) + { + using var rsa = RSA.Create(2048); + var req = new CertificateRequest("CN=localhost", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + var san = new SubjectAlternativeNameBuilder(); + san.AddDnsName("localhost"); + req.CertificateExtensions.Add(san.Build()); + using var ephemeral = req.CreateSelfSigned(DateTimeOffset.UtcNow.AddMinutes(-5), DateTimeOffset.UtcNow.AddDays(1)); + var bytes = ephemeral.Export(X509ContentType.Pfx, password); + var path = Path.Combine(_dir, "node.pfx"); + File.WriteAllBytes(path, bytes); + return (path, X509CertificateLoader.LoadPkcs12(bytes, password)); + } + + /// The scheme survives parsing. + [Theory] + [InlineData("http://+:5000", 5000, false)] + [InlineData("https://+:443", 443, true)] + [InlineData("HTTPS://*:8443", 8443, true)] + [InlineData("https://localhost", 443, true)] + public void Scheme_is_kept(string url, int port, bool https) + { + Assert.Equal(new HttpBindEndpoint(port, https), Assert.Single(Program.ParseHttpBindEndpoints(url))); + } + + /// One port with two schemes is a config error. + [Fact] + public void Same_port_with_two_schemes_throws() => + Assert.Throws(() => Program.ParseHttpBindEndpoints("http://+:5000;https://+:5000")); + + /// https without a certificate refuses to boot (the F1 downgrade is closed). + [Fact] + public void Https_without_certificate_refuses_to_boot() + { + var eps = Program.ParseHttpBindEndpoints("https://+:443"); + var ex = Assert.Throws(() => CentralHttpBinding.LoadCertificateIfNeeded(eps, new NodeTlsOptions())); + Assert.Contains("Refusing to boot", ex.Message); + } + + /// Both a path and a thumbprint is ambiguous. + [Fact] + public void Path_and_thumbprint_together_throw() + { + var eps = Program.ParseHttpBindEndpoints("https://+:443"); + Assert.Throws(() => CentralHttpBinding.LoadCertificateIfNeeded( + eps, new NodeTlsOptions { CertificatePath = "x.pfx", CertificateThumbprint = "ABCD" })); + } + + /// http-only URLs ignore the certificate settings and load nothing. + [Fact] + public void Http_only_loads_no_certificate() + { + var eps = Program.ParseHttpBindEndpoints("http://+:5000"); + Assert.Null(CentralHttpBinding.LoadCertificateIfNeeded(eps, new NodeTlsOptions { CertificatePath = "missing.pfx" })); + } + + private static List<(string endpoint, HttpProtocols protocols, bool tls)> Effective(Action configure) + { + var options = new KestrelServerOptions + { + ApplicationServices = new ServiceCollection().AddLogging().BuildServiceProvider(), + }; + configure(options); + var prop = typeof(KestrelServerOptions).GetProperty("CodeBackedListenOptions", BindingFlags.Instance | BindingFlags.NonPublic)!; + var isTls = typeof(ListenOptions).GetProperty("IsTls", BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public)!; + return ((System.Collections.IEnumerable)prop.GetValue(options)!).Cast() + .Select(l => (l.IPEndPoint!.ToString(), l.Protocols, (bool)isTls.GetValue(l)!)).ToList(); + } + + /// + /// http URLs produce the same effective Kestrel endpoint configuration as the pre-#108 inline + /// loop (ListenAnyIP + Http1AndHttp2, no TLS). + /// + [Fact] + public void Http_endpoints_are_identical_to_the_legacy_binding() + { + const string urls = "http://+:5000;http://*:8085"; + var legacy = Effective(o => + { + foreach (var port in Program.ParseHttpBindPorts(urls)) + { + o.ListenAnyIP(port, l => l.Protocols = HttpProtocols.Http1AndHttp2); + } + }); + var current = Effective(o => CentralHttpBinding.BindHttpSurface(o, Program.ParseHttpBindEndpoints(urls), null)); + + Assert.Equal(2, current.Count); + Assert.Equal(legacy, current); + Assert.All(current, c => Assert.False(c.tls)); + } + + private static int FreePort() + { + var l = new TcpListener(IPAddress.Loopback, 0); + l.Start(); + var p = ((IPEndPoint)l.LocalEndpoint).Port; + l.Stop(); + return p; + } + + /// + /// A real Kestrel listener bound through the production path (certificate loaded from a .pfx + + /// password) serves TLS on the https port with HTTP/1.1 and h2 by ALPN, and a plaintext client + /// cannot talk to it; the http port still serves plaintext. + /// + [Fact] + public async Task Real_https_listener_serves_tls_and_http_stays_plaintext() + { + var (pfx, expected) = MakePfx("pw"); + int httpsPort = FreePort(), httpPort = FreePort(); + var eps = Program.ParseHttpBindEndpoints($"https://+:{httpsPort};http://+:{httpPort}"); + var loaded = CentralHttpBinding.LoadCertificateIfNeeded(eps, new NodeTlsOptions { CertificatePath = pfx, CertificatePassword = "pw" }); + Assert.Equal(expected.Thumbprint, loaded!.Thumbprint); + + using var host = await new HostBuilder() + .ConfigureWebHost(web => web + .UseKestrel(k => CentralHttpBinding.BindHttpSurface(k, eps, loaded)) + .Configure(app => app.Run(ctx => ctx.Response.WriteAsync($"{ctx.Request.Scheme}/{ctx.Request.Protocol}")))) + .StartAsync(); + try + { + using var handler = new SocketsHttpHandler + { + SslOptions = { RemoteCertificateValidationCallback = (_, c, _, _) => c?.GetCertHashString() == expected.Thumbprint }, + }; + using var client = new HttpClient(handler); + + using var h2 = new HttpRequestMessage(HttpMethod.Get, $"https://localhost:{httpsPort}/") { Version = HttpVersion.Version20, VersionPolicy = HttpVersionPolicy.RequestVersionExact }; + Assert.Equal("https/HTTP/2", await (await client.SendAsync(h2)).Content.ReadAsStringAsync()); // ALPN h2 + + using var h11 = new HttpRequestMessage(HttpMethod.Get, $"https://localhost:{httpsPort}/") { Version = HttpVersion.Version11, VersionPolicy = HttpVersionPolicy.RequestVersionExact }; + Assert.Equal("https/HTTP/1.1", await (await client.SendAsync(h11)).Content.ReadAsStringAsync()); + + // Plaintext to the TLS port must fail (it is not silently plain HTTP any more). + await Assert.ThrowsAnyAsync(() => client.GetStringAsync($"http://localhost:{httpsPort}/")); + + Assert.Equal("http/HTTP/1.1", await client.GetStringAsync($"http://localhost:{httpPort}/")); + } + finally + { + await host.StopAsync(); + } + } +}