Skip to content

Commit 4d6a50b

Browse files
ashovlinashovlin
authored
Fix handling of SSE-C keys when copying unencrypted to encrypted objects or objects with different keys (#9559)
1 parent c02348d commit 4d6a50b

7 files changed

Lines changed: 435 additions & 11 deletions

File tree

‎.changes/next-release/bugfix-s3-83199.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"type": "bugfix",
3+
"category": "``s3``",
4+
"description": "Fix handling of SSE-C keys when copying unencrypted to encrypted objects or objects with different encryption keys"
5+
}

‎awscli/customizations/s3/subcommands.py‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1608,16 +1608,15 @@ def _map_sse_c_params(self, request_parameters, paths_type):
16081608
# SSE-C key and algorithm. Note the reverse FileGenerator does
16091609
# not need any of these because it is used only for sync operations
16101610
# which only use ListObjects which does not require HeadObject.
1611-
RequestParamsMapper.map_head_object_params(
1612-
request_parameters['HeadObject'], self.parameters
1613-
)
16141611
if paths_type == 's3s3':
1612+
RequestParamsMapper.map_head_object_params_with_copy_source_sse(
1613+
request_parameters['HeadObject'],
1614+
self.parameters,
1615+
)
1616+
else:
16151617
RequestParamsMapper.map_head_object_params(
16161618
request_parameters['HeadObject'],
1617-
{
1618-
'sse_c': self.parameters.get('sse_c_copy_source'),
1619-
'sse_c_key': self.parameters.get('sse_c_copy_source_key'),
1620-
},
1619+
self.parameters,
16211620
)
16221621

16231622
def _get_s3_handler_params(self):

‎awscli/customizations/s3/subscribers.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,9 @@ def _get_head_object_response(self, future):
330330
'Bucket': copy_source['Bucket'],
331331
'Key': copy_source['Key'],
332332
}
333-
utils.RequestParamsMapper.map_head_object_params(
334-
head_object_params, self._cli_params
333+
utils.RequestParamsMapper.map_head_object_params_with_copy_source_sse(
334+
head_object_params,
335+
self._cli_params,
335336
)
336337
return self._client.head_object(**head_object_params)
337338

‎awscli/customizations/s3/utils.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,19 @@ def map_head_object_params(cls, request_params, cli_params):
529529
cls._set_sse_c_request_params(request_params, cli_params)
530530
cls._set_request_payer_param(request_params, cli_params)
531531

532+
@classmethod
533+
def map_head_object_params_with_copy_source_sse(
534+
cls, request_params, cli_params
535+
):
536+
"""
537+
Map CLI params to HeadObject request params,
538+
using the SSE-C headers from the copy source
539+
"""
540+
cls._set_sse_c_request_params_with_copy_source_sse(
541+
request_params, cli_params
542+
)
543+
cls._set_request_payer_param(request_params, cli_params)
544+
532545
@classmethod
533546
def map_create_multipart_upload_params(cls, request_params, cli_params):
534547
"""Map CLI params to CreateMultipartUpload request params"""
@@ -662,6 +675,18 @@ def _set_sse_c_request_params(cls, request_params, cli_params):
662675
request_params['SSECustomerAlgorithm'] = cli_params['sse_c']
663676
request_params['SSECustomerKey'] = cli_params['sse_c_key']
664677

678+
@classmethod
679+
def _set_sse_c_request_params_with_copy_source_sse(
680+
cls, request_params, cli_params
681+
):
682+
if cli_params.get('sse_c_copy_source'):
683+
request_params['SSECustomerAlgorithm'] = cli_params[
684+
'sse_c_copy_source'
685+
]
686+
request_params['SSECustomerKey'] = cli_params[
687+
'sse_c_copy_source_key'
688+
]
689+
665690
@classmethod
666691
def _set_sse_c_copy_source_request_params(cls, request_params, cli_params):
667692
if cli_params.get('sse_c_copy_source'):

‎tests/functional/s3/test_cp_command.py‎

Lines changed: 191 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ class BaseCPCommandTest(BaseS3TransferCommandTest):
4444

4545

4646
class TestCPCommand(BaseCPCommandTest):
47+
def setUp(self):
48+
super().setUp()
49+
self.multipart_threshold = 8 * MB
50+
4751
def test_operations_used_in_upload(self):
4852
full_path = self.files.create_file('foo.txt', 'mycontent')
4953
cmdline = '%s %s s3://bucket/key.txt' % (self.prefix, full_path)
@@ -411,7 +415,7 @@ def test_no_overwrite_flag_on_copy_when_small_object_exists_on_target(
411415
):
412416
cmdline = f'{self.prefix} s3://bucket1/key.txt s3://bucket/key.txt --no-overwrite'
413417
self.parsed_responses = [
414-
self.head_object_response(ContentLength=5),
418+
self.head_object_response(ContentLength=5),
415419
self.precondition_failed_error_response(),
416420
]
417421
self.run_cmd(cmdline, expected_rc=0)
@@ -978,8 +982,15 @@ def test_cp_with_sse_c_copy_source_fileb(self):
978982
self.run_cmd(cmdline, expected_rc=0)
979983
self.assertEqual(len(self.operations_called), 2)
980984
self.assertEqual(self.operations_called[0][0].name, 'HeadObject')
981-
self.assertEqual(self.operations_called[1][0].name, 'CopyObject')
985+
expected_head_args = {
986+
'Bucket': 'bucket-one',
987+
'Key': 'key.txt',
988+
'SSECustomerAlgorithm': 'AES256',
989+
'SSECustomerKey': key_contents,
990+
}
991+
self.assertDictEqual(self.operations_called[0][1], expected_head_args)
982992

993+
self.assertEqual(self.operations_called[1][0].name, 'CopyObject')
983994
expected_args = {
984995
'Key': 'key.txt',
985996
'Bucket': 'bucket',
@@ -989,6 +1000,184 @@ def test_cp_with_sse_c_copy_source_fileb(self):
9891000
}
9901001
self.assertDictEqual(self.operations_called[1][1], expected_args)
9911002

1003+
def test_s3s3_cp_with_destination_sse_c(self):
1004+
"""S3->S3 copy with an unencrypted source and encrypted destination"""
1005+
self.parsed_responses = [
1006+
self.head_object_response(),
1007+
self.copy_object_response(),
1008+
]
1009+
cmdline = (
1010+
'%s s3://bucket-one/key.txt s3://bucket/key.txt '
1011+
'--sse-c --sse-c-key destination-key' % self.prefix
1012+
)
1013+
self.run_cmd(cmdline, expected_rc=0)
1014+
self.assertEqual(len(self.operations_called), 2)
1015+
self.assertEqual(self.operations_called[0][0].name, 'HeadObject')
1016+
expected_head_args = {
1017+
'Bucket': 'bucket-one',
1018+
'Key': 'key.txt',
1019+
# don't expect to see SSE-c params for the source
1020+
}
1021+
self.assertDictEqual(self.operations_called[0][1], expected_head_args)
1022+
1023+
self.assertEqual(self.operations_called[1][0].name, 'CopyObject')
1024+
expected_copy_args = {
1025+
'Key': 'key.txt',
1026+
'Bucket': 'bucket',
1027+
'CopySource': {'Bucket': 'bucket-one', 'Key': 'key.txt'},
1028+
'SSECustomerAlgorithm': 'AES256',
1029+
'SSECustomerKey': 'destination-key',
1030+
}
1031+
self.assertDictEqual(self.operations_called[1][1], expected_copy_args)
1032+
1033+
def test_s3s3_cp_with_different_sse_c_keys(self):
1034+
"""S3->S3 copy with different SSE-C keys for source and destination"""
1035+
self.parsed_responses = [
1036+
self.head_object_response(),
1037+
self.copy_object_response(),
1038+
]
1039+
cmdline = (
1040+
'%s s3://bucket-one/key.txt s3://bucket/key.txt '
1041+
'--sse-c-copy-source --sse-c-copy-source-key foo --sse-c --sse-c-key bar'
1042+
% self.prefix
1043+
)
1044+
self.run_cmd(cmdline, expected_rc=0)
1045+
self.assertEqual(len(self.operations_called), 2)
1046+
self.assertEqual(self.operations_called[0][0].name, 'HeadObject')
1047+
expected_head_args = {
1048+
'Bucket': 'bucket-one',
1049+
'Key': 'key.txt',
1050+
'SSECustomerAlgorithm': 'AES256',
1051+
'SSECustomerKey': 'foo',
1052+
}
1053+
self.assertDictEqual(self.operations_called[0][1], expected_head_args)
1054+
1055+
self.assertEqual(self.operations_called[1][0].name, 'CopyObject')
1056+
expected_copy_args = {
1057+
'Key': 'key.txt',
1058+
'Bucket': 'bucket',
1059+
'CopySource': {'Bucket': 'bucket-one', 'Key': 'key.txt'},
1060+
'SSECustomerAlgorithm': 'AES256',
1061+
'SSECustomerKey': 'bar',
1062+
'CopySourceSSECustomerAlgorithm': 'AES256',
1063+
'CopySourceSSECustomerKey': 'foo',
1064+
}
1065+
self.assertDictEqual(self.operations_called[1][1], expected_copy_args)
1066+
1067+
def test_s3s3_cp_with_destination_sse_c_multipart(self):
1068+
"""S3->S3 multipart copy with unencrypted source and encrypted destination"""
1069+
self.parsed_responses = [
1070+
self.head_object_response(ContentLength=self.multipart_threshold),
1071+
self.get_object_tagging_response({}),
1072+
self.create_mpu_response('upload_id'),
1073+
self.upload_part_copy_response(),
1074+
self.complete_mpu_response(),
1075+
]
1076+
cmdline = (
1077+
'%s s3://bucket-one/key.txt s3://bucket/key.txt '
1078+
'--sse-c --sse-c-key destination-key' % self.prefix
1079+
)
1080+
self.run_cmd(cmdline, expected_rc=0)
1081+
self.assert_operations_called(
1082+
[
1083+
self.head_object_request(
1084+
'bucket-one',
1085+
'key.txt',
1086+
# no SSE-C params — source is unencrypted
1087+
),
1088+
('GetObjectTagging', mock.ANY),
1089+
self.create_mpu_request(
1090+
'bucket',
1091+
'key.txt',
1092+
SSECustomerAlgorithm='AES256',
1093+
SSECustomerKey='destination-key',
1094+
),
1095+
self.upload_part_copy_request(
1096+
'bucket-one',
1097+
'key.txt',
1098+
'bucket',
1099+
'key.txt',
1100+
'upload_id',
1101+
PartNumber=mock.ANY,
1102+
CopySourceRange=mock.ANY,
1103+
CopySourceIfMatch='"foo-1"',
1104+
SSECustomerAlgorithm='AES256',
1105+
SSECustomerKey='destination-key',
1106+
),
1107+
self.complete_mpu_request(
1108+
'bucket',
1109+
'key.txt',
1110+
'upload_id',
1111+
num_parts=1,
1112+
SSECustomerAlgorithm='AES256',
1113+
SSECustomerKey='destination-key',
1114+
),
1115+
]
1116+
)
1117+
1118+
def test_s3s3_cp_with_different_sse_c_keys_multipart(self):
1119+
"""S3->S3 multipart copy with different SSE-C keys"""
1120+
self.parsed_responses = [
1121+
self.head_object_response(ContentLength=self.multipart_threshold),
1122+
self.get_object_tagging_response({}),
1123+
self.create_mpu_response('upload_id'),
1124+
self.upload_part_copy_response(),
1125+
self.complete_mpu_response(),
1126+
]
1127+
cmdline = (
1128+
'%s s3://bucket-one/key.txt s3://bucket/key.txt '
1129+
'--sse-c-copy-source --sse-c-copy-source-key source-key --sse-c --sse-c-key destination-key'
1130+
% self.prefix
1131+
)
1132+
self.run_cmd(cmdline, expected_rc=0)
1133+
self.assertEqual(self.operations_called[0][0].name, 'HeadObject')
1134+
expected_head_args = {
1135+
'Bucket': 'bucket-one',
1136+
'Key': 'key.txt',
1137+
'SSECustomerAlgorithm': 'AES256',
1138+
'SSECustomerKey': 'source-key',
1139+
}
1140+
self.assertDictEqual(self.operations_called[0][1], expected_head_args)
1141+
self.assert_operations_called(
1142+
[
1143+
self.head_object_request(
1144+
'bucket-one',
1145+
'key.txt',
1146+
SSECustomerAlgorithm='AES256',
1147+
SSECustomerKey='source-key',
1148+
),
1149+
('GetObjectTagging', mock.ANY),
1150+
self.create_mpu_request(
1151+
'bucket',
1152+
'key.txt',
1153+
SSECustomerAlgorithm='AES256',
1154+
SSECustomerKey='destination-key',
1155+
),
1156+
self.upload_part_copy_request(
1157+
'bucket-one',
1158+
'key.txt',
1159+
'bucket',
1160+
'key.txt',
1161+
'upload_id',
1162+
PartNumber=mock.ANY,
1163+
CopySourceRange=mock.ANY,
1164+
CopySourceIfMatch='"foo-1"',
1165+
SSECustomerAlgorithm='AES256',
1166+
SSECustomerKey='destination-key',
1167+
CopySourceSSECustomerAlgorithm='AES256',
1168+
CopySourceSSECustomerKey='source-key',
1169+
),
1170+
self.complete_mpu_request(
1171+
'bucket',
1172+
'key.txt',
1173+
'upload_id',
1174+
num_parts=1,
1175+
SSECustomerAlgorithm='AES256',
1176+
SSECustomerKey='destination-key',
1177+
),
1178+
]
1179+
)
1180+
9921181
# Note ideally the kms sse with a key id would be integration tests
9931182
# However, you cannot delete kms keys so there would be no way to clean
9941183
# up the tests

0 commit comments

Comments
 (0)