Skip to content

Log a warning when using an untrusted developer certificate #42621

Merged
Tratcher merged 4 commits into
dotnet:mainfrom
Tratcher:tratcher/httpswarn
Jul 23, 2022
Merged

Log a warning when using an untrusted developer certificate #42621
Tratcher merged 4 commits into
dotnet:mainfrom
Tratcher:tratcher/httpswarn

Conversation

@Tratcher
Copy link
Copy Markdown
Member

@Tratcher Tratcher commented Jul 7, 2022

Fixes #41990

If the developer certificate is present but untrusted, today Kestrel will load it without comment:

dbug: Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer[0]
      Using development certificate: CN=localhost (Thumbprint: 35775CD3DF9AD7ED5835C7AC665095CA89B001B1)

I've added a check to ensure the certificate is trusted. Now it shows:

warn: Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer[5]
      The ASP.NET Core developer certificate is not trusted.
fail: Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer[4]
      The ASP.NET Core developer certificate is in an invalid state. To fix this issue, run the following commands 'dotnet dev-certs https --clean' and 'dotnet dev-certs https' to remove all existing ASP.NET Core development certificates and create a new untrusted developer certificate. On macOS or Windows, use 'dotnet dev-certs https --trust' to trust the new certificate.
dbug: Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer[0]
      Using development certificate: CN=localhost (Thumbprint: 35775CD3DF9AD7ED5835C7AC665095CA89B001B1)

The second log is the same message used on mac clients when the cert isn't valid.

We do not attempt to trigger the interactive trust prompt from Kestrel.

@Tratcher Tratcher added this to the 7.0-preview7 milestone Jul 7, 2022
@Tratcher Tratcher self-assigned this Jul 7, 2022
@Tratcher Tratcher requested a review from Pilchie as a code owner July 7, 2022 21:53
Copy link
Copy Markdown
Member

@adityamandaleeka adityamandaleeka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM provided that you've confirmed it doesn't prompt when doing the trust check (I don't think it does on Windows given how IsTrusted is implemented, but just want us to be sure).

Comment on lines 44 to 49
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we checking this here?

CheckCertificateState was here to detect when we needed to take action to fix the certificate on Mac OS (which we might need to take again)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we can't do CheckCertificateState first on kestrel and then if that succeeds call IsTrusted(certificate)?

@JamesNK
Copy link
Copy Markdown
Member

JamesNK commented Jul 16, 2022

I like it.

For extra points, what about adding an aka.ms link to https://docs.microsoft.com/aspnet/core/security/enforcing-ssl#trust-the-aspnet-core-https-development-certificate-on-windows-and-macos to the log message. e.g.

... For more information, see https://aka.ms/aspnet/https-trust-dev-cert

@Tratcher
Copy link
Copy Markdown
Member Author

New warning (plus an aka.ms link that's in progress):

dbug: Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer[0]
      Using development certificate: CN=localhost (Thumbprint: E3920EABCAAC6D2B5535B40D406C5FFE74E8D06E)
warn: Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer[8]
      The ASP.NET Core developer certificate is not trusted.

@Tratcher Tratcher force-pushed the tratcher/httpswarn branch from 8ee98c4 to cd75fc4 Compare July 19, 2022 22:35
@Tratcher Tratcher requested a review from BrennanConroy as a code owner July 19, 2022 22:35
@Tratcher Tratcher enabled auto-merge (squash) July 20, 2022 17:22
Comment thread src/Servers/Kestrel/Core/src/Internal/LoggerExtensions.cs Outdated
Co-authored-by: James Newton-King <james@newtonking.com>
@Tratcher Tratcher merged commit 789dfdd into dotnet:main Jul 23, 2022
@Tratcher Tratcher deleted the tratcher/httpswarn branch July 23, 2022 01:15
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[EPIC] Revisiting HTTPS defaults in ASP.NET Core

7 participants