saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.11k stars 5.47k forks source link

Add Vault Pillar/SDB Module #27020

Closed arnisoph closed 7 years ago

arnisoph commented 9 years ago

Vault secures, stores, and tightly controls access to tokens, passwords, certificates, API keys, and other secrets in modern computing. Vault handles leasing, key revocation, key rolling, and auditing. Vault presents a unified API to access multiple backends: HSMs, AWS IAM, SQL databases, raw key/value, and more.

jfindlay commented 9 years ago

@bechtoldt, thanks for the report.

techhat commented 9 years ago

@bechtoldt that sounds fantastic!

eedgar commented 9 years ago

@bechtoldt do you have any implementation ideas for this?

eedgar commented 9 years ago

https://github.com/ianunruh/hvac

arnisoph commented 9 years ago

@eedgar Nope, I haven't planned to provide a patch so far.

eedgar commented 8 years ago

I did a little research and there are a few problems with the sdb implementation that make this challenging. currently sdb allows for module/key where modules are defined inside the minion or master config. this does not allow you to specify the full path within the vault very easily .. eg myvault/secret/hello ... sdb would only pass secret to the vault module and loose the hello parameter. Additionally vault secrets can store more than one key value pair inside them .. How can we support which one to retrieve. I think this functionality would be better served creating a vault module with read and write methods.

then use them inside a jinja template like this. {{ salt.vault.read('myvault/secret/hello', key='value1') }} key can be an optional parameter.

techhat commented 8 years ago

SDB is designed to be simple to call; long URIs defeat the purpose. The intent is that an sdb:// URI would be just enough to refer to a profile configuration which can be much more complex, if necessary.

My suggestion here would be to implement full paths inside the profile, not the URI. Think of the profiles like stored procedures, rather than just connection parameters.

eedgar commented 8 years ago

the only thing I was thinking was that the number of profiles could get to be quite numerous depending on the number of paths you need to traverse. I think a more feature complete vault module is the way to go here. maybe implement a subset of that for sdb if the demand is there.

eedgar commented 8 years ago

eg think dynamic paths such as /secret/server1/auth /secret/aws_domain1/credentials /secret/aws_domain2/credentials .. these things can add up fast.

DanyC97 commented 8 years ago

@eedgar have you made any progress on it or you hit a dead end with the sdb? (asking as i'm trying to do the same thing but i'm a bit limited by the options )

jbussdieker commented 8 years ago

+1 for a vault module

coen-hyde commented 8 years ago

I hate to just add another +1, but this would be good.

ketzacoatl commented 8 years ago

Would there be interest in a modules implementation that is not using SDB?

If only as a first pass, I believe it would be more valuable to get this working in a simple form, and leave the SDB interface questions for a later date. The simplest form I can see would have a vault module that could get a key, similar to pillar.get, and which could be used in a template like:

{%- set my_secret = salt['vault.get']('path/to/key') %}
{{ my_secret }}

This could also keep it simple (on first pass), by using VAULT_TOKEN from the environment.

EDIT: to implement this as a pillar module would put the secrets in the salt log (right?)

dmangot commented 8 years ago

@ketzacoatl I had always envisioned vault as just another pillar back end (like hiera, mysql, s3, etc.) so I'm interested whether it's implemented using SDB or not.

techhat commented 8 years ago

A pillar or execution module would be awesome.

ketzacoatl commented 8 years ago

An execution module means the secrets don't inadvertently end up in the salt log, but a pillar module means users can make use of this more seamlessly (retain loose coupling with salt formula).. but maybe the pillar module could use the execution module, so that both are available?

One thing I am not sure of is how you would retrieve all child keys from a parent path in Vault. I will look into that.

If that sounds good, I would be willing to take a first pass on this. I would use https://github.com/ianunruh/hvac, and keep authentication bare-minimum (use token to start).

tspecht commented 8 years ago

Any progress on this? Would be awesome if we could get vault as a pillar backend for Salt!

ketzacoatl commented 8 years ago

@tspecht, I have it on my todo list, but I have not started on it. I hope to do so in the next week or two. Anyone else is welcome to as well, I'm not looking to block, I am just interested in meeting this need but also tied up at the moment.

thatch45 commented 8 years ago

@techhat would be the best one to implement this one, but if you start before he does @ketzacoatl let us know. I will slate this for the Carbon release, since the boron release has been frozen

