Dependent actions allow broader permissions with wildcard

[knastase]

I am not sure if this was a conscious design decision or not. Dependent actions are always added with wildcards (*) and may not be the expected behavior.

Example

When generating a policy document for write actions on a KMS key ARN, the dependent actions are added as wildcards which could lead to granting more permissions than anticipated.

kms:ReplicateKey is a write action that is added when I specify a key ARN (as designed) Two dependent actions of it are kms:CreateKey and kms:PutKeyPolicy

I understand kms:CreateKey has no resource requirements and is added as a wildcard. kms:PutKeyPolicy on the other hand CAN take a key ARN as a resource constraint but is indiscriminately added as a wildcard action as well. This leads to the generated policy being able to put a key policy on any KMS key (if a key has the default key policy or an overly permissive one)

Observed behavior

- Sid: MultMultNone
  Effect: Allow
  Action:
  - kms:CreateKey
  - kms:PutKeyPolicy <--
  - kms:TagResource
  Resource:
  - '*' <--

Expected behavior

- Sid: KmsWriteKey
  Effect: Allow
  Action:
  ...
  - kms:Verify
  - kms:PutKeyPolicy <--
  - kms:TagResource
  Resource:
  - arn:aws:kms:us-east-1:123456789012:key/shaq <--
- Sid: MultMultNone
  Effect: Allow
  Action:
  - kms:CreateKey
  Resource:
  - '*'

Possible remediation

I attempted to add a bit of code to writing/sid_group.py to add the dependent action to the same SID as the other actions if they share the same resource constraint (but part of a different access_level).

if len(dependent_actions) > 0:
  for dep_action in dependent_actions:
      # If a dependent action has the same resource constraint, add it to actions 
      # instead of * which could grant more access than anticipated.
      for level in ["Read", "Write", "List", "Tagging", "Permissions management"]:
          if dep_action in get_actions_with_arn_type_and_access_level(
              service_prefix, resource_type_name, level):
                if not dep_action in actions:
                  actions.append(dep_action)
      if not dep_action in actions:
          self.add_action_without_resource_constraint(dep_action)

[kmcquade]

@knasty51 - right on! Would you mind sending that remediation in as a PR?

thanks so much for all of your contributions recently - you rock!

[knastase]

1st attempt - https://github.com/salesforce/policy_sentry/pull/395