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

Don't store SSL private_key in the tfstate file #927

Closed
ofpiyush opened this issue Jan 6, 2018 · 16 comments
Closed

Don't store SSL private_key in the tfstate file #927

ofpiyush opened this issue Jan 6, 2018 · 16 comments

Comments

@ofpiyush
Copy link
Contributor

ofpiyush commented Jan 6, 2018

Thank you for the amazing software

Terraform Version

Terraform v0.11.1

Affected Resource(s)

  • google_compute_ssl_certificate

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this.

Terraform Configuration Files

resource "google_compute_ssl_certificate" "default" {
  name_prefix = "my-certificate-"
  description = "a description"
  private_key = "${file("path/to/private.key")}"
  certificate = "${file("path/to/certificate.crt")}"
}

Expected Behavior

The private key should not show up in tfstate

Actual Behavior

It did

Steps to Reproduce

terraform apply

Possible solution

Store hash of the key instead of complete key, like we do for google_storage_bucket_object

@ofpiyush
Copy link
Contributor Author

ofpiyush commented Jan 6, 2018

It looks like whatever data is passed in, is stored as is.

Can we add two non-computed fields for completeness

  • private_key_file
  • certificate_file

And two computed fields

  • key_hash
  • cert_hash

This will allow us to track the old and new hash of the certificate without having to store the certificate itself.

I assume certs can not be edited once they are created, so this should do.

@ofpiyush ofpiyush changed the title Don't store SSL certificate or key in the tfstate file Don't store SSL private_key in the tfstate file Jan 6, 2018
@rosbo
Copy link
Contributor

rosbo commented Jan 8, 2018

@paddycarver is there any work in Terraform core to handle more securely the fields marked as Sensitive?

@paddycarver
Copy link
Contributor

There's talk, and thoughts, but I don't think it's on the immediate roadmap. Storing the hash of the private key is an option, and something we could do, but I'm wondering if it's worthwhile. After all, the private key must still be on disk, readable by whoever is running Terraform, and in cleartext anyways. I'm struggling to find a parallel to this to see if we've addressed it elsewhere.

I'm weakly against making this more complicated, and would rather advise that state be treated as sensitive (which it generally should be, anyways), but could be convinced that the complexity is worthwhile.

@ofpiyush
Copy link
Contributor Author

@paddycarver our use-case was to keep all workspaces and the state in the same github repo.

But with all things around secrets going sideways, we ended up making 2 levels of state.

  1. For ops project. This state is checked into github. Among other things, it has a gcs bucket with correct permissions to hold state for individual workspaces.
  2. Workspace specific state, that is kept in the bucket created above.

We would have preferred to keep it all in one place, secrets encrypted with keys that we could easily share offline.

Is such a setup possible?

@paddycarver
Copy link
Contributor

I think the prevalent recommendation among the Terraform team is to store your state in remote backends, which can then be encrypted. GCS' remote state backend, for example, is encrypted at rest, and access to it can be granted using GCP credentials. You can even supply a Customer Supplied Encryption Key to encrypt the state with your own key. My understanding is each developer would then need that key.

Unless I'm mistaken, storing Terraform state in GitHub is discouraged. It was encouraged early on as a practice when state files rarely carried sensitive information in them, but as sensitive information becomes more and more prevalent in the state file, it became clearer that storing the file in GitHub may not be the best solution.

@ofpiyush
Copy link
Contributor Author

Adding lessons we learnt about GCP. Using GCS buckets are a little weird at this moment.

One gets read access to storage buckets even if they are a viewer on that project or have any permission related to storage, irrespective of bucket's own permissions.

This means a new project needs to be made just to manage this state.

@paddycarver
Copy link
Contributor