techhat commented 8 years ago

@bechtoldt, @eedgar, @DanyC97, @jbussdieker, @coen-hyde, @ketzacoatl, @tspecht, would one or more of you please take a look at #31333? I only installed Vault about two hours ago, so I'm sure I'm not doing great on best practices.

techhat commented 8 years ago

Pillar module at #31371.

carlpett commented 8 years ago

I wrote a module for Vault which we've tested a bit internally for a few months but never got around to releasing it. There are a few things that needs to be solved for this to be actually useful:

I could do some polishing up make it possible to open source, but in general I'd say that quite a bit of thought is required to make sure that the salt integration does not completely compromise Vault by making anyone able to dump the contents

DanyC97 commented 8 years ago

Hi @techhat , i can't test it now but if you okay i'd like to add few things in case you would like to consider:

techhat commented 8 years ago

Thanks @carlpett and @DanyC97, this is exactly the kind of feedback that I was hoping for! @carlpett, if you would like to submit your module, I have no problem with replacing mine with it. Until I see it, I'll start working on your items. @DanyC97, I hadn't seen that gist, but outside of the fact that you should never use public classes inside any module that goes through the loader, it looks really good. I hadn't thought about making a renderer, but I'd love to see any PRs that contain one!

techhat commented 8 years ago

@DanyC97, what were you thinking in terms of a renderer? I'm not sure how you would go about replacing the GPG renderer with a Vault renderer.

techhat commented 8 years ago

@carlpett, could you elaborate on how you're currently using policies with Vault? Maybe some example HCL? As I said above, I'm very new to Vault. ATM, it looks like all of the policy stuff (and the backend stuff) is all configured on the Vault server itself, and I don't see any reason why a non-root token wouldn't work.

ketzacoatl commented 8 years ago

@techhat, right, the policies are a server-side thing. The other issue on tokens is that you might have a long-lived token, or there might be a TTL. Vault clients with non-root tokens need to know how to renew the token - see https://www.vaultproject.io/docs/concepts/tokens.html

carlpett commented 8 years ago

@techhat I'm not sure I understand your question, and probably that means I wasn't very understandable either :) What we want to ensure is that secrets cannot be rendered to a minion which shouldn't have access to them. A concrete example would be database credentials - a minion running the test environment shouldn't be able to get the production credentials through misconfiguration. What we did to get there was to set policies on the tokens that the salt master uses to fetch things from Vault, so when it is fetching data for some minion it uses a token with a policy dictating exactly what this minion can read. Given minions app-test and app-prod, we'd have policies salt/minion/app-test containing something like path "secret/my-app/test/* { policy = "read" } and salt/minion/app-prod with path "secret/my-app/production/* { policy = "read" }. This way, if my salt configuration says to use secret/my-app/production on app-test, it will fail, rather than pushing production credentials to the wrong machine.

I've had a nasty migration project at work which is just about done now. Hope to get time to do the cleaning up of the module shortly.

DanyC97 commented 8 years ago

@techhat regarding your question what were you thinking in terms of a renderer? is exactly this use case

The typical use-case would be to use ciphers in your pillar data, and your encryption key will be on the Vault server.

which currently exist in GPG render flow use-case

coen-hyde commented 8 years ago

@DanyC97 I can see that use case being valuable. It's different from my use case though. I would keep the pillar data in vault its self. This is because other applications use / update these secrets as well.

DanyC97 commented 8 years ago

@coen-hyde sure thing :+1:

carlpett commented 8 years ago

So, I've spent some time on this again. Considering the points I made earlier, I'm not fully satisfied with the solution that was merged in #31371 due to how I cannot control what secrets are possible to render to what minions. Not sure if those are final versions for Carbon, or first drafts, though? I'm having some problems with my own take on this, however, probably due to confusing a few concepts in salt. I would like parts of my code, the parts which handle the sensitive token-generating-token, to execute on the master only. Is there some pattern to defer parts of my module to the master in a secure way? Basically I would like the execution module running on the minion to fetch a one-time token from the master.

ghostsquad commented 8 years ago

@carlpett Getting a one-time token from the master could be done using ext_pillar.

https://docs.saltstack.com/en/latest/topics/development/external_pillars.html

