Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add more options for generating trust policy #535

Open
1 task done
enver-multibank opened this issue Nov 4, 2024 · 2 comments · May be fixed by #537
Open
1 task done

Add more options for generating trust policy #535

enver-multibank opened this issue Nov 4, 2024 · 2 comments · May be fixed by #537

Comments

@enver-multibank
Copy link
Contributor

Description

Trust role policy generated for IAM role does not have controls for conditionals like aws:SourceArn, aws:SourceAccount, aws:SourceOrgID, or aws:SourceOrgPaths used for confused deputy problem prevention. Workaround for the problem is to provide custom trust policy with appropriate conditions for role assumption, however I think we should promote good security practices and have some of these controls added to role modules and perhaps enabled by default at some point in the future.

If approved I'm willing to submit PR (for start maybe just one conditional, like aws:SourceAccount) with backward compatible changes to test it out.

  • ✋ I have searched the open/closed issues and my issue is not listed.

Versions

  • Module version [Required]:
    5.28.0
  • Terraform version:
    Terraform v1.7.5
    registry.terraform.io/hashicorp/aws v5.58.0

Steps to reproduce the behavior:

module "test_role" {
  source  = "terraform-aws-modules/iam/aws//modules/iam-assumable-role"
  version = "5.28.0"

  trusted_role_services = [
    "ec2.amazonaws.com"
  ]

  create_role             = true
  create_instance_profile = true

  role_name         = "test_role"
  role_description  = "Test role"
  role_requires_mfa = false

  custom_role_policy_arns = [
    "arn:aws:iam::aws:policy/AmazonSSMManagedInstanceCore"
  ]
}

Generated trust policy looks like this:

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

Expected behavior

We should be able to generate policy like this

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "",
            "Effect": "Allow",
            "Principal": {
                "Service": "ec2.amazonaws.com"
            },
            "Action": [
                "sts:TagSession",
                "sts:AssumeRole"
            ],
            "Condition": {
                "StringEquals": {
                    "aws:SourceAccount": "<account id>"
                }
            }
        }
    ]
}

Actual behavior

There is no option to add conditions except for: aws:PrincipalArn

Terminal Output Screenshot(s)

Additional context
tbalzer added a commit to ExB-Group/terraform-aws-iam that referenced this issue Nov 5, 2024
@tbalzer
feat: Add more trust policy options (terraform-aws-modules#535)
2b5d161 
Signed-off-by: Tim Balzer <[email protected]>
@enver-multibank
Copy link
Contributor Author

@tbalzer Do you plan to submit PR?

@enver enver linked a pull request Nov 18, 2024 that will close this issue
Open
3 tasks
@tbalzer
Copy link

tbalzer commented Nov 18, 2024

Hi @enver-multibank , thanks for creating a PR here. I got sidetracked and couldn't find the time recently to push this over the line with proper examples and local testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Add more trust policy condition options enver/terraform-aws-iam
2 participants
@tbalzer @enver-multibank