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

tls: need per-DNS-name (per-Silo) configuration of certificates #2367

Closed
davepacheco opened this issue Feb 15, 2023 · 1 comment · Fixed by #3095
Closed

tls: need per-DNS-name (per-Silo) configuration of certificates #2367

davepacheco opened this issue Feb 15, 2023 · 1 comment · Fixed by #3095
Milestone

Comments

@davepacheco
Copy link
Collaborator

Currently Nexus has external APIs for CRUD of "system-wide" x.509 TLS certificates. Nexus uses one of these configured TLS certificates for the external API's HTTP server.

The problem: TLS certificates are generally associated with a specific DNS Name, and each Silo will have its own DNS name. See RFD 357 for more on this.

I think the right approach here will be:

  • These certificate management APIs should probably move under the Silo level and manage certificates for a particular Silo's public endpoint. There will be no system-wide certificates because there is no system-wide HTTPS endpoint.
  • We need to use TLS SNI to select the right certificate for a particular incoming TCP connection. I've confirmed that rustls supports this by providing a ResolvesServerCertUsingSni when building the rustls::Config.
  • Today, in omicron_nexus::app::certificate::Nexus::get_nexus_tls_config(), we load all the system-wide certificates, pick one, use it to make a new dropshot::ConfigTls with that certificate. I think instead we're going to want to load every Silo's certificate (picking one for each Silo). Then we're going to want a new dropshot::ConfigTls variant that lets Omicron specify the certificate resolver (or else provides the information needed for Dropshot to do that, which would be something like a map of server names to certificates). I think much of the code around this won't need to change beyond that.
@davepacheco
Copy link
Collaborator Author

See also #2368 -- it may be worth doing that at the same time as this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant