dotnet / dotnet/aspnetcore

[certs] Full chain support followup

Open
#43,193 34 comments 0 reactions 0 assignees View on GitHub
area-networking bug feature-kestrel
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 6h
Merged PRs (30d)
290

Description

See https://github.com/dotnet/aspnetcore/pull/41944#discussion_r940879108

Some followup questions/work:

from @bartonjs
> In this if block, you could just make certificate be fullChain[0] (assuming it's non-empty), and then you could remove certificate from the collection; depending on what public API guarantees you're making.

> The two halves of that sentence are:

> We've already done the work of loading the certificate instance, why go open the file and process the contents a second time?
The perf of a chain build will be marginally faster if you remove unnecessary elements. Since the target certificate is already specified as the target it won't need to be found from the collection. By removing it early you save a "nope, not the one I'm looking for, next!"
If you don't remove it, because you want the "full" chain to be in the HttpsOptions.ServerCertificateChain collection, then you have to decide if you want to have the same instance in the property and the collection. If "yes" to the full chain and "no" to the same instance... then leave the code as-is 😄.

> And the reason I put "full" in quotes is that I don't think Let's Encrypt puts their root cert in that file, so the "full" chain is really "the chain except the root". But it's as "full" as that file is, I suppose.

From @davidfowl

>I did a bunch of research here a year ago and I can dig up my notes. When you get the certs from certbot for lets encrypt you can get both the fullchain.pem or the chain.pem + cert.pem (https://community.letsencrypt.org/t/generating-cert-pem-chain-pem-and-fullchain-pem-from-order-certificate/78376/6).

> We should also support loading the full chai from PFX files if it is there. NGINX supports the full chain as well (https://serverfault.com/questions/987612/nginx-ssl-config-for-cert-pem-and-chain-pem).

>I'd like us to support both providing just the intermediates without the leaf cert and the full chain and we can remove the leaf cert (assuming this is easy). That makes us a bit more friendly.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.