KMS key derivation: the disk-key HKDF context is an unframed concatenation, and only one of the two BootInfo producers maintains the width invariant it relies on
まだ誰も着手していません。
評価
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 初心者へのやさしさ
- 28/100
- issue の種類
- リファクタリング
- 明瞭さ
- 説明が足りない
- 活発さ
- 活発
- 技術スタック
- rust
- 領域
- cryptography, security
調査の方向性
Start with .agent/ROOT-KEY-DESIGN.md and the derivation in dstack/ra-tls/src/kdf.rs, then inspect get_app_key and both BootInfo producers in kms/src/main_service.rs and kms/src/main_service/amd_attest.rs. Compare the existing ensure_app_id_len checks with the length-prefixed pattern in ra-tls/src/api_v1.rs; done requires an agreed direction for local invariant checks, producer convergence, framing, or documentation, plus validation against the cited golden vectors.
索引モデルが issue の本文から書いたものです。
説明
Class: DESIGN. Not a vulnerability. I could not construct a reachable collision and I am not claiming one. This is about where the correctness of a KDF lives: today three of the four root derivations are injective by construction, and the fourth is injective only because a caller two modules away happens to enforce a width that the derivation never checks and that one code path does not maintain.
What the design currently is
ring's Prk::expand(&[&[u8]], _) concatenates the info parts with no separator and no length prefix (dstack/ra-tls/src/kdf.rs:52). The four root derivations are therefore separated only by what falls out of that concatenation:
| purpose | info bytes | variable-length fields | site |
|---|---|---|---|
| app CA | app_id ‖ "app-ca" |
1 | kms/src/main_service.rs:335 |
| env encrypt | app_id ‖ "env-encrypt-key" |
1 | kms/src/main_service.rs:384 |
| app k256 | app_id ‖ "app-key" |
1 | kms/src/crypto.rs:15 |
| app disk | app_id ‖ instance_id ‖ "app-disk-crypt-key" |
2 | kms/src/main_service.rs:380 |
All four run under the shared legacy salt LEGACY_SALT = b"RATLS" (kdf.rs:26).
The part that is sound, and worth recording as such
Three of the four are correct by construction, not by luck. Each has exactly one variable field followed by a distinct fixed literal, so the mapping is injective; and no literal is a suffix of another, so no choice of app_id can turn one purpose's context into another's. Run against the production functions:
== env / app-ca / app-key contexts have ONE variable field, so they are injective ==
exhaustive-ish probe for a second app_id giving app_a's env key: found=false
cross-purpose (disk vs env/app-ca/app-key): suffixes "app-disk-crypt-key" vs "env-encrypt-key" never match
The part that is not
The disk context concatenates two variable-length fields, so its split point is not recoverable from the info string. Demonstrated against the production kdf::derive_dh_secret, with the harness first validated against the two golden vectors already in the tree (ra-tls/src/kdf.rs:175, kms/src/crypto.rs:148):
victim app_id=aaaa…aa (20B) instance_id=bbbb…bb (20B)
split 0: app_id( 0B)||instance_id(40B) -> disk_key 896d7ed35b68a4349c4c979e == VICTIM
split 19: app_id(19B)||instance_id(21B) -> disk_key 896d7ed35b68a4349c4c979e == VICTIM
split 21: app_id(21B)||instance_id(19B) -> disk_key 896d7ed35b68a4349c4c979e == VICTIM
split 40: app_id(40B)||instance_id( 0B) -> disk_key 896d7ed35b68a4349c4c979e == VICTIM
distinct splits of the SAME 40 bytes that yield the victim disk key: 40 (+1 legitimate)
What keeps that harmless is a width invariant the derivation does not state:
ensure_app_id_len(kms/src/main_service/upgrade_authority.rs:45-50, exactly 20 bytes) has exactly two production call sites:build_boot_info(:29) andget_app_env_encrypt_pub_key(main_service.rs:412).- The SEV-SNP producer does not call it.
build_amd_snp_boot_info_with_tcb_status(kms/src/main_service/amd_attest.rs:116,118) takesmr_config.app_id.clone().unwrap_or_default()andmr_config.instance_id.clone().unwrap_or_default()— host-chosen values of host-chosen length, bound only by SNPHOST_DATA, which the hypervisor selects at launch. instance_idis never width-checked on any path.grep -rn "instance_id.len()"outside tests returns nothing.
This is the "right pattern next door" shape twice over: a check present on one sibling producer and absent on the other, and the correct construction already shipping one directory away.
Reachability — why this is DESIGN and not a bug
To exploit the ambiguity an attacker must control the prefix of the concatenation, and the prefix is app_id, which is pinned by DstackKms.isAppAllowed via registeredApps[appId] (DstackKms.sol:288) plus isContract(appId) (:297). The ethers backend's decodeHex(x, 20) only left-pads (kms/auth-eth/src/ethereum.ts:29-38), so a short app_id maps to an address with leading zero bytes; owning a contract at such an address is ~2^152 of grinding. A long app_id is passed through unpadded and ethers rejects it as an address. Additionally the whole SNP release path is off unless sev_snp_key_release = true (default false, kms/src/config.rs:47-48).
So the reachability fields are: who can trigger — the hypervisor of an SNP host, via HOST_DATA; what credential — a registered appId and a whitelisted compose hash, which pins the prefix and is what defeats it; who controls frequency — the host. Severity from those three: latent.
Note that #1268's ensure_identity_widths (at AuthApi::is_app_allowed) would incidentally close the reachability for both paths, which is good and is an argument for landing it. It would not change the design point: the derivation would still be correct only because of a check at the authorization boundary.
Relationship to existing records
- #552 ("Static HKDF salt
RATLSwith no key versioning") covers the salt. This issue is about framing of the info string and about which producer maintains the width invariant; they are different defects in the same file. - GHSA-whv5 (draft, "disk encryption key collision when
no_instance_id=true+ HKDF context ambiguity") points at this area but I could not reproduce it as a cross-app collision: withno_instance_idthe context becomesapp_id ‖ "app-disk-crypt-key", and sinceensure_app_id_lenforces|app_id| = 20on every path that reaches it, no colliding pair exists. Whatno_instance_idactually does — every instance of one app sharing a disk key — is the intended behaviour for portable disks. The genuine ambiguity is this one, and it needs a variable-lengthapp_id, which only the SNP path allows. - #1247 and #1268 are adjacent and neither changes the derivation.
Improvement directions
(1) Assert the invariant where the derivation is, not two modules away. Cheap.
get_app_key already has boot_info.app_id and boot_info.instance_id in hand at main_service.rs:375-380. A width check there — or better, at the BootInfo construction seam so both producers pass through it — makes the property local to the code that depends on it. This changes no derived key for any conforming input, so it is a pure fail-closed addition with no compatibility cost. I would take this one first.
(2) Have the SNP producer share the non-SNP producer's identity checks.
build_boot_info calls ensure_app_id_len; build_amd_snp_boot_info_with_tcb_status does not. Factoring that check to the point where both converge (build_boot_info_for_attestation, main_service.rs:182-196) removes the asymmetry rather than patching one arm.
Trade-off: none I can see — an SNP mr_config with a non-20-byte app_id cannot pass the on-chain gate today anyway, so nothing that works stops working.
(3) Length-prefix and tag the info strings, under a new salt. Migration event — probably not worth it alone.
The correct construction is already in-tree: ra-tls/src/api_v1.rs:33-44,96-121 uses KDF_SALT = b"dstack-guest-v1", a KEY_CONTEXT_TAG, and push_length_prefixed on every field. Applying the same shape to the root derivations would make them injective without depending on any caller.
Trade-off: it changes every deployed key, so it is only viable bundled with a root-epoch migration (#1287, option 3) — and if that migration ever happens, this should ride along with it rather than be done separately. On its own the cost is out of proportion to a latent property.
(4) Do nothing but record it. Legitimate, given the reachability analysis. If so, a comment at main_service.rs:380 saying that the disk context relies on fixed-width app_id/instance_id and naming ensure_app_id_len as the enforcer would stop the next reader from having to rediscover it.
Full derivation tree and reproduction harness: .agent/ROOT-KEY-DESIGN.md.
- 主要言語
- Rust
- スター
- 550
- フォーク
- 97
- 平均マージ
- 19時間 22分
- マージ済み PR(30日)
- 109
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
Dstack-TEE/dstack のほかの issue
-
難易度 5/5 1週間以上 初心者へのやさしさ 30/100
Dstack-TEE/dstack#1301 ·
-
難易度 3/5 1〜2日 初心者へのやさしさ 55/100
Dstack-TEE/dstack#1300 ·
-
難易度 4/5 3〜5日 初心者へのやさしさ 48/100
Dstack-TEE/dstack#1299 ·
-
難易度 4/5 3〜5日 初心者へのやさしさ 48/100
Dstack-TEE/dstack#1298 ·
-
P0
難易度 5/5 1週間以上 初心者へのやさしさ 25/100
Dstack-TEE/dstack#1297 ·
Dstack-TEE/dstack の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 85/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 84/100
Eynzof/Hermes-CN-Desktop#610 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
-
bug team:backend track:services-maintenance
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
cowprotocol/services#4950 ·
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
gitbutlerapp/gitbutler#15998 · コメント 1 件 ·