Tidy up some irregularities in certificate reloading

src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs

@@ -77,6 +77,7 @@ internal KestrelConfigurationLoader(
     private IList<Action> EndpointsToAdd { get; } = new List<Action>();
 
     private CertificateConfig? DefaultCertificateConfig { get; set; }
+    internal X509Certificate2? DefaultCertificate { get; set; }
 
     /// <summary>
     /// Specifies a configuration Action to run when an endpoint with the given name is loaded from configuration.
@@ -270,9 +271,10 @@ public void Load()
     {
         var endpointsToStop = Options.ConfigurationBackedListenOptions.ToList();
         var endpointsToStart = new List<ListenOptions>();
+        var endpointsToReuse = new List<ListenOptions>();
 
-        Options.ConfigurationBackedListenOptions.Clear();
         DefaultCertificateConfig = null;
+        DefaultCertificate = null;
 
         ConfigurationReader = new ConfigurationReader(Configuration);
 
@@ -336,7 +338,7 @@ public void Load()
                 if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null)
                 {
                     // Fallback
-                    Options.ApplyDefaultCert(httpsOptions);
+                    Options.ApplyDefaultCertificate(httpsOptions);
 
                     // Ensure endpoint is reloaded if it used the default certificate and the certificate changed.
                     endpoint.Certificate = DefaultCertificateConfig;
@@ -350,7 +352,7 @@ public void Load()
             if (matchingBoundEndpoints.Count > 0)
             {
                 endpointsToStop.RemoveAll(o => o.EndpointConfig == endpoint);
-                Options.ConfigurationBackedListenOptions.AddRange(matchingBoundEndpoints);
+                endpointsToReuse.AddRange(matchingBoundEndpoints);
                 continue;
             }
 
@@ -390,9 +392,16 @@ public void Load()
             listenOptions.EndpointConfig = endpoint;
 
             endpointsToStart.Add(listenOptions);
-            Options.ConfigurationBackedListenOptions.Add(listenOptions);
         }
 
+        // Update ConfigurationBackedListenOptions after everything else has been processed so that
+        // it's left in a good state (i.e. its former state) if something above throws an exception.
+        // Note that this isn't foolproof - we could run out of memory or something - but it covers
+        // exceptions resulting from user misconfiguration (e.g. bad endpoint cert password).
+        Options.ConfigurationBackedListenOptions.Clear();
+        Options.ConfigurationBackedListenOptions.AddRange(endpointsToReuse);
+        Options.ConfigurationBackedListenOptions.AddRange(endpointsToStart);
+
         return (endpointsToStop, endpointsToStart);
     }
 
@@ -404,7 +413,7 @@ private void LoadDefaultCert()
             if (defaultCert != null)
             {
                 DefaultCertificateConfig = defaultCertConfig;
-                Options.DefaultCertificate = defaultCert;
+                DefaultCertificate = defaultCert;
             }
         }
         else
@@ -414,7 +423,7 @@ private void LoadDefaultCert()
             {
                 Logger.LocatedDevelopmentCertificate(certificate);
                 DefaultCertificateConfig = certificateConfig;
-                Options.DefaultCertificate = certificate;
+                DefaultCertificate = certificate;
             }
         }
     }

src/Servers/Kestrel/Core/src/KestrelServerOptions.cs

@@ -151,14 +151,22 @@ public class KestrelServerOptions
     private Action<HttpsConnectionAdapterOptions> HttpsDefaults { get; set; } = _ => { };
 
     /// <summary>
-    /// The default server certificate for https endpoints. This is applied lazily after HttpsDefaults and user options.
+    /// The development server certificate for https endpoints. This is applied lazily after HttpsDefaults and user options.
     /// </summary>
-    internal X509Certificate2? DefaultCertificate { get; set; }
+    /// <remarks>
+    /// Getter exposed for testing.
+    /// </remarks>
+    internal X509Certificate2? DevelopmentCertificate { get; private set; }
+
+    /// <summary>
+    /// Allow tests to explicitly set the default certificate.
+    /// </summary>
+    internal X509Certificate2? TestOverrideDefaultCertificate { get; set; }
 
     /// <summary>
     /// Has the default dev certificate load been attempted?
     /// </summary>
-    internal bool IsDevCertLoaded { get; set; }
+    internal bool IsDevelopmentCertificateLoaded { get; set; }
 
     /// <summary>
     /// Internal AppContext switch to toggle the WebTransport and HTTP/3 datagrams experiemental features.
@@ -227,16 +235,34 @@ internal void ApplyHttpsDefaults(HttpsConnectionAdapterOptions httpsOptions)
         HttpsDefaults(httpsOptions);
     }
 
-    internal void ApplyDefaultCert(HttpsConnectionAdapterOptions httpsOptions)
+    internal void ApplyDefaultCertificate(HttpsConnectionAdapterOptions httpsOptions)
     {
         if (httpsOptions.ServerCertificate != null || httpsOptions.ServerCertificateSelector != null)
         {
             return;
         }
 
-        EnsureDefaultCert();
+        if (TestOverrideDefaultCertificate is X509Certificate2 certificateFromTest)
+        {
+            httpsOptions.ServerCertificate = certificateFromTest;
+            return;
+        }
+
+        if (ConfigurationLoader?.DefaultCertificate is X509Certificate2 certificateFromLoader)
+        {
+            httpsOptions.ServerCertificate = certificateFromLoader;
+            return;
+        }
 
-        httpsOptions.ServerCertificate = DefaultCertificate;
+        if (!IsDevelopmentCertificateLoaded)
+        {
+            IsDevelopmentCertificateLoaded = true;
+            Debug.Assert(DevelopmentCertificate is null);
+            var logger = ApplicationServices!.GetRequiredService<ILogger<KestrelServer>>();
+            DevelopmentCertificate = GetDevelopmentCertificateFromStore(logger);
+        }
+
+        httpsOptions.ServerCertificate = DevelopmentCertificate;
     }
 
     internal void Serialize(Utf8JsonWriter writer)
@@ -253,8 +279,8 @@ internal void Serialize(Utf8JsonWriter writer)
         writer.WritePropertyName(nameof(AllowResponseHeaderCompression));
         writer.WriteBooleanValue(AllowResponseHeaderCompression);
 
-        writer.WritePropertyName(nameof(IsDevCertLoaded));
-        writer.WriteBooleanValue(IsDevCertLoaded);
+        writer.WritePropertyName(nameof(IsDevelopmentCertificateLoaded));
+        writer.WriteBooleanValue(IsDevelopmentCertificateLoaded);
 
         writer.WriteString(nameof(RequestHeaderEncodingSelector), RequestHeaderEncodingSelector == DefaultHeaderEncodingSelector ? "default" : "configured");
         writer.WriteString(nameof(ResponseHeaderEncodingSelector), ResponseHeaderEncodingSelector == DefaultHeaderEncodingSelector ? "default" : "configured");
@@ -280,46 +306,44 @@ internal void Serialize(Utf8JsonWriter writer)
         writer.WriteEndArray();
     }
 
-    private void EnsureDefaultCert()
+    private static X509Certificate2? GetDevelopmentCertificateFromStore(ILogger<KestrelServer> logger)
     {
-        if (DefaultCertificate == null && !IsDevCertLoaded)
+        try
         {
-            IsDevCertLoaded = true; // Only try once
-            var logger = ApplicationServices!.GetRequiredService<ILogger<KestrelServer>>();
-            try
+            var cert = CertificateManager.Instance.ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: true, requireExportable: false)
+                .FirstOrDefault();
+
+            if (cert is null)
             {
-                DefaultCertificate = CertificateManager.Instance.ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: true, requireExportable: false)
-                    .FirstOrDefault();
-
-                if (DefaultCertificate != null)
-                {
-                    var status = CertificateManager.Instance.CheckCertificateState(DefaultCertificate, interactive: false);
-                    if (!status.Success)
-                    {
-                        // Display a warning indicating to the user that a prompt might appear and provide instructions on what to do in that
-                        // case. The underlying implementation of this check is specific to Mac OS and is handled within CheckCertificateState.
-                        // Kestrel must NEVER cause a UI prompt on a production system. We only attempt this here because Mac OS is not supported
-                        // in production.
-                        Debug.Assert(status.FailureMessage != null, "Status with a failure result must have a message.");
-                        logger.DeveloperCertificateFirstRun(status.FailureMessage);
-
-                        // Prevent binding to HTTPS if the certificate is not valid (avoid the prompt)
-                        DefaultCertificate = null;
-                    }
-                    else if (!CertificateManager.Instance.IsTrusted(DefaultCertificate))
-                    {
-                        logger.DeveloperCertificateNotTrusted();
-                    }
-                }
-                else
-                {
-                    logger.UnableToLocateDevelopmentCertificate();
-                }
+                logger.UnableToLocateDevelopmentCertificate();
+                return null;
             }
