Skip to content

Commit 3a6fb8d

Browse files
amcaseyamcasey
authored
Revert "Defer search for dev cert until build bind time (#46296)" (#46868)
This reverts commit 303115a. Changing the order in which various certificate loading operations are performed broke dotnet-monitor. dotnet/dotnet-docker#4448 (comment)
1 parent 965a8df commit 3a6fb8d

File tree

9 files changed

+56
-268
lines changed

9 files changed

+56
-268
lines changed

‎src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ public HttpsConnectionAdapterOptions()
7272
/// </summary>
7373
public SslProtocols SslProtocols { get; set; }
7474

75+
/// <summary>
76+
/// The protocols enabled on this endpoint.
77+
/// </summary>
78+
/// <remarks>Defaults to HTTP/1.x only.</remarks>
79+
internal HttpProtocols HttpProtocols { get; set; }
80+
7581
/// <summary>
7682
/// Specifies whether the certificate revocation list is checked during authentication.
7783
/// </summary>

‎src/Servers/Kestrel/Core/src/Internal/Infrastructure/TransportManager.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public async Task<EndPoint> BindAsync(EndPoint endPoint, MultiplexedConnectionDe
7777
// The QUIC transport will check if TlsConnectionCallbackOptions is missing.
7878
if (listenOptions.HttpsOptions != null)
7979
{
80-
var sslServerAuthenticationOptions = HttpsConnectionMiddleware.CreateHttp3Options(listenOptions.HttpsOptions.Value);
80+
var sslServerAuthenticationOptions = HttpsConnectionMiddleware.CreateHttp3Options(listenOptions.HttpsOptions);
8181
features.Set(new TlsConnectionCallbackOptions
8282
{
8383
ApplicationProtocols = sslServerAuthenticationOptions.ApplicationProtocols ?? new List<SslApplicationProtocol> { SslApplicationProtocol.Http3 },

‎src/Servers/Kestrel/Core/src/ListenOptions.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,7 @@ internal string Scheme
140140
}
141141

142142
internal bool IsTls { get; set; }
143-
/// <remarks>Should not be inspected until the configuration has been loaded.</remarks>
144-
internal Lazy<HttpsConnectionAdapterOptions>? HttpsOptions { get; set; }
143+
internal HttpsConnectionAdapterOptions? HttpsOptions { get; set; }
145144
internal TlsHandshakeCallbackOptions? HttpsCallbackOptions { get; set; }
146145

147146
/// <summary>

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

Lines changed: 28 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -163,24 +163,33 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, Action<Ht
163163
{
164164
ArgumentNullException.ThrowIfNull(configureOptions);
165165

166-
return listenOptions.UseHttps(new Lazy<HttpsConnectionAdapterOptions>(() =>
166+
var options = new HttpsConnectionAdapterOptions();
167+
listenOptions.KestrelServerOptions.ApplyHttpsDefaults(options);
168+
configureOptions(options);
169+
listenOptions.KestrelServerOptions.ApplyDefaultCert(options);
170+
171+
if (options.ServerCertificate == null && options.ServerCertificateSelector == null)
167172
{
168-
// We defer configuration of the https options until build time so that the IConfiguration will be available.
169-
// This is particularly important in docker containers, where the docker tools use IConfiguration to tell
170-
// us where the development certificates have been mounted.
173+
throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound);
174+
}
171175

172-
var options = new HttpsConnectionAdapterOptions();
173-
listenOptions.KestrelServerOptions.ApplyHttpsDefaults(options);
174-
configureOptions(options);
175-
listenOptions.KestrelServerOptions.ApplyDefaultCert(options);
176+
return listenOptions.UseHttps(options);
177+
}
176178

177-
if (options.ServerCertificate == null && options.ServerCertificateSelector == null)
178-
{
179-
throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound);
180-
}
179+
// Use Https if a default cert is available
180+
internal static bool TryUseHttps(this ListenOptions listenOptions)
181+
{
182+
var options = new HttpsConnectionAdapterOptions();
183+
listenOptions.KestrelServerOptions.ApplyHttpsDefaults(options);
184+
listenOptions.KestrelServerOptions.ApplyDefaultCert(options);
181185

182-
return options;
183-
}));
186+
if (options.ServerCertificate == null && options.ServerCertificateSelector == null)
187+
{
188+
return false;
189+
}
190+
191+
listenOptions.UseHttps(options);
192+
return true;
184193
}
185194

186195
/// <summary>
@@ -192,28 +201,16 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, Action<Ht
192201
/// <returns>The <see cref="ListenOptions"/>.</returns>
193202
public static ListenOptions UseHttps(this ListenOptions listenOptions, HttpsConnectionAdapterOptions httpsOptions)
194203
{
195-
return listenOptions.UseHttps(new Lazy<HttpsConnectionAdapterOptions>(httpsOptions));
196-
}
204+
var loggerFactory = listenOptions.KestrelServerOptions?.ApplicationServices.GetRequiredService<ILoggerFactory>() ?? NullLoggerFactory.Instance;
197205

198-
/// <summary>
199-
/// Configure Kestrel to use HTTPS. This does not use default certificates or other defaults specified via config or
200-
/// <see cref="KestrelServerOptions.ConfigureHttpsDefaults(Action{HttpsConnectionAdapterOptions})"/>.
201-
/// </summary>
202-
/// <param name="listenOptions">The <see cref="ListenOptions"/> to configure.</param>
203-
/// <param name="lazyHttpsOptions">Options to configure HTTPS.</param>
204-
/// <returns>The <see cref="ListenOptions"/>.</returns>
205-
private static ListenOptions UseHttps(this ListenOptions listenOptions, Lazy<HttpsConnectionAdapterOptions> lazyHttpsOptions)
206-
{
207206
listenOptions.IsTls = true;
208-
listenOptions.HttpsOptions = lazyHttpsOptions;
207+
listenOptions.HttpsOptions = httpsOptions;
209208

210-
// NB: This lambda will only be invoked if either HTTP/1.* or HTTP/2 is being used
211209
listenOptions.Use(next =>
212210
{
213-
// Evaluate the HttpsConnectionAdapterOptions, now that the configuration, if any, has been loaded
214-
var httpsOptions = listenOptions.HttpsOptions.Value;
215-
var loggerFactory = listenOptions.KestrelServerOptions?.ApplicationServices.GetRequiredService<ILoggerFactory>() ?? NullLoggerFactory.Instance;
216-
var middleware = new HttpsConnectionMiddleware(next, httpsOptions, listenOptions.Protocols, loggerFactory);
211+
// Set the list of protocols from listen options
212+
httpsOptions.HttpProtocols = listenOptions.Protocols;
213+
var middleware = new HttpsConnectionMiddleware(next, httpsOptions, loggerFactory);
217214
return middleware.OnConnectionAsync;
218215
});
219216

@@ -273,7 +270,6 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, TlsHandsh
273270
listenOptions.IsTls = true;
274271
listenOptions.HttpsCallbackOptions = callbackOptions;
275272

276-
// NB: This lambda will only be invoked if either HTTP/1.* or HTTP/2 is being used
277273
listenOptions.Use(next =>
278274
{
279275
// Set the list of protocols from listen options.

‎src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@ internal sealed class HttpsConnectionMiddleware
3333
private readonly ILogger<HttpsConnectionMiddleware> _logger;
3434
private readonly Func<Stream, SslStream> _sslStreamFactory;
3535

36-
// Internal for testing
37-
internal readonly HttpProtocols _httpProtocols;
38-
3936
// The following fields are only set by HttpsConnectionAdapterOptions ctor.
4037
private readonly HttpsConnectionAdapterOptions? _options;
4138
private readonly SslStreamCertificateContext? _serverCertificateContext;
@@ -45,16 +42,17 @@ internal sealed class HttpsConnectionMiddleware
4542
// The following fields are only set by TlsHandshakeCallbackOptions ctor.
4643
private readonly Func<TlsHandshakeCallbackContext, ValueTask<SslServerAuthenticationOptions>>? _tlsCallbackOptions;
4744
private readonly object? _tlsCallbackOptionsState;
45+
private readonly HttpProtocols _httpProtocols;
4846

4947
// Pool for cancellation tokens that cancel the handshake
5048
private readonly CancellationTokenSourcePool _ctsPool = new();
5149

52-
public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options, HttpProtocols httpProtocols)
53-
: this(next, options, httpProtocols, loggerFactory: NullLoggerFactory.Instance)
50+
public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options)
51+
: this(next, options, loggerFactory: NullLoggerFactory.Instance)
5452
{
5553
}
5654

57-
public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options, HttpProtocols httpProtocols, ILoggerFactory loggerFactory)
55+
public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options, ILoggerFactory loggerFactory)
5856
{
5957
ArgumentNullException.ThrowIfNull(options);
6058

@@ -74,7 +72,7 @@ public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapter
7472
//_sslStreamFactory = s => new SslStream(s);
7573

7674
_options = options;
77-
_httpProtocols = ValidateAndNormalizeHttpProtocols(httpProtocols, _logger);
75+
_options.HttpProtocols = ValidateAndNormalizeHttpProtocols(_options.HttpProtocols, _logger);
7876

7977
// capture the certificate now so it can't be switched after validation
8078
_serverCertificate = options.ServerCertificate;
@@ -322,7 +320,7 @@ private Task DoOptionsBasedHandshakeAsync(ConnectionContext context, SslStream s
322320
CertificateRevocationCheckMode = _options.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck,
323321
};
324322

325-
ConfigureAlpn(sslOptions, _httpProtocols);
323+
ConfigureAlpn(sslOptions, _options.HttpProtocols);
326324

327325
_options.OnAuthenticate?.Invoke(context, sslOptions);
328326

‎src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs

Lines changed: 0 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -252,104 +252,6 @@ public void ConfigureEndpointDevelopmentCertificateGetsLoadedWhenPresent()
252252
}
253253
}
254254

