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

fix(issue-147): resolve issue 147 by re-including the previous make list functionality #249

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
4 changes: 2 additions & 2 deletions parliament/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
config,
__version__,
)
from parliament.misc import make_list
from parliament.misc import make_simple_list

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -54,7 +54,7 @@ def is_finding_filtered(finding, minimum_severity="LOW"):
all_match = True
for location_type, locations_to_ignore in ignore_location.items():
has_array_match = False
for location_to_ignore in make_list(locations_to_ignore):
for location_to_ignore in make_simple_list(locations_to_ignore):
if re.fullmatch(
location_to_ignore.lower(),
str(finding.location.get(location_type, "")).lower(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"""
import re
from parliament import Policy
from parliament.misc import make_list


def audit(policy: Policy) -> None:
Expand Down
21 changes: 20 additions & 1 deletion parliament/misc.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,32 @@
import jsoncfg


def make_list(v):
def make_simple_list(v):
"""
If the object is not a list already, it converts it to one
Examples:
[1, 2, 3] -> [1, 2, 3]
[1] -> [1]
1 -> [1]

This is a very simple function to listify a single object as required.
"""
if not isinstance(v, list):
return [v]
return v


def make_location_list(v):
"""
Create a list of location identifiers for a file. This is either going to be
a single location identifier that is being made into a list, or a list of them
which will be returned automatically.

If the object is not a list already, it converts it to one
Examples:
[{line: 1, column: 1}, {line: 15, column: 3}] -> [{line: 1, column: 1}, {line: 15, column: 3}]
[{line: 1, column: 1}] -> [{line: 1, column: 1}]
{line: 1, column: 1} -> [{line: 1, column: 1}]
"""
if not jsoncfg.node_is_array(v):
if jsoncfg.node_is_scalar(v):
Expand Down
6 changes: 3 additions & 3 deletions parliament/policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from . import expand_action
from .finding import Finding
from .misc import make_list
from .misc import make_location_list
from .statement import Statement


Expand Down Expand Up @@ -93,7 +93,7 @@ def get_references(self, privilege_prefix, privilege_name):
def get_allowed_actions(self, raise_exceptions=True):
actions_referenced = set()
for stmt in self.statements:
actions = make_list(stmt.stmt["Action"])
actions = make_location_list(stmt.stmt["Action"])
for action in actions:
expanded_actions = expand_action(action.value, raise_exceptions)
for expanded_action in expanded_actions:
Expand Down Expand Up @@ -264,7 +264,7 @@ def analyze(
return False

sids = {}
stmts_json = make_list(self.policy_json["Statement"])
stmts_json = make_location_list(self.policy_json["Statement"])
for stmt_json in stmts_json:
stmt = Statement(stmt_json)
self.statements.append(stmt)
Expand Down
28 changes: 14 additions & 14 deletions parliament/statement.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
UnknownPrefixException,
)
from .finding import Finding
from .misc import make_list
from .misc import make_location_list


def is_condition_key_match(document_key, str):
Expand Down Expand Up @@ -312,7 +312,7 @@ def in_actions(self, privilege_prefix, privilege_name):
"""

if "Action" in self.stmt:
for action in make_list(self.stmt["Action"]):
for action in make_location_list(self.stmt["Action"]):
if action.value == "*" or action.value == "*:*":
return True

Expand All @@ -327,7 +327,7 @@ def in_actions(self, privilege_prefix, privilege_name):
return False

# Else, we're dealing with a NotAction
for action in make_list(self.stmt["NotAction"]):
for action in make_location_list(self.stmt["NotAction"]):
if action == "*" or action == "*:*":
# I don't think it makes sense to have a "NotAction" of "*", but I'm including this check anyway.
return False
Expand Down Expand Up @@ -378,14 +378,14 @@ def get_resources_for_privilege(self, privilege_prefix, privilege_name):
)

# At least one resource has to match the action's required resources
for resource in make_list(self.stmt["Resource"]):
for resource in make_location_list(self.stmt["Resource"]):
if is_arn_match(resource_type, arn_format, resource.value):
affected_resources.append(resource.value)
elif resource.value == "*":
affected_resources.append(resource.value)

# Ensure we match on "*"
for resource in make_list(self.stmt["Resource"]):
for resource in make_location_list(self.stmt["Resource"]):
if resource.value == "*":
affected_resources.append(resource.value)

Expand Down Expand Up @@ -435,7 +435,7 @@ def _check_principal(self, principal_element):
Checks that the Principal (or NotPrincipal) element conforms to expectations
"""

for principal in make_list(principal_element):
for principal in make_location_list(principal_element):
if jsoncfg.node_is_scalar(principal):
if principal.value == "*":
continue
Expand All @@ -447,7 +447,7 @@ def _check_principal(self, principal_element):
for json_object in principal:
key = json_object[0]
if key == "AWS":
for aws_principal in make_list(json_object[1]):
for aws_principal in make_location_list(json_object[1]):
text = aws_principal.value
account_id_regex = re.compile("^\d{12}$")
arn_regex = re.compile(
Expand All @@ -465,7 +465,7 @@ def _check_principal(self, principal_element):
"UNKNOWN_PRINCIPAL", location=principal, detail=text
)
elif key == "Federated":
for federation in make_list(json_object[1]):
for federation in make_location_list(json_object[1]):
federation = federation.value
saml_regex = re.compile(
"^arn:[-a-z\*]*:iam::\d{12}:saml-provider/.*$"
Expand Down Expand Up @@ -533,7 +533,7 @@ def _check_condition(self, operator, condition_block, expanded_actions):
for block in condition_block:
key = block[0]
values = []
for v in make_list(block[1]):
for v in make_location_list(block[1]):
values.append(v.value)

# Check for known bad pattern
Expand Down Expand Up @@ -748,9 +748,9 @@ def analyze_statement(self):
return False

if "Action" in self.stmt:
actions = make_list(self.stmt["Action"])
actions = make_location_list(self.stmt["Action"])
elif "NotAction" in self.stmt:
actions = make_list(self.stmt["NotAction"])
actions = make_location_list(self.stmt["NotAction"])
else:
self.add_finding(
"MALFORMED",
Expand All @@ -769,9 +769,9 @@ def analyze_statement(self):
return False

if "Resource" in self.stmt:
resources = make_list(self.stmt["Resource"])
resources = make_location_list(self.stmt["Resource"])
elif "NotResource" in self.stmt:
resources = make_list(self.stmt["NotResource"])
resources = make_location_list(self.stmt["NotResource"])
else:
self.add_finding(
"MALFORMED",
Expand All @@ -782,7 +782,7 @@ def analyze_statement(self):

# Check if a Condition element exists and if so save them for later
if "Condition" in self.stmt:
conditions = make_list(self.stmt["Condition"])
conditions = make_location_list(self.stmt["Condition"])
if len(conditions) > 1:
self.add_finding(
"MALFORMED",
Expand Down