Skip to content

Commit 3381965

Browse files
authored
Optionally allow noisy results to flag risky actions regardless of resource constraints or conditions usage (salesforce#257)
Add --flag-all-risky-actions to arguments for scan and scan-policy-file commands
1 parent 743adba commit 3381965

14 files changed

+471
-337
lines changed

cloudsplaining/command/scan.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from cloudsplaining import set_log_level
2626

2727

28+
# fmt: off
2829
@click.command(
2930
short_help="Scan a single file containing AWS IAM account authorization details and generate report on "
3031
"IAM security posture. "
@@ -34,13 +35,16 @@
3435
@click.option("-o", "--output", required=False, type=click.Path(exists=True), default=os.getcwd(), help="Output directory.")
3536
@click.option("-s", "--skip-open-report", required=False, default=False, is_flag=True, help="Don't open the HTML report in the web browser after creating. This helps when running the report in automation.")
3637
@click.option("-m", "--minimize", required=False, default=False, is_flag=True, help="Reduce the size of the HTML Report by pulling the Cloudsplaining Javascript code over the internet.")
38+
@click.option("-aR", "--flag-all-risky-actions", is_flag=True, help="Flag all risky actions, regardless of whether resource ARN constraints or conditions are used.")
3739
@click.option("-v", "--verbose", "verbosity", help="Log verbosity level.", count=True)
40+
# fmt: on
3841
def scan(
3942
input_file: str,
4043
exclusions_file: str,
4144
output: str,
4245
skip_open_report: bool,
4346
minimize: bool,
47+
flag_all_risky_actions: bool,
4448
verbosity: int,
4549
) -> None: # pragma: no cover
4650
"""
@@ -60,6 +64,13 @@ def scan(
6064
else:
6165
exclusions = DEFAULT_EXCLUSIONS
6266

67+
if flag_all_risky_actions:
68+
flag_conditional_statements = True
69+
flag_resource_arn_statements = True
70+
else:
71+
flag_conditional_statements = False
72+
flag_resource_arn_statements = False
73+
6374
if os.path.isfile(input_file):
6475
account_name = os.path.basename(input_file).split(".")[0]
6576
with open(input_file) as f:
@@ -72,6 +83,8 @@ def scan(
7283
output,
7384
write_data_files=True,
7485
minimize=minimize,
86+
flag_conditional_statements=flag_conditional_statements,
87+
flag_resource_arn_statements=flag_resource_arn_statements,
7588
)
7689
html_output_file = os.path.join(output, f"iam-report-{account_name}.html")
7790
logger.info("Saving the report to %s", html_output_file)
@@ -137,7 +150,9 @@ def scan_account_authorization_details(
137150
output_directory: str = os.getcwd(),
138151
write_data_files: bool = False,
139152
minimize: bool = False,
140-
return_json_results: bool = False
153+
return_json_results: bool = False,
154+
flag_conditional_statements: bool = False,
155+
flag_resource_arn_statements: bool = False,
141156
) -> Any: # pragma: no cover
142157
"""
143158
Given the path to account authorization details files and the exclusions config file, scan all inline and
@@ -150,7 +165,9 @@ def scan_account_authorization_details(
150165
)
151166
check_authorization_details_schema(account_authorization_details_cfg)
152167
authorization_details = AuthorizationDetails(
153-
account_authorization_details_cfg, exclusions
168+
account_authorization_details_cfg, exclusions=exclusions,
169+
flag_conditional_statements=flag_conditional_statements,
170+
flag_resource_arn_statements=flag_resource_arn_statements
154171
)
155172
results = authorization_details.results
156173

cloudsplaining/command/scan_policy_file.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,11 @@
3131
@click.option("-i", "--input-file", type=str, help="Path of the IAM policy file to evaluate.")
3232
@click.option("-e", "--exclusions-file", help="A yaml file containing a list of actions to ignore when scanning.", type=click.Path(exists=True), required=False, default=EXCLUSIONS_FILE)
3333
@click.option("--high-priority-only", required=False, default=False, is_flag=True, help="If issues are found, only print the high priority risks (Resource Exposure, Privilege Escalation, Data Exfiltration). This can help with prioritization.")
34+
@click.option("-aR", "--flag-all-risky-actions", is_flag=True, help="Flag all risky actions, regardless of whether resource ARN constraints or conditions are used.")
3435
@click.option("--verbose", "-v", "verbosity", count=True)
3536
# pylint: disable=redefined-builtin
3637
def scan_policy_file(
37-
input_file: str, exclusions_file: str, high_priority_only: bool, verbosity: int
38+
input_file: str, exclusions_file: str, high_priority_only: bool, flag_all_risky_actions: bool, verbosity: int
3839
) -> None: # pragma: no cover
3940
"""Scan a single policy file to identify missing resource constraints."""
4041
set_log_level(verbosity)
@@ -57,10 +58,16 @@ def scan_policy_file(
5758
exclusions_cfg = yaml.safe_load(yaml_file)
5859
except yaml.YAMLError as exc:
5960
logger.critical(exc)
60-
# exclusions = Exclusions(exclusions_cfg)
61+
62+
if flag_all_risky_actions:
63+
flag_conditional_statements = True
64+
flag_resource_arn_statements = True
65+
else:
66+
flag_conditional_statements = False
67+
flag_resource_arn_statements = False
6168

6269
# Run the scan and get the raw data.
63-
results = scan_policy(policy, exclusions_cfg)
70+
results = scan_policy(policy, exclusions_cfg, flag_resource_arn_statements=flag_resource_arn_statements, flag_conditional_statements=flag_conditional_statements)
6471

6572
# There will only be one finding in the results but it is in a list.
6673
results_exist = 0
@@ -135,15 +142,19 @@ def scan_policy_file(
135142
def scan_policy(
136143
policy_json: Dict[str, Any],
137144
exclusions_config: Dict[str, List[str]] = DEFAULT_EXCLUSIONS_CONFIG,
145+
flag_conditional_statements: bool = False,
146+
flag_resource_arn_statements: bool = False,
138147
) -> Dict[str, Any]:
139148
"""
140149
Scan a policy document for missing resource constraints.
141150
142151
:param policy_json: Dictionary containing the IAM policy.
143152
:param exclusions_config: Exclusions configuration. If none, just send an empty dictionary. Defaults to the contents of cloudsplaining.shared.default-exclusions.yml
153+
:param flag_resource_arn_statements:
154+
:param flag_conditional_statements:
144155
:return:
145156
"""
146-
policy_document = PolicyDocument(policy_json)
147157
exclusions = Exclusions(exclusions_config)
158+
policy_document = PolicyDocument(policy_json, exclusions=exclusions, flag_resource_arn_statements=flag_resource_arn_statements, flag_conditional_statements=flag_conditional_statements)
148159
policy_finding = PolicyFinding(policy_document, exclusions)
149160
return policy_finding.results

cloudsplaining/scan/authorization_details.py

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,29 +29,43 @@ def __init__(
2929
self,
3030
auth_json: Dict[str, List[Dict[str, Any]]],
3131
exclusions: Exclusions = DEFAULT_EXCLUSIONS,
32+
flag_conditional_statements: bool = False,
33+
flag_resource_arn_statements: bool = False,
3234
) -> None:
35+
"""
36+
Object to hold and analyze Account Authorization details.
37+
38+
:param auth_json: the JSON response of the aws iam get-account-authorization-details command
39+
:param exclusions: A list of exclusions to apply to the results
40+
:param flag_conditional_statements: Flag IAM statements with conditions, not just wildcards
41+
:param flag_resource_arn_statements: Flag IAM statements with resource ARN restrictions, not just wildcards
42+
"""
3343
self.auth_json = auth_json
3444

3545
if not isinstance(exclusions, Exclusions):
3646
raise Exception(
3747
"For exclusions, please provide an object of the Exclusions type"
3848
)
3949
self.exclusions = exclusions
50+
self.flag_conditional_statements = flag_conditional_statements
51+
self.flag_resource_arn_statements = flag_resource_arn_statements
4052

41-
self.policies = ManagedPolicyDetails(auth_json.get("Policies", []), exclusions)
53+
self.policies = ManagedPolicyDetails(auth_json.get("Policies", []), exclusions, flag_conditional_statements=flag_conditional_statements, flag_resource_arn_statements=flag_resource_arn_statements)
4254

4355
# New Authorization file stuff
4456
self.group_detail_list = GroupDetailList(
45-
auth_json.get("GroupDetailList", []), self.policies, exclusions
57+
auth_json.get("GroupDetailList", []), self.policies, exclusions, flag_conditional_statements=flag_conditional_statements, flag_resource_arn_statements=flag_resource_arn_statements
4658
)
4759
self.user_detail_list = UserDetailList(
4860
auth_json.get("UserDetailList", []),
4961
self.policies,
5062
self.group_detail_list,
5163
exclusions,
64+
flag_conditional_statements=flag_conditional_statements,
65+
flag_resource_arn_statements=flag_resource_arn_statements
5266
)
5367
self.role_detail_list = RoleDetailList(
54-
auth_json.get("RoleDetailList", []), self.policies, exclusions
68+
auth_json.get("RoleDetailList", []), self.policies, exclusions, flag_conditional_statements=flag_conditional_statements, flag_resource_arn_statements=flag_resource_arn_statements
5569
)
5670

5771
@property

cloudsplaining/scan/group_details.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,20 @@ def __init__(
2222
group_details: List[Dict[str, Any]],
2323
policy_details: ManagedPolicyDetails,
2424
exclusions: Exclusions = DEFAULT_EXCLUSIONS,
25+
flag_conditional_statements: bool = False,
26+
flag_resource_arn_statements: bool = False,
2527
) -> None:
2628
if not isinstance(exclusions, Exclusions):
2729
raise Exception(
2830
"The exclusions provided is not an Exclusions type object. "
2931
"Please supply an Exclusions object and try again."
3032
)
3133
self.exclusions = exclusions
34+
self.flag_conditional_statements = flag_conditional_statements
35+
self.flag_resource_arn_statements = flag_resource_arn_statements
3236

3337
self.groups = [
34-
GroupDetail(group_detail, policy_details, exclusions)
38+
GroupDetail(group_detail, policy_details, exclusions=exclusions, flag_conditional_statements=flag_conditional_statements, flag_resource_arn_statements=flag_resource_arn_statements)
3539
for group_detail in group_details
3640
]
3741

@@ -98,6 +102,8 @@ def __init__(
98102
group_detail: Dict[str, Any],
99103
policy_details: ManagedPolicyDetails,
100104
exclusions: Exclusions = DEFAULT_EXCLUSIONS,
105+
flag_conditional_statements: bool = False,
106+
flag_resource_arn_statements: bool = False,
101107
):
102108
"""
103109
Initialize the GroupDetail object.
@@ -111,6 +117,10 @@ def __init__(
111117
self.group_id = group_detail["GroupId"]
112118
self.group_name = group_detail["GroupName"]
113119

120+
# Fix Issue #254 - Allow flagging risky actions even when there are resource constraints
121+
self.flag_conditional_statements = flag_conditional_statements
122+
self.flag_resource_arn_statements = flag_resource_arn_statements
123+
114124
if not isinstance(exclusions, Exclusions):
115125
raise Exception(
116126
"The exclusions provided is not an Exclusions type object. "
@@ -130,7 +140,10 @@ def __init__(
130140
exclusions.is_policy_excluded(policy_name)
131141
or exclusions.is_policy_excluded(policy_id)
132142
):
133-
inline_policy = InlinePolicy(policy_detail)
143+
# NOTE: The Exclusions were not here before the #254 fix (which was an unfiled bug I just discovered) so the presence of this might break some older unit tests. Might need to fix that.
144+
inline_policy = InlinePolicy(
145+
policy_detail, exclusions=exclusions, flag_conditional_statements=flag_conditional_statements,
146+
flag_resource_arn_statements=flag_resource_arn_statements)
134147
self.inline_policies.append(inline_policy)
135148

136149
# Managed Policies (either AWS-managed or Customer managed)

cloudsplaining/scan/inline_policy.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ class InlinePolicy:
1313
"""
1414

1515
def __init__(
16-
self, policy_detail: Dict[str, Any], exclusions: Exclusions = DEFAULT_EXCLUSIONS
16+
self, policy_detail: Dict[str, Any], exclusions: Exclusions = DEFAULT_EXCLUSIONS,
17+
flag_conditional_statements: bool = False,
18+
flag_resource_arn_statements: bool = False,
1719
) -> None:
1820
"""
1921
Initialize the InlinePolicy object.
@@ -25,9 +27,15 @@ def __init__(
2527
"The exclusions provided is not an Exclusions type object. "
2628
"Please supply an Exclusions object and try again."
2729
)
30+
# Fix Issue #254 - Allow flagging risky actions even when there are resource constraints
31+
self.flag_conditional_statements = flag_conditional_statements
32+
self.flag_resource_arn_statements = flag_resource_arn_statements
33+
2834
self.policy_name = policy_detail.get("PolicyName", "")
2935
self.policy_document = PolicyDocument(
30-
cast(Dict[str, Any], policy_detail.get("PolicyDocument")), exclusions
36+
cast(Dict[str, Any], policy_detail.get("PolicyDocument")), exclusions=exclusions,
37+
flag_conditional_statements=flag_conditional_statements,
38+
flag_resource_arn_statements=flag_resource_arn_statements
3139
)
3240
# Generating the provider ID based on a string representation of the Policy Document,
3341
# to avoid collisions where there are inline policies with the same name but different contents

cloudsplaining/scan/managed_policy_detail.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ def __init__(
2828
self,
2929
policy_details: List[Dict[str, Any]],
3030
exclusions: Exclusions = DEFAULT_EXCLUSIONS,
31+
flag_conditional_statements: bool = False,
32+
flag_resource_arn_statements: bool = False,
3133
) -> None:
3234
self.policy_details = []
3335
if not isinstance(exclusions, Exclusions):
@@ -36,6 +38,8 @@ def __init__(
3638
"Please supply an Exclusions object and try again."
3739
)
3840
self.exclusions = exclusions
41+
self.flag_conditional_statements = flag_conditional_statements
42+
self.flag_resource_arn_statements = flag_resource_arn_statements
3943

4044
for policy_detail in policy_details:
4145
this_policy_name = policy_detail["PolicyName"]
@@ -66,7 +70,7 @@ def __init__(
6670
this_policy_path,
6771
)
6872
continue
69-
self.policy_details.append(ManagedPolicy(policy_detail, exclusions))
73+
self.policy_details.append(ManagedPolicy(policy_detail, exclusions=exclusions, flag_resource_arn_statements=self.flag_resource_arn_statements, flag_conditional_statements=self.flag_conditional_statements))
7074

7175
def get_policy_detail(self, arn: str) -> "ManagedPolicy":
7276
"""Get a ManagedPolicy object by providing the ARN. This is useful to PrincipalDetail objects"""
@@ -125,11 +129,16 @@ class ManagedPolicy:
125129
"""
126130

127131
def __init__(
128-
self, policy_detail: Dict[str, Any], exclusions: Exclusions = DEFAULT_EXCLUSIONS
132+
self, policy_detail: Dict[str, Any], exclusions: Exclusions = DEFAULT_EXCLUSIONS,
133+
flag_conditional_statements: bool = False,
134+
flag_resource_arn_statements: bool = False,
129135
) -> None:
130136
# Store the Raw JSON data from this for safekeeping
131137
self.policy_detail = policy_detail
132138

139+
self.flag_conditional_statements = flag_conditional_statements
140+
self.flag_resource_arn_statements = flag_resource_arn_statements
141+
133142
# Store the attributes per Policy item
134143
self.policy_name = policy_detail["PolicyName"]
135144
self.policy_id = policy_detail["PolicyId"]
@@ -172,7 +181,7 @@ def _policy_document(self) -> PolicyDocument:
172181
for policy_version in self.policy_version_list:
173182
if policy_version.get("IsDefaultVersion") is True:
174183
return PolicyDocument(
175-
policy_version.get("Document"), exclusions=self.exclusions
184+
policy_version.get("Document"), exclusions=self.exclusions, flag_resource_arn_statements=self.flag_resource_arn_statements, flag_conditional_statements=self.flag_conditional_statements
176185
)
177186
raise Exception(
178187
"Managed Policy ARN %s has no default Policy Document version", self.arn

cloudsplaining/scan/policy_document.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,15 @@ class PolicyDocument:
2828
"""
2929

3030
def __init__(
31-
self, policy: Dict[str, Any], exclusions: Exclusions = DEFAULT_EXCLUSIONS
31+
self, policy: Dict[str, Any], exclusions: Exclusions = DEFAULT_EXCLUSIONS,
32+
flag_conditional_statements: bool = False,
33+
flag_resource_arn_statements: bool = False,
3234
) -> None:
3335
statement_structure = policy.get("Statement", [])
3436
self.policy = policy
3537
self.statements = []
38+
self.flag_conditional_statements = flag_conditional_statements
39+
self.flag_resource_arn_statements = flag_resource_arn_statements
3640

3741
if not isinstance(exclusions, Exclusions):
3842
raise Exception(
@@ -46,7 +50,7 @@ def __init__(
4650
statement_structure = [statement_structure] # pragma: no cover
4751

4852
for statement in statement_structure:
49-
self.statements.append(StatementDetail(statement))
53+
self.statements.append(StatementDetail(statement, flag_conditional_statements=self.flag_conditional_statements, flag_resource_arn_statements=self.flag_resource_arn_statements))
5054

5155
@property
5256
def json(self) -> Dict[str, Any]:
@@ -88,6 +92,9 @@ def all_allowed_unrestricted_actions(self) -> List[str]:
8892
and statement.restrictable_actions
8993
):
9094
allowed_actions.update(statement.restrictable_actions)
95+
# Fix Issue #254 - Allow flagging risky actions even when there are resource constraints
96+
if self.flag_resource_arn_statements and statement.effect_allow:
97+
allowed_actions.update(statement.restrictable_actions)
9198
allowed_actions = self.filter_deny_statements(allowed_actions)
9299
return list(allowed_actions)
93100

@@ -102,6 +109,13 @@ def all_allowed_unrestrictable_actions(self) -> List[str]:
102109
and statement.unrestrictable_actions
103110
):
104111
allowed_actions.update(statement.unrestrictable_actions)
112+
# Fix Issue #254 - Allow flagging risky actions even when there are resource constraints
113+
if (
114+
statement.effect_allow
115+
and not statement.has_condition
116+
and self.flag_resource_arn_statements
117+
):
118+
allowed_actions.update(statement.unrestrictable_actions)
105119
allowed_actions = self.filter_deny_statements(allowed_actions)
106120
return list(allowed_actions)
107121

@@ -116,6 +130,7 @@ def infrastructure_modification(self) -> List[str]:
116130
):
117131
if action.lower() not in self.exclusions.exclude_actions:
118132
actions_missing_resource_constraints.append(action)
133+
actions_missing_resource_constraints.sort()
119134
return actions_missing_resource_constraints
120135

121136
@property
@@ -220,8 +235,9 @@ def allows_specific_actions_without_constraints(
220235
allowed.add(unrestricted_actions_lower[specific_action_lower])
221236
elif specific_action_lower in unrestrictable_actions_lower:
222237
allowed.add(unrestrictable_actions_lower[specific_action_lower])
223-
224-
return list(allowed)
238+
results = list(allowed)
239+
results.sort()
240+
return results
225241

226242
@property
227243
def allows_data_exfiltration_actions(self) -> List[str]:

0 commit comments

Comments
 (0)