255-
[Fact]
256-
public void LoadDevelopmentCertificate_ConfigureFirst()
257-
{
258-
try
259-
{
260-
var serverOptions = CreateServerOptions();
261-
var certificate = new X509Certificate2(TestResources.GetCertPath("aspnetdevcert.pfx"), "testPassword", X509KeyStorageFlags.Exportable);
262-
var bytes = certificate.Export(X509ContentType.Pkcs12, "1234");
263-
var path = GetCertificatePath();
264-
Directory.CreateDirectory(Path.GetDirectoryName(path));
265-
File.WriteAllBytes(path, bytes);
266-
267-
var config = new ConfigurationBuilder().AddInMemoryCollection(new[]
268-
{
269-
new KeyValuePair<string, string>("Certificates:Development:Password", "1234"),
270-
}).Build();
271-
272-
serverOptions.Configure(config);
273-
274-
Assert.Null(serverOptions.DefaultCertificate);
275-
276-
serverOptions.ConfigurationLoader.Load();
277-
278-
Assert.NotNull(serverOptions.DefaultCertificate);
279-
Assert.Equal(serverOptions.DefaultCertificate.SerialNumber, certificate.SerialNumber);
280-
281-
var ran1 = false;
282-
serverOptions.ListenAnyIP(4545, listenOptions =>
283-
{
284-
ran1 = true;
285-
listenOptions.UseHttps();
286-
});
287-
Assert.True(ran1);
288-
289-
var listenOptions = serverOptions.CodeBackedListenOptions.Single();
290-
Assert.False(listenOptions.HttpsOptions.IsValueCreated);
291-
listenOptions.Build();
292-
Assert.True(listenOptions.HttpsOptions.IsValueCreated);
293-
Assert.Equal(listenOptions.HttpsOptions.Value.ServerCertificate?.SerialNumber, certificate.SerialNumber);
294-
}
295-
finally
296-
{
297-
if (File.Exists(GetCertificatePath()))
298-
{
299-
File.Delete(GetCertificatePath());
300-
}
301-
}
302-
}
303-
304-
[Fact]
305-
public void LoadDevelopmentCertificate_UseHttpsFirst()
306-
{
307-
try
308-
{
309-
var serverOptions = CreateServerOptions();
310-
var certificate = new X509Certificate2(TestResources.GetCertPath("aspnetdevcert.pfx"), "testPassword", X509KeyStorageFlags.Exportable);
311-
var bytes = certificate.Export(X509ContentType.Pkcs12, "1234");
312-
var path = GetCertificatePath();
313-
Directory.CreateDirectory(Path.GetDirectoryName(path));
314-
File.WriteAllBytes(path, bytes);
315-
316-
var ran1 = false;
317-
serverOptions.ListenAnyIP(4545, listenOptions =>
318-
{
319-
ran1 = true;
320-
listenOptions.UseHttps();
321-
});
322-
Assert.True(ran1);
323-
324-
var config = new ConfigurationBuilder().AddInMemoryCollection(new[]
325-
{
326-
new KeyValuePair<string, string>("Certificates:Development:Password", "1234"),
327-
}).Build();
328-
329-
serverOptions.Configure(config);
330-
331-
Assert.Null(serverOptions.DefaultCertificate);
332-
333-
serverOptions.ConfigurationLoader.Load();
334-
335-
Assert.NotNull(serverOptions.DefaultCertificate);
336-
Assert.Equal(serverOptions.DefaultCertificate.SerialNumber, certificate.SerialNumber);
337-
338-
var listenOptions = serverOptions.CodeBackedListenOptions.Single();
339-
Assert.False(listenOptions.HttpsOptions.IsValueCreated);
340-
listenOptions.Build();
341-
Assert.True(listenOptions.HttpsOptions.IsValueCreated);
342-
Assert.Equal(listenOptions.HttpsOptions.Value.ServerCertificate?.SerialNumber, certificate.SerialNumber);
343-
}
344-
finally
345-
{
346-
if (File.Exists(GetCertificatePath()))
347-
{
348-
File.Delete(GetCertificatePath());
349-
}
350-
}
351-
}
352-
353255
[Fact]
354256
public void ConfigureEndpoint_ThrowsWhen_The_PasswordIsMissing()
355257
{
@@ -828,8 +730,6 @@ public void EndpointConfigureSection_CanSetSslProtocol()
828730
});
829731
});
830732

