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

オープン
#1,288 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
28/100
issue の種類
リファクタリング
明瞭さ
説明が足りない
活発さ
活発
技術スタック
rust

調査の方向性

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) and get_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) takes mr_config.app_id.clone().unwrap_or_default() and mr_config.instance_id.clone().unwrap_or_default() — host-chosen values of host-chosen length, bound only by SNP HOST_DATA, which the hypervisor selects at launch.
  • instance_id is 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 RATLS with 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: with no_instance_id the context becomes app_id ‖ "app-disk-crypt-key", and since ensure_app_id_len forces |app_id| = 20 on every path that reaches it, no colliding pair exists. What no_instance_id actually 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-length app_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

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

Dstack-TEE/dstack のほかの issue

Dstack-TEE/dstack の issue をすべて見る

似ている issue

Rust の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。