S3 backend should have configurable bucket versioning

[paul-pop]

Hi there 👋

For the vault-cluster module, an optional property enable_s3_bucket_versioning could be passed in to be able to configure versioning of the objects inside the bucket.

AWS Trusted Advisor suggests all S3 buckets should have versioning enabled.

Thanks,
Paul

[brikis98]

Is it really a good idea to keep multiple versions of all of your Vault secrets? I understand they are encrypted, but creating lots of copies of secret files still makes me feel a bit uneasy.

[patoarvizu]

One advantage I can see to enabling versioning is that you can enforce MFA-only delete to further protect from accidental deletions.

Just my $0.02

[brikis98]

I think Terraform/S3 give you deletion protection by default. Unless you set force_destroy to true, running terraform destroy won't destroy an S3 bucket that still has files in it.

I suppose MFA-only delete would prevent deletion of the files within it too?

[patoarvizu]

Yeah, I meant accidental deletions outside of Terraform. So, someone running awscli with the wrong profile for example.

Now that I'm thinking about it, doing this may make a Vault deployment not completely destroyable, since the S3 bucket can't be deleted because I don't think the aws_s3_bucket or aws_s3_bucket_object Terraform resources support any sort of MFA input, so if you ever want to completely delete a Vault cluster created with this module there might be some manual steps. But maybe that's actually a feature? ;)

That said, it might have the unwanted side effect of making some automated integration testing impractical.

[brikis98]

That said, it might have the unwanted side effect of making some automated integration testing impractical.

That's a pretty major downside :\

[patoarvizu]

To clarify: bucket versioning doesn't necessarily imply MFA-only delete, but I was pointing it out as something that can be a double edge sword when it comes to a Terraformized Vault cluster. And actually I was thinking last night that it might not even be that easy to enable MFA-only delete via Terraform, since it has to be done with root account credentials and an existing MFA device, which might make things not Terraform-friendly. So let's pretend I never suggested that :)

Adding an option to enable bucket versioning might be useful in some cases (unless there's something in the Vault implementation that would make it impossible to roll back to a previous version).

[brikis98]

Well, we can probably make versioning a configurable parameter defaulting to false. I'd welcome a PR for that.

[icereed]

Hey guys, opened a PR here: #135