From 9d84adc2f7427c6375e0aed057395e75c9af7a68 Mon Sep 17 00:00:00 2001 From: toddgiguere <83610458+toddgiguere@users.noreply.github.com> Date: Fri, 7 Feb 2025 11:49:33 -0500 Subject: [PATCH] feat: The s2s auth policies are all now scope to individual keys/buckets (#943) BREAKING CHANGE: If upgrading from a previous release, you will see the s2s auth policies being destroyed and recreated. Please be aware that the new policies will be finer scoped to individual KMS key or COS buckets so if you had any other infrastructure that relied on the policies created by this solution, you should check to see if they are impacted before upgrading --- README.md | 2 + atracker.tf | 2 +- dynamic_values.tf | 1 + .../service_authorizations.tf | 187 +++++++++++++++--- dynamic_values/service_authorizations.tf | 3 + dynamic_values/variables.tf | 4 + outputs.tf | 12 +- .../dynamic_values/cloud_object_storage.tf | 2 - patterns/vsi-extension/main.tf | 5 +- patterns/vsi-extension/variables.tf | 6 + service_authorizations.tf | 149 +++++++++++++- 11 files changed, 331 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 1d1b31b79..b8ff6ed41 100644 --- a/README.md +++ b/README.md @@ -874,6 +874,7 @@ module "cluster_pattern" { | [ibm_container_vpc_cluster.cluster](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/resources/container_vpc_cluster) | resource | | [ibm_container_vpc_worker_pool.pool](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/resources/container_vpc_worker_pool) | resource | | [ibm_cos_bucket.buckets](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/resources/cos_bucket) | resource | +| [ibm_iam_authorization_policy.cos_bucket_policy](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/resources/iam_authorization_policy) | resource | | [ibm_iam_authorization_policy.policy](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/resources/iam_authorization_policy) | resource | | [ibm_is_placement_group.placement_group](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/resources/is_placement_group) | resource | | [ibm_is_security_group.security_group](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/resources/is_security_group) | resource | @@ -895,6 +896,7 @@ module "cluster_pattern" { | [random_string.random_cos_suffix](https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/string) | resource | | [time_sleep.wait_30_seconds](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/sleep) | resource | | [time_sleep.wait_for_authorization_policy](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/sleep) | resource | +| [time_sleep.wait_for_authorization_policy_buckets](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/sleep) | resource | | [time_sleep.wait_for_vpc_creation_data](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/sleep) | resource | | [ibm_container_addons.existing_addons](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/data-sources/container_addons) | data source | | [ibm_container_cluster_versions.cluster_versions](https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/data-sources/container_cluster_versions) | data source | diff --git a/atracker.tf b/atracker.tf index 35b8d76fd..b8e69a7b0 100644 --- a/atracker.tf +++ b/atracker.tf @@ -31,7 +31,7 @@ resource "ibm_atracker_target" "atracker_target" { target_type = "cloud_object_storage" # Wait for buckets and auth policies to ensure successful provision - depends_on = [ibm_cos_bucket.buckets, ibm_iam_authorization_policy.policy] + depends_on = [ibm_cos_bucket.buckets, ibm_iam_authorization_policy.policy, ibm_iam_authorization_policy.cos_bucket_policy] } resource "ibm_atracker_route" "atracker_route" { diff --git a/dynamic_values.tf b/dynamic_values.tf index 068fb4b58..cc2014733 100644 --- a/dynamic_values.tf +++ b/dynamic_values.tf @@ -8,6 +8,7 @@ module "dynamic_values" { prefix = var.prefix key_management = var.key_management key_management_guid = module.key_management.key_management_guid + key_management_key_map = module.key_management.key_map clusters = var.clusters vpcs = var.vpcs resource_groups = local.resource_groups diff --git a/dynamic_values/config_modules/service_authorizations/service_authorizations.tf b/dynamic_values/config_modules/service_authorizations/service_authorizations.tf index 84cbf6561..2a26ba3b5 100644 --- a/dynamic_values/config_modules/service_authorizations/service_authorizations.tf +++ b/dynamic_values/config_modules/service_authorizations/service_authorizations.tf @@ -10,6 +10,10 @@ variable "key_management_guid" { description = "Key management guid" } +variable "key_management_key_map" { + description = "Key management key IDs" +} + variable "cos_instance_ids" { description = "Map of COS instance IDs" } @@ -38,7 +42,43 @@ variable "clusters" { description = "Add cluster to kms auth policies" } +variable "vsi" { + description = "Add vsi block storage to auth policies" +} + +variable "vpcs" { + description = "Direct reference to vpcs variable" +} + +############################################################################## + +############################################################################## +# BUCKET MAP +# Create a flattened out map of all configured cos buckets. +# This map will have the bucket name as the key, and have attributes +# from both the bucket and the parent instance that we need in further queries. +# This map will be used to perform lookups based on bucket name to get data related to either +# the bucket itself or its parent instance. ############################################################################## +module "cos_bucket_map" { + source = "../list_to_map" + list = flatten([ + for instance in var.cos : + [ + for bucket in instance.buckets : + [ + { + name = bucket.name + instance_name = instance.name + bucket_key_name = lookup(bucket, "kms_key", null) + skip_kms_s2s_auth_policy = lookup(instance, "skip_kms_s2s_auth_policy", false) + skip_flowlogs_s2s_auth_policy = lookup(instance, "skip_flowlogs_s2s_auth_policy", false) + skip_atracker_s2s_auth_policy = lookup(instance, "skip_atracker_s2s_auth_policy", false) + } + ] + ] + ]) +} ############################################################################## # Locals @@ -46,20 +86,79 @@ variable "clusters" { locals { target_key_management_service = lookup(var.key_management, "name", null) != null ? lookup(var.key_management, "use_hs_crypto", false) == true ? "hs-crypto" : "kms" : null + + # create a list of keys used for all buckets, since we are going to scope the auth policy to keys. + # doing this in a local first becase it needs a distinct to get rid of duplicates from same keys used + # on multiple buckets, and a distinct on the final map may error in terraform for_each before first apply. + cos_bucket_key_list_distinct = distinct( + flatten([ + for bucket in module.cos_bucket_map.value : + [ + { + instance_name = bucket.instance_name + bucket_key_name = lookup(bucket, "bucket_key_name", null) + } + ] if !bucket.skip_kms_s2s_auth_policy + ]) + ) + + # get all keys that will be used for VSI block storage + # this is combination of all boot volume keys, plus the extra storage volume keys + block_storage_key_list_distinct = distinct( + flatten([ + [ + for vsi in var.vsi : + [ + { block_key_name = lookup(vsi, "boot_volume_encryption_key_name", null) } + ] + ], + [ + for vsi in var.vsi : + [ + for block in coalesce(lookup(vsi, "block_storage_volumes", null), []) : + [ + { block_key_name = lookup(block, "encryption_key", null) } + ] + ] + ] + ]) + ) + + # get all keys used in clusters + # is combination of boot keys and config keys + kube_key_list_distinct = distinct( + flatten([ + [ + for cluster in var.clusters : + [ + { cluster_key_name = lookup(cluster, "boot_volume_crk_name", null) } + ] + ], + [ + for cluster in var.clusters : + [ + { cluster_key_name = lookup(cluster.kms_config, "crk_name", null) } + ] if lookup(cluster, "kms_config", null) != null + ] + ]) + ) } module "kms_to_block_storage" { source = "../list_to_map" list = [ - for instance in(var.skip_kms_block_storage_s2s_auth_policy ? [] : ["block-storage"]) : + for instance in local.block_storage_key_list_distinct : { - name = instance + name = "block-storage-to-${instance.block_key_name}" source_service_name = "server-protect" - description = "Allow block storage volumes to be encrypted by KMS instance" + description = "Allow block storage volumes to be encrypted by KMS key" roles = ["Reader"] target_service_name = local.target_key_management_service target_resource_instance_id = var.key_management_guid - } if local.target_key_management_service != null + target_resource_type = "key" + target_resource_id = split(":", var.key_management_key_map[instance.block_key_name].crn)[9] + target_resource_account_id = trimprefix(split(":", var.key_management_key_map[instance.block_key_name].crn)[6], "a/") + } if local.target_key_management_service != null && !var.skip_kms_block_storage_s2s_auth_policy && instance.block_key_name != null ] } @@ -69,15 +168,18 @@ module "kms_to_block_storage" { module "kube_to_kms" { source = "../list_to_map" list = [ - for instance in(length(var.clusters) > 0 ? ["containers-kubernetes"] : []) : + for instance in local.kube_key_list_distinct : { - name = instance + name = "kube-to-${instance.cluster_key_name}" source_service_name = "containers-kubernetes" description = "Allow cluster to be encrypted by KMS instance" roles = ["Reader"] target_service_name = local.target_key_management_service target_resource_instance_id = var.key_management_guid - } if local.target_key_management_service != null && !var.skip_kms_kube_s2s_auth_policy + target_resource_type = "key" + target_resource_id = split(":", var.key_management_key_map[instance.cluster_key_name].crn)[9] + target_resource_account_id = trimprefix(split(":", var.key_management_key_map[instance.cluster_key_name].crn)[6], "a/") + } if local.target_key_management_service != null && !var.skip_kms_kube_s2s_auth_policy && instance.cluster_key_name != null ] } @@ -90,32 +192,58 @@ module "kube_to_kms" { module "cos_to_key_management" { source = "../list_to_map" list = [ - for instance in var.cos : + for bucket_key in local.cos_bucket_key_list_distinct : { - name = "cos-${instance.name}-to-key-management" + name = "cos-${bucket_key.instance_name}-to-key-${bucket_key.bucket_key_name}" source_service_name = "cloud-object-storage" - source_resource_instance_id = split(":", var.cos_instance_ids[instance.name])[7] - description = "Allow COS instance to read from KMS instance" + source_resource_instance_id = split(":", var.cos_instance_ids[bucket_key.instance_name])[7] + description = "Allow COS instance to read KMS key" roles = ["Reader"] target_service_name = local.target_key_management_service target_resource_instance_id = var.key_management_guid - } if local.target_key_management_service != null && !instance.skip_kms_s2s_auth_policy + target_resource_type = "key" + target_resource_id = split(":", var.key_management_key_map[bucket_key.bucket_key_name].crn)[9] + target_resource_account_id = trimprefix(split(":", var.key_management_key_map[bucket_key.bucket_key_name].crn)[6], "a/") + } if local.target_key_management_service != null && bucket_key.bucket_key_name != null ] } +############################################################################## +# VPC Flow Logs to COS bucket +############################################################################## +locals { + flow_log_bucket_list_distinct = distinct( + flatten([ + for vpc in var.vpcs : + [ + { + bucket_name = vpc.flow_logs_bucket_name + } + ] if lookup(vpc, "flow_logs_bucket_name", null) != null + ]) + ) +} + +# NOTE: +# Due to terraform plan cycle issues, we cannot reference the true bucket instance here in this module, +# so we will pass back the reference name of the bucket in `target_resource_id` and look up details +# when applying the auth policy module "flow_logs_to_cos" { source = "../list_to_map" list = [ - for instance in var.cos : + for instance in local.flow_log_bucket_list_distinct : { - name = "flow-logs-${instance.name}" + name = "flow-logs-${instance.bucket_name}" source_service_name = "is" source_resource_type = "flow-log-collector" description = "Allow flow logs write access cloud object storage instance" roles = ["Writer"] target_service_name = "cloud-object-storage" - target_resource_instance_id = split(":", var.cos_instance_ids[instance.name])[7] - } if !instance.skip_flowlogs_s2s_auth_policy + target_resource_instance_id = null + target_resource_type = "bucket" + target_resource_id = instance.bucket_name + target_resource_account_id = null + } if !module.cos_bucket_map.value[instance.bucket_name].skip_flowlogs_s2s_auth_policy ] } @@ -125,30 +253,29 @@ module "flow_logs_to_cos" { # Atracker to COS ############################################################################## -locals { - atracker_cos_instance = var.atracker_cos_bucket == null ? null : one(flatten([ - for instance in var.cos : - [ - for bucket in instance.buckets : - [instance.name] if bucket.name == var.atracker_cos_bucket - ] if !instance.skip_atracker_s2s_auth_policy - ])) -} - +# NOTE: +# Due to terraform plan cycle issues, we cannot reference the true bucket instance here in this module, +# so we will pass back the reference name of the bucket in `target_resource_id` and look up details +# when applying the auth policy module "atracker_to_cos" { source = "../list_to_map" list = [ - for instance in(var.atracker_cos_bucket != null && local.atracker_cos_instance != null ? ["atracker-to-cos"] : []) : + for instance in ["atracker-to-cos"] : { name = instance source_service_name = "atracker" - description = "Allow atracker to write to COS" + description = "Allow atracker to write to COS bucket" roles = ["Object Writer"] target_service_name = "cloud-object-storage" - target_resource_instance_id = split(":", var.cos_instance_ids[local.atracker_cos_instance])[7] - } + target_resource_instance_id = null + target_resource_type = "bucket" + target_resource_id = var.atracker_cos_bucket + target_resource_account_id = null + } if var.atracker_cos_bucket != null && !try(module.cos_bucket_map.value[var.atracker_cos_bucket].skip_atracker_s2s_auth_policy, false) ] } +# DEV NOTE: needed a `try()` on the `cos_bucket_map` lookup above to take care of the case where atracker is turned off +# and a bucket name is NULL. This causes plan errors which the try should catch. ############################################################################## # Outputs diff --git a/dynamic_values/service_authorizations.tf b/dynamic_values/service_authorizations.tf index 0e1a3f428..1375fd177 100644 --- a/dynamic_values/service_authorizations.tf +++ b/dynamic_values/service_authorizations.tf @@ -6,6 +6,7 @@ module "service_authorizations" { source = "./config_modules/service_authorizations" key_management = var.key_management key_management_guid = var.key_management_guid + key_management_key_map = var.key_management_key_map cos = var.cos cos_instance_ids = local.cos_instance_ids skip_kms_block_storage_s2s_auth_policy = var.skip_kms_block_storage_s2s_auth_policy @@ -13,6 +14,8 @@ module "service_authorizations" { skip_kms_kube_s2s_auth_policy = var.skip_kms_kube_s2s_auth_policy atracker_cos_bucket = var.atracker_cos_bucket clusters = var.clusters + vsi = var.vsi + vpcs = var.vpcs } ############################################################################## diff --git a/dynamic_values/variables.tf b/dynamic_values/variables.tf index bf23c433c..89cc35a78 100644 --- a/dynamic_values/variables.tf +++ b/dynamic_values/variables.tf @@ -108,6 +108,10 @@ variable "key_management_guid" { description = "Key Management GUID" } +variable "key_management_key_map" { + description = "Key management key IDs" +} + ############################################################################## ############################################################################## diff --git a/outputs.tf b/outputs.tf index 56000753e..625b040a0 100644 --- a/outputs.tf +++ b/outputs.tf @@ -348,14 +348,20 @@ output "security_group_data" { output "service_authorization_names" { description = "List of service authorization names" - value = keys(ibm_iam_authorization_policy.policy) + value = concat(keys(ibm_iam_authorization_policy.policy), keys(ibm_iam_authorization_policy.cos_bucket_policy)) } output "service_authorization_data" { description = "List of service authorization data" value = flatten([ - for policy in ibm_iam_authorization_policy.policy : - policy + [ + for policy in ibm_iam_authorization_policy.policy : + policy + ], + [ + for bucket_policy in ibm_iam_authorization_policy.cos_bucket_policy : + bucket_policy + ] ]) } diff --git a/patterns/dynamic_values/cloud_object_storage.tf b/patterns/dynamic_values/cloud_object_storage.tf index 615daca87..852a8d64d 100644 --- a/patterns/dynamic_values/cloud_object_storage.tf +++ b/patterns/dynamic_values/cloud_object_storage.tf @@ -14,8 +14,6 @@ module "cloud_object_storage" { use_existing_cos_for_vpc_flowlogs = var.use_existing_cos_for_vpc_flowlogs endpoint_type = var.existing_cos_endpoint_type create_atracker_storage = var.add_atracker_route - # skip the kms s2s auth policy for existing cos instances: if existing kms name is null or empty - skip_kms_auth_for_existing_cos = coalesce(var.existing_kms_instance_name, "~EMPTY~") != "~EMPTY~" ? true : false } ############################################################################## diff --git a/patterns/vsi-extension/main.tf b/patterns/vsi-extension/main.tf index f30a24037..ac1f7869a 100644 --- a/patterns/vsi-extension/main.tf +++ b/patterns/vsi-extension/main.tf @@ -39,6 +39,8 @@ locals { for subnet in data.ibm_is_vpc.vpc_by_id.subnets : subnet if can(regex(local.default_subnet_name, subnet.name)) ] + + existing_kms_instance_guid = var.boot_volume_encryption_key == null ? null : split(":", var.boot_volume_encryption_key)[7] } module "vsi" { @@ -52,10 +54,11 @@ module "vsi" { tags = var.resource_tags access_tags = var.access_tags kms_encryption_enabled = true - skip_iam_authorization_policy = true + skip_iam_authorization_policy = var.skip_iam_authorization_policy user_data = var.user_data image_id = data.ibm_is_image.image.id boot_volume_encryption_key = var.boot_volume_encryption_key + existing_kms_instance_guid = local.existing_kms_instance_guid security_group_ids = var.security_group_ids ssh_key_ids = [local.ssh_key_id] machine_type = var.vsi_instance_profile diff --git a/patterns/vsi-extension/variables.tf b/patterns/vsi-extension/variables.tf index 15163c0f9..18f0cd730 100644 --- a/patterns/vsi-extension/variables.tf +++ b/patterns/vsi-extension/variables.tf @@ -111,6 +111,12 @@ variable "block_storage_volumes" { default = [] } +variable "skip_iam_authorization_policy" { + description = "Set to true to skip the creation of an IAM authorization policy that permits all Storage Blocks to read the encryption key from the KMS instance. If set to false, pass in a value for the KMS instance in the existing_kms_instance_guid variable. In addition, no policy is created if var.kms_encryption_enabled is set to false." + type = bool + default = false +} + variable "enable_floating_ip" { description = "Whether to create a floating IP for each virtual server." type = bool diff --git a/service_authorizations.tf b/service_authorizations.tf index 8f2307853..2c0bf14e3 100644 --- a/service_authorizations.tf +++ b/service_authorizations.tf @@ -5,7 +5,15 @@ ############################################################################## locals { - authorization_policies = module.dynamic_values.service_authorizations + authorization_policies_no_buckets = { + for auth_key, auth_value in module.dynamic_values.service_authorizations : auth_key => auth_value + if !var.skip_all_s2s_auth_policies && auth_value.target_resource_type != "bucket" + } + + authorization_policies_only_buckets = { + for auth_key, auth_value in module.dynamic_values.service_authorizations : auth_key => auth_value + if !var.skip_all_s2s_auth_policies && auth_value.target_resource_type == "bucket" + } } ############################################################################## @@ -13,19 +21,144 @@ locals { ############################################################################## # Authorization Policies +# DEV NOTE: +# The policy creations need to be split into multiple blocks due to dependencies, +# for instance key policies are needed to create buckets, so must be separated +# from each other to avoid plan cycle errors. ############################################################################## +# handle non-bucket policies resource "ibm_iam_authorization_policy" "policy" { - for_each = var.skip_all_s2s_auth_policies == true ? null : local.authorization_policies + for_each = local.authorization_policies_no_buckets source_service_name = each.value.source_service_name source_resource_type = lookup(each.value, "source_resource_type", null) source_resource_instance_id = lookup(each.value, "source_resource_instance_id", null) source_resource_group_id = lookup(each.value, "source_resource_group_id", null) - target_service_name = each.value.target_service_name - target_resource_instance_id = lookup(each.value, "target_resource_instance_id", null) - target_resource_group_id = lookup(each.value, "target_resource_group", null) roles = each.value.roles description = each.value.description + + resource_attributes { + name = "accountId" + operator = "stringEquals" + value = coalesce(each.value.target_resource_account_id, data.ibm_iam_account_settings.iam_account_settings.account_id) + } + + dynamic "resource_attributes" { + for_each = lookup(each.value, "target_service_name", null) != null ? [1] : [] + content { + name = "serviceName" + operator = "stringEquals" + value = each.value.target_service_name + } + } + + dynamic "resource_attributes" { + for_each = lookup(each.value, "target_resource_instance_id", null) != null ? [1] : [] + content { + name = "serviceInstance" + operator = "stringEquals" + value = each.value.target_resource_instance_id + } + } + + dynamic "resource_attributes" { + for_each = lookup(each.value, "target_resource_group", null) != null ? [1] : [] + content { + name = "resourceGroupId" + operator = "stringEquals" + value = each.value.target_resource_group + } + } + + dynamic "resource_attributes" { + for_each = lookup(each.value, "target_resource_type", null) != null ? [1] : [] + content { + name = "resourceType" + operator = "stringEquals" + value = each.value.target_resource_type + } + } + + dynamic "resource_attributes" { + for_each = lookup(each.value, "target_resource_id", null) != null ? [1] : [] + content { + name = "resource" + operator = "stringEquals" + value = each.value.target_resource_id + } + } + + lifecycle { + create_before_destroy = true + } +} + +# handle bucket policies +resource "ibm_iam_authorization_policy" "cos_bucket_policy" { + for_each = local.authorization_policies_only_buckets + source_service_name = each.value.source_service_name + source_resource_type = lookup(each.value, "source_resource_type", null) + source_resource_instance_id = lookup(each.value, "source_resource_instance_id", null) + source_resource_group_id = lookup(each.value, "source_resource_group_id", null) + roles = each.value.roles + description = each.value.description + + # NOTE: the `target_resource_id` from dynamic_values contains the key name of the bucket, + # which is then looked up for its CRN inside this resource block (this avoids plan cycle issues) + resource_attributes { + name = "accountId" + operator = "stringEquals" + value = trimprefix(split(":", ibm_cos_bucket.buckets[each.value.target_resource_id].crn)[6], "a/") + } + + dynamic "resource_attributes" { + for_each = lookup(each.value, "target_service_name", null) != null ? [1] : [] + content { + name = "serviceName" + operator = "stringEquals" + value = each.value.target_service_name + } + } + + dynamic "resource_attributes" { + for_each = lookup(each.value, "target_resource_instance_id", null) != null ? [1] : [] + content { + name = "serviceInstance" + operator = "stringEquals" + value = split(":", ibm_cos_bucket.buckets[each.value.target_resource_id].crn)[7] + } + } + + dynamic "resource_attributes" { + for_each = lookup(each.value, "target_resource_group", null) != null ? [1] : [] + content { + name = "resourceGroupId" + operator = "stringEquals" + value = each.value.target_resource_group + } + } + + dynamic "resource_attributes" { + for_each = lookup(each.value, "target_resource_type", null) != null ? [1] : [] + content { + name = "resourceType" + operator = "stringEquals" + value = each.value.target_resource_type + } + } + + dynamic "resource_attributes" { + for_each = lookup(each.value, "target_resource_id", null) != null ? [1] : [] + content { + name = "resource" + operator = "stringEquals" + value = split(":", ibm_cos_bucket.buckets[each.value.target_resource_id].crn)[9] + } + } + + lifecycle { + create_before_destroy = true + } } # workaround for https://github.com/IBM-Cloud/terraform-provider-ibm/issues/4478 @@ -35,4 +168,10 @@ resource "time_sleep" "wait_for_authorization_policy" { create_duration = "30s" } +resource "time_sleep" "wait_for_authorization_policy_buckets" { + depends_on = [ibm_iam_authorization_policy.cos_bucket_policy] + + create_duration = "30s" +} + ##############################################################################