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

Support passing secrets using systemd LoadCredential directive #491

Open
squalus opened this issue Jul 31, 2022 · 6 comments
Open

Support passing secrets using systemd LoadCredential directive #491

squalus opened this issue Jul 31, 2022 · 6 comments

Comments

@squalus
Copy link
Contributor

squalus commented Jul 31, 2022

I would like vouch-proxy to support the systemd LoadCredential directive for passing secrets. OAUTH_CLIENT_SECRET and OAUTH_CLIENT_ID are good candidates for this.

vouch-proxy could read the files at $CREDENTIALS_DIRECTORY/<secret_name> as a lower priority item than the environment variables.

Proposed implementation in #487

Systemd credentials documentation: https://systemd.io/CREDENTIALS/

@bnfinet
Copy link
Member

bnfinet commented Jul 31, 2022

@squalus thanks for your PRs and thank you for opening this issue and for bringing the conversation here. I very much appreciate you taking the time to look at VP's code and implementing this improvement.

I'm not familiar with systemd credentials, but I'm curious..

There are several items including the OAuth credentials in the configuration which are secrets. Could we hold the entire VP configuration in a file under $CREDENTIALS_DIRECTORY

By holding the entire configuration there, that feels future proof and succinct with regard to the implementation.

@squalus
Copy link
Contributor Author

squalus commented Jul 31, 2022

I think the main purpose of this is to be able to manage the config file and the secrets in a different place. For example, your config file can be committed to git, or generated with helm or nix. And if you use systemd credentials, you can still manage your configuration in a nice way but leave the secrets out of it.

So I wouldn't want to move the entire configuration file to the secrets management system. Then I wouldn't be able to, for example, store the listen address or OIDC domain in a variable in my templating system.

If there are only a small number of secrets and we don't expect that list to change very often, perhaps we can add a few more calls to readSystemdSecret? Another alternative is to accept an entire config file in $CREDENTIALS_DIRECTORY, but to "merge" it with the normal one. That way I only need to store the secret bits of the config in the secrets system.

@bnfinet
Copy link
Member

bnfinet commented Jul 31, 2022

Yes thanks, but just humor me for a minute and help me explore a bit.. Is it possible to put the entire configuration into the $CREDENTIALS_DIRECTORY?

@squalus
Copy link
Contributor Author

squalus commented Jul 31, 2022

Yes thanks, but just humor me for a minute and help me explore a bit.. Is it possible to put the entire configuration into the $CREDENTIALS_DIRECTORY?

Yes, you could do something like this:

LoadCredential=config:/real/path/to/vouch.yml

And at runtime, vouch-proxy will be able to read the config file from $CREDENTIALS_DIRECTORY/config

@bnfinet
Copy link
Member

bnfinet commented Aug 5, 2022

@squalus thanks for the clarification. Sorry for the delay in response. FYI - The next few weeks are going to be like this due to summer travel plans.

Another alternative is to accept an entire config file in $CREDENTIALS_DIRECTORY, but to "merge" it with the normal one

I like this idea. By my reckoning it would be straight forward to implement, it is future proof and it allows other sources of config to be added in a similar fashion.

I'm uncertain of how to handle config elements being set by one config and then overwritten by another config. Perhaps it's enough to be very clear about what's going on at startup with good logging; provide additional checks to ensure that any time a config item is already set and then overwritten by the next configuration it's log.Warn(). Perhaps some items such as secrets and any OAuth config items could only be set once. That may be too restrictive.

@squalus
Copy link
Contributor Author

squalus commented Aug 7, 2022

Updated the PR to support a full config file overlay. It can either come through a new env var, or through CREDENTIALS_DIRECTORY. Other secrets management systems, like Docker secrets, could use the env var approach.

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

No branches or pull requests

2 participants