aws_kms: Update policy on existing keys (when passed) (#60059)

* aws_kms: (integration tests) Use module_defaults to reduce the copy and paste

* aws_kms: (integration tests) make sure policy option functions.

* aws_kms: (integration tests) Move iam_role creation to start of playbook.

iam_roles aren't fully created when iam_role completes, there's a delay on the Amazon side before they're fully recognised.

* aws_kms: Update policy on existing keys (when passed)

changelogs/fragments/60059-aws-kms-update-policy.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - aws_kms - Update key policy when key already exists

hacking/aws_config/testing_policies/security-policy.json

@@ -108,6 +108,7 @@
                 "kms:GenerateRandom",
                 "kms:Get*",
                 "kms:List*",
+                "kms:PutKeyPolicy",
                 "kms:RetireGrant",
                 "kms:ScheduleKeyDeletion",
                 "kms:TagResource",

lib/ansible/modules/cloud/amazon/aws_kms.py

@@ -365,7 +365,7 @@ from ansible.module_utils.aws.core import AnsibleAWSModule, is_boto3_error_code
 from ansible.module_utils.ec2 import ec2_argument_spec
 from ansible.module_utils.ec2 import AWSRetry, camel_dict_to_snake_dict
 from ansible.module_utils.ec2 import boto3_tag_list_to_ansible_dict, ansible_dict_to_boto3_tag_list
-from ansible.module_utils.ec2 import compare_aws_tags
+from ansible.module_utils.ec2 import compare_aws_tags, compare_policies
 from ansible.module_utils.six import string_types
 
 import json
@@ -664,6 +664,28 @@ def update_key(connection, module, key):
         except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
             module.fail_json_aws(e, msg="Unable to add tag to key")
 
+    # Update existing policy before trying to tweak grants
+    if module.params.get('policy'):
+        policy = module.params.get('policy')
+        try:
+            keyret = connection.get_key_policy(KeyId=key['key_id'], PolicyName='default')
+        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+            # If we can't fetch the current policy assume we're making a change
+            # Could occur if we have PutKeyPolicy without GetKeyPolicy
+            original_policy = {}
+        original_policy = json.loads(keyret['Policy'])
+        try:
+            new_policy = json.loads(policy)
+        except ValueError as e:
+            module.fail_json_aws(e, msg="Unable to parse new policy as JSON")
+        if compare_policies(original_policy, new_policy):
+            changed = True
+            if not module.check_mode:
+                try:
+                    connection.put_key_policy(KeyId=key['key_id'], PolicyName='default', Policy=policy)
+                except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+                    module.fail_json_aws(e, msg="Unable to update key policy")
+
     desired_grants = module.params.get('grants')
     existing_grants = key['grants']
 
@@ -774,7 +796,7 @@ def do_policy_grant(kms, keyarn, role_arn, granttypes, mode='grant', dry_run=Tru
     policy = json.loads(keyret['Policy'])
 
     changes_needed = {}
-    # assert_policy_shape(policy)
+    assert_policy_shape(policy)
     had_invalid_entries = False
     for statement in policy['Statement']:
         for granttype in ['role', 'role grant', 'admin']:

test/integration/targets/aws_kms/tasks/main.yml

@@ -1,21 +1,36 @@
-- block:
-
-    # ============================================================
-    - name: See whether key exists and its current state
-      aws_kms_info:
+- module_defaults:
+    group/aws:
         region: "{{ aws_region }}"
         aws_access_key: "{{ aws_access_key }}"
         aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
+        security_token: "{{ security_token | default(omit) }}"
+  block:
+    # ============================================================
+    #   PREPARATION
+    #
+    # Get some information about who we are before starting our tests
+    # we'll need this as soon as we start working on the policies
+    - name: get ARN of calling user
+      aws_caller_info:
+      register: aws_caller_info
+
+    # IAM Roles completes before the Role is fully instantiated, create it here
+    # to ensure it exists when we need it for updating the policies
+    - name: create an IAM role that can do nothing
+      iam_role:
+        name: "{{ resource_prefix }}-kms-role"
+        state: present
+        assume_role_policy_document: '{"Version": "2012-10-17", "Statement": {"Action": "sts:AssumeRole", "Principal": {"Service": "ec2.amazonaws.com"}, "Effect": "Deny"} }'
+      register: iam_role_result
+    # ============================================================
+    #   TESTS
+    - name: See whether key exists and its current state
+      aws_kms_info:
         filters:
           alias: "{{ resource_prefix }}-kms"
 
     - name: create a key
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         tags:
           Hello: World
@@ -31,10 +46,6 @@
 
     - name: find facts about the key
       aws_kms_info:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         filters:
           alias: "{{ resource_prefix }}-kms"
       register: new_key
@@ -44,15 +55,27 @@
         that:
           - new_key["keys"]|length == 1
 
-    - name: create an IAM role that can do nothing
-      iam_role:
-        name: "{{ resource_prefix }}-kms-role"
-        state: present
-        assume_role_policy_document: '{"Version": "2012-10-17", "Statement": {"Action": "sts:AssumeRole", "Principal": {"Service": "ec2.amazonaws.com"}, "Effect": "Deny"} }'
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
-      register: iam_role_result
+    - name: Update Policy on key to match AWS Console generate policy
+      aws_kms:
+        key_alias: "alias/{{ resource_prefix }}-kms"
+        policy: "{{ lookup('template', 'console-policy.j2') | to_json }}"
+      register: kms_policy_changed
+
+    - name: Policy should have been changed
+      assert:
+        that:
+          - kms_policy_changed is changed
+
+    - name: Attempt to re-assert the same policy
+      aws_kms:
+        key_alias: "alias/{{ resource_prefix }}-kms"
+        policy: "{{ lookup('template', 'console-policy.j2') | to_json }}"
+      register: kms_policy_changed
+
+    - name: Policy should not have changed since it was last set
+      assert:
+        that:
+          - kms_policy_changed is succeeded
 
     - name: grant user-style access to production secrets
       aws_kms:
@@ -60,17 +83,9 @@
         key_alias: "alias/{{ resource_prefix }}-kms"
         role_name: "{{ resource_prefix }}-kms-role"
         grant_types: "role,role grant"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
-        region: "{{ aws_region }}"
 
     - name: find facts about the key
       aws_kms_info:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         filters:
           alias: "{{ resource_prefix }}-kms"
       register: new_key
@@ -80,44 +95,15 @@
         mode: deny
         key_alias: "alias/{{ resource_prefix }}-kms"
         role_arn: "{{ iam_role_result.iam_role.arn }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
-        region: "{{ aws_region }}"
 
     - name: find facts about the key
       aws_kms_info:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         filters:
           alias: "{{ resource_prefix }}-kms"
       register: new_key
 
-    - name: set aws environment base fact
-      set_fact:
-        aws_environment_base:
-          AWS_ACCESS_KEY_ID: "{{ aws_access_key }}"
-          AWS_SECRET_ACCESS_KEY: "{{ aws_secret_key }}"
-      no_log: True
-
-    - name: set aws environment fact
-      set_fact:
-        aws_environment: "{{ aws_environment_base|combine(security_token|ternary({'AWS_SECURITY_TOKEN': security_token}, {})) }}"
-      no_log: True
-
-    - name: get ARN of calling user
-      aws_caller_info:
-      environment: "{{ aws_environment }}"
-      register: aws_caller_info
-
     - name: Allow the IAM role to use a specific Encryption Context
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         state: present
         purge_grants: yes
@@ -143,10 +129,6 @@
 
     - name: Add a second grant
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         state: present
         grants:
@@ -170,10 +152,6 @@
 
     - name: Add a second grant again
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         state: present
         grants:
@@ -197,10 +175,6 @@
 
     - name: Update the grants with purge_grants set
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         state: present
         purge_grants: yes
@@ -225,10 +199,6 @@
 
     - name: update third grant to change encryption context equals to subset
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         state: present
         grants:
@@ -254,10 +224,6 @@
 
     - name: tag encryption key
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         state: present
         tags:
@@ -275,10 +241,6 @@
 
     - name: add, replace, remove tags
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         state: present
         purge_tags: yes
@@ -298,10 +260,6 @@
 
     - name: make no real tag change
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         state: present
       register: tag_kms_no_update
@@ -317,10 +275,6 @@
 
     - name: update the key's description and disable it
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         state: present
         description: test key for testing
@@ -336,10 +290,6 @@
 
     - name: delete the key
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         state: absent
       register: delete_kms
@@ -352,10 +302,6 @@
 
     - name: undelete and enable the key
       aws_kms:
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
         state: present
         enabled: yes
@@ -368,15 +314,11 @@
           - undelete_kms.changed
 
   always:
-
     # ============================================================
+    #   CLEAN-UP
     - name: finish off by deleting key
       aws_kms:
         state: absent
-        region: "{{ aws_region }}"
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
         alias: "{{ resource_prefix }}-kms"
       register: destroy_result
 
@@ -384,7 +326,4 @@
       iam_role:
         name: "{{ resource_prefix }}-kms-role"
         state: absent
-        aws_access_key: "{{ aws_access_key }}"
-        aws_secret_key: "{{ aws_secret_key }}"
-        security_token: "{{ security_token }}"
       register: iam_role_result

test/integration/targets/aws_kms/templates/console-policy.j2

@@ -0,0 +1,72 @@
+{
+    "Id": "key-consolepolicy-3",
+    "Version": "2012-10-17",
+    "Statement": [
+        {
+            "Sid": "Enable IAM User Permissions",
+            "Effect": "Allow",
+            "Principal": {
+                "AWS": "arn:aws:iam::{{ aws_caller_info.account }}:root"
+            },
+            "Action": "kms:*",
+            "Resource": "*"
+        },
+        {
+            "Sid": "Allow access for Key Administrators",
+            "Effect": "Allow",
+            "Principal": {
+                "AWS": "{{ aws_caller_info.arn }}"
+            },
+            "Action": [
+                "kms:Create*",
+                "kms:Describe*",
+                "kms:Enable*",
+                "kms:List*",
+                "kms:Put*",
+                "kms:Update*",
+                "kms:Revoke*",
+                "kms:Disable*",
+                "kms:Get*",
+                "kms:Delete*",
+                "kms:TagResource",
+                "kms:UntagResource",
+                "kms:ScheduleKeyDeletion",
+                "kms:CancelKeyDeletion"
+            ],
+            "Resource": "*"
+        },
+        {
+            "Sid": "Allow use of the key",
+            "Effect": "Allow",
+            "Principal": {
+                "AWS": "{{ aws_caller_info.arn }}"
+            },
+            "Action": [
+                "kms:Encrypt",
+                "kms:Decrypt",
+                "kms:ReEncrypt*",
+                "kms:GenerateDataKey*",
+                "kms:DescribeKey"
+            ],
+            "Resource": "*"
+        },
+        {
+            "Sid": "Allow attachment of persistent resources",
+            "Effect": "Allow",
+            "Principal": {
+                "AWS": "{{ aws_caller_info.arn }}"
+            },
+            "Action": [
+                "kms:CreateGrant",
+                "kms:ListGrants",
+                "kms:RevokeGrant"
+            ],
+            "Resource": "*",
+            "Condition": {
+                "Bool": {
+                    "kms:GrantIsForAWSResource": "true"
+                }
+            }
+        }
+    ]
+}