Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

fix(terraform): improve the adaptation of IAM resources #37

Merged
merged 10 commits into from
Nov 7, 2023

Conversation

nikpivkin
Copy link
Collaborator

@nikpivkin nikpivkin commented Oct 25, 2023

@nikpivkin nikpivkin changed the title fix(terraform): improve the adaptation of iam resources fix(terraform): improve the adaptation of IAM resources Oct 26, 2023
@nikpivkin
Copy link
Collaborator Author

nikpivkin commented Oct 26, 2023

@simar7 Do we need to verify that IAM resources have the same provider? . Why is it only present on theirs? AWS documentation says that IAM services are not region specific:

The resources that you create in one Region do not exist in any other Region unless you explicitly use a replication feature offered by an AWS service. For example, Amazon S3 and Amazon EC2 support cross-Region replication. Some services, such as AWS Identity and Access Management (IAM), do not have Regional resources.

I have successfully created the following IAC:

provider "aws" {
  region = "us-east-1"
  alias  = "us-east"
}

provider "aws" {
  region = "us-west-1"
  alias  = "us-west"
}

resource "aws_iam_role_policy" "this" {
  provider = aws.us-east
  name     = "test_policy"
  role     = aws_iam_role.this.id

  # Terraform's "jsonencode" function converts a
  # Terraform expression result to valid JSON syntax.
  policy = jsonencode({
    Version = "2012-10-17"
    Statement = [
      {
        Action = [
          "ec2:Describe*",
        ]
        Effect   = "Allow"
        Resource = "*"
      },
    ]
  })
}

resource "aws_iam_role" "this" {
  provider = aws.us-west
  name     = "test_role"

  assume_role_policy = jsonencode({
    Version = "2012-10-17"
    Statement = [
      {
        Action = "sts:AssumeRole"
        Effect = "Allow"
        Sid    = ""
        Principal = {
          Service = "ec2.amazonaws.com"
        }
      },
    ]
  })
}

As you can see, I've used different provider configurations for role and policy, but it doesn't affect anything.

aws_iam_role.this: Creating...
aws_iam_role.this: Creation complete after 2s [id=test_role]
aws_iam_role_policy.this: Creating...
aws_iam_role_policy.this: Creation complete after 1s [id=test_role:test_policy]

But through the github search I found where separate provider configurations are used for IAM services, am I missing something?

Ref:

@nikpivkin nikpivkin marked this pull request as ready for review October 26, 2023 17:31
@nikpivkin nikpivkin requested a review from simar7 as a code owner October 26, 2023 17:31
@simar7
Copy link
Member

simar7 commented Oct 26, 2023

As per your example

resource "aws_iam_role_policy" "this" {
  provider = aws.us-east
  name     = "test_policy"
  role     = aws_iam_role.this.id

Does the provider attribute exist? I don't see it document in terraform docs https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy#argument-reference - this omission makes sense to me because IAM is region agnostic as you pointed out.

As for the same provider code, I'm not sure. Have you checked git blame history in older version of defsec to see why it was added?

@nikpivkin
Copy link
Collaborator Author

@simar7
Copy link
Member

simar7 commented Oct 27, 2023

@simar7 provider is a meta-argument https://developer.hashicorp.com/terraform/language/meta-arguments/resource-provider

ah I missed that. I would assume aws doesn't care about it as IAM is region agnostic. I was reading the docs for the CLI and they do expose a region option (not sure why) my guess it was autogenerated doc and region is just a global option. https://awscli.amazonaws.com/v2/documentation/api/latest/reference/iam/create-policy.html

But if you see the IAM ARN info page, they talk about region is always kept blank https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_identifiers.html#identifiers-arns

@simar7
Copy link
Member

simar7 commented Oct 27, 2023

Could the check be in place to resolve such issues? https://stackoverflow.com/a/66859239

@nikpivkin
Copy link
Collaborator Author

@simar7 Which check do you mean?

@nikpivkin
Copy link
Collaborator Author

As for the same provider code, I'm not sure. Have you checked git blame history in older version of defsec to see why it was added?

I couldn't find any information on why the check was added:
https://github.com/aquasecurity/tfsec/pull/1361/files#diff-f6a469bc57cae0a81fc6120d0e65ee7b84ef208726e49c39a6dc09c9113cc33a

@simar7
Copy link
Member

simar7 commented Nov 1, 2023

@nikpivkin looks like there are some merge conflicts to resolve.

@nikpivkin
Copy link
Collaborator Author

@simar7 Done.

@simar7 simar7 merged commit e92df03 into aquasecurity:main Nov 7, 2023
3 checks passed
@nikpivkin nikpivkin deleted the fix/iam-resources branch November 7, 2023 07:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants