From 36e095c830b902d26bae5abf44df4bf2b7a4455d Mon Sep 17 00:00:00 2001 From: Sergio Garcia <38561120+sergargar@users.noreply.github.com> Date: Wed, 9 Aug 2023 10:41:48 +0200 Subject: [PATCH] fix(iam_role_cross_service_confused_deputy_prevention): add ResourceAccount and PrincipalAccount conditions (#2689) --- .../policy_condition_parser.py | 9 +- ...ross_service_confused_deputy_prevention.py | 45 +------- .../policy_condition_parser_test.py | 106 ++++++++++++++++++ ...service_confused_deputy_prevention_test.py | 100 +++++++++++++++++ 4 files changed, 219 insertions(+), 41 deletions(-) diff --git a/prowler/providers/aws/lib/policy_condition_parser/policy_condition_parser.py b/prowler/providers/aws/lib/policy_condition_parser/policy_condition_parser.py index 6c4d1956..f7ec103c 100644 --- a/prowler/providers/aws/lib/policy_condition_parser/policy_condition_parser.py +++ b/prowler/providers/aws/lib/policy_condition_parser/policy_condition_parser.py @@ -8,8 +8,15 @@ def is_account_only_allowed_in_condition( "aws:SourceAccount", "s3:ResourceAccount", "aws:PrincipalAccount", + "aws:ResourceAccount", + ], + "StringLike": [ + "aws:SourceAccount", + "aws:SourceArn", + "aws:PrincipalArn", + "aws:ResourceAccount", + "aws:PrincipalAccount", ], - "StringLike": ["aws:SourceArn", "aws:PrincipalArn"], "ArnLike": ["aws:SourceArn", "aws:PrincipalArn"], "ArnEquals": ["aws:SourceArn", "aws:PrincipalArn"], } diff --git a/prowler/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention.py b/prowler/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention.py index 053910eb..9b6f2d51 100644 --- a/prowler/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention.py +++ b/prowler/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention.py @@ -1,4 +1,7 @@ from prowler.lib.check.models import Check, Check_Report_AWS +from prowler.providers.aws.lib.policy_condition_parser.policy_condition_parser import ( + is_account_only_allowed_in_condition, +) from prowler.providers.aws.services.iam.iam_client import iam_client @@ -27,46 +30,8 @@ class iam_role_cross_service_confused_deputy_prevention(Check): and "Service" in statement["Principal"] # Check to see if the appropriate condition statements have been implemented and "Condition" in statement - and ( - ( - "StringEquals" in statement["Condition"] - and "aws:SourceAccount" - in statement["Condition"]["StringEquals"] - and iam_client.audited_account - in str( - statement["Condition"]["StringEquals"][ - "aws:SourceAccount" - ] - ) - ) - or ( - "StringLike" in statement["Condition"] - and "aws:SourceAccount" - in statement["Condition"]["StringLike"] - and iam_client.audited_account - in str( - statement["Condition"]["StringLike"][ - "aws:SourceAccount" - ] - ) - ) - or ( - "ArnEquals" in statement["Condition"] - and "aws:SourceArn" - in statement["Condition"]["ArnEquals"] - and iam_client.audited_account - in str( - statement["Condition"]["ArnEquals"]["aws:SourceArn"] - ) - ) - or ( - "ArnLike" in statement["Condition"] - and "aws:SourceArn" in statement["Condition"]["ArnLike"] - and iam_client.audited_account - in str( - statement["Condition"]["ArnLike"]["aws:SourceArn"] - ) - ) + and is_account_only_allowed_in_condition( + statement["Condition"], iam_client.audited_account ) ): report.status = "PASS" diff --git a/tests/providers/aws/lib/policy_condition_parser/policy_condition_parser_test.py b/tests/providers/aws/lib/policy_condition_parser/policy_condition_parser_test.py index 7a289b83..553a2e67 100644 --- a/tests/providers/aws/lib/policy_condition_parser/policy_condition_parser_test.py +++ b/tests/providers/aws/lib/policy_condition_parser/policy_condition_parser_test.py @@ -32,6 +32,32 @@ class Test_policy_condition_parser: condition_statement, AWS_ACCOUNT_NUMBER ) + def test_condition_parser_string_like_aws_SourceAccount_list(self): + condition_statement = {"StringLike": {"aws:SourceAccount": ["123456789012"]}} + assert is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_like_aws_SourceAccount_str(self): + condition_statement = {"StringLike": {"aws:SourceAccount": "123456789012"}} + assert is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_like_aws_SourceAccount_list_not_valid(self): + condition_statement = { + "StringLike": {"aws:SourceAccount": ["123456789012", "111222333444"]} + } + assert not is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_like_aws_SourceAccount_str_not_valid(self): + condition_statement = {"StringLike": {"aws:SourceAccount": "111222333444"}} + assert not is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + def test_condition_parser_string_equals_s3_ResourceAccount_list(self): condition_statement = {"StringEquals": {"s3:ResourceAccount": ["123456789012"]}} assert is_account_only_allowed_in_condition( @@ -86,6 +112,32 @@ class Test_policy_condition_parser: condition_statement, AWS_ACCOUNT_NUMBER ) + def test_condition_parser_string_like_aws_PrincipalAccount_list(self): + condition_statement = {"StringLike": {"aws:PrincipalAccount": ["123456789012"]}} + assert is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_like_aws_PrincipalAccount_str(self): + condition_statement = {"StringLike": {"aws:PrincipalAccount": "123456789012"}} + assert is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_like_aws_PrincipalAccount_list_not_valid(self): + condition_statement = { + "StringLike": {"aws:PrincipalAccount": ["123456789012", "111222333444"]} + } + assert not is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_like_aws_PrincipalAccount_str_not_valid(self): + condition_statement = {"StringLike": {"aws:PrincipalAccount": "111222333444"}} + assert not is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + def test_condition_parser_arn_like_aws_SourceArn_list(self): condition_statement = { "ArnLike": {"aws:SourceArn": ["arn:aws:cloudtrail:*:123456789012:trail/*"]} @@ -365,3 +417,57 @@ class Test_policy_condition_parser: assert not is_account_only_allowed_in_condition( condition_statement, AWS_ACCOUNT_NUMBER ) + + def test_condition_parser_string_equals_aws_ResourceAccount_list(self): + condition_statement = { + "StringEquals": {"aws:ResourceAccount": ["123456789012"]} + } + assert is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_equals_aws_ResourceAccount_str(self): + condition_statement = {"StringEquals": {"aws:ResourceAccount": "123456789012"}} + assert is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_equals_aws_ResourceAccount_list_not_valid(self): + condition_statement = { + "StringEquals": {"aws:ResourceAccount": ["123456789012", "111222333444"]} + } + assert not is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_equals_aws_ResourceAccount_str_not_valid(self): + condition_statement = {"StringEquals": {"aws:ResourceAccount": "111222333444"}} + assert not is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_like_aws_ResourceAccount_list(self): + condition_statement = {"StringLike": {"aws:ResourceAccount": ["123456789012"]}} + assert is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_like_aws_ResourceAccount_str(self): + condition_statement = {"StringLike": {"aws:ResourceAccount": "123456789012"}} + assert is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_like_aws_ResourceAccount_list_not_valid(self): + condition_statement = { + "StringLike": {"aws:ResourceAccount": ["123456789012", "111222333444"]} + } + assert not is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) + + def test_condition_parser_string_like_aws_ResourceAccount_str_not_valid(self): + condition_statement = {"StringLike": {"aws:ResourceAccount": "111222333444"}} + assert not is_account_only_allowed_in_condition( + condition_statement, AWS_ACCOUNT_NUMBER + ) diff --git a/tests/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention_test.py b/tests/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention_test.py index 10af3039..eb8f8ddb 100644 --- a/tests/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention_test.py +++ b/tests/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention_test.py @@ -249,3 +249,103 @@ class Test_iam_role_cross_service_confused_deputy_prevention: ) assert result[0].resource_id == "test" assert result[0].resource_arn == response["Role"]["Arn"] + + @mock_iam + def test_iam_service_role_with_cross_service_confused_deputy_prevention_PrincipalAccount( + self, + ): + iam_client = client("iam", region_name=AWS_REGION) + policy_document = { + "Version": "2008-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": {"Service": "workspaces.amazonaws.com"}, + "Action": "sts:AssumeRole", + "Condition": { + "StringLike": {"aws:PrincipalAccount": [AWS_ACCOUNT_ID]} + }, + } + ], + } + response = iam_client.create_role( + RoleName="test", + AssumeRolePolicyDocument=dumps(policy_document), + ) + + from prowler.providers.aws.services.iam.iam_service import IAM + + current_audit_info = self.set_mocked_audit_info() + current_audit_info.audited_account = AWS_ACCOUNT_ID + with mock.patch( + "prowler.providers.aws.lib.audit_info.audit_info.current_audit_info", + new=current_audit_info, + ), mock.patch( + "prowler.providers.aws.services.iam.iam_role_cross_service_confused_deputy_prevention.iam_role_cross_service_confused_deputy_prevention.iam_client", + new=IAM(current_audit_info), + ): + # Test Check + from prowler.providers.aws.services.iam.iam_role_cross_service_confused_deputy_prevention.iam_role_cross_service_confused_deputy_prevention import ( + iam_role_cross_service_confused_deputy_prevention, + ) + + check = iam_role_cross_service_confused_deputy_prevention() + result = check.execute() + assert len(result) == 1 + assert result[0].status == "PASS" + assert ( + result[0].status_extended + == "IAM Service Role test prevents against a cross-service confused deputy attack" + ) + assert result[0].resource_id == "test" + assert result[0].resource_arn == response["Role"]["Arn"] + + @mock_iam + def test_iam_service_role_with_cross_service_confused_deputy_prevention_ResourceAccount( + self, + ): + iam_client = client("iam", region_name=AWS_REGION) + policy_document = { + "Version": "2008-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": {"Service": "workspaces.amazonaws.com"}, + "Action": "sts:AssumeRole", + "Condition": { + "StringLike": {"aws:ResourceAccount": [AWS_ACCOUNT_ID]} + }, + } + ], + } + response = iam_client.create_role( + RoleName="test", + AssumeRolePolicyDocument=dumps(policy_document), + ) + + from prowler.providers.aws.services.iam.iam_service import IAM + + current_audit_info = self.set_mocked_audit_info() + current_audit_info.audited_account = AWS_ACCOUNT_ID + with mock.patch( + "prowler.providers.aws.lib.audit_info.audit_info.current_audit_info", + new=current_audit_info, + ), mock.patch( + "prowler.providers.aws.services.iam.iam_role_cross_service_confused_deputy_prevention.iam_role_cross_service_confused_deputy_prevention.iam_client", + new=IAM(current_audit_info), + ): + # Test Check + from prowler.providers.aws.services.iam.iam_role_cross_service_confused_deputy_prevention.iam_role_cross_service_confused_deputy_prevention import ( + iam_role_cross_service_confused_deputy_prevention, + ) + + check = iam_role_cross_service_confused_deputy_prevention() + result = check.execute() + assert len(result) == 1 + assert result[0].status == "PASS" + assert ( + result[0].status_extended + == "IAM Service Role test prevents against a cross-service confused deputy attack" + ) + assert result[0].resource_id == "test" + assert result[0].resource_arn == response["Role"]["Arn"]