feat(key-server): use TLS certificates with nginx#21243
Merged
Conversation
We need to manage the CA and TLS certificates with the lifecycle as the app as a whole, so make a FastAPI dependency and use it in the app lifecycle manager.
There are some things in the config that had defaults, and if multiple things depend on the defaults then who's supposed to set them? Now, the config has its own little dependency store that sets up things like base temporary directories when the server starts, so both the secure volume manager and the tls manager can rely on setting up tempdirs in a safe and sane way.
This will be useful for reloading nginx
When set, this will talk to nginx. We need this because we definitely will not have either nginx or systemd in dev and test setups.
Nginx will very aggressively cache TLS certs, so when we rotate certs we need to let it know to reload them. This is done by sending SIGHUP to the main process; this would need the pid, which is annoying, but luckily systemd has systemctl reload which just does that. Hooray!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
The point of having TLS certificates is to give them to nginx so that it can serve HTTPS traffic. This PR enables that!
We now actually use our fancy TLS EE and CA managers in the application lifecycle, so certs are generated and rotated automatically as the system comes up. This needed a little bit of config fettling; that's now a dependency and gets parsed in the lifecycle as well.
The other fun bit is that nginx needs to be explicitly told when a cert changes out from under it, because nginx aggressively caches them. We do this with systemd, but of course we don't have systemd in test or dev, so we have a new setting to just not do that.
There's a lot more to this, to do with nginx configuration, but that's over in oe-core: Opentrons/oe-core#302
Test Plan and Hands on Testing
This is the first thing that can be reasonably tested! You'll need an OE core build to do it, for reasons that become apparent in the OE core pr, but once you have that you can do the following:
/var/lib/opentrons-key-server/tls/ot-robot-tls-ca-SOMEDATE.cer. note that this is not in the secure volume; it is a public key and is public data. scp this cert to your computerhttps://robotip:32313/healthand note that the browser (a) displays a lock and (b) displays our json error text:{"message":"header.opentrons-version: Field required","errorCode":"4000"}Review requests
Not that much code change over here, though there is in OE-core; still, take a look especially about the config dependency.
Risk assessment
Low, not used yet.
Closes EXEC-2475