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.

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

Feature request: Exclusive policy attachment list for users, groups, and roles #4426

Closed
jbscare opened this issue May 2, 2018 · 31 comments
Closed
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/iam Issues and PRs that pertain to the iam service.

Comments

@jbscare
Copy link

jbscare commented May 2, 2018

Terraform Version

+$ terraform -v
Terraform v0.11.7
+ provider.aws v1.16.0

Affected Resource(s)

  • aws_iam_user_policy_attachment
  • aws_iam_group_policy_attachment
  • aws_iam_role_policy_attachment

Expected Behavior

This is a feature request for either (a) an enhancement to aws_iam_user_policy_attachment, aws_iam_group_policy_attachment, and aws_iam_role_policy_attachment; or (b) to create a new trio of resources (perhaps aws_iam_user_policy_list, aws_iam_group_policy_list, and aws_iam_role_policy_list?), to allow for the specification of an exclusive list of policies that should be attached to a group or role, much like aws_iam_group_membership allows for the specification of an exclusive list of users who should be members of a group. If any policies are found attached to the group or role that aren't on the list, they should be removed.

Actual Behavior

There doesn't seem to be a way to accomplish this effect at this time. (You can get sort of a converse of this effect with aws_iam_policy_attachment, which specifies an exclusive list of roles and users and groups to which a policy is attached, but doesn't exclude other policies from being attached to a role, user, or group.)

Use case

We'd like to use Terraform to fully specify the policies that should be attached to a user or group or role, and if someone adds a policy outside of Terraform, for Terraform to detect and correct that.

@jbscare jbscare changed the title Feature request: Exclusive policy attachment lists for users, groups, and roles Feature request: Exclusive policy attachment list for users, groups, and roles May 2, 2018
@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. service/iam Issues and PRs that pertain to the iam service. labels May 12, 2018
@lorengordon
Copy link
Contributor

Following. I thought terraform already did this. Yikes. Major security hole here.

@FernandoMiguel
Copy link
Contributor

@lorengordon it's by design.
No security risk.
If you allow anyone to modify your account, no security can save you

@lorengordon
Copy link
Contributor

It can certainly be one thing to you and another to me and our auditors. Call it whatever you like. We call it a security risk.

We want to use terraform to manage infrastructure as code. We create the IAM role with terraform and we create the IAM policies with terraform. But it turns out we cannot actually guarantee an exact configuration of a role using terraform, since any policies added to the role outside of terraform are basically invisible to terraform right now. That's pretty bad in our book. We'll need to use something else to manage IAM roles until this is fixed.

I see it as similar functionality as what terraform provides with security groups... You can either define the rules as part of the aws_security_group resource, and those rules will remove any other rules, ensuring the exact configuration of the security group. Or you can use the aws_security_group_rule resource to add rules to a security group without removing any externally added rules.

@jbscare
Copy link
Author

jbscare commented Jul 13, 2018

The only thing I'd quibble with is that using Terraform in this way doesn't create a security hole where there wasn't one before. If you consider users groups and roles that aren't managed by code to be a security hole, then it's true that Terraform doesn't fix that, but Terraform doesn't fix a lot of security holes.

All that said, we of course want this feature, since this seems like a security hole that Terraform could fix. :^) (Or an "issue" that it could "address", if you like.)

@YakDriver
Copy link
Member

YakDriver commented Sep 12, 2018

@jbscare @bflad @lorengordon I'm working on a PR for an exclusive list of role policies (aws_iam_role_policy_list) currently. There are a few design options. I'd like to get your thoughts.

  1. ONLY enforce exclusivity

The new resource would not associate listed policies with the role. It would detach managed or delete inline policies found that weren't listed. If you didn't want the resource to mess with managed policies, you could skip the argument and the role's managed policies would be left unaffected. Same for inline policies.

If you wanted to make sure that no inline and/or managed policies were put/attached to the role, you could include inline_policies = [] or managed_policies = [].

