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

Vault Provider #9158

Merged
merged 5 commits into from
Nov 8, 2016
Merged

Vault Provider #9158

merged 5 commits into from
Nov 8, 2016

Conversation

apparentlymart
Copy link
Contributor

This PR establishes a new provider for Vault, including support for reading and writing generic secrets to start.

Terraform lacks any proper support for managing secrets today, so there are some pretty big security caveats to using this provider. Rather than waiting for Terraform to grow features to deal with this, I chose to just be very explicit in the documentation about the implications of using this provider. I expect that users can decide for themselves if use of this provider is appropriate in their environment, and take any extra steps they wish in order to take to mitigate the risk that this provider creates.

In an attempt to minimize the risk of retrieving secrets using this provider, I have designed it to request a time-limited vault token so that the window of exposure can, where possible, be limited to a single Terraform plan/apply cycle, expecting that fresh secrets will be requested on the next run. This time limit defaults to 20 minutes but can be extended through configuration. An important caveat of this strategy is that when running plan and apply as separate steps the later apply will fail if the user waits too long.

For now this provider is focused on the following two use-cases:

  • Initial population of data into Terraform, for (careful) use by the team that manages the Vault installation.
  • Retrieving data from Vault in order to get credentials with which to configure other providers that Terraform will interact with directly.

There is a third use-case which this provider is explicitly not attempting to address yet: retrieving credentials on behalf of another party (e.g. using Response Wrapping) with the intent of those credentials "outliving" the Terraform run. I think issuing longer-lived credentials represents considerably more risk and so for now I think it best to encourage solving this problem out of band of Terraform, e.g. using secure introduction with the AWS-EC2 provider or with the forthcoming Nomad/Vault integration.

I have implemented both a resource and a data source called vault_generic_secret. These are primarily intended for and tested with the "generic" secret backend, but due to the generic nature of Vault's API they can be used with several other backends too.

I chose to call these "generic secret" rather than just "secret" because I anticipate later adding specialized resources to deal with more complex cases such as the PKI backend, the SSH backend, the Transit backend, etc, and so want to be able to clearly distinguish them.

I'd also like to later add resources for configuration of Vault, so that Terraform can be used to manage mounts, auth providers and policies.

@apparentlymart
Copy link
Contributor Author

