Skip to content

Commit 303115a

Browse files
amcaseyamcasey
authored
Defer search for dev cert until build bind time (#46296)
* Remove uncalled method * Defer search for dev cert until build bind time ...so that it can occur after `IConfiguration` is read. (In particular, so that it occurs during `AddressBinder.BindAsync` which happens strictly after `KestrelConfigurationLoader.Load`.) This is important in Docker scenarios since the Docker tools use `IConfiguration` to tell us where the dev cert directory is mounted. Fixes #45801 * Add regression tests * Call ListenOptions.Build in existing tests * Set IsTls eagerly for HTTP/3 scenarios * Store the lazy HttpsConnectionAdapterOptions somewhere accessible to TransportManager * Tidy up ListenOptions.Build calls in tests * Set HttpsConnectionAdapterOptions.Protocols outside middleware that may not be called * Ensure tests asserting in UseHttps trigger evaluation * Use Single rather than Assert.All * Fix incorrect Single call * Add dev cert test for HTTP/3 * Don't store HttpProtocols on HttpsConnectionAdapterOptions
1 parent f3836ae commit 303115a

File tree

9 files changed

+268
-56
lines changed

9 files changed

+268
-56
lines changed

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,6 @@ 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-
8175
/// <summary>
8276
/// Specifies whether the certificate revocation list is checked during authentication.
8377
/// </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);
80+
var sslServerAuthenticationOptions = HttpsConnectionMiddleware.CreateHttp3Options(listenOptions.HttpsOptions.Value);
8181
features.Set(new TlsConnectionCallbackOptions
8282
{
8383
ApplicationProtocols = sslServerAuthenticationOptions.ApplicationProtocols ?? new List<SslApplicationProtocol> { SslApplicationProtocol.Http3 },

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

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

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

146147
/// <summary>

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

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

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)
166+
return listenOptions.UseHttps(new Lazy<HttpsConnectionAdapterOptions>(() =>
172167
{
173-
throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound);
174-
}
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.
175171

176-
return listenOptions.UseHttps(options);
177-
}
178-
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);
172+
var options = new HttpsConnectionAdapterOptions();
173+
listenOptions.KestrelServerOptions.ApplyHttpsDefaults(options);
174+
configureOptions(options);
175+
listenOptions.KestrelServerOptions.ApplyDefaultCert(options);
185176

186-
if (options.ServerCertificate == null && options.ServerCertificateSelector == null)
187-
{
188-
return false;
189-
}
177+
if (options.ServerCertificate == null && options.ServerCertificateSelector == null)
178+
{
179+
throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound);
180+
}
190181

191-
listenOptions.UseHttps(options);
192-
return true;
182+
return options;
183+
}));
193184
}
194185

195186
/// <summary>
@@ -201,16 +192,28 @@ internal static bool TryUseHttps(this ListenOptions listenOptions)
201192
/// <returns>The <see cref="ListenOptions"/>.</returns>
202193
public static ListenOptions UseHttps(this ListenOptions listenOptions, HttpsConnectionAdapterOptions httpsOptions)
203194
{
204-
var loggerFactory = listenOptions.KestrelServerOptions?.ApplicationServices.GetRequiredService<ILoggerFactory>() ?? NullLoggerFactory.Instance;
195+
return listenOptions.UseHttps(new Lazy<HttpsConnectionAdapterOptions>(httpsOptions));
196+
}
205197

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+
{
206207
listenOptions.IsTls = true;
207-
listenOptions.HttpsOptions = httpsOptions;
208+
listenOptions.HttpsOptions = lazyHttpsOptions;
208209

210+
// NB: This lambda will only be invoked if either HTTP/1.* or HTTP/2 is being used
209211
listenOptions.Use(next =>
210212
{
211-
// Set the list of protocols from listen options
212-
httpsOptions.HttpProtocols = listenOptions.Protocols;
213-
var middleware = new HttpsConnectionMiddleware(next, httpsOptions, loggerFactory);
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);
214217
return middleware.OnConnectionAsync;
215218
});
216219

@@ -270,6 +273,7 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, TlsHandsh
270273
listenOptions.IsTls = true;
271274
listenOptions.HttpsCallbackOptions = callbackOptions;
272275

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

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ 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+
3639
// The following fields are only set by HttpsConnectionAdapterOptions ctor.
3740
private readonly HttpsConnectionAdapterOptions? _options;
3841
private readonly SslStreamCertificateContext? _serverCertificateContext;
@@ -42,17 +45,16 @@ internal sealed class HttpsConnectionMiddleware
4245
// The following fields are only set by TlsHandshakeCallbackOptions ctor.
4346
private readonly Func<TlsHandshakeCallbackContext, ValueTask<SslServerAuthenticationOptions>>? _tlsCallbackOptions;
4447
private readonly object? _tlsCallbackOptionsState;
45-
private readonly HttpProtocols _httpProtocols;
4648

4749
// Pool for cancellation tokens that cancel the handshake
4850
private readonly CancellationTokenSourcePool _ctsPool = new();
4951

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

55-
public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options, ILoggerFactory loggerFactory)
57+
public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options, HttpProtocols httpProtocols, ILoggerFactory loggerFactory)
5658
{
5759
ArgumentNullException.ThrowIfNull(options);
5860

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

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

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

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

325327
_options.OnAuthenticate?.Invoke(context, sslOptions);
326328

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

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,104 @@ 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+
255353
[Fact]
256354
public void ConfigureEndpoint_ThrowsWhen_The_PasswordIsMissing()
257355
{
@@ -730,6 +828,8 @@ public void EndpointConfigureSection_CanSetSslProtocol()
730828
});
731829
});
732830

831+
_ = serverOptions.CodeBackedListenOptions.Single().HttpsOptions.Value; // Force evaluation
832+
733833
Assert.True(ranDefault);
734834
Assert.True(ran1);
735835
Assert.True(ran2);
@@ -865,6 +965,8 @@ public void EndpointConfigureSection_CanSetClientCertificateMode()
865965
});
866966
});
867967

968+
_ = serverOptions.CodeBackedListenOptions.Single().HttpsOptions.Value; // Force evaluation
969+
868970
Assert.True(ranDefault);
869971
Assert.True(ran1);
870972
Assert.True(ran2);

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

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

@@ -1268,7 +1269,8 @@ public void AcceptsCertificateWithoutExtensions(string testCertName)
12681269
new HttpsConnectionMiddleware(context => Task.CompletedTask, new HttpsConnectionAdapterOptions
12691270
{
12701271
ServerCertificate = cert,
1271-
});
1272+
},
1273+
ListenOptions.DefaultHttpProtocols);
12721274
}
12731275

12741276
[Theory]
@@ -1286,7 +1288,8 @@ public void ValidatesEnhancedKeyUsageOnCertificate(string testCertName)
12861288
new HttpsConnectionMiddleware(context => Task.CompletedTask, new HttpsConnectionAdapterOptions
12871289
{
12881290
ServerCertificate = cert,
1289-
});
1291+
},
1292+
ListenOptions.DefaultHttpProtocols);
12901293
}
12911294

12921295
[Theory]
@@ -1305,7 +1308,7 @@ public void ThrowsForCertificatesMissingServerEku(string testCertName)
13051308
new HttpsConnectionMiddleware(context => Task.CompletedTask, new HttpsConnectionAdapterOptions
13061309
{
13071310
ServerCertificate = cert,
1308-
}));
1311+
}, ListenOptions.DefaultHttpProtocols));
13091312

13101313
Assert.Equal(CoreStrings.FormatInvalidServerCertificateEku(cert.Thumbprint), ex.Message);
13111314
}
@@ -1352,11 +1355,10 @@ public void Http1AndHttp2DowngradeToHttp1ForHttpsOnIncompatibleWindowsVersions()
13521355
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
13531356
{
13541357
ServerCertificate = _x509Certificate2,
1355-
HttpProtocols = HttpProtocols.Http1AndHttp2
13561358
};
1357-
new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions);
1359+
var middleware = new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions, HttpProtocols.Http1AndHttp2);
13581360

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

13621364
[ConditionalFact]
@@ -1367,11 +1369,10 @@ public void Http1AndHttp2DoesNotDowngradeOnCompatibleWindowsVersions()
13671369
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
13681370
{
13691371
ServerCertificate = _x509Certificate2,
1370-
HttpProtocols = HttpProtocols.Http1AndHttp2
13711372
};
1372-
new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions);
1373+
var middleware = new HttpsConnectionMiddleware(context => Task.CompletedTask, httpConnectionAdapterOptions, HttpProtocols.Http1AndHttp2);
13731374

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

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

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

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

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

14061405
private static async Task App(HttpContext httpContext)

0 commit comments

Comments
 (0)