This would provide all the security benefits with no redundant functionality. To associate a policy with a role, you would have to use aws_role_policy_attachment (managed) or aws_role_policy (inline).

resource "aws_iam_role_policy_list" "policy_list" {
  name = "yak_testing_role_policy_list"
  role = "${aws_iam_role.instance_role.name}"

  managed_policies = [
    "${aws_iam_policy.policy_one.arn}",
    "${aws_iam_policy.policy_two.arn}",
  ]

  inline_policies = [
    "${aws_iam_role_policy.inline_test_policy1.name}",
    "${aws_iam_role_policy.inline_test_policy2.name}",
  ]
}
  1. Enforce exclusivity and manage managed policies

The same as the first approach except with the added convenience that you would not need aws_role_policy_attachment to attach managed policies to the role. In other words, the listed managed policies would be attached to the role if they were not already. Inline policies would still be handled as above (inline cannot be managed in the same way because to put an inline policy requires the policy document).

  1. Ultimate management of policies

Redundant functionality but very convenient. You can create and associate managed and inline policies, and an exclusive list, all with the same resource.

resource "aws_iam_role_policy_list" "policy_list" {
  name = "yak_testing_role_policy_list"
  role = "${aws_iam_role.instance_role.name}"

  managed_policy {
  name        = "test_managed_policy"
  path        = "/"
  description = "My test policy"
  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": [
        "ec2:Describe*"
      ],
      "Effect": "Allow",
      "Resource": "*"
    }
  ]
}
EOF
  }

  inline_policy {
  name = "test_inline_policy"
  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": [
        "ec2:Describe*"
      ],
      "Effect": "Allow",
      "Resource": "*"
    }
  ]
}
EOF
  }
}
  1. My view

I think that #2 is probably the best approach but #1 has merit. #3 is the wrong approach IMO.

@jbscare
Copy link
Author

jbscare commented Sep 12, 2018

I like option 2: I think we want to be able to say "this role/user/group should have exactly this list of policies attached", and Terraform should cause that to happen, whether that means removing policies that are attached but aren't on the list, or adding policies that aren't attached and are on the list.

It seems like option 1 could get you into a weird state where your TF config could define a policy attachment but not have it on the list -- what would Terraform do then?

@lorengordon
Copy link
Contributor

It seems like option 1 could get you into a weird state where your TF config could define a policy attachment but not have it on the list -- what would Terraform do then?

Terraform would first attach it, then detach it! 😂 Don't do that!

Option 2 doesn't resolve that scenario for inline policies... If a user creates and associates the inline policy using aws_role_policy, but does not specify the inline policy in aws_iam_role_policy_list, then terraform would again create the inline policy and then delete it!

Personally, I'm ok with this, as I think it would be pretty apparent what was happening from one apply to the next. User would just need to update their config to match their intent.

I like option 1... I think option 3 could be compelling but only if we can reuse the code from the other modules.

@YakDriver
Copy link
Member

YakDriver commented Sep 12, 2018

whether that means... adding policies that aren't attached and are on the list

That's not a problem for managed policies. The scenario is not possible for inline. The scenarios would be:

Policy type Assoc Listed Outcome
Managed Attached Yes All - No change
Managed Not attached Yes 1 - Error; 2 - Attached; 3 - Attached
Managed Attached No All - Detached
Managed Not attached No All - No change
Inline Added Yes All - No change
Inline Not added Yes 1 & 2 - Error (policy cannot exist); 3 - Created
Inline Added No All - Deleted
Inline Not added No All - No change

Both option 1 and option 2 would do the same thing with inconsistent config (e.g., aws_role_policy defines an inline policy that is not listed in the aws_iam_role_policy_list). But, isn't that the exact point of an exclusive list? If it's not on the list, it can't go to the VIP room.

@YakDriver
Copy link
Member

YakDriver commented Sep 13, 2018

@lorengordon @jbscare @bflad Implementation of the new resource is pretty much ready. (Still working on docs and tests.) But, just to make sure we're on the same page, please consider the following config and outcomes and see if it works for you.

resource "aws_iam_role_policy_list" "rpl" {
  name = "yak_testing_role_policy_list"

  managed_policies = [
    "${aws_iam_policy.policy_one.arn}",
    "${aws_iam_policy.policy_two.arn}",
  ]

  inline_policies = [
    "${aws_iam_role_policy.inline_test_policy.name}",
  ]

  role = "${aws_iam_role.instance_role.name}"
}

resource "aws_iam_role_policy" "inline_test_policy" {
  name = "yak_inline_test_policy"
  role = "${aws_iam_role.instance_role.name}"

  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": [
        "ec2:Describe*"
      ],
      "Effect": "Allow",
      "Resource": "*"
    }
  ]
}
EOF
}

data "aws_iam_policy_document" "instance_assume_role_policy" {
  statement {
    actions = ["sts:AssumeRole"]

    principals {
      type        = "Service"
      identifiers = ["ec2.amazonaws.com"]
    }
  }
}

resource "aws_iam_role" "instance_role" {
  name               = "yak_instance_role"
  path               = "/system/"
  assume_role_policy = "${data.aws_iam_policy_document.instance_assume_role_policy.json}"
}

resource "aws_iam_policy" "policy_one" {
  name        = "yak_test_policy1"
  path        = "/"
  description = "My test policy"

  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": [
        "ec2:Describe*"
      ],
      "Effect": "Allow",
      "Resource": "*"
    }
  ]
}
EOF
}

resource "aws_iam_policy" "policy_two" {
  name        = "yak_test_policy2"
  path        = "/"
  description = "My test policy"

  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": [
        "ec2:Describe*"
      ],
      "Effect": "Allow",
      "Resource": "*"
    }
  ]
}
EOF
}

Assuming, this chain of events, the table lists the outcomes.

  1. Terraform created everything using the config above
  2. Some non-Terraform action takes place
  3. terraform apply is performed again
  4. Outcome as shown below
Non-TF Action On apply
Managed policy detached Policy reattached ☑️
Managed policy deleted Policy recreated, attached ☑️
Extra managed policy attached Policy detached ☑️
Extra inline policy added Policy deleted ☑️
Inline policy modified Policy reverted to original ☑️
No inline policy list, inline policy added No changes ☑️
No managed policy list, managed policy attached No changes ☑️
Empty inline policy list, inline policy added Inline policy deleted ☑️
Empty managed policy list, managed policy attached Managed policy detached ☑️

Let me know if you'd like more tests or different outcomes.

@jbscare
Copy link
Author

jbscare commented Sep 14, 2018

Just to clarify, in that table in the previous comment, the "Action" column describes what would happen if you did a terraform apply after someone took those actions via some out-of-band non-Terraform method (like the AWS web UI), right?

If so, that all sounds good to me!

One thought: I'm not sure about the aws_iam_role_policy_list name; it's not intuitively clear from the name how this relates to aws_iam_policy_attachment. It's sort of cumbersome, but would aws_iam_exclusive_policy_attachment_list be clearer? Any other ideas?

Also, is this only for roles? Could it be extended to cover users and groups as well?

Thanks for all your work on this!

@YakDriver
Copy link
Member

I'll clean up the table to make it a bit clearer.

aws_iam_role_policy_list has 24 characters. What about aws_iam_role_policy_sole_list (29), aws_iam_role_policy_exclusive_list (34), or aws_iam_role_policy_manager (27)?

Right now I'm just working roles. With some lessons learned with roles (with the implementation and merge process), we may have a chance to do users and groups as well.

@YakDriver
Copy link
Member

@jbscare @lorengordon Feel free to thumbs up the PR if you have a chance.

@YakDriver
Copy link
Member

The acceptance tests for PR #5904 cover the tests in the comment above.

