Skip to content

Commit

Permalink
Merge pull request #24 from kmcquade/fix/GH-23-bool-object-not-iterable
Browse files Browse the repository at this point in the history
Fixes #23 - issue arising from policies with "Deny" with no resource constraints
  • Loading branch information
kmcquade authored May 6, 2020
2 parents 51a3866 + 4ed85a5 commit 7a1737a
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# CHANGELOG

## 0.0.11 (2020-05-06)
* Fixed an issue arising from policies where "Deny" was used in effect with no resource constraints. Fixes #23.

## 0.0.10 (2020-05-05)
* Removed the recursive credentials method from the `download` command.
* Fixed occasional installation error occurring from outdated Policy Sentry versions.
Expand Down
2 changes: 1 addition & 1 deletion cloudsplaining/bin/cloudsplaining
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"""
Cloudsplaining is an AWS IAM Assessment tool that identifies violations of least privilege and generates a risk-prioritized HTML report with a triage worksheet.
"""
__version__ = "0.0.10"
__version__ = "0.0.11"
import click
from cloudsplaining import command

Expand Down
6 changes: 4 additions & 2 deletions cloudsplaining/scan/policy_document.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ def all_allowed_actions(self):
"""Output all allowed IAM Actions, regardless of resource constraints"""
allowed_actions = []
for statement in self.statements:
allowed_actions.extend(statement.expanded_actions)
if statement.expanded_actions:
allowed_actions.extend(statement.expanded_actions)
allowed_actions = list(dict.fromkeys(allowed_actions))
return allowed_actions

Expand All @@ -50,7 +51,8 @@ def all_allowed_unrestricted_actions(self):
allowed_actions = []
for statement in self.statements:
if not statement.has_resource_constraints:
allowed_actions.extend(statement.expanded_actions)
if statement.expanded_actions:
allowed_actions.extend(statement.expanded_actions)
allowed_actions = list(dict.fromkeys(allowed_actions))
return allowed_actions

Expand Down
8 changes: 4 additions & 4 deletions cloudsplaining/scan/statement_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def _not_action_effective_actions(self):
"""If NotAction is used, calculate the allowed actions - i.e., what it would be """
effective_actions = []
if not self.not_action:
return False
return None
not_actions_expanded = determine_actions_to_expand(self.not_action)
not_actions_expanded_lowercase = [x.lower() for x in not_actions_expanded]

Expand Down Expand Up @@ -116,12 +116,12 @@ def _not_action_effective_actions(self):
return effective_actions
elif self.has_resource_constraints and self.effect_deny:
logger.debug("NOTE: Haven't decided if we support Effect Deny here?")
return False
return None
elif not self.has_resource_constraints and self.effect_deny:
logger.debug("NOTE: Haven't decided if we support Effect Deny here?")
return False
return None
else:
return False
return None

@property
def has_not_resource_with_allow(self):
Expand Down
20 changes: 20 additions & 0 deletions test/scanning/test_policy_document.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,23 @@ def test_allows_specific_actions(self):
]
results = policy_document.allows_specific_actions_without_constraints(high_priority_read_only_actions)
self.assertListEqual(results, high_priority_read_only_actions)

def test_policy_document_not_action_deny_gh_23(self):
test_policy = {
"Version": "2012-10-17",
"Statement": [{
"Sid": "DenyAllUsersNotUsingMFA",
"Effect": "Deny",
"NotAction": "iam:*",
"Resource": "*",
"Condition": {"BoolIfExists": {"aws:MultiFactorAuthPresent": "false"}}
}]
}
policy_document = PolicyDocument(test_policy)
allowed_actions = []
for statement in policy_document.statements:
if not statement.has_resource_constraints:
if statement.expanded_actions:
allowed_actions.extend(statement.expanded_actions)
self.assertListEqual(allowed_actions, [])
self.assertListEqual(policy_document.all_allowed_unrestricted_actions, [])

0 comments on commit 7a1737a

Please sign in to comment.