fix(kms): bound compose_hash and instance_id before the policy backend - #1310
Merged
Merged
Conversation
Only `app_id` was length-checked, and both Ethereum backends `padStart` a short hex value to full width before the `readContract` call. A 31-byte compose hash is therefore submitted as - and is indistinguishable from - the whitelisted 32-byte hash whose leading byte is `0x00`. The trusted OS always emits a 32-byte sha256 and either a 20-byte instance id or none, but nothing between the event log and the contract call asserted that. Assert it at `AuthApi::is_app_allowed`, the one point every backend call goes through, and stop `DstackApp.addComposeHash`/`addDevice` from allowlisting `bytes32(0)` - which is exactly what that padding produces - since `_initializeCommon` already declines to seed it.
kvinwang
added a commit
that referenced
this pull request
Sep 25, 2026
#1278, #1310 and #1365 added rows to main_service::upgrade_authority::tests, so tc-kms-auth-010 failed its exact count of four. Count eight, name the new coverage, and state which KMS-local reuses #1311 and #1399 introduced: the auth API info and the self-authorization, never an app decision. Signed-off-by: Kevin Wang <wy721@qq.com>
kvinwang
added a commit
that referenced
this pull request
Sep 25, 2026
#1310 made addComposeHash and addDevice reject bytes32(0). Include its Foundry test in the shared policy run behind tc-kms-auth-006 and tc-kms-runtime-005. Signed-off-by: Kevin Wang <wy721@qq.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both Ethereum backends
padStartshort hex values, and nothing between attestation decoding and the backend call enforcedcompose_hash/instance_idwidths on the TDX path, so a 31-bytecompose_hashwould be submitted as the whitelisted hash with a leading0x00.AuthApi::is_app_allowedrequires a 20-byteapp_id, a 32-bytecompose_hashand a 20-byte-or-emptyinstance_id, for every backend call.DstackApp.addComposeHash/addDevicerejectbytes32(0), matching_initializeCommon.Defense in depth: trusted OS images already emit these widths, so no deployment changes.
Split out of #1268 (K-g).