bflad added a commit that referenced this issue Nov 25, 2019
…s necessary IAM Role permissions depends_on configuration (#11010)

Closes #10934
Reference: ENIs currently stuck in-use in main testing account us-west-2
Reference: #5904
Reference: #4426

Otherwise, EC2 ENIs managed by EKS can be left dangling on destroy. In the future, we can help reduce the need for explicit Terraform dependencies such as these via supporting the management of attached IAM Role policies directly in the aws_iam_role resource (e.g. #5904).

Output from acceptance testing:

```
--- PASS: TestAccAWSEksNodeGroup_Version (1449.66s)
--- PASS: TestAccAWSEksNodeGroup_AmiType (1462.03s)
--- PASS: TestAccAWSEksNodeGroup_DiskSize (1510.26s)
--- PASS: TestAccAWSEksNodeGroup_ReleaseVersion (1575.37s)
--- PASS: TestAccAWSEksNodeGroup_RemoteAccess_SourceSecurityGroupIds (1584.41s)
--- PASS: TestAccAWSEksNodeGroup_RemoteAccess_Ec2SshKey (1597.61s)
--- PASS: TestAccAWSEksNodeGroup_ScalingConfig_MinSize (1624.95s)
--- PASS: TestAccAWSEksNodeGroup_basic (1644.52s)
--- PASS: TestAccAWSEksNodeGroup_InstanceTypes (1646.77s)
--- PASS: TestAccAWSEksNodeGroup_disappears (1652.94s)
--- PASS: TestAccAWSEksNodeGroup_Labels (1655.54s)
--- PASS: TestAccAWSEksNodeGroup_ScalingConfig_DesiredSize (1702.82s)
--- PASS: TestAccAWSEksNodeGroup_ScalingConfig_MaxSize (1764.88s)
--- PASS: TestAccAWSEksNodeGroup_Tags (1768.71s)
```
@github-actions
Copy link

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

@github-actions github-actions bot added the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Sep 29, 2020
@lorengordon
Copy link
Contributor

Please don't close this, it would be a very important feature for maintaining strict control over iam roles (and users and groups).

@ghost ghost removed the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Sep 29, 2020
@soumyajk
Copy link

Hi,
This is a very valid problem(surprised to see the request in stale state).
Its very annoying to have additional policies attached against iam roles which are created through terraform, which the terraform is completely unaware of. I would have expected terraform to show it as a drift, detach the managed policy if not present in terraform.

This also makes it impossible to catch the drift, which stops the role from being deleted if force_detach_policies value is set true.

I completely agree with the model shared in comment : #4426 (comment)

I would appreciate if someone can share the latest update on this bug.

Thanks

@lorengordon
Copy link
Contributor

lorengordon commented Feb 8, 2021

If anyone subscribed here still really cares about this feature like I do, here are few new issues to please go give a thumbs up, plus the pr that implements exclusive attachment for IAM roles...

YakDriver added a commit to YakDriver/terraform-provider-aws that referenced this issue Feb 22, 2021
This provides a way to maintain an exhaustive list of policies
associated with a given role. When applied, any changes made
outside of Terraform, are reverted back to the configuration
provided to Terraform.

See hashicorp#4426
@AnthonyFoiani-at
Copy link

Hello! It took me a quite a while to figure out what aws_iam_role.managed_policy_arns meant, and why it was added.

It looks like I'm not the only person that was confused: https://discuss.hashicorp.com/t/confusion-about-aws-iam-role-managed-policy-arns-attribute/29728

Could the documentation be expanded / reworded? In particular, the usage of "exclusive" made me think that "oh, this role is the only user of this policy", not "this role is the only thing that controls which policies are attached to itself".

Maybe something along the lines of:

If you specify a list of managed_policy_arns, Terraform apply will attempt to enforce that exactly and only those policies are attached to this role; any out-of-band changes or separate attachment resources will be disregarded and/or reverted (and/or cause resource cycling).

Thank you!

@lorengordon
Copy link
Contributor

@AnthonyFoiani-at I do not believe that is accurate. When you specify this:

resource "aws_iam_role" "sensitive" {
  ...
  # Enforce that we want exactly and only these policies attached to this role:
  managed_policy_arns = [
    aws_iam_policy.sensitive_stuff,
    aws_iam_policy.other_sensitive_stuff,
  ]
}

The aws_iam_role resource fully manages the attachments. You do not need and should not use aws_iam_role_policy_attachment for that role at all.

@AnthonyFoiani-at
Copy link

The aws_iam_role resource fully manages the attachments. You do not need and should not use aws_iam_role_policy_attachment for that role at all.

Thank you for the correction -- I will do some experiments!

Hopefully it's still a constructive message that the documentation is confusing?

@AnthonyFoiani-at
Copy link

@lorengordon wrote:

The aws_iam_role resource fully manages the attachments. You do not need and should not use aws_iam_role_policy_attachment for that role at all.

Verified! Thank you for the correction.

@lorengordon
Copy link
Contributor

Nice. I think part of understanding the docs comes with the term "exclusive" and what terraform means by that. And then the tri-modal nature of assigning a value.

A few resources have this concept of what I call a "container" resource and an attachment. Like IAM Roles (container) and policies (attachment), and security groups (container) and rules (attachment). "Exclusive" is in relation to how terraform manages the attachments on the container resource.

When you use an attribute on the "container" resource that manages attachments, it is an "exclusive" relationship. I.e. Exactly and only these attachments should be present, and any other attachments other than what is specified will be removed.

When you use a separate attachment resource, it is non-exclusive. Meaning you could have one terraform state that manages the container resource (without specifying the attachment attribute), and then you could have any number of other terraform states managing attachments to that container resource. Or really, the attachments could be created any way at all, including manually in the console or cli. When running terraform on the state that manages the container resource, it will just ignore the attachments.

So, that kinda gets at the tri-modal nature also... Here is what I mean, see the comment on each example:

resource "aws_iam_role" "sensitive" {
  ...
  # Enforce that we want exactly and only these policies attached to this role:
  managed_policy_arns = [
    aws_iam_policy.sensitive_stuff,
    aws_iam_policy.other_sensitive_stuff,
  ]
}
resource "aws_iam_role" "sensitive" {
  ...
  # Enforce that there must be exactly 0 attached policies:
  managed_policy_arns = []
}
resource "aws_iam_role" "sensitive" {
  ...
  # Ignore all attachments, neither add nor remove them
  # managed_policy_arns = null
}

@AnthonyFoiani-at
Copy link

@lorengordon Those examples are great! "Tri-modal" isn't a word I hear very often. :-)

And now that I [am pretty sure! I] understand what's going on, the existing wording in the docs makes sense. That's a tough puzzle to crack; hopefully my naive comments above might help us find a starting point.

The only change I'd suggest is to the last one:

resource "aws_iam_role" "sensitive" {
  ...
  # This resource will not manipulate associated policy attachments on its own; you will
  # need to attach policies through either `aws_iam_role_policy_attaachment` resources,
  # or manually via the console / API / CLI.
  managed_policy_arns = null
}

Thank you all again, and have a great weekend!

@jar-b
Copy link
Member

jar-b commented Oct 22, 2024

As of v5.72.1 of the AWS provider, the following standalone resources are available to enable Terraform to exclusively manage inline and managed policy attachments for IAM roles, users, and groups.

Inline policies:

Customer managed policy attachments:

The following proposal contains more context for the creation of these resources, and this meta issue tracks the progress of "exclusive relationship management resources" across the entire provider.

With the availability of these resources, this issue can now be closed.

@jar-b jar-b closed this as completed Oct 22, 2024
Copy link

Warning

This issue has been closed, meaning that any additional comments are hard for our team to see. Please assume that the maintainers will not see them.

Ongoing conversations amongst community members are welcome, however, the issue will be locked after 30 days. Moving conversations to another venue, such as the AWS Provider forum, is recommended. If you have additional concerns, please open a new issue, referencing this one where needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/iam Issues and PRs that pertain to the iam service.
Projects
None yet
Development

No branches or pull requests

8 participants