$ make testacc TEST=./builtin/providers/vault
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/09/30 18:51:16 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/vault -v  -timeout 120m
=== RUN   TestDataSourceGenericSecret
--- PASS: TestDataSourceGenericSecret (0.04s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestResourceGenericSecret
--- PASS: TestResourceGenericSecret (0.04s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/vault  0.090s

@jen20
Copy link
Contributor

jen20 commented Oct 1, 2016

Hi @apparentlymart! There is also some work on this that @phinze did a long time back, which was waiting for data sources - it's on a branch here but there was never a pull request opened for it so it's understandable if you'd not seen it. Given that others have more context both on Vault and on the history of Vault integration with Terraform, I'm going to leave this for one of them to review, but it might be that we can reuse some of this code to expand the scope of this to include, for example, PostgreSQL and AWS backend support as well as generic secrets. Thanks for the pull request!

@jen20
Copy link
Contributor

jen20 commented Oct 1, 2016

(Also as an aside, the build errors are a missing package from the vendoring of the Vault API, should be easy to fix up).

@apparentlymart
Copy link
Contributor Author

Hmm apparently I still haven't quite figured out how to use govendor properly. I'll take another pass at getting this working hopefully sometime this weekend or early next week.

@apparentlymart
Copy link
Contributor Author

Also, I remembered that @phinze had talked about working on Vault provider support but I didn't realize he'd done so much work in that direction already. I'll take a look at what he did there; I kinda want to just get the basic infrastructure in place right now and then gradually expand it with more over time.

@apparentlymart
Copy link
Contributor Author

After looking at @phinze's branch, I notice some design differences that are probably worth some discussion:

  • It looks like his implementation was intended to support authenticating via one of the auth backends. In my case, I decided to just make Terraform accept a token and assume that this token was obtained by some means out of band of Terraform. I went the way I did because I assumed users would either use the vault CLI tool to get a token, which already has a nice UX for authenticating, or they'd be running Terraform in an automated context where e.g. a token would be issued by some sort of secure introduction mechanism.
  • He started with the "configuration"-type resources like mounts, policies, etc. I need those too so I have no objection to that... I just chose to start with secrets first because it seemed like the simplest initial resource.
  • It looks like he was planning to make a single "secret backend" resource that both creates mounts and has special code for configuring each kind of backend that can be mounted. I was expecting to split these ideas so that e.g. you'd use a vault_mount resource to mount it and then use other resources -- either the vault_generic_secret resource or something more backend-specific -- to configure it.

@apparentlymart apparentlymart force-pushed the f-vault-provider branch 2 times, most recently from 09c4569 to e96d6dd Compare October 2, 2016 16:36
@apparentlymart
Copy link
Contributor Author

I have now learned what all the govendor commands actually do and fixed my vendoring of the vault API. 😀

@daveadams
Copy link
Contributor

I'm very excited about this. Other than the secret reading and storage, I'm also interested in these use cases:

When I create an IAM instance role for a new service in Terraform, I want to be able to then create a secret mount in Vault associated with that role, corresponding Vault read and write policies to that mount, an aws-ec2 Vault role mapped to that IAM instance role's ARN with the read policy, and a pseudo-group on the LDAP auth mount with the write policy, and then grant a list of LDAP users membership in that pseudo group. Lots of pieces there, but that's a very immediate use case I would love to codify in Terraform vs a bunch of flaky bash scripts.

An option to store certain state attributes in Vault rather than in tfstate. I'm thinking here of RDS root passwords, IAM access key secrets, etc, etc. Thinking bigger, it would be great if there was some metasetting (similar to lifecycle{ignore_changes}) that would allow me to specify particular fields I wanted stored elsewhere instead of raw in the tfstate file. The tfstate format could perhaps provide a URL or other type of pointer to where the value is stored, which could be Vault, some other secret management system, a database, S3, whatever. This is sort of asking for an entirely new class of provider, so probably this use case is beyond the scope of this ticket.

@apparentlymart
Copy link
Contributor Author

Hi @daveadams. Thanks for that feedback!

I have many of the same use-cases you described in your first paragraph; it sounds like we're both using Vault in a similar way. I definitely would like to include mounts, auth providers and policies as well, but prefer to get the basic provider machinery reviewed and merged first. 👶👣

Regarding the secure storage of secrets rather than encoding them in state... I've been thinking a bunch about that separately myself. I'd agree that it's not in the scope of this ticket; I think such a mechanism would likely end up looking like a "data encrypted at rest" situation where the data does still live in the state file but the state file itself is either encrypted as a whole or supports encryption of just the sensitive attributes, and then have a selection of crypto providers to actually do the encryption/decryption of this data for Terraform as part of decoding the state.

The backends here could work in a similar way as our remote state storage backends, supporting services like Amazon's KMS and Vault's "transit" backend, but it would be an orthogonal setting to where the data is stored, so that e.g. I can encrypt the data using KMS but store it in Consul.

I should probably write up a separate ticket for that at some point; there are definitely some tricky details to figure out. 😀

@sushanthku
Copy link

When is this slated for release?

@mitchellh
Copy link
Contributor

Hey @apparentlymart: we're now prioritizing this for 0.8 or even sooner. What else do you have TODO on this? (and /cc @sushanthku since this answers your question)

Re: encrypted values: yes, @apparentlymart is correct in that encrypting the entire state seems much easier than per-field encrypted values.

@apparentlymart
Copy link
Contributor Author

@mitchellh it's been a little while since I looked at this so I might be forgetting something, but IIRC this is ready for review as far as I'm concerned.

One of my comments above, talking about @phinze's work, lists some situations where he and I went in different directions (since I didn't know about his branch at first). Wasn't sure if that was the result of some internal discussion within the team, so I wanted to point those out in case there were some strong reasons for those choices.

@daveadams
Copy link
Contributor

Great to hear this is targeted for 0.8. I'm very excited about it.

@apparentlymart @mitchellh In re encrypted values, yes, it would be easier to encrypt the entire state, but that could be easily handled outside Terraform. But encrypting the entire state to protect one or two sensitive values becomes an all-or-nothing proposition. So I can't share a state file with some other team to use as a remote state resource if it has any sensitive values in it.

@apparentlymart
Copy link
Contributor Author

Hi @daveadams... I just posted #9556 as a more detailed overview of what I was thinking for remote state storage, including discussion of some of the caveats of that design including your issue of sharing state. Let's continue the discussion over there, and leave this PR for just getting the basic Vault provider functionality in place.

@cblecker
Copy link
Contributor

To bring up another approach that I think could work, something similar to #7237. The only thing I would change is not forcing the use of the ignore_changes list.

The thing I like about this approach would be that it could easily be used in conjunction with a Vault provider, because then the secrets would always be pulled and evaluated directly from a secure, encrypted backend and not needed to be stored in state at all. Then on refresh, plan, or apply, you repull those secrets again and compare to what's out in the cloud.

May not work in all cases, but it would help in many of them.

Also includes no-op upgrades to various pre-existing vendored Vault packages.
To reduce the risk of secret exposure via Terraform state and log output,
we default to creating a relatively-short-lived token (20 minutes) such
that Vault can, where possible, automatically revoke any retrieved
secrets shortly after Terraform has finished running.

This has some implications for usage of this provider that will be spelled
out in more detail in the docs that will be added in a later commit, but
the most significant implication is that a plan created by "terraform plan"
that includes secrets leased from Vault must be *applied* before the
lease period expires to ensure that the issued secrets remain valid.

No resources yet. They will follow in subsequent commits.
This resource allows writing a generic secret, and indeed anything else
that obeys the expected create/update/delete lifecycle, into vault via
writes to its logical path namespace.
@mitchellh
Copy link
Contributor

Okay @apparentlymart I've taken a look at both yours and @phinze's.

I'm going to merge yours as the base point. I think @phinze has some good extra stuff that is worth taking a look but I think yours is a clean slate to start with with only one resource, and I think the generic secret datasource and resource is a good way to go.

I also like your auth style, I think removing auth out of the provider is indeed a much simpler way to do things.

@mitchellh mitchellh merged commit 646b3c1 into master Nov 8, 2016
@mitchellh mitchellh deleted the f-vault-provider branch November 8, 2016 23:27
@Bhuwan Bhuwan mentioned this pull request Nov 12, 2016
@triggity
Copy link

Hi, do you have an idea of when this will be released?
I don't see it in the 0.7.11 releases, it is in the 0.8 beta releases. when is that expected to be out?

@mitchellh
Copy link
Contributor

0.8 RC1 will start tomorrow, 0.8 final will be out mid-December

@triggity
Copy link

thanks

@ghost
Copy link

ghost commented Apr 19, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants