fix: perform integrity checks for remote function execution #3854

rohangujarathi · 2023-05-11T22:36:00Z

Issue #, if available:

Description of changes:

Testing done:

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

I have read the CONTRIBUTING doc
I certify that the changes I am introducing will be backward compatible, and I have discussed concerns about this, if any, with the Python SDK team
I used the commit message format described in CONTRIBUTING
I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
I have checked that my tests are not configured for a specific region or account (if appropriate)
I have used unique_name_from_base to create resource names in integ tests (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

sagemaker-bot · 2023-05-11T22:40:24Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: b30716f
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-11T23:07:01Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: a47f24c
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-12T00:19:33Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 4ed20dd
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-12T00:27:46Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 4ed20dd
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-12T00:32:08Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 4ed20dd
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-12T00:44:40Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 4ed20dd
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

codecov-commenter · 2023-05-12T00:45:52Z

Codecov Report

Merging #3854 (4d59690) into master (fd90b1d) will decrease coverage by 0.71%.
The diff coverage is 98.55%.

@@            Coverage Diff             @@
##           master    #3854      +/-   ##
==========================================
- Coverage   89.96%   89.25%   -0.71%     
==========================================
  Files        1140      267     -873     
  Lines      105300    26056   -79244     
==========================================
- Hits        94733    23257   -71476     
+ Misses      10567     2799    -7768

Impacted Files	Coverage Δ
...rc/sagemaker/remote_function/core/serialization.py	`96.07% <97.67%> (ø)`
src/sagemaker/remote_function/client.py	`96.90% <100.00%> (ø)`
.../sagemaker/remote_function/core/stored_function.py	`100.00% <100.00%> (ø)`
src/sagemaker/remote_function/errors.py	`100.00% <100.00%> (ø)`
src/sagemaker/remote_function/invoke_function.py	`77.35% <100.00%> (ø)`
src/sagemaker/remote_function/job.py	`88.93% <100.00%> (ø)`
...agemaker/serverless/serverless_inference_config.py	`100.00% <100.00%> (ø)`

... and 1400 files with indirect coverage changes

sagemaker-bot · 2023-05-12T01:22:25Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 4ed20dd
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

nmadan · 2023-05-12T18:56:22Z

src/sagemaker/remote_function/core/serialization.py


+        if not sha256_hash:
+            raise DeserializationError(
+                "Corrupt metadata file. SHA256 hash for the serialized data do not exist"


nit: typo "does not exist"

nmadan · 2023-05-12T19:02:31Z

src/sagemaker/remote_function/core/serialization.py

+    actual_hash_value = _compute_hash(buffer=buffer, secret_key=secret_key)
+    if expected_hash_value != actual_hash_value:
+        raise DeserializationError(
+            "Integrity check for the serialized function failed. "


nit: not just function right? It could be data too.

nmadan · 2023-05-12T19:04:47Z

src/sagemaker/remote_function/invoke_function.py

        s3_kms_key = args.s3_kms_key
        run_in_context = args.run_in_context

+        hmac_key = os.getenv("SECRET_KEY")


nit: we should give this env variable a unique name - something the user will not overwrite

sagemaker-bot · 2023-05-12T20:42:02Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 5ca6ec0
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-12T20:45:50Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 5ca6ec0
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-12T20:50:45Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 5ca6ec0
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-12T21:02:20Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 5ca6ec0
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-12T22:55:58Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 5ca6ec0
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

dpatro · 2023-05-12T23:35:15Z

src/sagemaker/remote_function/core/serialization.py

+    if expected_hash_value != actual_hash_value:
+        raise DeserializationError(
+            "Integrity check for the serialized function or data failed. "
+            "Please restrict access to your S3 bucket"
+        )


The best practice for comparing hmac is using the function hmac.compare_digest(expected_hash_value, actual_hash_value) instead of doing a ==.

Refer: https://docs.python.org/3/library/hmac.html#hmac.compare_digest

Thanks, Addressed

sagemaker-bot · 2023-05-13T01:00:05Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 4d59690
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-13T01:07:13Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 4d59690
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-13T01:11:33Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 4d59690
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-13T01:23:03Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 4d59690
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-13T01:24:59Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 4d59690
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-05-15T01:20:31Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 4d59690
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

rohangujarathi force-pushed the security_fix branch from b30716f to a47f24c Compare May 11, 2023 22:58

rohangujarathi force-pushed the security_fix branch from a47f24c to 4ed20dd Compare May 11, 2023 23:36

mufaddal-rohawala self-assigned this May 12, 2023

nmadan reviewed May 12, 2023

View reviewed changes

rohangujarathi force-pushed the security_fix branch 2 times, most recently from 025c7a9 to 5ca6ec0 Compare May 12, 2023 19:55

nmadan approved these changes May 12, 2023

View reviewed changes

dpatro reviewed May 13, 2023

View reviewed changes

rohangujarathi force-pushed the security_fix branch from 5ca6ec0 to 2319eaf Compare May 13, 2023 00:15

fix: perform integrity checks for remote function execution

4d59690

rohangujarathi force-pushed the security_fix branch from 2319eaf to 4d59690 Compare May 13, 2023 00:15

dpatro approved these changes May 13, 2023

View reviewed changes

rohangujarathi marked this pull request as ready for review May 15, 2023 21:25

rohangujarathi requested a review from a team as a code owner May 15, 2023 21:25

rohangujarathi requested review from knikure and removed request for a team May 15, 2023 21:25

rohangujarathi marked this pull request as draft May 15, 2023 22:31

rohangujarathi marked this pull request as ready for review May 16, 2023 17:27

knikure approved these changes May 16, 2023

View reviewed changes

knikure merged commit 1c3f4b1 into aws:master May 16, 2023

fix: perform integrity checks for remote function execution #3854

fix: perform integrity checks for remote function execution #3854

Uh oh!

Conversation

rohangujarathi commented May 11, 2023

Merge Checklist

General

Tests

Uh oh!

sagemaker-bot commented May 11, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 11, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 12, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 12, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 12, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 12, 2023

AWS CodeBuild CI Report

Uh oh!

codecov-commenter commented May 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sagemaker-bot commented May 12, 2023

AWS CodeBuild CI Report

Uh oh!

nmadan May 12, 2023

Choose a reason for hiding this comment

Uh oh!

nmadan May 12, 2023

Choose a reason for hiding this comment

Uh oh!

nmadan May 12, 2023

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented May 12, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 12, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 12, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 12, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 12, 2023

AWS CodeBuild CI Report

Uh oh!

dpatro May 12, 2023

Choose a reason for hiding this comment

Uh oh!

rohangujarathi May 13, 2023

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented May 13, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 13, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 13, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 13, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 13, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented May 15, 2023

codecov-commenter commented May 12, 2023 •

edited

Loading