Repository: salesforce/policy_sentry (python)

Issue:
Ability to exclude actions from resulting policy
For example:

```yml
mode: crud
write:
- arn:aws:kms:${var.region}:${var.account_id}:key/${var.kms_key_id}
```

That will include this in the results:

```json
        {
            "Sid": "KmsWriteKey",
            "Effect": "Allow",
            "Action": [
                "kms:CancelKeyDeletion",
                "kms:CreateAlias",
                "kms:Decrypt",
                "kms:DeleteAlias",
                "kms:DeleteImportedKeyMaterial",
                "kms:DisableKey",
                "kms:DisableKeyRotation",
                "kms:EnableKey",
                "kms:EnableKeyRotation",
                "kms:Encrypt",
                "kms:GenerateDataKey",
                "kms:GenerateDataKeyPair",
                "kms:GenerateDataKeyPairWithoutPlaintext",
                "kms:GenerateDataKeyWithoutPlaintext",
                "kms:ImportKeyMaterial",
                "kms:ReEncryptFrom",
                "kms:ReEncryptTo",
                "kms:ScheduleKeyDeletion",
                "kms:Sign",
                "kms:UpdateAlias",
                "kms:UpdateKeyDescription",
                "kms:Verify"
            ],
            "Resource": [
                "arn:aws:kms:${var.region}:${var.account_id}:key/${var.kms_key_id}"
            ]
        }
```

Due to the fact that AWS does not list their access levels in true CRUD terminology (differentiating create, update, and delete vs. Execute), that means that if you have `kms:Encrypt`, you also have `kms:DisableKey`.

Now, we could override the access level - and perhaps we should do that. But this shows that it would be very useful to remove matching patterns from the resulting policy. Otherwise, you'd still have to edit the resulting policy manually, which is what we want to avoid.

Pull request:
Fixes #182 - adds the ability to exclude some actions from the resulting policy

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1de67970..8382ef14 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,9 @@
 ## Future release (Unreleased)
 * Add conditions support
 
+## 0.8.0.7 (2020-05-20)
+* Fixes #182 - adding the ability to exclude some actions from the resulting policy.
+
 ## 0.8.0.6 (2020-05-06)
 * Fixes overly verbose logging issue for `remove_actions_not_matching_access_level` (Fixes #178)
 * Homebrew support
diff --git a/README.md b/README.md
index 5e4beb0f..b1a10551 100644
--- a/README.md
+++ b/README.md
@@ -155,9 +155,6 @@ tagging:
 - ''
 permissions-management:
 - ''
-# Skip resource constraint requirements by listing actions here.
-skip-resource-constraints:
-- ''
 # Actions that do not support resource constraints
 wildcard-only:
   single-actions: # standalone actions
@@ -173,6 +170,12 @@ wildcard-only:
   - ''
   service-permissions-management:
   - ''
+# Skip resource constraint requirements by listing actions here.
+skip-resource-constraints:
+- ''
+# Exclude actions from the output by specifying them here. Accepts wildcards, like kms:Delete*
+exclude-actions:
+- ''
 ```
 
 ### Step 2: Copy/paste ARNs
@@ -181,7 +184,6 @@ wildcard-only:
 
 ```yaml
 mode: crud
-name: ''
 read:
 - 'arn:aws:ssm:us-east-1:123456789012:parameter/myparameter'
 write:
@@ -192,20 +194,6 @@ tagging:
 - 'arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret'
 permissions-management:
 - 'arn:aws:secretsmanager:us-east-1:123456789012:secret:mysecret'
-wildcard-only:
-  single-actions: # standalone actions
-  - ''
-  # Service-wide - like 's3' or 'ec2'
-  service-read:
-  - ''
-  service-write:
-  - ''
-  service-list:
-  - ''
-  service-tagging:
-  - ''
-  service-permissions-management:
-  - ''
 ```
 
 ### Step 3: Write-policy command
diff --git a/docs/writing-policies/excluding-actions.md b/docs/writing-policies/excluding-actions.md
new file mode 100644
index 00000000..0118d5fb
--- /dev/null
+++ b/docs/writing-policies/excluding-actions.md
@@ -0,0 +1,66 @@
+
+In basic CRUD mode, Policy Sentry provides a "granular select all" approach, so to speak, by identifying all Access Levels for actions under a Resource ARN.
+
+However, there are sometimes cases where you don't want to select **all** of those actions - you just want some of them.
+
+The `exclude-actions` section allows you to do this.
+
+You can specify actions that you don't want to include in the resulting policy, no matter what. See the example below.
+
+**Input**:
+
+```yaml
+mode: crud
+write:
+- arn:aws:kms:us-east-1:123456789012:key/aaaa-bbbb-cccc
+exclude-actions:
+- "kms:Delete*"
+- "kms:Disable*"
+- "kms:Schedule*"
+```
+
+**Output**
+
+As you can see in the `exclude-actions` section above, we are telling Policy Sentry to not include `kms:Delete*`, `kms:Disable*`, and `kms:Schedule*`. Notice that each line includes a wildcard (`*`) so you can tell Policy Sentry to exclude actions matching that pattern. The resulting policy should not include any of these actions that would normally be generated if the `exclude-actions` section were not used:
+
+* `kms:DeleteAlias`
+* `kms:DeleteImportedKeyMaterial`
+* `kms:DisableKey`
+* `kms:DisableKeyRotation`
+* `kms:ScheduleKeyDeletion`
+
+Notice how none of those actions are included in the resulting JSON policy output below:
+
+```json
+{
+    "Version": "2012-10-17",
+    "Statement": [
+        {
+            "Sid": "KmsWriteKey",
+            "Effect": "Allow",
+            "Action": [
+                "kms:CancelKeyDeletion",
+                "kms:CreateAlias",
+                "kms:Decrypt",
+                "kms:EnableKey",
+                "kms:EnableKeyRotation",
+                "kms:Encrypt",
+                "kms:GenerateDataKey",
+                "kms:GenerateDataKeyPair",
+                "kms:GenerateDataKeyPairWithoutPlaintext",
+                "kms:GenerateDataKeyWithoutPlaintext",
+                "kms:ImportKeyMaterial",
+                "kms:ReEncryptFrom",
+                "kms:ReEncryptTo",
+                "kms:Sign",
+                "kms:UpdateAlias",
+                "kms:UpdateKeyDescription",
+                "kms:Verify"
+            ],
+            "Resource": [
+                "arn:aws:kms:us-east-1:123456789012:key/aaaa-bbbb-cccc"
+            ]
+        }
+    ]
+}
+```
diff --git a/docs/writing-policies/overrides.md b/docs/writing-policies/skipping-resource-constraints.md
similarity index 100%
rename from docs/writing-policies/overrides.md
rename to docs/writing-policies/skipping-resource-constraints.md
diff --git a/examples/yml/crud-with-exclude-actions.yml b/examples/yml/crud-with-exclude-actions.yml
new file mode 100644
index 00000000..07149c66
--- /dev/null
+++ b/examples/yml/crud-with-exclude-actions.yml
@@ -0,0 +1,7 @@
+mode: crud
+write:
+- arn:aws:kms:us-east-1:123456789012:key/aaaa-bbbb-cccc
+exclude-actions:
+- "kms:Delete*"
+- "kms:Disable*"
+- "kms:Schedule*"
diff --git a/mkdocs.yml b/mkdocs.yml
index b1493280..561d9bdb 100644
--- a/mkdocs.yml
+++ b/mkdocs.yml
@@ -35,7 +35,8 @@ nav:
       - Service-wide: 'writing-policies/wildcard-only/service-wide.md'
       - "<i>Example</i>: Service-wide": 'writing-policies/wildcard-only/example-service-wide.md'
       - "<i>Example</i>: Combining approaches": 'writing-policies/wildcard-only/combined-approaches.md'
-    - "<i>Example</i>: Skipping Resource Constraints": 'writing-policies/overrides.md'
+    - "<i>Example</i>: Skipping Resource Constraints": 'writing-policies/skipping-resource-constraints.md'
+    - "<i>Example</i>: Excluding Actions": 'writing-policies/excluding-actions.md'
     - Minimizing characters: 'writing-policies/minimization.md'
     - Actions Mode: 'writing-policies/actions-mode.md'
 
diff --git a/policy_sentry/bin/cli.py b/policy_sentry/bin/cli.py
index 2d4c3220..b3458711 100755
--- a/policy_sentry/bin/cli.py
+++ b/policy_sentry/bin/cli.py
@@ -2,7 +2,7 @@
 """
     Policy Sentry is a tool for generating least-privilege IAM Policies.
 """
-__version__ = "0.8.0.6"
+__version__ = "0.8.0.7"
 import click
 from policy_sentry import command
 
diff --git a/policy_sentry/writing/sid_group.py b/policy_sentry/writing/sid_group.py
index 5b89cdce..2a099acf 100644
--- a/policy_sentry/writing/sid_group.py
+++ b/policy_sentry/writing/sid_group.py
@@ -3,6 +3,7 @@ sid_group indicates that this is a collection of policy-related data organized b
 """
 import logging
 import re
+from policy_sentry.analysis.expand import determine_actions_to_expand
 from policy_sentry.querying.all import get_all_actions
 from policy_sentry.querying.actions import (
     get_action_data,
@@ -55,7 +56,8 @@ class SidGroup:
         # sids instead of ARN
         self.sids = {}
         self.universal_conditions = {}
-        self.overrides = []
+        self.skip_resource_constraints = []
+        self.exclude_actions = []
         self.wildcard_only_single_actions = []
         # When a user requests all wildcard-only actions available under a service at a specific access level
         self.wildcard_only_service_read = []
@@ -86,26 +88,25 @@ class SidGroup:
         """
         return self.sids.keys()
 
-    def get_universal_conditions(self):
-        """
-        Get the universal conditions maps back as a dict
-
-        Returns:
-            Dictionary: The universal condtiion maps
-        """
-        return self.universal_conditions
+    def add_exclude_actions(self, exclude_actions):
+        """To exclude actions from the output"""
+        if exclude_actions:
+            expanded_actions = determine_actions_to_expand(exclude_actions)
+            self.exclude_actions = [x.lower() for x in expanded_actions]
+        else:
+            self.exclude_actions = []
 
-    def add_overrides(self, overrides):
+    def add_skip_resource_constraints(self, skip_resource_constraints_actions):
         """
         To override resource constraint requirements - i.e., instead of restricting `s3:PutObject` to a path and
         allowing `s3:PutObject` to `*` resources, put `s3:GetObject` here.
         """
-        if isinstance(overrides, list):
-            self.overrides.extend(overrides)
-        elif isinstance(overrides, str):
-            self.overrides.append([overrides])
+        if isinstance(skip_resource_constraints_actions, list):
+            self.skip_resource_constraints.extend(skip_resource_constraints_actions)
+        elif isinstance(skip_resource_constraints_actions, str):
+            self.skip_resource_constraints.append([skip_resource_constraints_actions])
         else:
-            raise Exception("Please provide 'overrides' as a list of IAM actions.")
+            raise Exception("Please provide 'skip_resource_constraints' as a list of IAM actions.")
 
     def add_requested_service_wide(self, service_prefixes, access_level):
         """
@@ -130,7 +131,6 @@ class SidGroup:
         """
         After (1) the list of wildcard-only single actions have been added and (2) the list of wildcard-only service-wide actions have been added, process them and store them under the proper SID.
         """
-        provided_wildcard_actions = []
         provided_wildcard_actions = (
             self.wildcard_only_single_actions
             + get_wildcard_only_actions_matching_services_and_access_level(self.wildcard_only_service_read, "Read")
@@ -158,10 +158,21 @@ class SidGroup:
 
         # render the policy
         for sid in self.sids:
-            actions = self.sids[sid]["actions"]
-            if len(actions) == 0:
+            temp_actions = self.sids[sid]["actions"]
+            if len(temp_actions) == 0:
                 logger.debug(f"No actions for sid {sid}")
                 continue
+            actions = []
+            if self.exclude_actions:
+                for temp_action in temp_actions:
+                    if temp_action.lower() in self.exclude_actions:
+                        logger.debug(f"\tExcluded action: {temp_action}")
+                    else:
+                        if temp_action not in actions:
+                            actions.append(temp_action)
+            else:
+                actions = temp_actions
+            # temp_actions.clear()
             if minimize is not None and isinstance(minimize, int):
                 logger.debug("Minimizing statements...")
                 actions = minimize_statement_actions(
@@ -377,6 +388,10 @@ class SidGroup:
             if cfg["mode"] == "crud":
                 logger.debug("CRUD mode selected")
                 check_crud_schema(cfg)
+                if "exclude-actions" in cfg:
+                    if cfg["exclude-actions"]:
+                        if cfg["exclude-actions"][0] != "":
+                            self.add_exclude_actions(cfg["exclude-actions"])
                 if "wildcard-only" in cfg.keys():
                     if "single-actions" in cfg["wildcard-only"]:
                         if cfg["wildcard-only"]["single-actions"]:
@@ -473,10 +488,10 @@ class SidGroup:
                                 f"Requested override: the actions {str(cfg['skip-resource-constraints'])} will "
                                 f"skip resource constraints."
                             )
-                            self.add_overrides(cfg["skip-resource-constraints"])
-                            for override_action in self.overrides:
+                            self.add_skip_resource_constraints(cfg["skip-resource-constraints"])
+                            for skip_resource_constraints_action in self.skip_resource_constraints:
                                 self.add_action_without_resource_constraint(
-                                    override_action, "SkipResourceConstraints"
+                                    skip_resource_constraints_action, "SkipResourceConstraints"
                                 )
             if cfg["mode"] == "actions":
                 check_actions_schema(cfg)
@@ -587,7 +602,7 @@ class SidGroup:
                 if "*" not in self.sids[sid]["arn_format"]:
                     for action in actions_under_wildcard_resources:
                         if action in self.sids[sid]["actions"]:
-                            if action not in self.overrides:
+                            if action not in self.skip_resource_constraints:
                                 # add it to a list of actions to nuke when they are under other SIDs
                                 actions_under_wildcard_resources_to_nuke.append(action)
 
diff --git a/policy_sentry/writing/template.py b/policy_sentry/writing/template.py
index 1a3cf3b8..b7c5625f 100644
--- a/policy_sentry/writing/template.py
+++ b/policy_sentry/writing/template.py
@@ -21,9 +21,6 @@ tagging:
 - ''
 permissions-management:
 - ''
-# Skip resource constraint requirements by listing actions here.
-skip-resource-constraints:
-- ''
 # Actions that do not support resource constraints
 wildcard-only:
   single-actions: # standalone actions
@@ -39,6 +36,12 @@ wildcard-only:
   - ''
   service-permissions-management:
   - ''
+# Skip resource constraint requirements by listing actions here.
+skip-resource-constraints:
+- ''
+# Exclude actions from the output by specifying them here. Accepts wildcards, like kms:Delete*
+exclude-actions:
+- ''
 """
 
 CRUD_TEMPLATE_DICT = {
@@ -49,7 +52,6 @@ CRUD_TEMPLATE_DICT = {
     "list": [],
     "tagging": [],
     "permissions-management": [],
-    "skip-resource-constraints": [],
     "wildcard-only": {
         "single-actions": [],
         "service-read": [],
@@ -58,6 +60,8 @@ CRUD_TEMPLATE_DICT = {
         "service-tagging": [],
         "service-permissions-management": [],
     },
+    "skip-resource-constraints": [],
+    "exclude-actions": []
 }
 
 ACTIONS_TEMPLATE_DICT = {"mode": "actions", "name": "", "actions": []}
diff --git a/policy_sentry/writing/validate.py b/policy_sentry/writing/validate.py
index f7d626bd..bcd49138 100644
--- a/policy_sentry/writing/validate.py
+++ b/policy_sentry/writing/validate.py
@@ -50,6 +50,7 @@ CRUD_SCHEMA = Schema(
             Optional("service-permissions-management"): [str],
         },
         Optional("skip-resource-constraints"): [str],
+        Optional("exclude-actions"): [str],
     }
 )
 
diff --git a/tasks.py b/tasks.py
index 8cc15294..169b3fad 100755
--- a/tasks.py
+++ b/tasks.py
@@ -233,7 +233,7 @@ def format(c):
 # TEST - LINT
 @task
 def run_linter(c):
-    """Linting with `pylint` and `black`"""
+    """Linting with `pylint`"""
     try:
         c.run('pylint policy_sentry/', warn=False)
     except UnexpectedExit as u_e:


Test patch:
diff --git a/test/writing/test_sid_group_crud.py b/test/writing/test_sid_group_crud.py
index eb05c7f8..dafddd86 100644
--- a/test/writing/test_sid_group_crud.py
+++ b/test/writing/test_sid_group_crud.py
@@ -475,3 +475,107 @@ class SidGroupCrudTestCase(unittest.TestCase):
         }
         print(json.dumps(output, indent=4))
         self.assertDictEqual(output, desired_output)
+
+    def test_exclude_actions_from_crud_output(self):
+        sid_group = SidGroup()
+        crud_with_exclude_actions = os.path.abspath(
+            os.path.join(
+                os.path.dirname(__file__),
+                os.path.pardir,
+                os.path.pardir,
+                "examples",
+                "yml",
+                "crud-with-exclude-actions.yml",
+            )
+        )
+
+        with open(crud_with_exclude_actions, "r") as this_yaml_file:
+            crud_with_exclude_actions_cfg = yaml.safe_load(this_yaml_file)
+        # crud_with_exclude_actions_cfg = {
+        #     "mode": "crud",
+        #     "write": [
+        #         "arn:aws:kms:us-east-1:123456789012:key/aaaa-bbbb-cccc"
+        #     ],
+        #     "exclude-actions": [
+        #         "kms:Delete*"
+        #     ]
+        # }
+
+        # print(json.dumps(crud_with_exclude_actions_cfg, indent=4))
+        sid_group.process_template(crud_with_exclude_actions_cfg)
+        result = sid_group.get_rendered_policy(crud_with_exclude_actions_cfg)
+        print(json.dumps(result, indent=4))
+        expected_result = {
+            "Version": "2012-10-17",
+            "Statement": [
+                {
+                    "Sid": "KmsWriteKey",
+                    "Effect": "Allow",
+                    "Action": [
+                        "kms:CancelKeyDeletion",
+                        "kms:CreateAlias",
+                        "kms:Decrypt",
+                        "kms:EnableKey",
+                        "kms:EnableKeyRotation",
+                        "kms:Encrypt",
+                        "kms:GenerateDataKey",
+                        "kms:GenerateDataKeyPair",
+                        "kms:GenerateDataKeyPairWithoutPlaintext",
+                        "kms:GenerateDataKeyWithoutPlaintext",
+                        "kms:ImportKeyMaterial",
+                        "kms:ReEncryptFrom",
+                        "kms:ReEncryptTo",
+                        "kms:Sign",
+                        "kms:UpdateAlias",
+                        "kms:UpdateKeyDescription",
+                        "kms:Verify"
+                    ],
+                    "Resource": [
+                        "arn:aws:kms:us-east-1:123456789012:key/aaaa-bbbb-cccc"
+                    ]
+                }
+            ]
+        }
+        self.assertDictEqual(result, expected_result)
+
+        crud_with_exclude_actions_cfg = {
+            "mode": "crud",
+            "write": [
+                "arn:aws:kms:us-east-1:123456789012:key/aaaa-bbbb-cccc"
+            ],
+            # This is really only because KMS is a special case.
+            "exclude-actions": [
+                "kms:Delete*",
+                "kms:Disable*",
+                "kms:Enable*",
+                "kms:Generate*",
+                "kms:Cancel*",
+                "kms:Create*",
+                "kms:Import*",
+                "kms:ReEncrypt*",
+                "kms:Sign*",
+                "kms:Schedule*",
+                "kms:Update*",
+                "kms:Verify*"
+            ]
+        }
+        sid_group.process_template(crud_with_exclude_actions_cfg)
+        results = sid_group.get_rendered_policy()
+        print(json.dumps(results, indent=4))
+        expected_result = {
+            "Version": "2012-10-17",
+            "Statement": [
+                {
+                    "Sid": "KmsWriteKey",
+                    "Effect": "Allow",
+                    "Action": [
+                        "kms:Decrypt",
+                        "kms:Encrypt"
+                    ],
+                    "Resource": [
+                        "arn:aws:kms:us-east-1:123456789012:key/aaaa-bbbb-cccc"
+                    ]
+                }
+            ]
+        }
+        self.assertDictEqual(results, expected_result)
diff --git a/test/writing/test_template.py b/test/writing/test_template.py
index 2e06ce04..e82a39f5 100644
--- a/test/writing/test_template.py
+++ b/test/writing/test_template.py
@@ -26,9 +26,6 @@ tagging:
 - ''
 permissions-management:
 - ''
-# Skip resource constraint requirements by listing actions here.
-skip-resource-constraints:
-- ''
 # Actions that do not support resource constraints
 wildcard-only:
   single-actions: # standalone actions
@@ -44,6 +41,12 @@ wildcard-only:
   - ''
   service-permissions-management:
   - ''
+# Skip resource constraint requirements by listing actions here.
+skip-resource-constraints:
+- ''
+# Exclude actions from the output by specifying them here. Accepts wildcards, like kms:Delete*
+exclude-actions:
+- ''
 """
         crud_template = create_crud_template()
         self.maxDiff = None