core: Adding Sensitive attribute to resource schema

[vancluever]

Reference: #516

This is something that I've been mulling for a while but whipped up because we have a use case for it at PBP.

While state can be encrypted on storage, and things can be scripted easily enough to ensure that cached state is dropped after a run, it's a little tougher to hide values from logs when there isn't support for it in core.

This adds the Sensitive attribute to the schema, which allows resource and data source maintainers the ability to mark some fields as sensitive so that their output does not get displayed in the diffs and logs that are output to the user. Much like the sensitive attribute on an output, this currently is a very superficial setting, and only affects output.

Also, in the future, we could possibly look at leveraging this to start encrypting values in the state, say via something like a pre-shared key supplied via input or an environment variable.

I figured that this was a pretty non-invasive way of working towards better storage of sensitive data, in addition to extending the output obfuscation that's already available in outputs to TF as a whole.

Let me know what you guys think!

PS: As an example, I've updated the aws_db_instance resource to mark the password field as sensitive.

[jen20]

Hi @vancluever! Thanks for opening this and doing the work! We should probably discuss this with @phinze and or @mitchellh before merging - it seems reasonable to me but we should check how it fits into a future roadmap where state might be encrypted.

[vancluever]

@jen20 yeah fair enough - the main thing here for me is the stop-gap to make sure that passwords aren't in our logs :)

This is one of a couple of ideas I had around this - the other was crypto interpolation functions, but I'd imagine that one is massively more complicated when it comes to reading values back from state. To be honest I'd almost think that approach would be better, as it puts power in the hands of the user, however the decryption part notwithstanding, on the other hand this opinionated approach creates a bit of a "best practice" that people can follow, ie: this field (ie: maybe user data) is not sensitive, you should probably find a different place to put credentials (or maybe if that is legit, then we make that sensitive too).

This starts a bit of a conversation at least!

[phinze]

Nice work on this @vancluever!

I agree this makes sense as a good stop-gap approach. We're going to dive into a more holistic solution eventually, but this change does not seem like it will complicate any future work.

It's good to mention that this particular implementation is provider-author-managed rather than user-managed for now. But I think that's okay - it catches the low hanging fruit of getting obviously sensitive values out of UI output, and leaves our options open for the next round of improvements.

[vancluever]

Awesome! Thanks @phinze!

[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.