Skip to content
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

CertificateManager.ListCertificates impacting ASP.NET startup #29650

Closed
stephentoub opened this issue Jan 26, 2021 · 6 comments
Closed

CertificateManager.ListCertificates impacting ASP.NET startup #29650

stephentoub opened this issue Jan 26, 2021 · 6 comments
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Milestone

Comments

@stephentoub
Copy link
Member

stephentoub commented Jan 26, 2021

In an app created from the Blazor server template, CertificateManager.ListCertificates is consuming ~5% of startup time. Within ListCertificates, a significant portion of normal processing time is spent opening the certificate stores and enumerating/validating every certificate. However, in the typical developer use case, an ASP.NET certificate will be found in the user store (since that's where it's generally added), but we still open and enumerate the local machine store, anyway. It could be beneficial to first check the user store and only look at the local machine store if a valid cert can't be found in the user store.

cc: @halter73

@blowdart
Copy link
Contributor

However, in the typical use case, an ASP.NET certificate will be found in the user store (since that's where it's generally added)

Are we sure about that?

For IIS the typical approach was to put it into local machine so all app pools could see it, and ACL appropriately there.
Service Fabric also uses the machine store, including in their kestrel instructions.

The dotnet cert tooling installs the self signed cert into the user store, because it's a testing cert. I'm not sure you can say that the typical, real life use case does the same.

@stephentoub
Copy link
Member Author

stephentoub commented Jan 27, 2021

The dotnet cert tooling installs the self signed cert into the user store, because it's a testing cert. I'm not sure you can say that the typical, real life use case does the same.

Test cert is "real life use case" when focused on developer inner loop performance.

And, regardless, it doesn't really matter. Today both costs are paid up-front: we fetch all certs from both stores, and then process the user certs, and then process the local machine certs. We may as well just fetch one, and only fetch the other if necessary.

var currentUserCertificates = ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: true, requireExportable: true);
var trustedCertificates = ListCertificates(StoreName.My, StoreLocation.LocalMachine, isValid: true, requireExportable: true);
var certificates = currentUserCertificates.Concat(trustedCertificates);

@ShreyasJejurkar
Copy link
Contributor

If a certificate gets found in the current user store location, should we skip the call to find certificates from the local machine store? Or vice versa?

@stephentoub

@stephentoub
Copy link
Member Author

If a certificate gets found in the current user store location, should we skip the call to find certificates from the local machine store

That is what I was suggesting.

@BrennanConroy BrennanConroy added this to the Next sprint planning milestone Jan 27, 2021
@ghost
Copy link

ghost commented Jan 27, 2021

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@stephentoub
Copy link
Member Author

stephentoub commented Feb 1, 2021

After the changes made to CertificateManager recently (e.g. #27956), ListCertificates has dropped to only ~2% of startup. On top of that, the path Kestrel takes already only checks the user store:

private void EnsureDefaultCert()

It's not clear why
public EnsureCertificateResult EnsureAspNetCoreHttpsDevelopmentCertificate(
and EnsureDefaultCert don't need to do the same thing, but it appears that's used only by dotnet-dev-certs rather than by Kestrel, and so isn't an issue for ASP.NET startup.

So, we can close this, unless someone would like to keep it open for tracking improvements to dotnet-dev-certs.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 23, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

No branches or pull requests

6 participants