contracts: an app can never be deregistered, so the KMS owner's only revocation lever is the global OS-image allowlist
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Idoneità per principianti
- 30/100
- Tipo di issue
- Funzionalità
- Chiarezza
- Da chiarire
- Stato di attività
- Attiva
- Stack tecnologico
- solidity
- Ambito
- blockchain, security
Direzione di ricerca
Start with contracts/DstackKms.sol:144, docs/specification.md §2 and §8, and docs/onchain-governance.md:176; then run the scenarios in dstack/kms/auth-eth/test/ScenarioWalk.t.sol. The issue presents several alternatives, so first determine which revocation or documentation policy is accepted. Done means the chosen behavior and its security trade-offs are implemented or documented, with the cited scenarios still covering the result.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Label: DESIGN. No test fails against the current contracts; the tests below pass and pin present behaviour.
What the design currently is
DstackKms.registerApp is permissionless and monotonic:
// contracts/DstackKms.sol:144 — natspec confirms the permissionlessness is intentional
function registerApp(address appId) public {
require(appId != address(0), "Invalid app ID");
registeredApps[appId] = true;
...
}
No function in either contract ever sets registeredApps[x] back to false. There is no deregisterApp, unregisterApp or removeApp at any privilege level, including the owner's. docs/specification.md §2 (slot 5) lists the writers and names no remover; §8's open questions ask about removing an OS image (§8.3) and not about deregistering an app.
The consequence for the emergency-revocation scenario is that the KMS owner has zero app-scoped levers. The per-app compose-hash and device lists live on the DstackApp contract under the app owner's key, so:
kmsOwner : app.removeComposeHash(V1) -> reverts, OwnableUnauthorizedAccount
kmsOwner : kms.deregisterApp(app) -> no such function
kmsOwner : kms.removeOsImageHash(IMAGE) -> works, and revokes every app on that image
Pinned in the new test/ScenarioWalk.t.sol:
[PASS] test_S3_KmsOwnerCannotRevokeASingleApp() (gas: 284866)
assertFalse(hit, "no deregisterApp(address)");
assertFalse(hit, "no unregisterApp(address)");
assertFalse(hit, "no removeApp(address)");
assertTrue(kms.registeredApps(address(app)), "registration is write-once-true, forever");
[PASS] test_S3_OnlyKmsLeverIsGlobalAndHitsEveryTenant() (gas: 504630)
assertFalse(v, "target revoked");
assertFalse(bnd, "and so is every unrelated app on that image");
Two compositions make it sharper. If the compromised credential is the app owner key, the attacker re-adds a compose hash faster than anyone can remove one and can transfer ownership away — and inherits the existing app identity, so the keys they receive are the ones the legitimate app has been using:
[PASS] test_S3_CompromisedAppOwnerKeyIsUnrevokable() (gas: 321721)
assertTrue(ok, "attacker-chosen compose hash authorized under the original app identity");
assertEq(app.owner(), attacker, "and the legitimate owner is locked out");
And an app owner can destroy their own remaining levers permanently, with no KMS-side fallback:
[PASS] test_S6_DisableUpgradesPlusRenounce_FreezesThePolicyOpenForever() (gas: 278111)
app.disableUpgrades(); app.renounceOwnership();
assertTrue(ok, "V1 is authorized in perpetuity, by nobody, revocable by nobody");
Both disableUpgrades() and renounceOwnership() are presented to operators as hardening (docs/onchain-governance.md:176, Manage.s.sol:DisableAppUpgrades).
Steelman
The separation is the point. dstack's model is that an app owner governs their own app and the KMS owner governs the platform, and a KMS-owner kill switch over individual apps would hand the infrastructure operator exactly the unilateral power the on-chain design exists to remove. registerApp's natspec makes the reasoning explicit: registration confers no privilege on its own, since allowedOsImages and the app's own isAppAllowed still gate everything downstream — so there is nothing to "unregister" in the authorization sense. A deregisterApp is also trivially griefable in the other direction if it were permissionless, and a centralising change if it were not.
renounceOwnership and disableUpgrades are similarly deliberate: an app that can prove nobody can change its policy is a stronger claim to its users than one that can.
What it costs
Concretely: when an app's owner key is stolen, there is no one who can stop that app, and the platform's only response is to pull the OS image — which stops every unrelated tenant on the same image. The blast radius of a single-tenant incident is the whole image cohort.
registeredApps also grows monotonically under permissionless writes, so the PolicyChanged/AppRegistered log that the audit story depends on can be padded indefinitely by anyone, and never pruned.
Reachability: who — anyone can add (permissionless registerApp), nobody can remove; the revocation gap is exercised by whoever holds a compromised app owner key. Credential — none to add; a stolen app owner key for the incident case. Frequency — attacker-paced for the additions; incident-paced for the revocation gap.
Improvement direction
Redeployment status: no new proxy needed, and the simplest form needs no new storage. DstackKms is UUPS with __gap[50]; registeredApps already exists, so an owner-only setter that writes false is an implementation upgrade with zero storage-layout change. That is what decides the priority here — this is cheap to ship if the team wants it.
Options:
- Do nothing, document it (no contract change). State in
docs/onchain-governance.mdand the security guide that app registration is permanent, that the KMS owner has no per-app revocation lever by design, and that the incident playbook for a stolen app owner key is the global OS-image rotation with its cohort-wide blast radius. Zero cost, reaches every deployment, changes nothing operationally. - Owner-only
deregisterApp(address)(impl upgrade, no new storage). Restores a platform-level lever with a blast radius of one app instead of one image. Trade-off: it is a real centralisation step and needs a decision about whether the KMS owner should have it. A middle form is aderegisterAppthat is itself time-locked or requires the app owner's countersignature, which does need new storage past__gap. - Make
registerAppnon-permissionless (impl upgrade, no new storage). Bounds the log growth, but breaks the documented third-party bootstrapping use case inregisterApp's natspec. Probably the wrong trade on its own. - Override
renounceOwnership()to revert onDstackApp(impl upgrade, no new storage) so an app cannot reach a state where its policy is frozen and nobody holds a lever. Narrower than (2) and does not touch the trust model — it only removes an irreversible operator footgun.docs/specification.md§8.1 already asks this question forDstackKms; the app-side composition withdisableUpgradesis the sharper case. Note this reaches an app only if its owner upgrades, and never for one that already calleddisableUpgrades().
(1) is required regardless — whichever of (2)/(4) the team picks, the current behaviour is not written down anywhere.
Found during a scenario-driven review of the authorization contracts; full walk in .agent/CONTRACT-SCENARIOS.md (scenarios 3 and 6), tests in dstack/kms/auth-eth/test/ScenarioWalk.t.sol.
- Lingua principale
- Rust
- Stelle
- 550
- Fork
- 97
- Merge medio
- 19h 22m
- PR unite (30g)
- 109
Guida per i contributori
Apri la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di Dstack-TEE/dstack
-
Difficoltà 5/5 Più di una settimana Idoneità per principianti 30/100
Dstack-TEE/dstack#1301 ·
-
Difficoltà 3/5 1-2 giorni Idoneità per principianti 55/100
Dstack-TEE/dstack#1300 ·
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 48/100
Dstack-TEE/dstack#1299 ·
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 48/100
Dstack-TEE/dstack#1298 ·
-
P0
Difficoltà 5/5 Più di una settimana Idoneità per principianti 25/100
Dstack-TEE/dstack#1297 ·
Tutte le issue di Dstack-TEE/dstack
Issue simili
-
Browser (wasm) relay client cannot connect to relays whose URL has a trailing-dot FQDN hostname Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
n0-computer/iroh#4550 ·
-
impl detach for native Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 65/100
paritytech/zombienet-sdk#591 ·
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
farion1231/cc-switch#7638 · 1 commento ·
-
onnx-ir re-exports ModelProto and GraphProto but not NodeProto, AttributeProto and AttributeType Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100