v1.1.1 (#79)

* apply fixes to setup

* fixing #78 and adding a unit-test to prevent future regression

* update language around orgs update command (addresses #76)

* fix querying involving admins

* fix for analysis: will not report current admins as being able to privesc (#77)

* version update

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*
+prune tests

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 <https://www.gnu.org/licenses/>.
 
-__version__ = '1.1.0'
+__version__ = '1.1.1'

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))

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

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
 

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

setup.py

@@ -35,7 +35,7 @@
     author_email='[email protected]',
     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'],

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 <https://www.gnu.org/licenses/>.
+
+
+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')

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 = {