831-
_ = serverOptions.CodeBackedListenOptions.Single().HttpsOptions.Value; // Force evaluation
832-
833733
Assert.True(ranDefault);
834734
Assert.True(ran1);
835735
Assert.True(ran2);
@@ -965,8 +865,6 @@ public void EndpointConfigureSection_CanSetClientCertificateMode()
965865
});
966866
});
967867

968-
_ = serverOptions.CodeBackedListenOptions.Single().HttpsOptions.Value; // Force evaluation
969-
970868
Assert.True(ranDefault);
971869
Assert.True(ran1);
972870
Assert.True(ran2);

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,7 @@ void ConfigureListenOptions(ListenOptions listenOptions)
228228
public void ThrowsWhenNoServerCertificateIsProvided()
229229
{
230230
Assert.Throws<ArgumentException>(() => new HttpsConnectionMiddleware(context => Task.CompletedTask,
231-
new HttpsConnectionAdapterOptions(),
232-
ListenOptions.DefaultHttpProtocols)
231+
new HttpsConnectionAdapterOptions())
233232
);
234233
}
235234

@@ -1269,8 +1268,7 @@ public void AcceptsCertificateWithoutExtensions(string testCertName)
12691268
new HttpsConnectionMiddleware(context => Task.CompletedTask, new HttpsConnectionAdapterOptions
12701269
{
12711270
ServerCertificate = cert,
1272-
},
1273-
ListenOptions.DefaultHttpProtocols);
1271+
});
12741272
}
12751273

12761274
[Theory]
@@ -1288,8 +1286,7 @@ public void ValidatesEnhancedKeyUsageOnCertificate(string testCertName)
12881286
new HttpsConnectionMiddleware(context => Task.CompletedTask, new HttpsConnectionAdapterOptions
12891287
{
12901288
ServerCertificate = cert,
1291-
},
1292-
ListenOptions.DefaultHttpProtocols);
1289+
});
12931290
}
12941291

12951292
[Theory]
@@ -1308,7 +1305,7 @@ public void ThrowsForCertificatesMissingServerEku(string testCertName)
13081305
new HttpsConnectionMiddleware(context => Task.CompletedTask, new HttpsConnectionAdapterOptions
13091306
{
13101307
ServerCertificate = cert,
1311-
}, ListenOptions.DefaultHttpProtocols));
1308+
}));
13121309

13131310
Assert.Equal(CoreStrings.FormatInvalidServerCertificateEku(cert.Thumbprint), ex.Message);
13141311
}
@@ -1355,10 +1352,11 @@ public void Http1AndHttp2DowngradeToHttp1ForHttpsOnIncompatibleWindowsVersions()
13551352
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
13561353
{
13571354
ServerCertificate = _x509Certificate2,
1355+
HttpProtocols = HttpProtocols.Http1AndHttp2
13581356
};
1359-
var middleware = new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions, HttpProtocols.Http1AndHttp2);
1357+
new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions);
13601358

