[aws] Support custom KMS keys in `aws_s3` module (#35761)

* Allow the use of 'aws:kms' as an encryption method
* Allow the use of a non standard KMS key
* Deduce whether AWS Signature Version 4 is required rather than specifying with a parameter

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

@@ -52,6 +52,14 @@ options:
       - When set for PUT mode, asks for server-side encryption.
     default: True
     version_added: "2.0"
+  encryption_mode:
+    description:
+      - What encryption mode to use if C(encrypt) is set
+    default: AES256
+    choices:
+      - AES256
+      - aws:kms
+    version_added: "2.7"
   expiration:
     description:
       - Time limit (in seconds) for the URL generated and returned by S3/Walrus when performing a mode=put or mode=geturl operation.
@@ -140,6 +148,10 @@ options:
         GetObject permission but no other permissions. In this case using the option mode: get will fail without specifying
         ignore_nonexistent_bucket: True."
     version_added: "2.3"
+  encryption_kms_key_id:
+    description:
+      - KMS key id to use when encrypting objects using C(aws:kms) encryption. Ignored if encryption is not C(aws:kms)
+    version_added: "2.7"
 
 requirements: [ "boto3", "botocore" ]
 author:
@@ -290,6 +302,10 @@ except ImportError:
     pass  # will be detected by imported HAS_BOTO3
 
 
+class Sigv4Required(Exception):
+    pass
+
+
 def key_check(module, s3, bucket, obj, version=None, validate=True):
     exists = True
     try:
@@ -443,7 +459,9 @@ def create_dirkey(module, s3, bucket, obj, encrypt):
     try:
         params = {'Bucket': bucket, 'Key': obj, 'Body': b''}
         if encrypt:
-            params['ServerSideEncryption'] = 'AES256'
+            params['ServerSideEncryption'] = module.params['encryption_mode']
+        if module.params['encryption_kms_key_id'] and module.params['encryption_mode'] == 'aws:kms':
+            params['SSEKMSKeyId'] = module.params['encryption_kms_key_id']
 
         s3.put_object(**params)
         for acl in module.params.get('permission'):
@@ -481,7 +499,9 @@ def upload_s3file(module, s3, bucket, obj, src, expiry, metadata, encrypt, heade
     try:
         extra = {}
         if encrypt:
-            extra['ServerSideEncryption'] = 'AES256'
+            extra['ServerSideEncryption'] = module.params['encryption_mode']
+        if module.params['encryption_kms_key_id'] and module.params['encryption_mode'] == 'aws:kms':
+            extra['SSEKMSKeyId'] = module.params['encryption_kms_key_id']
         if metadata:
             extra['Metadata'] = {}
 
@@ -522,7 +542,9 @@ def download_s3file(module, s3, bucket, obj, dest, retries, version=None):
         else:
             key = s3.get_object(Bucket=bucket, Key=obj)
     except botocore.exceptions.ClientError as e:
-        if e.response['Error']['Code'] != "404":
+        if e.response['Error']['Code'] == 'InvalidArgument' and 'require AWS Signature Version 4' in to_text(e):
+            raise Sigv4Required()
+        elif e.response['Error']['Code'] != "404":
             module.fail_json(msg="Could not find the key %s." % obj, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
 
     for x in range(0, retries + 1):
@@ -551,6 +573,9 @@ def download_s3str(module, s3, bucket, obj, version=None, validate=True):
             contents = to_native(s3.get_object(Bucket=bucket, Key=obj)["Body"].read())
         module.exit_json(msg="GET operation complete", contents=contents, changed=True)
     except botocore.exceptions.ClientError as e:
+        if e.response['Error']['Code'] == 'InvalidArgument' and 'require AWS Signature Version 4' in to_text(e):
+            raise Sigv4Required()
+        else:
             module.fail_json(msg="Failed while getting contents of object %s as a string." % obj,
                              exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
 
@@ -584,7 +609,7 @@ def is_walrus(s3_url):
         return False
 
 
-def get_s3_connection(module, aws_connect_kwargs, location, rgw, s3_url):
+def get_s3_connection(module, aws_connect_kwargs, location, rgw, s3_url, sig_4=False):
     if s3_url and rgw:  # TODO - test this
         rgw = urlparse(s3_url)
         params = dict(module=module, conn_type='client', resource='s3', use_ssl=rgw.scheme == 'https', region=location, endpoint=s3_url, **aws_connect_kwargs)
@@ -607,6 +632,10 @@ def get_s3_connection(module, aws_connect_kwargs, location, rgw, s3_url):
         params = dict(module=module, conn_type='client', resource='s3', region=location, endpoint=walrus, **aws_connect_kwargs)
     else:
         params = dict(module=module, conn_type='client', resource='s3', region=location, endpoint=s3_url, **aws_connect_kwargs)
+        if module.params['mode'] == 'put' and module.params['encryption_mode'] == 'aws:kms':
+            params['config'] = botocore.client.Config(signature_version='s3v4')
+        elif module.params['mode'] in ('get', 'getstr') and sig_4:
+            params['config'] = botocore.client.Config(signature_version='s3v4')
     return boto3_conn(**params)
 
 
@@ -617,6 +646,7 @@ def main():
             bucket=dict(required=True),
             dest=dict(default=None, type='path'),
             encrypt=dict(default=True, type='bool'),
+            encryption_mode=dict(choices=['AES256', 'aws:kms'], default='AES256'),
             expiry=dict(default=600, type='int', aliases=['expiration']),
             headers=dict(type='dict'),
             marker=dict(default=""),
@@ -632,7 +662,8 @@ def main():
             s3_url=dict(aliases=['S3_URL']),
             rgw=dict(default='no', type='bool'),
             src=dict(),
-            ignore_nonexistent_bucket=dict(default=False, type='bool')
+            ignore_nonexistent_bucket=dict(default=False, type='bool'),
+            encryption_kms_key_id=dict()
         ),
     )
     module = AnsibleModule(
@@ -746,6 +777,10 @@ def main():
             if keysum_compare(module, dest, s3, bucket, obj, version=version):
                 sum_matches = True
                 if overwrite == 'always':
+                    try:
+                        download_s3file(module, s3, bucket, obj, dest, retries, version=version)
+                    except Sigv4Required:
+                        s3 = get_s3_connection(module, aws_connect_kwargs, location, rgw, s3_url, sig_4=True)
                         download_s3file(module, s3, bucket, obj, dest, retries, version=version)
                 else:
                     module.exit_json(msg="Local and remote object are identical, ignoring. Use overwrite=always parameter to force.", changed=False)
@@ -753,10 +788,18 @@ def main():
                 sum_matches = False
 
                 if overwrite in ('always', 'different'):
+                    try:
+                        download_s3file(module, s3, bucket, obj, dest, retries, version=version)
+                    except Sigv4Required:
+                        s3 = get_s3_connection(module, aws_connect_kwargs, location, rgw, s3_url, sig_4=True)
                         download_s3file(module, s3, bucket, obj, dest, retries, version=version)
                 else:
                     module.exit_json(msg="WARNING: Checksums do not match. Use overwrite parameter to force download.")
         else:
+            try:
+                download_s3file(module, s3, bucket, obj, dest, retries, version=version)
+            except Sigv4Required:
+                s3 = get_s3_connection(module, aws_connect_kwargs, location, rgw, s3_url, sig_4=True)
                 download_s3file(module, s3, bucket, obj, dest, retries, version=version)
 
     # if our mode is a PUT operation (upload), go through the procedure as appropriate ...
@@ -887,6 +930,10 @@ def main():
         if bucket and obj:
             keyrtn = key_check(module, s3, bucket, obj, version=version, validate=validate)
             if keyrtn:
+                try:
+                    download_s3str(module, s3, bucket, obj, version=version)
+                except Sigv4Required:
+                    s3 = get_s3_connection(module, aws_connect_kwargs, location, rgw, s3_url, sig_4=True)
                     download_s3str(module, s3, bucket, obj, version=version)
             elif version is not None:
                 module.fail_json(msg="Key %s with version id %s does not exist." % (obj, version))

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

@@ -1,6 +1,6 @@
 ---
 # tasks file for test_s3
-# ============================================================
+
 - name: set up aws connection info
   set_fact:
     aws_connection_info: &aws_connection_info
@@ -9,7 +9,8 @@
       security_token: "{{ security_token }}"
       region: "{{ aws_region }}"
   no_log: yes
-# ============================================================
+
+- block:
     - name: test create bucket
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -20,7 +21,7 @@
       assert:
         that:
           - result.changed == True
-# ============================================================
+
     - name: trying to create a bucket name that already exists
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -31,7 +32,7 @@
       assert:
         that:
           - result.changed == False
-# ============================================================
+
     - name: create temporary file object to put in a bucket
       tempfile:
       register: tmp1
@@ -48,7 +49,7 @@
         path: "{{ tmp1.path }}"
         get_checksum: yes
       register: file1stat
-# ============================================================
+
     - name: test putting an object in the bucket
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -64,7 +65,7 @@
         that:
           - result.changed == True
           - result.msg == "PUT operation complete"
-# ============================================================
+
     - name: check that roles file lookups work as expected
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -89,10 +90,11 @@
       retries: 3
       delay: 3
       register: result
-# ============================================================
+
     - name: create a second temp file to download the object from the bucket
       tempfile:
       register: tmp2
+
     - name: test get object
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -113,7 +115,7 @@
       assert:
         that:
           - file1stat.stat.checksum == file2stat.stat.checksum
-# ============================================================
+
     - name: test geturl of the object
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -129,7 +131,7 @@
         that:
           - "'Download url:' in result.msg"
           - result.changed == True
-# ============================================================
+
     - name: test getstr of the object
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -144,7 +146,7 @@
         that:
           - result.msg == "GET operation complete"
           - result.contents == content
-# ============================================================
+
     - name: test list to get all objects in the bucket
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -158,7 +160,7 @@
         that:
           - "'delete.txt' in result.s3_keys"
           - result.msg == "LIST operation complete"
-# ============================================================
+
     - name: test delobj to just delete an object in the bucket
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -178,7 +180,133 @@
         that:
           - "'Object deleted from bucket' in result.msg"
           - result.changed == True
-# ============================================================
+    - name: clean up temp file
+      file:
+        path: "{{ tmp2.path }}"
+        state: absent
+
+    - name: test putting an encrypted object in the bucket
+      aws_s3:
+        bucket: "{{ bucket_name }}"
+        mode: put
+        src: "{{ tmp1.path }}"
+        encrypt: yes
+        object: delete_encrypt.txt
+        <<: *aws_connection_info
+      retries: 3
+      delay: 3
+      register: result
+    - name: assert object exists
+      assert:
+        that:
+          - result.changed == True
+          - result.msg == "PUT operation complete"
+
+    - name: create a second temp file to download the object from the bucket
+      tempfile:
+      register: tmp2
+    - name: test get encrypted object
+      aws_s3:
+        bucket: "{{ bucket_name }}"
+        mode: get
+        dest: "{{ tmp2.path }}"
+        object: delete_encrypt.txt
+        <<: *aws_connection_info
+      retries: 3
+      delay: 3
+      register: result
+      until: "result.msg == 'GET operation complete'"
+    - name: get the stat of the file so we can compare the checksums
+      stat:
+        path: "{{ tmp2.path }}"
+        get_checksum: yes
+      register: file2stat
+    - name: assert checksums are the same
+      assert:
+        that:
+          - file1stat.stat.checksum == file2stat.stat.checksum
+    - name: delete encrypted file
+      aws_s3:
+        bucket: "{{ bucket_name }}"
+        mode: delobj
+        object: delete_encrypt.txt
+        <<: *aws_connection_info
+      retries: 3
+      delay: 3
+    - name: clean up temp file
+      file:
+        path: "{{ tmp2.path }}"
+        state: absent
+
+    - name: test putting an aws:kms encrypted object in the bucket
+      aws_s3:
+        bucket: "{{ bucket_name }}"
+        mode: put
+        src: "{{ tmp1.path }}"
+        encrypt: yes
+        encryption_mode: aws:kms
+        object: delete_encrypt_kms.txt
+        <<: *aws_connection_info
+      retries: 3
+      delay: 3
+      register: result
+    - name: assert object exists
+      assert:
+        that:
+          - result.changed == True
+          - result.msg == "PUT operation complete"
+
+    - name: create a second temp file to download the object from the bucket
+      tempfile:
+      register: tmp2
+    - name: test get KMS encrypted object
+      aws_s3:
+        bucket: "{{ bucket_name }}"
+        mode: get
+        dest: "{{ tmp2.path }}"
+        object: delete_encrypt_kms.txt
+        <<: *aws_connection_info
+      retries: 3
+      delay: 3
+      register: result
+      until: "result.msg == 'GET operation complete'"
+    - name: get the stat of the file so we can compare the checksums
+      stat:
+        path: "{{ tmp2.path }}"
+        get_checksum: yes
+      register: file2stat
+    - name: assert checksums are the same
+      assert:
+        that:
+          - file1stat.stat.checksum == file2stat.stat.checksum
+      # FIXME - could use a test that checks uploaded file is *actually* aws:kms encrypted
+    - name: test get KMS encrypted object using v4 signature
+      aws_s3:
+        bucket: "{{ bucket_name }}"
+        mode: get
+        dest: "{{ tmp2.path }}"
+        object: delete_encrypt_kms.txt
+        <<: *aws_connection_info
+      retries: 3
+      delay: 3
+      until: "result.msg == 'GET operation complete'"
+    - name: delete KMS encrypted file
+      aws_s3:
+        bucket: "{{ bucket_name }}"
+        mode: delobj
+        object: delete_encrypt_kms.txt
+        <<: *aws_connection_info
+      retries: 3
+      delay: 3
+    - name: clean up temp file
+      file:
+        path: "{{ tmp2.path }}"
+        state: absent
+
+    # FIXME: could use a test that checks non standard KMS key
+    #        but that would require ability to create and remove such keys.
+    #        PRs exist for that, but propose deferring until after merge.
+
     - name: test creation of empty path
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -201,7 +329,7 @@
         <<: *aws_connection_info
       retries: 3
       delay: 3
-# ============================================================
+
     - name: test delete bucket
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -215,16 +343,7 @@
       assert:
         that:
           - result.changed == True
-# ============================================================
-- name: delete temporary file 1
-  file:
-    state: absent
-    path: "{{ tmp1.path }}"
-- name: delete temporary file 2
-  file:
-    state: absent
-    path: "{{ tmp2.path }}"
-# ============================================================
+
     - name: test create a bucket with a dot in the name
       aws_s3:
         bucket: "{{ bucket_name + '.bucket' }}"
@@ -235,7 +354,7 @@
       assert:
         that:
           - result.changed == True
-# ============================================================
+
     - name: test delete a bucket with a dot in the name
       aws_s3:
         bucket: "{{ bucket_name + '.bucket' }}"
@@ -246,7 +365,7 @@
       assert:
         that:
           - result.changed == True
-# ============================================================
+
     - name: test delete a nonexistent bucket
       aws_s3:
         bucket: "{{ bucket_name + '.bucket' }}"
@@ -257,10 +376,6 @@
       assert:
         that:
           - result.changed == False
-# ============================================================
-- name: create a tempfile for the path
-  tempfile:
-  register: tmp1
 
     - name: make tempfile 4 GB for OSX
       command:
@@ -270,7 +385,7 @@
     - name: make tempfile 4 GB for linux
       command:
         _raw_params: "dd if=/dev/zero of={{ tmp1.path }} bs=1M count=4096"
-  when: ansible_distribution == 'Linux'
+      when: ansible_system == 'Linux'
 
     - name: test multipart download - platform specific
       block:
@@ -292,7 +407,7 @@
           aws_s3:
             bucket: "{{ bucket_name }}"
             mode: get
-        dest: /tmp/multipart_download.txt
+            dest: "{{ tmp2.path }}"
             object: multipart.txt
             overwrite: different
             <<: *aws_connection_info
@@ -310,7 +425,7 @@
           aws_s3:
             bucket: "{{ bucket_name }}"
             mode: get
-        dest: /tmp/multipart_download.txt
+            dest: "{{ tmp2.path }}"
             object: multipart.txt
             overwrite: different
             <<: *aws_connection_info
@@ -320,22 +435,39 @@
           assert:
             that:
               - not result.changed
+      when: ansible_system == 'Linux' or ansible_distribution == 'MacOSX'
 
-    - name: delete file used for upload
+  always:
+    ###### TEARDOWN STARTS HERE ######
+
+    - name: remove uploaded files
+      aws_s3:
+        bucket: "{{ bucket_name }}"
+        mode: delobj
+        object: "{{ item }}"
+        <<: *aws_connection_info
+      with_items:
+        - hello.txt
+        - delete.txt
+        - delete_encrypt.txt
+        - delete_encrypt_kms.txt
+      ignore_errors: yes
+
+    - name: delete temporary file 1
       file:
         state: absent
         path: "{{ tmp1.path }}"
+      ignore_errors: yes
 
-    - name: delete downloaded file
+    - name: delete temporary file 2
       file:
         state: absent
-        path: /tmp/multipart_download.txt
+        path: "{{ tmp2.path }}"
+      ignore_errors: yes
 
     - name: delete the bucket
       aws_s3:
         bucket: "{{ bucket_name }}"
         mode: delete
         <<: *aws_connection_info
-
-  when: ansible_distribution in ['MacOSX', 'Linux']
-# ============================================================
+      ignore_errors: yes