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

Sealed state, or encrypted state at rest #17198

Closed
wants to merge 16 commits into from

Conversation

hargettp
Copy link

@hargettp hargettp commented Jan 25, 2018

Inspired by Ansible's vault files, this feature enables encrypted state for the local backend. The intent is to provide a roughly analogous feature. The feature is intended to enable reasonably secure sharing of state through Git repositories within small teams. Keys may be shared externally but generally stored in consistent filesystem locations (e.g., ~/.terraform/keys) on team members' machines outside of the Git repository containing sealed state.

The changes include:

  • New configuration options for the local backend
  • Additional ReadSealedState and WriteSealedState functions for reading and writing encrypted state; while not implemented here, these same functions may be appropriate for implementing sealed state in other backends (e.g., S3)
  • Updates to documentation for the website
  • A unit test to ensure basic roundtrip encryption / decryption works

NOTE: I am not a cryptography expert. All code written here is an attempt to emulate good patterns that already exist. The implementation depends on keys generated with a password-based key derivation function and AES-256.

While not as complex as the features requested #16066 or #9556, it is intended to enable the basic scenario for sharing encrypted state.

@hargettp hargettp changed the title [WIP] Sealed state, or encrypted state at rest Sealed state, or encrypted state at rest Jan 26, 2018
@apparentlymart
Copy link
Contributor

Hi @hargettp! Thanks for working on this.

This is an interesting alternative approach to the more general options from the issues you linked to. Possibly implementing encryption at rest only for the local backend is a reasonable stop-gap until we're able to design and implement a more general solution.

Given that there are security implications here we'll need to review this one pretty carefully. Unfortunately, the Terraform Core team at HashiCorp is currently focused in a different area (the configuration language) and so we won't be able to give this a thorough review at least until the current project work is complete. I'm sorry for the undefined delay here, but I'm sure you can appreciate that extra attention is required for a feature like this, to properly understand the target threat model, any limitations that users should be aware of, etc.

The main blocker for the other issues you referred to here was, though, the need for a very similar security review, and so we may find that when we are able to look more closely at this that it makes sense to instead apply that review to one of the more general solutions. For a change of this magnitude we usually encourage some discussion about approach before writing code; I apologize that I'm not able to say right now when and whether we could move forward with this, but it looks like there's some good stuff here that could potentially be adapted to form part of a local implementation of what was discussed in #9556.

Thanks again for working on this, and sorry that we're not able to move forward with it immediately. 😖

@hargettp
Copy link
Author

hargettp commented Feb 3, 2018

Hey no worries @apparentlymart !

I totally understand the need to take it slow with this one, especially with respect to security.

A stop-gap is exactly why I chose this path: it was a pretty easy piece of code to put together, and it fills some of the need for securely sharing state in a low-touch way. Smaller organizations may not be ready for Vault or Atlas, nor are they concerned about locking state during updates: this approach makes Terraform usable for them. This choice creates an offering for them, and when they are ready they can scale up to more sophisticated means of securely sharing state.

The other reason I chose this path is that it's a small step towards larger goal: #9556 in particular implies a much larger change. This PR doesn't require a commitment to a larger architectural approach, it simply augments the elements already present (e.g., the local backend). Hence, I thought it less risky, and adds value immediately (e.g., enables encrypted state) without the need to decide on larger architectural issues.

Anyway, happy to leave this open, keep it up to date, and discuss as needed. Totally understand the process may be slow. If it's a good change, it will be worth it.

Thanks!

@hashicorp-cla
Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

@mildwonkey
Copy link
Contributor

Hi! I am going some PR cleanup and am going to close this older PR. Please don't take this as dismissal of the problem, we have not forgotten the state encryption requests, but it is a feature that will we need to design internally in conjunction with our security team. Thank you!

@mildwonkey mildwonkey closed this Oct 1, 2020
@ghost
Copy link

ghost commented Nov 1, 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 as resolved and limited conversation to collaborators Nov 1, 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.

None yet

4 participants