From 4897b37ff09c5bc645094bd14ac725ff984936c3 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 10 Nov 2026 21:36:44 -0800 Subject: [PATCH 1/5] fix(kms-auth)!: make policy revocation cover every selectable epoch A request may select any epoch up to the current one and is decided on that epoch's row, but the epoch-less setters only wrote rows from the current epoch on. After a rotation, removing an OS image, compose hash or device left older rows granting it. - DstackKms: drop the epoch-less add/remove setters for OS images, KMS aggregated MRs and KMS devices. Keep the *AtEpoch setters and add *AtEpochs(fromEpoch, toEpoch, value) range setters, so the caller names the rows and one transaction revokes a value from every row. - DstackApp: the IAppAuthBasicManagement setters and setAllowAnyDevice keep their ABI and interface ID but now write every row from 0 through the next epoch, so a removal is a full revocation. - Key root policy rows by registration instead of chain hash. A popped epoch re-registered with the same keys had the same chain hash and got its old rows back; every registration now starts with empty rows. Epoch 0 still uses the v0.5.x mappings. - Cap kms_serve at the current root epoch. Only the key handover operations may select a pending epoch. - Manage.s.sol KMS scripts take ROOT_EPOCH or FROM_ROOT_EPOCH and TO_ROOT_EPOCH. By default additions write the current through the latest epoch, and removals write every row. - Fix two stale DstackKms comments: evidence report data does not bind the epoch, and KMS authorization does not check the app ID. Signed-off-by: Kevin Wang --- docs/app-key-epochs.md | 45 +++-- docs/onchain-governance.md | 4 +- docs/tutorials/gateway-build-configuration.md | 2 +- docs/tutorials/kms-cvm-deployment.md | 2 +- .../troubleshooting-gateway-deployment.md | 4 +- dstack/kms/auth-eth/README.md | 22 +-- dstack/kms/auth-eth/contracts/DstackApp.sol | 43 +++-- dstack/kms/auth-eth/contracts/DstackKms.sol | 110 +++++++----- .../kms/auth-eth/docs/formal-verification.md | 34 ++-- dstack/kms/auth-eth/docs/specification.md | 13 +- dstack/kms/auth-eth/script/Manage.s.sol | 76 ++++---- dstack/kms/auth-eth/script/README.md | 5 + dstack/kms/auth-eth/test/DstackApp.t.sol | 47 ++--- .../test/DstackAppEpoch.symbolic.t.sol | 55 +++--- .../auth-eth/test/DstackKms.symbolic.t.sol | 23 +-- dstack/kms/auth-eth/test/DstackKms.t.sol | 163 ++++++++++++------ .../test/DstackKmsEpoch.symbolic.t.sol | 51 +++--- dstack/kms/auth-eth/test/EventAudit.t.sol | 10 +- .../auth-eth/test/UpgradesWithPlugin.t.sol | 14 +- 19 files changed, 414 insertions(+), 309 deletions(-) diff --git a/docs/app-key-epochs.md b/docs/app-key-epochs.md index 64d4d6d01..42f932c0f 100644 --- a/docs/app-key-epochs.md +++ b/docs/app-key-epochs.md @@ -53,28 +53,35 @@ epochs. For an image handover, explicitly approve the new image in the old row s it can migrate old keys, approve it in the new row, and remove the old image from the old row after the handover. The old image never needs access to the new row. -Existing `DstackKms` add/remove methods (OS images, KMS aggregated MRs, KMS -devices) operate on every root row from the current epoch through the latest -**registered** one, so a removal during a rotation revokes the material in both -the serving and the pending row, and an addition allows it in both. Before any -root epoch is registered, they operate on epoch zero. `DstackApp` add/remove -methods, and `setAllowAnyDevice`, likewise operate on the current app epoch and -the next one. The `*AtEpoch(epoch, material)` methods select a particular -registered root epoch, or a historical, current or next app epoch; removal -affects only that row. `setAllowAnyDeviceAtEpoch(epoch, allowed)` is also -row-specific. Single-material add/remove scripts in `Manage.s.sol` accept optional -`ROOT_EPOCH` or `APP_EPOCH` variables, respectively, as does `SetAllowAnyDevice`. +A request may select any epoch up to the current one and is decided on that +epoch's row, so a value is revoked only once every row allowing it is cleared. +`DstackKms` has no epoch-less setters for OS images, KMS aggregated MRs and KMS +devices: `*AtEpoch(epoch, material)` changes one registered root row and +`*AtEpochs(fromEpoch, toEpoch, material)` every row in the range, so one +transaction revokes a value from all rows with `remove*AtEpochs(0, latest, …)`. +Before any root epoch is registered, only epoch zero can be written. `DstackApp` +keeps its `IAppAuthBasicManagement` add/remove methods and `setAllowAnyDevice`; +they change every app row from 0 through the next epoch, so a removal is a full +revocation. Its `*AtEpoch` methods, and `setAllowAnyDeviceAtEpoch`, change one +historical, current or next row. The KMS scripts in `Manage.s.sol` take +`ROOT_EPOCH` for one row or `FROM_ROOT_EPOCH`/`TO_ROOT_EPOCH` for a range; without +them, additions write the current through the latest row and removals every row. +The app scripts take an optional `APP_EPOCH`, as does `SetAllowAnyDevice`. + +Disk and environment migration after a root rotation fetches the keys of the +recorded epoch, so a new image migrating old data must be allowed in that old +row too. Without it the guest stops with an error naming the root and app epoch +it could not get keys for. The `*AtEpoch` getters read the selected row, including legacy epoch zero. `EpochPolicyChanged` events include the row and the resulting permission. Legacy bool getters and events describe **epoch zero only**; changing a nonzero row does not change its legacy bool or emit a misleading legacy material event. -Root policy rows from epoch 1 belong to the epoch's keys: the contract stores -them under its chain hash, so a row can be written only once the epoch is -registered (register, prepare the row, then activate). A popped epoch's rows do -not carry over: other keys registered under the same number start with empty -rows, which must be prepared again before activation. Epoch 0 keeps the legacy -mappings. +Root policy rows from epoch 1 belong to the epoch's registration, so a row can +be written only once the epoch is registered (register, prepare the row, then +activate). A popped epoch's rows do not carry over: the next registration of the +number starts with empty rows, even with the same keys, which must be prepared +again before activation. Epoch 0 keeps the legacy mappings. ### KMS keyring handover @@ -91,8 +98,8 @@ This permits staged key distribution without changing which epoch the KMS serves ### Upgrading existing proxies The original bool mappings and `allowAnyDevice` retain their storage locations and -are used directly as row zero. New nested mappings consume three storage-gap slots -in each contract; no entries are copied and no reinitializer is required. Existing +are used directly as row zero. New storage consumes storage-gap slots in each +contract (four in `DstackKms`, three in `DstackApp`); no entries are copied and no reinitializer is required. Existing owners, pending owners, policy flags, root registries and app epoch counters are preserved. Even if the pre-upgrade counter is nonzero, old global approvals grant only row zero after upgrading: explicitly populate the intended nonzero rows. diff --git a/docs/onchain-governance.md b/docs/onchain-governance.md index 016865ed2..bafe3478d 100644 --- a/docs/onchain-governance.md +++ b/docs/onchain-governance.md @@ -155,8 +155,8 @@ The central governance contract that manages OS image whitelisting, app registra | Function | Description | |----------|-------------| -| `addOsImageHash(bytes32)` | Whitelist an OS image hash | -| `removeOsImageHash(bytes32)` | Remove an OS image from whitelist | +| `addOsImageHashAtEpochs(uint256,uint256,bytes32)` | Whitelist an OS image hash in a range of root epochs | +| `removeOsImageHashAtEpochs(uint256,uint256,bytes32)` | Remove an OS image from a range of root epochs; `0` through the latest revokes it | | `setGatewayAppId(string)` | Set the trusted Gateway app ID(s) | | `registerApp(address)` | Register an app contract | | `deployAndRegisterApp(...)` | Deploy and register app in one transaction | diff --git a/docs/tutorials/gateway-build-configuration.md b/docs/tutorials/gateway-build-configuration.md index 53afb1e27..a1bc205ea 100644 --- a/docs/tutorials/gateway-build-configuration.md +++ b/docs/tutorials/gateway-build-configuration.md @@ -278,7 +278,7 @@ echo "OS image hash: 0x$OS_IMAGE_HASH" ``` ```bash -cast send "$KMS_CONTRACT_ADDR" "addOsImageHash(bytes32)" "0x$OS_IMAGE_HASH" --rpc-url "$ETH_RPC_URL" --private-key "$PRIVATE_KEY" +cast send "$KMS_CONTRACT_ADDR" "addOsImageHashAtEpoch(uint256,bytes32)" 0 "0x$OS_IMAGE_HASH" --rpc-url "$ETH_RPC_URL" --private-key "$PRIVATE_KEY" ``` Verify it was added: diff --git a/docs/tutorials/kms-cvm-deployment.md b/docs/tutorials/kms-cvm-deployment.md index 8089dadd0..def92725c 100644 --- a/docs/tutorials/kms-cvm-deployment.md +++ b/docs/tutorials/kms-cvm-deployment.md @@ -317,7 +317,7 @@ Onboarding > **Critical prerequisite:** before bootstrap can succeed, the KMS must already be authorized by your auth backend. > > - For `dstack-auth`, add the KMS `mrAggregated` to `kms.mrAggregated` -> - For `auth-eth`, add the KMS `mrAggregated` on-chain with `addKmsAggregatedMr(...)` +> - For `auth-eth`, add the KMS `mrAggregated` on-chain with `script/Manage.s.sol:AddKmsAggregatedMr` > > You can fetch the value before bootstrap with: > diff --git a/docs/tutorials/troubleshooting-gateway-deployment.md b/docs/tutorials/troubleshooting-gateway-deployment.md index 4cae22f53..cbd2b4560 100644 --- a/docs/tutorials/troubleshooting-gateway-deployment.md +++ b/docs/tutorials/troubleshooting-gateway-deployment.md @@ -141,8 +141,8 @@ echo "OS image hash: 0x$OS_IMAGE_HASH" # Whitelist it on the KMS contract export KMS_CONTRACT_ADDR=$(cat ~/.dstack/secrets/kms-contract-address) cast send "$KMS_CONTRACT_ADDR" \ - "addOsImageHash(bytes32)" \ - "0x$OS_IMAGE_HASH" \ + "addOsImageHashAtEpoch(uint256,bytes32)" \ + 0 "0x$OS_IMAGE_HASH" \ --rpc-url "https://ethereum-sepolia-rpc.publicnode.com" \ --private-key "$(cat ~/.dstack/secrets/sepolia-private-key)" ``` diff --git a/dstack/kms/auth-eth/README.md b/dstack/kms/auth-eth/README.md index 8a1f807ff..173f6228d 100644 --- a/dstack/kms/auth-eth/README.md +++ b/dstack/kms/auth-eth/README.md @@ -307,23 +307,23 @@ See `script/README.md` for complete documentation of all available scripts. #### Epoch-scoped material approvals Material approvals use exact root/app epoch rows; neither older nor newer rows -are implicitly authorized. Existing add/remove methods change every root row from -the current epoch through the latest registered one, and the current and next -app rows. The `*AtEpoch` methods and optional -`ROOT_EPOCH` / `APP_EPOCH` variables on single-material management scripts select -an explicit row. Original bool mappings and events remain epoch-zero views after +are implicitly authorized. `DstackKms` material setters name their rows: +`*AtEpoch` one root row, `*AtEpochs` a range. `DstackApp` add/remove methods and +`setAllowAnyDevice` change every app row through the next epoch, so a removal is +a full revocation; its `*AtEpoch` methods change one row. The management scripts +take `ROOT_EPOCH`, `FROM_ROOT_EPOCH`/`TO_ROOT_EPOCH` or `APP_EPOCH` to select +rows. Original bool mappings and events remain epoch-zero views after upgrading; epoch-specific getters/events describe the new rows. KMS keyring handover separately checks its highest exported root epoch, which authorizes the whole keyring. See [epoch-scoped allowlists](../../../docs/app-key-epochs.md#epoch-scoped-allowlists) for migration, upgrade order and compatibility. -### Static Analysis & Symbolic Verification (local only) +### Static Analysis & Symbolic Verification -We run [Slither](https://github.com/crytic/slither) and -[Halmos](https://github.com/a16z/halmos) locally during development. They -are intentionally not part of CI — symbolic proofs are slow, sensitive to -solver versions, and most valuable as an interactive design check rather -than a gate. +We run [Slither](https://github.com/crytic/slither) locally during +development. [Halmos](https://github.com/a16z/halmos) also runs in CI +(`.github/workflows/halmos.yml`, pinned Halmos and Forge versions) whenever +this directory changes. ```bash pipx install slither-analyzer halmos diff --git a/dstack/kms/auth-eth/contracts/DstackApp.sol b/dstack/kms/auth-eth/contracts/DstackApp.sol index 07ab15812..0274ba1e8 100644 --- a/dstack/kms/auth-eth/contracts/DstackApp.sol +++ b/dstack/kms/auth-eth/contracts/DstackApp.sol @@ -172,11 +172,19 @@ contract DstackApp is return epoch == 0 ? allowAnyDevice : _allowAnyDeviceByEpoch[epoch]; } - // The epoch-less setters change the current row and the next one, which the - // owner prepares with the *AtEpoch setters before calling advanceAppEpoch. - function addComposeHash(bytes32 composeHash) external { - addComposeHashAtEpoch(currentAppEpoch, composeHash); - addComposeHashAtEpoch(currentAppEpoch + 1, composeHash); + // A request may select any app epoch up to the current one and is decided + // on that epoch's row, so the epoch-less setters change every row, 0 through + // the next epoch: a removal is a full revocation. The *AtEpoch setters + // change a single row. + function _setAllEpochs(function(uint256, bytes32, bool) internal setter, bytes32 value, bool allowed) private { + for (uint256 epoch = 0; epoch <= currentAppEpoch + 1; ++epoch) { + setter(epoch, value, allowed); + } + } + + function addComposeHash(bytes32 composeHash) external onlyOwner { + require(composeHash != bytes32(0), "invalid compose hash"); + _setAllEpochs(_setComposeHash, composeHash, true); } function addComposeHashAtEpoch(uint256 epoch, bytes32 composeHash) public onlyOwner { @@ -184,9 +192,8 @@ contract DstackApp is _setComposeHash(epoch, composeHash, true); } - function removeComposeHash(bytes32 composeHash) external { - removeComposeHashAtEpoch(currentAppEpoch, composeHash); - removeComposeHashAtEpoch(currentAppEpoch + 1, composeHash); + function removeComposeHash(bytes32 composeHash) external onlyOwner { + _setAllEpochs(_setComposeHash, composeHash, false); } function removeComposeHashAtEpoch(uint256 epoch, bytes32 composeHash) public onlyOwner { @@ -210,12 +217,15 @@ contract DstackApp is emit EpochPolicyChanged(keccak256("compose-hash"), composeHash, epoch, allowed); } - function setAllowAnyDevice(bool allowed) external { - setAllowAnyDeviceAtEpoch(currentAppEpoch, allowed); - setAllowAnyDeviceAtEpoch(currentAppEpoch + 1, allowed); + function setAllowAnyDevice(bool allowed) external onlyOwner { + _setAllEpochs(_setAllowAnyDevice, bytes32(0), allowed); } function setAllowAnyDeviceAtEpoch(uint256 epoch, bool allowed) public onlyOwner { + _setAllowAnyDevice(epoch, bytes32(0), allowed); + } + + function _setAllowAnyDevice(uint256 epoch, bytes32, bool allowed) private { _requireWritableEpoch(epoch); if (epoch == 0) { allowAnyDevice = allowed; @@ -234,9 +244,9 @@ contract DstackApp is _emitPolicy("require-tcb-up-to-date", bytes32(0), _requireUpToDate); } - function addDevice(bytes32 deviceId) external { - addDeviceAtEpoch(currentAppEpoch, deviceId); - addDeviceAtEpoch(currentAppEpoch + 1, deviceId); + function addDevice(bytes32 deviceId) external onlyOwner { + require(deviceId != bytes32(0), "invalid device ID"); + _setAllEpochs(_setDevice, deviceId, true); } function addDeviceAtEpoch(uint256 epoch, bytes32 deviceId) public onlyOwner { @@ -244,9 +254,8 @@ contract DstackApp is _setDevice(epoch, deviceId, true); } - function removeDevice(bytes32 deviceId) external { - removeDeviceAtEpoch(currentAppEpoch, deviceId); - removeDeviceAtEpoch(currentAppEpoch + 1, deviceId); + function removeDevice(bytes32 deviceId) external onlyOwner { + _setAllEpochs(_setDevice, deviceId, false); } function removeDeviceAtEpoch(uint256 epoch, bytes32 deviceId) public onlyOwner { diff --git a/dstack/kms/auth-eth/contracts/DstackKms.sol b/dstack/kms/auth-eth/contracts/DstackKms.sol index 86ed7de4e..91651a39a 100644 --- a/dstack/kms/auth-eth/contracts/DstackKms.sol +++ b/dstack/kms/auth-eth/contracts/DstackKms.sol @@ -58,13 +58,15 @@ contract DstackKms is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, E bytes caPubkey; bytes k256Pubkey; // Where to fetch the generating KMS's attestation. Its report data binds - // the epoch and both public keys, so the evidence proves itself: anyone - // can check the keys came from an approved KMS in a TEE. + // both public keys, so the evidence proves itself: anyone can check the + // keys came from an approved KMS in a TEE. string evidenceUrl; // Commits to the public keys of this epoch and every earlier one: // keccak256(previous chainHash || keccak256(caPubkey) || // keccak256(k256Pubkey)), with a zero previous hash for epoch 0. bytes32 chainHash; + // Keys this epoch's policy rows; unique to each registration. + uint256 registration; } // Root keys of each KMS root epoch, indexed by epoch; epoch 0 is the @@ -77,12 +79,15 @@ contract DstackKms is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, E // later epochs take effect only when activated, one at a time. uint256 private _currentRootEpoch; - // Policy rows of epochs from 1, keyed by the epoch's chain hash so that they - // belong to its keys: a popped and re-registered epoch starts empty. Epoch - // zero continues to use the original fields above after an upgrade. - mapping(bytes32 => mapping(bytes32 => bool)) private _kmsAggregatedMrsByRoot; - mapping(bytes32 => mapping(bytes32 => bool)) private _kmsDeviceIdsByRoot; - mapping(bytes32 => mapping(bytes32 => bool)) private _osImagesByRoot; + // Number of root epoch registrations, popped ones included. + uint256 private _rootEpochRegistrations; + + // Policy rows of epochs from 1, keyed by the epoch's registration so that a + // popped and re-registered epoch starts empty, even with the same keys. + // Epoch zero continues to use the original fields above after an upgrade. + mapping(uint256 => mapping(bytes32 => bool)) private _kmsAggregatedMrsByRoot; + mapping(uint256 => mapping(bytes32 => bool)) private _kmsDeviceIdsByRoot; + mapping(uint256 => mapping(bytes32 => bool)) private _osImagesByRoot; event EpochPolicyChanged(bytes32 indexed policy, bytes32 indexed value, uint256 indexed epoch, bool enabled); @@ -187,7 +192,7 @@ contract DstackKms is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, E } bytes32 previous = epoch == 0 ? bytes32(0) : _rootEpochs[epoch - 1].chainHash; bytes32 chainHash = keccak256(abi.encodePacked(previous, keccak256(caPubkey), keccak256(k256Pubkey))); - _rootEpochs.push(RootEpoch(caPubkey, k256Pubkey, evidenceUrl, chainHash)); + _rootEpochs.push(RootEpoch(caPubkey, k256Pubkey, evidenceUrl, chainHash, _rootEpochRegistrations++)); emit RootEpochRegistered(epoch, chainHash, caPubkey, k256Pubkey, evidenceUrl); _emitPolicy("root-epoch", chainHash, true); if (epoch == 0) { @@ -230,8 +235,8 @@ contract DstackKms is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, E // Drop the last registered epoch while it is still pending, for when the // nodes holding it are lost before enough others have onboarded it. The // epoch is named by number and chain hash, so a stale call cannot drop a - // later registration of the same number. Its policy rows stay with its chain - // hash: registering other keys under the same number starts with empty rows, + // later registration of the same number. Its policy rows go with it: the + // next registration of the number starts with empty rows, whatever its keys, // which must be prepared again before activation. function popRootEpoch(uint256 epoch, bytes32 chainHash) external onlyOwner { require(epoch + 1 == _rootEpochs.length && epoch > _currentRootEpoch, "Not the pending root epoch"); @@ -349,25 +354,32 @@ contract DstackKms is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, E function kmsAllowedAggregatedMrsAtEpoch(uint256 epoch, bytes32 mrAggregated) public view returns (bool) { if (epoch == 0) return kmsAllowedAggregatedMrs[mrAggregated]; - return epoch < _rootEpochs.length && _kmsAggregatedMrsByRoot[_rootEpochs[epoch].chainHash][mrAggregated]; - } - - function addKmsAggregatedMr(bytes32 mrAggregated) external onlyOwner { - _setFromCurrentEpoch(_setKmsAggregatedMr, mrAggregated, true); + return epoch < _rootEpochs.length && _kmsAggregatedMrsByRoot[_rootEpochs[epoch].registration][mrAggregated]; } function addKmsAggregatedMrAtEpoch(uint256 epoch, bytes32 mrAggregated) public onlyOwner { _setKmsAggregatedMr(epoch, mrAggregated, true); } - function removeKmsAggregatedMr(bytes32 mrAggregated) external onlyOwner { - _setFromCurrentEpoch(_setKmsAggregatedMr, mrAggregated, false); + function addKmsAggregatedMrAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 mrAggregated) external onlyOwner { + _setAtEpochs(_setKmsAggregatedMr, fromEpoch, toEpoch, mrAggregated, true); } function removeKmsAggregatedMrAtEpoch(uint256 epoch, bytes32 mrAggregated) public onlyOwner { _setKmsAggregatedMr(epoch, mrAggregated, false); } + function removeKmsAggregatedMrAtEpochs( + uint256 fromEpoch, + uint256 toEpoch, + bytes32 mrAggregated + ) + external + onlyOwner + { + _setAtEpochs(_setKmsAggregatedMr, fromEpoch, toEpoch, mrAggregated, false); + } + function _setKmsAggregatedMr(uint256 epoch, bytes32 mrAggregated, bool allowed) private { require(epoch <= _latestRootEpoch(), "Unknown root epoch"); if (epoch == 0) { @@ -376,32 +388,32 @@ contract DstackKms is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, E else emit KmsAggregatedMrRemoved(mrAggregated); _emitPolicy("kms-aggregated-mr", mrAggregated, allowed); } else { - _kmsAggregatedMrsByRoot[_rootEpochs[epoch].chainHash][mrAggregated] = allowed; + _kmsAggregatedMrsByRoot[_rootEpochs[epoch].registration][mrAggregated] = allowed; } emit EpochPolicyChanged(keccak256("kms-aggregated-mr"), mrAggregated, epoch, allowed); } function kmsAllowedDeviceIdsAtEpoch(uint256 epoch, bytes32 deviceId) public view returns (bool) { if (epoch == 0) return kmsAllowedDeviceIds[deviceId]; - return epoch < _rootEpochs.length && _kmsDeviceIdsByRoot[_rootEpochs[epoch].chainHash][deviceId]; - } - - function addKmsDevice(bytes32 deviceId) external onlyOwner { - _setFromCurrentEpoch(_setKmsDevice, deviceId, true); + return epoch < _rootEpochs.length && _kmsDeviceIdsByRoot[_rootEpochs[epoch].registration][deviceId]; } function addKmsDeviceAtEpoch(uint256 epoch, bytes32 deviceId) public onlyOwner { _setKmsDevice(epoch, deviceId, true); } - function removeKmsDevice(bytes32 deviceId) external onlyOwner { - _setFromCurrentEpoch(_setKmsDevice, deviceId, false); + function addKmsDeviceAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 deviceId) external onlyOwner { + _setAtEpochs(_setKmsDevice, fromEpoch, toEpoch, deviceId, true); } function removeKmsDeviceAtEpoch(uint256 epoch, bytes32 deviceId) public onlyOwner { _setKmsDevice(epoch, deviceId, false); } + function removeKmsDeviceAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 deviceId) external onlyOwner { + _setAtEpochs(_setKmsDevice, fromEpoch, toEpoch, deviceId, false); + } + function _setKmsDevice(uint256 epoch, bytes32 deviceId, bool allowed) private { require(epoch <= _latestRootEpoch(), "Unknown root epoch"); if (epoch == 0) { @@ -410,32 +422,32 @@ contract DstackKms is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, E else emit KmsDeviceRemoved(deviceId); _emitPolicy("kms-device", deviceId, allowed); } else { - _kmsDeviceIdsByRoot[_rootEpochs[epoch].chainHash][deviceId] = allowed; + _kmsDeviceIdsByRoot[_rootEpochs[epoch].registration][deviceId] = allowed; } emit EpochPolicyChanged(keccak256("kms-device"), deviceId, epoch, allowed); } function allowedOsImagesAtEpoch(uint256 epoch, bytes32 osImageHash) public view returns (bool) { if (epoch == 0) return allowedOsImages[osImageHash]; - return epoch < _rootEpochs.length && _osImagesByRoot[_rootEpochs[epoch].chainHash][osImageHash]; - } - - function addOsImageHash(bytes32 osImageHash) external onlyOwner { - _setFromCurrentEpoch(_setOsImageHash, osImageHash, true); + return epoch < _rootEpochs.length && _osImagesByRoot[_rootEpochs[epoch].registration][osImageHash]; } function addOsImageHashAtEpoch(uint256 epoch, bytes32 osImageHash) public onlyOwner { _setOsImageHash(epoch, osImageHash, true); } - function removeOsImageHash(bytes32 osImageHash) external onlyOwner { - _setFromCurrentEpoch(_setOsImageHash, osImageHash, false); + function addOsImageHashAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 osImageHash) external onlyOwner { + _setAtEpochs(_setOsImageHash, fromEpoch, toEpoch, osImageHash, true); } function removeOsImageHashAtEpoch(uint256 epoch, bytes32 osImageHash) public onlyOwner { _setOsImageHash(epoch, osImageHash, false); } + function removeOsImageHashAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 osImageHash) external onlyOwner { + _setAtEpochs(_setOsImageHash, fromEpoch, toEpoch, osImageHash, false); + } + function _setOsImageHash(uint256 epoch, bytes32 osImageHash, bool allowed) private { require(epoch <= _latestRootEpoch(), "Unknown root epoch"); if (epoch == 0) { @@ -444,24 +456,25 @@ contract DstackKms is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, E else emit OsImageHashRemoved(osImageHash); _emitPolicy("os-image", osImageHash, allowed); } else { - _osImagesByRoot[_rootEpochs[epoch].chainHash][osImageHash] = allowed; + _osImagesByRoot[_rootEpochs[epoch].registration][osImageHash] = allowed; } emit EpochPolicyChanged(keccak256("os-image"), osImageHash, epoch, allowed); } - // The epoch-less setters change every row from the current epoch through - // any registered pending one, so a revocation during a rotation reaches - // both the serving and the prepared row. Use the *AtEpoch setters to change - // a single row. - function _setFromCurrentEpoch( + // A request may select any epoch up to the current one and is decided on + // that epoch's row, so revoking a value takes every row that allows it: + // 0 through rootEpochCount() - 1. + function _setAtEpochs( function(uint256, bytes32, bool) internal setter, + uint256 fromEpoch, + uint256 toEpoch, bytes32 value, bool allowed ) private { - uint256 latest = _latestRootEpoch(); - for (uint256 epoch = _currentRootEpoch; epoch <= latest; ++epoch) { + require(fromEpoch <= toEpoch, "Invalid root epoch range"); + for (uint256 epoch = fromEpoch; epoch <= toEpoch; ++epoch) { setter(epoch, value, allowed); } } @@ -486,7 +499,7 @@ contract DstackKms is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, E } /// Authorize an attested identity and return the root epoch from the same state. - /// The caller selects KMS or app policy; KMS authorization requires the proxy app ID. + /// The caller selects KMS or app policy. function authorizeKms(AuthorizationRequest calldata request) external view returns (AuthorizationResult memory) { return _checkKmsAuthorization(request); } @@ -572,6 +585,15 @@ contract DstackKms is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, E result.rejectionReason = "Requested root epoch is not registered"; return result; } + // Serving issues keys of the epoch, so only the current one; the key + // handover may move a pending epoch between instances. + if ( + request.operation == AuthorizationOperation.KMS_SERVE && request.requestedRootEpoch.isSet + && request.requestedRootEpoch.value > _currentRootEpoch + ) { + result.rejectionReason = "Requested root epoch is not active"; + return result; + } if (request.requestedAppEpoch.isSet && request.requestedAppEpoch.value != 0) { result.rejectionReason = "Requested app epoch is not active"; return result; @@ -638,7 +660,7 @@ contract DstackKms is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, E } // Add storage gap for upgradeable contracts - uint256[45] private __gap; + uint256[44] private __gap; } function isContract(address addr) view returns (bool) { diff --git a/dstack/kms/auth-eth/docs/formal-verification.md b/dstack/kms/auth-eth/docs/formal-verification.md index 8a5bed9a0..c36b36e45 100644 --- a/dstack/kms/auth-eth/docs/formal-verification.md +++ b/dstack/kms/auth-eth/docs/formal-verification.md @@ -84,8 +84,8 @@ reuse the existing `.t.sol` scaffolding and OZ-foundry-upgrades plugin. ### Setup - [x] `pipx install halmos` documented in `README.md` -- [ ] No CI integration (intentional — see "What's intentionally not in - scope" below) +- [x] Runs in CI (`.github/workflows/halmos.yml`) when `dstack/kms/auth-eth` + changes, with pinned Halmos and Forge versions ### What we verify @@ -136,15 +136,16 @@ and admits one app per epoch, then one symbolic owner write where noted): - [x] `check_IsAppAllowed_EqualsEpochZeroDecisionBeforeAdvance` — at app epoch 0, with the next row prepared and any one owner write, the legacy `isAppAllowed` returns exactly the `authorizeApp` result at epoch 0. -- [x] `check_PolicyWrite_PreservesOtherEpochDecisions` — a policy write at - one epoch, through the explicit or the current-epoch setters, never - changes an authorization decision at another epoch, including epoch - 0's legacy fields. +- [x] `check_PolicyWrite_PreservesOtherEpochDecisions` — an `*AtEpoch` + write at one epoch never changes an authorization decision at another + epoch, including epoch 0's legacy fields. - [x] `check_AuthorizeApp_MatchesPolicy` — for both app operations, `authorizeApp` allows exactly when the epoch is active, the TCB requirement holds, and that epoch admits the compose hash and device. -- [x] `check_PolicyWrite_TakesEffect` — each `*AtEpoch` setter on an active - epoch either sets the value it was given or reverts on a zero ID. +- [x] `check_PolicyWrite_TakesEffect` — each `*AtEpoch` setter on an epoch + up to the next one, and each epoch-less setter on every such epoch, + either sets the value it was given or reverts on a zero ID; so an + epoch-less removal leaves no selectable epoch allowing the value. Mutation-tested: requiring both the any-device flag and a listed device, ignoring the value of a per-epoch `allowAnyDevice` write, dropping a setter's @@ -171,22 +172,21 @@ per epoch, and deploys an app at app epoch 1): succeeds exactly for a 91-byte uncompressed P-256 SPKI and a 33-byte compressed secp256k1 key. - [x] `check_Authorize_EnforcesEpochBounds` — a wrong-kind operation, an app - root epoch above the current one, a KMS root epoch above the latest - registered one, or an app epoch above the app's current one is never + or `kms_serve` root epoch above the current one, another KMS root epoch + above the latest registered one, or an app epoch above the app's current one is never authorized; every result reports the current root epoch and hash. - [x] `check_Authorize_OnlyBootstrapBeforeRegistration` — with no root epoch registered, only `kms_bootstrap` can be authorized. - [x] `check_AuthorizeKms_MatchesEpochPolicy` — after any one owner write, KMS authorization allows exactly when the selected root epoch is - registered, the TCB is up to date, and that epoch's row admits the OS + registered (current at most for `kms_serve`), the TCB is up to date, and that epoch's row admits the OS image, aggregated MR and device. - [x] `check_AuthorizeApp_MatchesKmsAndAppPolicy` — app authorization allows exactly when both epochs are active, the selected root epoch admits the OS image, and the app's own `authorizeApp` admits it. - [x] `check_KmsPolicyWrite_PreservesOtherEpochDecisions` — a KMS policy - write at one root epoch, or through the epoch-less setters at every - epoch from the current one to the latest, never changes a KMS decision - at an epoch it did not write. + write at one root epoch, or through the range setters from one epoch to + the latest, never changes a KMS decision at an epoch it did not write. - [x] `check_LegacyEntryPoints_RefuseOnceRotated` — once a later root epoch is current, `isKmsAllowed` and `isAppAllowed` refuse. - [x] `check_LegacyKmsCheck_ReadsEpochZeroBeforeRotation` — before any @@ -319,10 +319,8 @@ If we revisit, target the same invariants as Layer 2 but add: ## What's intentionally not in scope -- **CI gating of Slither / Halmos.** Symbolic execution is slow, solver- - version-sensitive, and produces non-actionable noise as a PR gate. - Run locally during development; treat these as design checks rather - than blocking lints. +- **CI gating of Slither.** It produces non-actionable noise as a PR gate; + run it locally during development. - **Echidna** — overlaps with Foundry's built-in fuzzer (already runs 10k iterations under `FOUNDRY_PROFILE=ci` in `.github/workflows/foundry-test.yml`). - **Manticore / Mythril** — bytecode-level tools, slow, awkward with our diff --git a/dstack/kms/auth-eth/docs/specification.md b/dstack/kms/auth-eth/docs/specification.md index 01f12bf22..d1eedd72a 100644 --- a/dstack/kms/auth-eth/docs/specification.md +++ b/dstack/kms/auth-eth/docs/specification.md @@ -55,9 +55,9 @@ return-value spoofing are still possible — see §6. | 0–3 | `kmsInfo` | `KmsInfo` (struct, 4 fields) | `registerRootEpoch` (epoch 0), `activateRootEpoch` (owner) | Preserved for proxy upgrades and legacy readers; quote and event log are cleared on activation | | 4 | `gatewayAppId` | `string` | `setGatewayAppId` (owner) | Renamed from `tproxyAppId`; carries `@custom:oz-renamed-from` | | 5 | `registeredApps` | `mapping(address => bool)` | `registerApp` (anyone), `deployAndRegisterApp` (anyone) | Permissionless — see §6.1 | -| 6 | `kmsAllowedAggregatedMrs` | `mapping(bytes32 => bool)` | `addKmsAggregatedMr`, `removeKmsAggregatedMr` (owner) | | -| 7 | `kmsAllowedDeviceIds` | `mapping(bytes32 => bool)` | `addKmsDevice`, `removeKmsDevice` (owner) | | -| 8 | `allowedOsImages` | `mapping(bytes32 => bool)` | `addOsImageHash`, `removeOsImageHash` (owner) | | +| 6 | `kmsAllowedAggregatedMrs` | `mapping(bytes32 => bool)` | `addKmsAggregatedMrAtEpoch(s)`, `removeKmsAggregatedMrAtEpoch(s)` at epoch 0 (owner) | | +| 7 | `kmsAllowedDeviceIds` | `mapping(bytes32 => bool)` | `addKmsDeviceAtEpoch(s)`, `removeKmsDeviceAtEpoch(s)` at epoch 0 (owner) | | +| 8 | `allowedOsImages` | `mapping(bytes32 => bool)` | `addOsImageHashAtEpoch(s)`, `removeOsImageHashAtEpoch(s)` at epoch 0 (owner) | | | 9 | `appImplementation` | `address` | `setAppImplementation` (owner) | Used by `deployAndRegisterApp` | | 10 | `__gap[50]` | `uint256[50]` | (none — must always read zero) | Reserved | @@ -173,8 +173,9 @@ setup uses `registerRootEpoch`. On an upgraded proxy with existing legacy roots, epoch 0 must match the stored `kmsInfo` keys; the upgrade preserves that storage. For each of `setGatewayAppId`, -`setAppImplementation`, `addKmsAggregatedMr`, `removeKmsAggregatedMr`, -`addKmsDevice`, `removeKmsDevice`, `addOsImageHash`, `removeOsImageHash`: +`setAppImplementation`, and the `*AtEpoch` and `*AtEpochs` forms of +`addKmsAggregatedMr`, `removeKmsAggregatedMr`, `addKmsDevice`, `removeKmsDevice`, +`addOsImageHash`, `removeOsImageHash`: - **pre**: `msg.sender == owner()`. - **frame**: every storage cell except the one mapped to the operation. @@ -408,7 +409,7 @@ formal audit: apps have been deployed via the factory, the older proxies stay on the older impl. Is that intended (versioning) or should the KMS track which impl was current at deploy time? -3. **Removing an OS image.** `removeOsImageHash` does not retroactively +3. **Removing an OS image.** `removeOsImageHashAtEpochs` does not retroactively un-authorize already-running apps that booted under that image. Is that intended (revocation requires explicit downstream action) or should the spec require it? diff --git a/dstack/kms/auth-eth/script/Manage.s.sol b/dstack/kms/auth-eth/script/Manage.s.sol index 86db680d6..7548ec645 100644 --- a/dstack/kms/auth-eth/script/Manage.s.sol +++ b/dstack/kms/auth-eth/script/Manage.s.sol @@ -25,6 +25,33 @@ abstract contract BaseScript is Script { app = DstackApp(appAddr); } } + + // The root epoch rows a KMS policy change applies to: ROOT_EPOCH alone, or + // FROM_ROOT_EPOCH (default `defaultFrom`) through TO_ROOT_EPOCH (default the + // latest registered epoch). + function rootEpochRange(uint256 defaultFrom) internal view returns (uint256 from, uint256 to) { + if (vm.envExists("ROOT_EPOCH")) { + from = to = vm.envUint("ROOT_EPOCH"); + } else { + uint256 count = kms.rootEpochCount(); + from = vm.envOr("FROM_ROOT_EPOCH", defaultFrom); + to = vm.envOr("TO_ROOT_EPOCH", count == 0 ? 0 : count - 1); + } + } + + // Additions default to the current epoch onwards; older rows only matter to + // instances still holding older keys, such as disks to migrate. + function addRange() internal view returns (uint256, uint256) { + uint256 current; + if (kms.rootEpochCount() != 0) (current,) = kms.currentRootEpoch(); + return rootEpochRange(current); + } + + // Removals default to every row, as a request may select any epoch up to the + // current one. + function removeRange() internal view returns (uint256, uint256) { + return rootEpochRange(0); + } } // KMS Management Scripts @@ -32,12 +59,9 @@ contract AddKmsAggregatedMr is BaseScript { function run() external { bytes32 mrAggregated = vm.envBytes32("MR_AGGREGATED"); + (uint256 from, uint256 to) = addRange(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); - if (vm.envExists("ROOT_EPOCH")) { - kms.addKmsAggregatedMrAtEpoch(vm.envUint("ROOT_EPOCH"), mrAggregated); - } else { - kms.addKmsAggregatedMr(mrAggregated); - } + kms.addKmsAggregatedMrAtEpochs(from, to, mrAggregated); vm.stopBroadcast(); console.log("Added KMS aggregated MR:", vm.toString(mrAggregated)); @@ -48,12 +72,9 @@ contract RemoveKmsAggregatedMr is BaseScript { function run() external { bytes32 mrAggregated = vm.envBytes32("MR_AGGREGATED"); + (uint256 from, uint256 to) = removeRange(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); - if (vm.envExists("ROOT_EPOCH")) { - kms.removeKmsAggregatedMrAtEpoch(vm.envUint("ROOT_EPOCH"), mrAggregated); - } else { - kms.removeKmsAggregatedMr(mrAggregated); - } + kms.removeKmsAggregatedMrAtEpochs(from, to, mrAggregated); vm.stopBroadcast(); console.log("Removed KMS aggregated MR:", vm.toString(mrAggregated)); @@ -64,12 +85,9 @@ contract AddOsImage is BaseScript { function run() external { bytes32 imageHash = vm.envBytes32("OS_IMAGE_HASH"); + (uint256 from, uint256 to) = addRange(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); - if (vm.envExists("ROOT_EPOCH")) { - kms.addOsImageHashAtEpoch(vm.envUint("ROOT_EPOCH"), imageHash); - } else { - kms.addOsImageHash(imageHash); - } + kms.addOsImageHashAtEpochs(from, to, imageHash); vm.stopBroadcast(); console.log("Added OS image hash:", vm.toString(imageHash)); @@ -80,12 +98,9 @@ contract AddKmsDevice is BaseScript { function run() external { bytes32 deviceId = vm.envBytes32("DEVICE_ID"); + (uint256 from, uint256 to) = addRange(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); - if (vm.envExists("ROOT_EPOCH")) { - kms.addKmsDeviceAtEpoch(vm.envUint("ROOT_EPOCH"), deviceId); - } else { - kms.addKmsDevice(deviceId); - } + kms.addKmsDeviceAtEpochs(from, to, deviceId); vm.stopBroadcast(); console.log("Added KMS device:", vm.toString(deviceId)); @@ -262,6 +277,7 @@ contract BatchKmsSetup is BaseScript { bytes32[] memory osImages = vm.envOr("OS_IMAGE_LIST", ",", new bytes32[](0)); bytes32[] memory devices = vm.envOr("DEVICE_LIST", ",", new bytes32[](0)); + (uint256 from, uint256 to) = addRange(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); // Set gateway ID if provided @@ -272,19 +288,19 @@ contract BatchKmsSetup is BaseScript { // Add aggregated MRs for (uint256 i = 0; i < mrAggregated.length; i++) { - kms.addKmsAggregatedMr(mrAggregated[i]); + kms.addKmsAggregatedMrAtEpochs(from, to, mrAggregated[i]); console.log("Added MR aggregated:", vm.toString(mrAggregated[i])); } // Add OS images for (uint256 i = 0; i < osImages.length; i++) { - kms.addOsImageHash(osImages[i]); + kms.addOsImageHashAtEpochs(from, to, osImages[i]); console.log("Added OS image:", vm.toString(osImages[i])); } // Add devices for (uint256 i = 0; i < devices.length; i++) { - kms.addKmsDevice(devices[i]); + kms.addKmsDeviceAtEpochs(from, to, devices[i]); console.log("Added device:", vm.toString(devices[i])); } @@ -300,12 +316,9 @@ contract RemoveOsImage is BaseScript { function run() external { bytes32 imageHash = vm.envBytes32("OS_IMAGE_HASH"); + (uint256 from, uint256 to) = removeRange(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); - if (vm.envExists("ROOT_EPOCH")) { - kms.removeOsImageHashAtEpoch(vm.envUint("ROOT_EPOCH"), imageHash); - } else { - kms.removeOsImageHash(imageHash); - } + kms.removeOsImageHashAtEpochs(from, to, imageHash); vm.stopBroadcast(); console.log("Removed OS image hash:", vm.toString(imageHash)); @@ -316,12 +329,9 @@ contract RemoveKmsDevice is BaseScript { function run() external { bytes32 deviceId = vm.envBytes32("DEVICE_ID"); + (uint256 from, uint256 to) = removeRange(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); - if (vm.envExists("ROOT_EPOCH")) { - kms.removeKmsDeviceAtEpoch(vm.envUint("ROOT_EPOCH"), deviceId); - } else { - kms.removeKmsDevice(deviceId); - } + kms.removeKmsDeviceAtEpochs(from, to, deviceId); vm.stopBroadcast(); console.log("Removed KMS device:", vm.toString(deviceId)); diff --git a/dstack/kms/auth-eth/script/README.md b/dstack/kms/auth-eth/script/README.md index f993aa951..99d3f1778 100644 --- a/dstack/kms/auth-eth/script/README.md +++ b/dstack/kms/auth-eth/script/README.md @@ -28,6 +28,11 @@ Type-safe contract management scripts. Set the environment variables and run the ### KMS Management +The KMS material scripts below write root epoch rows: `ROOT_EPOCH=e` one row, or +`FROM_ROOT_EPOCH` through `TO_ROOT_EPOCH`. By default additions write the current +through the latest registered epoch, and removals every row, so a removal +revokes the value at every epoch a request can select. + #### Add KMS Aggregated MR ```bash KMS_CONTRACT_ADDR=0x... MR_AGGREGATED=0x1234... \ diff --git a/dstack/kms/auth-eth/test/DstackApp.t.sol b/dstack/kms/auth-eth/test/DstackApp.t.sol index 9bfe7f87d..7fb840748 100644 --- a/dstack/kms/auth-eth/test/DstackApp.t.sol +++ b/dstack/kms/auth-eth/test/DstackApp.t.sol @@ -503,18 +503,18 @@ contract DstackAppTest is Test { assertFalse(allowed); (allowed,) = authorizeAt(info, 0, keccak256("app_get_key")); assertTrue(allowed); - app.addComposeHash(info.composeHash); + app.addComposeHashAtEpoch(1, info.composeHash); allowed = authorizeCurrent(info); assertFalse(allowed, "each material must be approved in the selected row"); - app.addDevice(info.deviceId); + app.addDeviceAtEpoch(1, info.deviceId); allowed = authorizeCurrent(info); assertTrue(allowed); - app.removeComposeHash(info.composeHash); + app.removeComposeHashAtEpoch(1, info.composeHash); assertFalse(app.allowedComposeHashesAtEpoch(1, info.composeHash)); assertTrue(app.allowedComposeHashes(info.composeHash)); (allowed,) = authorizeAt(info, 0, keccak256("app_get_key")); assertTrue(allowed); - app.addComposeHash(info.composeHash); + app.addComposeHashAtEpoch(1, info.composeHash); app.removeDeviceAtEpoch(0, info.deviceId); assertFalse(app.allowedDeviceIds(info.deviceId)); (allowed,) = authorizeAt(info, 0, keccak256("app_get_key")); @@ -534,8 +534,8 @@ contract DstackAppTest is Test { app.addComposeHashAtEpoch(0, oldInfo.composeHash); app.addDeviceAtEpoch(0, oldInfo.deviceId); app.advanceAppEpoch(0); - app.addComposeHash(newInfo.composeHash); - app.addDevice(newInfo.deviceId); + app.addComposeHashAtEpoch(1, newInfo.composeHash); + app.addDeviceAtEpoch(1, newInfo.deviceId); (bool allowed,) = authorizeAt(newInfo, 0, keccak256("app_get_key")); assertFalse(allowed, "new epoch approval must not imply historical access"); app.addComposeHashAtEpoch(0, newInfo.composeHash); @@ -638,30 +638,33 @@ contract DstackAppTest is Test { vm.stopPrank(); } - function test_EpochlessAppSettersChangeTheCurrentAndNextRows() public { - bytes32 composeHash = bytes32(uint256(7)); - bytes32 deviceId = bytes32(uint256(8)); + function test_EpochlessAppSettersChangeEveryRow() public { + IAppAuth.AppInfo memory info = epochInfo(); vm.startPrank(owner); app.advanceAppEpoch(0); - app.addComposeHash(composeHash); - app.addDevice(deviceId); + app.addComposeHash(info.composeHash); + app.addDevice(info.deviceId); app.setAllowAnyDevice(true); - for (uint256 epoch = 1; epoch <= 2; ++epoch) { - assertTrue(app.allowedComposeHashesAtEpoch(epoch, composeHash)); - assertTrue(app.allowedDeviceIdsAtEpoch(epoch, deviceId)); + for (uint256 epoch = 0; epoch <= 2; ++epoch) { + assertTrue(app.allowedComposeHashesAtEpoch(epoch, info.composeHash)); + assertTrue(app.allowedDeviceIdsAtEpoch(epoch, info.deviceId)); assertTrue(app.allowAnyDeviceAtEpoch(epoch)); } - assertFalse(app.allowedComposeHashesAtEpoch(0, composeHash)); - assertFalse(app.allowedDeviceIdsAtEpoch(0, deviceId)); - app.removeComposeHash(composeHash); - app.removeDevice(deviceId); + (bool allowed,) = authorizeAt(info, 0, keccak256("app_get_key")); + assertTrue(allowed); + + // A removal is a full revocation: no selectable epoch still allows it. + app.removeComposeHash(info.composeHash); + app.removeDevice(info.deviceId); app.setAllowAnyDevice(false); - for (uint256 epoch = 1; epoch <= 2; ++epoch) { - assertFalse(app.allowedComposeHashesAtEpoch(epoch, composeHash)); - assertFalse(app.allowedDeviceIdsAtEpoch(epoch, deviceId)); + for (uint256 epoch = 0; epoch <= 2; ++epoch) { + assertFalse(app.allowedComposeHashesAtEpoch(epoch, info.composeHash)); + assertFalse(app.allowedDeviceIdsAtEpoch(epoch, info.deviceId)); assertFalse(app.allowAnyDeviceAtEpoch(epoch)); } - assertFalse(app.allowedComposeHashesAtEpoch(3, composeHash)); + (allowed,) = authorizeAt(info, 0, keccak256("app_get_key")); + assertFalse(allowed); + assertFalse(authorizeCurrent(info)); vm.stopPrank(); } diff --git a/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol b/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol index 97e8acf01..9969bc7c6 100644 --- a/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol +++ b/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol @@ -49,18 +49,19 @@ contract DstackAppEpochSymbolicTest is Test { // per-epoch maps. Each epoch admits some app so that decisions can be // allowed as well as denied. vm.startPrank(OWNER); - app.addComposeHash(bytes32(uint256(0xc0))); - app.addDevice(bytes32(uint256(0xd0))); + app.addComposeHashAtEpoch(0, bytes32(uint256(0xc0))); + app.addDeviceAtEpoch(0, bytes32(uint256(0xd0))); app.advanceAppEpoch(0); - app.addComposeHash(bytes32(uint256(0xc1))); - app.setAllowAnyDevice(true); + app.addComposeHashAtEpoch(1, bytes32(uint256(0xc1))); + app.setAllowAnyDeviceAtEpoch(1, true); app.advanceAppEpoch(1); - app.addComposeHash(bytes32(uint256(0xc2))); - app.addDevice(bytes32(uint256(0xd2))); + app.addComposeHashAtEpoch(2, bytes32(uint256(0xc2))); + app.addDeviceAtEpoch(2, bytes32(uint256(0xd2))); vm.stopPrank(); } - /// Calldata of one policy setter and the epoch it writes. + /// Calldata of one policy setter: 0-4 write the row of `epoch`, 5-9 are + /// their epoch-less forms, which write every row. function _policyCall( uint256 which, uint256 epoch, @@ -69,10 +70,8 @@ contract DstackAppEpochSymbolicTest is Test { ) internal view - returns (bytes memory data, uint256 target) + returns (bytes memory data) { - uint256 current = app.currentAppEpoch(); - target = which < 5 ? epoch : current; if (which == 0) data = abi.encodeCall(DstackApp.addComposeHashAtEpoch, (epoch, word)); else if (which == 1) data = abi.encodeCall(DstackApp.removeComposeHashAtEpoch, (epoch, word)); else if (which == 2) data = abi.encodeCall(DstackApp.addDeviceAtEpoch, (epoch, word)); @@ -87,7 +86,7 @@ contract DstackAppEpochSymbolicTest is Test { function _ownerPolicyWrite(uint256 which, uint256 epoch, bytes32 word, bool flag) internal { if (which >= POLICY_SETTERS) return; - (bytes memory data,) = _policyCall(which, epoch, word, flag); + bytes memory data = _policyCall(which, epoch, word, flag); vm.prank(OWNER); (bool ok,) = address(app).call(data); ok; // a reverted write leaves the setUp state, which is also a valid pre-state @@ -130,7 +129,7 @@ contract DstackAppEpochSymbolicTest is Test { uint256 before = app.currentAppEpoch(); bytes memory data; - if (which < POLICY_SETTERS) (data,) = _policyCall(which, epoch, word, flag); + if (which < POLICY_SETTERS) data = _policyCall(which, epoch, word, flag); else if (which == 10) data = abi.encodeCall(DstackApp.advanceAppEpoch, (epoch)); else if (which == 11) data = abi.encodeCall(DstackApp.setRequireTcbUpToDate, (flag)); else if (which == 12) data = abi.encodeCall(DstackApp.disableUpgrades, ()); @@ -165,7 +164,7 @@ contract DstackAppEpochSymbolicTest is Test { external { vm.assume(which < 5 && epoch > app.currentAppEpoch() + 1); - (bytes memory data,) = _policyCall(which, epoch, word, flag); + bytes memory data = _policyCall(which, epoch, word, flag); vm.prank(caller); (bool ok,) = address(app).call(data); assert(!ok); @@ -248,9 +247,8 @@ contract DstackAppEpochSymbolicTest is Test { } // --------------------------------------------------------------- - // Epoch isolation: a policy write at one epoch never changes an - // authorization decision at another epoch, including epoch 0's - // legacy fields. Covers both the explicit and current-epoch setters. + // Epoch isolation: an *AtEpoch write never changes an authorization + // decision at another epoch, including epoch 0's legacy fields. // --------------------------------------------------------------- function check_PolicyWrite_PreservesOtherEpochDecisions( @@ -265,9 +263,8 @@ contract DstackAppEpochSymbolicTest is Test { ) external { - vm.assume(which < POLICY_SETTERS); - (bytes memory data, uint256 target) = _policyCall(which, writeEpoch, word, flag); - vm.assume(target != epoch); + vm.assume(which < 5 && writeEpoch != epoch); + bytes memory data = _policyCall(which, writeEpoch, word, flag); IAppAuth.AppInfo memory info = _info(composeHash, deviceId, upToDate); (bool allowedBefore, bytes32 reasonBefore) = _decision(info, epoch); @@ -305,19 +302,23 @@ contract DstackAppEpochSymbolicTest is Test { assert(allowed == expected); } + // A write takes effect on its row; an epoch-less one on every row up to + // the next epoch, so an epoch-less removal leaves no selectable epoch + // allowing the value. function check_PolicyWrite_TakesEffect(uint256 which, uint256 epoch, bytes32 word, bool flag) external { - vm.assume(which < 5 && epoch <= app.currentAppEpoch()); - (bytes memory data,) = _policyCall(which, epoch, word, flag); + vm.assume(which < POLICY_SETTERS && epoch <= app.currentAppEpoch() + 1); + bytes memory data = _policyCall(which, epoch, word, flag); vm.prank(OWNER); (bool ok,) = address(app).call(data); + uint256 kind = which % 5; if (!ok) { - assert(word == bytes32(0) && (which == 0 || which == 2)); + assert(word == bytes32(0) && (kind == 0 || kind == 2)); return; } - if (which == 0) assert(app.allowedComposeHashesAtEpoch(epoch, word)); - if (which == 1) assert(!app.allowedComposeHashesAtEpoch(epoch, word)); - if (which == 2) assert(app.allowedDeviceIdsAtEpoch(epoch, word)); - if (which == 3) assert(!app.allowedDeviceIdsAtEpoch(epoch, word)); - if (which == 4) assert(app.allowAnyDeviceAtEpoch(epoch) == flag); + if (kind == 0) assert(app.allowedComposeHashesAtEpoch(epoch, word)); + if (kind == 1) assert(!app.allowedComposeHashesAtEpoch(epoch, word)); + if (kind == 2) assert(app.allowedDeviceIdsAtEpoch(epoch, word)); + if (kind == 3) assert(!app.allowedDeviceIdsAtEpoch(epoch, word)); + if (kind == 4) assert(app.allowAnyDeviceAtEpoch(epoch) == flag); } } diff --git a/dstack/kms/auth-eth/test/DstackKms.symbolic.t.sol b/dstack/kms/auth-eth/test/DstackKms.symbolic.t.sol index ad9f7ac63..ba34bb846 100644 --- a/dstack/kms/auth-eth/test/DstackKms.symbolic.t.sol +++ b/dstack/kms/auth-eth/test/DstackKms.symbolic.t.sol @@ -125,7 +125,7 @@ contract DstackKmsSymbolicTest is Test { vm.startPrank(OWNER); kms.registerApp(address(mock)); - kms.addOsImageHash(osImageHash); + kms.addOsImageHashAtEpoch(0, osImageHash); vm.stopPrank(); IAppAuth.AppInfo memory b = IAppAuth.AppInfo({ @@ -155,7 +155,7 @@ contract DstackKmsSymbolicTest is Test { vm.startPrank(OWNER); kms.registerApp(address(mock)); - kms.addOsImageHash(osImageHash); + kms.addOsImageHashAtEpoch(0, osImageHash); vm.stopPrank(); IAppAuth.AppInfo memory b = IAppAuth.AppInfo({ @@ -190,7 +190,7 @@ contract DstackKmsSymbolicTest is Test { // Allow the OS image so the second gate passes; leave // kmsAllowedAggregatedMrs empty. vm.prank(OWNER); - kms.addOsImageHash(osImageHash); + kms.addOsImageHashAtEpoch(0, osImageHash); IAppAuth.AppInfo memory b = IAppAuth.AppInfo({ appId: address(0), @@ -217,8 +217,8 @@ contract DstackKmsSymbolicTest is Test { external { vm.startPrank(OWNER); - kms.addOsImageHash(osImageHash); - kms.addKmsAggregatedMr(mrAggregated); + kms.addOsImageHashAtEpoch(0, osImageHash); + kms.addKmsAggregatedMrAtEpoch(0, mrAggregated); vm.stopPrank(); // kmsAllowedDeviceIds is left empty. @@ -338,17 +338,18 @@ contract DstackKmsSymbolicTest is Test { } else if (which == 2) { data = abi.encodeWithSelector(DstackKms.registerApp.selector, addr); } else if (which == 3) { - data = abi.encodeWithSelector(DstackKms.addKmsAggregatedMr.selector, word); + data = abi.encodeWithSelector(DstackKms.addKmsAggregatedMrAtEpochs.selector, uint256(0), uint256(0), word); } else if (which == 4) { - data = abi.encodeWithSelector(DstackKms.removeKmsAggregatedMr.selector, word); + data = + abi.encodeWithSelector(DstackKms.removeKmsAggregatedMrAtEpochs.selector, uint256(0), uint256(0), word); } else if (which == 5) { - data = abi.encodeWithSelector(DstackKms.addKmsDevice.selector, word); + data = abi.encodeWithSelector(DstackKms.addKmsDeviceAtEpochs.selector, uint256(0), uint256(0), word); } else if (which == 6) { - data = abi.encodeWithSelector(DstackKms.removeKmsDevice.selector, word); + data = abi.encodeWithSelector(DstackKms.removeKmsDeviceAtEpochs.selector, uint256(0), uint256(0), word); } else if (which == 7) { - data = abi.encodeWithSelector(DstackKms.addOsImageHash.selector, word); + data = abi.encodeWithSelector(DstackKms.addOsImageHashAtEpochs.selector, uint256(0), uint256(0), word); } else if (which == 8) { - data = abi.encodeWithSelector(DstackKms.removeOsImageHash.selector, word); + data = abi.encodeWithSelector(DstackKms.removeOsImageHashAtEpochs.selector, uint256(0), uint256(0), word); } else if (which == 9) { data = abi.encodeWithSelector( bytes4(keccak256("deployAndRegisterApp(address,bool,bool,bool,bytes32,bytes32)")), diff --git a/dstack/kms/auth-eth/test/DstackKms.t.sol b/dstack/kms/auth-eth/test/DstackKms.t.sol index 4c9ae3363..b3d1b6913 100644 --- a/dstack/kms/auth-eth/test/DstackKms.t.sol +++ b/dstack/kms/auth-eth/test/DstackKms.t.sol @@ -394,7 +394,7 @@ contract DstackKmsTest is Test { bytes32 mr = bytes32(uint256(123)); vm.prank(owner); - kms.addKmsAggregatedMr(mr); + kms.addKmsAggregatedMrAtEpoch(0, mr); assertTrue(kms.kmsAllowedAggregatedMrs(mr)); } @@ -403,10 +403,10 @@ contract DstackKmsTest is Test { bytes32 mr = bytes32(uint256(123)); vm.startPrank(owner); - kms.addKmsAggregatedMr(mr); + kms.addKmsAggregatedMrAtEpoch(0, mr); assertTrue(kms.kmsAllowedAggregatedMrs(mr)); - kms.removeKmsAggregatedMr(mr); + kms.removeKmsAggregatedMrAtEpoch(0, mr); assertFalse(kms.kmsAllowedAggregatedMrs(mr)); vm.stopPrank(); } @@ -458,10 +458,10 @@ contract DstackKmsTest is Test { bytes32 deviceId = bytes32(uint256(999)); vm.startPrank(owner); - kms.addKmsDevice(deviceId); + kms.addKmsDeviceAtEpoch(0, deviceId); assertTrue(kms.kmsAllowedDeviceIds(deviceId)); - kms.removeKmsDevice(deviceId); + kms.removeKmsDeviceAtEpoch(0, deviceId); assertFalse(kms.kmsAllowedDeviceIds(deviceId)); vm.stopPrank(); } @@ -470,10 +470,10 @@ contract DstackKmsTest is Test { bytes32 imageHash = bytes32(uint256(888)); vm.startPrank(owner); - kms.addOsImageHash(imageHash); + kms.addOsImageHashAtEpoch(0, imageHash); assertTrue(kms.allowedOsImages(imageHash)); - kms.removeOsImageHash(imageHash); + kms.removeOsImageHashAtEpoch(0, imageHash); assertFalse(kms.allowedOsImages(imageHash)); vm.stopPrank(); } @@ -505,9 +505,9 @@ contract DstackKmsTest is Test { function test_AuthorizationOperationsFailClosedAndBootstrapBeforeRegistration() public { IAppAuth.AppInfo memory info = authorizationInfo(address(kms)); vm.startPrank(owner); - kms.addOsImageHash(info.osImageHash); - kms.addKmsDevice(info.deviceId); - kms.addKmsAggregatedMr(info.mrAggregated); + kms.addOsImageHashAtEpoch(0, info.osImageHash); + kms.addKmsDeviceAtEpoch(0, info.deviceId); + kms.addKmsAggregatedMrAtEpoch(0, info.mrAggregated); vm.stopPrank(); AuthorizationRequest memory request = AuthorizationRequest( info, keccak256("kms_bootstrap"), OptionalUint256(false, 0), OptionalUint256(false, 0) @@ -568,9 +568,9 @@ contract DstackKmsTest is Test { vm.startPrank(owner); kms.registerRootEpoch(ca(0xa0), k256(0xb0), "https://example.com/epoch-0"); kms.setGatewayAppId("gateway"); - kms.addOsImageHash(info.osImageHash); - kms.addKmsDevice(info.deviceId); - kms.addKmsAggregatedMr(info.mrAggregated); + kms.addOsImageHashAtEpoch(0, info.osImageHash); + kms.addKmsDeviceAtEpoch(0, info.deviceId); + kms.addKmsAggregatedMrAtEpoch(0, info.mrAggregated); address appId = kms.deployAndRegisterApp(user, false, false, info.deviceId, info.composeHash); vm.stopPrank(); @@ -659,7 +659,7 @@ contract DstackKmsTest is Test { DstackKms.AppInfo memory info = authorizationInfo(address(0)); vm.startPrank(owner); kms.registerRootEpoch(ca(0xa0), k256(0xb0), "https://example.com/epoch-0"); - kms.addOsImageHash(info.osImageHash); + kms.addOsImageHashAtEpoch(0, info.osImageHash); address appId = kms.deployAndRegisterApp(user, false, false, info.deviceId, info.composeHash); address otherApp = kms.deployAndRegisterApp(user, false, false, info.deviceId, info.composeHash); vm.stopPrank(); @@ -731,9 +731,9 @@ contract DstackKmsTest is Test { // Setup allowed values vm.startPrank(owner); - kms.addKmsDevice(deviceId); - kms.addKmsAggregatedMr(mrAggregated); - kms.addOsImageHash(osImageHash); + kms.addKmsDeviceAtEpoch(0, deviceId); + kms.addKmsAggregatedMrAtEpoch(0, mrAggregated); + kms.addOsImageHashAtEpoch(0, osImageHash); vm.stopPrank(); IAppAuth.AppInfo memory bootInfo = IAppAuth.AppInfo({ @@ -760,9 +760,9 @@ contract DstackKmsTest is Test { // Setup allowed values vm.startPrank(owner); - kms.addKmsDevice(deviceId); - kms.addKmsAggregatedMr(mrAggregated); - kms.addOsImageHash(osImageHash); + kms.addKmsDeviceAtEpoch(0, deviceId); + kms.addKmsAggregatedMrAtEpoch(0, mrAggregated); + kms.addOsImageHashAtEpoch(0, osImageHash); vm.stopPrank(); IAppAuth.AppInfo memory bootInfo = IAppAuth.AppInfo({ @@ -789,7 +789,7 @@ contract DstackKmsTest is Test { bytes32 osImageHash = bytes32(uint256(111)); vm.startPrank(owner); - kms.addOsImageHash(osImageHash); + kms.addOsImageHashAtEpoch(0, osImageHash); address appId = kms.deployAndRegisterApp(user, false, false, deviceId, composeHash); vm.stopPrank(); @@ -921,11 +921,11 @@ contract DstackKmsTest is Test { // Old owner has lost authority. vm.prank(owner); vm.expectRevert(abi.encodeWithSignature("OwnableUnauthorizedAccount(address)", owner)); - kms.addKmsAggregatedMr(mr); + kms.addKmsAggregatedMrAtEpoch(0, mr); // New owner has it. vm.prank(newOwner); - kms.addKmsAggregatedMr(mr); + kms.addKmsAggregatedMrAtEpoch(0, mr); assertTrue(kms.kmsAllowedAggregatedMrs(mr)); } @@ -985,17 +985,17 @@ contract DstackKmsTest is Test { (bool allowed, string memory reason) = kms.isKmsAllowed(info); assertFalse(allowed, "legacy KMS check ends at rotation"); assertEq(reason, "Root epoch requires epoch-aware authorization"); - kms.addOsImageHash(info.osImageHash); + kms.addOsImageHashAtEpoch(1, info.osImageHash); assertFalse(authorize(info, true).authorized); - kms.addKmsAggregatedMr(info.mrAggregated); + kms.addKmsAggregatedMrAtEpoch(1, info.mrAggregated); assertFalse(authorize(info, true).authorized); - kms.addKmsDevice(info.deviceId); + kms.addKmsDeviceAtEpoch(1, info.deviceId); assertTrue(authorize(info, true).authorized); - kms.removeOsImageHash(info.osImageHash); + kms.removeOsImageHashAtEpoch(1, info.osImageHash); assertFalse(authorize(info, true).authorized); assertTrue(kms.allowedOsImages(info.osImageHash)); assertTrue(kms.authorizeKms(kmsEpochRequest(info, 0, 0)).authorized); - kms.addOsImageHash(info.osImageHash); + kms.addOsImageHashAtEpoch(1, info.osImageHash); kms.removeKmsAggregatedMrAtEpoch(0, info.mrAggregated); assertFalse(kms.authorizeKms(kmsEpochRequest(info, 0, 0)).authorized); assertTrue(authorize(info, true).authorized); @@ -1010,7 +1010,7 @@ contract DstackKmsTest is Test { IAppAuth.AppInfo memory info = authorizationInfo(address(0)); vm.startPrank(owner); registerNextRootEpoch(); - kms.addOsImageHash(info.osImageHash); + kms.addOsImageHashAtEpoch(0, info.osImageHash); info.appId = kms.deployAndRegisterApp(user, false, false, info.deviceId, info.composeHash); registerNextRootEpoch(); activateNextRootEpoch(); @@ -1028,7 +1028,7 @@ contract DstackKmsTest is Test { assertTrue(kms.authorizeApp(epochRequest(info, 0, 1)).authorized); assertFalse(authorize(info, false).authorized); vm.prank(owner); - kms.addOsImageHash(info.osImageHash); + kms.addOsImageHashAtEpoch(1, info.osImageHash); assertTrue(authorize(info, false).authorized); assertTrue(kms.authorizeApp(epochRequest(info, 1, 0)).authorized); vm.prank(owner); @@ -1073,7 +1073,7 @@ contract DstackKmsTest is Test { IAppAuth.AppInfo memory info = authorizationInfo(address(0)); vm.startPrank(owner); registerNextRootEpoch(); - kms.addOsImageHash(info.osImageHash); + kms.addOsImageHashAtEpoch(0, info.osImageHash); info.appId = kms.deployAndRegisterApp(user, false, false, info.deviceId, info.composeHash); vm.stopPrank(); AuthorizationRequest memory request = epochRequest(info, 0, 0); @@ -1108,7 +1108,7 @@ contract DstackKmsTest is Test { vm.startPrank(owner); registerNextRootEpoch(); kms.setGatewayAppId("gateway"); - kms.addOsImageHash(info.osImageHash); + kms.addOsImageHashAtEpoch(0, info.osImageHash); info.appId = kms.deployAndRegisterApp(user, false, false, info.deviceId, info.composeHash); vm.stopPrank(); (, bytes32 rootHash) = kms.currentRootEpoch(); @@ -1142,7 +1142,7 @@ contract DstackKmsTest is Test { assertEq(result.latestRootEpochHash, bytes32(0)); vm.startPrank(owner); registerNextRootEpoch(); - kms.addOsImageHash(info.osImageHash); + kms.addOsImageHashAtEpoch(0, info.osImageHash); info.appId = kms.deployAndRegisterApp(user, false, false, info.deviceId, info.composeHash); registerNextRootEpoch(); vm.stopPrank(); @@ -1187,7 +1187,7 @@ contract DstackKmsTest is Test { IAppAuth.AppInfo memory info = authorizationInfo(address(0)); vm.startPrank(owner); registerNextRootEpoch(); - kms.addOsImageHash(info.osImageHash); + kms.addOsImageHashAtEpoch(0, info.osImageHash); info.appId = kms.deployAndRegisterApp(user, false, false, info.deviceId, info.composeHash); vm.stopPrank(); vm.prank(user); @@ -1211,14 +1211,14 @@ contract DstackKmsTest is Test { kms.authorizeApp(epochRequest(info, 0, 0)); } - function test_LegacySettersCoverCurrentAndPendingRows() public { + function test_RangeSettersCoverEveryRowInRange() public { IAppAuth.AppInfo memory info = authorizationInfo(address(0)); vm.startPrank(owner); registerNextRootEpoch(); registerNextRootEpoch(); - kms.addOsImageHash(info.osImageHash); - kms.addKmsAggregatedMr(info.mrAggregated); - kms.addKmsDevice(info.deviceId); + kms.addOsImageHashAtEpochs(0, 1, info.osImageHash); + kms.addKmsAggregatedMrAtEpochs(0, 1, info.mrAggregated); + kms.addKmsDeviceAtEpochs(0, 1, info.deviceId); info.appId = kms.deployAndRegisterApp(user, false, false, info.deviceId, info.composeHash); for (uint256 epoch = 0; epoch < 2; ++epoch) { assertTrue(kms.allowedOsImagesAtEpoch(epoch, info.osImageHash)); @@ -1229,31 +1229,88 @@ contract DstackKmsTest is Test { assertTrue(kms.authorizeKms(kmsEpochRequest(info, 1, 0)).authorized); assertTrue(authorize(info, false).authorized); - // A revocation during the rotation reaches the serving and the pending row. - kms.removeKmsAggregatedMr(info.mrAggregated); + kms.removeKmsAggregatedMrAtEpochs(0, 1, info.mrAggregated); assertFalse(authorize(info, true).authorized); assertFalse(kms.authorizeKms(kmsEpochRequest(info, 1, 0)).authorized); - kms.addKmsAggregatedMr(info.mrAggregated); - kms.removeKmsDevice(info.deviceId); + kms.addKmsAggregatedMrAtEpochs(0, 1, info.mrAggregated); + kms.removeKmsDeviceAtEpochs(0, 1, info.deviceId); assertFalse(authorize(info, true).authorized); assertFalse(kms.authorizeKms(kmsEpochRequest(info, 1, 0)).authorized); - kms.addKmsDevice(info.deviceId); - kms.removeOsImageHash(info.osImageHash); - assertFalse(authorize(info, true).authorized); - assertFalse(kms.authorizeKms(kmsEpochRequest(info, 1, 0)).authorized); - assertFalse(authorize(info, false).authorized); + kms.addKmsDeviceAtEpochs(0, 1, info.deviceId); + + vm.expectRevert("Invalid root epoch range"); + kms.removeOsImageHashAtEpochs(1, 0, info.osImageHash); + vm.expectRevert("Unknown root epoch"); + kms.removeOsImageHashAtEpochs(0, 2, info.osImageHash); + vm.stopPrank(); + } + + function test_RangeRemovalRevokesOlderRootRows() public { + IAppAuth.AppInfo memory info = authorizationInfo(address(0)); + vm.startPrank(owner); + registerNextRootEpoch(); + kms.addOsImageHashAtEpoch(0, info.osImageHash); + info.appId = kms.deployAndRegisterApp(user, false, false, info.deviceId, info.composeHash); + registerNextRootEpoch(); activateNextRootEpoch(); - assertFalse(authorize(info, true).authorized); + kms.addOsImageHashAtEpoch(1, info.osImageHash); + assertTrue(authorize(info, false).authorized); + assertTrue(kms.authorizeApp(epochRequest(info, 0, 0)).authorized); + + kms.removeOsImageHashAtEpochs(0, 1, info.osImageHash); + vm.stopPrank(); assertFalse(authorize(info, false).authorized); + for (uint256 epoch = 0; epoch < 2; ++epoch) { + assertFalse(kms.allowedOsImagesAtEpoch(epoch, info.osImageHash)); + AuthorizationResult memory result = kms.authorizeApp(epochRequest(info, epoch, 0)); + assertEq(result.rejectionReason, "OS image is not allowed"); + } + } - // Once epoch one is current, epoch zero is no longer written. - kms.addOsImageHash(info.osImageHash); - assertFalse(kms.allowedOsImagesAtEpoch(0, info.osImageHash)); - assertTrue(authorize(info, true).authorized); - assertTrue(authorize(info, false).authorized); + function test_PopThenReRegisterIdenticalKeysStartsWithEmptyRows() public { + IAppAuth.AppInfo memory info = authorizationInfo(address(kms)); + vm.startPrank(owner); + registerNextRootEpoch(); + registerNextRootEpoch(); + approveKmsAtEpoch(info, 1); + assertTrue(kms.authorizeKms(kmsEpochRequest(info, 1, 0)).authorized); + (,,, bytes32 popped) = kms.rootEpoch(1); + + kms.popRootEpoch(1, popped); + assertFalse(kms.allowedOsImagesAtEpoch(1, info.osImageHash)); + vm.expectRevert("Unknown root epoch"); + kms.removeOsImageHashAtEpoch(1, info.osImageHash); + + registerNextRootEpoch(); + (,,, bytes32 again) = kms.rootEpoch(1); + assertEq(again, popped); + assertFalse(kms.allowedOsImagesAtEpoch(1, info.osImageHash)); + assertFalse(kms.kmsAllowedAggregatedMrsAtEpoch(1, info.mrAggregated)); + assertFalse(kms.kmsAllowedDeviceIdsAtEpoch(1, info.deviceId)); + assertFalse(kms.authorizeKms(kmsEpochRequest(info, 1, 0)).authorized); vm.stopPrank(); } + function test_KmsServeIsLimitedToTheCurrentRootEpoch() public { + IAppAuth.AppInfo memory info = authorizationInfo(address(kms)); + vm.startPrank(owner); + registerNextRootEpoch(); + registerNextRootEpoch(); + approveKmsAtEpoch(info, 0); + approveKmsAtEpoch(info, 1); + vm.stopPrank(); + AuthorizationRequest memory request = kmsEpochRequest(info, 1, 0); + assertTrue(kms.authorizeKms(request).authorized); + request.operation = keccak256("kms_export_keys"); + assertTrue(kms.authorizeKms(request).authorized); + request.operation = keccak256("kms_serve"); + assertEq(kms.authorizeKms(request).rejectionReason, "Requested root epoch is not active"); + request.requestedRootEpoch = OptionalUint256(true, 0); + assertTrue(kms.authorizeKms(request).authorized); + request.requestedRootEpoch = OptionalUint256(false, 0); + assertTrue(kms.authorizeKms(request).authorized); + } + function test_AtEpochSettersChangeOnlyTheirRow() public { IAppAuth.AppInfo memory info = authorizationInfo(address(kms)); vm.startPrank(owner); diff --git a/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol b/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol index 1ffe7ff84..a035c5ff9 100644 --- a/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol +++ b/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol @@ -121,32 +121,22 @@ contract DstackKmsEpochSymbolicTest is Test { return AuthorizationRequest(info, operation, OptionalUint256(appSet, appEpoch), OptionalUint256(rootSet, root)); } - /// Calldata of one KMS policy setter and the root epoch it writes. - function _kmsPolicyCall( - uint256 which, - uint256 epoch, - bytes32 word - ) - internal - view - returns (bytes memory data, uint256 target) - { - // Setters without an epoch write every row from the current epoch to the - // latest registered one; `target` is the lowest row written. - (target,) = kms.currentRootEpoch(); - if (which < 6) target = epoch; + /// Calldata of one KMS policy setter writing the row of `epoch` and, for the + /// range setters, every later registered row. + function _kmsPolicyCall(uint256 which, uint256 epoch, bytes32 word) internal view returns (bytes memory data) { + uint256 latest = kms.rootEpochCount() - 1; if (which == 0) data = abi.encodeCall(DstackKms.addKmsAggregatedMrAtEpoch, (epoch, word)); else if (which == 1) data = abi.encodeCall(DstackKms.removeKmsAggregatedMrAtEpoch, (epoch, word)); else if (which == 2) data = abi.encodeCall(DstackKms.addKmsDeviceAtEpoch, (epoch, word)); else if (which == 3) data = abi.encodeCall(DstackKms.removeKmsDeviceAtEpoch, (epoch, word)); else if (which == 4) data = abi.encodeCall(DstackKms.addOsImageHashAtEpoch, (epoch, word)); else if (which == 5) data = abi.encodeCall(DstackKms.removeOsImageHashAtEpoch, (epoch, word)); - else if (which == 6) data = abi.encodeCall(DstackKms.addKmsAggregatedMr, (word)); - else if (which == 7) data = abi.encodeCall(DstackKms.removeKmsAggregatedMr, (word)); - else if (which == 8) data = abi.encodeCall(DstackKms.addKmsDevice, (word)); - else if (which == 9) data = abi.encodeCall(DstackKms.removeKmsDevice, (word)); - else if (which == 10) data = abi.encodeCall(DstackKms.addOsImageHash, (word)); - else data = abi.encodeCall(DstackKms.removeOsImageHash, (word)); + else if (which == 6) data = abi.encodeCall(DstackKms.addKmsAggregatedMrAtEpochs, (epoch, latest, word)); + else if (which == 7) data = abi.encodeCall(DstackKms.removeKmsAggregatedMrAtEpochs, (epoch, latest, word)); + else if (which == 8) data = abi.encodeCall(DstackKms.addKmsDeviceAtEpochs, (epoch, latest, word)); + else if (which == 9) data = abi.encodeCall(DstackKms.removeKmsDeviceAtEpochs, (epoch, latest, word)); + else if (which == 10) data = abi.encodeCall(DstackKms.addOsImageHashAtEpochs, (epoch, latest, word)); + else data = abi.encodeCall(DstackKms.removeOsImageHashAtEpochs, (epoch, latest, word)); } // --------------------------------------------------------------- @@ -177,7 +167,7 @@ contract DstackKmsEpochSymbolicTest is Test { returns (bytes memory data) { if (which < KMS_POLICY_SETTERS) { - (data,) = _kmsPolicyCall(which, epoch, word); + data = _kmsPolicyCall(which, epoch, word); } else if (which == 12) { data = abi.encodeCall(DstackKms.registerRootEpoch, (_ca(word, hash), _k256(flag, word), "url")); } else if (which == 13) { @@ -334,7 +324,8 @@ contract DstackKmsEpochSymbolicTest is Test { assert(result.currentRootEpoch == current && result.currentRootEpochHash == currentHash); bool kindMatches = isKms ? AuthorizationOperation.isKms(operation) : AuthorizationOperation.isApp(operation); - uint256 rootBound = isKms ? kms.rootEpochCount() - 1 : current; + bool issuesKeys = !isKms || operation == AuthorizationOperation.KMS_SERVE; + uint256 rootBound = issuesKeys ? current : kms.rootEpochCount() - 1; uint256 appBound = isKms ? 0 : app.currentAppEpoch(); if (!kindMatches || (rootSet && root > rootBound) || (appSet && appEpoch > appBound)) { assert(!result.authorized); @@ -357,9 +348,9 @@ contract DstackKmsEpochSymbolicTest is Test { bytes memory initData = abi.encodeCall(DstackKms.initialize, (OWNER, address(new DstackApp()))); DstackKms fresh = DstackKms(address(new ERC1967Proxy(address(impl), initData))); vm.startPrank(OWNER); - fresh.addOsImageHash(id.osImageHash); - fresh.addKmsAggregatedMr(id.mrAggregated); - fresh.addKmsDevice(id.deviceId); + fresh.addOsImageHashAtEpoch(0, id.osImageHash); + fresh.addKmsAggregatedMrAtEpoch(0, id.mrAggregated); + fresh.addKmsDeviceAtEpoch(0, id.deviceId); vm.stopPrank(); AuthorizationRequest memory request = _request(_info(id), operation, false, 0, false, 0); @@ -384,7 +375,7 @@ contract DstackKmsEpochSymbolicTest is Test { external { if (which < KMS_POLICY_SETTERS) { - (bytes memory data,) = _kmsPolicyCall(which, writeEpoch, word); + bytes memory data = _kmsPolicyCall(which, writeEpoch, word); vm.prank(OWNER); (bool ok,) = address(kms).call(data); ok; @@ -393,7 +384,8 @@ contract DstackKmsEpochSymbolicTest is Test { AuthorizationResult memory result = kms.authorizeKms(_request(_info(id), operation, rootSet, root, false, 0)); uint256 epoch = rootSet ? root : _current(); - bool expected = AuthorizationOperation.isKms(operation) && epoch < kms.rootEpochCount() && id.upToDate + uint256 bound = operation == AuthorizationOperation.KMS_SERVE ? _current() : kms.rootEpochCount() - 1; + bool expected = AuthorizationOperation.isKms(operation) && epoch <= bound && id.upToDate && kms.allowedOsImagesAtEpoch(epoch, id.osImageHash) && kms.kmsAllowedAggregatedMrsAtEpoch(epoch, id.mrAggregated) && kms.kmsAllowedDeviceIdsAtEpoch(epoch, id.deviceId); @@ -445,9 +437,8 @@ contract DstackKmsEpochSymbolicTest is Test { external { vm.assume(which < KMS_POLICY_SETTERS); - (bytes memory data, uint256 target) = _kmsPolicyCall(which, writeEpoch, word); - // Epoch-less setters write every row from `target` on. - vm.assume(which < 6 ? target != root : root < target); + bytes memory data = _kmsPolicyCall(which, writeEpoch, word); + vm.assume(which < 6 ? writeEpoch != root : root < writeEpoch); AuthorizationRequest memory request = _request(_info(id), operation, true, root, false, 0); AuthorizationResult memory before = kms.authorizeKms(request); diff --git a/dstack/kms/auth-eth/test/EventAudit.t.sol b/dstack/kms/auth-eth/test/EventAudit.t.sol index 9631b355a..93a26cc67 100644 --- a/dstack/kms/auth-eth/test/EventAudit.t.sol +++ b/dstack/kms/auth-eth/test/EventAudit.t.sol @@ -54,12 +54,12 @@ contract EventAuditTest is Test { vm.recordLogs(); vm.startPrank(owner); - kms.addKmsAggregatedMr(mr); - kms.addKmsDevice(device); - kms.addOsImageHash(image); + kms.addKmsAggregatedMrAtEpoch(0, mr); + kms.addKmsDeviceAtEpoch(0, device); + kms.addOsImageHashAtEpoch(0, image); kms.setGatewayAppId(gateway); kms.registerApp(address(app)); - kms.removeKmsAggregatedMr(mr); + kms.removeKmsAggregatedMrAtEpoch(0, mr); vm.stopPrank(); Vm.Log[] memory logs = vm.getRecordedLogs(); @@ -110,7 +110,7 @@ contract EventAuditTest is Test { vm.recordLogs(); vm.prank(outsider); vm.expectRevert(); - kms.addOsImageHash(image); + kms.addOsImageHashAtEpoch(0, image); Vm.Log[] memory logs = vm.getRecordedLogs(); assertEq(_auditCount(logs), 0); assertFalse(kms.allowedOsImages(image)); diff --git a/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol b/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol index 9525b8730..4dc665bad 100644 --- a/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol +++ b/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol @@ -124,7 +124,7 @@ contract UpgradesWithPluginTest is Test { // Set some state to verify it's preserved kms.setGatewayAppId("original-gateway"); bytes32 mrAggregated = bytes32(uint256(123)); - kms.addKmsAggregatedMr(mrAggregated); + kms.addKmsAggregatedMrAtEpoch(0, mrAggregated); // Upgrade to new implementation using plugin Upgrades.upgradeProxy(kmsProxy, "DstackKmsV2.sol", ""); @@ -305,8 +305,8 @@ contract UpgradesWithPluginTest is Test { bytes32 osImageHash = bytes32(uint256(0xA1)); bytes32 composeHash = bytes32(uint256(0xA2)); bytes32 mrAggregated = bytes32(uint256(0xA3)); - kms.addOsImageHash(osImageHash); - kms.addKmsAggregatedMr(mrAggregated); + kms.addOsImageHashAtEpoch(0, osImageHash); + kms.addKmsAggregatedMrAtEpoch(0, mrAggregated); address appId = kms.deployAndRegisterApp( owner, @@ -359,8 +359,8 @@ contract UpgradesWithPluginTest is Test { bytes32 osImageHash = bytes32(uint256(0xB1)); bytes32 composeHash = bytes32(uint256(0xB2)); bytes32 mrAggregated = bytes32(uint256(0xB3)); - kms.addOsImageHash(osImageHash); - kms.addKmsAggregatedMr(mrAggregated); + kms.addOsImageHashAtEpoch(0, osImageHash); + kms.addKmsAggregatedMrAtEpoch(0, mrAggregated); address appId = kms.deployAndRegisterApp( owner, @@ -419,8 +419,8 @@ contract UpgradesWithPluginTest is Test { bytes32 osImageHash = bytes32(uint256(789)); bytes32 mrAggregated = bytes32(uint256(101_112)); - kms.addOsImageHash(osImageHash); - kms.addKmsAggregatedMr(mrAggregated); + kms.addOsImageHashAtEpoch(0, osImageHash); + kms.addKmsAggregatedMrAtEpoch(0, mrAggregated); kms.setGatewayAppId("complex-test-gateway"); // Upgrade both contracts From 3a28138414a9a43726bda80ed153912cfe8220cd Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 10 Nov 2026 21:36:44 -0800 Subject: [PATCH 2/5] fix(guest): name the epochs whose keys a migration could not get Disk and env migration fetch the keys of the recorded epochs, so an image not allowed in those rows stops boot. Say which root and app epoch it was. Signed-off-by: Kevin Wang --- dstack/dstack-util/src/system_setup.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dstack/dstack-util/src/system_setup.rs b/dstack/dstack-util/src/system_setup.rs index 358250364..78aa51c05 100644 --- a/dstack/dstack-util/src/system_setup.rs +++ b/dstack/dstack-util/src/system_setup.rs @@ -2469,7 +2469,13 @@ impl HostShared { Some(app_epoch), Some(keys.ca_cert.clone()), ) - .await? + .await + .with_context(|| { + format!( + "failed to get the keys of root epoch {epoch}, app epoch {app_epoch}; \ + the policy rows of those epochs must still allow this instance" + ) + })? .0) } } From 6acb1090afb36444ed8529ff0eabe159a242ac0e Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 10 Nov 2026 21:36:44 -0800 Subject: [PATCH 3/5] ci: run the auth-eth Halmos proofs The symbolic checks cover properties no Forge test covers, such as epoch monotonicity and the legacy entry points refusing after rotation. Run each suite in its own job when dstack/kms/auth-eth changes, with Halmos and Forge pinned. Signed-off-by: Kevin Wang --- .github/workflows/halmos.yml | 60 ++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 .github/workflows/halmos.yml diff --git a/.github/workflows/halmos.yml b/.github/workflows/halmos.yml new file mode 100644 index 000000000..40f8bf6ad --- /dev/null +++ b/.github/workflows/halmos.yml @@ -0,0 +1,60 @@ +# SPDX-FileCopyrightText: © 2025 Phala Network +# +# SPDX-License-Identifier: Apache-2.0 + +name: KMS Auth-ETH Halmos + +on: + push: + paths: + - 'dstack/kms/auth-eth/**' + - '.github/workflows/halmos.yml' + pull_request: + paths: + - 'dstack/kms/auth-eth/**' + - '.github/workflows/halmos.yml' + workflow_dispatch: + +permissions: + contents: read + +jobs: + halmos: + name: ${{ matrix.contract }} + runs-on: ubuntu-latest + timeout-minutes: 40 + strategy: + fail-fast: false + matrix: + contract: + - DstackAppSymbolicTest + - DstackAppEpochSymbolicTest + - DstackKmsSymbolicTest + - DstackKmsEpochSymbolicTest + defaults: + run: + working-directory: dstack/kms/auth-eth + env: + # Halmos does not support the vm.deployCode that dynamic test linking emits. + FOUNDRY_DYNAMIC_TEST_LINKING: 'false' + steps: + - uses: actions/checkout@v5 + + - name: Checkout contract dependencies + working-directory: . + run: | + git submodule update --init --recursive --depth 1 -- \ + dstack/kms/auth-eth/lib/forge-std \ + dstack/kms/auth-eth/lib/openzeppelin-contracts-upgradeable \ + dstack/kms/auth-eth/lib/openzeppelin-foundry-upgrades + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + with: + version: v1.8.4 + + - name: Install Halmos + run: pipx install halmos==0.3.3 + + - name: Run Halmos + run: halmos --contract ${{ matrix.contract }} From b40367949073b7f069f77bdef5c603d87c3e6b34 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 10 Nov 2026 21:39:32 -0800 Subject: [PATCH 4/5] test(kms-auth): pin the legacy app check refusing a nonzero app epoch Signed-off-by: Kevin Wang --- dstack/kms/auth-eth/test/DstackKms.t.sol | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dstack/kms/auth-eth/test/DstackKms.t.sol b/dstack/kms/auth-eth/test/DstackKms.t.sol index b3d1b6913..ed886357e 100644 --- a/dstack/kms/auth-eth/test/DstackKms.t.sol +++ b/dstack/kms/auth-eth/test/DstackKms.t.sol @@ -1103,6 +1103,24 @@ contract DstackKmsTest is Test { assertEq(reason, "Root epoch requires epoch-aware authorization"); } + function test_LegacyAppCheckRefusesOnceTheAppEpochAdvances() public { + IAppAuth.AppInfo memory info = authorizationInfo(address(0)); + vm.startPrank(owner); + registerNextRootEpoch(); + kms.addOsImageHashAtEpoch(0, info.osImageHash); + info.appId = kms.deployAndRegisterApp(user, false, false, info.deviceId, info.composeHash); + vm.stopPrank(); + (bool allowed,) = kms.isAppAllowed(info); + assertTrue(allowed); + + vm.prank(user); + DstackApp(info.appId).advanceAppEpoch(0); + string memory reason; + (allowed, reason) = kms.isAppAllowed(info); + assertFalse(allowed); + assertEq(reason, "App epoch requires epoch-aware authorization"); + } + function test_AuthorizeAppForwardsRequestAndFillsKmsFields() public { IAppAuth.AppInfo memory info = authorizationInfo(address(0)); vm.startPrank(owner); From 474edd28887beadd6a892e1af0e0d530ad698cee Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Wed, 11 Nov 2026 00:43:28 -0800 Subject: [PATCH 5/5] fix(kms-auth)!: remove the epoch-less DstackApp setters As in DstackKms, every DstackApp setter now names the app epoch rows it writes: *AtEpoch one row, and new *AtEpochs(fromEpoch, toEpoch, ...) a range up to the next epoch. The epoch-less addComposeHash, removeComposeHash, addDevice, removeDevice and setAllowAnyDevice are gone, and with them IAppAuthBasicManagement (0x8fd37527). The new IAppEpochManagement interface (0x853d2b79) lets tools tell current apps from those predating app epochs. App policies deployed from v0.5.9 and v0.6.0 keep working under the upgraded DstackKms: it authorizes them through isAppAllowed for app epoch 0 only. Manage.s.sol detects them by interface ID and calls their epoch-less setters, accepting only APP_EPOCH=0 or no epoch. For current policies, and for every KMS script, it requires an explicit epoch or range. Upgrading an old app keeps its allowlist as app epoch 0. New tests cover both releases against the upgraded KMS, before and after a root rotation, the interface IDs, and the upgrade. Formal verification: - The Halmos properties now cover symbolic ranges, full-range revocation on both sides, empty rows after pop and re-registration, epoch 0 rows being the legacy mappings, and legacy app policies limited to app epoch 0. - New TLA+ model EpochPolicy covers registration-keyed rows, range writes, pop and re-registration, legacy apps and their upgrade, and key release on the selected rows. Two mutation configs show the chain-hash keying and a legacy fallback that ignores the app epoch each break it. Signed-off-by: Kevin Wang --- docs/app-key-epochs.md | 33 ++-- docs/formal/key-rotation/EpochPolicy.cfg | 20 ++ docs/formal/key-rotation/EpochPolicy.tla | 166 ++++++++++++++++ .../key-rotation/EpochPolicyChainHash.cfg | 16 ++ .../EpochPolicyLegacyAppEpoch.cfg | 16 ++ docs/formal/key-rotation/README.md | 19 +- docs/onchain-governance.md | 42 +++- docs/tutorials/gateway-build-configuration.md | 4 +- docs/tutorials/hello-world-app.md | 6 +- .../troubleshooting-gateway-deployment.md | 6 +- dstack/kms/auth-eth/README.md | 11 +- dstack/kms/auth-eth/contracts/DstackApp.sol | 70 ++++--- .../contracts/IAppAuthBasicManagement.sol | 75 ------- .../contracts/IAppEpochManagement.sol | 24 +++ .../kms/auth-eth/docs/formal-verification.md | 40 ++-- dstack/kms/auth-eth/docs/specification.md | 11 +- dstack/kms/auth-eth/script/Manage.s.sol | 145 ++++++++------ dstack/kms/auth-eth/script/README.md | 36 ++-- .../auth-eth/test/DstackApp.symbolic.t.sol | 37 ++-- dstack/kms/auth-eth/test/DstackApp.t.sol | 93 +++++---- .../test/DstackAppEpoch.symbolic.t.sol | 91 +++++++-- dstack/kms/auth-eth/test/DstackKms.t.sol | 4 +- .../test/DstackKmsEpoch.symbolic.t.sol | 171 ++++++++++++++-- dstack/kms/auth-eth/test/EventAudit.t.sol | 12 +- .../kms/auth-eth/test/LegacyAppCompat.t.sol | 186 ++++++++++++++++++ dstack/kms/auth-eth/test/ManageScript.t.sol | 55 ++++++ .../auth-eth/test/UpgradesWithPlugin.t.sol | 6 +- 27 files changed, 1070 insertions(+), 325 deletions(-) create mode 100644 docs/formal/key-rotation/EpochPolicy.cfg create mode 100644 docs/formal/key-rotation/EpochPolicy.tla create mode 100644 docs/formal/key-rotation/EpochPolicyChainHash.cfg create mode 100644 docs/formal/key-rotation/EpochPolicyLegacyAppEpoch.cfg delete mode 100644 dstack/kms/auth-eth/contracts/IAppAuthBasicManagement.sol create mode 100644 dstack/kms/auth-eth/contracts/IAppEpochManagement.sol create mode 100644 dstack/kms/auth-eth/test/LegacyAppCompat.t.sol create mode 100644 dstack/kms/auth-eth/test/ManageScript.t.sol diff --git a/docs/app-key-epochs.md b/docs/app-key-epochs.md index 42f932c0f..ca1bf697a 100644 --- a/docs/app-key-epochs.md +++ b/docs/app-key-epochs.md @@ -55,18 +55,27 @@ the old row after the handover. The old image never needs access to the new row. A request may select any epoch up to the current one and is decided on that epoch's row, so a value is revoked only once every row allowing it is cleared. -`DstackKms` has no epoch-less setters for OS images, KMS aggregated MRs and KMS -devices: `*AtEpoch(epoch, material)` changes one registered root row and -`*AtEpochs(fromEpoch, toEpoch, material)` every row in the range, so one -transaction revokes a value from all rows with `remove*AtEpochs(0, latest, …)`. -Before any root epoch is registered, only epoch zero can be written. `DstackApp` -keeps its `IAppAuthBasicManagement` add/remove methods and `setAllowAnyDevice`; -they change every app row from 0 through the next epoch, so a removal is a full -revocation. Its `*AtEpoch` methods, and `setAllowAnyDeviceAtEpoch`, change one -historical, current or next row. The KMS scripts in `Manage.s.sol` take -`ROOT_EPOCH` for one row or `FROM_ROOT_EPOCH`/`TO_ROOT_EPOCH` for a range; without -them, additions write the current through the latest row and removals every row. -The app scripts take an optional `APP_EPOCH`, as does `SetAllowAnyDevice`. +Neither contract has epoch-less material setters: `*AtEpoch(epoch, material)` +changes one row and `*AtEpochs(fromEpoch, toEpoch, material)` every row in the +range, so one transaction revokes a value from all rows, with +`remove*AtEpochs(0, latest, …)` in `DstackKms` and +`remove*AtEpochs(0, currentAppEpoch, …)` in `DstackApp`. `DstackKms` writes +registered root epochs (before any is registered, only epoch zero); `DstackApp` +writes app epochs up to the next one, as do `setAllowAnyDeviceAtEpoch(s)`. +`DstackApp` reports these setters as the ERC-165 interface +`IAppEpochManagement` and no longer reports `IAppAuthBasicManagement` +(`0x8fd37527`), whose epoch-less setters it removed. The scripts in +`Manage.s.sol` take `ROOT_EPOCH` or `APP_EPOCH` for one row, or +`FROM_ROOT_EPOCH`/`TO_ROOT_EPOCH` and `FROM_APP_EPOCH`/`TO_APP_EPOCH` for a range, +and fail without them. + +App policies deployed before app epochs (v0.5.x, v0.6.0) keep working with an +upgraded `DstackKms` and KMS: they report no `IAppEpochAuth`, so `authorizeApp` +asks their `isAppAllowed` and only for app epoch zero, and `currentAppEpoch` +reports zero for them. Their owners keep the epoch-less setters until they +upgrade, which `Manage.s.sol` detects and uses with no epoch or `APP_EPOCH=0`. +Upgrading keeps their allowlists as app epoch zero. See +[onchain-governance.md](./onchain-governance.md#apps-deployed-before-app-epochs). Disk and environment migration after a root rotation fetches the keys of the recorded epoch, so a new image migrating old data must be allowed in that old diff --git a/docs/formal/key-rotation/EpochPolicy.cfg b/docs/formal/key-rotation/EpochPolicy.cfg new file mode 100644 index 000000000..18cb9cfe4 --- /dev/null +++ b/docs/formal/key-rotation/EpochPolicy.cfg @@ -0,0 +1,20 @@ +\* SPDX-FileCopyrightText: © 2026 Phala Network +\* SPDX-License-Identifier: Apache-2.0 + +SPECIFICATION Spec +CONSTANTS + Values = {v1, v2} + Keys = {1, 2} + MaxEpoch = 2 + MaxRegistrations = 5 + KeyByChainHash = FALSE + Composes = {c1} + MaxAppEpoch = 2 + LegacyIgnoresAppEpoch = FALSE +CHECK_DEADLOCK FALSE +INVARIANTS + TypeOK + RevocationIsTotal + AppKeyMatchesSelectedRow +PROPERTIES + RegistrationStartsEmpty diff --git a/docs/formal/key-rotation/EpochPolicy.tla b/docs/formal/key-rotation/EpochPolicy.tla new file mode 100644 index 000000000..e47a305a0 --- /dev/null +++ b/docs/formal/key-rotation/EpochPolicy.tla @@ -0,0 +1,166 @@ +\* SPDX-FileCopyrightText: © 2026 Phala Network +\* SPDX-License-Identifier: Apache-2.0 + +----------------------------- MODULE EpochPolicy ----------------------------- +(***************************************************************************) +(* Epoch-scoped policy of DstackKms and DstackApp, and key release decided *) +(* on the selected rows. *) +(* *) +(* Root side (DstackKms): one row of allowed values (OS images; KMS MRs *) +(* and devices alike) per root epoch, written by *AtEpoch(s) over a range *) +(* of registered rows. A pending epoch can be popped and registered again, *) +(* with the same or other keys. Rows are keyed by registration *) +(* (RootEpoch.registration); KeyByChainHash = TRUE models the earlier *) +(* contract, which keyed them by chain hash. *) +(* *) +(* App side (DstackApp): one row of allowed compose hashes per app epoch, *) +(* written over a range up to the next epoch. An app policy predating app *) +(* epochs (v0.5.x, v0.6.0) has only its flat mapping, which is row 0, and *) +(* epoch-less setters writing it; DstackKms authorizes it through *) +(* isAppAllowed for app epoch 0 only. Its owner may upgrade it at any *) +(* time, keeping the mapping as row 0. LegacyIgnoresAppEpoch = TRUE drops *) +(* the app epoch 0 check of that fallback. *) +(***************************************************************************) +EXTENDS Naturals, Sequences + +CONSTANTS Values, Keys, MaxEpoch, MaxRegistrations, KeyByChainHash, + Composes, MaxAppEpoch, LegacyIgnoresAppEpoch + +VARIABLES + keys, \* keys of the registered root epochs, epoch e at index e + 1 + regs, \* registration of each registered root epoch + nextReg, \* registrations so far + cur, \* current root epoch + rows, \* <> pairs allowed + revoked, \* values removed from every registered row, not added since + legacy, \* the app policy predates app epochs + appCur, \* current app epoch + appRows, \* <> pairs allowed + appRevoked, \* compose hashes removed from every app row, not added since + leaked, \* a key released for a revoked value or compose hash + mismatched \* an app key released for an app epoch whose row disallows it + +vars == <> +rootVars == <> +appVars == <> +ghostVars == <> + +Latest == Len(keys) - 1 +RowKey(ks, rs, e) == IF KeyByChainHash THEN SubSeq(ks, 1, e + 1) ELSE <> +Row(e) == {v \in Values: <> \in rows} +AppRow(a) == {c \in Composes: <> \in appRows} + +Init == + /\ \E k \in Keys: keys = <> + /\ regs = <<0>> /\ nextReg = 1 /\ cur = 0 + /\ rows = {} /\ revoked = {} + /\ legacy \in BOOLEAN /\ appCur = 0 /\ appRows = {} /\ appRevoked = {} + /\ leaked = {} /\ mismatched = FALSE + +\* ---- DstackKms ---- +Register(k) == + /\ Latest < MaxEpoch /\ nextReg < MaxRegistrations + /\ keys' = Append(keys, k) + /\ regs' = Append(regs, nextReg) + /\ nextReg' = nextReg + 1 + /\ UNCHANGED <> + +Activate == + /\ cur < Latest + /\ cur' = cur + 1 + /\ UNCHANGED <> + +Pop == + /\ Latest > cur + /\ keys' = SubSeq(keys, 1, Latest) + /\ regs' = SubSeq(regs, 1, Latest) + /\ UNCHANGED <> + +\* add*AtEpochs / remove*AtEpochs(from, to, v); *AtEpoch is from = to. +Write(from, to, v, allowed) == + /\ from <= to /\ to <= Latest + /\ LET written == {<>: e \in from..to} + IN rows' = IF allowed THEN rows \cup written ELSE rows \ written + /\ revoked' = IF allowed THEN revoked \ {v} + ELSE IF from = 0 /\ to = Latest THEN revoked \cup {v} + ELSE revoked + /\ UNCHANGED <> + +\* ---- DstackApp ---- +AppWrite(from, to, c, allowed) == + /\ ~legacy /\ from <= to /\ to <= appCur + 1 + /\ LET written == {<>: a \in from..to} + IN appRows' = IF allowed THEN appRows \cup written ELSE appRows \ written + /\ appRevoked' = IF allowed THEN appRevoked \ {c} + ELSE IF from = 0 /\ to = appCur + 1 THEN appRevoked \cup {c} + ELSE appRevoked + /\ UNCHANGED <> + +\* The epoch-less setters of a legacy policy write its only row. +LegacyWrite(c, allowed) == + /\ legacy + /\ appRows' = IF allowed THEN appRows \cup {<<0, c>>} ELSE appRows \ {<<0, c>>} + /\ appRevoked' = IF allowed THEN appRevoked \ {c} ELSE appRevoked \cup {c} + /\ UNCHANGED <> + +Upgrade == + /\ legacy /\ legacy' = FALSE + /\ UNCHANGED <> + +Advance == + /\ ~legacy /\ appCur < MaxAppEpoch + /\ appCur' = appCur + 1 + /\ UNCHANGED <> + +\* ---- Key release ---- +\* kms_serve releases keys of the current epoch at most; the key handover +\* may carry a registered pending one. +KmsRelease(op, e, v) == + /\ e <= (IF op = "kms_export_keys" THEN Latest ELSE cur) + /\ v \in Row(e) + /\ leaked' = IF v \in revoked THEN leaked \cup {<<"root", e, v>>} ELSE leaked + /\ UNCHANGED <> + +\* app_get_key for root epoch e <= current and app epoch a. A legacy policy +\* is asked through isAppAllowed, which reads its only row. +AppRelease(e, a, v, c) == + /\ e <= cur /\ v \in Row(e) + /\ IF legacy + THEN (a = 0 \/ LegacyIgnoresAppEpoch) /\ c \in AppRow(0) + ELSE a <= appCur /\ c \in AppRow(a) + /\ leaked' = leaked \cup (IF v \in revoked THEN {<<"root", e, v>>} ELSE {}) + \cup (IF c \in appRevoked THEN {<<"app", a, c>>} ELSE {}) + /\ mismatched' = (mismatched \/ c \notin AppRow(a)) + /\ UNCHANGED <> + +Next == + \/ Activate \/ Pop \/ Upgrade \/ Advance + \/ \E k \in Keys: Register(k) + \/ \E from, to \in 0..MaxEpoch, v \in Values, allowed \in BOOLEAN: Write(from, to, v, allowed) + \/ \E from, to \in 0..MaxAppEpoch + 1, c \in Composes, allowed \in BOOLEAN: AppWrite(from, to, c, allowed) + \/ \E c \in Composes, allowed \in BOOLEAN: LegacyWrite(c, allowed) + \/ \E op \in {"kms_serve", "kms_export_keys"}, e \in 0..MaxEpoch, v \in Values: KmsRelease(op, e, v) + \/ \E e \in 0..MaxEpoch, a \in 0..MaxAppEpoch, v \in Values, c \in Composes: AppRelease(e, a, v, c) + +Spec == Init /\ [][Next]_vars + +----------------------------------------------------------------------------- +TypeOK == cur <= Latest /\ Latest <= MaxEpoch /\ Len(regs) = Len(keys) /\ appCur <= MaxAppEpoch + /\ (legacy => appCur = 0) + +\* A value or compose hash removed from every row gets no key of any root or +\* app epoch, for any operation, until it is added again; including through a +\* root epoch popped and registered again, and across the upgrade of a legacy +\* app policy. +RevocationIsTotal == leaked = {} + +\* An app key of app epoch a is released only when row a allows the compose +\* hash; for a legacy policy, only app epoch 0, its flat mapping. +AppKeyMatchesSelectedRow == ~mismatched + +\* A root epoch registration starts with an empty row, whatever its keys. +RegistrationStartsEmpty == + [][Len(keys') > Len(keys) => + \A v \in Values: <> \notin rows']_vars +============================================================================= diff --git a/docs/formal/key-rotation/EpochPolicyChainHash.cfg b/docs/formal/key-rotation/EpochPolicyChainHash.cfg new file mode 100644 index 000000000..58751f4fb --- /dev/null +++ b/docs/formal/key-rotation/EpochPolicyChainHash.cfg @@ -0,0 +1,16 @@ +\* SPDX-FileCopyrightText: © 2026 Phala Network +\* SPDX-License-Identifier: Apache-2.0 + +SPECIFICATION Spec +CONSTANTS + Values = {v1, v2} + Keys = {1, 2} + MaxEpoch = 2 + MaxRegistrations = 5 + KeyByChainHash = TRUE + Composes = {c1} + MaxAppEpoch = 2 + LegacyIgnoresAppEpoch = FALSE +CHECK_DEADLOCK FALSE +INVARIANTS + RevocationIsTotal diff --git a/docs/formal/key-rotation/EpochPolicyLegacyAppEpoch.cfg b/docs/formal/key-rotation/EpochPolicyLegacyAppEpoch.cfg new file mode 100644 index 000000000..b96abd94a --- /dev/null +++ b/docs/formal/key-rotation/EpochPolicyLegacyAppEpoch.cfg @@ -0,0 +1,16 @@ +\* SPDX-FileCopyrightText: © 2026 Phala Network +\* SPDX-License-Identifier: Apache-2.0 + +SPECIFICATION Spec +CONSTANTS + Values = {v1, v2} + Keys = {1, 2} + MaxEpoch = 2 + MaxRegistrations = 5 + KeyByChainHash = FALSE + Composes = {c1} + MaxAppEpoch = 2 + LegacyIgnoresAppEpoch = TRUE +CHECK_DEADLOCK FALSE +INVARIANTS + AppKeyMatchesSelectedRow diff --git a/docs/formal/key-rotation/README.md b/docs/formal/key-rotation/README.md index ce5a40cf8..b5e8cfede 100644 --- a/docs/formal/key-rotation/README.md +++ b/docs/formal/key-rotation/README.md @@ -13,6 +13,7 @@ as implemented, checked exhaustively with TLC within the stated bounds. | `KmsRotation.tla` | Root epoch registry, KMS keyrings, onboarding, key release | `DstackKms.sol`, `kms/src/main_service.rs`, `kms/src/onboard_service.rs` | | `DiskMigration.tla` | Data disk setup and LUKS key migration, with crashes and rotations | `dstack-util/src/system_setup.rs`, `system_setup/luks.rs` | | `EnvUpdate.tla` | Env re-encryption at boot and the VMM update window | `dstack-util/src/system_setup.rs`, `vmm/src/app.rs` | +| `EpochPolicy.tla` | Root and app epoch policy rows, range setters, pop and re-registration, legacy app policies, key release on the selected rows | `DstackKms.sol`, `DstackApp.sol` | ## Running @@ -26,6 +27,9 @@ java -cp tla2tools.jar tlc2.TLC -config DiskMigration.cfg DiskMigration.tla | `DiskMigration.cfg` | epochs 0-3 | all invariants hold (828 states) | | `EnvUpdate.cfg` | epochs 0-2, 2 owner updates, lossy host API, fail-closed guest | `OwnerUpdateKept` and `NoAppEnv` hold (267 states) | | `EnvUpdateLossy.cfg` | same, guest only logs an unanswered update | `NoAppEnv` violated | +| `EpochPolicy.cfg` | root epochs 0-2, 5 registrations, 2 choices of keys, 2 values; app epochs 0-2, 1 compose hash | `RevocationIsTotal`, `AppKeyMatchesSelectedRow` and `RegistrationStartsEmpty` hold (6,104,564 states) | +| `EpochPolicyChainHash.cfg` | same, rows keyed by chain hash | `RevocationIsTotal` violated | +| `EpochPolicyLegacyAppEpoch.cfg` | same, legacy app fallback ignores the requested app epoch | `AppKeyMatchesSelectedRow` violated | Checked with TLC 2026.11.06 on OpenJDK 21. `EnvUpdate.cfg` models the guest of #1546; `EnvUpdateLossy.cfg` the guest before it. @@ -40,11 +44,24 @@ produces a counterexample: - `KmsRotation`: skipping the latest registered epoch's chain hash check on handover, which lets an ordinary join pass on a losing pending root. - `EnvUpdate`: starting applications after an unanswered update. +- `EpochPolicy`: the two configs above; and treating a removal from the + current epoch through the latest as a revocation, as the epoch-less setters + before #1561 did, which lets a historical row still release the key. ## Abstractions - A key epoch in `DiskMigration` and `EnvUpdate` stands for the (root, app) - epoch pair; the KMS serves every epoch up to the current one. + epoch pair; the KMS serves every epoch up to the current one. Authorization + is assumed to allow the instance at each of them; `EpochPolicy` models the + rows that decide it, and a migrating instance must be allowed in the old + rows too. +- In `KmsRotation`, keys are released for epochs up to the current one and + handed over up to the latest registered one, the operation bounds of + `kms_serve`/`app_get_key` and `kms_export_keys`/`kms_import_keys`. +- `EpochPolicy` merges the root materials (OS image, KMS MR and device) into + one row per epoch, and the app materials into compose hashes; a release + needs one allowed value in each selected row. It has one app and no KMS + nodes; `KmsRotation` covers the keyrings. - A boot fetches its keys once; the KMS may rotate at any point of `DiskMigration`, and between boots in `EnvUpdate`. - Each cryptsetup command on the device is atomic: it writes a new keyslot diff --git a/docs/onchain-governance.md b/docs/onchain-governance.md index bafe3478d..e5d19387d 100644 --- a/docs/onchain-governance.md +++ b/docs/onchain-governance.md @@ -63,7 +63,7 @@ The auth-api validates boot requests against the smart contract. See [Deployment ## Whitelist OS Image ```bash -OS_IMAGE_HASH=0x \ +OS_IMAGE_HASH=0x ROOT_EPOCH=0 \ forge script script/Manage.s.sol:AddOsImage --broadcast --rpc-url $RPC_URL ``` @@ -109,7 +109,7 @@ sha256sum /path/to/gateway-compose.json | awk '{print "0x"$1}' Then add it: ```bash -APP_CONTRACT_ADDR= COMPOSE_HASH= \ +APP_CONTRACT_ADDR= COMPOSE_HASH= APP_EPOCH=0 \ forge script script/Manage.s.sol:AddComposeHash --broadcast --rpc-url $RPC_URL ``` @@ -139,7 +139,7 @@ sha256sum /path/to/your-app-compose.json | awk '{print "0x"$1}' Then add it: ```bash -APP_CONTRACT_ADDR= COMPOSE_HASH= \ +APP_CONTRACT_ADDR= COMPOSE_HASH= APP_EPOCH=0 \ forge script script/Manage.s.sol:AddComposeHash --broadcast --rpc-url $RPC_URL ``` @@ -165,18 +165,40 @@ The central governance contract that manages OS image whitelisting, app registra ### DstackApp (Per-App Contract) -Each app has its own contract controlling which compose hashes and devices are allowed. +Each app has its own contract controlling which compose hashes and devices are allowed, +per app epoch. Every setter names the app epoch rows it writes: one epoch, or a range up +to the next epoch. Removing a value from rows `0` through the current epoch revokes it. | Function | Description | |----------|-------------| -| `addComposeHash(bytes32)` | Whitelist a compose hash | -| `removeComposeHash(bytes32)` | Remove a compose hash from whitelist | -| `addDevice(bytes32)` | Whitelist a device ID | -| `removeDevice(bytes32)` | Remove a device from whitelist | -| `setAllowAnyDevice(bool)` | Allow any device to run this app | -| `isAppAllowed(AppBootInfo)` | Check if app can boot with given config | +| `addComposeHashAtEpoch(uint256,bytes32)` / `addComposeHashAtEpochs(uint256,uint256,bytes32)` | Whitelist a compose hash in one app epoch or a range | +| `removeComposeHashAtEpoch(uint256,bytes32)` / `removeComposeHashAtEpochs(uint256,uint256,bytes32)` | Remove a compose hash from one app epoch or a range | +| `addDeviceAtEpoch(uint256,bytes32)` / `addDeviceAtEpochs(uint256,uint256,bytes32)` | Whitelist a device ID | +| `removeDeviceAtEpoch(uint256,bytes32)` / `removeDeviceAtEpochs(uint256,uint256,bytes32)` | Remove a device | +| `setAllowAnyDeviceAtEpoch(uint256,bool)` / `setAllowAnyDeviceAtEpochs(uint256,uint256,bool)` | Allow any device to run this app | +| `currentAppEpoch()` | The app epoch new keys are derived for | +| `isAppAllowed(AppBootInfo)` | Check if app can boot with given config (app epoch 0 only) | | `disableUpgrades()` | Permanently disable contract upgrades | +#### Apps deployed before app epochs + +App contracts deployed from dstack 0.6.x or earlier, and not upgraded since, have only +app epoch 0 and the epoch-less setters `addComposeHash(bytes32)`, +`removeComposeHash(bytes32)`, `addDevice(bytes32)`, `removeDevice(bytes32)` and +`setAllowAnyDevice(bool)`. They keep working with an upgraded `DstackKms` and KMS, +which authorize them for app epoch 0. Tell the two kinds apart by ERC-165 interface ID: + +```bash +cast call "supportsInterface(bytes4)(bool)" 0x853d2b79 --rpc-url $RPC_URL # IAppEpochManagement: epoch setters +cast call "supportsInterface(bytes4)(bool)" 0x8fd37527 --rpc-url $RPC_URL # IAppAuthBasicManagement: epoch-less setters +``` + +The `Manage.s.sol` app scripts check this themselves: on an older app they call the +epoch-less setter and accept only `APP_EPOCH=0` or no epoch; on a current app they +require `APP_EPOCH`, or `FROM_APP_EPOCH` and `TO_APP_EPOCH`. Upgrading an older app to +the current `DstackApp` keeps its compose hashes, devices and `allowAnyDevice` as app +epoch 0. + ### AppBootInfo Structure Both `isAppAllowed` and `isKmsAllowed` take an `AppBootInfo` struct: diff --git a/docs/tutorials/gateway-build-configuration.md b/docs/tutorials/gateway-build-configuration.md index a1bc205ea..dd29fcc32 100644 --- a/docs/tutorials/gateway-build-configuration.md +++ b/docs/tutorials/gateway-build-configuration.md @@ -510,7 +510,9 @@ export GATEWAY_APP_ID=$(cat ~/.dstack/secrets/gateway-app-id) Add the compose hash to the gateway app's allowed list: ```bash -cast send "$GATEWAY_APP_ID" "addComposeHash(bytes32)" "0x$COMPOSE_HASH" --rpc-url "$ETH_RPC_URL" --private-key "$PRIVATE_KEY" +# App contracts deployed before app epochs (0.6.x or earlier) take "addComposeHash(bytes32)" "0x$COMPOSE_HASH"; +# see onchain-governance.md#apps-deployed-before-app-epochs. +cast send "$GATEWAY_APP_ID" "addComposeHashAtEpoch(uint256,bytes32)" 0 "0x$COMPOSE_HASH" --rpc-url "$ETH_RPC_URL" --private-key "$PRIVATE_KEY" ``` Verify the hash was added: diff --git a/docs/tutorials/hello-world-app.md b/docs/tutorials/hello-world-app.md index 51d1ce656..31fa493ab 100644 --- a/docs/tutorials/hello-world-app.md +++ b/docs/tutorials/hello-world-app.md @@ -236,9 +236,11 @@ COMPOSE_HASH="" HELLO_APP_ID=$(cat ~/.dstack/secrets/hello-world-app-id) +# App contracts deployed before app epochs (0.6.x or earlier) take "addComposeHash(bytes32)" "0x$COMPOSE_HASH"; +# see onchain-governance.md#apps-deployed-before-app-epochs. cast send "$HELLO_APP_ID" \ - "addComposeHash(bytes32)" \ - "0x$COMPOSE_HASH" \ + "addComposeHashAtEpoch(uint256,bytes32)" \ + 0 "0x$COMPOSE_HASH" \ --rpc-url "$ETH_RPC_URL" \ --private-key "$PRIVATE_KEY" ``` diff --git a/docs/tutorials/troubleshooting-gateway-deployment.md b/docs/tutorials/troubleshooting-gateway-deployment.md index cbd2b4560..053c42c57 100644 --- a/docs/tutorials/troubleshooting-gateway-deployment.md +++ b/docs/tutorials/troubleshooting-gateway-deployment.md @@ -196,9 +196,11 @@ cast call "$(cat ~/.dstack/secrets/gateway-app-id)" \ --rpc-url "https://ethereum-sepolia-rpc.publicnode.com" # If false, add it +# App contracts deployed before app epochs (0.6.x or earlier) take "addComposeHash(bytes32)" "0x$COMPOSE_HASH"; +# see onchain-governance.md#apps-deployed-before-app-epochs. cast send "$(cat ~/.dstack/secrets/gateway-app-id)" \ - "addComposeHash(bytes32)" \ - "0x$COMPOSE_HASH" \ + "addComposeHashAtEpoch(uint256,bytes32)" \ + 0 "0x$COMPOSE_HASH" \ --rpc-url "https://ethereum-sepolia-rpc.publicnode.com" \ --private-key "$(cat ~/.dstack/secrets/sepolia-private-key)" ``` diff --git a/dstack/kms/auth-eth/README.md b/dstack/kms/auth-eth/README.md index 173f6228d..dcb6c647c 100644 --- a/dstack/kms/auth-eth/README.md +++ b/dstack/kms/auth-eth/README.md @@ -307,12 +307,11 @@ See `script/README.md` for complete documentation of all available scripts. #### Epoch-scoped material approvals Material approvals use exact root/app epoch rows; neither older nor newer rows -are implicitly authorized. `DstackKms` material setters name their rows: -`*AtEpoch` one root row, `*AtEpochs` a range. `DstackApp` add/remove methods and -`setAllowAnyDevice` change every app row through the next epoch, so a removal is -a full revocation; its `*AtEpoch` methods change one row. The management scripts -take `ROOT_EPOCH`, `FROM_ROOT_EPOCH`/`TO_ROOT_EPOCH` or `APP_EPOCH` to select -rows. Original bool mappings and events remain epoch-zero views after +are implicitly authorized. Material setters name their rows: `*AtEpoch` one +row, `*AtEpochs` a range; `DstackApp` reports them as `IAppEpochManagement`. +The management scripts take `ROOT_EPOCH`/`APP_EPOCH`, or +`FROM_ROOT_EPOCH`/`TO_ROOT_EPOCH` and `FROM_APP_EPOCH`/`TO_APP_EPOCH`, and use +the epoch-less setters of app policies predating app epochs. Original bool mappings and events remain epoch-zero views after upgrading; epoch-specific getters/events describe the new rows. KMS keyring handover separately checks its highest exported root epoch, which authorizes the whole keyring. See [epoch-scoped allowlists](../../../docs/app-key-epochs.md#epoch-scoped-allowlists) diff --git a/dstack/kms/auth-eth/contracts/DstackApp.sol b/dstack/kms/auth-eth/contracts/DstackApp.sol index 0274ba1e8..722614c67 100644 --- a/dstack/kms/auth-eth/contracts/DstackApp.sol +++ b/dstack/kms/auth-eth/contracts/DstackApp.sol @@ -9,7 +9,7 @@ pragma solidity ^0.8.24; import "./IAppAuth.sol"; import "./AuthorizationOperation.sol"; import "./IAppEpochAuth.sol"; -import "./IAppAuthBasicManagement.sol"; +import "./IAppEpochManagement.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; @@ -21,8 +21,8 @@ contract DstackApp is UUPSUpgradeable, ERC165Upgradeable, IAppAuth, - IAppAuthBasicManagement, - IAppEpochAuth + IAppEpochAuth, + IAppEpochManagement { /// @notice Allowed compose hashes for this app. Epoch 0 only; for other epochs use the *AtEpoch getters and /// EpochPolicyChanged. @@ -52,7 +52,11 @@ contract DstackApp is event EpochPolicyChanged(bytes32 indexed policy, bytes32 indexed value, uint256 indexed epoch, bool enabled); - // Additional events specific to DstackApp + /// @notice Legacy material events. Epoch 0 only; for other epochs use the *AtEpoch getters and EpochPolicyChanged. + event ComposeHashAdded(bytes32 composeHash); + event ComposeHashRemoved(bytes32 composeHash); + event DeviceAdded(bytes32 deviceId); + event DeviceRemoved(bytes32 deviceId); event UpgradesDisabled(); /// @notice Epoch 0 only; for other epochs use the *AtEpoch getters and EpochPolicyChanged. event AllowAnyDeviceSet(bool allowAny); @@ -145,8 +149,8 @@ contract DstackApp is override(ERC165Upgradeable, IERC165) returns (bool) { - return interfaceId == type(IAppEpochAuth).interfaceId || interfaceId == 0x1e079198 // IAppAuth - || interfaceId == 0x8fd37527 // IAppAuthBasicManagement + return interfaceId == type(IAppEpochAuth).interfaceId || interfaceId == type(IAppEpochManagement).interfaceId + || interfaceId == 0x1e079198 // IAppAuth || super.supportsInterface(interfaceId); } @@ -173,33 +177,41 @@ contract DstackApp is } // A request may select any app epoch up to the current one and is decided - // on that epoch's row, so the epoch-less setters change every row, 0 through - // the next epoch: a removal is a full revocation. The *AtEpoch setters - // change a single row. - function _setAllEpochs(function(uint256, bytes32, bool) internal setter, bytes32 value, bool allowed) private { - for (uint256 epoch = 0; epoch <= currentAppEpoch + 1; ++epoch) { + // on that epoch's row, so revoking a value takes every row that allows it: + // 0 through currentAppEpoch. + function _setAtEpochs( + function(uint256, bytes32, bool) internal setter, + uint256 fromEpoch, + uint256 toEpoch, + bytes32 value, + bool allowed + ) + private + { + require(fromEpoch <= toEpoch, "Invalid app epoch range"); + for (uint256 epoch = fromEpoch; epoch <= toEpoch; ++epoch) { setter(epoch, value, allowed); } } - function addComposeHash(bytes32 composeHash) external onlyOwner { - require(composeHash != bytes32(0), "invalid compose hash"); - _setAllEpochs(_setComposeHash, composeHash, true); - } - function addComposeHashAtEpoch(uint256 epoch, bytes32 composeHash) public onlyOwner { require(composeHash != bytes32(0), "invalid compose hash"); _setComposeHash(epoch, composeHash, true); } - function removeComposeHash(bytes32 composeHash) external onlyOwner { - _setAllEpochs(_setComposeHash, composeHash, false); + function addComposeHashAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 composeHash) external onlyOwner { + require(composeHash != bytes32(0), "invalid compose hash"); + _setAtEpochs(_setComposeHash, fromEpoch, toEpoch, composeHash, true); } function removeComposeHashAtEpoch(uint256 epoch, bytes32 composeHash) public onlyOwner { _setComposeHash(epoch, composeHash, false); } + function removeComposeHashAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 composeHash) external onlyOwner { + _setAtEpochs(_setComposeHash, fromEpoch, toEpoch, composeHash, false); + } + function _requireWritableEpoch(uint256 epoch) private view { require(epoch <= currentAppEpoch + 1, "Unknown app epoch"); } @@ -217,14 +229,14 @@ contract DstackApp is emit EpochPolicyChanged(keccak256("compose-hash"), composeHash, epoch, allowed); } - function setAllowAnyDevice(bool allowed) external onlyOwner { - _setAllEpochs(_setAllowAnyDevice, bytes32(0), allowed); - } - function setAllowAnyDeviceAtEpoch(uint256 epoch, bool allowed) public onlyOwner { _setAllowAnyDevice(epoch, bytes32(0), allowed); } + function setAllowAnyDeviceAtEpochs(uint256 fromEpoch, uint256 toEpoch, bool allowed) external onlyOwner { + _setAtEpochs(_setAllowAnyDevice, fromEpoch, toEpoch, bytes32(0), allowed); + } + function _setAllowAnyDevice(uint256 epoch, bytes32, bool allowed) private { _requireWritableEpoch(epoch); if (epoch == 0) { @@ -244,24 +256,24 @@ contract DstackApp is _emitPolicy("require-tcb-up-to-date", bytes32(0), _requireUpToDate); } - function addDevice(bytes32 deviceId) external onlyOwner { - require(deviceId != bytes32(0), "invalid device ID"); - _setAllEpochs(_setDevice, deviceId, true); - } - function addDeviceAtEpoch(uint256 epoch, bytes32 deviceId) public onlyOwner { require(deviceId != bytes32(0), "invalid device ID"); _setDevice(epoch, deviceId, true); } - function removeDevice(bytes32 deviceId) external onlyOwner { - _setAllEpochs(_setDevice, deviceId, false); + function addDeviceAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 deviceId) external onlyOwner { + require(deviceId != bytes32(0), "invalid device ID"); + _setAtEpochs(_setDevice, fromEpoch, toEpoch, deviceId, true); } function removeDeviceAtEpoch(uint256 epoch, bytes32 deviceId) public onlyOwner { _setDevice(epoch, deviceId, false); } + function removeDeviceAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 deviceId) external onlyOwner { + _setAtEpochs(_setDevice, fromEpoch, toEpoch, deviceId, false); + } + function _setDevice(uint256 epoch, bytes32 deviceId, bool allowed) private { _requireWritableEpoch(epoch); if (epoch == 0) { diff --git a/dstack/kms/auth-eth/contracts/IAppAuthBasicManagement.sol b/dstack/kms/auth-eth/contracts/IAppAuthBasicManagement.sol deleted file mode 100644 index f9f34e8d9..000000000 --- a/dstack/kms/auth-eth/contracts/IAppAuthBasicManagement.sol +++ /dev/null @@ -1,75 +0,0 @@ -/* - * SPDX-FileCopyrightText: © 2025 Phala Network - * - * SPDX-License-Identifier: Apache-2.0 - */ - -pragma solidity ^0.8.0; - -import "@openzeppelin/contracts/utils/introspection/IERC165.sol"; - -/** - * @title IAppAuthBasicManagement - * @notice Basic management interface for App Authentication contracts - * @dev This interface defines the standard functions that UI tools and other contracts - * can use to interact with App Auth contracts. Any contract implementing this - * interface should also implement ERC-165 to allow interface detection. - * - * Interface ID: 0x8fd37527 - * - * UI tools can check if a contract supports this interface by calling: - * contract.supportsInterface(type(IAppAuthBasicManagement).interfaceId) - */ -interface IAppAuthBasicManagement is IERC165 { - /// @notice Emitted when a new compose hash is added to the allowed list - /// @dev Epoch 0 only; for other epochs use the *AtEpoch getters and EpochPolicyChanged. - /// @param composeHash The compose hash that was added - event ComposeHashAdded(bytes32 composeHash); - - /// @notice Emitted when a compose hash is removed from the allowed list - /// @dev Epoch 0 only; for other epochs use the *AtEpoch getters and EpochPolicyChanged. - /// @param composeHash The compose hash that was removed - event ComposeHashRemoved(bytes32 composeHash); - - /// @notice Emitted when a new device ID is added to the allowed list - /// @dev Epoch 0 only; for other epochs use the *AtEpoch getters and EpochPolicyChanged. - /// @param deviceId The device ID that was added - event DeviceAdded(bytes32 deviceId); - - /// @notice Emitted when a device ID is removed from the allowed list - /// @dev Epoch 0 only; for other epochs use the *AtEpoch getters and EpochPolicyChanged. - /// @param deviceId The device ID that was removed - event DeviceRemoved(bytes32 deviceId); - - /** - * @notice Add a compose hash to the allowed list - * @dev MUST emit ComposeHashAdded event on success - * MUST revert if caller is not authorized - * @param composeHash The compose hash to add - */ - function addComposeHash(bytes32 composeHash) external; - - /** - * @notice Remove a compose hash from the allowed list - * @dev MUST emit ComposeHashRemoved event on success - * MUST revert if caller is not authorized - * @param composeHash The compose hash to remove - */ - function removeComposeHash(bytes32 composeHash) external; - - /** - * @notice Add a device ID to the allowed list - * @dev MUST emit DeviceAdded event on success - * MUST revert if caller is not authorized - * @param deviceId The device ID to add - */ - function addDevice(bytes32 deviceId) external; - - /** - * @notice Remove a device ID from the allowed list - * @dev MUST emit DeviceRemoved event on success - * MUST revert if caller is not authorized - * @param deviceId The device ID to remove - */ - function removeDevice(bytes32 deviceId) external; -} diff --git a/dstack/kms/auth-eth/contracts/IAppEpochManagement.sol b/dstack/kms/auth-eth/contracts/IAppEpochManagement.sol new file mode 100644 index 000000000..a8585ecad --- /dev/null +++ b/dstack/kms/auth-eth/contracts/IAppEpochManagement.sol @@ -0,0 +1,24 @@ +/* + * SPDX-FileCopyrightText: © 2026 Phala Network + * + * SPDX-License-Identifier: Apache-2.0 + */ + +pragma solidity ^0.8.24; + +/// Epoch-scoped allowlist management. Each setter names the app epoch rows it +/// writes: one epoch, or `fromEpoch` through `toEpoch`, at most the next epoch. +/// Replaces the epoch-less IAppAuthBasicManagement (0x8fd37527), which policies +/// predating app epochs still report; tools tell the two apart by interface ID. +interface IAppEpochManagement { + function addComposeHashAtEpoch(uint256 epoch, bytes32 composeHash) external; + function addComposeHashAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 composeHash) external; + function removeComposeHashAtEpoch(uint256 epoch, bytes32 composeHash) external; + function removeComposeHashAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 composeHash) external; + function addDeviceAtEpoch(uint256 epoch, bytes32 deviceId) external; + function addDeviceAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 deviceId) external; + function removeDeviceAtEpoch(uint256 epoch, bytes32 deviceId) external; + function removeDeviceAtEpochs(uint256 fromEpoch, uint256 toEpoch, bytes32 deviceId) external; + function setAllowAnyDeviceAtEpoch(uint256 epoch, bool allowed) external; + function setAllowAnyDeviceAtEpochs(uint256 fromEpoch, uint256 toEpoch, bool allowed) external; +} diff --git a/dstack/kms/auth-eth/docs/formal-verification.md b/dstack/kms/auth-eth/docs/formal-verification.md index c36b36e45..636387b27 100644 --- a/dstack/kms/auth-eth/docs/formal-verification.md +++ b/dstack/kms/auth-eth/docs/formal-verification.md @@ -21,7 +21,7 @@ correctness matters more than gas. - `contracts/DstackApp.sol` (209 LOC) — per-app boot authorization - `contracts/DstackKms.sol` (276 LOC) — KMS whitelist + factory -- `contracts/IAppAuth.sol`, `contracts/IAppAuthBasicManagement.sol` — interfaces +- `contracts/IAppAuth.sol`, `contracts/IAppEpochAuth.sol`, `contracts/IAppEpochManagement.sol` — interfaces ## Layer 0 — Slither (static analysis) @@ -126,8 +126,9 @@ and admits one app per epoch, then one symbolic owner write where noted): function, by any caller, decreases `currentAppEpoch`; it moves only by one, through the owner's `advanceAppEpoch(current)`. - [x] `check_PolicyWrite_RejectsEpochPastNext` — every `*AtEpoch` setter - reverts for an epoch above the next one, for any caller; the owner may - prepare the next epoch's row before advancing. + reverts for an epoch above the next one, and every `*AtEpochs` setter + for a range ending above it, for any caller; the owner may prepare the + next epoch's row before advancing. - [x] `check_AuthorizeApp_RejectsInactiveEpochAndForeignOperation` — an epoch above the current one or a non-app operation is never authorized. - [x] `check_IsAppAllowed_RefusesAtNonzeroEpoch` — once the app epoch has @@ -136,16 +137,19 @@ and admits one app per epoch, then one symbolic owner write where noted): - [x] `check_IsAppAllowed_EqualsEpochZeroDecisionBeforeAdvance` — at app epoch 0, with the next row prepared and any one owner write, the legacy `isAppAllowed` returns exactly the `authorizeApp` result at epoch 0. -- [x] `check_PolicyWrite_PreservesOtherEpochDecisions` — an `*AtEpoch` - write at one epoch never changes an authorization decision at another - epoch, including epoch 0's legacy fields. +- [x] `check_PolicyWrite_PreservesOtherEpochDecisions` — a write never + changes an authorization decision at an epoch outside the rows it names + (one epoch, or a symbolic range), including epoch 0's legacy fields. - [x] `check_AuthorizeApp_MatchesPolicy` — for both app operations, `authorizeApp` allows exactly when the epoch is active, the TCB requirement holds, and that epoch admits the compose hash and device. - [x] `check_PolicyWrite_TakesEffect` — each `*AtEpoch` setter on an epoch - up to the next one, and each epoch-less setter on every such epoch, - either sets the value it was given or reverts on a zero ID; so an - epoch-less removal leaves no selectable epoch allowing the value. + up to the next one, and each `*AtEpochs` setter on every row of a range + ending at most there, either sets the value it was given or reverts on + a zero ID. +- [x] `check_FullRangeRemoval_RevokesAtEveryAppEpoch` — after removing a + compose hash, or a device and the any-device flag, from rows 0 through + the next epoch, no app epoch authorizes it, for either app operation. Mutation-tested: requiring both the any-device flag and a listed device, ignoring the value of a per-epoch `allowAnyDevice` write, dropping a setter's @@ -185,8 +189,22 @@ per epoch, and deploys an app at app epoch 1): exactly when both epochs are active, the selected root epoch admits the OS image, and the app's own `authorizeApp` admits it. - [x] `check_KmsPolicyWrite_PreservesOtherEpochDecisions` — a KMS policy - write at one root epoch, or through the range setters from one epoch to - the latest, never changes a KMS decision at an epoch it did not write. + write never changes a KMS decision at a root epoch outside the rows it + names (one epoch, or a symbolic range). +- [x] `check_FullRangeRemoval_RevokesAtEveryEpoch` — after removing an OS + image, aggregated MR or KMS device from rows 0 through the latest, no + request carrying it is authorized at any root or app epoch, for any KMS + operation, nor for app operations in the OS image case. +- [x] `check_PopThenRegister_StartsWithEmptyRows` — popping the pending + epoch and registering the number again, with the same keys or others, + leaves all three of its rows empty. +- [x] `check_EpochZeroRows_AreTheLegacyMappings` — after any one owner + write, the v0.5.x getters and the epoch 0 getters agree. +- [x] `check_AuthorizeApp_LegacyPolicyOnlyAtAppEpochZero` — an app policy + without `IAppEpochAuth`, like v0.5.x and v0.6.0 `DstackApp`, is + authorized exactly when the KMS checks pass at the selected root epoch, + no nonzero app epoch is requested, and its `isAppAllowed` admits it; + its app epoch is reported as 0. - [x] `check_LegacyEntryPoints_RefuseOnceRotated` — once a later root epoch is current, `isKmsAllowed` and `isAppAllowed` refuse. - [x] `check_LegacyKmsCheck_ReadsEpochZeroBeforeRotation` — before any diff --git a/dstack/kms/auth-eth/docs/specification.md b/dstack/kms/auth-eth/docs/specification.md index d1eedd72a..b99fe2e37 100644 --- a/dstack/kms/auth-eth/docs/specification.md +++ b/dstack/kms/auth-eth/docs/specification.md @@ -69,9 +69,9 @@ Plus inherited storage in ERC-7201 namespaces: `OwnableUpgradeable`, | Slot | Name | Type | Writable by | |---|---|---|---| -| 0 | `allowedComposeHashes` | `mapping(bytes32 => bool)` | `addComposeHash`, `removeComposeHash` (owner); 5/6-arg initializer | -| 1 | `_upgradesDisabled` + `allowAnyDevice` (packed) | `bool` + `bool` | `disableUpgrades` (owner, monotonic); `setAllowAnyDevice` (owner); initializer | -| 2 | `allowedDeviceIds` | `mapping(bytes32 => bool)` | `addDevice`, `removeDevice` (owner); initializer | +| 0 | `allowedComposeHashes` | `mapping(bytes32 => bool)` | `addComposeHashAtEpoch(s)`, `removeComposeHashAtEpoch(s)` at epoch 0 (owner); 5/6-arg initializer | +| 1 | `_upgradesDisabled` + `allowAnyDevice` (packed) | `bool` + `bool` | `disableUpgrades` (owner, monotonic); `setAllowAnyDeviceAtEpoch(s)` at epoch 0 (owner); initializer | +| 2 | `allowedDeviceIds` | `mapping(bytes32 => bool)` | `addDeviceAtEpoch(s)`, `removeDeviceAtEpoch(s)` at epoch 0 (owner); initializer | | 3 | `requireTcbUpToDate` | `bool` | `setRequireTcbUpToDate` (owner); 6-arg initializer only | | 4 | `__gap[49]` | `uint256[49]` | (none) | @@ -237,8 +237,9 @@ byte sequence — no case variations, no trailing nulls, no whitespace. ### 3.11 Owner-only App mutations -For each of `addComposeHash`, `removeComposeHash`, `addDevice`, `removeDevice`, -`setAllowAnyDevice`, `setRequireTcbUpToDate`, `disableUpgrades`: +For each of the `*AtEpoch` and `*AtEpochs` forms of `addComposeHash`, +`removeComposeHash`, `addDevice`, `removeDevice` and `setAllowAnyDevice`, and +`setRequireTcbUpToDate`, `disableUpgrades`: - **pre**: `msg.sender == owner()`. - **frame**: every storage cell except the one mapped to the operation. diff --git a/dstack/kms/auth-eth/script/Manage.s.sol b/dstack/kms/auth-eth/script/Manage.s.sol index 7548ec645..63ddd6f67 100644 --- a/dstack/kms/auth-eth/script/Manage.s.sol +++ b/dstack/kms/auth-eth/script/Manage.s.sol @@ -9,6 +9,17 @@ pragma solidity ^0.8.24; import "forge-std/Script.sol"; import "../contracts/DstackKms.sol"; import "../contracts/DstackApp.sol"; +import "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; + +// The epoch-less setters of app policies predating app epochs (v0.5.x, +// v0.6.0), which report IAppAuthBasicManagement instead of IAppEpochManagement. +interface ILegacyAppManagement { + function addComposeHash(bytes32 composeHash) external; + function removeComposeHash(bytes32 composeHash) external; + function addDevice(bytes32 deviceId) external; + function removeDevice(bytes32 deviceId) external; + function setAllowAnyDevice(bool allowed) external; +} // Base contract with common functionality abstract contract BaseScript is Script { @@ -26,31 +37,46 @@ abstract contract BaseScript is Script { } } - // The root epoch rows a KMS policy change applies to: ROOT_EPOCH alone, or - // FROM_ROOT_EPOCH (default `defaultFrom`) through TO_ROOT_EPOCH (default the - // latest registered epoch). - function rootEpochRange(uint256 defaultFrom) internal view returns (uint256 from, uint256 to) { - if (vm.envExists("ROOT_EPOCH")) { - from = to = vm.envUint("ROOT_EPOCH"); - } else { - uint256 count = kms.rootEpochCount(); - from = vm.envOr("FROM_ROOT_EPOCH", defaultFrom); - to = vm.envOr("TO_ROOT_EPOCH", count == 0 ? 0 : count - 1); + // The rows a policy change applies to: `single` names one epoch, `from` and + // `to` a range. There is no default, so a change never misses a row + // silently. + function epochRange( + string memory single, + string memory from, + string memory to + ) + internal + view + returns (uint256, uint256) + { + if (vm.envExists(single)) { + uint256 epoch = vm.envUint(single); + return (epoch, epoch); } + require(vm.envExists(from) && vm.envExists(to), string.concat("set ", single, ", or ", from, " and ", to)); + return (vm.envUint(from), vm.envUint(to)); } - // Additions default to the current epoch onwards; older rows only matter to - // instances still holding older keys, such as disks to migrate. - function addRange() internal view returns (uint256, uint256) { - uint256 current; - if (kms.rootEpochCount() != 0) (current,) = kms.currentRootEpoch(); - return rootEpochRange(current); + function rootEpochs() internal view returns (uint256, uint256) { + return epochRange("ROOT_EPOCH", "FROM_ROOT_EPOCH", "TO_ROOT_EPOCH"); } - // Removals default to every row, as a request may select any epoch up to the - // current one. - function removeRange() internal view returns (uint256, uint256) { - return rootEpochRange(0); + function appEpochs() internal view returns (uint256, uint256) { + return epochRange("APP_EPOCH", "FROM_APP_EPOCH", "TO_APP_EPOCH"); + } + + // The legacy setters of an app without IAppEpochManagement, or none. Such + // an app has only app epoch 0, so only APP_EPOCH=0, or no epoch, applies. + function legacyApp() internal view returns (ILegacyAppManagement legacy) { + require(address(app) != address(0), "APP_CONTRACT_ADDR not set"); + if (ERC165Checker.supportsInterface(address(app), type(IAppEpochManagement).interfaceId)) { + return ILegacyAppManagement(address(0)); + } + require( + !vm.envExists("FROM_APP_EPOCH") && !vm.envExists("TO_APP_EPOCH") && vm.envOr("APP_EPOCH", uint256(0)) == 0, + "this app predates app epochs: only APP_EPOCH=0 applies" + ); + return ILegacyAppManagement(address(app)); } } @@ -59,7 +85,7 @@ contract AddKmsAggregatedMr is BaseScript { function run() external { bytes32 mrAggregated = vm.envBytes32("MR_AGGREGATED"); - (uint256 from, uint256 to) = addRange(); + (uint256 from, uint256 to) = rootEpochs(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); kms.addKmsAggregatedMrAtEpochs(from, to, mrAggregated); vm.stopBroadcast(); @@ -72,7 +98,7 @@ contract RemoveKmsAggregatedMr is BaseScript { function run() external { bytes32 mrAggregated = vm.envBytes32("MR_AGGREGATED"); - (uint256 from, uint256 to) = removeRange(); + (uint256 from, uint256 to) = rootEpochs(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); kms.removeKmsAggregatedMrAtEpochs(from, to, mrAggregated); vm.stopBroadcast(); @@ -85,7 +111,7 @@ contract AddOsImage is BaseScript { function run() external { bytes32 imageHash = vm.envBytes32("OS_IMAGE_HASH"); - (uint256 from, uint256 to) = addRange(); + (uint256 from, uint256 to) = rootEpochs(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); kms.addOsImageHashAtEpochs(from, to, imageHash); vm.stopBroadcast(); @@ -98,7 +124,7 @@ contract AddKmsDevice is BaseScript { function run() external { bytes32 deviceId = vm.envBytes32("DEVICE_ID"); - (uint256 from, uint256 to) = addRange(); + (uint256 from, uint256 to) = rootEpochs(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); kms.addKmsDeviceAtEpochs(from, to, deviceId); vm.stopBroadcast(); @@ -175,16 +201,17 @@ contract PopRootEpoch is BaseScript { // App Management Scripts contract AddComposeHash is BaseScript { function run() external { - require(address(app) != address(0), "APP_CONTRACT_ADDR not set"); bytes32 composeHash = vm.envBytes32("COMPOSE_HASH"); - vm.startBroadcast(vm.envUint("PRIVATE_KEY")); - if (vm.envExists("APP_EPOCH")) { - app.addComposeHashAtEpoch(vm.envUint("APP_EPOCH"), composeHash); + ILegacyAppManagement legacy = legacyApp(); + if (address(legacy) != address(0)) { + vm.broadcast(vm.envUint("PRIVATE_KEY")); + legacy.addComposeHash(composeHash); } else { - app.addComposeHash(composeHash); + (uint256 from, uint256 to) = appEpochs(); + vm.broadcast(vm.envUint("PRIVATE_KEY")); + app.addComposeHashAtEpochs(from, to, composeHash); } - vm.stopBroadcast(); console.log("Added compose hash:", vm.toString(composeHash)); } @@ -192,16 +219,17 @@ contract AddComposeHash is BaseScript { contract AddDevice is BaseScript { function run() external { - require(address(app) != address(0), "APP_CONTRACT_ADDR not set"); bytes32 deviceId = vm.envBytes32("DEVICE_ID"); - vm.startBroadcast(vm.envUint("PRIVATE_KEY")); - if (vm.envExists("APP_EPOCH")) { - app.addDeviceAtEpoch(vm.envUint("APP_EPOCH"), deviceId); + ILegacyAppManagement legacy = legacyApp(); + if (address(legacy) != address(0)) { + vm.broadcast(vm.envUint("PRIVATE_KEY")); + legacy.addDevice(deviceId); } else { - app.addDevice(deviceId); + (uint256 from, uint256 to) = appEpochs(); + vm.broadcast(vm.envUint("PRIVATE_KEY")); + app.addDeviceAtEpochs(from, to, deviceId); } - vm.stopBroadcast(); console.log("Added device:", vm.toString(deviceId)); } @@ -209,16 +237,17 @@ contract AddDevice is BaseScript { contract SetAllowAnyDevice is BaseScript { function run() external { - require(address(app) != address(0), "APP_CONTRACT_ADDR not set"); bool allow = vm.envBool("ALLOW_ANY_DEVICE"); - vm.startBroadcast(vm.envUint("PRIVATE_KEY")); - if (vm.envExists("APP_EPOCH")) { - app.setAllowAnyDeviceAtEpoch(vm.envUint("APP_EPOCH"), allow); + ILegacyAppManagement legacy = legacyApp(); + if (address(legacy) != address(0)) { + vm.broadcast(vm.envUint("PRIVATE_KEY")); + legacy.setAllowAnyDevice(allow); } else { - app.setAllowAnyDevice(allow); + (uint256 from, uint256 to) = appEpochs(); + vm.broadcast(vm.envUint("PRIVATE_KEY")); + app.setAllowAnyDeviceAtEpochs(from, to, allow); } - vm.stopBroadcast(); console.log("Set allowAnyDevice to:", allow); } @@ -277,7 +306,7 @@ contract BatchKmsSetup is BaseScript { bytes32[] memory osImages = vm.envOr("OS_IMAGE_LIST", ",", new bytes32[](0)); bytes32[] memory devices = vm.envOr("DEVICE_LIST", ",", new bytes32[](0)); - (uint256 from, uint256 to) = addRange(); + (uint256 from, uint256 to) = rootEpochs(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); // Set gateway ID if provided @@ -316,7 +345,7 @@ contract RemoveOsImage is BaseScript { function run() external { bytes32 imageHash = vm.envBytes32("OS_IMAGE_HASH"); - (uint256 from, uint256 to) = removeRange(); + (uint256 from, uint256 to) = rootEpochs(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); kms.removeOsImageHashAtEpochs(from, to, imageHash); vm.stopBroadcast(); @@ -329,7 +358,7 @@ contract RemoveKmsDevice is BaseScript { function run() external { bytes32 deviceId = vm.envBytes32("DEVICE_ID"); - (uint256 from, uint256 to) = removeRange(); + (uint256 from, uint256 to) = rootEpochs(); vm.startBroadcast(vm.envUint("PRIVATE_KEY")); kms.removeKmsDeviceAtEpochs(from, to, deviceId); vm.stopBroadcast(); @@ -341,16 +370,17 @@ contract RemoveKmsDevice is BaseScript { // Remove operations for App management contract RemoveComposeHash is BaseScript { function run() external { - require(address(app) != address(0), "APP_CONTRACT_ADDR not set"); bytes32 composeHash = vm.envBytes32("COMPOSE_HASH"); - vm.startBroadcast(vm.envUint("PRIVATE_KEY")); - if (vm.envExists("APP_EPOCH")) { - app.removeComposeHashAtEpoch(vm.envUint("APP_EPOCH"), composeHash); + ILegacyAppManagement legacy = legacyApp(); + if (address(legacy) != address(0)) { + vm.broadcast(vm.envUint("PRIVATE_KEY")); + legacy.removeComposeHash(composeHash); } else { - app.removeComposeHash(composeHash); + (uint256 from, uint256 to) = appEpochs(); + vm.broadcast(vm.envUint("PRIVATE_KEY")); + app.removeComposeHashAtEpochs(from, to, composeHash); } - vm.stopBroadcast(); console.log("Removed compose hash:", vm.toString(composeHash)); } @@ -358,16 +388,17 @@ contract RemoveComposeHash is BaseScript { contract RemoveDevice is BaseScript { function run() external { - require(address(app) != address(0), "APP_CONTRACT_ADDR not set"); bytes32 deviceId = vm.envBytes32("DEVICE_ID"); - vm.startBroadcast(vm.envUint("PRIVATE_KEY")); - if (vm.envExists("APP_EPOCH")) { - app.removeDeviceAtEpoch(vm.envUint("APP_EPOCH"), deviceId); + ILegacyAppManagement legacy = legacyApp(); + if (address(legacy) != address(0)) { + vm.broadcast(vm.envUint("PRIVATE_KEY")); + legacy.removeDevice(deviceId); } else { - app.removeDevice(deviceId); + (uint256 from, uint256 to) = appEpochs(); + vm.broadcast(vm.envUint("PRIVATE_KEY")); + app.removeDeviceAtEpochs(from, to, deviceId); } - vm.stopBroadcast(); console.log("Removed device:", vm.toString(deviceId)); } diff --git a/dstack/kms/auth-eth/script/README.md b/dstack/kms/auth-eth/script/README.md index 99d3f1778..1f50fae69 100644 --- a/dstack/kms/auth-eth/script/README.md +++ b/dstack/kms/auth-eth/script/README.md @@ -29,25 +29,25 @@ Type-safe contract management scripts. Set the environment variables and run the ### KMS Management The KMS material scripts below write root epoch rows: `ROOT_EPOCH=e` one row, or -`FROM_ROOT_EPOCH` through `TO_ROOT_EPOCH`. By default additions write the current -through the latest registered epoch, and removals every row, so a removal -revokes the value at every epoch a request can select. +`FROM_ROOT_EPOCH` through `TO_ROOT_EPOCH`. One of them is required. A request +may select any epoch up to the current one, so revoke a value with +`FROM_ROOT_EPOCH=0` and `TO_ROOT_EPOCH` the latest registered epoch. #### Add KMS Aggregated MR ```bash -KMS_CONTRACT_ADDR=0x... MR_AGGREGATED=0x1234... \ +KMS_CONTRACT_ADDR=0x... ROOT_EPOCH=0 MR_AGGREGATED=0x1234... \ forge script script/Manage.s.sol:AddKmsAggregatedMr --broadcast --rpc-url $RPC_URL ``` #### Add OS Image ```bash -KMS_CONTRACT_ADDR=0x... OS_IMAGE_HASH=0x1234... \ +KMS_CONTRACT_ADDR=0x... ROOT_EPOCH=0 OS_IMAGE_HASH=0x1234... \ forge script script/Manage.s.sol:AddOsImage --broadcast --rpc-url $RPC_URL ``` #### Add KMS Device ```bash -KMS_CONTRACT_ADDR=0x... DEVICE_ID=0x1234... \ +KMS_CONTRACT_ADDR=0x... ROOT_EPOCH=0 DEVICE_ID=0x1234... \ forge script script/Manage.s.sol:AddKmsDevice --broadcast --rpc-url $RPC_URL ``` @@ -105,19 +105,19 @@ forge script script/Manage.s.sol:PopRootEpoch --broadcast --rpc-url $RPC_URL #### Remove KMS Aggregated MR ```bash -KMS_CONTRACT_ADDR=0x... MR_AGGREGATED=0x1234... \ +KMS_CONTRACT_ADDR=0x... ROOT_EPOCH=0 MR_AGGREGATED=0x1234... \ forge script script/Manage.s.sol:RemoveKmsAggregatedMr --broadcast --rpc-url $RPC_URL ``` #### Remove OS Image ```bash -KMS_CONTRACT_ADDR=0x... OS_IMAGE_HASH=0x1234... \ +KMS_CONTRACT_ADDR=0x... ROOT_EPOCH=0 OS_IMAGE_HASH=0x1234... \ forge script script/Manage.s.sol:RemoveOsImage --broadcast --rpc-url $RPC_URL ``` #### Remove KMS Device ```bash -KMS_CONTRACT_ADDR=0x... DEVICE_ID=0x1234... \ +KMS_CONTRACT_ADDR=0x... ROOT_EPOCH=0 DEVICE_ID=0x1234... \ forge script script/Manage.s.sol:RemoveKmsDevice --broadcast --rpc-url $RPC_URL ``` @@ -136,7 +136,7 @@ forge script script/Manage.s.sol:RegisterApp --broadcast --rpc-url $RPC_URL #### Batch KMS Setup Configure multiple settings at once: ```bash -KMS_CONTRACT_ADDR=0x... \ +KMS_CONTRACT_ADDR=0x... ROOT_EPOCH=0 \ GATEWAY_APP_ID="my-gateway" \ MR_AGGREGATED_LIST=0x1111...,0x2222...,0x3333... \ OS_IMAGE_LIST=0xaaaa...,0xbbbb... \ @@ -146,33 +146,39 @@ forge script script/Manage.s.sol:BatchKmsSetup --broadcast --rpc-url $RPC_URL ### App Management +The app scripts write app epoch rows: `APP_EPOCH=e`, or `FROM_APP_EPOCH` through +`TO_APP_EPOCH` (at most the next epoch); one of them is required. Revoke a value +with `FROM_APP_EPOCH=0` and `TO_APP_EPOCH` the current app epoch. App contracts +deployed before app epochs (no `IAppEpochManagement`) have only app epoch 0: the +scripts call their epoch-less setters and accept only `APP_EPOCH=0` or no epoch. + #### Add Compose Hash ```bash -KMS_CONTRACT_ADDR=0x... APP_CONTRACT_ADDR=0x... COMPOSE_HASH=0x1234... \ +KMS_CONTRACT_ADDR=0x... APP_CONTRACT_ADDR=0x... APP_EPOCH=0 COMPOSE_HASH=0x1234... \ forge script script/Manage.s.sol:AddComposeHash --broadcast --rpc-url $RPC_URL ``` #### Add Device ```bash -KMS_CONTRACT_ADDR=0x... APP_CONTRACT_ADDR=0x... DEVICE_ID=0x1234... \ +KMS_CONTRACT_ADDR=0x... APP_CONTRACT_ADDR=0x... APP_EPOCH=0 DEVICE_ID=0x1234... \ forge script script/Manage.s.sol:AddDevice --broadcast --rpc-url $RPC_URL ``` #### Set Allow Any Device ```bash -KMS_CONTRACT_ADDR=0x... APP_CONTRACT_ADDR=0x... ALLOW_ANY_DEVICE=true \ +KMS_CONTRACT_ADDR=0x... APP_CONTRACT_ADDR=0x... APP_EPOCH=0 ALLOW_ANY_DEVICE=true \ forge script script/Manage.s.sol:SetAllowAnyDevice --broadcast --rpc-url $RPC_URL ``` #### Remove Compose Hash ```bash -KMS_CONTRACT_ADDR=0x... APP_CONTRACT_ADDR=0x... COMPOSE_HASH=0x1234... \ +KMS_CONTRACT_ADDR=0x... APP_CONTRACT_ADDR=0x... APP_EPOCH=0 COMPOSE_HASH=0x1234... \ forge script script/Manage.s.sol:RemoveComposeHash --broadcast --rpc-url $RPC_URL ``` #### Remove Device ```bash -KMS_CONTRACT_ADDR=0x... APP_CONTRACT_ADDR=0x... DEVICE_ID=0x1234... \ +KMS_CONTRACT_ADDR=0x... APP_CONTRACT_ADDR=0x... APP_EPOCH=0 DEVICE_ID=0x1234... \ forge script script/Manage.s.sol:RemoveDevice --broadcast --rpc-url $RPC_URL ``` diff --git a/dstack/kms/auth-eth/test/DstackApp.symbolic.t.sol b/dstack/kms/auth-eth/test/DstackApp.symbolic.t.sol index 2af3492f8..c1ee862d2 100644 --- a/dstack/kms/auth-eth/test/DstackApp.symbolic.t.sol +++ b/dstack/kms/auth-eth/test/DstackApp.symbolic.t.sol @@ -111,18 +111,31 @@ contract DstackAppSymbolicTest is Test { assert(_upgradesDisabledFlag(address(app))); // base: flag is set bytes memory data; - if (which == 0) data = abi.encodeWithSelector(DstackApp.addComposeHash.selector, word); - else if (which == 1) data = abi.encodeWithSelector(DstackApp.removeComposeHash.selector, word); - else if (which == 2) data = abi.encodeWithSelector(DstackApp.addDevice.selector, word); - else if (which == 3) data = abi.encodeWithSelector(DstackApp.removeDevice.selector, word); - else if (which == 4) data = abi.encodeWithSelector(DstackApp.setAllowAnyDevice.selector, flag); - else if (which == 5) data = abi.encodeWithSelector(DstackApp.setRequireTcbUpToDate.selector, flag); - else if (which == 6) data = abi.encodeWithSelector(DstackApp.disableUpgrades.selector); - else if (which == 7) data = abi.encodeWithSelector(UUPSUpgradeable.upgradeToAndCall.selector, addr, ""); - else if (which == 8) data = abi.encodeWithSelector(Ownable2StepUpgradeable.transferOwnership.selector, addr); - else if (which == 9) data = abi.encodeWithSelector(Ownable2StepUpgradeable.acceptOwnership.selector); - else if (which == 10) data = abi.encodeWithSelector(OwnableUpgradeable.renounceOwnership.selector); - else return; // outside the enumerated mutating surface + if (which == 0) { + data = abi.encodeWithSelector(DstackApp.addComposeHashAtEpoch.selector, uint256(0), word); + } else if (which == 1) { + data = abi.encodeWithSelector(DstackApp.removeComposeHashAtEpoch.selector, uint256(0), word); + } else if (which == 2) { + data = abi.encodeWithSelector(DstackApp.addDeviceAtEpoch.selector, uint256(0), word); + } else if (which == 3) { + data = abi.encodeWithSelector(DstackApp.removeDeviceAtEpoch.selector, uint256(0), word); + } else if (which == 4) { + data = abi.encodeWithSelector(DstackApp.setAllowAnyDeviceAtEpoch.selector, uint256(0), flag); + } else if (which == 5) { + data = abi.encodeWithSelector(DstackApp.setRequireTcbUpToDate.selector, flag); + } else if (which == 6) { + data = abi.encodeWithSelector(DstackApp.disableUpgrades.selector); + } else if (which == 7) { + data = abi.encodeWithSelector(UUPSUpgradeable.upgradeToAndCall.selector, addr, ""); + } else if (which == 8) { + data = abi.encodeWithSelector(Ownable2StepUpgradeable.transferOwnership.selector, addr); + } else if (which == 9) { + data = abi.encodeWithSelector(Ownable2StepUpgradeable.acceptOwnership.selector); + } else if (which == 10) { + data = abi.encodeWithSelector(OwnableUpgradeable.renounceOwnership.selector); + } else { + return; // outside the enumerated mutating surface + } vm.prank(caller); (bool ok,) = address(app).call(data); diff --git a/dstack/kms/auth-eth/test/DstackApp.t.sol b/dstack/kms/auth-eth/test/DstackApp.t.sol index 7fb840748..a94a3d38f 100644 --- a/dstack/kms/auth-eth/test/DstackApp.t.sol +++ b/dstack/kms/auth-eth/test/DstackApp.t.sol @@ -76,7 +76,7 @@ contract DstackAppTest is Test { vm.expectEmit(true, true, true, true); emit ComposeHashAdded(hash); - app.addComposeHash(hash); + app.addComposeHashAtEpoch(0, hash); assertTrue(app.allowedComposeHashes(hash)); } @@ -84,13 +84,13 @@ contract DstackAppTest is Test { bytes32 hash = bytes32(uint256(123)); vm.startPrank(owner); - app.addComposeHash(hash); + app.addComposeHashAtEpoch(0, hash); assertTrue(app.allowedComposeHashes(hash)); vm.expectEmit(true, true, true, true); emit ComposeHashRemoved(hash); - app.removeComposeHash(hash); + app.removeComposeHashAtEpoch(0, hash); assertFalse(app.allowedComposeHashes(hash)); vm.stopPrank(); } @@ -102,7 +102,7 @@ contract DstackAppTest is Test { vm.expectEmit(true, true, true, true); emit DeviceAdded(deviceId); - app.addDevice(deviceId); + app.addDeviceAtEpoch(0, deviceId); assertTrue(app.allowedDeviceIds(deviceId)); } @@ -110,13 +110,13 @@ contract DstackAppTest is Test { bytes32 deviceId = bytes32(uint256(456)); vm.startPrank(owner); - app.addDevice(deviceId); + app.addDeviceAtEpoch(0, deviceId); assertTrue(app.allowedDeviceIds(deviceId)); vm.expectEmit(true, true, true, true); emit DeviceRemoved(deviceId); - app.removeDevice(deviceId); + app.removeDeviceAtEpoch(0, deviceId); assertFalse(app.allowedDeviceIds(deviceId)); vm.stopPrank(); } @@ -126,14 +126,14 @@ contract DstackAppTest is Test { vm.expectEmit(true, true, true, true); emit AllowAnyDeviceSet(true); - app.setAllowAnyDevice(true); + app.setAllowAnyDeviceAtEpoch(0, true); assertTrue(app.allowAnyDevice()); vm.prank(owner); vm.expectEmit(true, true, true, true); emit AllowAnyDeviceSet(false); - app.setAllowAnyDevice(false); + app.setAllowAnyDeviceAtEpoch(0, false); assertFalse(app.allowAnyDevice()); } @@ -144,19 +144,19 @@ contract DstackAppTest is Test { vm.startPrank(user); vm.expectRevert(); - app.addComposeHash(hash); + app.addComposeHashAtEpoch(0, hash); vm.expectRevert(); - app.removeComposeHash(hash); + app.removeComposeHashAtEpoch(0, hash); vm.expectRevert(); - app.addDevice(deviceId); + app.addDeviceAtEpoch(0, deviceId); vm.expectRevert(); - app.removeDevice(deviceId); + app.removeDeviceAtEpoch(0, deviceId); vm.expectRevert(); - app.setAllowAnyDevice(true); + app.setAllowAnyDeviceAtEpoch(0, true); vm.stopPrank(); } @@ -166,8 +166,8 @@ contract DstackAppTest is Test { bytes32 composeHash = bytes32(uint256(456)); vm.startPrank(owner); - app.addDevice(deviceId); - app.addComposeHash(composeHash); + app.addDeviceAtEpoch(0, deviceId); + app.addComposeHashAtEpoch(0, composeHash); vm.stopPrank(); IAppAuth.AppInfo memory bootInfo = IAppAuth.AppInfo({ @@ -195,7 +195,7 @@ contract DstackAppTest is Test { // Test with allowAnyDevice = true vm.prank(owner); - app.setAllowAnyDevice(true); + app.setAllowAnyDeviceAtEpoch(0, true); (allowed, reason) = app.isAppAllowed(bootInfo); assertTrue(allowed); @@ -208,8 +208,8 @@ contract DstackAppTest is Test { bytes32 unallowedHash = bytes32(uint256(789)); vm.startPrank(owner); - app.addDevice(deviceId); - app.addComposeHash(allowedHash); + app.addDeviceAtEpoch(0, deviceId); + app.addComposeHashAtEpoch(0, allowedHash); vm.stopPrank(); IAppAuth.AppInfo memory bootInfo = IAppAuth.AppInfo({ @@ -263,8 +263,8 @@ contract DstackAppTest is Test { bytes32 composeHash = bytes32(uint256(456)); vm.startPrank(owner); - app.addDevice(deviceId); - app.addComposeHash(composeHash); + app.addDeviceAtEpoch(0, deviceId); + app.addComposeHashAtEpoch(0, composeHash); vm.stopPrank(); string[] memory advisoryIds = new string[](1); @@ -352,8 +352,8 @@ contract DstackAppTest is Test { // Test IAppAuth interface assertTrue(app.supportsInterface(0x1e079198)); - // Test IAppAuthBasicManagement interface - assertTrue(app.supportsInterface(0x8fd37527)); + // IAppAuthBasicManagement, whose epoch-less setters are gone + assertFalse(app.supportsInterface(0x8fd37527)); // Test IERC165 interface assertTrue(app.supportsInterface(0x01ffc9a7)); @@ -394,19 +394,19 @@ contract DstackAppTest is Test { // owner-only authority follows the new owner vm.prank(owner); vm.expectRevert(abi.encodeWithSignature("OwnableUnauthorizedAccount(address)", owner)); - app.addComposeHash(bytes32(uint256(1))); + app.addComposeHashAtEpoch(0, bytes32(uint256(1))); vm.prank(newOwner); - app.addComposeHash(bytes32(uint256(1))); + app.addComposeHashAtEpoch(0, bytes32(uint256(1))); assertTrue(app.allowedComposeHashes(bytes32(uint256(1)))); } function test_AddZeroValuedPolicyEntriesIsRejected() public { vm.startPrank(owner); vm.expectRevert("invalid compose hash"); - app.addComposeHash(bytes32(0)); + app.addComposeHashAtEpoch(0, bytes32(0)); vm.expectRevert("invalid device ID"); - app.addDevice(bytes32(0)); + app.addDeviceAtEpoch(0, bytes32(0)); vm.stopPrank(); } @@ -480,8 +480,8 @@ contract DstackAppTest is Test { function test_AppOperationsSharePolicyButRejectUnknownOrKmsOperations() public { IAppAuth.AppInfo memory info = epochInfo(); vm.startPrank(owner); - app.addComposeHash(info.composeHash); - app.addDevice(info.deviceId); + app.addComposeHashAtEpoch(0, info.composeHash); + app.addDeviceAtEpoch(0, info.deviceId); vm.stopPrank(); (bool allowed,) = authorizeAt(info, 0, keccak256("app_get_cert")); assertTrue(allowed); @@ -557,14 +557,14 @@ contract DstackAppTest is Test { app.addComposeHashAtEpoch(0, info.composeHash); app.setAllowAnyDeviceAtEpoch(0, true); app.advanceAppEpoch(0); - app.addComposeHash(info.composeHash); + app.addComposeHashAtEpoch(1, info.composeHash); bool allowed = authorizeCurrent(info); assertFalse(allowed); - app.addDevice(info.deviceId); + app.addDeviceAtEpoch(1, info.deviceId); allowed = authorizeCurrent(info); assertTrue(allowed, "explicit device can replace wildcard in a newer row"); - app.removeDevice(info.deviceId); - app.setAllowAnyDevice(true); + app.removeDeviceAtEpoch(1, info.deviceId); + app.setAllowAnyDeviceAtEpoch(1, true); app.setAllowAnyDeviceAtEpoch(0, false); assertFalse(app.allowAnyDevice()); assertTrue(app.allowAnyDeviceAtEpoch(1)); @@ -572,7 +572,7 @@ contract DstackAppTest is Test { assertFalse(allowed); allowed = authorizeCurrent(info); assertTrue(allowed); - app.setAllowAnyDevice(false); + app.setAllowAnyDeviceAtEpoch(1, false); allowed = authorizeCurrent(info); assertFalse(allowed); vm.stopPrank(); @@ -585,8 +585,8 @@ contract DstackAppTest is Test { function test_LegacyAppCheckRefusesOnceTheAppEpochAdvances() public { IAppAuth.AppInfo memory info = epochInfo(); vm.startPrank(owner); - app.addComposeHash(info.composeHash); - app.addDevice(info.deviceId); + app.addComposeHashAtEpochs(0, 1, info.composeHash); + app.addDeviceAtEpochs(0, 1, info.deviceId); // At epoch 0 the legacy check reads row 0, as in v0.6.0. (bool allowed, string memory reason) = app.isAppAllowed(info); assertTrue(allowed, reason); @@ -638,13 +638,13 @@ contract DstackAppTest is Test { vm.stopPrank(); } - function test_EpochlessAppSettersChangeEveryRow() public { + function test_RangeAppSettersChangeEveryRowInRange() public { IAppAuth.AppInfo memory info = epochInfo(); vm.startPrank(owner); app.advanceAppEpoch(0); - app.addComposeHash(info.composeHash); - app.addDevice(info.deviceId); - app.setAllowAnyDevice(true); + app.addComposeHashAtEpochs(0, 2, info.composeHash); + app.addDeviceAtEpochs(0, 2, info.deviceId); + app.setAllowAnyDeviceAtEpochs(0, 2, true); for (uint256 epoch = 0; epoch <= 2; ++epoch) { assertTrue(app.allowedComposeHashesAtEpoch(epoch, info.composeHash)); assertTrue(app.allowedDeviceIdsAtEpoch(epoch, info.deviceId)); @@ -653,10 +653,10 @@ contract DstackAppTest is Test { (bool allowed,) = authorizeAt(info, 0, keccak256("app_get_key")); assertTrue(allowed); - // A removal is a full revocation: no selectable epoch still allows it. - app.removeComposeHash(info.composeHash); - app.removeDevice(info.deviceId); - app.setAllowAnyDevice(false); + // Removing from every row revokes at every selectable epoch. + app.removeComposeHashAtEpochs(0, 2, info.composeHash); + app.removeDeviceAtEpochs(0, 2, info.deviceId); + app.setAllowAnyDeviceAtEpochs(0, 2, false); for (uint256 epoch = 0; epoch <= 2; ++epoch) { assertFalse(app.allowedComposeHashesAtEpoch(epoch, info.composeHash)); assertFalse(app.allowedDeviceIdsAtEpoch(epoch, info.deviceId)); @@ -665,6 +665,13 @@ contract DstackAppTest is Test { (allowed,) = authorizeAt(info, 0, keccak256("app_get_key")); assertFalse(allowed); assertFalse(authorizeCurrent(info)); + + vm.expectRevert("Invalid app epoch range"); + app.removeComposeHashAtEpochs(1, 0, info.composeHash); + vm.expectRevert("Unknown app epoch"); + app.removeComposeHashAtEpochs(0, 3, info.composeHash); + vm.expectRevert("invalid compose hash"); + app.addComposeHashAtEpochs(0, 1, bytes32(0)); vm.stopPrank(); } diff --git a/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol b/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol index 9969bc7c6..76ba5b148 100644 --- a/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol +++ b/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol @@ -34,6 +34,7 @@ function _appAuth( /// AppInfo, so those are the symbolic inputs; the remaining fields stay zero. /// /// Run with `halmos --contract DstackAppEpochSymbolicTest`. +/// @custom:halmos --loop 5 contract DstackAppEpochSymbolicTest is Test { DstackApp internal app; address internal constant OWNER = address(0xA11CE); @@ -61,15 +62,21 @@ contract DstackAppEpochSymbolicTest is Test { } /// Calldata of one policy setter: 0-4 write the row of `epoch`, 5-9 are - /// their epoch-less forms, which write every row. - function _policyCall( + /// their range forms, writing every row from `epoch` through the next one. + function _policyCall(uint256 which, uint256 epoch, bytes32 word, bool flag) internal view returns (bytes memory) { + return _rangePolicyCall(which, epoch, app.currentAppEpoch() + 1, word, flag); + } + + /// As _policyCall, with the range setters writing `epoch` through `next`. + function _rangePolicyCall( uint256 which, uint256 epoch, + uint256 next, bytes32 word, bool flag ) internal - view + pure returns (bytes memory data) { if (which == 0) data = abi.encodeCall(DstackApp.addComposeHashAtEpoch, (epoch, word)); @@ -77,11 +84,11 @@ contract DstackAppEpochSymbolicTest is Test { else if (which == 2) data = abi.encodeCall(DstackApp.addDeviceAtEpoch, (epoch, word)); else if (which == 3) data = abi.encodeCall(DstackApp.removeDeviceAtEpoch, (epoch, word)); else if (which == 4) data = abi.encodeCall(DstackApp.setAllowAnyDeviceAtEpoch, (epoch, flag)); - else if (which == 5) data = abi.encodeCall(DstackApp.addComposeHash, (word)); - else if (which == 6) data = abi.encodeCall(DstackApp.removeComposeHash, (word)); - else if (which == 7) data = abi.encodeCall(DstackApp.addDevice, (word)); - else if (which == 8) data = abi.encodeCall(DstackApp.removeDevice, (word)); - else data = abi.encodeCall(DstackApp.setAllowAnyDevice, (flag)); + else if (which == 5) data = abi.encodeCall(DstackApp.addComposeHashAtEpochs, (epoch, next, word)); + else if (which == 6) data = abi.encodeCall(DstackApp.removeComposeHashAtEpochs, (epoch, next, word)); + else if (which == 7) data = abi.encodeCall(DstackApp.addDeviceAtEpochs, (epoch, next, word)); + else if (which == 8) data = abi.encodeCall(DstackApp.removeDeviceAtEpochs, (epoch, next, word)); + else data = abi.encodeCall(DstackApp.setAllowAnyDeviceAtEpochs, (epoch, next, flag)); } function _ownerPolicyWrite(uint256 which, uint256 epoch, bytes32 word, bool flag) internal { @@ -158,13 +165,14 @@ contract DstackAppEpochSymbolicTest is Test { address caller, uint256 which, uint256 epoch, + uint256 to, bytes32 word, bool flag ) external { - vm.assume(which < 5 && epoch > app.currentAppEpoch() + 1); - bytes memory data = _policyCall(which, epoch, word, flag); + vm.assume(which < POLICY_SETTERS && (which < 5 ? epoch : to) > app.currentAppEpoch() + 1); + bytes memory data = _rangePolicyCall(which, epoch, to, word, flag); vm.prank(caller); (bool ok,) = address(app).call(data); assert(!ok); @@ -233,8 +241,8 @@ contract DstackAppEpochSymbolicTest is Test { ); app = DstackApp(address(new ERC1967Proxy(address(new DstackApp()), initData))); vm.startPrank(OWNER); - app.addComposeHash(bytes32(uint256(0xc0))); - app.addDevice(bytes32(uint256(0xd0))); + app.addComposeHashAtEpoch(0, bytes32(uint256(0xc0))); + app.addDeviceAtEpoch(0, bytes32(uint256(0xd0))); // A prepared next row must not leak into the legacy decision. app.addComposeHashAtEpoch(1, bytes32(uint256(0xc1))); app.setAllowAnyDeviceAtEpoch(1, true); @@ -247,13 +255,14 @@ contract DstackAppEpochSymbolicTest is Test { } // --------------------------------------------------------------- - // Epoch isolation: an *AtEpoch write never changes an authorization - // decision at another epoch, including epoch 0's legacy fields. + // Epoch isolation: a policy write never changes an authorization decision + // at an epoch it did not write, including epoch 0's legacy fields. // --------------------------------------------------------------- function check_PolicyWrite_PreservesOtherEpochDecisions( uint256 which, uint256 writeEpoch, + uint256 writeTo, bytes32 word, bool flag, bytes32 composeHash, @@ -263,8 +272,9 @@ contract DstackAppEpochSymbolicTest is Test { ) external { - vm.assume(which < 5 && writeEpoch != epoch); - bytes memory data = _policyCall(which, writeEpoch, word, flag); + vm.assume(which < POLICY_SETTERS); + vm.assume(which < 5 ? writeEpoch != epoch : epoch < writeEpoch || epoch > writeTo); + bytes memory data = _rangePolicyCall(which, writeEpoch, writeTo, word, flag); IAppAuth.AppInfo memory info = _info(composeHash, deviceId, upToDate); (bool allowedBefore, bytes32 reasonBefore) = _decision(info, epoch); @@ -302,12 +312,21 @@ contract DstackAppEpochSymbolicTest is Test { assert(allowed == expected); } - // A write takes effect on its row; an epoch-less one on every row up to - // the next epoch, so an epoch-less removal leaves no selectable epoch - // allowing the value. - function check_PolicyWrite_TakesEffect(uint256 which, uint256 epoch, bytes32 word, bool flag) external { + // A write takes effect on its row, and a range write on every row in its + // range. + function check_PolicyWrite_TakesEffect( + uint256 which, + uint256 from, + uint256 to, + uint256 epoch, + bytes32 word, + bool flag + ) + external + { vm.assume(which < POLICY_SETTERS && epoch <= app.currentAppEpoch() + 1); - bytes memory data = _policyCall(which, epoch, word, flag); + vm.assume(which < 5 ? epoch == from : from <= epoch && epoch <= to && to <= app.currentAppEpoch() + 1); + bytes memory data = _rangePolicyCall(which, from, to, word, flag); vm.prank(OWNER); (bool ok,) = address(app).call(data); uint256 kind = which % 5; @@ -321,4 +340,34 @@ contract DstackAppEpochSymbolicTest is Test { if (kind == 3) assert(!app.allowedDeviceIdsAtEpoch(epoch, word)); if (kind == 4) assert(app.allowAnyDeviceAtEpoch(epoch) == flag); } + + // --------------------------------------------------------------- + // Revocation: removing a compose hash, or a device and the any-device + // flag, from every row, 0 through the next epoch, leaves no selectable + // app epoch at which it is authorized. + // --------------------------------------------------------------- + + function check_FullRangeRemoval_RevokesAtEveryAppEpoch( + bool byDevice, + bytes32 composeHash, + bytes32 deviceId, + bool upToDate, + uint256 epoch, + bool getCert + ) + external + { + uint256 next = app.currentAppEpoch() + 1; + vm.startPrank(OWNER); + if (byDevice) { + app.removeDeviceAtEpochs(0, next, deviceId); + app.setAllowAnyDeviceAtEpochs(0, next, false); + } else { + app.removeComposeHashAtEpochs(0, next, composeHash); + } + vm.stopPrank(); + bytes32 op = getCert ? AuthorizationOperation.APP_GET_CERT : AuthorizationOperation.APP_GET_KEY; + (bool allowed,) = _appAuth(app, _info(composeHash, deviceId, upToDate), epoch, op); + assert(!allowed); + } } diff --git a/dstack/kms/auth-eth/test/DstackKms.t.sol b/dstack/kms/auth-eth/test/DstackKms.t.sol index ed886357e..0281634a8 100644 --- a/dstack/kms/auth-eth/test/DstackKms.t.sol +++ b/dstack/kms/auth-eth/test/DstackKms.t.sol @@ -1022,8 +1022,8 @@ contract DstackKmsTest is Test { assertFalse(kms.authorizeApp(epochRequest(info, 1, 0)).authorized); assertTrue(kms.authorizeApp(epochRequest(info, 0, 0)).authorized); vm.startPrank(user); - policy.addComposeHash(info.composeHash); - policy.addDevice(info.deviceId); + policy.addComposeHashAtEpoch(1, info.composeHash); + policy.addDeviceAtEpoch(1, info.deviceId); vm.stopPrank(); assertTrue(kms.authorizeApp(epochRequest(info, 0, 1)).authorized); assertFalse(authorize(info, false).authorized); diff --git a/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol b/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol index a035c5ff9..c7f2d6be2 100644 --- a/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol +++ b/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol @@ -27,6 +27,23 @@ function _appAuth( return (result.authorized, result.rejectionReason); } +/// An app policy predating app epochs: IAppAuth only, one allowed compose hash. +contract LegacyAppPolicy is IAppAuth { + bytes32 internal immutable allowed; + + constructor(bytes32 composeHash) { + allowed = composeHash; + } + + function supportsInterface(bytes4 interfaceId) external pure returns (bool) { + return interfaceId == 0x01ffc9a7 || interfaceId == 0x1e079198; + } + + function isAppAllowed(AppInfo calldata info) external view returns (bool, string memory) { + return info.composeHash == allowed ? (true, "") : (false, "Compose hash not allowed"); + } +} + /// @notice Halmos symbolic tests for DstackKms root epochs and epoch-scoped /// authorization. /// @@ -70,8 +87,8 @@ contract DstackKmsEpochSymbolicTest is Test { app = DstackApp(kms.deployAndRegisterApp(OWNER, false, false, true, bytes32(0), _id(0xc0, 0))); app.advanceAppEpoch(0); - app.addComposeHash(_id(0xc0, 1)); - app.setAllowAnyDevice(true); + app.addComposeHashAtEpoch(1, _id(0xc0, 1)); + app.setAllowAnyDeviceAtEpoch(1, true); vm.stopPrank(); } @@ -121,10 +138,19 @@ contract DstackKmsEpochSymbolicTest is Test { return AuthorizationRequest(info, operation, OptionalUint256(appSet, appEpoch), OptionalUint256(rootSet, root)); } - /// Calldata of one KMS policy setter writing the row of `epoch` and, for the - /// range setters, every later registered row. - function _kmsPolicyCall(uint256 which, uint256 epoch, bytes32 word) internal view returns (bytes memory data) { - uint256 latest = kms.rootEpochCount() - 1; + /// Calldata of one KMS policy setter: 0-5 write the row of `epoch`, 6-11 + /// every row from `epoch` through `to`. + function _kmsPolicyCall( + uint256 which, + uint256 epoch, + uint256 to, + bytes32 word + ) + internal + pure + returns (bytes memory data) + { + uint256 latest = to; if (which == 0) data = abi.encodeCall(DstackKms.addKmsAggregatedMrAtEpoch, (epoch, word)); else if (which == 1) data = abi.encodeCall(DstackKms.removeKmsAggregatedMrAtEpoch, (epoch, word)); else if (which == 2) data = abi.encodeCall(DstackKms.addKmsDeviceAtEpoch, (epoch, word)); @@ -167,7 +193,7 @@ contract DstackKmsEpochSymbolicTest is Test { returns (bytes memory data) { if (which < KMS_POLICY_SETTERS) { - data = _kmsPolicyCall(which, epoch, word); + data = _kmsPolicyCall(which, epoch, kms.rootEpochCount() - 1, word); } else if (which == 12) { data = abi.encodeCall(DstackKms.registerRootEpoch, (_ca(word, hash), _k256(flag, word), "url")); } else if (which == 13) { @@ -363,9 +389,11 @@ contract DstackKmsEpochSymbolicTest is Test { // (requested, or current when unset), after any one owner write. // --------------------------------------------------------------- + /// @custom:halmos --loop 4 function check_AuthorizeKms_MatchesEpochPolicy( uint256 which, uint256 writeEpoch, + uint256 writeTo, bytes32 word, bytes32 operation, bool rootSet, @@ -375,7 +403,7 @@ contract DstackKmsEpochSymbolicTest is Test { external { if (which < KMS_POLICY_SETTERS) { - bytes memory data = _kmsPolicyCall(which, writeEpoch, word); + bytes memory data = _kmsPolicyCall(which, writeEpoch, writeTo, word); vm.prank(OWNER); (bool ok,) = address(kms).call(data); ok; @@ -422,13 +450,15 @@ contract DstackKmsEpochSymbolicTest is Test { } // --------------------------------------------------------------- - // Epoch isolation: a KMS policy write at one root epoch never - // changes a KMS authorization decision at another. + // Epoch isolation: a KMS policy write never changes a KMS + // authorization decision at a root epoch outside the rows it names. // --------------------------------------------------------------- + /// @custom:halmos --loop 4 function check_KmsPolicyWrite_PreservesOtherEpochDecisions( uint256 which, uint256 writeEpoch, + uint256 writeTo, bytes32 word, bytes32 operation, uint256 root, @@ -437,8 +467,8 @@ contract DstackKmsEpochSymbolicTest is Test { external { vm.assume(which < KMS_POLICY_SETTERS); - bytes memory data = _kmsPolicyCall(which, writeEpoch, word); - vm.assume(which < 6 ? writeEpoch != root : root < writeEpoch); + bytes memory data = _kmsPolicyCall(which, writeEpoch, writeTo, word); + vm.assume(which < 6 ? writeEpoch != root : root < writeEpoch || root > writeTo); AuthorizationRequest memory request = _request(_info(id), operation, true, root, false, 0); AuthorizationResult memory before = kms.authorizeKms(request); @@ -451,6 +481,123 @@ contract DstackKmsEpochSymbolicTest is Test { assert(keccak256(bytes(before.rejectionReason)) == keccak256(bytes(afterWrite.rejectionReason))); } + // --------------------------------------------------------------- + // Revocation: removing a material from every registered row, 0 + // through the latest, leaves no selectable root epoch at which an + // identity carrying it is authorized, for any KMS or app operation. + // --------------------------------------------------------------- + + /// @custom:halmos --loop 4 + function check_FullRangeRemoval_RevokesAtEveryEpoch( + uint256 kind, + bytes32 operation, + bool rootSet, + uint256 root, + bool appSet, + uint256 appEpoch, + Identity calldata id + ) + external + { + vm.assume(kind < 3); + uint256 latest = kms.rootEpochCount() - 1; + vm.startPrank(OWNER); + if (kind == 0) kms.removeOsImageHashAtEpochs(0, latest, id.osImageHash); + else if (kind == 1) kms.removeKmsAggregatedMrAtEpochs(0, latest, id.mrAggregated); + else kms.removeKmsDeviceAtEpochs(0, latest, id.deviceId); + vm.stopPrank(); + + AuthorizationRequest memory request = _request(_info(id), operation, rootSet, root, appSet, appEpoch); + assert(!kms.authorizeKms(request).authorized); + if (kind == 0) assert(!kms.authorizeApp(request).authorized); + } + + // --------------------------------------------------------------- + // Policy rows belong to a registration: popping the pending epoch + // and registering the number again, with the same keys or others, + // starts with empty rows. + // --------------------------------------------------------------- + + function check_PopThenRegister_StartsWithEmptyRows( + bool sameKeys, + bytes32 x, + bytes32 y, + bool even, + bytes32 word + ) + external + { + (bytes memory ca, bytes memory k256,, bytes32 hash) = kms.rootEpoch(2); + if (!sameKeys) { + ca = _ca(x, y); + k256 = _k256(even, x); + } + vm.startPrank(OWNER); + kms.popRootEpoch(2, hash); + kms.registerRootEpoch(ca, k256, "evidence"); + vm.stopPrank(); + + assert(!kms.allowedOsImagesAtEpoch(2, word)); + assert(!kms.kmsAllowedAggregatedMrsAtEpoch(2, word)); + assert(!kms.kmsAllowedDeviceIdsAtEpoch(2, word)); + } + + // --------------------------------------------------------------- + // Root epoch 0's rows are the v0.5.x mappings: after any one owner + // write, the legacy getters and the epoch 0 getters agree. + // --------------------------------------------------------------- + + /// @custom:halmos --loop 4 + function check_EpochZeroRows_AreTheLegacyMappings( + uint256 which, + uint256 writeEpoch, + uint256 writeTo, + bytes32 word, + bytes32 probe + ) + external + { + vm.assume(which < KMS_POLICY_SETTERS); + vm.prank(OWNER); + (bool ok,) = address(kms).call(_kmsPolicyCall(which, writeEpoch, writeTo, word)); + ok; + assert(kms.allowedOsImages(probe) == kms.allowedOsImagesAtEpoch(0, probe)); + assert(kms.kmsAllowedAggregatedMrs(probe) == kms.kmsAllowedAggregatedMrsAtEpoch(0, probe)); + assert(kms.kmsAllowedDeviceIds(probe) == kms.kmsAllowedDeviceIdsAtEpoch(0, probe)); + } + + // --------------------------------------------------------------- + // An app policy predating app epochs (no IAppEpochAuth) is decided by + // its isAppAllowed and only for app epoch 0: an explicit nonzero app + // epoch is refused, and otherwise the decision is the KMS checks at + // the selected root epoch and the policy's own answer. + // --------------------------------------------------------------- + + function check_AuthorizeApp_LegacyPolicyOnlyAtAppEpochZero( + bool getCert, + bool rootSet, + uint256 root, + bool appSet, + uint256 appEpoch, + Identity calldata id + ) + external + { + LegacyAppPolicy legacy = new LegacyAppPolicy(_id(0xc0, 0)); + kms.registerApp(address(legacy)); + IAppAuth.AppInfo memory info = _info(id); + info.appId = address(legacy); + bytes32 operation = getCert ? AuthorizationOperation.APP_GET_CERT : AuthorizationOperation.APP_GET_KEY; + AuthorizationResult memory result = kms.authorizeApp(_request(info, operation, rootSet, root, appSet, appEpoch)); + + uint256 rootEpoch = rootSet ? root : _current(); + bool expected = rootEpoch <= _current() && (!appSet || appEpoch == 0) + && kms.allowedOsImagesAtEpoch(rootEpoch, id.osImageHash) && id.composeHash == _id(0xc0, 0); + assert(result.authorized == expected); + assert(result.currentAppEpoch == 0); + assert(kms.currentAppEpoch(address(legacy)) == 0); + } + // --------------------------------------------------------------- // Legacy entry points serve KMS binaries that only hold root epoch 0: // before any rotation isKmsAllowed decides as KMS_SERVE at epoch 0, diff --git a/dstack/kms/auth-eth/test/EventAudit.t.sol b/dstack/kms/auth-eth/test/EventAudit.t.sol index 93a26cc67..84bd2205f 100644 --- a/dstack/kms/auth-eth/test/EventAudit.t.sol +++ b/dstack/kms/auth-eth/test/EventAudit.t.sol @@ -83,11 +83,11 @@ contract EventAuditTest is Test { vm.recordLogs(); vm.startPrank(owner); - app.addComposeHash(composeHash); - app.addDevice(device); - app.setAllowAnyDevice(true); + app.addComposeHashAtEpoch(0, composeHash); + app.addDeviceAtEpoch(0, device); + app.setAllowAnyDeviceAtEpoch(0, true); app.setRequireTcbUpToDate(true); - app.removeDevice(device); + app.removeDeviceAtEpoch(0, device); app.disableUpgrades(); vm.stopPrank(); Vm.Log[] memory logs = vm.getRecordedLogs(); @@ -123,7 +123,7 @@ contract EventAuditTest is Test { vm.recordLogs(); vm.prank(owner); - app.addComposeHash(orphaned); + app.addComposeHashAtEpoch(0, orphaned); Vm.Log[] memory orphanLogs = vm.getRecordedLogs(); _assertAudit(orphanLogs, owner, "compose-hash", orphaned, true); assertTrue(app.allowedComposeHashes(orphaned)); @@ -133,7 +133,7 @@ contract EventAuditTest is Test { vm.recordLogs(); vm.prank(owner); - app.addComposeHash(canonical); + app.addComposeHashAtEpoch(0, canonical); Vm.Log[] memory canonicalLogs = vm.getRecordedLogs(); _assertAudit(canonicalLogs, owner, "compose-hash", canonical, true); assertEq(_auditCount(canonicalLogs), 1); diff --git a/dstack/kms/auth-eth/test/LegacyAppCompat.t.sol b/dstack/kms/auth-eth/test/LegacyAppCompat.t.sol new file mode 100644 index 000000000..193eba714 --- /dev/null +++ b/dstack/kms/auth-eth/test/LegacyAppCompat.t.sol @@ -0,0 +1,186 @@ +/* + * SPDX-FileCopyrightText: © 2026 Phala Network + * + * SPDX-License-Identifier: Apache-2.0 + */ + +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; +import "openzeppelin-foundry-upgrades/Upgrades.sol"; +import "../contracts/DstackKms.sol"; +import "../contracts/DstackApp.sol"; + +// The epoch-less setters of the released app policies. +interface ILegacyApp { + function addComposeHash(bytes32 composeHash) external; + function removeComposeHash(bytes32 composeHash) external; + function setAllowAnyDevice(bool allowed) external; +} + +/// An upgraded DstackKms with app policies still on the v0.5.9 and v0.6.0 +/// implementations, which app owners upgrade on their own schedule. +contract LegacyAppCompatTest is Test { + DstackKms kms; + address owner = makeAddr("owner"); + address appOwner = makeAddr("appOwner"); + + string[2] internal appReleases = ["DstackAppV059.sol:DstackApp", "DstackAppV060.sol:DstackApp"]; + + bytes32 constant COMPOSE = bytes32(uint256(0xc0)); + bytes32 constant DEVICE = bytes32(uint256(0xd0)); + bytes32 constant IMAGE = bytes32(uint256(0x10)); + + function setUp() public { + vm.startPrank(owner); + kms = DstackKms( + Upgrades.deployUUPSProxy("DstackKms.sol", abi.encodeCall(DstackKms.initialize, (owner, address(0)))) + ); + kms.registerRootEpoch(key(91, 0xa0), key(33, 0xb0), "ipfs://0"); + kms.addOsImageHashAtEpoch(0, IMAGE); + vm.stopPrank(); + } + + function key(uint256 length, uint8 fill) internal pure returns (bytes memory k) { + k = new bytes(length); + for (uint256 i; i < length; ++i) { + k[i] = bytes1(fill); + } + if (length == 91) { + bytes memory prefix = hex"3059301306072a8648ce3d020106082a8648ce3d03010703420004"; + for (uint256 i; i < prefix.length; ++i) { + k[i] = prefix[i]; + } + } else { + k[0] = 0x02; + } + } + + function deployLegacyApp(uint256 release) internal returns (address app) { + vm.prank(appOwner); + app = Upgrades.deployUUPSProxy( + appReleases[release], + abi.encodeWithSignature( + "initialize(address,bool,bool,bytes32,bytes32)", appOwner, false, false, DEVICE, COMPOSE + ) + ); + kms.registerApp(app); + } + + function request(address app, bool appSet, uint256 appEpoch) internal pure returns (AuthorizationRequest memory) { + IAppAuth.AppInfo memory info; + info.appId = app; + info.composeHash = COMPOSE; + info.deviceId = DEVICE; + info.osImageHash = IMAGE; + info.tcbStatus = "UpToDate"; + info.advisoryIds = new string[](0); + return AuthorizationRequest( + info, keccak256("app_get_key"), OptionalUint256(appSet, appEpoch), OptionalUint256(false, 0) + ); + } + + function rotateRoot() internal { + vm.startPrank(owner); + kms.registerRootEpoch(key(91, 0xa1), key(33, 0xb1), "ipfs://1"); + kms.addOsImageHashAtEpoch(1, IMAGE); + (,,, bytes32 hash) = kms.rootEpoch(1); + kms.activateRootEpoch(1, hash); + vm.stopPrank(); + } + + function test_LegacyAppsAreAuthorizedAtAppEpochZeroAcrossRootRotation() public { + for (uint256 release; release < appReleases.length; ++release) { + address app = deployLegacyApp(release); + assertEq(kms.currentAppEpoch(app), 0); + assertTrue(kms.authorizeApp(request(app, false, 0)).authorized); + assertTrue(kms.authorizeApp(request(app, true, 0)).authorized); + AuthorizationResult memory result = kms.authorizeApp(request(app, true, 1)); + assertFalse(result.authorized); + assertEq(result.rejectionReason, "Requested app epoch is not active"); + + // Its epoch-less setters still decide app epoch 0. + vm.prank(appOwner); + ILegacyApp(app).removeComposeHash(COMPOSE); + assertFalse(kms.authorizeApp(request(app, false, 0)).authorized); + vm.prank(appOwner); + ILegacyApp(app).addComposeHash(COMPOSE); + } + rotateRoot(); + for (uint256 release; release < appReleases.length; ++release) { + address app = deployLegacyApp(release); + assertEq(kms.currentAppEpoch(app), 0); + AuthorizationResult memory result = kms.authorizeApp(request(app, false, 0)); + assertTrue(result.authorized, result.rejectionReason); + assertEq(result.currentRootEpoch, 1); + assertEq(result.currentAppEpoch, 0); + assertFalse(kms.authorizeApp(request(app, true, 1)).authorized); + } + } + + function test_InterfaceIdsTellLegacyAndEpochAppsApart() public { + bytes4 basicManagement = 0x8fd37527; + for (uint256 release; release < appReleases.length; ++release) { + address app = deployLegacyApp(release); + assertTrue(IERC165(app).supportsInterface(basicManagement)); + assertFalse(IERC165(app).supportsInterface(type(IAppEpochManagement).interfaceId)); + assertFalse(IERC165(app).supportsInterface(type(IAppEpochAuth).interfaceId)); + } + DstackApp current = DstackApp( + Upgrades.deployUUPSProxy( + "DstackApp.sol", + abi.encodeWithSignature( + "initialize(address,bool,bool,bytes32,bytes32)", appOwner, false, false, DEVICE, COMPOSE + ) + ) + ); + assertFalse(current.supportsInterface(basicManagement)); + assertTrue(current.supportsInterface(type(IAppEpochManagement).interfaceId)); + assertTrue(current.supportsInterface(type(IAppEpochAuth).interfaceId)); + } + + function test_UpgradedV059AppKeepsItsPolicyAtAppEpochZero() public { + checkUpgradeKeepsPolicy(0, false); + } + + function test_UpgradedV059AnyDeviceAppKeepsItsPolicyAtAppEpochZero() public { + checkUpgradeKeepsPolicy(0, true); + } + + function test_UpgradedV060AppKeepsItsPolicyAtAppEpochZero() public { + checkUpgradeKeepsPolicy(1, false); + } + + function test_UpgradedV060AnyDeviceAppKeepsItsPolicyAtAppEpochZero() public { + checkUpgradeKeepsPolicy(1, true); + } + + function checkUpgradeKeepsPolicy(uint256 release, bool anyDevice) internal { + address app = deployLegacyApp(release); + if (anyDevice) { + vm.prank(appOwner); + ILegacyApp(app).setAllowAnyDevice(true); + } + Options memory opts; + opts.referenceContract = appReleases[release]; + vm.startPrank(appOwner); + Upgrades.upgradeProxy(app, "DstackApp.sol", "", opts); + vm.stopPrank(); + + DstackApp upgraded = DstackApp(app); + assertEq(upgraded.currentAppEpoch(), 0); + assertTrue(upgraded.allowedComposeHashesAtEpoch(0, COMPOSE)); + assertTrue(upgraded.allowedDeviceIdsAtEpoch(0, DEVICE)); + assertEq(upgraded.allowAnyDeviceAtEpoch(0), anyDevice); + assertFalse(upgraded.allowedComposeHashesAtEpoch(1, COMPOSE)); + assertTrue(kms.authorizeApp(request(app, true, 0)).authorized); + + // The epoch-less setters are gone; the rows are written by epoch. + vm.prank(appOwner); + vm.expectRevert(); + ILegacyApp(app).addComposeHash(bytes32(uint256(0xc1))); + vm.prank(appOwner); + upgraded.removeComposeHashAtEpochs(0, 1, COMPOSE); + assertFalse(kms.authorizeApp(request(app, true, 0)).authorized); + } +} diff --git a/dstack/kms/auth-eth/test/ManageScript.t.sol b/dstack/kms/auth-eth/test/ManageScript.t.sol new file mode 100644 index 000000000..503e3a878 --- /dev/null +++ b/dstack/kms/auth-eth/test/ManageScript.t.sol @@ -0,0 +1,55 @@ +/* + * SPDX-FileCopyrightText: © 2026 Phala Network + * + * SPDX-License-Identifier: Apache-2.0 + */ + +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; +import "openzeppelin-foundry-upgrades/Upgrades.sol"; +import "../script/Manage.s.sol"; + +contract ManageScriptTest is Test { + function deployApp(string memory release, address owner) internal returns (address) { + return Upgrades.deployUUPSProxy( + release, + abi.encodeWithSignature( + "initialize(address,bool,bool,bytes32,bytes32)", owner, false, false, bytes32(0), bytes32(0) + ) + ); + } + + function addComposeHash(address app) internal returns (AddComposeHash script) { + vm.setEnv("APP_CONTRACT_ADDR", vm.toString(app)); + script = new AddComposeHash(); + script.setUp(); + } + + // Environment variables persist across tests, so the cases run in order. + function test_AppScriptsUseTheSettersTheAppHas() public { + (address owner, uint256 key) = makeAddrAndKey("owner"); + address legacy = deployApp("DstackAppV060.sol:DstackApp", owner); + address current = deployApp("DstackApp.sol", owner); + bytes32 composeHash = bytes32(uint256(0xc0)); + vm.setEnv("KMS_CONTRACT_ADDR", vm.toString(address(1))); + vm.setEnv("PRIVATE_KEY", vm.toString(key)); + vm.setEnv("COMPOSE_HASH", vm.toString(composeHash)); + + // A legacy app takes its epoch-less setter, which writes app epoch 0. + addComposeHash(legacy).run(); + assertTrue(DstackApp(legacy).allowedComposeHashes(composeHash)); + // An epoch-aware app needs its rows named. + AddComposeHash script = addComposeHash(current); + vm.expectRevert(bytes("set APP_EPOCH, or FROM_APP_EPOCH and TO_APP_EPOCH")); + script.run(); + + vm.setEnv("APP_EPOCH", "1"); + addComposeHash(current).run(); + assertFalse(DstackApp(current).allowedComposeHashesAtEpoch(0, composeHash)); + assertTrue(DstackApp(current).allowedComposeHashesAtEpoch(1, composeHash)); + script = addComposeHash(legacy); + vm.expectRevert(bytes("this app predates app epochs: only APP_EPOCH=0 applies")); + script.run(); + } +} diff --git a/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol b/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol index 4dc665bad..64334e594 100644 --- a/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol +++ b/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol @@ -153,9 +153,9 @@ contract UpgradesWithPluginTest is Test { // Set some state to verify it's preserved bytes32 deviceId = bytes32(uint256(456)); bytes32 composeHash = bytes32(uint256(789)); - app.addDevice(deviceId); - app.addComposeHash(composeHash); - app.setAllowAnyDevice(true); + app.addDeviceAtEpoch(0, deviceId); + app.addComposeHashAtEpoch(0, composeHash); + app.setAllowAnyDeviceAtEpoch(0, true); // Upgrade to new implementation using plugin Upgrades.upgradeProxy(appProxy, "DstackAppV2.sol", "");