-            catch
+
+            var status = CertificateManager.Instance.CheckCertificateState(cert, interactive: false);
+            if (!status.Success)
             {
-                logger.UnableToLocateDevelopmentCertificate();
+                // Display a warning indicating to the user that a prompt might appear and provide instructions on what to do in that
+                // case. The underlying implementation of this check is specific to Mac OS and is handled within CheckCertificateState.
+                // Kestrel must NEVER cause a UI prompt on a production system. We only attempt this here because Mac OS is not supported
+                // in production.
+                Debug.Assert(status.FailureMessage != null, "Status with a failure result must have a message.");
+                logger.DeveloperCertificateFirstRun(status.FailureMessage);
+
+                // Prevent binding to HTTPS if the certificate is not valid (avoid the prompt)
+                return null;
             }
+
+            if (!CertificateManager.Instance.IsTrusted(cert))
+            {
+                logger.DeveloperCertificateNotTrusted();
+            }
+
+            return cert;
+        }
+        catch
+        {
+            logger.UnableToLocateDevelopmentCertificate();
+            return null;
         }
     }
 

src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs

@@ -166,7 +166,7 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, Action<Ht
         var options = new HttpsConnectionAdapterOptions();
         listenOptions.KestrelServerOptions.ApplyHttpsDefaults(options);
         configureOptions(options);
-        listenOptions.KestrelServerOptions.ApplyDefaultCert(options);
+        listenOptions.KestrelServerOptions.ApplyDefaultCertificate(options);
 
         if (options.ServerCertificate == null && options.ServerCertificateSelector == null)
         {
@@ -181,7 +181,7 @@ internal static bool TryUseHttps(this ListenOptions listenOptions)
     {
         var options = new HttpsConnectionAdapterOptions();
         listenOptions.KestrelServerOptions.ApplyHttpsDefaults(options);
-        listenOptions.KestrelServerOptions.ApplyDefaultCert(options);
+        listenOptions.KestrelServerOptions.ApplyDefaultCertificate(options);
 
         if (options.ServerCertificate == null && options.ServerCertificateSelector == null)
         {

src/Servers/Kestrel/Core/test/KestrelServerTests.cs

@@ -54,7 +54,7 @@ public void StartWithInvalidAddressThrows()
     public void StartWithHttpsAddressConfiguresHttpsEndpoints()
     {
         var options = CreateServerOptions();
-        options.DefaultCertificate = TestResources.GetTestCertificate();
+        options.TestOverrideDefaultCertificate = TestResources.GetTestCertificate();
         using (var server = CreateServer(options))
         {
             server.Features.Get<IServerAddressesFeature>().Addresses.Add("https://127.0.0.1:0");
@@ -70,7 +70,7 @@ public void StartWithHttpsAddressConfiguresHttpsEndpoints()
     public void KestrelServerThrowsUsefulExceptionIfDefaultHttpsProviderNotAdded()
     {
         var options = CreateServerOptions();
-        options.IsDevCertLoaded = true; // Prevent the system default from being loaded
+        options.IsDevelopmentCertificateLoaded = true; // Prevent the system default from being loaded
         using (var server = CreateServer(options, throwOnCriticalErrors: false))
         {
             server.Features.Get<IServerAddressesFeature>().Addresses.Add("https://127.0.0.1:0");