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

Updates AWS managed policies #878

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"ecs:UpdateService",
"cloudwatch:PutMetricAlarm",
"cloudwatch:DescribeAlarms",
"cloudwatch:GetMetricData",
"cloudwatch:DeleteAlarms"
],
"Resource": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"bedrock:GetModelInvocationLoggingConfiguration",
"bedrock:ListCustomModels",
"bedrock:ListFoundationModels",
"bedrock:ListGuardrails",
"bedrock:ListModelCustomizationJobs",
"cloudfront:GetDistribution",
"cloudfront:GetDistributionConfig",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,18 @@
"Effect": "Allow",
"Action": [
"storagegateway:DescribeGatewayInformation",
"storagegateway:ListVolumes",
"storagegateway:ListLocalDisks"
],
"Resource": "arn:aws:storagegateway:*:*:gateway/*"
},
{
"Sid": "StorageGatewayGatewayStarPermissions",
"Effect": "Allow",
"Action": [
"storagegateway:ListVolumes"
],
"Resource": "*"
},
{
"Sid": "IamRolePermissions",
"Effect": "Allow",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,10 @@
"Resource": "*"
},
{
"Sid": "RDSModifyPermissions",
"Sid": "RDSInstanceAutomatedBackupPermissions",
"Effect": "Allow",
"Action": [
"rds:ModifyDBInstance"
],
"Resource": [
"arn:aws:rds:*:*:db:*"
]
"Action": "rds:DeleteDBInstanceAutomatedBackup",
"Resource": "arn:aws:rds:*:*:auto-backup:*"
Comment on lines +41 to +44
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Action Required: Update or Remove Deprecated RDS Permissions

The old permission rds:ModifyDBInstance is still present in multiple policy files. Please review and update or remove this permission as necessary to ensure consistency and security across all policies.

  • AWSElasticBeanstalkRoleRDS.json
  • AWSBackupServiceRolePolicyForBackup.json
  • NeptuneConsoleFullAccess.json
  • NeptuneFullAccess.json
  • AmazonDocDBFullAccess.json
  • AmazonDocDBConsoleFullAccess.json
  • AdministratorAccess-AWSElasticBeanstalk.json
🔗 Analysis chain

LGTM. Verify the impact of this permission change.

The change from modifying DB instances to deleting automated backups is appropriate and aligns well with the new Sid name. This update provides more specific and targeted permissions for managing RDS automated backups.

To ensure this change doesn't negatively impact existing workflows, please run the following verification:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any existing scripts or CloudFormation templates that might be using the old permission
rg "rds:ModifyDBInstance" --type yaml --type json

Length of output: 763

},
{
"Sid": "RDSClusterPermissions",
Expand All @@ -60,10 +56,18 @@
{
"Sid": "RDSClusterBackupPermissions",
"Effect": "Allow",
"Action": "rds:DeleteDBClusterAutomatedBackup",
"Resource": "arn:aws:rds:*:*:cluster-auto-backup:*"
},
{
"Sid": "RDSModifyPermissions",
"Effect": "Allow",
"Action": [
"rds:DeleteDBClusterAutomatedBackup"
"rds:ModifyDBInstance"
],
"Resource": "arn:aws:rds:*:*:cluster-auto-backup:*"
"Resource": [
"arn:aws:rds:*:*:db:*"
]
},
{
"Sid": "RDSBackupPermissions",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,20 @@
"invoicing:GetInvoiceEmailDeliveryPreferences",
"invoicing:GetInvoicePDF",
"invoicing:ListInvoiceSummaries",
"payments:GetFinancingApplication",
"payments:GetFinancingLine",
"payments:GetFinancingLineWithdrawal",
"payments:GetFinancingOption",
"payments:GetPaymentInstrument",
"payments:GetPaymentStatus",
"payments:ListFinancingApplications",
"payments:ListFinancingLines",
"payments:ListFinancingLineWithdrawals",
"payments:ListPaymentInstruments",
"payments:ListPaymentPreferences",
"payments:ListPaymentProgramOptions",
"payments:ListPaymentProgramStatus",
"payments:ListTagsForResource",
"payments:ListPaymentInstruments",
"purchase-orders:GetPurchaseOrder",
"purchase-orders:ViewPurchaseOrders",
"purchase-orders:ListPurchaseOrderInvoices",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "EC2Action1",
"Effect": "Allow",
"Action": [
"ec2:CreateNetworkInterface"
],
"Condition": {
"StringEquals": {
"aws:RequestTag/aws.cloudfront.vpcorigin": "enabled"
}
},
"Resource": "arn:aws:ec2:*:*:network-interface/*"
},
{
"Sid": "EC2Action2",
"Effect": "Allow",
"Action": [
"ec2:CreateNetworkInterface"
],
"Resource": [
"arn:aws:ec2:*:*:subnet/*",
"arn:aws:ec2:*:*:security-group/*"
]
},
{
"Sid": "EC2Action3",
"Effect": "Allow",
"Action": [
"ec2:CreateSecurityGroup"
],
"Condition": {
"StringEquals": {
"aws:RequestTag/aws.cloudfront.vpcorigin": "enabled"
}
},
"Resource": [
"arn:aws:ec2:*:*:security-group/*"
]
},
{
"Sid": "EC2Action4",
"Effect": "Allow",
"Action": [
"ec2:CreateSecurityGroup"
],
"Resource": [
"arn:aws:ec2:*:*:vpc/*"
]
},
{
"Sid": "EC2Action5",
"Effect": "Allow",
"Action": [
"ec2:ModifyNetworkInterfaceAttribute",
"ec2:DeleteNetworkInterface",
"ec2:DeleteSecurityGroup",
"ec2:AssignIpv6Addresses",
"ec2:UnassignIpv6Addresses"
],
"Condition": {
"StringEquals": {
"aws:ResourceTag/aws.cloudfront.vpcorigin": "enabled"
}
},
"Resource": "*"
},
Comment on lines +53 to +69
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using more specific resource ARNs

While the tag condition provides good control, the Resource field using "*" could be more specific. Consider listing explicit ARNs for each resource type being modified:

       ],
       "Resource": [
-        "*"
+        "arn:aws:ec2:*:*:network-interface/*",
+        "arn:aws:ec2:*:*:security-group/*"
       ]
📝 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.

Suggested change
{
"Sid": "EC2Action5",
"Effect": "Allow",
"Action": [
"ec2:ModifyNetworkInterfaceAttribute",
"ec2:DeleteNetworkInterface",
"ec2:DeleteSecurityGroup",
"ec2:AssignIpv6Addresses",
"ec2:UnassignIpv6Addresses"
],
"Condition": {
"StringEquals": {
"aws:ResourceTag/aws.cloudfront.vpcorigin": "enabled"
}
},
"Resource": "*"
},
{
"Sid": "EC2Action5",
"Effect": "Allow",
"Action": [
"ec2:ModifyNetworkInterfaceAttribute",
"ec2:DeleteNetworkInterface",
"ec2:DeleteSecurityGroup",
"ec2:AssignIpv6Addresses",
"ec2:UnassignIpv6Addresses"
],
"Condition": {
"StringEquals": {
"aws:ResourceTag/aws.cloudfront.vpcorigin": "enabled"
}
},
"Resource": [
"arn:aws:ec2:*:*:network-interface/*",
"arn:aws:ec2:*:*:security-group/*"
]
},

{
"Sid": "EC2Action6",
"Effect": "Allow",
"Action": [
"ec2:DescribeNetworkInterfaces",
"ec2:DescribeSecurityGroups",
"ec2:DescribeInstances",
"ec2:DescribeInternetGateways",
"ec2:DescribeSubnets",
"ec2:DescribeRegions",
"ec2:DescribeAddresses"
],
"Resource": "*"
},
{
"Sid": "EC2Action7",
"Effect": "Allow",
"Action": "ec2:CreateTags",
"Condition": {
"StringEquals": {
"aws:RequestTag/aws.cloudfront.vpcorigin": "enabled",
"ec2:CreateAction": [
"CreateNetworkInterface",
"CreateSecurityGroup"
]
}
},
"Resource": [
"arn:aws:ec2:*:*:security-group/*",
"arn:aws:ec2:*:*:network-interface/*"
]
},
{
"Sid": "ElbAction1",
"Effect": "Allow",
"Action": [
"elasticloadbalancing:DescribeLoadBalancers",
"elasticloadbalancing:DescribeListeners",
"elasticloadbalancing:DescribeTargetGroups"
],
"Resource": "*"
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,36 @@
"ec2:PurchaseReservedInstancesOffering",
"ec2:AcceptReservedInstancesExchangeQuote",
"ec2:CreateReservedInstancesListing",
"savingsplans:CreateSavingsPlan"
"savingsplans:CreateSavingsPlan",
"ecs:CreateService",
"ecs:CreateCluster",
"ecs:RegisterTaskDefinition",
"ecr:GetAuthorizationToken",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Consider allowing read-only ECR, Lambda, and SNS actions

While most additions are appropriate, denying these read-only actions might hinder incident investigation:

  • ecr:GetAuthorizationToken - needed to audit container images
  • lambda:GetEventSourceMapping - helps trace event flows
  • sns:GetSMSAttributes - aids in communication audit

Consider removing these read-only actions from the deny list to facilitate incident response, unless there's a specific security concern that justifies their inclusion.

Also applies to: 93-94

Comment on lines +67 to +70
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

LGTM for ECS actions, reconsider ECR authentication

The ECS actions appropriately prevent the creation of new container resources. However, denying ecr:GetAuthorizationToken might hinder incident investigation by preventing security teams from auditing container images.

Consider removing ecr:GetAuthorizationToken from the deny list to facilitate incident response, unless there's a specific security concern that justifies its inclusion.

"bedrock:CreateModelInvocationJob",
"bedrock:InvokeModelWithResponseStream",
"bedrock:CreateFoundationModelAgreement",
"bedrock:PutFoundationModelEntitlement",
"bedrock:InvokeModel",
Comment on lines +71 to +75
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider additional Bedrock restrictions

While denying model creation and invocation is good, consider also denying:

  • bedrock:CreateCustomModel - prevents creation of custom models that could be used maliciously
  • bedrock:CreateProvisionedModel - prevents resource-intensive provisioned models
  • bedrock:CreateModelCustomizationJob - prevents model customization that could be exploited

"s3:CreateBucket",
"s3:PutBucketCors",
"s3:GetObject",
"s3:ListBucket",
"sagemaker:CreateEndpointConfig",
"sagemaker:CreateProcessingJob",
Comment on lines +76 to +81
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

LGTM for most additions, but some S3 actions need reconsideration

The additions for S3 and SageMaker are generally appropriate:

  • Preventing the creation of S3 buckets and modifying CORS settings limits potential data exfiltration routes.
  • Blocking SageMaker endpoint creation and processing jobs prevents abuse of ML resources.

However, the recent analysis shows that "s3:GetObject" and "s3:ListBucket" are extensively used across many managed policies. Denying these actions in this policy might hinder legitimate administrative and incident response tasks.

Suggestion: Remove "s3:GetObject" and "s3:ListBucket" from the deny list to facilitate necessary operations, unless there is a specific security concern that justifies their inclusion.

🔗 Analysis chain

LGTM for most additions, reconsider some S3 actions

The additions for S3 and SageMaker are generally appropriate:

  • Preventing the creation of S3 buckets and modifying CORS settings limits potential data exfiltration routes.
  • Blocking SageMaker endpoint creation and processing jobs prevents abuse of ML resources.

However, consider the following:

  • Denying "s3:GetObject" and "s3:ListBucket" might hinder legitimate administrative tasks during incident response. These read-only operations could be useful for security teams investigating the compromise.

Suggestion: Remove "s3:GetObject" and "s3:ListBucket" from the deny list to facilitate incident response activities, unless there's a specific security concern that justifies their inclusion.

To verify the potential impact, you can run the following command to check if these S3 actions are commonly used in other IAM policies:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for S3 read actions in other policies
rg -i 's3:(GetObject|ListBucket)' docs/source/_static/managed-policies/ --type json

Length of output: 4190


Script:

#!/bin/bash
# Check for S3 read actions in other policies
rg -i 's3:(GetObject|ListBucket)' docs/source/_static/managed-policies/ --type json

Length of output: 47682

"ses:GetSendQuota",
"ses:ListIdentities",
"sts:GetSessionToken",
"sts:GetFederationToken",
Comment on lines +82 to +85
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

LGTM for STS actions, reconsider SES read actions

The addition of STS actions (GetSessionToken, GetFederationToken) is appropriate for a compromised key quarantine policy, as they prevent obtaining new credentials that could be misused.

However, the SES actions (GetSendQuota, ListIdentities) are read-only operations that don't pose a direct security risk. Denying these might hinder legitimate administrative tasks or incident response efforts.

Consider removing "ses:GetSendQuota" and "ses:ListIdentities" from the deny list, as they are read-only actions that could be useful for investigating the extent of a potential compromise.

"amplify:CreateDeployment",
"amplify:CreateBackendEnvironment",
"codebuild:CreateProject",
"glue:CreateJob",
"iam:DeleteRole",
"iam:DeleteAccessKey",
"iam:ListUsers",
Comment on lines +86 to +92
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

LGTM for most additions, reconsider IAM:ListUsers

The additions for Amplify, CodeBuild, Glue, and most IAM actions are appropriate:

  • Amplify, CodeBuild, and Glue actions prevent creating new resources.
  • IAM DeleteRole and DeleteAccessKey prevent destructive actions on access controls.

However, including iam:ListUsers in the deny list might hinder legitimate administrative tasks during incident response. Consider removing this action from the deny list, as it's a read-only operation that could be useful for security teams investigating the compromise.

Suggest removing iam:ListUsers from the deny list to facilitate incident response activities.

"lambda:GetEventSourceMapping",
"sns:GetSMSAttributes",
"mediapackagev2:CreateChannel"
Comment on lines +93 to +95
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Approve MediaPackage action, query Lambda and SNS inclusions

The addition of mediapackagev2:CreateChannel is appropriate as it prevents the creation of new resources, aligning with the quarantine policy's purpose.

However, the inclusion of lambda:GetEventSourceMapping and sns:GetSMSAttributes raises questions:

  1. These are read-only actions that don't modify resources or escalate privileges.
  2. Denying these actions might hinder incident response efforts by restricting access to potentially useful information.

Consider removing these read-only actions from the deny list unless there's a specific security concern that justifies their inclusion.

Recommend removing lambda:GetEventSourceMapping and sns:GetSMSAttributes from the deny list unless there's a compelling reason to include them.

Comment on lines +66 to +95
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding these commonly abused actions

While the current deny list is comprehensive, consider adding these critical actions:

  • cloudformation:CreateStack - prevents infrastructure deployment
  • iam:CreateServiceLinkedRole - prevents privilege escalation via service-linked roles
  • organizations:LeaveOrganization - prevents account isolation
  • organizations:DeleteOrganization - prevents organizational dismantling
  • kms:ScheduleKeyDeletion - prevents crypto key deletion
  • route53:DeleteHostedZone - prevents DNS hijacking

],
"Resource": [
"*"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Deny",
"Action": [
"cloudtrail:LookupEvents",
"ec2:RequestSpotInstances",
"ec2:RunInstances",
"ec2:StartInstances",
"iam:AddUserToGroup",
"iam:AttachGroupPolicy",
"iam:AttachRolePolicy",
"iam:AttachUserPolicy",
"iam:ChangePassword",
"iam:CreateAccessKey",
"iam:CreateInstanceProfile",
"iam:CreateLoginProfile",
"iam:CreatePolicyVersion",
"iam:CreateRole",
"iam:CreateUser",
"iam:DetachUserPolicy",
"iam:PassRole",
"iam:PutGroupPolicy",
"iam:PutRolePolicy",
"iam:PutUserPermissionsBoundary",
"iam:PutUserPolicy",
"iam:SetDefaultPolicyVersion",
"iam:UpdateAccessKey",
"iam:UpdateAccountPasswordPolicy",
"iam:UpdateAssumeRolePolicy",
"iam:UpdateLoginProfile",
"iam:UpdateUser",
"lambda:AddLayerVersionPermission",
"lambda:AddPermission",
"lambda:CreateFunction",
"lambda:GetPolicy",
"lambda:ListTags",
"lambda:PutProvisionedConcurrencyConfig",
"lambda:TagResource",
"lambda:UntagResource",
"lambda:UpdateFunctionCode",
"lightsail:Create*",
"lightsail:Delete*",
"lightsail:DownloadDefaultKeyPair",
"lightsail:GetInstanceAccessDetails",
"lightsail:Start*",
"lightsail:Update*",
"organizations:CreateAccount",
"organizations:CreateOrganization",
"organizations:InviteAccountToOrganization",
"s3:DeleteBucket",
"s3:DeleteObject",
"s3:DeleteObjectVersion",
"s3:PutLifecycleConfiguration",
"s3:PutBucketAcl",
"s3:PutBucketOwnershipControls",
"s3:DeleteBucketPolicy",
"s3:ObjectOwnerOverrideToBucketOwner",
"s3:PutAccountPublicAccessBlock",
"s3:PutBucketPolicy",
"s3:ListAllMyBuckets",
"ec2:PurchaseReservedInstancesOffering",
"ec2:AcceptReservedInstancesExchangeQuote",
"ec2:CreateReservedInstancesListing",
"savingsplans:CreateSavingsPlan",
"ecs:CreateService",
"ecs:CreateCluster",
"ecs:RegisterTaskDefinition",
"ecr:GetAuthorizationToken",
"bedrock:CreateModelInvocationJob",
"bedrock:InvokeModelWithResponseStream",
"bedrock:CreateFoundationModelAgreement",
"bedrock:PutFoundationModelEntitlement",
"bedrock:InvokeModel",
"s3:CreateBucket",
"s3:PutBucketCors",
"s3:GetObject",
"s3:ListBucket",
"sagemaker:CreateEndpointConfig",
"sagemaker:CreateProcessingJob",
"ses:GetSendQuota",
"ses:ListIdentities",
"sts:GetSessionToken",
"sts:GetFederationToken",
"amplify:CreateDeployment",
"amplify:CreateBackendEnvironment",
"codebuild:CreateProject",
"glue:CreateJob",
"iam:DeleteRole",
"iam:DeleteAccessKey",
"iam:ListUsers",
"lambda:GetEventSourceMapping",
"sns:GetSMSAttributes",
"mediapackagev2:CreateChannel"
],
"Resource": [
"*"
]
}
]
}
Loading
Loading