From 8ed357b29704d7a5c937980a20a8c80647e2cad8 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Thu, 12 Nov 2026 01:19:00 -0800 Subject: [PATCH 1/2] fix(kms-auth)!: make DstackApp allowAnyDevice cover every app epoch Which hosts may run an app is not a property of a key generation, and a per-epoch switch is easy to get wrong: restricting the current epoch leaves any older row that allows any device open to every host, which can still fetch that epoch's keys. Make allowAnyDevice one switch again. - setAllowAnyDevice(bool) replaces setAllowAnyDeviceAtEpoch(s) and the allowAnyDeviceAtEpoch getter. It is the selector apps predating app epochs already have, so Manage.s.sol SetAllowAnyDevice calls it on either kind and takes no epoch. - Device ID rows stay per epoch. The decision is now allowAnyDevice or the selected epoch's device row. - Drop the unreleased _allowAnyDeviceByEpoch slot; the gap grows by one. - IAppEpochManagement swaps the two epoch setters for setAllowAnyDevice; its interface ID becomes 0xc65f31b6. BREAKING CHANGE: unreleased only: setAllowAnyDeviceAtEpoch(s), allowAnyDeviceAtEpoch and the IAppEpochManagement interface ID. Signed-off-by: Kevin Wang --- CHANGELOG.md | 13 ++- docs/app-key-epochs.md | 9 +- docs/onchain-governance.md | 10 +- dstack/kms/auth-eth/contracts/DstackApp.sol | 36 ++----- .../contracts/IAppEpochManagement.sol | 6 +- .../kms/auth-eth/docs/formal-verification.md | 10 +- dstack/kms/auth-eth/docs/specification.md | 4 +- dstack/kms/auth-eth/script/Manage.s.sol | 14 +-- dstack/kms/auth-eth/script/README.md | 2 +- .../auth-eth/test/DstackApp.symbolic.t.sol | 2 +- dstack/kms/auth-eth/test/DstackApp.t.sol | 46 +++------ .../test/DstackAppEpoch.symbolic.t.sol | 99 +++++++++---------- .../test/DstackKmsEpoch.symbolic.t.sol | 1 - dstack/kms/auth-eth/test/EventAudit.t.sol | 2 +- .../kms/auth-eth/test/LegacyAppCompat.t.sol | 2 +- .../auth-eth/test/UpgradesWithPlugin.t.sol | 2 +- 16 files changed, 110 insertions(+), 148 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5ea494a4..f4a360311 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,13 +76,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 range; `remove*AtEpochs(0, latest root epoch, value)` in `DstackKms` and `remove*AtEpochs(0, currentAppEpoch, value)` in `DstackApp` revoke a value everywhere. `DstackApp` reports its setters as the new ERC-165 interface - `IAppEpochManagement` (`0x853d2b79`). The `Manage.s.sol` scripts require - `ROOT_EPOCH` or `APP_EPOCH`, or a `FROM_*`/`TO_*` range. + `IAppEpochManagement` (`0xc65f31b6`). The `Manage.s.sol` scripts require + `ROOT_EPOCH` or `APP_EPOCH`, or a `FROM_*`/`TO_*` range. `DstackApp`'s + `allowAnyDevice` stays a single switch: `setAllowAnyDevice(bool)` covers every + app epoch, so dropping the device restriction cannot leave an older epoch + open to any host. - App contracts deployed before app epochs (v0.5.x, v0.6.0) keep working with the upgraded `DstackKms` and KMS, which authorize them for app epoch 0 only. `Manage.s.sol` detects them by interface ID and calls their epoch-less - setters. Upgrading one to the current `DstackApp` keeps its compose hashes, - devices and `allowAnyDevice` as app epoch 0. + setters. Upgrading one to the current `DstackApp` keeps its compose hashes and + devices as app epoch 0, and its `allowAnyDevice` for every app epoch. - DstackKms, DstackApp: ownership transfers take two steps for proxies upgraded from v0.5.9: `transferOwnership` nominates, and the new owner must call `acceptOwnership`. @@ -118,7 +121,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `removeKmsDevice`. Use the `*AtEpoch` or `*AtEpochs` forms; before any rotation, `addOsImageHashAtEpoch(0, hash)` is the old call. - **Breaking:** DstackApp: the epoch-less `addComposeHash`, `removeComposeHash`, - `addDevice`, `removeDevice` and `setAllowAnyDevice`, and with them + `addDevice` and `removeDevice`, and with them `IAppAuthBasicManagement`: an upgraded app no longer reports interface ID `0x8fd37527`, and calls to those functions revert. Use the `*AtEpoch` or `*AtEpochs` forms (`IAppEpochManagement`); at app epoch 0, diff --git a/docs/app-key-epochs.md b/docs/app-key-epochs.md index fd73df504..f261e8235 100644 --- a/docs/app-key-epochs.md +++ b/docs/app-key-epochs.md @@ -43,8 +43,11 @@ Authorization checks only the selected row, with O(1) add, removal and lookup: - `DstackKms`: OS images, KMS aggregated measurements and KMS device IDs are indexed by **root epoch**. OS image limits also apply to app key requests. -- `DstackApp`: compose hashes, device IDs and `allowAnyDevice` are indexed by - **app epoch**. Each required material must be allowed in the selected row. +- `DstackApp`: compose hashes and device IDs are indexed by **app epoch**. Each + required material must be allowed in the selected row. `allowAnyDevice` is a + single switch over every app epoch: which hosts may run the app is not a + property of a key generation, and a per-epoch switch could leave an older + epoch open to any host after the current one is restricted. There is no implicit access in either direction. Rotating keys starts an empty row; approving a new row neither copies old approvals nor grants historical keys. @@ -61,7 +64,7 @@ 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)`. +writes app epochs up to the next one. `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 diff --git a/docs/onchain-governance.md b/docs/onchain-governance.md index e5d19387d..cb4555244 100644 --- a/docs/onchain-governance.md +++ b/docs/onchain-governance.md @@ -175,7 +175,7 @@ to the next epoch. Removing a value from rows `0` through the current epoch revo | `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 | +| `setAllowAnyDevice(bool)` | Allow any device to run this app, at every app epoch | | `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 | @@ -189,15 +189,15 @@ app epoch 0 and the epoch-less setters `addComposeHash(bytes32)`, 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)" 0xc65f31b6 --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. +require `APP_EPOCH`, or `FROM_APP_EPOCH` and `TO_APP_EPOCH`. `SetAllowAnyDevice` takes +no epoch on either kind. Upgrading an older app to the current `DstackApp` keeps its +compose hashes and devices as app epoch 0, and its `allowAnyDevice` for every epoch. ### AppBootInfo Structure diff --git a/dstack/kms/auth-eth/contracts/DstackApp.sol b/dstack/kms/auth-eth/contracts/DstackApp.sol index 722614c67..284bb4c3a 100644 --- a/dstack/kms/auth-eth/contracts/DstackApp.sol +++ b/dstack/kms/auth-eth/contracts/DstackApp.sol @@ -31,8 +31,7 @@ contract DstackApp is // State variable to track if upgrades are disabled bool private _upgradesDisabled; - /// @notice Whether any device may boot this app. Epoch 0 only; for other epochs use the *AtEpoch getters and - /// EpochPolicyChanged. + /// @notice Whether any device may boot this app, at every app epoch. bool public allowAnyDevice; /// @notice Allowed device IDs for this app. Epoch 0 only; for other epochs use the *AtEpoch getters and @@ -48,7 +47,6 @@ contract DstackApp is // Epoch zero continues to use the original fields above after an upgrade. mapping(uint256 => mapping(bytes32 => bool)) private _composeHashesByEpoch; mapping(uint256 => mapping(bytes32 => bool)) private _deviceIdsByEpoch; - mapping(uint256 => bool) private _allowAnyDeviceByEpoch; event EpochPolicyChanged(bytes32 indexed policy, bytes32 indexed value, uint256 indexed epoch, bool enabled); @@ -172,10 +170,6 @@ contract DstackApp is return epoch == 0 ? allowedDeviceIds[deviceId] : _deviceIdsByEpoch[epoch][deviceId]; } - function allowAnyDeviceAtEpoch(uint256 epoch) public view returns (bool) { - return epoch == 0 ? allowAnyDevice : _allowAnyDeviceByEpoch[epoch]; - } - // A request may select any app 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 currentAppEpoch. @@ -229,24 +223,12 @@ contract DstackApp is emit EpochPolicyChanged(keccak256("compose-hash"), composeHash, epoch, 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) { - allowAnyDevice = allowed; - emit AllowAnyDeviceSet(allowed); - _emitPolicy("allow-any-device", bytes32(0), allowed); - } else { - _allowAnyDeviceByEpoch[epoch] = allowed; - } - emit EpochPolicyChanged(keccak256("allow-any-device"), bytes32(0), epoch, allowed); + // Covers every app epoch: restricting devices is a decision about hosts, not + // key generations, and must not leave an older epoch open to any host. + function setAllowAnyDevice(bool allowed) external onlyOwner { + allowAnyDevice = allowed; + emit AllowAnyDeviceSet(allowed); + _emitPolicy("allow-any-device", bytes32(0), allowed); } // Set whether TCB status must be UpToDate to boot this app @@ -338,7 +320,7 @@ contract DstackApp is } // Check if device is allowed (when device restriction is enabled) - if (!allowAnyDeviceAtEpoch(appEpoch) && !allowedDeviceIdsAtEpoch(appEpoch, bootInfo.deviceId)) { + if (!allowAnyDevice && !allowedDeviceIdsAtEpoch(appEpoch, bootInfo.deviceId)) { return (false, "Device not allowed"); } @@ -362,5 +344,5 @@ contract DstackApp is } // Add storage gap for upgradeable contracts - uint256[45] private __gap; + uint256[46] private __gap; } diff --git a/dstack/kms/auth-eth/contracts/IAppEpochManagement.sol b/dstack/kms/auth-eth/contracts/IAppEpochManagement.sol index a8585ecad..8af0f5b85 100644 --- a/dstack/kms/auth-eth/contracts/IAppEpochManagement.sol +++ b/dstack/kms/auth-eth/contracts/IAppEpochManagement.sol @@ -6,8 +6,9 @@ pragma solidity ^0.8.24; -/// Epoch-scoped allowlist management. Each setter names the app epoch rows it +/// Allowlist management. Each allowlist setter names the app epoch rows it /// writes: one epoch, or `fromEpoch` through `toEpoch`, at most the next epoch. +/// `setAllowAnyDevice` covers every app epoch. /// Replaces the epoch-less IAppAuthBasicManagement (0x8fd37527), which policies /// predating app epochs still report; tools tell the two apart by interface ID. interface IAppEpochManagement { @@ -19,6 +20,5 @@ interface IAppEpochManagement { 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; + function setAllowAnyDevice(bool allowed) external; } diff --git a/dstack/kms/auth-eth/docs/formal-verification.md b/dstack/kms/auth-eth/docs/formal-verification.md index 636387b27..95823ec4c 100644 --- a/dstack/kms/auth-eth/docs/formal-verification.md +++ b/dstack/kms/auth-eth/docs/formal-verification.md @@ -142,17 +142,19 @@ and admits one app per epoch, then one symbolic owner write where noted): (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. + requirement holds, that epoch admits the compose hash, and the device + is allowed by `allowAnyDevice` or that epoch's device row. - [x] `check_PolicyWrite_TakesEffect` — each `*AtEpoch` setter on an epoch 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. + compose hash from rows 0 through the next epoch, or a device from those + rows and clearing `allowAnyDevice`, 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 +dropping a setter's epoch bound or widening it past the next epoch, aliasing epoch 1 onto the legacy fields, letting `isAppAllowed` decide at a nonzero epoch or read the prepared next row, a permissionless diff --git a/dstack/kms/auth-eth/docs/specification.md b/dstack/kms/auth-eth/docs/specification.md index b99fe2e37..49a7b7de9 100644 --- a/dstack/kms/auth-eth/docs/specification.md +++ b/dstack/kms/auth-eth/docs/specification.md @@ -70,7 +70,7 @@ Plus inherited storage in ERC-7201 namespaces: `OwnableUpgradeable`, | Slot | Name | Type | Writable by | |---|---|---|---| | 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 | +| 1 | `_upgradesDisabled` + `allowAnyDevice` (packed) | `bool` + `bool` | `disableUpgrades` (owner, monotonic); `setAllowAnyDevice` (owner, every app epoch); 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) | @@ -238,7 +238,7 @@ byte sequence — no case variations, no trailing nulls, no whitespace. ### 3.11 Owner-only App mutations For each of the `*AtEpoch` and `*AtEpochs` forms of `addComposeHash`, -`removeComposeHash`, `addDevice`, `removeDevice` and `setAllowAnyDevice`, and +`removeComposeHash`, `addDevice` and `removeDevice`, and `setAllowAnyDevice`, `setRequireTcbUpToDate`, `disableUpgrades`: - **pre**: `msg.sender == owner()`. diff --git a/dstack/kms/auth-eth/script/Manage.s.sol b/dstack/kms/auth-eth/script/Manage.s.sol index 63ddd6f67..abb95311a 100644 --- a/dstack/kms/auth-eth/script/Manage.s.sol +++ b/dstack/kms/auth-eth/script/Manage.s.sol @@ -18,7 +18,6 @@ interface ILegacyAppManagement { 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 @@ -238,16 +237,11 @@ contract AddDevice is BaseScript { contract SetAllowAnyDevice is BaseScript { function run() external { bool allow = vm.envBool("ALLOW_ANY_DEVICE"); + require(address(app) != address(0), "APP_CONTRACT_ADDR not set"); - ILegacyAppManagement legacy = legacyApp(); - if (address(legacy) != address(0)) { - vm.broadcast(vm.envUint("PRIVATE_KEY")); - legacy.setAllowAnyDevice(allow); - } else { - (uint256 from, uint256 to) = appEpochs(); - vm.broadcast(vm.envUint("PRIVATE_KEY")); - app.setAllowAnyDeviceAtEpochs(from, to, allow); - } + // Apps with and without app epochs share this setter; it covers every epoch. + vm.broadcast(vm.envUint("PRIVATE_KEY")); + app.setAllowAnyDevice(allow); console.log("Set allowAnyDevice to:", allow); } diff --git a/dstack/kms/auth-eth/script/README.md b/dstack/kms/auth-eth/script/README.md index 1f50fae69..cb322fc19 100644 --- a/dstack/kms/auth-eth/script/README.md +++ b/dstack/kms/auth-eth/script/README.md @@ -166,7 +166,7 @@ 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... APP_EPOCH=0 ALLOW_ANY_DEVICE=true \ +KMS_CONTRACT_ADDR=0x... APP_CONTRACT_ADDR=0x... ALLOW_ANY_DEVICE=true \ forge script script/Manage.s.sol:SetAllowAnyDevice --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 c1ee862d2..1b3bb1244 100644 --- a/dstack/kms/auth-eth/test/DstackApp.symbolic.t.sol +++ b/dstack/kms/auth-eth/test/DstackApp.symbolic.t.sol @@ -120,7 +120,7 @@ contract DstackAppSymbolicTest is Test { } 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); + data = abi.encodeWithSelector(DstackApp.setAllowAnyDevice.selector, flag); } else if (which == 5) { data = abi.encodeWithSelector(DstackApp.setRequireTcbUpToDate.selector, flag); } else if (which == 6) { diff --git a/dstack/kms/auth-eth/test/DstackApp.t.sol b/dstack/kms/auth-eth/test/DstackApp.t.sol index a94a3d38f..0a7e0acce 100644 --- a/dstack/kms/auth-eth/test/DstackApp.t.sol +++ b/dstack/kms/auth-eth/test/DstackApp.t.sol @@ -126,14 +126,14 @@ contract DstackAppTest is Test { vm.expectEmit(true, true, true, true); emit AllowAnyDeviceSet(true); - app.setAllowAnyDeviceAtEpoch(0, true); + app.setAllowAnyDevice(true); assertTrue(app.allowAnyDevice()); vm.prank(owner); vm.expectEmit(true, true, true, true); emit AllowAnyDeviceSet(false); - app.setAllowAnyDeviceAtEpoch(0, false); + app.setAllowAnyDevice(false); assertFalse(app.allowAnyDevice()); } @@ -156,7 +156,7 @@ contract DstackAppTest is Test { app.removeDeviceAtEpoch(0, deviceId); vm.expectRevert(); - app.setAllowAnyDeviceAtEpoch(0, true); + app.setAllowAnyDevice(true); vm.stopPrank(); } @@ -195,7 +195,7 @@ contract DstackAppTest is Test { // Test with allowAnyDevice = true vm.prank(owner); - app.setAllowAnyDeviceAtEpoch(0, true); + app.setAllowAnyDevice(true); (allowed, reason) = app.isAppAllowed(bootInfo); assertTrue(allowed); @@ -551,30 +551,23 @@ contract DstackAppTest is Test { vm.stopPrank(); } - function test_AllowAnyDeviceIsScopedToExactlyOneEpoch() public { + function test_AllowAnyDeviceCoversEveryAppEpoch() public { IAppAuth.AppInfo memory info = epochInfo(); vm.startPrank(owner); - app.addComposeHashAtEpoch(0, info.composeHash); - app.setAllowAnyDeviceAtEpoch(0, true); + app.addComposeHashAtEpochs(0, 1, info.composeHash); app.advanceAppEpoch(0); - app.addComposeHashAtEpoch(1, info.composeHash); - bool allowed = authorizeCurrent(info); - assertFalse(allowed); + assertFalse(authorizeCurrent(info)); + app.setAllowAnyDevice(true); + (bool allowed,) = authorizeAt(info, 0, keccak256("app_get_key")); + assertTrue(allowed); + assertTrue(authorizeCurrent(info)); + + // Clearing it leaves every epoch to its own device row at once. app.addDeviceAtEpoch(1, info.deviceId); - allowed = authorizeCurrent(info); - assertTrue(allowed, "explicit device can replace wildcard in a newer row"); - app.removeDeviceAtEpoch(1, info.deviceId); - app.setAllowAnyDeviceAtEpoch(1, true); - app.setAllowAnyDeviceAtEpoch(0, false); - assertFalse(app.allowAnyDevice()); - assertTrue(app.allowAnyDeviceAtEpoch(1)); + app.setAllowAnyDevice(false); (allowed,) = authorizeAt(info, 0, keccak256("app_get_key")); assertFalse(allowed); - allowed = authorizeCurrent(info); - assertTrue(allowed); - app.setAllowAnyDeviceAtEpoch(1, false); - allowed = authorizeCurrent(info); - assertFalse(allowed); + assertTrue(authorizeCurrent(info)); vm.stopPrank(); } @@ -616,7 +609,6 @@ contract DstackAppTest is Test { app.addDeviceAtEpoch(0, info.deviceId); app.addComposeHashAtEpoch(1, next.composeHash); app.addDeviceAtEpoch(1, next.deviceId); - app.setAllowAnyDeviceAtEpoch(1, true); // Preparing the next row leaves the current one as it was. bool allowed = authorizeCurrent(info); assertTrue(allowed); @@ -644,11 +636,9 @@ contract DstackAppTest is Test { app.advanceAppEpoch(0); 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)); - assertTrue(app.allowAnyDeviceAtEpoch(epoch)); } (bool allowed,) = authorizeAt(info, 0, keccak256("app_get_key")); assertTrue(allowed); @@ -656,11 +646,9 @@ contract DstackAppTest is Test { // 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)); - assertFalse(app.allowAnyDeviceAtEpoch(epoch)); } (allowed,) = authorizeAt(info, 0, keccak256("app_get_key")); assertFalse(allowed); @@ -686,8 +674,6 @@ contract DstackAppTest is Test { app.addDeviceAtEpoch(2, bytes32(uint256(1))); vm.expectRevert("Unknown app epoch"); app.removeDeviceAtEpoch(2, bytes32(uint256(1))); - vm.expectRevert("Unknown app epoch"); - app.setAllowAnyDeviceAtEpoch(2, true); vm.stopPrank(); bytes memory unauthorized = abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, user); vm.startPrank(user); @@ -700,7 +686,7 @@ contract DstackAppTest is Test { vm.expectRevert(unauthorized); app.removeDeviceAtEpoch(0, bytes32(uint256(1))); vm.expectRevert(unauthorized); - app.setAllowAnyDeviceAtEpoch(0, true); + app.setAllowAnyDevice(true); 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 76ba5b148..c2c62813e 100644 --- a/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol +++ b/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol @@ -38,7 +38,7 @@ function _appAuth( contract DstackAppEpochSymbolicTest is Test { DstackApp internal app; address internal constant OWNER = address(0xA11CE); - uint256 internal constant POLICY_SETTERS = 10; + uint256 internal constant POLICY_SETTERS = 8; function setUp() public { DstackApp impl = new DstackApp(); @@ -54,17 +54,17 @@ contract DstackAppEpochSymbolicTest is Test { app.addDeviceAtEpoch(0, bytes32(uint256(0xd0))); app.advanceAppEpoch(0); app.addComposeHashAtEpoch(1, bytes32(uint256(0xc1))); - app.setAllowAnyDeviceAtEpoch(1, true); + app.addDeviceAtEpoch(1, bytes32(uint256(0xd1))); app.advanceAppEpoch(1); app.addComposeHashAtEpoch(2, bytes32(uint256(0xc2))); app.addDeviceAtEpoch(2, bytes32(uint256(0xd2))); vm.stopPrank(); } - /// Calldata of one policy setter: 0-4 write the row of `epoch`, 5-9 are - /// 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); + /// Calldata of one epoch-scoped policy setter: 0-3 write the row of `epoch`, + /// 4-7 are their range forms, writing every row from `epoch` through the next one. + function _policyCall(uint256 which, uint256 epoch, bytes32 word) internal view returns (bytes memory) { + return _rangePolicyCall(which, epoch, app.currentAppEpoch() + 1, word); } /// As _policyCall, with the range setters writing `epoch` through `next`. @@ -72,8 +72,7 @@ contract DstackAppEpochSymbolicTest is Test { uint256 which, uint256 epoch, uint256 next, - bytes32 word, - bool flag + bytes32 word ) internal pure @@ -83,17 +82,15 @@ contract DstackAppEpochSymbolicTest is Test { else if (which == 1) data = abi.encodeCall(DstackApp.removeComposeHashAtEpoch, (epoch, word)); 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.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)); + else if (which == 4) data = abi.encodeCall(DstackApp.addComposeHashAtEpochs, (epoch, next, word)); + else if (which == 5) data = abi.encodeCall(DstackApp.removeComposeHashAtEpochs, (epoch, next, word)); + else if (which == 6) data = abi.encodeCall(DstackApp.addDeviceAtEpochs, (epoch, next, word)); + else data = abi.encodeCall(DstackApp.removeDeviceAtEpochs, (epoch, next, word)); } - function _ownerPolicyWrite(uint256 which, uint256 epoch, bytes32 word, bool flag) internal { + function _ownerPolicyWrite(uint256 which, uint256 epoch, bytes32 word) internal { if (which >= POLICY_SETTERS) return; - bytes memory data = _policyCall(which, epoch, word, flag); + bytes memory data = _policyCall(which, epoch, word); vm.prank(OWNER); (bool ok,) = address(app).call(data); ok; // a reverted write leaves the setUp state, which is also a valid pre-state @@ -136,14 +133,15 @@ contract DstackAppEpochSymbolicTest is Test { uint256 before = app.currentAppEpoch(); bytes memory data; - 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, ()); - else if (which == 13) data = abi.encodeCall(UUPSUpgradeable.upgradeToAndCall, (addr, "")); - else if (which == 14) data = abi.encodeCall(Ownable2StepUpgradeable.transferOwnership, (addr)); - else if (which == 15) data = abi.encodeCall(Ownable2StepUpgradeable.acceptOwnership, ()); - else if (which == 16) data = abi.encodeCall(OwnableUpgradeable.renounceOwnership, ()); + if (which < POLICY_SETTERS) data = _policyCall(which, epoch, word); + else if (which == 8) data = abi.encodeCall(DstackApp.setAllowAnyDevice, (flag)); + else if (which == 9) data = abi.encodeCall(DstackApp.advanceAppEpoch, (epoch)); + else if (which == 10) data = abi.encodeCall(DstackApp.setRequireTcbUpToDate, (flag)); + else if (which == 11) data = abi.encodeCall(DstackApp.disableUpgrades, ()); + else if (which == 12) data = abi.encodeCall(UUPSUpgradeable.upgradeToAndCall, (addr, "")); + else if (which == 13) data = abi.encodeCall(Ownable2StepUpgradeable.transferOwnership, (addr)); + else if (which == 14) data = abi.encodeCall(Ownable2StepUpgradeable.acceptOwnership, ()); + else if (which == 15) data = abi.encodeCall(OwnableUpgradeable.renounceOwnership, ()); else return; vm.prank(caller); @@ -153,7 +151,7 @@ contract DstackAppEpochSymbolicTest is Test { uint256 afterEpoch = app.currentAppEpoch(); if (afterEpoch != before) { assert(afterEpoch == before + 1); - assert(caller == OWNER && which == 10 && epoch == before); + assert(caller == OWNER && which == 9 && epoch == before); } } @@ -166,13 +164,12 @@ contract DstackAppEpochSymbolicTest is Test { uint256 which, uint256 epoch, uint256 to, - bytes32 word, - bool flag + bytes32 word ) external { - vm.assume(which < POLICY_SETTERS && (which < 5 ? epoch : to) > app.currentAppEpoch() + 1); - bytes memory data = _rangePolicyCall(which, epoch, to, word, flag); + vm.assume(which < POLICY_SETTERS && (which < 4 ? epoch : to) > app.currentAppEpoch() + 1); + bytes memory data = _rangePolicyCall(which, epoch, to, word); vm.prank(caller); (bool ok,) = address(app).call(data); assert(!ok); @@ -187,7 +184,6 @@ contract DstackAppEpochSymbolicTest is Test { uint256 which, uint256 writeEpoch, bytes32 word, - bool flag, bytes32 composeHash, bytes32 deviceId, bool upToDate, @@ -196,7 +192,7 @@ contract DstackAppEpochSymbolicTest is Test { ) external { - _ownerPolicyWrite(which, writeEpoch, word, flag); + _ownerPolicyWrite(which, writeEpoch, word); IAppAuth.AppInfo memory info = _info(composeHash, deviceId, upToDate); vm.assume(epoch > app.currentAppEpoch() || !AuthorizationOperation.isApp(operation)); (bool allowed,) = _appAuth(app, info, epoch, operation); @@ -212,14 +208,13 @@ contract DstackAppEpochSymbolicTest is Test { uint256 which, uint256 writeEpoch, bytes32 word, - bool flag, bytes32 composeHash, bytes32 deviceId, bool upToDate ) external { - _ownerPolicyWrite(which, writeEpoch, word, flag); + _ownerPolicyWrite(which, writeEpoch, word); (bool allowed, string memory reason) = app.isAppAllowed(_info(composeHash, deviceId, upToDate)); assert(!allowed); assert(keccak256(bytes(reason)) == keccak256("App epoch requires epoch-aware authorization")); @@ -229,7 +224,6 @@ contract DstackAppEpochSymbolicTest is Test { uint256 which, uint256 writeEpoch, bytes32 word, - bool flag, bytes32 composeHash, bytes32 deviceId, bool upToDate @@ -245,9 +239,9 @@ contract DstackAppEpochSymbolicTest is Test { 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); + app.addDeviceAtEpoch(1, bytes32(uint256(0xd1))); vm.stopPrank(); - _ownerPolicyWrite(which, writeEpoch, word, flag); + _ownerPolicyWrite(which, writeEpoch, word); IAppAuth.AppInfo memory info = _info(composeHash, deviceId, upToDate); (bool allowed, string memory reason) = app.isAppAllowed(info); (bool expected, bytes32 expectedReason) = _decision(info, 0); @@ -264,7 +258,6 @@ contract DstackAppEpochSymbolicTest is Test { uint256 writeEpoch, uint256 writeTo, bytes32 word, - bool flag, bytes32 composeHash, bytes32 deviceId, bool upToDate, @@ -273,8 +266,8 @@ contract DstackAppEpochSymbolicTest is Test { external { vm.assume(which < POLICY_SETTERS); - vm.assume(which < 5 ? writeEpoch != epoch : epoch < writeEpoch || epoch > writeTo); - bytes memory data = _rangePolicyCall(which, writeEpoch, writeTo, word, flag); + vm.assume(which < 4 ? writeEpoch != epoch : epoch < writeEpoch || epoch > writeTo); + bytes memory data = _rangePolicyCall(which, writeEpoch, writeTo, word); IAppAuth.AppInfo memory info = _info(composeHash, deviceId, upToDate); (bool allowedBefore, bytes32 reasonBefore) = _decision(info, epoch); @@ -290,7 +283,7 @@ contract DstackAppEpochSymbolicTest is Test { uint256 which, uint256 writeEpoch, bytes32 word, - bool flag, + bool anyDevice, bool tcbFlag, bytes32 composeHash, bytes32 deviceId, @@ -300,15 +293,17 @@ contract DstackAppEpochSymbolicTest is Test { ) external { - vm.prank(OWNER); + vm.startPrank(OWNER); app.setRequireTcbUpToDate(tcbFlag); - _ownerPolicyWrite(which, writeEpoch, word, flag); + app.setAllowAnyDevice(anyDevice); + vm.stopPrank(); + _ownerPolicyWrite(which, writeEpoch, word); IAppAuth.AppInfo memory info = _info(composeHash, deviceId, upToDate); bytes32 op = getCert ? AuthorizationOperation.APP_GET_CERT : AuthorizationOperation.APP_GET_KEY; (bool allowed,) = _appAuth(app, info, epoch, op); bool expected = epoch <= app.currentAppEpoch() && (!tcbFlag || upToDate) && app.allowedComposeHashesAtEpoch(epoch, composeHash) - && (app.allowAnyDeviceAtEpoch(epoch) || app.allowedDeviceIdsAtEpoch(epoch, deviceId)); + && (anyDevice || app.allowedDeviceIdsAtEpoch(epoch, deviceId)); assert(allowed == expected); } @@ -319,17 +314,16 @@ contract DstackAppEpochSymbolicTest is Test { uint256 from, uint256 to, uint256 epoch, - bytes32 word, - bool flag + bytes32 word ) external { vm.assume(which < POLICY_SETTERS && epoch <= app.currentAppEpoch() + 1); - vm.assume(which < 5 ? epoch == from : from <= epoch && epoch <= to && to <= app.currentAppEpoch() + 1); - bytes memory data = _rangePolicyCall(which, from, to, word, flag); + vm.assume(which < 4 ? epoch == from : from <= epoch && epoch <= to && to <= app.currentAppEpoch() + 1); + bytes memory data = _rangePolicyCall(which, from, to, word); vm.prank(OWNER); (bool ok,) = address(app).call(data); - uint256 kind = which % 5; + uint256 kind = which % 4; if (!ok) { assert(word == bytes32(0) && (kind == 0 || kind == 2)); return; @@ -338,13 +332,12 @@ contract DstackAppEpochSymbolicTest is Test { 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); } // --------------------------------------------------------------- - // 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. + // Revocation: removing a compose hash from every row, or a device from + // every row and clearing the any-device flag, 0 through the next epoch, + // leaves no selectable app epoch at which it is authorized. // --------------------------------------------------------------- function check_FullRangeRemoval_RevokesAtEveryAppEpoch( @@ -361,7 +354,7 @@ contract DstackAppEpochSymbolicTest is Test { vm.startPrank(OWNER); if (byDevice) { app.removeDeviceAtEpochs(0, next, deviceId); - app.setAllowAnyDeviceAtEpochs(0, next, false); + app.setAllowAnyDevice(false); } else { app.removeComposeHashAtEpochs(0, next, composeHash); } diff --git a/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol b/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol index c7f2d6be2..da39b1004 100644 --- a/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol +++ b/dstack/kms/auth-eth/test/DstackKmsEpoch.symbolic.t.sol @@ -88,7 +88,6 @@ contract DstackKmsEpochSymbolicTest is Test { app = DstackApp(kms.deployAndRegisterApp(OWNER, false, false, true, bytes32(0), _id(0xc0, 0))); app.advanceAppEpoch(0); app.addComposeHashAtEpoch(1, _id(0xc0, 1)); - app.setAllowAnyDeviceAtEpoch(1, true); vm.stopPrank(); } diff --git a/dstack/kms/auth-eth/test/EventAudit.t.sol b/dstack/kms/auth-eth/test/EventAudit.t.sol index 84bd2205f..c391d120e 100644 --- a/dstack/kms/auth-eth/test/EventAudit.t.sol +++ b/dstack/kms/auth-eth/test/EventAudit.t.sol @@ -85,7 +85,7 @@ contract EventAuditTest is Test { vm.startPrank(owner); app.addComposeHashAtEpoch(0, composeHash); app.addDeviceAtEpoch(0, device); - app.setAllowAnyDeviceAtEpoch(0, true); + app.setAllowAnyDevice(true); app.setRequireTcbUpToDate(true); app.removeDeviceAtEpoch(0, device); app.disableUpgrades(); diff --git a/dstack/kms/auth-eth/test/LegacyAppCompat.t.sol b/dstack/kms/auth-eth/test/LegacyAppCompat.t.sol index 193eba714..5ba631ba6 100644 --- a/dstack/kms/auth-eth/test/LegacyAppCompat.t.sol +++ b/dstack/kms/auth-eth/test/LegacyAppCompat.t.sol @@ -171,7 +171,7 @@ contract LegacyAppCompatTest is Test { assertEq(upgraded.currentAppEpoch(), 0); assertTrue(upgraded.allowedComposeHashesAtEpoch(0, COMPOSE)); assertTrue(upgraded.allowedDeviceIdsAtEpoch(0, DEVICE)); - assertEq(upgraded.allowAnyDeviceAtEpoch(0), anyDevice); + assertEq(upgraded.allowAnyDevice(), anyDevice); assertFalse(upgraded.allowedComposeHashesAtEpoch(1, COMPOSE)); assertTrue(kms.authorizeApp(request(app, true, 0)).authorized); diff --git a/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol b/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol index 64334e594..768934b9c 100644 --- a/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol +++ b/dstack/kms/auth-eth/test/UpgradesWithPlugin.t.sol @@ -155,7 +155,7 @@ contract UpgradesWithPluginTest is Test { bytes32 composeHash = bytes32(uint256(789)); app.addDeviceAtEpoch(0, deviceId); app.addComposeHashAtEpoch(0, composeHash); - app.setAllowAnyDeviceAtEpoch(0, true); + app.setAllowAnyDevice(true); // Upgrade to new implementation using plugin Upgrades.upgradeProxy(appProxy, "DstackAppV2.sol", ""); From 414990b0834fcf148a1d5d5317d663d0fa6bef10 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Thu, 12 Nov 2026 02:22:31 -0800 Subject: [PATCH 2/2] fix(kms-auth): address review of the global allowAnyDevice - Correct the AllowAnyDeviceSet NatSpec and the epoch-zero-only notes in app-key-epochs.md, which allowAnyDevice no longer follows. - SetAllowAnyDevice refuses APP_EPOCH and FROM_/TO_APP_EPOCH instead of ignoring them, since it changes every epoch. - Check the legacy-equals-epoch-0 Halmos property with a symbolic allowAnyDevice, and that an upgraded any-device app stays open to unlisted devices at later epochs. - Tidy spec and formal-verification wording. Signed-off-by: Kevin Wang --- docs/app-key-epochs.md | 9 ++++++--- dstack/kms/auth-eth/contracts/DstackApp.sol | 2 +- dstack/kms/auth-eth/docs/formal-verification.md | 3 +-- dstack/kms/auth-eth/docs/specification.md | 4 ++-- dstack/kms/auth-eth/script/Manage.s.sol | 6 +++++- dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol | 2 ++ dstack/kms/auth-eth/test/LegacyAppCompat.t.sol | 7 +++++++ 7 files changed, 24 insertions(+), 9 deletions(-) diff --git a/docs/app-key-epochs.md b/docs/app-key-epochs.md index f261e8235..f89f13c64 100644 --- a/docs/app-key-epochs.md +++ b/docs/app-key-epochs.md @@ -88,7 +88,9 @@ 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. +not change its legacy bool or emit a misleading legacy material event. The +exception is `allowAnyDevice` and its `AllowAnyDeviceSet` event, which cover every +app epoch. 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 @@ -113,8 +115,9 @@ The original bool mappings and `allowAnyDevice` retain their storage locations a 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. +preserved. Even if the pre-upgrade counter is nonzero, old global material +approvals grant only row zero after upgrading: explicitly populate the intended +nonzero rows. `allowAnyDevice` keeps covering every app epoch. Upgrade the KMS policy before app policies, then approve the intended rows before restarting guests or activating a new root. `DstackApp` advertises the optional diff --git a/dstack/kms/auth-eth/contracts/DstackApp.sol b/dstack/kms/auth-eth/contracts/DstackApp.sol index 284bb4c3a..96b974c14 100644 --- a/dstack/kms/auth-eth/contracts/DstackApp.sol +++ b/dstack/kms/auth-eth/contracts/DstackApp.sol @@ -56,7 +56,7 @@ contract DstackApp is event DeviceAdded(bytes32 deviceId); event DeviceRemoved(bytes32 deviceId); event UpgradesDisabled(); - /// @notice Epoch 0 only; for other epochs use the *AtEpoch getters and EpochPolicyChanged. + /// @notice Covers every app epoch; there is no EpochPolicyChanged counterpart. event AllowAnyDeviceSet(bool allowAny); event RequireTcbUpToDateSet(bool requireUpToDate); /// @notice Additive audit event for reconstructing authorization policy. diff --git a/dstack/kms/auth-eth/docs/formal-verification.md b/dstack/kms/auth-eth/docs/formal-verification.md index 95823ec4c..409330c5e 100644 --- a/dstack/kms/auth-eth/docs/formal-verification.md +++ b/dstack/kms/auth-eth/docs/formal-verification.md @@ -154,8 +154,7 @@ and admits one app per epoch, then one symbolic owner write where noted): either app operation. Mutation-tested: requiring both the any-device flag and a listed device, -dropping a setter's -epoch bound or widening it past the next epoch, aliasing epoch 1 onto the +dropping a setter's epoch bound or widening it past the next epoch, aliasing epoch 1 onto the legacy fields, letting `isAppAllowed` decide at a nonzero epoch or read the prepared next row, a permissionless `advanceAppEpoch` and dropping its stale-transaction guard each produce a diff --git a/dstack/kms/auth-eth/docs/specification.md b/dstack/kms/auth-eth/docs/specification.md index 49a7b7de9..5bbb1d7f7 100644 --- a/dstack/kms/auth-eth/docs/specification.md +++ b/dstack/kms/auth-eth/docs/specification.md @@ -238,8 +238,8 @@ byte sequence — no case variations, no trailing nulls, no whitespace. ### 3.11 Owner-only App mutations For each of the `*AtEpoch` and `*AtEpochs` forms of `addComposeHash`, -`removeComposeHash`, `addDevice` and `removeDevice`, and `setAllowAnyDevice`, -`setRequireTcbUpToDate`, `disableUpgrades`: +`removeComposeHash`, `addDevice` and `removeDevice`, and for `setAllowAnyDevice`, +`setRequireTcbUpToDate` and `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 abb95311a..ff5c6bfd7 100644 --- a/dstack/kms/auth-eth/script/Manage.s.sol +++ b/dstack/kms/auth-eth/script/Manage.s.sol @@ -238,8 +238,12 @@ contract SetAllowAnyDevice is BaseScript { function run() external { bool allow = vm.envBool("ALLOW_ANY_DEVICE"); require(address(app) != address(0), "APP_CONTRACT_ADDR not set"); + require( + !vm.envExists("APP_EPOCH") && !vm.envExists("FROM_APP_EPOCH") && !vm.envExists("TO_APP_EPOCH"), + "allowAnyDevice covers every app epoch: unset APP_EPOCH, FROM_APP_EPOCH and TO_APP_EPOCH" + ); - // Apps with and without app epochs share this setter; it covers every epoch. + // Apps with and without app epochs share this setter. vm.broadcast(vm.envUint("PRIVATE_KEY")); app.setAllowAnyDevice(allow); diff --git a/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol b/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol index c2c62813e..e9c644dfe 100644 --- a/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol +++ b/dstack/kms/auth-eth/test/DstackAppEpoch.symbolic.t.sol @@ -224,6 +224,7 @@ contract DstackAppEpochSymbolicTest is Test { uint256 which, uint256 writeEpoch, bytes32 word, + bool anyDevice, bytes32 composeHash, bytes32 deviceId, bool upToDate @@ -240,6 +241,7 @@ contract DstackAppEpochSymbolicTest is Test { // A prepared next row must not leak into the legacy decision. app.addComposeHashAtEpoch(1, bytes32(uint256(0xc1))); app.addDeviceAtEpoch(1, bytes32(uint256(0xd1))); + app.setAllowAnyDevice(anyDevice); vm.stopPrank(); _ownerPolicyWrite(which, writeEpoch, word); IAppAuth.AppInfo memory info = _info(composeHash, deviceId, upToDate); diff --git a/dstack/kms/auth-eth/test/LegacyAppCompat.t.sol b/dstack/kms/auth-eth/test/LegacyAppCompat.t.sol index 5ba631ba6..59750a092 100644 --- a/dstack/kms/auth-eth/test/LegacyAppCompat.t.sol +++ b/dstack/kms/auth-eth/test/LegacyAppCompat.t.sol @@ -179,6 +179,13 @@ contract LegacyAppCompatTest is Test { vm.prank(appOwner); vm.expectRevert(); ILegacyApp(app).addComposeHash(bytes32(uint256(0xc1))); + // The upgraded allowAnyDevice covers later epochs too: row 1 lists no device. + vm.startPrank(appOwner); + upgraded.advanceAppEpoch(0); + upgraded.addComposeHashAtEpoch(1, COMPOSE); + vm.stopPrank(); + assertEq(kms.authorizeApp(request(app, true, 1)).authorized, anyDevice); + vm.prank(appOwner); upgraded.removeComposeHashAtEpochs(0, 1, COMPOSE); assertFalse(kms.authorizeApp(request(app, true, 0)).authorized);