fix(grpc): control-plane PSK credentials work on https endpoints (ScadaBridge#64)
WithSiteCredentials always composed the per-site PSK over
ChannelCredentials.Insecure. Grpc.Net.Client refuses that on an https
address ("insecure channel credentials and can't use a HttpClient with a
'https' scheme"), so every https control-plane endpoint threw at channel
construction - in SitePairChannelProvider the throw was caught and the
site address logged as malformed, i.e. a TLS site was silently
undialable. The old comment claiming TLS needed no change here was wrong.
WithSiteCredentials now takes the endpoint: https composes over
ChannelCredentials.SecureSsl (PSK retained inside TLS, no insecure
opt-in); anything else, including callers that omit it, keeps the
previous composition unchanged. SitePairChannelProvider,
SiteStreamGrpcClient and CentralChannelProvider pass their endpoint. The
three AuditLog pull clients are deliberately left for after #42.
Tests: real Kestrel HTTP/2-over-TLS (ALPN) with the real
ControlPlaneAuthInterceptor - correct PSK accepted, wrong PSK still
PermissionDenied, untrusted server cert fails with an
AuthenticationException cause, the pre-fix composition throws on https,
http composition unchanged; plus each production builder accepts https
(mutation check: disabling the TLS branch turns those red).
Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
@@ -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]);
|
||||
|
||||
@@ -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".
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>TLS (ScadaBridge#64).</b> An earlier version of this comment claimed TLS would need no change
|
||||
/// here. It did: Grpc.Net.Client refuses an <c>https://</c> address on a channel whose credentials
|
||||
/// are composed over <see cref="ChannelCredentials.Insecure"/> ("insecure channel credentials and
|
||||
/// can't use a HttpClient with a 'https' scheme"), so every https endpoint threw at channel
|
||||
/// construction. <see cref="WithSiteCredentials"/> now takes the endpoint and composes over
|
||||
/// <see cref="ChannelCredentials.SecureSsl"/> for <c>https</c> — the PSK still rides every call,
|
||||
/// now inside TLS — and keeps the insecure composition, byte for byte, for <c>http</c>.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public static class ControlPlaneCredentials
|
||||
@@ -119,9 +127,15 @@ public static class ControlPlaneCredentials
|
||||
/// <param name="options">Channel options being built.</param>
|
||||
/// <param name="provider">Key provider, or null to leave the channel unauthenticated.</param>
|
||||
/// <param name="siteId">The site this channel talks to.</param>
|
||||
/// <param name="endpoint">
|
||||
/// The address the channel will dial. An <c>https</c> 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
|
||||
/// <c>GrpcChannel.ForAddress</c>, or an https channel throws at construction.
|
||||
/// </param>
|
||||
/// <returns>The same options instance, for chaining.</returns>
|
||||
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;
|
||||
}
|
||||
|
||||
/// <summary>True when <paramref name="endpoint"/> is an absolute <c>https</c> URI.</summary>
|
||||
/// <param name="endpoint">The channel address, or null.</param>
|
||||
/// <returns>Whether the channel will run over TLS.</returns>
|
||||
internal static bool IsTlsEndpoint(string? endpoint) =>
|
||||
Uri.TryCreate(endpoint, UriKind.Absolute, out var uri)
|
||||
&& string.Equals(uri.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user