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

caddy trust command to manually trigger the root CA trust setup #3204

Closed
lukasbestle opened this issue Mar 31, 2020 · 13 comments
Closed

caddy trust command to manually trigger the root CA trust setup #3204

lukasbestle opened this issue Mar 31, 2020 · 13 comments
Labels
feature ⚙️ New feature or request
Milestone

Comments

@lukasbestle
Copy link

I'm currently working on an Ansible role to automate the installation and setup of a Caddy-based local development setup. I need a way to manually trigger the root CA setup to ensure that Caddy can afterwards be run as a system service without having to enter a password anywhere.

I propose a new command caddy trust that could be run with sudo to do the following without user interaction:

  1. Check if a local root CA was already generated, otherwise generate one now.
  2. Check if the local root CA is already trusted. If yes, exit without any output or with a simple one-line message so that tools like Ansible can easily detect this unchanged state.
  3. Add the local root CA to all trust stores where it isn't yet trusted.
@lukasbestle
Copy link
Author

Additional feature request: The caddy trust command should ideally work with or without sudo:

  • If the user running caddy trust is root, Caddy should check if SUDO_USER is set. If yes, Caddy should generate the root CA for that user and not for root – otherwise the trusted root CA will be useless if Caddy is subsequently run by a non-root user, which would have its own root CA.
  • If caddy trust was run without sudo (i.e. by a non-root user), it should ask for the password interactively if needed like Caddy currently does in the caddy run command.

@mholt mholt closed this as completed Mar 31, 2020
@mholt mholt added upstream ⬆️ Relates to some dependency of this project feature ⚙️ New feature or request and removed upstream ⬆️ Relates to some dependency of this project labels Mar 31, 2020
@mholt mholt reopened this Mar 31, 2020
@mholt
Copy link
Member

mholt commented Mar 31, 2020

Okay, after misunderstanding this first I think I get it now. We can probably do this... however, the details of sudo and stuff still need to be implemented by the upstream library: https://github.com/FiloSottile/mkcert

@mholt mholt added this to the 2.x milestone Mar 31, 2020
@mholt mholt closed this as completed in 244b839 Mar 31, 2020
@mholt
Copy link
Member

mholt commented Mar 31, 2020

@lukasbestle Alrighty, I have implemented this in 244b839 - please try it out!

@mholt mholt modified the milestones: 2.x, v2.0.0-rc.1 Mar 31, 2020
@lukasbestle
Copy link
Author

@mholt Thanks for the fast implementation! It generally works well, there's just one minor issue: If I run it with sudo caddy trust and there isn't already a set of keys, the pki/authorities/local directory and all files within it will be created and owned by root, not the current user. It works great without sudo, but then there will be an interactive password prompt.

So either the PKI generation should happen with user permissions or chown should be run after the files have been generated. Otherwise Caddy will afterwards not be able to access the files when running the server with user permissions.

@francislavoie
Copy link
Member

That's a tricky problem. When running as root with sudo, it's not completely reliable, but you can use the SUDO_UID and SUDO_GID env vars, but those can be overridden before the program is run.

See https://stackoverflow.com/a/29733790/846934

@lukasbestle
Copy link
Author

@francislavoie Yep, that's what I would suggest as well.

The env vars cannot be overridden when Caddy is really called with sudo caddy trust as there is no command running between sudo and caddy. So the only thing Caddy needs to be careful about is to verify that it is being run as root (UID 0).

@mholt
Copy link
Member

mholt commented Apr 1, 2020

If I run it with sudo caddy trust and there isn't already a set of keys, the pki/authorities/local directory and all files within it will be created and owned by root, not the current user.

If running Caddy as root, this is expected. I would run chown -R to change the owner.

Caddy can't install root certificates without root privileges.

@lukasbestle
Copy link
Author

Caddy can't install root certificates without root privileges, but it can generate them without root privileges. What I suggest is that Caddy could ensure itself that all files it creates are created with the proper permissions and owners. The user of Caddy doesn't know from the outside which files Caddy has created, so they can also not manually chown the files.

I personally am fine with manually chowning the directories, but the current implementation may result in bug reports that Caddy doesn't work because it can't read its own CA files.

@mholt
Copy link
Member

mholt commented Apr 1, 2020

Hmm, I see what you mean, but what is the safe alternative? If caddy trust is run as root, the files will be owned by root if they didn't already exist. (I'm assuming the $HOME var is the same regardless -- make sure that's the case or it will use different storage locations.) If caddy trust isn't run as root, it needs a password when generating the certs.

@lukasbestle
Copy link
Author

If caddy trust isn't run as root, it needs a password when generating the certs.

That's the issue – if I run caddy trust from an automation tool like Ansible, I can't have it interactively ask for a password, so running it with sudo (which is managed by Ansible internally) is the most solid way from Ansible's side.

The $HOME var is passed through by sudo (unless the -H flag is used), so that works just fine. Only the file ownership issue is a problem.

A possible solution could be to detect the sudo state in Caddy (when UID = 0 and the SUDO_UID env var is set) and then drop privileges for the CA generation code back to the SUDO_UID user. The trust store code could still run as root.

@mholt
Copy link
Member

mholt commented Apr 1, 2020

Isn't it risky to trust the SUDO_UID variable, since it can be set to anything?

In any case, this is a pretty nuanced/involved feature request (privilege deescalation in Go is tricky) so I'll probably shelve it for now and just recommend using chown explicitly. A lot less magic involved and a lot less head scratching when you run something as root and it ends up making files owned by non-root.

@lukasbestle
Copy link
Author

Isn't it risky to trust the SUDO_UID variable, since it can be set to anything?

I'd say if Caddy is run by root, you can trust everything as root could do everything on the system anyway. But you are right that there are probably edge-cases like if caddy is added to the sudoers file and an attacker finds a weird edge-case that allows nasty things.

So maybe it's indeed better to leave it as is for now. The current implementation already helps a lot, thanks for that! 👍

@mholt
Copy link
Member

mholt commented Apr 1, 2020

Great, thanks for the consideration!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants