diff --git a/MANIFEST.in b/MANIFEST.in index e74cdf0..b9b1a39 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,3 +1,6 @@ -include LICENSE README.md example-viz.png +include LICENSE +include README.md +include examples/example-viz.png +include examples/example-privesc-only-viz.svg recursive-exclude * *.pyc -prune tests* \ No newline at end of file +prune tests \ No newline at end of file diff --git a/principalmapper/__init__.py b/principalmapper/__init__.py index eb55150..f04cb97 100644 --- a/principalmapper/__init__.py +++ b/principalmapper/__init__.py @@ -15,4 +15,4 @@ # You should have received a copy of the GNU Affero General Public License # along with Principal Mapper. If not, see . -__version__ = '1.1.0' +__version__ = '1.1.1' diff --git a/principalmapper/analysis/find_risks.py b/principalmapper/analysis/find_risks.py index 7787b54..8598c62 100644 --- a/principalmapper/analysis/find_risks.py +++ b/principalmapper/analysis/find_risks.py @@ -84,6 +84,8 @@ def gen_privesc_findings(graph: Graph) -> List[Finding]: node_path_list = [] for node in graph.nodes: + if node.is_admin: + continue # skip current admins privesc_res, edge_list = can_privesc(graph, node) if privesc_res: node_path_list.append((node, edge_list)) diff --git a/principalmapper/graphing/orgs_cli.py b/principalmapper/graphing/orgs_cli.py index cf79aba..32be1c7 100644 --- a/principalmapper/graphing/orgs_cli.py +++ b/principalmapper/graphing/orgs_cli.py @@ -103,7 +103,7 @@ def process_arguments(parsed_args: Namespace): org_tree = get_organizations_data(session) logger.info('Generated initial organization data for {}'.format(org_tree.org_id)) - # create the account -> OU path map and apply to all accounts (same as org_update operation) + # create the account -> OU path map and apply to all accounts (same as orgs update operation) account_ou_map = _map_account_ou_paths(org_tree) logger.debug('account_ou_map: {}'.format(account_ou_map)) _update_accounts_with_ou_path_map(org_tree.org_id, account_ou_map, get_storage_root()) @@ -121,7 +121,7 @@ def process_arguments(parsed_args: Namespace): except Exception as ex: logger.warning('Unable to load a Graph object for account {}, possibly because it is not mapped yet. ' 'Please map all accounts and then update the Organization Tree ' - '(`pmapper graph org_update`).'.format(account)) + '(`pmapper graph orgs update`).'.format(account)) logger.debug(str(ex)) for graph_obj_a in graph_objs: @@ -161,7 +161,7 @@ def process_arguments(parsed_args: Namespace): except Exception as ex: logger.warning('Unable to load a Graph object for account {}, possibly because it is not mapped yet. ' 'Please map all accounts and then update the Organization Tree ' - '(`pmapper graph org_update`).'.format(account)) + '(`pmapper graph orgs update`).'.format(account)) logger.debug(str(ex)) for graph_obj_a in graph_objs: @@ -264,5 +264,5 @@ def _update_accounts_with_ou_path_map(org_id: str, account_ou_map: dict, root_di else: logger.debug( 'Account {} of organization {} does not have a Graph. You will need to update the ' - 'organization data at a later point (`pmapper graph org_update`).'.format(account.account_id, org_id) + 'organization data at a later point (`pmapper graph orgs update`).'.format(account.account_id, org_id) ) # warning gets thrown up by caller, no need to reiterate diff --git a/principalmapper/querying/query_interface.py b/principalmapper/querying/query_interface.py index ac974db..2cc4a8a 100644 --- a/principalmapper/querying/query_interface.py +++ b/principalmapper/querying/query_interface.py @@ -284,6 +284,11 @@ def local_check_authorization_full(principal: Node, action_to_check: str, resour logger.debug('Explicit Deny: Principal\'s attached policies.') return False + for iam_group in principal.group_memberships: + for policy in iam_group.attached_policies: + if policy_has_matching_statement(policy, 'Deny', action_to_check, resource_to_check, conditions_keys_copy): + logger.debug('Explicit Deny: Principal\'s IAM Group policies') + if service_control_policy_groups is not None: for service_control_policy_group in service_control_policy_groups: for service_control_policy in service_control_policy_group: @@ -363,6 +368,12 @@ def local_check_authorization_full(principal: Node, action_to_check: str, resour if policy_has_matching_statement(policy, 'Allow', action_to_check, resource_to_check, conditions_keys_copy): return True # already did Deny statement checks, so we're done + # Check principal's IAM Groups policies + for iam_group in principal.group_memberships: + for policy in iam_group.attached_policies: + if policy_has_matching_statement(policy, 'Allow', action_to_check, resource_to_check, conditions_keys_copy): + return True # already did Deny statement checks, so we're done + logger.debug('Implicit Deny: Principal\'s Attached Policies') return False diff --git a/principalmapper/querying/query_result.py b/principalmapper/querying/query_result.py index 23047e8..bbb8eb4 100644 --- a/principalmapper/querying/query_result.py +++ b/principalmapper/querying/query_result.py @@ -41,13 +41,16 @@ def __init__(self, allowed: bool, edge_list: Union[List[Edge], Node], node: Node def print_result(self, action_param: str, resource_param: str): """Prints information about the QueryResult object to stdout.""" if self.allowed: - if isinstance(self.edge_list, Node) and self.edge_list == self.node: - # node is an Admin but can't directly call the action - print('{} IS authorized to call action {} for resource {} THRU its admin privileges'.format( - self.node.searchable_name(), action_param, resource_param - )) - - if len(self.edge_list) == 0: + if isinstance(self.edge_list, Node): + if self.edge_list == self.node: + # node is an Admin but can't directly call the action + print('{} CAN BECOME authorized to call action {} for resource {} THRU its admin privileges'.format( + self.node.searchable_name(), action_param, resource_param + )) + else: + raise ValueError('Improperly-generated QueryResult object: edge_list is a Node but not the input Node') + + elif len(self.edge_list) == 0: # node itself is auth'd print('{} IS authorized to call action {} for resource {}'.format( self.node.searchable_name(), action_param, resource_param diff --git a/setup.py b/setup.py index d551877..0a495ae 100644 --- a/setup.py +++ b/setup.py @@ -35,7 +35,7 @@ author_email='erik.steringer@nccgroup.com', scripts=[], include_package_data=True, - packages=find_packages(), + packages=find_packages(exclude=("tests", )), package_data={}, python_requires='>=3.5, <4', # assume Python 4 will break install_requires=['botocore', 'packaging', 'python-dateutil', 'pydot'], diff --git a/tests/test_admin_identification.py b/tests/test_admin_identification.py new file mode 100644 index 0000000..20dda81 --- /dev/null +++ b/tests/test_admin_identification.py @@ -0,0 +1,83 @@ +# Copyright (c) NCC Group and Erik Steringer 2021. This file is part of Principal Mapper. +# +# Principal Mapper is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Principal Mapper is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with Principal Mapper. If not, see . + + +import logging +import unittest + + +from principalmapper.common import Node, Policy, Group +from principalmapper.graphing.gathering import update_admin_status + + +class TestAdminIdentification(unittest.TestCase): + def test_admin_verified_by_inline_policies(self): + admin_policy = Policy('arn:aws:iam::000000000000:user/user_1', 'inline_admin', { + 'Version': '2012-10-17', + 'Statement': [{ + 'Effect': 'Allow', + 'Action': '*', + 'Resource': '*' + }] + }) + + not_admin_policy = Policy('arn:aws:iam::000000000000:user/user_2', 'inline_not_admin', { + 'Version': '2012-10-17', + 'Statement': [ + { + 'Effect': 'Allow', + 'Action': '*', + 'Resource': '*' + }, + { + 'Effect': 'Deny', + 'Action': '*', + 'Resource': '*' + } + ] + }) + + new_node_1 = Node('arn:aws:iam::000000000000:user/user_1', 'id1', [admin_policy], [], None, None, 1, False, + False, None, False, None) + new_node_2 = Node('arn:aws:iam::000000000000:user/user_2', 'id2', [not_admin_policy], [], None, None, 1, False, + False, None, False, None) + + update_admin_status([new_node_1, new_node_2]) + + self.assertTrue(new_node_1.is_admin, 'User with admin policy should be marked as an admin') + self.assertFalse(new_node_2.is_admin, 'User with non-admin policy should not be marked as an admin') + + def test_admin_verified_for_group_member(self): + admin_policy = Policy('arn:aws:iam::000000000000:group/admins', 'inline_admin', { + 'Version': '2012-10-17', + 'Statement': [{ + 'Effect': 'Allow', + 'Action': '*', + 'Resource': '*' + }] + }) + + admin_group = Group('arn:aws:iam::000000000000:group/admins', [admin_policy]) + not_admin_group = Group('arn:aws:iam::000000000000:group/losers', []) + + new_node_1 = Node('arn:aws:iam::000000000000:user/node_1', 'id1', [], [admin_group], None, None, 1, False, + False, None, False, None) + new_node_2 = Node('arn:aws:iam::000000000000:user/node_2', 'id2', [], [not_admin_group], None, None, 1, False, + False, None, False, None) + + update_admin_status([new_node_1, new_node_2]) + + self.assertTrue(new_node_1.is_admin, 'Member of admin group should be marked as an admin') + self.assertFalse(new_node_2.is_admin, 'Member of non-admin group should not be marked as an admin') diff --git a/tests/test_resource_policy_evaluation.py b/tests/test_resource_policy_evaluation.py index 7584981..c13d657 100644 --- a/tests/test_resource_policy_evaluation.py +++ b/tests/test_resource_policy_evaluation.py @@ -127,6 +127,45 @@ def test_iam_assume_role(self): ) ) + # A user from another account + other_account_node = Node( + 'arn:aws:iam::999999999999:role/test_other', + 'ARIA00', + [ + Policy( + 'arn:aws:iam::999999999999:role/test_other', + 'inline1', + { + 'Version': '2012-10-17', + 'Statement': [{ + 'Effect': 'Allow', + 'Action': 'sts:AssumeRole', + 'Resource': '*' + }] + } + ) + ], + [], + {}, + [], + 0, + False, + False, + None, + False, + None + ) + self.assertFalse( + local_check_authorization_full( + other_account_node, + 'sts:AssumeRole', + 'arn:aws:iam::000000000000:role/test1', + {}, + trust_doc_1, + '000000000000' + ) + ) + def test_match_action_resource_policy_elements(self): """Test if we're correctly testing Action/Resource elements in resource policies""" bucket_policy_1 = {