Create CircuitSecret

Fixes: #13012

This change introduces CircuitSecret as the concept that's used when
doing handshaking between the client and the server, and makes CircuitId
(visible to user-code) a separate concept (not a secret, can't be used
to open a connection).

The scope of this grew once I realized that we probably shouldn't be
logging Circuit Secret in so many places, we should be logging CircuitId
as the piece of data we use for correlation, and try to keep the secret
out of logs except where really necessary (and with trace level).

I ended up creating a new type to represent the secret and prevent
accidental misuse, and then chased down all of the build errors.

Author: rynowak

src/Components/Server/src/CircuitDisconnectMiddleware.cs

@@ -17,18 +17,18 @@ internal class CircuitDisconnectMiddleware
         public CircuitDisconnectMiddleware(
             ILogger<CircuitDisconnectMiddleware> logger,
             CircuitRegistry registry,
-            CircuitIdFactory circuitIdFactory,
+            CircuitSecretFactory circuitSecretFactory,
             RequestDelegate next)
         {
             Logger = logger;
             Registry = registry;
-            CircuitIdFactory = circuitIdFactory;
+            CircuitSecretFactory = circuitSecretFactory;
             Next = next;
         }
 
         public ILogger<CircuitDisconnectMiddleware> Logger { get; }
         public CircuitRegistry Registry { get; }
-        public CircuitIdFactory CircuitIdFactory { get; }
+        public CircuitSecretFactory CircuitSecretFactory { get; }
         public RequestDelegate Next { get; }
 
         public async Task Invoke(HttpContext context)
@@ -39,65 +39,66 @@ public async Task Invoke(HttpContext context)
                 return;
             }
 
-            var (hasCircuitId, circuitId) = await TryGetCircuitIdAsync(context);
-            if (!hasCircuitId)
+            var circuitSecret = await GetCircuitSecretAsync(context);
+            if (circuitSecret is null)
             {
                 context.Response.StatusCode = StatusCodes.Status400BadRequest;
                 return;
             }
 
-            await TerminateCircuitGracefully(circuitId);
+            await TerminateCircuitGracefully(circuitSecret.Value);
 
             context.Response.StatusCode = StatusCodes.Status200OK;
         }
 
-        private async Task<(bool, string)> TryGetCircuitIdAsync(HttpContext context)
+        private async Task<CircuitSecret?> GetCircuitSecretAsync(HttpContext context)
         {
             try
             {
                 if (!context.Request.HasFormContentType)
                 {
-                    return (false, null);
+                    return default;
                 }
 
                 var form = await context.Request.ReadFormAsync();
-                if (!form.TryGetValue(CircuitIdKey, out var circuitId) || !CircuitIdFactory.ValidateCircuitId(circuitId))
+                if (!form.TryGetValue(CircuitIdKey, out var text))
                 {
-                    return (false, null);
+                    return default;
                 }
 
-                return (true, circuitId);
+                var circuitSecret = new CircuitSecret(text);
+                if (!CircuitSecretFactory.ValidateCircuitSecret(circuitSecret))
+                {
+                    return default;
+                }
+
+                return circuitSecret;
             }
             catch
             {
-                return (false, null);
+                return default;
             }
         }
 
-        private async Task TerminateCircuitGracefully(string circuitId)
+        private async Task TerminateCircuitGracefully(CircuitSecret circuitSecret)
         {
-            try
-            {
-                await Registry.TerminateAsync(circuitId);
-                Log.CircuitTerminatedGracefully(Logger, circuitId);
-            }
-            catch (Exception e)
-            {
-                Log.UnhandledExceptionInCircuit(Logger, circuitId, e);
-            }
+            // We don't expect TerminateAsync to throw.
+            Log.CircuitTerminatingGracefully(Logger, circuitSecret);
+            await Registry.TerminateAsync(circuitSecret);
+            Log.CircuitTerminatedGracefully(Logger, circuitSecret);
         }
 
         private class Log
         {
-            private static readonly Action<ILogger, string, Exception> _circuitTerminatedGracefully =
-                LoggerMessage.Define<string>(LogLevel.Debug, new EventId(1, "CircuitTerminatedGracefully"), "Circuit '{CircuitId}' terminated gracefully");
+            private static readonly Action<ILogger, string, Exception> _circuitTerminatingGracefully =
+                LoggerMessage.Define<string>(LogLevel.Trace, new EventId(1, "CircuitTerminatingGracefully"), "Circuit '{CircuitSecret}' terminating gracefully.");
 
-            private static readonly Action<ILogger, string, Exception> _unhandledExceptionInCircuit =
-                LoggerMessage.Define<string>(LogLevel.Warning, new EventId(2, "UnhandledExceptionInCircuit"), "Unhandled exception in circuit {CircuitId} while terminating gracefully.");
+            private static readonly Action<ILogger, string, Exception> _circuitTerminatedGracefully =
+                LoggerMessage.Define<string>(LogLevel.Trace, new EventId(2, "CircuitTerminatedGracefully"), "Circuit '{CircuitSecret}' terminated gracefully.");
 
-            public static void CircuitTerminatedGracefully(ILogger logger, string circuitId) => _circuitTerminatedGracefully(logger, circuitId, null);
+            public static void CircuitTerminatingGracefully(ILogger logger, CircuitSecret circuitSecret) => _circuitTerminatingGracefully(logger, circuitSecret.Secret, null);
 
-            public static void UnhandledExceptionInCircuit(ILogger logger, string circuitId, Exception exception) => _unhandledExceptionInCircuit(logger, circuitId, exception);
+            public static void CircuitTerminatedGracefully(ILogger logger, CircuitSecret circuitSecret) => _circuitTerminatedGracefully(logger, circuitSecret.Secret, null);
         }
     }
 }

src/Components/Server/src/Circuits/CircuitHost.cs

@@ -40,6 +40,7 @@ internal class CircuitHost : IAsyncDisposable
 
         public CircuitHost(
             string circuitId,
+            CircuitSecret circuitSecret,
             IServiceScope scope,
             CircuitOptions options,
             CircuitClientProxy client,
@@ -50,6 +51,12 @@ public CircuitHost(
             ILogger logger)
         {
             CircuitId = circuitId ?? throw new ArgumentNullException(nameof(circuitId));
+            CircuitSecret = circuitSecret;
+            if (circuitSecret.Secret is null)
+            {
+                // Prevent the use of a 'default' secret.
+                throw new ArgumentException(nameof(circuitSecret));
+            }
 
             _scope = scope ?? throw new ArgumentNullException(nameof(scope));
             _options = options ?? throw new ArgumentNullException(nameof(options));
@@ -71,8 +78,18 @@ public CircuitHost(
 
         public CircuitHandle Handle { get; }
 
+        /// <summary>
+        /// Gets a reference to the Circuit ID. This is an identifier that can be used safely in server-side
+        /// code to identify the circuit for logging, correlation or remote storage.
+        /// </summary>
         public string CircuitId { get; }
 
+        /// <summary>
+        /// Gets a reference to the Circuit Secret. This is an identifier that is used to make the handshake
+        /// between the client and server. This should not exposed to user-code.
+        /// </summary>
+        public CircuitSecret CircuitSecret { get; }
+
         public Circuit Circuit { get; }
 
         public CircuitClientProxy Client { get; set; }
@@ -648,7 +665,7 @@ private static class EventIds
                 public static readonly EventId LocationChangeSucceded = new EventId(209, "LocationChangeSucceeded");
                 public static readonly EventId LocationChangeFailed = new EventId(210, "LocationChangeFailed");
                 public static readonly EventId LocationChangeFailedInCircuit = new EventId(211, "LocationChangeFailedInCircuit");
-                public static readonly EventId OnRenderCompletedFailed = new EventId(212, " OnRenderCompletedFailed");
+                public static readonly EventId OnRenderCompletedFailed = new EventId(212, "OnRenderCompletedFailed");
             }
 
             static Log()