Skip to content

fix(core): don't send kms_key as an SSE-C key in S3Downloader.download #6414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AkshayShah03 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(core): don't send kms_key as an SSE-C key in S3Downloader.download #6414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions sagemaker-core/src/sagemaker/core/s3/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ def download(s3_uri, local_path, kms_key=None, sagemaker_session=None):
Args:
s3_uri (str): An S3 uri to download from.
local_path (str): A local path to download the file(s) to.
kms_key (str): The KMS key to use to decrypt the files.
kms_key (str): Not used. S3 decrypts objects encrypted with SSE-KMS
transparently, so no key needs to be sent with the download request.
Kept for backwards compatibility.
sagemaker_session (sagemaker.core.helper.session_helper.Session): Session object which
manages interactions with Amazon SageMaker APIs and any other
AWS services needed. If not specified, one is created
Expand All @@ -154,13 +156,12 @@ def download(s3_uri, local_path, kms_key=None, sagemaker_session=None):
"""
sagemaker_session = sagemaker_session or Session()
bucket, key_prefix = parse_s3_url(url=s3_uri)
if kms_key is not None:
extra_args = {"SSECustomerKey": kms_key}
else:
extra_args = None

# Do not pass kms_key as SSECustomerKey: that header is for SSE-C (a raw
# customer-provided key), and sending a KMS key ARN in it makes S3 reject the
# request with 400 Bad Request.
return sagemaker_session.download_data(
path=local_path, bucket=bucket, key_prefix=key_prefix, extra_args=extra_args
path=local_path, bucket=bucket, key_prefix=key_prefix, extra_args=None
)

@staticmethod
Expand Down
4 changes: 3 additions & 1 deletion sagemaker-core/tests/unit/s3/test_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ def test_download_with_kms_key(sagemaker_session):
path="/path/for/download/",
bucket=BUCKET_NAME,
key_prefix=os.path.join(CURRENT_JOB_NAME, SOURCE_NAME),
extra_args={"SSECustomerKey": KMS_KEY},
# SSE-KMS objects are decrypted by S3 transparently; the KMS key must not be
# sent as an SSE-C customer key.
extra_args=None,
)


Expand Down
Loading