fix: Explicitly assume with condition matching role arn (#283)

FernandoMiguel · antonbabenko · web-flow · commit 470b6ffa8e28 · 2022-10-13T22:16:52.000+02:00
Co-authored-by: Anton Babenko &lt;anton@antonbabenko.com&gt;
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -1,6 +1,6 @@
 repos:
   - repo: https://github.com/antonbabenko/pre-commit-terraform
-    rev: v1.75.0
+    rev: v1.76.0
     hooks:
       - id: terraform_fmt
       - id: terraform_validate
diff --git a/modules/iam-account/README.md b/modules/iam-account/README.md
@@ -19,7 +19,7 @@ module.iam_account.aws_iam_account_alias.this: Import complete!
 module.iam_account.aws_iam_account_alias.this: Refreshing state... (ID: this)
 
 Import successful!
-``` 
+```
 
 <!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
 ## Requirements
diff --git a/modules/iam-assumable-role-with-oidc/README.md b/modules/iam-assumable-role-with-oidc/README.md
@@ -4,7 +4,7 @@ Creates single IAM role which can be assumed by trusted resources using OpenID C
 
 [Creating IAM OIDC Identity Providers](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_oidc.html)
 
-This module supports IAM Roles for kubernetes service accounts as described in the [EKS documentation](https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html). 
+This module supports IAM Roles for kubernetes service accounts as described in the [EKS documentation](https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html).
 
 <!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
 ## Requirements
diff --git a/modules/iam-assumable-role-with-oidc/main.tf b/modules/iam-assumable-role-with-oidc/main.tf
@@ -6,6 +6,7 @@ locals {
     replace(url, "https://", "")
   ]
   number_of_role_policy_arns = coalesce(var.number_of_role_policy_arns, length(var.role_policy_arns))
+  role_name_condition        = var.role_name != null ? var.role_name : "${var.role_name_prefix}*"
 }
 
 data "aws_caller_identity" "current" {}
@@ -25,7 +26,13 @@ data "aws_iam_policy_document" "assume_role_with_oidc" {
 
       principals {
         type        = "AWS"
-        identifiers = ["arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:role${var.role_path}${var.role_name}"]
+        identifiers = ["*"]
+      }
+
+      condition {
+        test     = "ArnLike"
+        variable = "aws:PrincipalArn"
+        values   = ["arn:${local.partition}:iam::${local.account_id}:role${var.role_path}${local.role_name_condition}"]
       }
     }
   }
diff --git a/modules/iam-assumable-role-with-saml/README.md b/modules/iam-assumable-role-with-saml/README.md
@@ -2,8 +2,8 @@
 
 Creates single IAM role which can be assumed by trusted resources using SAML Federated Users.
 
-[Creating IAM SAML Identity Providers](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_saml.html) 
-[Enabling SAML 2.0 Federated Users to Access the AWS Management Console](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_enable-console-saml.html) 
+[Creating IAM SAML Identity Providers](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_saml.html)
+[Enabling SAML 2.0 Federated Users to Access the AWS Management Console](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_enable-console-saml.html)
 
 <!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
 ## Requirements
