Skip to content

[HTTPS] Update certificate strategy for Mac OS #20022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/ProjectTemplates/test/Helpers/AspNetProcess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ public AspNetProcess(
internal void EnsureDevelopmentCertificates()
{
var now = DateTimeOffset.Now;
var manager = new CertificateManager();
var certificate = manager.CreateAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), "CN=localhost");
var manager = CertificateManager.Instance;
var certificate = manager.CreateAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1));
manager.ExportCertificate(certificate, path: _certificatePath, includePrivateKey: true, _certificatePassword);
}

Expand Down
16 changes: 16 additions & 0 deletions src/Servers/Kestrel/Core/src/Internal/LoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,28 @@ internal static class LoggerExtensions
new EventId(3, "FailedToLoadDevelopmentCertificate"),
"Failed to load the development https certificate at '{certificatePath}'.");

private static readonly Action<ILogger, Exception> _badDeveloperCertificateState =
LoggerMessage.Define(
LogLevel.Error,
new EventId(4, "BadDeveloperCertificateState"),
CoreStrings.BadDeveloperCertificateState);

private static readonly Action<ILogger, string, Exception> _developerCertificateFirstRun =
LoggerMessage.Define<string>(
LogLevel.Warning,
new EventId(5, "DeveloperCertificateFirstRun"),
"{Message}");

public static void LocatedDevelopmentCertificate(this ILogger logger, X509Certificate2 certificate) => _locatedDevelopmentCertificate(logger, certificate.Subject, certificate.Thumbprint, null);

public static void UnableToLocateDevelopmentCertificate(this ILogger logger) => _unableToLocateDevelopmentCertificate(logger, null);

public static void FailedToLocateDevelopmentCertificateFile(this ILogger logger, string certificatePath) => _failedToLocateDevelopmentCertificateFile(logger, certificatePath, null);

public static void FailedToLoadDevelopmentCertificate(this ILogger logger, string certificatePath) => _failedToLoadDevelopmentCertificate(logger, certificatePath, null);

public static void BadDeveloperCertificateState(this ILogger logger) => _badDeveloperCertificateState(logger, null);

public static void DeveloperCertificateFirstRun(this ILogger logger, string message) => _developerCertificateFirstRun(logger, message, null);
}
}
20 changes: 19 additions & 1 deletion src/Servers/Kestrel/Core/src/KestrelServerOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,29 @@ private void EnsureDefaultCert()
var logger = ApplicationServices.GetRequiredService<ILogger<KestrelServer>>();
try
{
DefaultCertificate = CertificateManager.ListCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true)
DefaultCertificate = CertificateManager.Instance.ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: true)
.FirstOrDefault();

if (DefaultCertificate != null)
{
var status = CertificateManager.Instance.CheckCertificateState(DefaultCertificate, interactive: false);
if (!status.Result)
{
// 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.
logger.DeveloperCertificateFirstRun(status.Message);

// Now that we've displayed a warning in the logs so that the user gets a notification that a prompt might appear, try
// and access the certificate key, which might trigger a prompt.
status = CertificateManager.Instance.CheckCertificateState(DefaultCertificate, interactive: true);
if (!status.Result)
{
logger.BadDeveloperCertificateState();
}
}

logger.LocatedDevelopmentCertificate(DefaultCertificate);
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,16 +220,7 @@ public async Task OnConnectionAsync(ConnectionContext context)
}
catch (AuthenticationException ex)
{
if (_serverCertificate == null ||
!CertificateManager.IsHttpsDevelopmentCertificate(_serverCertificate) ||
CertificateManager.CheckDeveloperCertificateKey(_serverCertificate))
{
_logger.LogDebug(1, ex, CoreStrings.AuthenticationFailed);
}
else
{
_logger.LogError(3, ex, CoreStrings.BadDeveloperCertificateState);
}
_logger.LogDebug(1, ex, CoreStrings.AuthenticationFailed);

await sslStream.DisposeAsync();
return;
Expand Down
29 changes: 0 additions & 29 deletions src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -385,35 +385,6 @@ await Assert.ThrowsAnyAsync<Exception>(() =>
Assert.Equal(LogLevel.Debug, loggerProvider.FilterLogger.LastLogLevel);
}

[Fact]
public async Task DevCertWithInvalidPrivateKeyProducesCustomWarning()
{
var loggerProvider = new HandshakeErrorLoggerProvider();
LoggerFactory.AddProvider(loggerProvider);

await using (var server = new TestServer(context => Task.CompletedTask,
new TestServiceContext(LoggerFactory),
listenOptions =>
{
listenOptions.UseHttps(TestResources.GetTestCertificate("aspnetdevcert.pfx", "testPassword"));
}))
{
using (var connection = server.CreateConnection())
using (var sslStream = new SslStream(connection.Stream, true, (sender, certificate, chain, errors) => true))
{
// SslProtocols.Tls is TLS 1.0 which isn't supported by Kestrel by default.
await Assert.ThrowsAnyAsync<Exception>(() =>
sslStream.AuthenticateAsClientAsync("127.0.0.1", clientCertificates: null,
enabledSslProtocols: SslProtocols.Tls,
checkCertificateRevocation: false));
}
}

await loggerProvider.FilterLogger.LogTcs.Task.DefaultTimeout();
Assert.Equal(3, loggerProvider.FilterLogger.LastEventId);
Assert.Equal(LogLevel.Error, loggerProvider.FilterLogger.LastLogLevel);
}

[Fact]
public async Task OnAuthenticate_SeesOtherSettings()
{
Expand Down
Loading