Fix aws_db_instance to not recreate each time

[bitglue]

Several of the arguments were optional, and if omitted, they are
calculated. Mark them as such in the schema to avoid triggering an
update.

Hash the password for the state file, rather than omitting it entirely,
again to avoid an update each time.

Make the name parameter optional. It's not required in any engine, and
in some (MS SQL Server) it's not allowed at all.

Drop the skip_final_snapshot argument. If final_snapshot_identifier
isn't specified, then don't make a final snapshot. As things were, it
was possible to create a resource with neither of these arguments
specified which would later fail when it was to be deleted since the RDS
API requires exactly one of the two.

Resolves issue #689.

[bitglue]

I took the liberty of making a few backwards incompatible changes since from the previous broken state it does not seem like many people are using this resource provider.

[bitglue]

I changed my mind on hashing the password. Hashing the password means interpolating on it yields a hash, not a password, which is surprising and not terribly useful. The reasoning from #309 applies.

[phinze]

This looks great, @bitglue! Let me pull it down quickly and verify.

I changed my mind on hashing the password. Hashing the password means interpolating on it yields a hash, not a password, which is surprising and not terribly useful.

Good point. Sensitive data handling will either need to be supported first class, or, as you say in #516, will need to be explicitly defined as a problem external to terraform. Either way we'll track that over in #516.

[phinze]

It looks like the acceptance tests still reference skip_final_snapshot...

https://github.com/bitglue/terraform/blob/b087a8f475b57b7132e5d82f4df4937f3f1afd7a/builtin/providers/aws/resource_aws_db_instance_test.go

Should be as easy as removing it from the config and from the check. Once that's done this LGTM!

[bitglue]

@phinze done.

[phinze]

@bitglue here too: https://github.com/bitglue/terraform/blob/fix_rds/builtin/providers/aws/resource_aws_db_instance_test.go#L160

[bitglue]

Done for serious now, @phinze. That will teach me to use grep...

[phinze]

Nice! LGTM

[omeid]

@bitglue Hashing the password means interpolating on it yields a hash, not a password..

Could you tell me why would anyone want to interpolate a password?

[bitglue]

Why would anyone want to interpolate a hash?

[omeid]

I don't think there is any, nor I can think of one for password values.
Absent use-cases for interpolating the password value, I think hashing of password seems like a practical precaution. The "no plain text password should hit the disk" is a fairly common policy and hashing the password would help with that too.

[bitglue]

OK, so don't save the state in plain-text: use an encrypted disk, or one of the several state plugins that support encryption. I use the password for connecting to the database. This PR is three years old. Please see #516 if you have something new to add.

[omeid]

Right, I ended up here from that PR, I am considering to create a pull request to hash passwords, I was just trying to understand your rationale against it.

[bitglue]

If Terraform hashes the password, how will I use it to provision a key in a secret store with the password, or interpolate it into cloud-init, or use it as an output for a human operator, the terraform_remote_state data source, or other automated tools?

If the password is generated with the random provider, will that also be hashed?

If the password is provided by a variable, how will that input be secured?

Hashing the password is avoiding rather than solving the issue. Hashing the password is functionally about as good as removing it and thus significantly limits automation possibilities, while being less secure than actually removing it.

Furthermore in most cases hashing doesn't solve the issue since the password may still exist in other places within the state, or in terraform.tfvars. If the password in terraform.tfstate is unacceptable, then its existence in terraform.tfvars must be equally so. The only scenario which doesn't involve securing a sensitive file is one where Terraform is always run interactively and the operator types the password every time, and there's no one around to read it off the screen as it's typed.

One could envision a tool which reads the password from some preexisting secure store and generates a FIFO which Terraform reads as the variables file containing the password. The usability burden of implementing this tool aside, its existence implies a system acceptable for password storage. Why not put the state in that by mounting a secure filesystem or through a remote state plugin?

Doing so secures not just the password, but also a exhaustive specification of your system architecture. Allowing an attacker access to the state file is tantamount to giving read-only access to the AWS (or other) account: exactly the information an attacker would need to find a small exploit and escalate it to a full compromise with a minimum of effort, time, and risk of detection.

Further consider that even if the password is hashed, it's still subject to brute-force attack. A previous implementation used an unsalted SHA1 hash. SHA1 isn't expensive to calculate so a brute-force attack is quite effective. The lack of salting makes it subject to rainbow table attacks.

The state file aside, Terraform does not provide the internal security to properly protect the password. I doubt it marks the memory containing it as unswappable, nor scrub the RAM before freeing it, for example. Short of an extensive security audit of Terraform and additional implementation complexity, Terraform must be removed from handling of passwords entirely. Automated, periodic password rotation achieves this, and is best practice anyhow. Bootstrapping such a process is made significantly easier when Terraform has the initial password. The rotation tool can either scan the state for databases where the initial password is still active, or the process can be provisioned as a Terraform resource.

[omeid]

If Terraform hashes the password, how will I use it to provision a key in a secret store with the password, or interpolate it into cloud-init, or use it as an output for a human operator, the terraform_remote_state data source, or other automated tools?

If the password is generated with the random provider, will that also be hashed?

The passwords could only be hashed when stored in the state.

Hashing the password is avoiding rather than solving the issue.
Yes, avoiding a problem is the ideal solution, IMO.

Hashing the password is functionally about as good as removing it and thus significantly limits automation possibilities, while being less secure than actually removing it.

Again, if password is only hashed when it hits the state, I don't believe it impacts any of those scenarios.

Furthermore in most cases hashing doesn't solve the issue since the password may still exist in other places within the state, or in terraform.tfvars. If the password in terraform.tfstate is unacceptable, then its existence in terraform.tfvars must be equally so.

This problem renders hashing passwords in state a fragile and risky proposition. The complexity of tackling that problem makes alternatives (using a secret store, for example) more approachable and practical.

Thanks for your detailed reply.

[bitglue]

Seems I've already talked you out of it, but for posterity I want to clarify a few points for posterity since this discussion comes up regularly.

By "password is only hashed when it hits the state" I think you mean the password would remain available until the end of the run, when the state is written out. But that means subsequent runs won't have the same information the first one had, breaking a fundamental property of Terraform. It should be the case that given a configuration, running an apply results in the same final state whether this is the first apply or a subsequent one.

(In fact I'm vaguely remembering an early implementation worked this way, and as a consequence if you interpolated the password anywhere it would work fine the first time, and then running an apply a second time, having not changed the configuration, would generate a plan changing all the places that contained the password into containing a hash of the password.)

The other problem I was alluding to here is the password may show up in places other than the password attribute of the database instance. If the random provider generates it for example, that output is stored in the state, but currently there's no way Terraform would know that random string is a password and should be hashed. And if the password is interpolated into other things (ec2 userdata, perhaps) those too become tainted with the password and would require similar protections. Correctly generating and tracking these taints seems error-prone enough to be ill-advised as a security cornerstone.

[omeid]

By "password is only hashed when it hits the state" I think you mean the password would remain available until the end of the run, when the state is written out. But that means subsequent runs won't have the same information the first one had, breaking a fundamental property of Terraform. It should be the case that given a configuration, running an apply results in the same final state whether this is the first apply or a subsequent one.

The password in the configuration would remain the same, I don't believe it is impossible to make terraform aware of this in a way that planning and apply work seamlessly.

The other problem I was alluding to here is the password may show up in places other than the password attribute of the database instance.

Yes, this is the fundamental complexity around this, though I am not convinced of other reasoning, for example that some hashing algos are subject to brute-force attacks.

but currently there's no way Terraform would know that random string is a password and should be hashed.

This could be a configuration on different attributes, just like Optional, Computed, and ForceNew, in fact, there is already the Sensitive flag that hides a given attribute's value in outputs for example, it could be used to hash the password before it hits the state as well.

Just to clarify my position, I don't believe it is impossible to make any particular attribute hashed while keeping all other functionalities.

I am only concerned of not being able to enforce this hashing in all the other places (which you provided some good examples) that said attribute may be interpolated and so creating an illusion of secrecy, which I am of the opinion that —llusion of secrecy— is worst than no secrecy at all.

[ghost]

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.