Adding a way to block newly added permissions from repo

This commit adds a new feature that detects newly added
permissions and adds them to a list.  The temporary block period
is configurable.  When permissions are scanned any that are new
from the last policy version are added to 'NoRepoPermissions'.
Expired entries are removed during update, but any that are
expired will be ignored from _get_repoable_permissions.

Also fix a bug where sometimes when adding a new policy the
new policy version wouldn't get added at the right place.

repokid/repokid.py

@@ -30,6 +30,7 @@
 import pprint
 import requests
 import sys
+import time
 
 import botocore
 from cloudaux.aws.iam import list_roles, get_role_inline_policies
@@ -131,7 +132,8 @@ def _generate_default_config(filename=None):
             }
         },
         "repo_requirements": {
-            "oldest_aa_data_days": 5
+            "oldest_aa_data_days": 5,
+            "exclude_new_permissions_for_days": 14
         }
     }
     if filename:
@@ -219,27 +221,32 @@ def _get_role_permissions(role):
     return permissions
 
 
-def _get_repoable_permissions(permissions, aa_data):
+def _get_repoable_permissions(permissions, aa_data, no_repo_permissions):
     """
     Generate a list of repoable permissions for a role based on the list of all permissions the role's policies
     currently allow and Access Advisor data for the services included in the role's policies.
 
     The first step is to come up with a list of services that were used within the time threshold (the same defined)
-    in the age filter config. Permissions are repoable if they aren't in the used list and aren't in the global list
-    of unsupported services/actions (IAM_ACCESS_ADVISOR_UNSUPPORTED_SERVICES, IAM_ACCESS_ADVISOR_UNSUPPORTED_ACTIONS).
+    in the age filter config. Permissions are repoable if they aren't in the used list, aren't in the global list
+    of unsupported services/actions (IAM_ACCESS_ADVISOR_UNSUPPORTED_SERVICES, IAM_ACCESS_ADVISOR_UNSUPPORTED_ACTIONS),
+    and aren't being temporarily ignored because they're on the no_repo_permissions list (newly added).
 
     Args:
         permissions (list): The full list of permissions that the role's permissions allow
         aa_data (list): A list of Access Advisor data for a role. Each element is a dictionary with a couple required
                         attributes: lastAuthenticated (epoch time in milliseconds when the service was last used and
                         serviceNamespace (the service used)
+        no_repo_permissions (dict): Keys are the name of permissions and values are the time the entry expires
 
     Returns:
         set: Permissions that are 'repoable' (not used within the time threshold)
     """
     ago = datetime.timedelta(CONFIG['filter_config']['AgeFilter']['minimum_age'])
     now = datetime.datetime.now(tzlocal())
 
+    current_time = time.time()
+    no_repo_list = [perm.lower() for perm in no_repo_permissions if no_repo_permissions[perm] > current_time]
+
     used_services = set()
     for service in aa_data:
         accessed = service['lastAuthenticated']
@@ -260,6 +267,10 @@ def _get_repoable_permissions(permissions, aa_data):
                 LOGGER.warn('skipping {}'.format(permission))
                 continue
 
+            if permission.lower() in no_repo_list:
+                LOGGER.warn('skipping {} because it is in the no repo list'.format(permission))
+                continue
+
             repoable_permissions.add(permission.lower())
 
     return repoable_permissions
@@ -294,7 +305,7 @@ def _calculate_repo_scores(roles):
 
         # permissions are only repoable if the role isn't being disqualified by filter(s)
         if len(role.disqualified_by) == 0:
-            repoable_permissions = _get_repoable_permissions(permissions, role.aa_data)
+            repoable_permissions = _get_repoable_permissions(permissions, role.aa_data, role.no_repo_permissions)
             repoable_services = set([permission.split(':')[0] for permission in repoable_permissions])
             repoable_services = sorted(repoable_services)
             role.repoable_permissions = len(repoable_permissions)
@@ -593,6 +604,23 @@ def display_roles(account_number, inactive=False):
             csv_writer.writerow(row)
 
 
+def _find_newly_added_permissions(old_policy, new_policy):
+    """
+    Compare and old version of policies to a new version and return a set of permissions that were added.  This will
+    be used to maintain a list of permissions that were newly added and should not be repoed for a period of time.
+
+    Args:
+        old_policy
+        new_policy
+
+    Returns:
+        set: Exapnded set of permissions that are in the new policy and not the old one
+    """
+    old_permissions = _get_role_permissions(Role({'Policies': [{'Policy': old_policy}]}))
+    new_permissions = _get_role_permissions(Role({'Policies': [{'Policy': new_policy}]}))
+    return new_permissions - old_permissions
+
+
 def find_roles_with_permission(permission):
     """
     Search roles in all accounts for a policy with a given permission, log the ARN of each role with this permission
@@ -675,7 +703,7 @@ def display_role(account_number, role_name):
     repoable_permissions = set([])
     permissions = _get_role_permissions(role)
     if len(role.disqualified_by) == 0:
-        repoable_permissions = _get_repoable_permissions(permissions, role.aa_data)
+        repoable_permissions = _get_repoable_permissions(permissions, role.aa_data, role.no_repo_permissions)
 
     print "Repoable services:"
     headers = ['Service', 'Action', 'Repoable']
@@ -749,7 +777,7 @@ def repo_role(account_number, role_name, commit=False):
         return
 
     permissions = _get_role_permissions(role)
-    repoable_permissions = _get_repoable_permissions(permissions, role.aa_data)
+    repoable_permissions = _get_repoable_permissions(permissions, role.aa_data, role.no_repo_permissions)
     repoed_policies, deleted_policy_names = _get_repoed_policy(role.policies[-1]['Policy'], repoable_permissions)
 
     policies_length = len(json.dumps(repoed_policies))

repokid/role.py

@@ -6,6 +6,7 @@ def __init__(self, role_dict):
         self.assume_role_policy_document = role_dict.get('AssumeRolePolicyDocument', None)
         self.create_date = role_dict.get('CreateDate', None)
         self.disqualified_by = role_dict.get('DisqualifiedBy', [])
+        self.no_repo_permissions = role_dict.get('NoRepoPermissions', {})
         self.policies = role_dict.get('Policies', [])
         self.refreshed = role_dict.get('Refreshed', '')
         self.repoable_permissions = role_dict.get('RepoablePermissions', 0)
@@ -27,6 +28,7 @@ def as_dict(self):
                 'CreateDate': self.create_date,
                 'DisqualifiedBy': self.disqualified_by,
                 'Policies': self.policies,
+                'NoRepoPermissions': self.no_repo_permissions,
                 'Refreshed': self.refreshed,
                 'RepoablePermissions': self.repoable_permissions,
                 'RepoableServices': self.repoable_services,

repokid/utils/roledata.py

@@ -1,10 +1,13 @@
 from datetime import datetime
 import sys
+import time
 
 import boto3
 from botocore.exceptions import ClientError as BotoClientError
 from cloudaux.aws.sts import boto3_cached_conn
 
+import repokid.repokid
+
 # used as a placeholder for empty SID to work around this: https://github.com/aws/aws-sdk-js/issues/833
 DYNAMO_EMPTY_STRING = "---DYNAMO-EMPTY-STRING---"
 DYNAMO_TABLE = None
@@ -24,7 +27,8 @@ def add_new_policy_version(role, current_policy, update_source):
     Returns:
         None
     """
-    new_item_index = len(role.policies)
+    cur_role_data = _get_role_data(role.role_id, fields=['Policies'])
+    new_item_index = len(cur_role_data.get('Policies', []))
 
     try:
         policy_entry = {'Source': update_source, 'Discovered': datetime.utcnow().isoformat(), 'Policy': current_policy}
@@ -279,6 +283,46 @@ def update_aardvark_data(aardvark_data, roles):
             LOGGER.error('Dynamo table error: {}'.format(e))
 
 
+def update_no_repo_permissions(role, newly_added_permissions):
+    """
+    Update Dyanmo entry for newly added permissions. Any that were newly detected get added with an expiration
+    date of now plus the config setting for 'repo_requirements': 'exclude_new_permissions_for_days'. Expired entries
+    get deleted. Also update the role object with the new no-repo-permissions.
+
+    Args:
+        role
+        newly_added_permissions (set)
+
+    Returns:
+        None
+    """
+    current_ignored_permissions = _get_role_data(role.role_id, fields=['NoRepoPermissions']).get(
+                                                 'NoRepoPermissions', {})
+    new_ignored_permissions = {}
+
+    current_time = int(time.time())
+    new_perms_expire_time = current_time + (
+        24 * 60 * 60 * repokid.repokid.CONFIG['repo_requirements']['exclude_new_permissions_for_days'])
+
+    # only copy non-expired items to the new dictionary
+    for permission, expire_time in current_ignored_permissions.items():
+        if expire_time > current_time:
+            new_ignored_permissions[permission] = current_ignored_permissions[permission]
+
+    for permission in newly_added_permissions:
+        new_ignored_permissions[permission] = new_perms_expire_time
+
+    role.no_repo_permissions = new_ignored_permissions
+
+    try:
+        DYNAMO_TABLE.update_item(Key={'RoleId': role.role_id},
+                                 UpdateExpression="SET NoRepoPermissions=:nrp",
+                                 ExpressionAttributeValues={":nrp": new_ignored_permissions})
+    except BotoClientError as e:
+        from repokid.repokid import LOGGER
+        LOGGER.error('Dynamo table error: {}'.format(e))
+
+
 def update_repoable_data(roles):
     """
     Update total permissions and repoable permissions count and a list of repoable services in Dynamo for each role
@@ -307,7 +351,8 @@ def update_repoable_data(roles):
 def update_role_data(role, current_policy):
     """
     Compare the current version of a policy for a role and what has been previously stored in Dynamo.