Hm, I wonder if that is changing with the new custom roles. I also bet the Google Cloud Storage team would love to hear about it (you can open an issue against GCS here: https://issuetracker.google.com)

I think my vote at the moment, however, is to hold off on making this more complex, but I'll check with some of the other provider developers and see what other people are doing in this kind of situation. I'd stay away from _file designations, because that requires the key to be on disk at some point, but perhaps we could get away with storing a hash and nothing else, though at the expense of more complicated code.

For what it's worth, however, I think putting in the effort to get onto a secure remote state backend will pay off more quickly and more universally than updating every resource containing potentially sensitive information.

@ofpiyush
Copy link
Contributor Author

ofpiyush commented Mar 1, 2018

They already know of it. Documentation

I don't see a way to say "Give viewer minus storage object viewer access"

@paddycarver paddycarver added this to the 2.0.0 milestone Mar 8, 2018
@morgante
Copy link

morgante commented May 2, 2018

@paddycarver
Copy link
Contributor

The Terraform team's stance on this is basically that state is a sensitive artifact and should be handled as such. I know, historically, that storing state in a git repository has been a recommended practice, but I think in recent years we've moved away from that as we came to better understand security implications.

I'm open to better documenting best practices for working with GCS state to avoid security mistakes, but I think it's a tough sell to convince me on pgp_key-esque functionality unless it's part of the API in question. For example, if GCP offered a KMS input for SSL certificates, where we could specify a secret and the SSL cert would be pulled from it, I'd be in favor of implementing that. But building our own encryption into each field feels like a losing battle and like the wrong direction to head in, to me. More information on this can be found in the docs and some history of the discussion can be seen in hashicorp/terraform#516 and hashicorp/terraform#9556.

@morgante
Copy link

morgante commented Jul 6, 2018

@paddycarver I agree that state is generally sensitive, but in this case I think there's value in having an additional layer of protection. In particular, I often see developers having access to read the state bucket so they can develop Terraform config but that doesn't mean they should have access to read the cert contents after it has been uploaded.

Importantly, the underlying API is specifically designed to not allow reading back the SSL cert key contents. I feel like Terraform should try to be as secure as the underlying API and not require that either. We already have pgp_key functionality on other resources, so would like to have some sort of equivalent here (or store the hash).

@paddycarver
Copy link
Contributor

Importantly, the underlying API is specifically designed to not allow reading back the SSL cert key contents. I feel like Terraform should try to be as secure as the underlying API and not require that either.

I think that's fair.

We already have pgp_key functionality on other resources, so would like to have some sort of equivalent here (or store the hash).

I'd be game for only storing the hash if we can determine it doesn't break existing use cases. What I think it's harder to sell me on is functionality like pgp_key, which is more complicated, fragile, may not achieve the desired result, and ultimately requires giving the developer access to the sensitive information anyways, albeit with more of an annoying process to get it. (They still need to have the key available to them, IIUC, or Terraform will claim there's a change. And if Terraform has the key to decrypt the information, the user has the information, and can get to the key.)

@morgante
Copy link

morgante commented Jul 10, 2018

I'd be game for only storing the hash if we can determine it doesn't break existing use cases.

Agreed, I think storing the hash is the best solution.

@paddycarver
Copy link
Contributor

My largest remaining concern is that the Terraform paradigm is that you declare the state of the world, and Terraform goes out and makes it so. Imperative things like "someone privileged uploads the key and then unprivileged users can work with the config" don't really match with Terraform, because what happens if a user deletes the certificate in the console? What should Terraform do when a user that doesn't have access to the private key tries to run terraform apply?

I think if organizations are at the point where they're trying to control access to secrets that Terraform relies upon, they probably want something more like a CI server or TFE running applies with access to the sensitive information.

Which isn't to say we can't store the minimum viable information in state, but I think even if we do that, I still wouldn't recommend that people try to include a secret in a Terraform config that a user operating Terraform shouldn't have access to. I don't think that'll ever end well, and automation is probably needed then.

@paddycarver
Copy link
Contributor

paddycarver commented Feb 1, 2019

This should be resolved by GoogleCloudPlatform/magic-modules#1336.

@ghost
Copy link

ghost commented Mar 4, 2019

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

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

No branches or pull requests

5 participants