From looking at #24050, most calls to pillar will use the cache, explicitly calling pillar.items or saltutil.refresh_pillar within the module will force the ext_pillar to execute, which will get a fresh token.

carlpett commented 8 years ago

@ghostsquad Ah, interesting idea. I've been fiddling around with peer runners a while, but there are some bugs there that make ensuring it works across different versions a bit of a pain. ext_pillar could be a good workaround there, I'll give it a shot!

carlpett commented 8 years ago

Ok, finally finished it up. Works well enough and also supports state management of Saltstack policies: https://github.com/carlpett/salt-vault

Would appreciate feedback, especially regarding any security-related oversights. The minions get tokens by communicating with the master using the peer-runner mechanism. Due to a bug in saltstack (https://github.com/saltstack/salt/pull/32335), the id that is presented to the runner code is not correct, so I'm sending a signed message (using the minion private key) as verification of identity. Any issues with this?

EDIT: Also, this is my first salt module with multiple files, so a structural review from that perspective would also be much appreciated. Esp, is there a good way to share code between state and execution modules?

ketzacoatl commented 8 years ago

@carlpett, to put in voice in for those with ultra-simplified (or non-existent) salt-masters - I generally run consul + salt-call in a way that avoids the HA burdens for a salt-master. While I like the architectural options available with salt-minions retrieving temp tokens from vault, through their salt-master, a solution that explicitly requires the salt-master can be problematic, and where possible, I like to advocate for the minion retaining the same capabilities. FWIW.

carlpett commented 8 years ago

@ketzacoatl Interesting point, had not thought of that one. The issue with that is how to manage what the minions can read? Configuring all the minions with a Vault root token would sort of default the point of using Vault? If the machine is compromised, it is capable of reading or altering anything within Vault, and even locking everyone else out.

Do you agree, or am I missing something? Is there a benefit of using Vault over just having the credentials in pillar?

ketzacoatl commented 8 years ago

@carlpett, well, with ACLs, you are not required to use a root token - https://www.vaultproject.io/docs/concepts/tokens.html.

carlpett commented 8 years ago

@ketzacoatl Yes, but non-root tokens expire, though, so you'll need to have a system that renews them periodically (you can do mount-tunes to make the expiry time longer, but still something to be aware of). And it might be a pain to manually generate all those tokens with the correct policies.

But if this is a common thing, I could add some support for setting the token in the minion config as well, instead of going to the master, shouldn't be very complicated. I probably wouldn't recommend using it to anyone, though.

carlpett commented 8 years ago

@ketzacoatl I just pushed a branch for masterless, care to have a look if it looks like it would fill you requirements? https://github.com/carlpett/salt-vault/tree/f/masterless

ketzacoatl commented 8 years ago

@carlpett, It's a tiny bit difficult for me to see the details (not sure if that's end of week or something else). Maybe you can add a short doc example for how that would be used with the minion or salt-call?

carlpett commented 8 years ago

@ketzacoatl Sorry about that. I've updated the README (last section) with instructions for masterless.

ketzacoatl commented 8 years ago

LGTM, thanks for all your work on this!

eliasp commented 8 years ago

is there a good way to share code between state and execution modules?

@carlpett You might want to move common code to a utils module…

carlpett commented 8 years ago

@eliasp That sounds like just the thing :) Does that work for modules that are not distributed along with salt itself, though? Inhouse, we add our custom modules by adding their git-repos as gitfs_remotes; but this may not be the best way to do it?

carlpett commented 8 years ago

@techhat Since I took my time in getting this ready and you built a module in the meantime, what are the plans for this issue?

techhat commented 8 years ago

@carlpett, I don't mind my code being replaced with superior code.

eliasp commented 8 years ago

@carlpett The best way to distribute custom modules (including utils) is to use Dynamic Module Distribution. For this reason I maintain a local salt-dynmods repo containing the corresponding module directories.

Although it's not perfect yet (#24488), it's quite useful for distributing newer or development versions of specific modules.

carlpett commented 8 years ago

@eliasp Aha, didn't know about _utils. I had already put the state, exec and runners in underscored directories. I'll extract the shared logic to _utils then.

carlpett commented 8 years ago

Took quite some time again between work and refactoring to get _utils to work, but I've pushed the changes now. What's the process of getting it into "mainline salt"? Just create a PR?