1361-
Assert.Equal(HttpProtocols.Http1, middleware._httpProtocols);
1359+
Assert.Equal(HttpProtocols.Http1, httpConnectionAdapterOptions.HttpProtocols);
13621360
}
13631361

13641362
[ConditionalFact]
@@ -1369,10 +1367,11 @@ public void Http1AndHttp2DoesNotDowngradeOnCompatibleWindowsVersions()
13691367
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
13701368
{
13711369
ServerCertificate = _x509Certificate2,
1370+
HttpProtocols = HttpProtocols.Http1AndHttp2
13721371
};
1373-
var middleware = new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions, HttpProtocols.Http1AndHttp2);
1372+
new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions);
13741373

1375-
Assert.Equal(HttpProtocols.Http1AndHttp2, middleware._httpProtocols);
1374+
Assert.Equal(HttpProtocols.Http1AndHttp2, httpConnectionAdapterOptions.HttpProtocols);
13761375
}
13771376

13781377
[ConditionalFact]
@@ -1383,9 +1382,10 @@ public void Http2ThrowsOnIncompatibleWindowsVersions()
13831382
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
13841383
{
13851384
ServerCertificate = _x509Certificate2,
1385+
HttpProtocols = HttpProtocols.Http2
13861386
};
13871387

1388-
Assert.Throws<NotSupportedException>(() => new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions, HttpProtocols.Http2));
1388+
Assert.Throws<NotSupportedException>(() => new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions));
13891389
}
13901390

13911391
[ConditionalFact]
@@ -1396,10 +1396,11 @@ public void Http2DoesNotThrowOnCompatibleWindowsVersions()
13961396
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
13971397
{
13981398
ServerCertificate = _x509Certificate2,
1399+
HttpProtocols = HttpProtocols.Http2
13991400
};
14001401

14011402
// Does not throw
1402-
new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions, HttpProtocols.Http2);
1403+
new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions);
14031404
}
14041405

14051406
private static async Task App(HttpContext httpContext)

0 commit comments

Comments
 (0)