[WIP] Tighter IAM policies #612
Conversation
Signed-off-by: Naadir Jeewa <[email protected]>
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: randomvariable The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
EC2 and ELB instances are tagged upon creation. Therefore, ensuring that only EC2/ELB instances are deleted when they match the Cluster API Provider AWS tag minimises privileges. This approach cannot be used for other resources as they are tagged after creation. Signed-off-by: Naadir Jeewa <[email protected]>
randomvariable
force-pushed
the
tighter-iam
branch
from
b42ad08 to
0b27511
Compare
k8s-ci-robot
added
size/L
Signed-off-by: Naadir Jeewa <[email protected]>
randomvariable
force-pushed
the
tighter-iam
branch
from
8afffbe to
2a1bdaa
Compare
k8s-ci-robot
added
size/M
Signed-off-by: Naadir Jeewa <[email protected]>
Signed-off-by: Naadir Jeewa <[email protected]>
k8s-ci-robot
added
size/L
k8s-ci-robot
added
the
do-not-merge/work-in-progress
randomvariable
force-pushed
the
tighter-iam
branch
from
0382c53 to
dcd9101
Compare
|
Believe this preserves the intent of the current controllers now. The main advantage with this PR is that EC2 & ELB instance modification and deletions of things which do not belong to Cluster API should be blocked. Until AWS supports tagging on creation for the other objects, ability to limit the scope of actions to only cluster API managed objects will be limited. Now need to add e2e to this. |
randomvariable
force-pushed
the
tighter-iam
branch
from
54e732d to
5f81c73
Compare
Signed-off-by: Naadir Jeewa <[email protected]>
randomvariable
force-pushed
the
tighter-iam
branch
from
5f81c73 to
829afc0
Compare
|
TODO: Restrict |
I'm actually wondering if it would make sense to leverage InstanceProfiles when a MachineClass is in use? |
Just want to check. InstanceProfile is what EC2 uses to reference IAM roles for machines, I would have thought it's Launch Templates which is the analog to |
Hah, that's what I get for commenting during the first half of the day. Launch Templates was indeed what I was thinking of. |
|
|
randomvariable
force-pushed
the
tighter-iam
branch
from
9cb65cb to
fd05fc9
Compare
In order to restrict privilege escalation via instance profile, all resources are now put under a `sigs.k8s.io/cluster-api-provider-aws` path so this can be referenced in a policy condition. Unfortunately, this breaks CloudFormation being able to do an in-place update. There doesn't seem to be a way around this, so have also taken the opportunity to rename two incorrectly named resources. Signed-off-by: Naadir Jeewa <[email protected]>
randomvariable
force-pushed
the
tighter-iam
branch
from
fd05fc9 to
3f3969b
Compare
|
/milestone v1alpha1 |
|
@vincepri this is actually |
|
@randomvariable: You must be a member of the kubernetes-sigs/cluster-api-provider-aws-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Cluster API Provider AWS Maintainers and have them propose you as an additional delegate for this responsibility. In response to this:
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. |
|
/milestone Next |
|
Closing this for now, as can't actively work on it at the moment. |
What this PR does / why we need it:
EC2 and ELB instances are tagged upon creation. Therefore, ensuring that only EC2/ELB instances are deleted when they match the Cluster API Provider AWS tag minimises privileges.
This approach cannot be used for other resources as they are tagged after creation - backing out of a failed tagging may prevent deletion, and controller can tag a resource appropriately to delete it.
Also additionally constrain which tags are allowed to be used by the controller.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #608
Special notes for your reviewer:
In order to restrict privilege escalation via instance profile, all resources are now put under a
sigs.k8s.io/cluster-api-provider-awspath so this can be referenced in a policy condition. Unfortunately, this breaks CloudFormation being able to do an in-place update. There doesn't seem to be a way around this, so have also taken the opportunity to rename two incorrectly named resources. These are in the release notes below.Recommend this is not merged prior to e2e tests making use of CloudFormation with a delete process.
Current example policy:
{ "Version": "2012-10-17", "Statement": [ { "Condition": { "Bool": { "aws:SecureTransport": [ "true" ] } }, "Action": [ "ec2:AllocateAddress", "ec2:AssociateRouteTable", "ec2:AttachInternetGateway", "ec2:AuthorizeSecurityGroupIngress", "ec2:CreateInternetGateway", "ec2:CreateNatGateway", "ec2:CreateRoute", "ec2:CreateRouteTable", "ec2:CreateSecurityGroup", "ec2:CreateSubnet", "ec2:CreateVpc", "ec2:DeleteInternetGateway", "ec2:DeleteNatGateway", "ec2:DeleteRouteTable", "ec2:DeleteSecurityGroup", "ec2:DeleteSubnet", "ec2:DeleteVpc", "ec2:DescribeAddresses", "ec2:DescribeAvailabilityZones", "ec2:DescribeInternetGateways", "ec2:DescribeImages", "ec2:DescribeNatGateways", "ec2:DescribeRouteTables", "ec2:DescribeSecurityGroups", "ec2:DescribeSubnets", "ec2:DescribeVpcs", "ec2:DetachInternetGateway", "ec2:DisassociateRouteTable", "ec2:ModifySubnetAttribute", "ec2:ReleaseAddress", "ec2:RevokeSecurityGroupIngress" ], "Resource": [ "*" ], "Effect": "Allow" }, { "Condition": { "StringLike": { "iam:AWSServiceName": "elasticloadbalancing.amazonaws.com" }, "Bool": { "aws:SecureTransport": [ "true" ] } }, "Action": [ "iam:CreateServiceLinkedRole" ], "Resource": [ "arn:aws:iam::xxxxxxxx:role/aws-service-role/elasticloadbalancing.amazonaws.com/AWSServiceRoleForElasticLoadBalancing" ], "Effect": "Allow" }, { "Condition": { "StringEquals": { "aws:RequestTag/sigs.k8s.io/cluster-api-provider-aws/managed": [ "true" ], "aws:RequestTag/sigs.k8s.io/cluster-api-provider-aws/role": [ "apiserver", "bastion", "common", "private", "public" ] }, "Bool": { "aws:SecureTransport": [ "true" ] }, "ForAnyValue:StringLike": { "aws:TagKeys": [ "kubernetes.io/cluster/*", "sigs.k8s.io/cluster-api-provider-aws/managed", "sigs.k8s.io/cluster-api-provider-aws/role", "Name" ] } }, "Action": [ "ec2:CreateTags", "ec2:RunInstances", "elasticloadbalancing:CreateLoadBalancer" ], "Resource": [ "*" ], "Effect": "Allow" }, { "Condition": { "StringEquals": { "aws:RequestTag/sigs.k8s.io/cluster-api-provider-aws/managed": [ "true" ], "aws:RequestTag/sigs.k8s.io/cluster-api-provider-aws/role": [ "apiserver", "bastion", "common", "private", "public" ] }, "StringLike": { "ec2:InstanceProfile": "arn:aws:iam::xxxxxxxx:instance-profile/sigs.k8s.io/cluster-api-provider-aws/*" }, "Bool": { "aws:SecureTransport": [ "true" ] }, "ForAnyValue:StringLike": { "aws:TagKeys": [ "kubernetes.io/cluster/*", "sigs.k8s.io/cluster-api-provider-aws/managed", "sigs.k8s.io/cluster-api-provider-aws/role", "Name" ] } }, "Action": [ "ec2:RunInstances" ], "Resource": [ "*" ], "Effect": "Allow" }, { "Condition": { "StringEquals": { "aws:RequestTag/sigs.k8s.io/cluster-api-provider-aws/managed": [ "true" ], "aws:RequestTag/sigs.k8s.io/cluster-api-provider-aws/role": [ "apiserver", "bastion", "common", "private", "public" ], "ec2:ResourceTag/sigs.k8s.io/cluster-api-provider-aws/managed": [ "true" ] }, "Bool": { "aws:SecureTransport": [ "true" ] }, "ForAnyValue:StringLike": { "aws:TagKeys": [ "kubernetes.io/cluster/*", "sigs.k8s.io/cluster-api-provider-aws/managed", "sigs.k8s.io/cluster-api-provider-aws/role", "Name" ] } }, "Action": [ "ec2:CreateTags" ], "Resource": [ "arn:aws:ec2:::instance/*" ], "Effect": "Allow" }, { "Condition": { "StringEquals": { "ec2:ResourceTag/sigs.k8s.io/cluster-api-provider-aws/managed": [ "true" ] }, "Bool": { "aws:SecureTransport": [ "true" ] } }, "Action": [ "ec2:TerminateInstances", "ec2:DescribeInstances" ], "Resource": [ "*" ], "Effect": "Allow" }, { "Condition": { "StringEquals": { "elasticloadbalancing:ResourceTag/sigs.k8s.io/cluster-api-provider-aws/managed": [ "true" ] }, "Bool": { "aws:SecureTransport": [ "true" ] } }, "Action": [ "elasticloadbalancing:DeleteLoadBalancer", "elasticloadbalancing:ConfigureHealthCheck", "elasticloadbalancing:DescribeLoadBalancers", "elasticloadbalancing:DescribeLoadBalancerAttributes", "elasticloadbalancing:ModifyLoadBalancerAttributes", "elasticloadbalancing:RegisterInstancesWithLoadBalancer" ], "Resource": [ "*" ], "Effect": "Allow" }, { "Condition": { "Bool": { "aws:SecureTransport": [ "true" ] } }, "Action": [ "iam:PassRole" ], "Resource": [ "arn:aws:iam::xxxxxxxx:role/sigs.k8s.io/cluster-api-provider-aws/*" ], "Effect": "Allow" } ] }/hold
Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
Release note: