Updates AWS managed policies #878
Conversation
WalkthroughThe recent updates to various AWS IAM policies involve the addition and modification of permissions across multiple services. Notable changes include the introduction of new permission statements for the AWS Backup Full Access policy, modifications to the AWS Backup Service Role Policy, and the creation of new policies aimed at managing compromised keys. The updates also enhance permissions for AWS services like Lambda, IVS, and others, refining access controls to improve security and operational capabilities. Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
udondan
force-pushed
the
update-aws-managed-policies
branch
4 times, most recently
from
e5dd68b to
ecc2f69
Compare
udondan
force-pushed
the
update-aws-managed-policies
branch
3 times, most recently
from
1f93f67 to
55588a4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
docs/source/_static/managed-policies/AwsGlueSessionUserRestrictedNotebookPolicy.json (1)
24-37: Conditions for Tagging Correctly Implemented in Notebook Policy, Inconsistencies Found ElsewhereThe conditions for tagging in
AwsGlueSessionUserRestrictedNotebookPolicy.jsonare correctly implemented using "aws:PrincipalTag/owner". However, other policy files such asAwsGlueSessionUserRestrictedPolicy.jsonandAwsGlueSessionUserRestrictedServiceRole.jsonuse "${aws:userid}" instead, which may not align with the intended policy structure.
- Ensure consistent use of "aws:PrincipalTag/owner" across all policies if that's the intended condition.
- Review and update other policy files for consistency if required.
Analysis chain
Ensure conditions for tagging are correctly implemented.
The new "AllowGlueTaggingAction" statement allows tagging resources with strict conditions based on the
ownertag. This change enhances security by ensuring only authorized users can tag resources. Verify that the conditions are correctly implemented and that the necessary tags are applied to all relevant resources and principals.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the conditions for tagging are correctly implemented. # Test: Search for the usage of `AllowGlueTaggingAction`. Expect: Correct implementation of conditions. rg --type json $'AllowGlueTaggingAction'Length of output: 529
Script:
#!/bin/bash # Extract and display the condition sections for "AllowGlueTaggingAction" across relevant policy files rg --type json -A 10 '"Sid": "AllowGlueTaggingAction"' docs/source/_static/managed-policies/Length of output: 5125
docs/source/_static/managed-policies/AwsGlueSessionUserRestrictedNotebookServiceRole.json (1)
43-56: Inconsistent Tagging Conditions in "AllowGlueTaggingAction"The conditions for "AllowGlueTaggingAction" vary across different policy files. Ensure consistent implementation of conditions using
${aws:PrincipalTag/owner}if that is the intended approach:
AwsGlueSessionUserRestrictedServiceRole.jsonandAwsGlueSessionUserRestrictedPolicy.jsonuse${aws:userid}.AwsGlueSessionUserRestrictedNotebookServiceRole.jsonandAwsGlueSessionUserRestrictedNotebookPolicy.jsonuse${aws:PrincipalTag/owner}.Verify and standardize the conditions to align with the intended security model.
Analysis chain
Verify tagging conditions in "AllowGlueTaggingAction".
The new "AllowGlueTaggingAction" statement introduces tagging permissions with conditions based on the
ownertag. This ensures that only authorized users can tag resources. Verify that the conditions are correctly implemented and enforced across all relevant resources and principals.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify tagging conditions in "AllowGlueTaggingAction". # Test: Search for the usage of `AllowGlueTaggingAction`. Expect: Correct implementation of conditions. rg --type json $'AllowGlueTaggingAction'Length of output: 529
Script:
#!/bin/bash # Description: Extract and review the conditions associated with "AllowGlueTaggingAction" in relevant JSON files. # Extract the relevant sections from each JSON file where "AllowGlueTaggingAction" is defined. for file in $(rg --files-with-matches --type json "AllowGlueTaggingAction"); do echo "File: $file" rg -A 10 '"Sid": "AllowGlueTaggingAction"' "$file" echo "-------------------------" doneLength of output: 3223
docs/source/_static/managed-policies/AWSDataExchangeProviderFullAccess.json (2)
Restrict KMS Permissions to Necessary Resources
The AWSDataExchangeProviderFullAccess.json policy file contains multiple instances of broad permissions with "Resource": "". Specifically, the "KMSActions" SID allows key management operations on all KMS keys. To enhance security, consider scoping these permissions to specific resources rather than using "". This will help prevent unauthorized access to KMS keys.
- File:
docs/source/_static/managed-policies/AWSDataExchangeProviderFullAccess.json- Lines with broad permissions: Multiple instances of
"Resource": "*"Analysis chain
Line range hint
124-131:
Review KMSActions SID.The permissions under "KMSActions" allow for key management operations. Ensure that these permissions are limited to necessary resources to prevent unauthorized key access.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the scope of KMS permissions. # Test: Search for kms permissions usage. Expect: Limited to necessary resources. rg --type json $'kms:DescribeKey'Length of output: 8338
Script:
#!/bin/bash # Description: Check for broad resource permissions in AWSDataExchangeProviderFullAccess.json. # Search for instances of "Resource": "*" in the specified policy file. rg --type json '"Resource": "*"' docs/source/_static/managed-policies/AWSDataExchangeProviderFullAccess.jsonLength of output: 419
Review the scope of APIGatewayActions SID in AWSDataExchangeProviderFullAccess.json
The "APIGatewayActions" SID currently allows the "apigateway:GET" action on all resources ("Resource": "*"). This broad permission could lead to unintended exposure. Consider scoping this permission to specific resources to enhance security.
- File:
docs/source/_static/managed-policies/AWSDataExchangeProviderFullAccess.json, Lines: 153-159Analysis chain
Line range hint
156-161:
Review APIGatewayActions SID.The "APIGatewayActions" SID allows GET requests on API Gateway. Ensure that this permission is scoped correctly to prevent unintentional exposure.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the scope of API Gateway permissions. # Test: Search for apigateway permissions usage. Expect: Scoped correctly to prevent exposure. rg --type json $'apigateway:GET'Length of output: 3461
Script:
#!/bin/bash # Extract the full context of the "apigateway:GET" permission in AWSDataExchangeProviderFullAccess.json rg -A 5 -B 5 '"apigateway:GET"' docs/source/_static/managed-policies/AWSDataExchangeProviderFullAccess.jsonLength of output: 267
udondan
force-pushed
the
update-aws-managed-policies
branch
4 times, most recently
from
0894070 to
23c7398
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
docs/source/_static/managed-policies/AmazonECS_FullAccess.json (1)
Security Concern: Unrestricted Role Passing in
TasksPassRolePolicyThe
TasksPassRolePolicyallows theiam:PassRoleaction on all resources ("Resource": ["*"]). This can pose a security risk as it enables passing any role to ECS tasks. It is recommended to restrict this policy to specific roles necessary for the tasks to enhance security.
- File:
docs/source/_static/managed-policies/AmazonECS_FullAccess.json- Lines: 153-159
Analysis chain
Line range hint
156-164: Verify restriction and security of role passing.The
TasksPassRolePolicyallows passing roles to ECS tasks. Ensure that this is restricted to necessary roles only to maintain security.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the restriction and security of role passing in `TasksPassRolePolicy`. # Test: Check for any unintended role passing permissions. rg --type json '"TasksPassRolePolicy"'Length of output: 139
Script:
#!/bin/bash # Description: Extract detailed information about the conditions and resources in `TasksPassRolePolicy`. # Extract the full policy statement for `TasksPassRolePolicy` to verify conditions and resources. rg --type json -A 5 '"Sid": "TasksPassRolePolicy"' docs/source/_static/managed-policies/AmazonECS_FullAccess.jsonLength of output: 249
udondan
force-pushed
the
update-aws-managed-policies
branch
6 times, most recently
from
0690482 to
c42a503
Compare
| "Sid": "PermissionsToManagePCSLaunchTemplates", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "ec2:DeleteLaunchTemplate", | ||
| "ec2:DeleteLaunchTemplateVersions", | ||
| "ec2:CreateLaunchTemplateVersion" | ||
| ], | ||
| "Resource": "arn:aws:ec2:*:*:launch-template/*", | ||
| "Condition": { | ||
| "Null": { | ||
| "aws:ResourceTag/AWSPCSManaged": "false" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Refine condition to ensure explicit tag value.
The condition should ensure that the AWSPCSManaged tag is explicitly set to true to avoid unintended actions.
- "Null": {
- "aws:ResourceTag/AWSPCSManaged": "false"
- }
+ "StringEquals": {
+ "aws:ResourceTag/AWSPCSManaged": "true"
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Sid": "PermissionsToManagePCSLaunchTemplates", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:DeleteLaunchTemplate", | |
| "ec2:DeleteLaunchTemplateVersions", | |
| "ec2:CreateLaunchTemplateVersion" | |
| ], | |
| "Resource": "arn:aws:ec2:*:*:launch-template/*", | |
| "Condition": { | |
| "Null": { | |
| "aws:ResourceTag/AWSPCSManaged": "false" | |
| } | |
| } | |
| }, | |
| "Sid": "PermissionsToManagePCSLaunchTemplates", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:DeleteLaunchTemplate", | |
| "ec2:DeleteLaunchTemplateVersions", | |
| "ec2:CreateLaunchTemplateVersion" | |
| ], | |
| "Resource": "arn:aws:ec2:*:*:launch-template/*", | |
| "Condition": { | |
| "StringEquals": { | |
| "aws:ResourceTag/AWSPCSManaged": "true" | |
| } | |
| } |
| "Sid": "PermissionsToCreatePCSNetworkInterfaces", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "ec2:CreateNetworkInterface" | ||
| ], | ||
| "Resource": "arn:aws:ec2:*:*:network-interface/*", | ||
| "Condition": { | ||
| "Null": { | ||
| "aws:RequestTag/AWSPCSManaged": "false" | ||
| } | ||
| } |
There was a problem hiding this comment.
Refine condition to ensure explicit tag value.
The condition should ensure that the AWSPCSManaged tag is explicitly set to true to avoid unintended actions.
- "Null": {
- "aws:RequestTag/AWSPCSManaged": "false"
- }
+ "StringEquals": {
+ "aws:RequestTag/AWSPCSManaged": "true"
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Sid": "PermissionsToCreatePCSNetworkInterfaces", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:CreateNetworkInterface" | |
| ], | |
| "Resource": "arn:aws:ec2:*:*:network-interface/*", | |
| "Condition": { | |
| "Null": { | |
| "aws:RequestTag/AWSPCSManaged": "false" | |
| } | |
| } | |
| "Sid": "PermissionsToCreatePCSNetworkInterfaces", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:CreateNetworkInterface" | |
| ], | |
| "Resource": "arn:aws:ec2:*:*:network-interface/*", | |
| "Condition": { | |
| "StringEquals": { | |
| "aws:RequestTag/AWSPCSManaged": "true" | |
| } | |
| } |
| "Sid": "PermissionsToProvisionClusterInstances", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "ec2:RunInstances", | ||
| "ec2:CreateFleet" | ||
| ], | ||
| "Resource": [ | ||
| "arn:aws:ec2:*:*:instance/*" | ||
| ], | ||
| "Condition": { | ||
| "Null": { | ||
| "aws:RequestTag/AWSPCSManaged": "false" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Refine condition to ensure explicit tag value.
The condition should ensure that the AWSPCSManaged tag is explicitly set to true to avoid unintended actions.
- "Null": {
- "aws:RequestTag/AWSPCSManaged": "false"
- }
+ "StringEquals": {
+ "aws:RequestTag/AWSPCSManaged": "true"
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Sid": "PermissionsToProvisionClusterInstances", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:RunInstances", | |
| "ec2:CreateFleet" | |
| ], | |
| "Resource": [ | |
| "arn:aws:ec2:*:*:instance/*" | |
| ], | |
| "Condition": { | |
| "Null": { | |
| "aws:RequestTag/AWSPCSManaged": "false" | |
| } | |
| } | |
| }, | |
| "Sid": "PermissionsToProvisionClusterInstances", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:RunInstances", | |
| "ec2:CreateFleet" | |
| ], | |
| "Resource": [ | |
| "arn:aws:ec2:*:*:instance/*" | |
| ], | |
| "Condition": { | |
| "StringEquals": { | |
| "aws:RequestTag/AWSPCSManaged": "true" | |
| } | |
| } |
| "Sid": "PermissionsToCreatePCSLaunchTemplates", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "ec2:CreateLaunchTemplate" | ||
| ], | ||
| "Resource": "arn:aws:ec2:*:*:launch-template/*", | ||
| "Condition": { | ||
| "Null": { | ||
| "aws:RequestTag/AWSPCSManaged": "false" | ||
| } | ||
| } |
There was a problem hiding this comment.
Refine condition to ensure explicit tag value.
The condition should ensure that the AWSPCSManaged tag is explicitly set to true to avoid unintended actions.
- "Null": {
- "aws:RequestTag/AWSPCSManaged": "false"
- }
+ "StringEquals": {
+ "aws:RequestTag/AWSPCSManaged": "true"
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Sid": "PermissionsToCreatePCSLaunchTemplates", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:CreateLaunchTemplate" | |
| ], | |
| "Resource": "arn:aws:ec2:*:*:launch-template/*", | |
| "Condition": { | |
| "Null": { | |
| "aws:RequestTag/AWSPCSManaged": "false" | |
| } | |
| } | |
| "Sid": "PermissionsToCreatePCSLaunchTemplates", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:CreateLaunchTemplate" | |
| ], | |
| "Resource": "arn:aws:ec2:*:*:launch-template/*", | |
| "Condition": { | |
| "StringEquals": { | |
| "aws:RequestTag/AWSPCSManaged": "true" | |
| } | |
| } |
| "Sid": "PermissionsToManagePCSNetworkInterfaces", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "ec2:DeleteNetworkInterface", | ||
| "ec2:CreateNetworkInterfacePermission" | ||
| ], | ||
| "Resource": "arn:aws:ec2:*:*:network-interface/*", | ||
| "Condition": { | ||
| "Null": { | ||
| "aws:ResourceTag/AWSPCSManaged": "false" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Refine condition to ensure explicit tag value.
The condition should ensure that the AWSPCSManaged tag is explicitly set to true to avoid unintended actions.
- "Null": {
- "aws:ResourceTag/AWSPCSManaged": "false"
- }
+ "StringEquals": {
+ "aws:ResourceTag/AWSPCSManaged": "true"
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Sid": "PermissionsToManagePCSNetworkInterfaces", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:DeleteNetworkInterface", | |
| "ec2:CreateNetworkInterfacePermission" | |
| ], | |
| "Resource": "arn:aws:ec2:*:*:network-interface/*", | |
| "Condition": { | |
| "Null": { | |
| "aws:ResourceTag/AWSPCSManaged": "false" | |
| } | |
| } | |
| }, | |
| "Sid": "PermissionsToManagePCSNetworkInterfaces", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:DeleteNetworkInterface", | |
| "ec2:CreateNetworkInterfacePermission" | |
| ], | |
| "Resource": "arn:aws:ec2:*:*:network-interface/*", | |
| "Condition": { | |
| "StringEquals": { | |
| "aws:ResourceTag/AWSPCSManaged": "true" | |
| } | |
| } | |
| }, |
| "Sid": "PermissionsToTerminatePCSManagedInstances", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "ec2:TerminateInstances" | ||
| ], | ||
| "Resource": "arn:aws:ec2:*:*:instance/*", | ||
| "Condition": { | ||
| "Null": { | ||
| "aws:ResourceTag/AWSPCSManaged": "false" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Refine condition to ensure explicit tag value.
The condition should ensure that the AWSPCSManaged tag is explicitly set to true to avoid unintended actions.
- "Null": {
- "aws:ResourceTag/AWSPCSManaged": "false"
- }
+ "StringEquals": {
+ "aws:ResourceTag/AWSPCSManaged": "true"
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Sid": "PermissionsToTerminatePCSManagedInstances", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:TerminateInstances" | |
| ], | |
| "Resource": "arn:aws:ec2:*:*:instance/*", | |
| "Condition": { | |
| "Null": { | |
| "aws:ResourceTag/AWSPCSManaged": "false" | |
| } | |
| } | |
| }, | |
| "Sid": "PermissionsToTerminatePCSManagedInstances", | |
| "Effect": "Allow", | |
| "Action": [ | |
| "ec2:TerminateInstances" | |
| ], | |
| "Resource": "arn:aws:ec2:*:*:instance/*", | |
| "Condition": { | |
| "StringEquals": { | |
| "aws:ResourceTag/AWSPCSManaged": "true" | |
| } | |
| } | |
| }, |
udondan
force-pushed
the
update-aws-managed-policies
branch
3 times, most recently
from
a427e98 to
0734cc4
Compare
| /** Denies access to certain actions, applied by AWS in the event that an IAM user's credentials have been compromised or exposed publicly. The policy aims to limit the potential damage that may be caused by fraud-related activity leading to unauthorized charges, while not impacting the existing resources. Do NOT remove this policy. Instead, please follow the instructions specified in the support case created for you regarding this event. */ | ||
| public static AWSCompromisedKeyQuarantineV3 = 'AWSCompromisedKeyQuarantineV3'; |
There was a problem hiding this comment.
Security Concern: AWSCompromisedKeyQuarantineV3
The addition of the AWSCompromisedKeyQuarantineV3 policy is correctly implemented. However, the static analysis tool flagged this line as containing a potential security issue related to a generic API key. This is likely a false positive since the policy is designed to deny access to certain actions when an IAM user's credentials are compromised, which is a security feature.
It's important to verify that no sensitive keys or credentials are hardcoded in this policy or elsewhere in the codebase.
Please ensure that no sensitive information or credentials are exposed in the policy definitions or elsewhere. If this is a false positive, you may disregard the security warning from the static analysis tool.
Tools
Gitleaks
1224-1224: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
udondan
force-pushed
the
update-aws-managed-policies
branch
2 times, most recently
from
2aa1aa3 to
4459e7c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
docs/source/_static/managed-policies/AWSDataExchangeProviderFullAccess.json (2)
Line range hint
94-102: Avoid using broad permissions for S3 read actions.The new statement with SID "S3ReadActions" allows read actions like s3:ListAllMyBuckets on any S3 resource (
"Resource": "*"), which seems too permissive. Consider scoping down the Resource to specific buckets or using conditions to restrict the permissions for better security.
Line range hint
156-161: Avoid using broad permissions for API Gateway actions.The new statement with SID "APIGatewayActions" allows the apigateway:GET action on any resource (
"Resource": "*"), which seems too permissive. Consider scoping down the Resource to specific API Gateway resources or using conditions to restrict the permissions for better security.
udondan
force-pushed
the
update-aws-managed-policies
branch
from
4459e7c to
432e443
Compare
udondan
force-pushed
the
update-aws-managed-policies
branch
4 times, most recently
from
3d59c4b to
c776da3
Compare
udondan
force-pushed
the
update-aws-managed-policies
branch
7 times, most recently
from
72bfad7 to
c08e617
Compare
udondan
force-pushed
the
update-aws-managed-policies
branch
3 times, most recently
from
54bae2e to
fe0eb5f
Compare
udondan
force-pushed
the
update-aws-managed-policies
branch
6 times, most recently
from
64faca9 to
bf5a13a
Compare
udondan
force-pushed
the
update-aws-managed-policies
branch
from
bf5a13a to
1c142f5
Compare
udondan
force-pushed
the
update-aws-managed-policies
branch
from
1c142f5 to
166f47c
Compare
Updates AWS managed policies