Salt: Add Vault Pillar/SDB Module

Created on 10 Sep 2015  Â·  57Comments  Â·  Source: saltstack/salt

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.

11147

26236

Execution Module Feature P2 Pillar Platform

Most helpful comment

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?

All 57 comments

@bechtoldt, thanks for the report.

@bechtoldt that sounds fantastic!

@bechtoldt do you have any implementation ideas for this?

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

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.

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.

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.

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

@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 )

+1 for a vault module

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

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?)

@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.

A pillar or execution module would be awesome.

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).

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

@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.

@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

@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.

Pillar module at #31371.

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:

  • Policies: You need to be able to determine that the minion should actually have access to the secret that you are reading. Our prototype assumes there is a policy saltstack/minion/<minion-id> and generates single-use tokens with this policy for every secret that is read. That _works_ but assumes that you want saltstack to have a root token, which might not be the case (the newly released Vault 0.5 does some changes with the permissions model which I haven't had time to look into yet, maybe a root token is no longer needed?).
  • More policies: Also, it gets a bit old to set up these policies manually when provisioning new machines, so a state module which allows for managing the polcies is needed. I haven't written this part yet, due to not feeling quite confident in how this affects security.
  • Better support for non-generic backend: esp support for amazon tokens and pki backends would be nice. Some require POSTs or other not-quite-so-straightforward communication, so some thought is required in how to design the interface to salt here.

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

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:

  • your PR #31371 looks very similar with https://gist.github.com/ryancurrah/40aefb2baa8a69522127 (note is not my credit :) ), sorry i didn't mention earlier although knew/ tested it a bit.

    • the note i'd add is that the work done by Ryan came as result of https://github.com/hashicorp/vault/issues/323 where the Transit backend is used (in this way you don't care about the lease time as very well explained by Armon)

  • the time doesn't allow me to push a PR but i'd add add more docs info as the the gist i mentioned earlier including the note about the Transit backend (i bet a lot of people will get caught by this when they will just want a simple place to store passwords :) )
  • also maybe you will consider the renderer too so we can replace the GPG one

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!

@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.

@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.

@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

@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.

@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

@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.

@coen-hyde sure thing :+1:

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.

@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.

@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!

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?

@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.

@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?

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

@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.

@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

@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?

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

LGTM, thanks for all your work on this!

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…

@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?

@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?

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

@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.

@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.

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?

@carlpett yes

+1 for anything which allows me to use decentralized vault as an alternative to pillars for secrets management.

Sorry - I totally forgot to create the PR for this. Got stuck with some linting problem or similar. I'll try to get it done during the weekend!

@carlpett Has there been an update? Is your module releasable?

@mchugh19 Wow, good reminder... Sorry about being so slow. We've been using it internally for over half a year without any problems, so I guess so. With 2016.11 having support for utils-syncing on the master it should actually be pretty simple to just add it as another file_root (or gitfs remote, that is what we are doing). For 2016.3 you need to do a manual copy of the util to the master's extmod, which is a bit of a bother.

Anyway, it would be better to get it into the distribution. I'll give it a go and hope I can finish it up this time.

@mchugh19 As an update, I've merged the code into a branch on the main saltstack repo now, and removed the majority of linting issues. I'm still working on minimizing the breakage from the existing code, hopefully I can finalize it tomorrow.

This has now been merged, a year later. There were a few fixes on the separate repo just the last days, which I'll port over in a separate PR.
But I think this issue can be closed now?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sfozz picture sfozz  Â·  3Comments

udf2457 picture udf2457  Â·  3Comments

seanacais picture seanacais  Â·  3Comments

Oloremo picture Oloremo  Â·  3Comments

qiushics picture qiushics  Â·  3Comments