contracts: an app can never be deregistered, so the KMS owner's only revocation lever is the global OS-image allowlist
Los mantenedores suelen responder en 1 día
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Aptitud para principiantes
- 30/100
- Tipo de issue
- Nueva funcionalidad
- Claridad
- Necesita aclaración
- Estado de actividad
- Activo
- Stack tecnológico
- solidity
- Área
- blockchain, security
Línea de trabajo
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.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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.
- Lenguaje dominante
- Rust
- Estrellas
- 555
- Forks
- 97
- Merge medio
- 1 d 3 h
- PR fusionados (30 d)
- 199
Preparar el entorno
- Sin Dockerfile ni archivo de Docker Compose
- Sin plantilla de pull request
- Leer la guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de Dstack-TEE/dstack
-
Dificultad 5/5 Más de una semana Aptitud para principiantes 35/100
Dstack-TEE/dstack#1384 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 5/5 Más de una semana Aptitud para principiantes 30/100
Dstack-TEE/dstack#1301 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 3/5 1-2 días Aptitud para principiantes 55/100
Dstack-TEE/dstack#1300 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 4/5 3-5 días Aptitud para principiantes 48/100
Dstack-TEE/dstack#1299 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 4/5 3-5 días Aptitud para principiantes 48/100
Dstack-TEE/dstack#1298 ·
Los mantenedores suelen responder en 1 día
Todos los issues de Dstack-TEE/dstack
Issues similares
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
Los mantenedores suelen responder en 1 día
-
Dificultad 1/5 Menos de una hora Aptitud para principiantes 78/100
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 86/100
dani-garcia/vaultwarden#7801 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 90/100
boxlite-ai/boxlite#1814 ·
Los mantenedores suelen responder en 1 día