Skip to content

Commit ede4e0b

Browse files
amcaseyamcasey
authored
Load configuration in UseHttps (#48138)
So that configuration-based certs will be considered, if necessary. Largely salvaged from #48056 Builds on #48137 Fixes #45801
1 parent e9505bc commit ede4e0b

File tree

7 files changed

+286
-34
lines changed

7 files changed

+286
-34
lines changed

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

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

80-
/// <summary>
81-
/// The protocols enabled on this endpoint.
82-
/// </summary>
83-
/// <remarks>Defaults to HTTP/1.x only.</remarks>
84-
internal HttpProtocols HttpProtocols { get; set; }
85-
8680
/// <summary>
8781
/// Specifies whether the certificate revocation list is checked during authentication.
8882
/// </summary>

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,9 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, Action<Ht
166166
// We consider calls to `UseHttps` to be a clear expression of user intent to pull in HTTPS configuration support
167167
listenOptions.KestrelServerOptions.EnableHttpsConfiguration();
168168

169+
// If there's a configuration, load it so that the results will be available to ApplyDefaultCertificate
170+
listenOptions.KestrelServerOptions.ConfigurationLoader?.LoadInternal();
171+
169172
var options = new HttpsConnectionAdapterOptions();
170173
listenOptions.KestrelServerOptions.ApplyHttpsDefaults(options);
171174
configureOptions(options);
@@ -196,9 +199,7 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, HttpsConn
196199

197200
listenOptions.Use(next =>
198201
{
199-
// Set the list of protocols from listen options
200-
httpsOptions.HttpProtocols = listenOptions.Protocols;
201-
var middleware = new HttpsConnectionMiddleware(next, httpsOptions, loggerFactory, metrics);
202+
var middleware = new HttpsConnectionMiddleware(next, httpsOptions, listenOptions.Protocols, loggerFactory, metrics);
202203
return middleware.OnConnectionAsync;
203204
});
204205

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,19 @@ internal sealed class HttpsConnectionMiddleware
4343
// The following fields are only set by TlsHandshakeCallbackOptions ctor.
4444
private readonly Func<TlsHandshakeCallbackContext, ValueTask<SslServerAuthenticationOptions>>? _tlsCallbackOptions;
4545
private readonly object? _tlsCallbackOptionsState;
46-
private readonly HttpProtocols _httpProtocols;
46+
47+
// Internal for testing
48+
internal readonly HttpProtocols _httpProtocols;
4749

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

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

56-
public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options, ILoggerFactory loggerFactory, KestrelMetrics metrics)
58+
public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options, HttpProtocols httpProtocols, ILoggerFactory loggerFactory, KestrelMetrics metrics)
5759
{
5860
ArgumentNullException.ThrowIfNull(options);
5961

@@ -74,7 +76,7 @@ public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapter
7476
//_sslStreamFactory = s => new SslStream(s);
7577

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

7981
// capture the certificate now so it can't be switched after validation
8082
_serverCertificate = options.ServerCertificate;
@@ -331,7 +333,7 @@ private Task DoOptionsBasedHandshakeAsync(ConnectionContext context, SslStream s
331333
CertificateRevocationCheckMode = _options.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck,
332334
};
333335

334-
ConfigureAlpn(sslOptions, _options.HttpProtocols);
336+
ConfigureAlpn(sslOptions, _httpProtocols);
335337

336338
_options.OnAuthenticate?.Invoke(context, sslOptions);
337339

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

Lines changed: 150 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,156 @@ public void ConfigureEndpoint_RecoverFromBadPassword()
402402
void CheckListenOptions(X509Certificate2 expectedCert)
403403
{
404404
var listenOptions = Assert.Single(serverOptions.ConfigurationBackedListenOptions);
405-
Assert.Equal(expectedCert.SerialNumber, listenOptions.HttpsOptions.ServerCertificate.SerialNumber);
405+
Assert.Equal(expectedCert.SerialNumber, listenOptions.HttpsOptions!.ServerCertificate.SerialNumber);
406+
}
407+
}
408+
409+
[Fact]
410+
public void LoadDevelopmentCertificate_LoadBeforeUseHttps()
411+
{
412+
try
413+
{
414+
var serverOptions = CreateServerOptions();
415+
var certificate = new X509Certificate2(TestResources.GetCertPath("aspnetdevcert.pfx"), "testPassword", X509KeyStorageFlags.Exportable);
416+
var bytes = certificate.Export(X509ContentType.Pkcs12, "1234");
417+
var path = GetCertificatePath();
418+
Directory.CreateDirectory(Path.GetDirectoryName(path));
419+
File.WriteAllBytes(path, bytes);
420+
421+
var config = new ConfigurationBuilder().AddInMemoryCollection(new[]
422+
{
423+
new KeyValuePair<string, string>("Certificates:Development:Password", "1234"),
424+
}).Build();
425+
426+
serverOptions.Configure(config);
427+
428+
Assert.Null(serverOptions.ConfigurationLoader.DefaultCertificate);
429+
430+
serverOptions.ConfigurationLoader.Load();
431+
432+
Assert.NotNull(serverOptions.ConfigurationLoader.DefaultCertificate);
433+
Assert.Equal(serverOptions.ConfigurationLoader.DefaultCertificate.SerialNumber, certificate.SerialNumber);
434+
435+
var ran1 = false;
436+
serverOptions.ListenAnyIP(4545, listenOptions =>
437+
{
438+
ran1 = true;
439+
listenOptions.UseHttps();
440+
});
441+
Assert.True(ran1);
442+
443+
var listenOptions = serverOptions.CodeBackedListenOptions.Single();
444+
listenOptions.Build();
445+
Assert.Equal(listenOptions.HttpsOptions.ServerCertificate?.SerialNumber, certificate.SerialNumber);
446+
}
447+
finally
448+
{
449+
if (File.Exists(GetCertificatePath()))
450+
{
451+
File.Delete(GetCertificatePath());
452+
}
453+
}
454+
}
455+
456+
[Fact]
457+
public void LoadDevelopmentCertificate_UseHttpsBeforeLoad()
458+
{
459+
try
460+
{
461+
var serverOptions = CreateServerOptions();
462+
var certificate = new X509Certificate2(TestResources.GetCertPath("aspnetdevcert.pfx"), "testPassword", X509KeyStorageFlags.Exportable);
463+
var bytes = certificate.Export(X509ContentType.Pkcs12, "1234");
464+
var path = GetCertificatePath();
465+
Directory.CreateDirectory(Path.GetDirectoryName(path));
466+
File.WriteAllBytes(path, bytes);
467+
468+
var config = new ConfigurationBuilder().AddInMemoryCollection(new[]
469+
{
470+
new KeyValuePair<string, string>("Certificates:Development:Password", "1234"),
471+
}).Build();
472+
473+
serverOptions.Configure(config);
474+
475+
Assert.Null(serverOptions.ConfigurationLoader.DefaultCertificate);
476+
477+
var ran1 = false;
478+
serverOptions.ListenAnyIP(4545, listenOptions =>
479+
{
480+
ran1 = true;
481+
listenOptions.UseHttps();
482+
});
483+
Assert.True(ran1);
484+
485+
// Use Https triggers a load, so the default cert is already set
486+
Assert.NotNull(serverOptions.ConfigurationLoader.DefaultCertificate);
487+
Assert.Equal(serverOptions.ConfigurationLoader.DefaultCertificate.SerialNumber, certificate.SerialNumber);
488+
489+
// This Load is a no-op (tested elsewhere)
490+
serverOptions.ConfigurationLoader.Load();
491+
492+
var listenOptions = serverOptions.CodeBackedListenOptions.Single();
493+
listenOptions.Build();
494+
Assert.Equal(listenOptions.HttpsOptions.ServerCertificate?.SerialNumber, certificate.SerialNumber);
495+
}
496+
finally
497+
{
498+
if (File.Exists(GetCertificatePath()))
499+
{
500+
File.Delete(GetCertificatePath());
501+
}
502+
}
503+
}
504+
505+
[Fact]
506+
public void LoadDevelopmentCertificate_UseHttpsBeforeConfigure()
507+
{
508+
try
509+
{
510+
var serverOptions = CreateServerOptions();
511+
var certificate = new X509Certificate2(TestResources.GetCertPath("aspnetdevcert.pfx"), "testPassword", X509KeyStorageFlags.Exportable);
512+
var bytes = certificate.Export(X509ContentType.Pkcs12, "1234");
513+
var path = GetCertificatePath();
514+
Directory.CreateDirectory(Path.GetDirectoryName(path));
515+
File.WriteAllBytes(path, bytes);
516+
517+
var defaultCertificate = TestResources.GetTestCertificate();
518+
Assert.NotEqual(certificate.SerialNumber, defaultCertificate.SerialNumber);
519+
serverOptions.TestOverrideDefaultCertificate = defaultCertificate;
520+
521+
var ran1 = false;
522+
serverOptions.ListenAnyIP(4545, listenOptions =>
523+
{
524+
ran1 = true;
525+
listenOptions.UseHttps();
526+
});
527+
Assert.True(ran1);
528+
529+
var config = new ConfigurationBuilder().AddInMemoryCollection(new[]
530+
{
531+
new KeyValuePair<string, string>("Certificates:Development:Password", "1234"),
532+
}).Build();
533+
534+
serverOptions.Configure(config);
535+
536+
Assert.Null(serverOptions.ConfigurationLoader.DefaultCertificate);
537+
538+
serverOptions.ConfigurationLoader.Load();
539+
540+
Assert.NotNull(serverOptions.ConfigurationLoader.DefaultCertificate);
541+
Assert.Equal(serverOptions.ConfigurationLoader.DefaultCertificate.SerialNumber, certificate.SerialNumber);
542+
543+
var listenOptions = serverOptions.CodeBackedListenOptions.Single();
544+
listenOptions.Build();
545+
// In a perfect world, it would match certificate.SerialNumber, but there's no way for an eager UseHttps
546+
// to do that before Configure is called.
547+
Assert.Equal(listenOptions.HttpsOptions.ServerCertificate?.SerialNumber, defaultCertificate.SerialNumber);
548+
}
549+
finally
550+
{
551+
if (File.Exists(GetCertificatePath()))
552+
{
553+
File.Delete(GetCertificatePath());
554+
}
406555
}
407556
}
408557

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ void ConfigureListenOptions(ListenOptions listenOptions)
264264
[Fact]
265265
public void ThrowsWhenNoServerCertificateIsProvided()
266266
{
267-
Assert.Throws<ArgumentException>(() => CreateMiddleware(new HttpsConnectionAdapterOptions()));
267+
Assert.Throws<ArgumentException>(() => CreateMiddleware(new HttpsConnectionAdapterOptions(), ListenOptions.DefaultHttpProtocols));
268268
}
269269

270270
[Fact]
@@ -1318,7 +1318,8 @@ public void ValidatesEnhancedKeyUsageOnCertificate(string testCertName)
13181318
CreateMiddleware(new HttpsConnectionAdapterOptions
13191319
{
13201320
ServerCertificate = cert,
1321-
});
1321+
},
1322+
ListenOptions.DefaultHttpProtocols);
13221323
}
13231324

13241325
[Theory]
@@ -1337,7 +1338,8 @@ public void ThrowsForCertificatesMissingServerEku(string testCertName)
13371338
CreateMiddleware(new HttpsConnectionAdapterOptions
13381339
{
13391340
ServerCertificate = cert,
1340-
}));
1341+
},
1342+
ListenOptions.DefaultHttpProtocols));
13411343

13421344
Assert.Equal(CoreStrings.FormatInvalidServerCertificateEku(cert.Thumbprint), ex.Message);
13431345
}
@@ -1357,6 +1359,7 @@ public void LogsForCertificateMissingSubjectAlternativeName(string testCertName)
13571359
{
13581360
ServerCertificate = cert,
13591361
},
1362+
ListenOptions.DefaultHttpProtocols,
13601363
testLogger);
13611364

13621365
Assert.Single(testLogger.Messages.Where(log => log.EventId == 9));
@@ -1404,11 +1407,10 @@ public void Http1AndHttp2DowngradeToHttp1ForHttpsOnIncompatibleWindowsVersions()
14041407
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
14051408
{
14061409
ServerCertificate = _x509Certificate2,
1407-
HttpProtocols = HttpProtocols.Http1AndHttp2
14081410
};
1409-
CreateMiddleware(httpConnectionAdapterOptions);
1411+
var middleware = CreateMiddleware(httpConnectionAdapterOptions, HttpProtocols.Http1AndHttp2);
14101412

1411-
Assert.Equal(HttpProtocols.Http1, httpConnectionAdapterOptions.HttpProtocols);
1413+
Assert.Equal(HttpProtocols.Http1, middleware._httpProtocols);
14121414
}
14131415

14141416
[ConditionalFact]
@@ -1419,11 +1421,10 @@ public void Http1AndHttp2DoesNotDowngradeOnCompatibleWindowsVersions()
14191421
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
14201422
{
14211423
ServerCertificate = _x509Certificate2,
1422-
HttpProtocols = HttpProtocols.Http1AndHttp2
14231424
};
1424-
CreateMiddleware(httpConnectionAdapterOptions);
1425+
var middleware = CreateMiddleware(httpConnectionAdapterOptions, HttpProtocols.Http1AndHttp2);
14251426

1426-
Assert.Equal(HttpProtocols.Http1AndHttp2, httpConnectionAdapterOptions.HttpProtocols);
1427+
Assert.Equal(HttpProtocols.Http1AndHttp2, middleware._httpProtocols);
14271428
}
14281429

14291430
[ConditionalFact]
@@ -1434,10 +1435,9 @@ public void Http2ThrowsOnIncompatibleWindowsVersions()
14341435
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
14351436
{
14361437
ServerCertificate = _x509Certificate2,
1437-
HttpProtocols = HttpProtocols.Http2
14381438
};
14391439

