From 3c465ebfa3deb8d5f74c7d45498037b343d60283 Mon Sep 17 00:00:00 2001 From: bcpenta Date: Wed, 30 Oct 2024 22:28:53 -0400 Subject: [PATCH 1/2] s3 bucket confused deputy attack --- packs/aws.yml | 1 + .../aws_s3_bucket_policy_confused_deputy.py | 24 ++++++++++++ .../aws_s3_bucket_policy_confused_deputy.yml | 39 +++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.py create mode 100644 policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.yml diff --git a/packs/aws.yml b/packs/aws.yml index e47135121..c4591af93 100644 --- a/packs/aws.yml +++ b/packs/aws.yml @@ -25,6 +25,7 @@ PackDefinition: - AWS.S3.Bucket.PublicWrite - AWS.S3.Bucket.SecureAccess - AWS.S3.Bucket.Versioning + - AWS.S3.Bucket.PolicyConfusedDeputyProtection # Encryption Status - AWS.EC2.EBS.Encryption.Disabled - AWS.EC2.Volume.Encryption diff --git a/policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.py b/policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.py new file mode 100644 index 000000000..39967acb1 --- /dev/null +++ b/policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.py @@ -0,0 +1,24 @@ +import json + +REQUIRED_CONDITIONS = {"aws:SourceArn", "aws:SourceAccount", "aws:SourceOrgID", "aws:SourceOrgPaths"} + +def policy(resource): + bucket_policy = resource.get("Policy") + if bucket_policy is None: + return True # Pass if there is no bucket policy + + policy_statements = json.loads(bucket_policy).get("Statement", []) + for statement in policy_statements: + # Check if the statement includes a service principal and allows access + principal = statement.get("Principal", {}) + if "Service" in principal and statement["Effect"] == "Allow": + conditions = statement.get("Condition", {}) + # Flatten nested condition keys (e.g., inside "StringEquals") + flat_condition_keys = set() + for condition in conditions.values(): + if isinstance(condition, dict): + flat_condition_keys.update(condition.keys()) + # Check if any required condition key is present + if not REQUIRED_CONDITIONS.intersection(flat_condition_keys): + return False + return True diff --git a/policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.yml b/policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.yml new file mode 100644 index 000000000..38191a865 --- /dev/null +++ b/policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.yml @@ -0,0 +1,39 @@ +AnalysisType: policy +Filename: aws_s3_bucket_policy_confused_deputy.py +PolicyID: "AWS.S3.Bucket.PolicyConfusedDeputyProtection" +DisplayName: "S3 Bucket Policy Confused Deputy Protection for Service Principals" +Enabled: true +ResourceTypes: + - AWS.S3.Bucket +Tags: + - AWS + - Security Control + - Best Practices +Severity: High +Description: > + Ensures that S3 bucket policies with service principals include conditions to prevent the confused deputy problem. +Runbook: > + Update the bucket policy to include conditions such as aws:SourceArn, aws:SourceAccount, + aws:SourceOrgID, or aws:SourceOrgPaths when a service principal is specified. +Reference: https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html +Tests: + - Name: Compliant Policy with Service Principal and Condition + ExpectedResult: true + Resource: + { + "Policy": '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Service":"cloudtrail.amazonaws.com"},"Action":"s3:PutObject","Resource":"arn:aws:s3:::my-example-bucket/*","Condition":{"StringEquals":{"aws:SourceAccount":"123456789012"}}}]}' + } + + - Name: Non-Compliant Policy with Service Principal and No Condition + ExpectedResult: false + Resource: + { + "Policy": '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Service":"cloudtrail.amazonaws.com"},"Action":"s3:PutObject","Resource":"arn:aws:s3:::my-example-bucket/*"}]}' + } + + - Name: Policy without Service Principal + ExpectedResult: true + Resource: + { + "Policy": '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"AWS":"arn:aws:iam::123456789012:root"},"Action":"s3:GetObject","Resource":"arn:aws:s3:::my-example-bucket/*"}]}' + } From b5f6f9e7a06b3c930f451d8a323e5cfa095996b4 Mon Sep 17 00:00:00 2001 From: Ariel Date: Tue, 12 Nov 2024 08:37:54 -0700 Subject: [PATCH 2/2] make fmt --- .../aws_s3_bucket_policy_confused_deputy.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.py b/policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.py index 39967acb1..c1e552f94 100644 --- a/policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.py +++ b/policies/aws_s3_policies/aws_s3_bucket_policy_confused_deputy.py @@ -1,6 +1,12 @@ import json -REQUIRED_CONDITIONS = {"aws:SourceArn", "aws:SourceAccount", "aws:SourceOrgID", "aws:SourceOrgPaths"} +REQUIRED_CONDITIONS = { + "aws:SourceArn", + "aws:SourceAccount", + "aws:SourceOrgID", + "aws:SourceOrgPaths", +} + def policy(resource): bucket_policy = resource.get("Policy")