Skip to content

Commit 0c42c03

Browse files
amcaseyamcasey
authored
Log if a server certificate lacks the subjectAlternativeName extensions (#47678)
* Log if a server certificate lacks the subjectAlternativeName extensions CommonName was deprecated in favor of subjectAlternativeName so there's a good chance of getting a browser security warning if it's missing. * Address minor PR feedback
1 parent d34fefb commit 0c42c03

File tree

7 files changed

+75
-14
lines changed

7 files changed

+75
-14
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ internal static bool IsCertificateAllowedForServerAuth(X509Certificate2 certific
106106
internal static bool DoesCertificateHaveAnAccessiblePrivateKey(X509Certificate2 certificate)
107107
=> certificate.HasPrivateKey;
108108

109+
internal static bool DoesCertificateHaveASubjectAlternativeName(X509Certificate2 certificate)
110+
=> certificate.Extensions.OfType<X509SubjectAlternativeNameExtension>().Any();
111+
109112
private static void DisposeCertificates(X509Certificate2Collection? certificates, X509Certificate2? except)
110113
{
111114
if (certificates != null)

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@ internal sealed class HttpsConfigurationService : IHttpsConfigurationService
2424
private bool _isInitialized;
2525

2626
private TlsConfigurationLoader? _tlsConfigurationLoader;
27-
private Action<FeatureCollection, ListenOptions>? _populateMultiplexedTransportFeatures;
27+
private Action<FeatureCollection, ListenOptions, ILogger<HttpsConnectionMiddleware>>? _populateMultiplexedTransportFeatures;
2828
private Func<ListenOptions, ListenOptions>? _useHttpsWithDefaults;
2929

30+
private ILogger<HttpsConnectionMiddleware>? _httpsLogger;
31+
3032
/// <summary>
3133
/// Create an uninitialized <see cref="HttpsConfigurationService"/>.
3234
/// To initialize it later, call <see cref="Initialize"/>.
@@ -67,6 +69,7 @@ public void Initialize(
6769
_tlsConfigurationLoader = new TlsConfigurationLoader(hostEnvironment, serverLogger, httpsLogger);
6870
_populateMultiplexedTransportFeatures = PopulateMultiplexedTransportFeaturesWorker;
6971
_useHttpsWithDefaults = UseHttpsWithDefaultsWorker;
72+
_httpsLogger = httpsLogger;
7073
}
7174

7275
/// <inheritdoc/>
@@ -100,7 +103,7 @@ public ListenOptions UseHttpsWithSni(ListenOptions listenOptions, HttpsConnectio
100103
public void PopulateMultiplexedTransportFeatures(FeatureCollection features, ListenOptions listenOptions)
101104
{
102105
EnsureInitialized(CoreStrings.NeedHttpsConfigurationToUseHttp3);
103-
_populateMultiplexedTransportFeatures.Invoke(features, listenOptions);
106+
_populateMultiplexedTransportFeatures.Invoke(features, listenOptions, _httpsLogger);
104107
}
105108

106109
/// <inheritdoc/>
@@ -114,7 +117,7 @@ public ListenOptions UseHttpsWithDefaults(ListenOptions listenOptions)
114117
/// If this instance has not been initialized, initialize it if possible and throw otherwise.
115118
/// </summary>
116119
/// <exception cref="InvalidOperationException">If initialization is not possible.</exception>
117-
[MemberNotNull(nameof(_useHttpsWithDefaults), nameof(_tlsConfigurationLoader), nameof(_populateMultiplexedTransportFeatures))]
120+
[MemberNotNull(nameof(_useHttpsWithDefaults), nameof(_tlsConfigurationLoader), nameof(_populateMultiplexedTransportFeatures), nameof(_httpsLogger))]
118121
private void EnsureInitialized(string uninitializedError)
119122
{
120123
if (!_isInitialized)
@@ -132,18 +135,19 @@ private void EnsureInitialized(string uninitializedError)
132135
Debug.Assert(_useHttpsWithDefaults is not null);
133136
Debug.Assert(_tlsConfigurationLoader is not null);
134137
Debug.Assert(_populateMultiplexedTransportFeatures is not null);
138+
Debug.Assert(_httpsLogger is not null);
135139
}
136140

137141
/// <summary>
138142
/// The initialized implementation of <see cref="PopulateMultiplexedTransportFeatures"/>.
139143
/// </summary>
140-
internal static void PopulateMultiplexedTransportFeaturesWorker(FeatureCollection features, ListenOptions listenOptions)
144+
internal static void PopulateMultiplexedTransportFeaturesWorker(FeatureCollection features, ListenOptions listenOptions, ILogger<HttpsConnectionMiddleware> logger)
141145
{
142146
// HttpsOptions or HttpsCallbackOptions should always be set in production, but it's not set for InMemory tests.
143147
// The QUIC transport will check if TlsConnectionCallbackOptions is missing.
144148
if (listenOptions.HttpsOptions != null)
145149
{
146-
var sslServerAuthenticationOptions = HttpsConnectionMiddleware.CreateHttp3Options(listenOptions.HttpsOptions);
150+
var sslServerAuthenticationOptions = HttpsConnectionMiddleware.CreateHttp3Options(listenOptions.HttpsOptions, logger);
147151
features.Set(new TlsConnectionCallbackOptions
148152
{
149153
ApplicationProtocols = sslServerAuthenticationOptions.ApplicationProtocols ?? new List<SslApplicationProtocol> { SslApplicationProtocol.Http3 },

‎src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ internal sealed class SniOptionsSelector
2020
private const string WildcardPrefix = "*.";
2121

2222
private readonly string _endpointName;
23+
private readonly ILogger<HttpsConnectionMiddleware> _logger;
2324

2425
private readonly Func<ConnectionContext, string?, X509Certificate2?>? _fallbackServerCertificateSelector;
2526
private readonly Action<ConnectionContext, SslServerAuthenticationOptions>? _onAuthenticateCallback;
@@ -37,6 +38,7 @@ public SniOptionsSelector(
3738
ILogger<HttpsConnectionMiddleware> logger)
3839
{
3940
_endpointName = endpointName;
41+
_logger = logger;
4042

4143
_fallbackServerCertificateSelector = fallbackHttpsOptions.ServerCertificateSelector;
4244
_onAuthenticateCallback = fallbackHttpsOptions.OnAuthenticate;
@@ -75,7 +77,7 @@ public SniOptionsSelector(
7577

7678
if (!certifcateConfigLoader.IsTestMock && sslOptions.ServerCertificate is X509Certificate2 cert2)
7779
{
78-
HttpsConnectionMiddleware.EnsureCertificateIsAllowedForServerAuth(cert2);
80+
HttpsConnectionMiddleware.EnsureCertificateIsAllowedForServerAuth(cert2, logger);
7981
}
8082

8183
var clientCertificateMode = sniConfig.ClientCertificateMode ?? fallbackHttpsOptions.ClientCertificateMode;
@@ -158,7 +160,7 @@ public SniOptionsSelector(
158160

159161
if (fallbackCertificate != null)
160162
{
161-
HttpsConnectionMiddleware.EnsureCertificateIsAllowedForServerAuth(fallbackCertificate);
163+
HttpsConnectionMiddleware.EnsureCertificateIsAllowedForServerAuth(fallbackCertificate, _logger);
162164
}
163165

164166
sslOptions.ServerCertificate = fallbackCertificate;

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public KestrelServer(IOptions<KestrelServerOptions> options, IConnectionListener
3535
options,
3636
new[] { transportFactory ?? throw new ArgumentNullException(nameof(transportFactory)) },
3737
Array.Empty<IMultiplexedConnectionListenerFactory>(),
38-
new SimpleHttpsConfigurationService(),
38+
new SimpleHttpsConfigurationService(loggerFactory),
3939
loggerFactory,
4040
new KestrelMetrics(new DummyMeterFactory()));
4141
}
@@ -78,6 +78,13 @@ private sealed class DummyMeterFactory : IMeterFactory
7878

7979
private sealed class SimpleHttpsConfigurationService : IHttpsConfigurationService
8080
{
81+
private readonly ILogger<HttpsConnectionMiddleware> _httpsLogger;
82+
83+
public SimpleHttpsConfigurationService(ILoggerFactory loggerFactory)
84+
{
85+
_httpsLogger = loggerFactory.CreateLogger<HttpsConnectionMiddleware>();
86+
}
87+
8188
public bool IsInitialized => true;
8289

8390
public void Initialize(IHostEnvironment hostEnvironment, ILogger<KestrelServer> serverLogger, ILogger<HttpsConnectionMiddleware> httpsLogger)
@@ -87,7 +94,7 @@ public void Initialize(IHostEnvironment hostEnvironment, ILogger<KestrelServer>
8794

8895
public void PopulateMultiplexedTransportFeatures(FeatureCollection features, ListenOptions listenOptions)
8996
{
90-
HttpsConfigurationService.PopulateMultiplexedTransportFeaturesWorker(features, listenOptions);
97+
HttpsConfigurationService.PopulateMultiplexedTransportFeaturesWorker(features, listenOptions, _httpsLogger);
9198
}
9299

93100
public ListenOptions UseHttpsWithDefaults(ListenOptions listenOptions)

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapter
9090
{
9191
Debug.Assert(_serverCertificate != null);
9292

93-
EnsureCertificateIsAllowedForServerAuth(_serverCertificate);
93+
EnsureCertificateIsAllowedForServerAuth(_serverCertificate, _logger);
9494

9595
var certificate = _serverCertificate;
9696
if (!certificate.HasPrivateKey)
@@ -314,7 +314,7 @@ private Task DoOptionsBasedHandshakeAsync(ConnectionContext context, SslStream s
314314
var cert = _serverCertificateSelector(context, name);
315315
if (cert != null)
316316
{
317-
EnsureCertificateIsAllowedForServerAuth(cert);
317+
EnsureCertificateIsAllowedForServerAuth(cert, _logger);
318318
}
319319
return cert!;
320320
};
@@ -452,12 +452,16 @@ private static async ValueTask<SslServerAuthenticationOptions> ServerOptionsCall
452452
return sslOptions;
453453
}
454454

455-
internal static void EnsureCertificateIsAllowedForServerAuth(X509Certificate2 certificate)
455+
internal static void EnsureCertificateIsAllowedForServerAuth(X509Certificate2 certificate, ILogger<HttpsConnectionMiddleware> logger)
456456
{
457457
if (!CertificateLoader.IsCertificateAllowedForServerAuth(certificate))
458458
{
459459
throw new InvalidOperationException(CoreStrings.FormatInvalidServerCertificateEku(certificate.Thumbprint));
460460
}
461+
else if (!CertificateLoader.DoesCertificateHaveASubjectAlternativeName(certificate))
462+
{
463+
logger.NoSubjectAlternativeName(certificate.Thumbprint);
464+
}
461465
}
462466

463467
private static X509Certificate2? ConvertToX509Certificate2(X509Certificate? certificate)
@@ -514,7 +518,7 @@ private static bool IsWindowsVersionIncompatibleWithHttp2()
514518
return false;
515519
}
516520

517-
internal static SslServerAuthenticationOptions CreateHttp3Options(HttpsConnectionAdapterOptions httpsOptions)
521+
internal static SslServerAuthenticationOptions CreateHttp3Options(HttpsConnectionAdapterOptions httpsOptions, ILogger<HttpsConnectionMiddleware> logger)
518522
{
519523
if (httpsOptions.OnAuthenticate != null)
520524
{
@@ -539,7 +543,7 @@ internal static SslServerAuthenticationOptions CreateHttp3Options(HttpsConnectio
539543
var cert = httpsOptions.ServerCertificateSelector(null, host);
540544
if (cert != null)
541545
{
542-
EnsureCertificateIsAllowedForServerAuth(cert);
546+
EnsureCertificateIsAllowedForServerAuth(cert, logger);
543547
}
544548
return cert!;
545549
};
@@ -596,6 +600,9 @@ public static void FoundCertWithPrivateKey(this ILogger<HttpsConnectionMiddlewar
596600
[LoggerMessage(8, LogLevel.Debug, "Failed to open certificate store {StoreName}.", EventName = "FailToOpenStore")]
597601
public static partial void FailedToOpenStore(this ILogger<HttpsConnectionMiddleware> logger, string? storeName, Exception exception);
598602

603+
[LoggerMessage(9, LogLevel.Information, "Certificate with thumbprint {Thumbprint} lacks the subjectAlternativeName (SAN) extension and may not be accepted by browsers.", EventName = "NoSubjectAlternativeName")]
604+
public static partial void NoSubjectAlternativeName(this ILogger<HttpsConnectionMiddleware> logger, string thumbprint);
605+
599606
public static void FailedToOpenStore(this ILogger<HttpsConnectionMiddleware> logger, StoreLocation storeLocation, Exception exception)
600607
{
601608
var storeLocationString = storeLocation == StoreLocation.LocalMachine ? nameof(StoreLocation.LocalMachine) : nameof(StoreLocation.CurrentUser);

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,15 @@ public void IsCertificateAllowedForServerAuth_RejectsCertificatesMissingServerEk
5353

5454
Assert.False(CertificateLoader.IsCertificateAllowedForServerAuth(cert));
5555
}
56+
57+
[Theory]
58+
[InlineData("aspnetdevcert.pfx", true)]
59+
[InlineData("no_extensions.pfx", false)]
60+
public void DoesCertificateHaveASubjectAlternativeName(string testCertName, bool hasSan)
61+
{
62+
var certPath = TestResources.GetCertPath(testCertName);
63+
TestOutputHelper.WriteLine("Loading " + certPath);
64+
var cert = new X509Certificate2(certPath, "testPassword");
65+
Assert.Equal(hasSan, CertificateLoader.DoesCertificateHaveASubjectAlternativeName(cert));
66+
}
5667
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
using Microsoft.Extensions.DependencyInjection;
2424
using Microsoft.Extensions.Hosting;
2525
using Microsoft.Extensions.Logging;
26+
using Microsoft.Extensions.Logging.Abstractions;
2627
using Microsoft.Extensions.Metrics;
2728
using Moq;
2829

@@ -1341,6 +1342,26 @@ public void ThrowsForCertificatesMissingServerEku(string testCertName)
13411342
Assert.Equal(CoreStrings.FormatInvalidServerCertificateEku(cert.Thumbprint), ex.Message);
13421343
}
13431344

1345+
[Theory]
1346+
[InlineData("no_extensions.pfx")]
1347+
public void LogsForCertificateMissingSubjectAlternativeName(string testCertName)
1348+
{
1349+
var certPath = TestResources.GetCertPath(testCertName);
1350+
TestOutputHelper.WriteLine("Loading " + certPath);
1351+
var cert = new X509Certificate2(certPath, "testPassword");
1352+
Assert.False(CertificateLoader.DoesCertificateHaveASubjectAlternativeName(cert));
1353+
1354+
var testLogger = new TestApplicationErrorLogger();
1355+
CreateMiddleware(
1356+
new HttpsConnectionAdapterOptions
1357+
{
1358+
ServerCertificate = cert,
1359+
},
1360+
testLogger);
1361+
1362+
Assert.Single(testLogger.Messages.Where(log => log.EventId == 9));
1363+
}
1364+
13441365
[ConditionalTheory]
13451366
[InlineData(HttpProtocols.Http1)]
13461367
[InlineData(HttpProtocols.Http2)]
@@ -1442,6 +1463,12 @@ private static HttpsConnectionMiddleware CreateMiddleware(X509Certificate2 serve
14421463
});
14431464
}
14441465

1466+
private static HttpsConnectionMiddleware CreateMiddleware(HttpsConnectionAdapterOptions options, TestApplicationErrorLogger testLogger = null)
1467+
{
1468+
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()));
1470+
}
1471+
14451472
private static HttpsConnectionMiddleware CreateMiddleware(HttpsConnectionAdapterOptions options)
14461473
{
14471474
return new HttpsConnectionMiddleware(context => Task.CompletedTask, options, new KestrelMetrics(new TestMeterFactory()));

0 commit comments

Comments
 (0)