1440-
Assert.Throws<NotSupportedException>(() => CreateMiddleware(httpConnectionAdapterOptions));
1440+
Assert.Throws<NotSupportedException>(() => CreateMiddleware(httpConnectionAdapterOptions, HttpProtocols.Http2));
14411441
}
14421442

14431443
[ConditionalFact]
@@ -1448,30 +1448,30 @@ public void Http2DoesNotThrowOnCompatibleWindowsVersions()
14481448
var httpConnectionAdapterOptions = new HttpsConnectionAdapterOptions
14491449
{
14501450
ServerCertificate = _x509Certificate2,
1451-
HttpProtocols = HttpProtocols.Http2
14521451
};
14531452

14541453
// Does not throw
1455-
CreateMiddleware(httpConnectionAdapterOptions);
1454+
CreateMiddleware(httpConnectionAdapterOptions, HttpProtocols.Http2);
14561455
}
14571456

14581457
private static HttpsConnectionMiddleware CreateMiddleware(X509Certificate2 serverCertificate)
14591458
{
14601459
return CreateMiddleware(new HttpsConnectionAdapterOptions
14611460
{
14621461
ServerCertificate = serverCertificate,
1463-
});
1462+
},
1463+
ListenOptions.DefaultHttpProtocols);
14641464
}
14651465

1466-
private static HttpsConnectionMiddleware CreateMiddleware(HttpsConnectionAdapterOptions options, TestApplicationErrorLogger testLogger = null)
1466+
private static HttpsConnectionMiddleware CreateMiddleware(HttpsConnectionAdapterOptions options, HttpProtocols httpProtocols, TestApplicationErrorLogger testLogger = null)
14671467
{
14681468
var loggerFactory = testLogger is null ? (ILoggerFactory)NullLoggerFactory.Instance : new LoggerFactory(new[] { new KestrelTestLoggerProvider(testLogger) });
1469-
return new HttpsConnectionMiddleware(context => Task.CompletedTask, options, loggerFactory, new KestrelMetrics(new TestMeterFactory()));
1469+
return new HttpsConnectionMiddleware(context => Task.CompletedTask, options, httpProtocols, loggerFactory, new KestrelMetrics(new TestMeterFactory()));
14701470
}
14711471

1472-
private static HttpsConnectionMiddleware CreateMiddleware(HttpsConnectionAdapterOptions options)
1472+
private static HttpsConnectionMiddleware CreateMiddleware(HttpsConnectionAdapterOptions options, HttpProtocols httpProtocols)
14731473
{
1474-
return new HttpsConnectionMiddleware(context => Task.CompletedTask, options, new KestrelMetrics(new TestMeterFactory()));
1474+
return new HttpsConnectionMiddleware(context => Task.CompletedTask, options, httpProtocols, new KestrelMetrics(new TestMeterFactory()));
14751475
}
14761476

14771477
private static async Task App(HttpContext httpContext)

0 commit comments

Comments
 (0)