Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

contracts: an app can never be deregistered, so the KMS owner's only revocation lever is the global OS-image allowlist

Aperta
#1,293 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

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

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:

  1. Do nothing, document it (no contract change). State in docs/onchain-governance.md and 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.
  2. 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 a deregisterApp that is itself time-locked or requires the app owner's countersignature, which does need new storage past __gap.
  3. Make registerApp non-permissionless (impl upgrade, no new storage). Bounds the log growth, but breaks the documented third-party bootstrapping use case in registerApp's natspec. Probably the wrong trade on its own.
  4. Override renounceOwnership() to revert on DstackApp (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 for DstackKms; the app-side composition with disableUpgrades is the sharper case. Note this reaches an app only if its owner upgrades, and never for one that already called disableUpgrades().

(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

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Dstack-TEE/dstack

Tutte le issue di Dstack-TEE/dstack

Issue simili

Altre issue su Rust

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.