hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.42k stars 9.51k forks source link

Allow users to specify permissions for .terraform dirs #15488

Open ThePletch opened 7 years ago

ThePletch commented 7 years ago

My organization uses Terraform on an Ubuntu system that requires multiple users to be able to interact with our Terraform configuration. We intended to accomplish this by putting all of the users in the same group and granting g+w permissions to our configuration files, but when a new .terraform directory is created by the terraform binary, it is given the permissions -rwxr-xr--, which prevents any user other than the original executor from modifying our persisted Terraform state.

If there isn't already a way to specify what permissions should be granted to files created by Terraform, I'd like to request that as a feature. Working around this is extremely frustrating.

apparentlymart commented 7 years ago

Hi @ThePletch,

Looking at the code I see that Terraform makes this directory with mode 0755, hard-coded. I agree that this seems overly limiting; I think it would be better for it to use 0777 and let the current umask filter out undesired mode bits, and that way you could achieve what you want to do here by:

This approach would then automatically apply to other files and directories that Terraform creates, by building on standard OS features rather than inventing something unique just for the .terraform directory. Multiple users sharing the same working directory is a pretty uncommon approach, but I've no objection to supporting it (with a "buyer beware" caveat) if we can get there with just better application of existing OS features.


With that said, in the short term could you ensure that the .terraform dir exists with the right permissions before running terraform init, or are you running into problems with files in that directory also being created with unsuitable permissions?

cytim commented 6 years ago

I am having exactly the same question. Hopefully 0777 could be applied to the .terraform dir by default.

rbestbmj commented 5 years ago

This seems fixed to me in 0.12.0-alpha4 (and when I build from master) but I can't see how because in the source code it seems to me that the directory is still created with 0755 (and files with 0644).

apparentlymart commented 5 years ago

Indeed we haven't intentionally made any attempt to fix this yet because we've been focused on configuration language stuff for the 0.12 cycle. I'm as puzzled as you are as to why the behavior would've changed here. My guess would be that there is another codepath using os.MkdirAll to create a subdirectory of .terraform and it happens to sometimes run before the codepath that would normally create it and thus implicitly create that .terraform directory with a different mode. We'll need to trace through the code to figure out what codepath is doing that, if indeed that's the cause.

As noted before, creating the directory with the appropriate permissions before running terraform init should get the same end-result for now, since Terraform will only try to create the directory if it doesn't already exist, and so I'd recommend using that workaround for now until we're able to more directly address this. (Even if something is inadvertently creating the directory with looser permissions in 0.12.0 alphas, it's possible that it isn't reliable since it wasn't a change made intentionally.)

rbestbmj commented 5 years ago

Creating .terraform before running terraform init doesn't workaround the issues we're getting with 0.11.11 because other files and directories inside .terraform don't respect the umask either.

For instance:

I can mkdir the modules dir and touch the lock.json file into place with the desired permissions but I think the providers present an issue for a shared workspace because they will be removed and recreated as providers are added and updated.

For now, we're wrapping our terraform executions and following them up with chmod -R to keep the permissions in check, which works for us, as ugly as it is. As you say, a shared workspace for terraform is a bit of an unusual case and the real solution is probably to move away from that model.

As for 0.12, looking at the code it seems to me like terraform init respects the umask because the directory and the modules/providers are created with os.ModePerm rather than a hardcoded octal value. For instance, here.

However, terraform workspace new <name> produces a directory with the mode set to 0755. See here.

Does it make sense to replace all explicit numeric notations with os.ModePerm when creating files and dirs?

apparentlymart commented 5 years ago

I was expecting (though admittedly I didn't test) that if you create .terraform and add the setgid bit to it then anything inside the directory would be created with the inherited group ownership. I can see though that this won't help if the contents don't have the group-writable bit set.

Sharing a single Terraform directory between multiple users isn't an intended usage pattern, as I mentioned above, so while I'm still open to making it possible I will be honest and say it's unlikely to be something the Terraform team and HashiCorp will be able to prioritize in the near future. However, after 0.12.0 is released (not before then, because the release scope is already large) we'd be happy to review a PR to make the permissions handling more consistent and deliberate.

Although in principle os.ModePerm should be fine for those who have a suitable setting of umask, creating files and directories that way can make it easy to accidentally make access too permissive without expecting it, so I think perhaps the best compromise would be to set rwxrwxr-- (that is, only owner+group read/execute) so that the umask can control the owner+group mode but the files can never become world-writable even if the umask is configured incorrectly. I think this multi-layer defense is warranted for the .terraform directory because it contains executable files for plugins, and so an replacing those plugin executables would lead to execution of arbitrary code, likely with access to cloud provider credentials.

rbestbmj commented 5 years ago

Thanks @apparentlymart for the prompt and reasoned responses! I agree with your points RE: os.ModePerm and I can completely understand the low priority of this issue considering the nature of it and the impending 0.12.0 release.

I'll look at getting a PR together for after 0.12.0 drops.

apparentlymart commented 5 years ago

Just to connect the dots here for future reference: over in #20410 there is a request that makes essentially the opposite assumption, making it so that a Terraform directory using local state could not be shared between users.

As I mentioned over there, I think it's clear that right now Terraform's position on this topic is unclear and inconsistent, so I've labeled both issues for future research so we can reach some conclusions about which usage patterns are considered safe/valid and which are unsupported, and then we can update all of the parts of Terraform that generate files to be consistent with those conclusions.

Unfortunately we won't be able to engage in this research in the near future since our focus is elsewhere, but we'll take a look at it next time our attention moves to the CLI/backend parts of Terraform, since that's when we'll have the suitable context loaded to think this through properly.

leonardoauribe commented 2 years ago

Hi @apparentlymart,

Has there been any progress with this? We are managing some resources within our production boundary and limited to a specific host and directory structure. Right now all of our team members execute Terraform from the same directory. They are unable to run cases due to this limitation with the .terraform directory locked down to only the owner.