Merge pull request #1026 from kyleknap/encrypted-md5

Do not check md5 for downloading kms objects

CHANGELOG.rst

@@ -7,7 +7,10 @@ Next Release (TBD)
 
 * bugfix:``aws s3``: Fix issue where requests were not being
   resigned correctly when using Signature Version 4
-  (`botocore issue 388 https://github.com/boto/botocore/pull/388>`__)
+  (`botocore issue 388 <https://github.com/boto/botocore/pull/388>`__)
+* bugfix:``aws s3``: Fix issue where KMS encrypted objects could not be
+  downloaded
+  (`issue 1026 <https://github.com/aws/aws-cli/pull/1026>`__)
 
 
 1.6.4

awscli/customizations/s3/fileinfo.py

@@ -35,6 +35,7 @@ def save_file(filename, response_data, last_update, is_stream=False):
     """
     body = response_data['Body']
     etag = response_data['ETag'][1:-1]
+    sse = response_data.get('ServerSideEncryption', None)
     if not is_stream:
         d = os.path.dirname(filename)
         try:
@@ -55,7 +56,7 @@ def save_file(filename, response_data, last_update, is_stream=False):
         with open(filename, 'wb') as out_file:
             write_to_file(out_file, etag, md5, file_chunks)
 
-    if not _is_multipart_etag(etag):
+    if not _is_multipart_etag(etag) and sse != 'aws:kms':
         if etag != md5.hexdigest():
             if not is_stream:
                 os.remove(filename)

tests/integration/customizations/s3/test_plugin.py

@@ -101,11 +101,16 @@ def create_bucket(self, name=None, region=None):
         self.addCleanup(self.delete_bucket, bucket_name)
         return bucket_name
 
-    def put_object(self, bucket_name, key_name, contents=''):
+    def put_object(self, bucket_name, key_name, contents='', extra_args=None):
         operation = self.service.get_operation('PutObject')
         endpoint = self.service.get_endpoint(self.regions[bucket_name])
-        http = operation.call(endpoint, bucket=bucket_name,
-                              key=key_name, body=contents)[0]
+        call_args = {
+            'endpoint': endpoint, 'bucket': bucket_name,
+            'key': key_name, 'body': contents
+        }
+        if extra_args is not None:
+            call_args.update(extra_args)
+        http = operation.call(**call_args)[0]
         self.assertEqual(http.status_code, 200)
         self.addCleanup(self.delete_key, bucket_name, key_name)
 
@@ -472,6 +477,24 @@ def test_download_non_existent_key(self):
             'HeadObject operation: Key "foo.txt" does not exist')
         self.assertIn(expected_err_msg, p.stderr)
 
+    def test_download_encrypted_kms_object(self):
+        bucket_name = self.create_bucket(region='eu-central-1')
+        extra_args = {
+            'server_side_encryption': 'aws:kms',
+            'ssekms_key_id': 'alias/aws/s3'
+        }
+        object_name = 'foo.txt'
+        contents = 'this is foo.txt'
+        self.put_object(bucket_name, object_name, contents,
+                        extra_args=extra_args)
+        local_filename = self.files.full_path('foo.txt')
+        p = aws('s3 cp s3://%s/%s %s --region eu-central-1' %
+                (bucket_name, object_name, local_filename))
+        self.assertEqual(p.rc, 0)
+        # Assert that the file was downloaded properly.
+        with open(local_filename, 'r') as f:
+            self.assertEqual(f.read(), contents)
+
 
 class TestSync(BaseS3CLICommand):
     def test_sync_with_plus_chars_paginate(self):

tests/unit/customizations/s3/test_fileinfo.py

@@ -74,6 +74,26 @@ def test_stream_file_md5_error(self):
             # Make sure nothing is written to stdout.
             self.assertEqual(mock_stdout.getvalue(), "")
 
+    def test_raise_md5_with_no_kms_sse(self):
+        # Ensure MD5 is checked if the sse algorithm is not kms.
+        self.response_data['ETag'] = '"0"'
+        self.response_data['ServerSideEncryption'] = 'AES256'
+        # Should raise a md5 error.
+        with self.assertRaises(MD5Error):
+            fileinfo.save_file(self.filename, self.response_data,
+                               self.last_update)
+        # The file should not have been saved.
+        self.assertFalse(os.path.isfile(self.filename))
+
+    def test_no_raise_md5_with_kms(self):
+        # Ensure MD5 is not checked when kms is used by providing a bad MD5.
+        self.response_data['ETag'] = '"0"'
+        self.response_data['ServerSideEncryption'] = 'aws:kms'
+        # Should not raise any md5 error.
+        fileinfo.save_file(self.filename, self.response_data, self.last_update)
+        # The file should have been saved.
+        self.assertTrue(os.path.isfile(self.filename))
+
 
 class TestSetSizeFromS3(unittest.TestCase):
     def test_set_size_from_s3(self):