Skip to content

Commit 88cc026

Browse files
committed
Fixes and improvements for dotnet-dev-certs:
* Revamps the HTTPS developer certificate tool implementation. * It accumulated a lot of cruft during the past few years and that has made making changes harder. * Separated the CertificateManager implementation into different classes per platform. * This centralizes the decision point of choosing a platform in a single place. * Makes clear what the flow is for a given platform. * Isolates changes needed for a given platform in the future. * Moved CertificateManager to a singleton * No more statics! * Updates logging to use EventSource * We didn't have a good way of performing logging as the code is shared and must run in multiple contexts and the set of dependencies need to be kept to a minimum. * Adding ETW allow us to log/monitor the the tool execution and capture the logs with `dotnet trace` without having to invent our own logging. * We can decide to write an EventListener in `dotnet-dev-certs` to write the results to the console output. * Updates the way we handle the dev-cert in Mac OS to use the security tool to import the certificate into the store instead of using the certificate store.
1 parent da136fe commit 88cc026

File tree

13 files changed

+1139
-868
lines changed

13 files changed

+1139
-868
lines changed

src/ProjectTemplates/test/Helpers/AspNetProcess.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public AspNetProcess(
8989
internal void EnsureDevelopmentCertificates()
9090
{
9191
var now = DateTimeOffset.Now;
92-
var manager = new CertificateManager();
92+
var manager = CertificateManager.Instance;
9393
var certificate = manager.CreateAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), "CN=localhost");
9494
manager.ExportCertificate(certificate, path: _certificatePath, includePrivateKey: true, _certificatePassword);
9595
}

src/Servers/Kestrel/Core/src/Internal/LoggerExtensions.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,28 @@ internal static class LoggerExtensions
3434
new EventId(3, "FailedToLoadDevelopmentCertificate"),
3535
"Failed to load the development https certificate at '{certificatePath}'.");
3636

37+
private static readonly Action<ILogger, Exception> _badDeveloperCertificateState =
38+
LoggerMessage.Define(
39+
LogLevel.Error,
40+
new EventId(4, "BadDeveloperCertificateState"),
41+
CoreStrings.BadDeveloperCertificateState);
42+
43+
private static readonly Action<ILogger, string, Exception> _developerCertificateFirstRun =
44+
LoggerMessage.Define<string>(
45+
LogLevel.Warning,
46+
new EventId(5, "DeveloperCertificateFirstRun"),
47+
"{Message}");
48+
3749
public static void LocatedDevelopmentCertificate(this ILogger logger, X509Certificate2 certificate) => _locatedDevelopmentCertificate(logger, certificate.Subject, certificate.Thumbprint, null);
3850

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

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

4355
public static void FailedToLoadDevelopmentCertificate(this ILogger logger, string certificatePath) => _failedToLoadDevelopmentCertificate(logger, certificatePath, null);
56+
57+
public static void BadDeveloperCertificateState(this ILogger logger) => _badDeveloperCertificateState(logger, null);
58+
59+
public static void DeveloperCertificateFirstRun(this ILogger logger, string message) => _developerCertificateFirstRun(logger, message, null);
4460
}
4561
}

src/Servers/Kestrel/Core/src/KestrelServerOptions.cs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,29 @@ private void EnsureDefaultCert()
152152
var logger = ApplicationServices.GetRequiredService<ILogger<KestrelServer>>();
153153
try
154154
{
155-
DefaultCertificate = CertificateManager.ListCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true)
155+
DefaultCertificate = CertificateManager.Instance.ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: true)
156156
.FirstOrDefault();
157157

158158
if (DefaultCertificate != null)
159159
{
160+
var status = CertificateManager.Instance.CheckCertificateState(DefaultCertificate, interactive: false);
161+
if (!status.Result)
162+
{
163+
// Display a warning indicating to the user that a prompt might appear and provide instructions on what to do in that
164+
// case. The underlying implementation of this check is specific to Mac OS and is handled within CheckCertificateState.
165+
// Kestrel must NEVER cause a UI prompt on a production system. We only attempt this here because Mac OS is not supported
166+
// in production.
167+
logger.DeveloperCertificateFirstRun(status.Message);
168+
169+
// Now that we've displayed a warning in the logs so that the user gets a notification that a prompt might appear, try
170+
// and access the certificate key, which might trigger a prompt.
171+
status = CertificateManager.Instance.CheckCertificateState(DefaultCertificate, interactive: true);
172+
if (!status.Result)
173+
{
174+
logger.BadDeveloperCertificateState();
175+
}
176+
}
177+
160178
logger.LocatedDevelopmentCertificate(DefaultCertificate);
161179
}
162180
else

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,16 +220,10 @@ public async Task OnConnectionAsync(ConnectionContext context)
220220
}
221221
catch (AuthenticationException ex)
222222
{
223-
if (_serverCertificate == null ||
224-
!CertificateManager.IsHttpsDevelopmentCertificate(_serverCertificate) ||
225-
CertificateManager.CheckDeveloperCertificateKey(_serverCertificate))
223+
if (_serverCertificate == null)
226224
{
227225
_logger.LogDebug(1, ex, CoreStrings.AuthenticationFailed);
228226
}
229-
else
230-
{
231-
_logger.LogError(3, ex, CoreStrings.BadDeveloperCertificateState);
232-
}
233227

234228
await sslStream.DisposeAsync();
235229
return;

src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,9 @@ await Assert.ThrowsAnyAsync<Exception>(() =>
386386
}
387387

388388
[Fact]
389+
[SkipOnHelix("", Queues = "Ubuntu.1604.Amd64.Open;Windows.10.Amd64.Open")]
390+
[OSSkipCondition(OperatingSystems.Linux, SkipReason = "This check only applies to Mac OS as it is the one that checks the key for validity.")]
391+
[OSSkipCondition(OperatingSystems.Windows, SkipReason = "This check only applies to Mac OS as it is the one that checks the key for validity.")]
389392
public async Task DevCertWithInvalidPrivateKeyProducesCustomWarning()
390393
{
391394
var loggerProvider = new HandshakeErrorLoggerProvider();

0 commit comments

Comments
 (0)