Skip to content

fix: Replace HMAC with SHA based integrity checks in model builder.#5656

Open
pravali96 wants to merge 1 commit intoaws:master-v2from
pravali96:fix/triton-hmac-security-v2-all-bugs
Open

fix: Replace HMAC with SHA based integrity checks in model builder.#5656
pravali96 wants to merge 1 commit intoaws:master-v2from
pravali96:fix/triton-hmac-security-v2-all-bugs

Conversation

@pravali96
Copy link
Copy Markdown
Collaborator

@pravali96 pravali96 commented Mar 19, 2026

Issue #, if available:

Description of changes:
Replace HMAC with SHA based integrity checks in model builder.

Issue

Fix

Switch from HMAC-SHA256 (requires a secret key) to plain SHA-256 (no key needed)

Changes

check_integrity.py

  • Removed generate_secret_key() — no longer needed
  • compute_hash() now uses hashlib.sha256() instead of hmac.new()
  • perform_integrity_check() no longer reads SAGEMAKER_SERVE_SECRET_KEY from environment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@pravali96 pravali96 requested a review from a team as a code owner March 19, 2026 22:58
@pravali96 pravali96 requested a review from mollyheamazon March 19, 2026 22:58
@pravali96 pravali96 force-pushed the fix/triton-hmac-security-v2-all-bugs branch from 6e9283e to 8fe96c8 Compare March 20, 2026 17:48
@pravali96 pravali96 force-pushed the fix/triton-hmac-security-v2-all-bugs branch from c8d05ec to 079e1f2 Compare March 20, 2026 19:08
@pravali96 pravali96 changed the title fix: Security fixes for Triton HMAC key exposure and missing integrit… fix: fixes for Triton HMAC key exposure and missing integrit… Mar 31, 2026
@pravali96 pravali96 changed the title fix: fixes for Triton HMAC key exposure and missing integrit… fix: Replace HMAC with SHA based integrity checks in model builder. Mar 31, 2026
Copy link
Copy Markdown
Contributor

@nargokul nargokul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix integ test failures.

metadata.write(_MetaData(hash_value).to_json())

return secret_key
return ""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We need not return anything here

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 11 failing tests are in JumpStart, not model builder. They are unrelated to these changes. is that a blocker?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not needed for long-term but its better to keep return "" for backward compatibility in this security fix. Callers (self.secret_key = prepare_for_mms(...)) still assign this return value, so it cannot be none.

@pravali96 pravali96 force-pushed the fix/triton-hmac-security-v2-all-bugs branch 2 times, most recently from 079e1f2 to 334346a Compare March 31, 2026 22:15
@pravali96 pravali96 force-pushed the fix/triton-hmac-security-v2-all-bugs branch from 95dc333 to a809727 Compare March 31, 2026 22:47
…y check (v2)

Backport of v3 security fixes for P400136088 and V2146375387.

1. check_integrity.py: Switch from HMAC-SHA256 to plain SHA-256.
   Remove generate_secret_key, remove env var dependency.

2. triton/model.py: Add integrity check in initialize() BEFORE
   cloudpickle deserialization.

3. triton/server.py: Remove SAGEMAKER_SERVE_SECRET_KEY from
   container environment variables.

4. triton/triton_builder.py: Remove hardcoded dummy secret key
   for ONNX path. Rename _hmac_signing to _compute_integrity_hash.
   Use plain SHA-256.

5. All prepare.py files (torchserve, mms, tf_serving, smd):
   Remove generate_secret_key usage, switch to plain SHA-256.
@pravali96 pravali96 force-pushed the fix/triton-hmac-security-v2-all-bugs branch from 032f58f to d10bf43 Compare April 1, 2026 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants