[v2] Overly permissive/redundant IAM permission

[mike-stewart]

Related to #1302. Reviewing the required IAM policy for the v2 controller, it appears that there is some duplication.

There are two statements providing "ec2:AuthorizeSecurityGroupIngress" and "ec2:RevokeSecurityGroupIngress". This more restricted statement that checks for resource tags, and this more permissive statement that does not.

Can the more permissive one be deleted in favour of the more restricted one? Since AuthorizeSecurityGroupIngress is a fairly sensitive permission, it would be great to be able to lock that down further.

[M00nF1sh]

@mike-stewart
we layout the permission like so because they are for different purpose and can be scoped down differently. (we are planning a guide on how to further scope down these permissions).
the first statement(without resourceTags) is let controller modify worker node security groups, which can be further scope down by vpc-id and kubernetes.io/cluster/cluster-name: owned/shared tag.
the second statement(with resourceTag) is let controller modify the securityGroup created for ALB. which can be further scope down with vpc-id and elbv2.k8s.aws/cluster: cluster-name tag.

[mike-stewart]

@M00nF1sh Makes sense! Is there an existing issue for the guide on scoping down the permissions? If so, we could close this issue in favour of that one.

[M00nF1sh]

@mike-stewart
we don't have a guide about how to further scope down the permissions yet. Which we'll work on soon. will update this issue once we have it :D

[zhujik]

I'll just highjack this issue to tell you my findings, because we are migrating to v2 and the iam permissions are a bit different.

There are some iam permissions that are now missing that have been used in the previous v1 version, namely:

• acm:GetCertificate
• ec2:DescribeInstanceStatus
• ec2:ModifyInstanceAttribute
• ec2:ModifyNetworkInterfaceAttribute
• shield:DescribeSubscription
• shield:ListProtections
• waf:GetWebACL

For the migration process, for the time being, we have re-included them in the iam policy to avoid any unforseen consequences. So I would just kindly ask if you left them out intentionally or if the might have been omitted accidentally, because the git history of the iam-policy.json does not indicate the history of the file in regards to the v1 version.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[mike-stewart]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[mike-stewart]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[mike-stewart]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[mike-stewart]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[mike-stewart]

/remove-lifecycle stale

[kishorj]

@mike-stewart, please refer to the v2.4 live docs https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.4/deploy/installation/#setup-iam-role-for-service-accounts for scoping the IAM permissions.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[mike-stewart]

@kishorj @M00nF1sh Thanks for updating the documentation for the AuthorizeSecurityGroupIngress policy I originally asked about.

I have a few more questions about how to scope down this policy, in particular:

• Could this have a "aws:ResourceTag/elbv2.k8s.aws/cluster" condition?

  aws-load-balancer-controller/docs/install/iam_policy.json

  Lines 139 to 148 in 9c79e45

  	{
  	"Effect": "Allow",
  	"Action": [
  	"elasticloadbalancing:CreateListener",
  	"elasticloadbalancing:DeleteListener",
  	"elasticloadbalancing:CreateRule",
  	"elasticloadbalancing:DeleteRule"
  	],
  	"Resource": "*"
  	},

• Could this have a "aws:RequestTag/elbv2.k8s.aws/cluster" condition?

  aws-load-balancer-controller/docs/install/iam_policy.json

  Lines 167 to 179 in 9c79e45

  	{
  	"Effect": "Allow",
  	"Action": [
  	"elasticloadbalancing:AddTags",
  	"elasticloadbalancing:RemoveTags"
  	],
  	"Resource": [
  	"arn:aws:elasticloadbalancing:*:*:listener/net/*/*/*",
  	"arn:aws:elasticloadbalancing:*:*:listener/app/*/*/*",
  	"arn:aws:elasticloadbalancing:*:*:listener-rule/net/*/*/*",
  	"arn:aws:elasticloadbalancing:*:*:listener-rule/app/*/*/*"
  	]
  	},

• Could this have a "aws:ResourceTag/elbv2.k8s.aws/cluster" condition?

  aws-load-balancer-controller/docs/install/iam_policy.json

  Lines 199 to 206 in 9c79e45

  	{
  	"Effect": "Allow",
  	"Action": [
  	"elasticloadbalancing:RegisterTargets",
  	"elasticloadbalancing:DeregisterTargets"
  	],
  	"Resource": "arn:aws:elasticloadbalancing:*:*:targetgroup/*/*"
  	},

• Could this have a "aws:ResourceTag/elbv2.k8s.aws/cluster" condition?

  aws-load-balancer-controller/docs/install/iam_policy.json

  Lines 207 to 217 in 9c79e45

  	{
  	"Effect": "Allow",
  	"Action": [
  	"elasticloadbalancing:SetWebAcl",
  	"elasticloadbalancing:ModifyListener",
  	"elasticloadbalancing:AddListenerCertificates",
  	"elasticloadbalancing:RemoveListenerCertificates",
  	"elasticloadbalancing:ModifyRule"
  	],
  	"Resource": "*"
  	}

• I understand that this one has documentation on how to restrict it further, but why doesn't it have a "Null": {"aws:ResourceTag/elbv2.k8s.aws/cluster": "false"} condition to make it more secure by default?

  aws-load-balancer-controller/docs/install/iam_policy.json

  Lines 68 to 75 in 9c79e45

  	{
  	"Effect": "Allow",
  	"Action": [
  	"ec2:AuthorizeSecurityGroupIngress",
  	"ec2:RevokeSecurityGroupIngress"
  	],
  	"Resource": "*"
  	},

I think it would be great if the default policy packaged with this app could follow the principle of least privilege to the extent possible without affecting the out-of-the-box install experience of users. If you could broadly apply the "aws:ResourceTag" Null check conditions in the default policy, that would give users an easy path to change those tag conditions from Null check to StringEquals to really lock this down. As it stands, it's not clear if those can be safely scoped down without thorough testing.

/remove-lifecycle stale

[mike-stewart]

/reopen

[k8s-ci-robot]

@mike-stewart: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[mike-stewart]

/remove-lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[mike-stewart]

/reopen

[k8s-ci-robot]

@mike-stewart: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[mike-stewart]

/remove-lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[mike-stewart]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[mike-stewart]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[mike-stewart]

/remove-lifecycle stale