@@ -29,9 +29,7 @@ No modules.
 |------|------|
 | [aws_iam_role.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) | resource |
 | [aws_iam_role_policy_attachment.custom](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
-| [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source |
 | [aws_iam_policy_document.assume_role_with_saml](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
-| [aws_partition.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/partition) | data source |
 
 ## Inputs
 
diff --git a/modules/iam-assumable-role-with-saml/main.tf b/modules/iam-assumable-role-with-saml/main.tf
@@ -1,9 +1,7 @@
-data "aws_caller_identity" "current" {}
-data "aws_partition" "current" {}
-
 locals {
   identifiers                = compact(distinct(concat(var.provider_ids, [var.provider_id])))
   number_of_role_policy_arns = coalesce(var.number_of_role_policy_arns, length(var.role_policy_arns))
+  role_name_condition        = var.role_name != null ? var.role_name : "${var.role_name_prefix}*"
 }
 
 data "aws_iam_policy_document" "assume_role_with_saml" {
@@ -18,7 +16,13 @@ data "aws_iam_policy_document" "assume_role_with_saml" {
 
       principals {
         type        = "AWS"
-        identifiers = ["arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:role${var.role_path}${var.role_name}"]
+        identifiers = ["*"]
+      }
+
+      condition {
+        test     = "ArnLike"
+        variable = "aws:PrincipalArn"
+        values   = ["arn:${local.partition}:iam::${local.account_id}:role${var.role_path}${local.role_name_condition}"]
       }
     }
   }
diff --git a/modules/iam-assumable-role/README.md b/modules/iam-assumable-role/README.md
@@ -32,10 +32,8 @@ No modules.
 | [aws_iam_role_policy_attachment.custom](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
 | [aws_iam_role_policy_attachment.poweruser](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
 | [aws_iam_role_policy_attachment.readonly](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
-| [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source |
 | [aws_iam_policy_document.assume_role](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.assume_role_with_mfa](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
-| [aws_partition.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/partition) | data source |
 
 ## Inputs
 
@@ -58,6 +56,7 @@ No modules.
 | <a name="input_readonly_role_policy_arn"></a> [readonly\_role\_policy\_arn](#input\_readonly\_role\_policy\_arn) | Policy ARN to use for readonly role | `string` | `"arn:aws:iam::aws:policy/ReadOnlyAccess"` | no |
 | <a name="input_role_description"></a> [role\_description](#input\_role\_description) | IAM Role description | `string` | `""` | no |
 | <a name="input_role_name"></a> [role\_name](#input\_role\_name) | IAM role name | `string` | `""` | no |
+| <a name="input_role_name_prefix"></a> [role\_name\_prefix](#input\_role\_name\_prefix) | IAM role name prefix | `string` | `null` | no |
 | <a name="input_role_path"></a> [role\_path](#input\_role\_path) | Path of IAM role | `string` | `"/"` | no |
 | <a name="input_role_permissions_boundary_arn"></a> [role\_permissions\_boundary\_arn](#input\_role\_permissions\_boundary\_arn) | Permissions boundary ARN to use for IAM role | `string` | `""` | no |
 | <a name="input_role_requires_mfa"></a> [role\_requires\_mfa](#input\_role\_requires\_mfa) | Whether role requires MFA | `bool` | `true` | no |
diff --git a/modules/iam-assumable-role/main.tf b/modules/iam-assumable-role/main.tf
@@ -1,8 +1,6 @@
-data "aws_caller_identity" "current" {}
-data "aws_partition" "current" {}
-
 locals {
   role_sts_externalid = flatten([var.role_sts_externalid])
+  role_name_condition = var.role_name != null ? var.role_name : "${var.role_name_prefix}*"
 }
 
 data "aws_iam_policy_document" "assume_role" {
@@ -19,7 +17,13 @@ data "aws_iam_policy_document" "assume_role" {
 
       principals {
         type        = "AWS"
-        identifiers = ["arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:role${var.role_path}${var.role_name}"]
+        identifiers = ["*"]
+      }
+
+      condition {
+        test     = "ArnLike"
+        variable = "aws:PrincipalArn"
+        values   = ["arn:${local.partition}:iam::${local.account_id}:role${var.role_path}${local.role_name_condition}"]
       }
     }
   }
@@ -63,7 +67,13 @@ data "aws_iam_policy_document" "assume_role_with_mfa" {
 
       principals {
         type        = "AWS"
-        identifiers = ["arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:role${var.role_path}${var.role_name}"]
+        identifiers = ["*"]
+      }
+
+      condition {
+        test     = "ArnLike"
+        variable = "aws:PrincipalArn"
+        values   = ["arn:${local.partition}:iam::${local.account_id}:role${var.role_path}${local.role_name_condition}"]
       }
     }
   }
@@ -109,6 +119,7 @@ resource "aws_iam_role" "this" {
   count = var.create_role ? 1 : 0
 
   name                 = var.role_name
+  name_prefix          = var.role_name_prefix
   path                 = var.role_path
   max_session_duration = var.max_session_duration
   description          = var.role_description
diff --git a/modules/iam-assumable-role/variables.tf b/modules/iam-assumable-role/variables.tf
@@ -46,6 +46,12 @@ variable "role_name" {
   default     = ""
 }
 
+variable "role_name_prefix" {
+  description = "IAM role name prefix"
+  type        = string
+  default     = null
+}
+
 variable "role_path" {
   description = "Path of IAM role"
   type        = string
diff --git a/modules/iam-assumable-roles-with-saml/README.md b/modules/iam-assumable-roles-with-saml/README.md
@@ -3,8 +3,8 @@
 Creates predefined IAM roles (admin, poweruser and readonly) which can be assumed by trusted resources using SAML Federated Users.
 
 
-[Creating IAM SAML Identity Providers](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_saml.html) 
-[Enabling SAML 2.0 Federated Users to Access the AWS Management Console](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_enable-console-saml.html) 
+[Creating IAM SAML Identity Providers](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_saml.html)
+[Enabling SAML 2.0 Federated Users to Access the AWS Management Console](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_enable-console-saml.html)
 
 <!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
 ## Requirements
@@ -34,9 +34,7 @@ No modules.
 | [aws_iam_role_policy_attachment.admin](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
 | [aws_iam_role_policy_attachment.poweruser](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
 | [aws_iam_role_policy_attachment.readonly](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
-| [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source |
 | [aws_iam_policy_document.assume_role_with_saml](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
-| [aws_partition.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/partition) | data source |
 
 ## Inputs
 
diff --git a/modules/iam-assumable-roles-with-saml/main.tf b/modules/iam-assumable-roles-with-saml/main.tf
@@ -1,23 +1,70 @@
-data "aws_caller_identity" "current" {}
-data "aws_partition" "current" {}
-
 locals {
   identifiers = compact(distinct(concat(var.provider_ids, [var.provider_id])))
 }
 
 data "aws_iam_policy_document" "assume_role_with_saml" {
   dynamic "statement" {
     # https://aws.amazon.com/blogs/security/announcing-an-update-to-iam-role-trust-policy-behavior/
-    for_each = var.allow_self_assume_role ? [1] : []
+    for_each = var.allow_self_assume_role && var.create_admin_role ? [1] : []
 
     content {
-      sid     = "ExplicitSelfRoleAssumption"
+      sid     = "ExplicitSelfAdminRoleAssumption"
       effect  = "Allow"
       actions = ["sts:AssumeRole"]
 
       principals {
         type        = "AWS"
-        identifiers = ["arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:role${var.role_path}${var.role_name}"]
+        identifiers = ["*"]
+      }
+
+      condition {
+        test     = "ArnLike"
+        variable = "aws:PrincipalArn"
+        values   = ["arn:${local.partition}:iam::${local.account_id}:role${var.admin_role_path}${var.admin_role_name}"]
+      }
+    }
+  }
+
+  dynamic "statement" {
+    # https://aws.amazon.com/blogs/security/announcing-an-update-to-iam-role-trust-policy-behavior/
+    for_each = var.allow_self_assume_role && var.create_poweruser_role ? [1] : []
+
+    content {
+      sid     = "ExplicitSelfPowerUserRoleAssumption"
+      effect  = "Allow"
+      actions = ["sts:AssumeRole"]
+
+      principals {
+        type        = "AWS"
+        identifiers = ["*"]
+      }
+
+      condition {
+        test     = "ArnLike"
+        variable = "aws:PrincipalArn"
+        values   = ["arn:${local.partition}:iam::${local.account_id}:role${var.poweruser_role_path}${var.poweruser_role_name}"]
+      }
+    }
+  }
+
+  dynamic "statement" {
+    # https://aws.amazon.com/blogs/security/announcing-an-update-to-iam-role-trust-policy-behavior/
+    for_each = var.allow_self_assume_role && var.create_readonly_role ? [1] : []
+
+    content {
+      sid     = "ExplicitSelfReadOnlyRoleAssumption"
+      effect  = "Allow"
+      actions = ["sts:AssumeRole"]
+
+      principals {
+        type        = "AWS"
+        identifiers = ["*"]
+      }
+
+      condition {
+        test     = "ArnLike"
+        variable = "aws:PrincipalArn"
+        values   = ["arn:${local.partition}:iam::${local.account_id}:role${var.readonly_role_path}${var.readonly_role_name}"]
       }
     }
   }
diff --git a/modules/iam-assumable-roles/README.md b/modules/iam-assumable-roles/README.md
@@ -32,10 +32,8 @@ No modules.
 | [aws_iam_role_policy_attachment.admin](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
 | [aws_iam_role_policy_attachment.poweruser](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
 | [aws_iam_role_policy_attachment.readonly](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
-| [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source |
 | [aws_iam_policy_document.assume_role](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.assume_role_with_mfa](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
-| [aws_partition.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/partition) | data source |
 
 ## Inputs
 
diff --git a/modules/iam-assumable-roles/main.tf b/modules/iam-assumable-roles/main.tf
diff --git a/modules/iam-eks-role/main.tf b/modules/iam-eks-role/main.tf
diff --git a/modules/iam-role-for-service-accounts-eks/README.md b/modules/iam-role-for-service-accounts-eks/README.md
diff --git a/modules/iam-role-for-service-accounts-eks/main.tf b/modules/iam-role-for-service-accounts-eks/main.tf
diff --git a/modules/iam-role-for-service-accounts-eks/variables.tf b/modules/iam-role-for-service-accounts-eks/variables.tf

Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,7 @@ locals {`
`6`	`6`	`replace(url, "https://", "")`
`7`	`7`	`]`
`8`	`8`	`number_of_role_policy_arns = coalesce(var.number_of_role_policy_arns, length(var.role_policy_arns))`
	`9`	`+ role_name_condition = var.role_name != null ? var.role_name : "${var.role_name_prefix}*"`
`9`	`10`	`}`
`10`	`11`
`11`	`12`	`data "aws_caller_identity" "current" {}`
`@@ -25,7 +26,13 @@ data "aws_iam_policy_document" "assume_role_with_oidc" {`
`25`	`26`
`26`	`27`	`principals {`
`27`	`28`	`type = "AWS"`
`28`		`- identifiers = ["arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:role${var.role_path}${var.role_name}"]`
	`29`	`+ identifiers = ["*"]`
	`30`	`+ }`
	`31`	`+`
	`32`	`+ condition {`
	`33`	`+ test = "ArnLike"`
	`34`	`+ variable = "aws:PrincipalArn"`
	`35`	`+ values = ["arn:${local.partition}:iam::${local.account_id}:role${var.role_path}${local.role_name_condition}"]`
`29`	`36`	`}`
`30`	`37`	`}`
`31`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,9 +1,7 @@`
`1`		`-data "aws_caller_identity" "current" {}`
`2`		`-data "aws_partition" "current" {}`
`3`		`-`
`4`	`1`	`locals {`
`5`	`2`	`identifiers = compact(distinct(concat(var.provider_ids, [var.provider_id])))`
`6`	`3`	`number_of_role_policy_arns = coalesce(var.number_of_role_policy_arns, length(var.role_policy_arns))`
	`4`	`+ role_name_condition = var.role_name != null ? var.role_name : "${var.role_name_prefix}*"`
`7`	`5`	`}`
`8`	`6`
`9`	`7`	`data "aws_iam_policy_document" "assume_role_with_saml" {`
`@@ -18,7 +16,13 @@ data "aws_iam_policy_document" "assume_role_with_saml" {`
`18`	`16`
`19`	`17`	`principals {`
`20`	`18`	`type = "AWS"`
`21`		`- identifiers = ["arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:role${var.role_path}${var.role_name}"]`
	`19`	`+ identifiers = ["*"]`
	`20`	`+ }`
	`21`	`+`
	`22`	`+ condition {`
	`23`	`+ test = "ArnLike"`
	`24`	`+ variable = "aws:PrincipalArn"`
	`25`	`+ values = ["arn:${local.partition}:iam::${local.account_id}:role${var.role_path}${local.role_name_condition}"]`
`22`	`26`	`}`
`23`	`27`	`}`
`24`	`28`	`}`