fix: [S3] permissions when specifying custom kms (#1202)

* fix: [S3] permissions when specifying custom kms

[#186276329](https://www.pivotaltracker.com/story/show/186276329)

This commit implements the fix for the bug when enabling sse with
a custom kms_key.

We may want to investigate the consequences of the following
scenarios not yet covered by any of the existing tests:
- enabling/disabling sse after provisioning
- creating multiple bindings
- changing kms_key after provisioning

* fix: [S3] allow safely updating default kms key

[#186276329](https://www.pivotaltracker.com/story/show/186276329)

This change allows customers to specify sse_extra_kms_key_ids
as a comma-separated list of keys to be used for decryption.
This is specially useful when rotating sse_default_kms_key_id
since previous objects are not automatically reencrypted with
the new key.

When that happens, specifying any previous default_kms_key_id
as part of sse_extra_kms_key_ids allows the binding to decryp
any existing objects created with these extra keys, otherwise
any read operation for such objects would fail.

Extra keys will still be used for decryption if you decide to
disable server-side-encryption but some of the objects in the
bucket were created while encryption was enabled.

* fix: [S3] corner-case when sse_all_kms_key_ids:""

[#186276329](https://www.pivotaltracker.com/story/show/186276329)

Fix scenario when bind input includes sse_all_kms_key_ids: ""
Currently, when that happens key_ids_list=[""] instead of []
therefore, count=length(key_ids_list) is 1 instead of 0 which
is what we want.

Compacting key_ids_list is an easy and mostly future proof fix.

---------

Co-authored-by: Andrea Zucchini <zandrea@vmware.com>

aws-s3-bucket.yml

@@ -98,6 +98,11 @@ provision:
     nullable: true
     details: The AWS KMS key ID used for the SSE-KMS encryption. This can only be used when you set the value of `sse_default_algorithm` as `aws:kms`.
     default: null
+  - field_name: sse_extra_kms_key_ids
+    type: string
+    nullable: true
+    details: A comma-separated list of AWS KMS key IDs used for the SSE-KMS decryption. This can only be used when you set the value of `sse_default_algorithm` as `aws:kms`.
+    default: null
   - field_name: sse_default_algorithm
     type: string
     nullable: true
@@ -179,6 +184,9 @@ provision:
   - field_name: bucket_name
     type: string
     details: Name of created bucket
+  - field_name: sse_all_kms_key_ids
+    type: string
+    details: The default and extra AWS KMS key IDs used for SSE-KMS encryption and decryption.
 bind:
   plan_inputs: []
   user_inputs:
@@ -203,6 +211,10 @@ bind:
     default: csb-${request.binding_id}
     overwrite: true
     type: string
+  - name: sse_all_kms_key_ids
+    default: ${instance.details["sse_all_kms_key_ids"]}
+    overwrite: true
+    type: string
   template_refs:
     data: terraform/s3/bind/data.tf
     main: terraform/s3/bind/main.tf

integration-tests/s3_test.go

@@ -306,6 +306,11 @@ var _ = Describe("S3", Label("s3"), func() {
 					Type:  "string",
 					Value: "arn:aws:s3:::examplebucket/developers/design_info.doc",
 				},
+				{
+					Name:  "sse_all_kms_key_ids",
+					Type:  "string",
+					Value: "some-default-kms-key-id,some-extra-kms-key-id",
+				},
 			})
 			Expect(err).NotTo(HaveOccurred())
 
@@ -327,6 +332,8 @@ var _ = Describe("S3", Label("s3"), func() {
 					"secret_access_key": "subsequent.secret.access.key.test",
 					"region":            "ap-northeast-3",
 					"arn":               "arn:aws:s3:::examplebucket/developers/design_info.doc",
+
+					"sse_all_kms_key_ids": "some-default-kms-key-id,some-extra-kms-key-id",
 				}),
 			)
 		})

terraform-tests/s3_test.go

@@ -31,6 +31,7 @@ var _ = Describe("S3", Label("S3-terraform"), Ordered, func() {
 		"pab_ignore_public_acls":                     false,
 		"pab_restrict_public_buckets":                false,
 		"sse_default_kms_key_id":                     nil,
+		"sse_extra_kms_key_ids":                      nil,
 		"sse_default_algorithm":                      nil,
 		"sse_bucket_key_enabled":                     false,
 		"ol_enabled":                                 false,

terraform/s3/bind/data.tf

@@ -12,6 +12,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+locals {
+  user_policy_with_or_without_encryption = try(data.aws_iam_policy_document.user_policy_sse[0], data.aws_iam_policy_document.user_policy)
+
+  key_ids_list = try(compact(split(",", var.sse_all_kms_key_ids)), [])
+}
+
+
 data "aws_iam_policy_document" "user_policy" {
   statement {
     sid = "bucketAccess"
@@ -68,4 +75,25 @@ data "aws_iam_policy_document" "user_policy" {
       format("%s/*", var.arn)
     ]
   }
-}
+}
+
+data "aws_kms_key" "customer_provided_keys" {
+  count  = length(local.key_ids_list) == 0 ? 0 : length(local.key_ids_list)
+  key_id = local.key_ids_list[count.index]
+}
+
+data "aws_iam_policy_document" "user_policy_sse" {
+  count = length(local.key_ids_list) == 0 ? 0 : 1
+
+  source_policy_documents = [data.aws_iam_policy_document.user_policy.json]
+
+  statement {
+    sid = "kmsperms"
+    actions = [
+      "kms:Decrypt",
+      "kms:Encrypt",
+      "kms:GenerateDataKey",
+    ]
+    resources = [for key in data.aws_kms_key.customer_provided_keys : key.arn]
+  }
+}

terraform/s3/bind/main.tf

@@ -26,5 +26,5 @@ resource "aws_iam_user_policy" "user_policy" {
 
   user = aws_iam_user.user.name
 
-  policy = data.aws_iam_policy_document.user_policy.json
-}
+  policy = local.user_policy_with_or_without_encryption.json
+}

terraform/s3/bind/variables.tf

@@ -13,4 +13,5 @@
 # limitations under the License.
 
 variable "arn" { type = string }
-variable "user_name" { type = string }
+variable "user_name" { type = string }
+variable "sse_all_kms_key_ids" { type = string }

terraform/s3/provision/data.tf

@@ -7,4 +7,8 @@ locals {
   # When creating a bucket with Object Lock enabled, Amazon S3 automatically enables versioning for the bucket.
   # To avoid differences between the local state and the AWS state, we will enable versioning when enabling Object Lock.
   is_versioning_enabled = var.enable_versioning ? true : var.ol_enabled
-}
+
+  default_kms_key_as_list = try([coalesce(var.sse_default_kms_key_id)], [])
+  extra_kms_keys_as_list  = try(split(",", var.sse_extra_kms_key_ids), [])
+  sse_all_kms_key_ids     = join(",", compact(distinct(concat(local.default_kms_key_as_list, local.extra_kms_keys_as_list))))
+}

terraform/s3/provision/outputs.tf

@@ -15,4 +15,5 @@
 output "arn" { value = aws_s3_bucket.b.arn }
 output "bucket_domain_name" { value = aws_s3_bucket.b.bucket_domain_name }
 output "region" { value = aws_s3_bucket.b.region }
-output "bucket_name" { value = aws_s3_bucket.b.bucket }
+output "bucket_name" { value = aws_s3_bucket.b.bucket }
+output "sse_all_kms_key_ids" { value = local.sse_all_kms_key_ids }

terraform/s3/provision/variables.tf

@@ -27,6 +27,7 @@ variable "pab_restrict_public_buckets" { type = bool }
 
 # Resource aws_s3_bucket_server_side_encryption_configuration
 variable "sse_default_kms_key_id" { type = string }
+variable "sse_extra_kms_key_ids" { type = string }
 variable "sse_default_algorithm" { type = string }
 variable "sse_bucket_key_enabled" { type = bool }