Skip to content

Consider support for SNI certificate selection from X509Store #21300

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

Open
analogrelay opened this issue Apr 28, 2020 · 11 comments
Open

Consider support for SNI certificate selection from X509Store #21300

analogrelay opened this issue Apr 28, 2020 · 11 comments
Labels
affected-few This issue impacts only small number of customers area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-yarp This issue is related to work on yarp severity-minor This label is used by an internal tool
Milestone

Comments

@analogrelay
Copy link
Contributor

This is something that came up in YARP (dotnet/yarp#86).

We should consider an "automatic" certificate selection option in Kestrel to select certificates from X509Store based on the server name in the SNI store.

The design proposed by @davidni in the YARP issue is something like this:

  1. A background task periodically scans all reasonable certs from a given cert store. More on what is a reasonable cert below
  2. Extract all SAN entries from each reasonable cert, and build a dictionary mapping acceptable host names to certs. Wildcard entries are supported per RFC6125 section 6.4.3.
  3. Deterministic logic selects the best cert for a host name out of all available certs. More on the definition of best below.
  4. When any changes are detected, atomically swap the old dictionary with the new one
  5. We expose an interface that can be called from Kestrel's server cert selection callback to produce the appropriate cert for a given host name, or null if none match. Cert selection is always O(1) w.r.t number of bound host names, including for wildcard matches.

With "reasonable" and "best" defined as:

  • Definition of reasonable cert: Similar to Kestrel's existing logic:

    • 1.3.6.1.5.5.7.3.1 Enhanced Key Usage oid when the extension is present
    • Private key is available
    • Additional validity + revocation checks. we call X509Certificate2.Verify() to also check for revocation, whereas X509CertificateStore.Find(... validOnly: true) (used in Kestrel's defaults) does not check revocation.
  • Definition of best cert: The most recently-issued certificate that is reasonable.

@blowdart
Copy link
Contributor

I'd make this a service or interface based, so it can be swapped out for things that may be more suitable for, say, linux or a background task that supports Let's Encrypt

@webprofusion-chrisc
Copy link

webprofusion-chrisc commented Apr 29, 2020

The usual convention is to put the administrator in charge of assigning TLS certs to a service. They also need to be able to dynamically re-apply the latest cert to any service that needs it (for expired, revoked or differently-scoped certs).

Having the service auto decide for itself sounds open to problems, for instance a matching cert may also include other SAN names, for other domains/subdomain that the service is not entitled to represent or the administrator may no longer want the service to be able to associate itself with that domain.

p.s. I'm the developer of the Let's Encrypt /ACME GUI for windows: https://certifytheweb.com, which manages this for IIS, RDP, Exchange etc, so I have some bias.

[Edit]
Note also that this can probably be achieved using a handful of lines of middleware for those that really want it, so something like this sounds like a nuget extension to me.

@blowdart
Copy link
Contributor

Autoselection isn't that weird, after all that's what lets encrypt solutions end up doing, or they replace the file that holds the cert, which is arguably the same, but basing it on SAN is a little hookey.

However for a proxy I can see it, just not for a web server.

@analogrelay analogrelay added the feature-yarp This issue is related to work on yarp label Apr 29, 2020
@analogrelay
Copy link
Contributor Author

We'll build the feature in YARP if we don't think the layering is appropriate to have in ASP.NET Core itself. The presence of a background task does make me a little wary.

@analogrelay analogrelay added this to the Next sprint planning milestone Apr 29, 2020
@Tratcher
Copy link
Member

The presence of a background task does make me a little wary.

Use an IHostedService to populate and refresh an in-memory SNI table?

@analogrelay
Copy link
Contributor Author

I'm not worried implementation-wise, more about the impact of having the framework start a time-based background worker. I guess we do similar things in plenty of places (heartbeats, etc.)

@ghost
Copy link

ghost commented Jul 24, 2020

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@jkotalik
Copy link
Contributor

This was done (based on the epic above).

@Tratcher
Copy link
Member

No, this is a different scenario about scanning the cert store for applicable certificates.

@Tratcher Tratcher reopened this Nov 13, 2020
@jkotalik
Copy link
Contributor

Okay, I saw that it was checked off in the EPIC so I assumed it was done.

@Tratcher Tratcher added affected-few This issue impacts only small number of customers enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-minor This label is used by an internal tool labels Nov 13, 2020 — with ASP.NET Core Issue Ranking
@Tratcher Tratcher modified the milestones: Backlog, .NET 8 Planning Sep 28, 2022
@ghost
Copy link

ghost commented Sep 28, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023
@amcasey amcasey modified the milestones: .NET 8 Planning, Backlog Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-few This issue impacts only small number of customers area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-yarp This issue is related to work on yarp severity-minor This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

7 participants