-
Couldn't load subscription status.
- Fork 4
🔒 Improve AWS Cloud IAM policy security #839
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,11 +70,21 @@ The following permissions are required to provision resources in the AWS account | |
| "iam:CreateInstanceProfile", | ||
| "iam:AttachRolePolicy", | ||
| "iam:PutRolePolicy", | ||
| "iam:PassRole", | ||
| "iam:TagRole", | ||
| "iam:TagInstanceProfile" | ||
| ], | ||
| "Resource": "*" | ||
| "Resource": [ | ||
| "arn:aws:iam::*:role/TowerForge*", | ||
| "arn:aws:iam::*:instance-profile/TowerForge*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AwsCloudCreatePassRole", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "iam:PassRole" | ||
| ], | ||
| "Resource": "arn:aws:iam::*:role/TowerForge*" | ||
| } | ||
| ] | ||
| } | ||
|
|
@@ -111,15 +121,49 @@ The following permissions are required to launch pipelines, run Studio sessions, | |
| "Version": "2012-10-17", | ||
| "Statement": [ | ||
| { | ||
| "Sid": "AwsCloudLaunch", | ||
| "Sid": "AwsCloudLaunchEC2", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "ec2:RunInstances", | ||
| "ec2:DescribeInstances", | ||
| "ec2:CreateTags", | ||
| "ec2:TerminateInstances", | ||
| "ec2:DeleteTags", | ||
| "logs:GetLogEvents", | ||
| "ec2:DeleteTags" | ||
| ], | ||
| "Resource": [ | ||
| "arn:aws:ec2:*:*:instance/*", | ||
| "arn:aws:ec2:*:*:volume/*", | ||
| "arn:aws:ec2:*:*:network-interface/*", | ||
| "arn:aws:ec2:*:*:subnet/*", | ||
| "arn:aws:ec2:*:*:security-group/*", | ||
| "arn:aws:ec2:*:*:key-pair/*", | ||
| "arn:aws:ec2:*:*:image/*" | ||
| ] | ||
| }, | ||
| { | ||
| "Sid": "AwsCloudLaunchInstances", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "ec2:DescribeInstances", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you test this? AWS Docs say that using the |
||
| "ec2:TerminateInstances" | ||
| ], | ||
| "Resource": "*", | ||
| "Condition": { | ||
| "StringLike": { | ||
| "ec2:ResourceTag/Name": "TowerForge-*" | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| "Sid": "AwsCloudLaunchLogs", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "logs:GetLogEvents" | ||
| ], | ||
| "Resource": "arn:aws:logs:*:*:log-group:*:log-stream:*" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above, this does not seem to move the needle much in terms of actual permission scopes. However here we could be more specific, since we know the exact log group we're interested into, which is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great. Please implement. |
||
| }, | ||
| { | ||
| "Sid": "AwsCloudLaunchS3", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "s3:GetObject" | ||
| ], | ||
| "Resource": "*" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leaving the star here is inconsistent with the general approach taken of specify arns, even if the actual scoping outcome is the same. I would either do it everywhere or nowhere There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. |
||
|
|
@@ -149,7 +193,10 @@ The following permissions are required to remove resources created by Seqera whe | |
| "iam:DetachRolePolicy", | ||
| "iam:DeleteRolePolicy" | ||
| ], | ||
| "Resource": "*" | ||
| "Resource": [ | ||
| "arn:aws:iam::*:role/TowerForge*", | ||
| "arn:aws:iam::*:instance-profile/TowerForge*" | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really any different than using
*? Actions are anyhow not applicable to all resource types, so defining individual resource types still with a star permission doesn't seem to change much the scope of the actual permissions.For example, with the current config I can't use
ec2:RunInstanceson a Kinesis stream, because the action already carries the scope of what resource type it can be taken against.On the other hand I get this might have a placebo effect if someone reads it through without giving it much thought, it seems more scoped.