-      - If current and new policy versions are different store the new version in Dynamo.
+      - If current and new policy versions are different store the new version in Dynamo. Add any newly added
+          permissions to temporary permission blacklist. Purge any old entries from permission blacklist.
       - Refresh the updated time on the role policy
       - If the role is completely new, store the first version in Dynamo
       - Updates the role with full history of policies, including current version
@@ -326,10 +371,14 @@ def update_role_data(role, current_policy):
 
     if stored_role:
         # is the policy list the same as the last we had?
-        if current_policy != _empty_string_from_dynamo_replace(stored_role['Policies'][-1]['Policy']):
+        old_policy = _empty_string_from_dynamo_replace(stored_role['Policies'][-1]['Policy'])
+        if current_policy != old_policy:
             add_new_policy_version(role, current_policy, 'Scan')
             LOGGER.info('{} has different inline policies than last time, adding to role store'.format(role.arn))
 
+            newly_added_permissions = repokid.repokid._find_newly_added_permissions(old_policy, current_policy)
+            update_no_repo_permissions(role, newly_added_permissions)
+
         _refresh_updated_time(role.role_id)
     else:
         _store_item(role, current_policy)

tests/test_repokid.py

@@ -248,10 +248,13 @@ def test_generate_default_config(self):
 
         required_iam_config = ['assume_role', 'session_name', 'region']
 
+        required_repo_requirements = ['oldest_aa_data_days', 'exclude_new_permissions_for_days']
+
         assert all(field in generated_config for field in required_config_fields)
         assert all(field in generated_config['filter_config'] for field in required_filter_configs)
         assert all(field in generated_config['dynamo_db'] for field in required_dynamo_config)
         assert all(field in generated_config['connection_iam'] for field in required_iam_config)
+        assert all(field in generated_config['repo_requirements'] for field in required_repo_requirements)
 
     @patch('repokid.repokid.expand_policy')
     @patch('repokid.repokid.get_actions_from_statement')
@@ -278,16 +281,19 @@ def test_get_repoable_permissions(self):
         repokid.repokid.IAM_ACCESS_ADVISOR_UNSUPPORTED_ACTIONS = ['service_1:action_3', 'service_1:action_4']
 
         permissions = ['service_1:action_1', 'service_1:action_2', 'service_1:action_3', 'service_1:action_4',
-                       'service_2:action_1', 'service_3:action_1', 'service_3:action_2']
+                       'service_2:action_1', 'service_3:action_1', 'service_3:action_2', 'service_4:action_1',
+                       'service_4:action_2']
 
         aa_data = [{'serviceNamespace': 'service_1', 'lastAuthenticated': (time.time() - 90000) * 1000},
                    {'serviceNamespace': 'service_2', 'lastAuthenticated': (time.time() - 90000) * 1000},
                    {'serviceNamespace': 'service_3', 'lastAuthenticated': time.time() * 1000}]
 
-        repoable_permissions = repokid.repokid._get_repoable_permissions(permissions, aa_data)
+        no_repo_permissions = {'service_4:action_1': time.time()-1, 'service_4:action_2': time.time()+1000}
+
+        repoable_permissions = repokid.repokid._get_repoable_permissions(permissions, aa_data, no_repo_permissions)
         # service_1:action_3 and action_4 are unsupported actions, service_2 is an unsupported service, service_3
-        # was used too recently
-        assert repoable_permissions == set(['service_1:action_1', 'service_1:action_2'])
+        # was used too recently, service_4 action 2 is in no_repo_permissions and not expired
+        assert repoable_permissions == set(['service_1:action_1', 'service_1:action_2', 'service_4:action_1'])
 
     @patch('repokid.repokid._get_role_permissions')
     @patch('repokid.repokid._get_repoable_permissions')
@@ -333,3 +339,10 @@ def test_get_repoed_policy(self):
                                                                   'Resource': ['*'],
                                                                   'Effect': 'Allow'}]}}
         assert empty_policies == ['iam_perms']
+
+    def test_find_newly_added_permissions(self):
+        old_policy = ROLE_POLICIES['all_services_used']
+        new_policy = ROLE_POLICIES['unused_ec2']
+
+        new_perms = repokid.repokid._find_newly_added_permissions(old_policy, new_policy)
+        assert new_perms == set(['ec2:allocatehosts', 'ec2:associateaddress'])