Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kill switch: Polish and integrate MVP version #518

Draft
wants to merge 38 commits into
base: next
Choose a base branch
from
Draft
Changes from 32 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
383dd79
contracts: base kill switch implementation
facuspagnuolo Apr 24, 2019
9d7a118
contracts: binary kill switch implementation
facuspagnuolo Apr 24, 2019
c3f78fd
contracts: severities kill switch implementation
facuspagnuolo Apr 24, 2019
4d9ce01
contracts: application level kill switch
facuspagnuolo Apr 24, 2019
51127c4
contracts: kernel level kill switch
facuspagnuolo Apr 24, 2019
b75fdca
tests: add kill switch unit tests
facuspagnuolo Apr 29, 2019
4f4421e
contracts: use ternary ignoration for kill-switch settings
facuspagnuolo Apr 29, 2019
00b76b9
contracts: fix linting issues
facuspagnuolo Apr 29, 2019
df3568f
kill-switch: cleanup models leaving only app level
facuspagnuolo Apr 30, 2019
b6c5531
kill-switch: use `highest` instead of `lowest` allowed severities
facuspagnuolo Apr 30, 2019
c20c608
kill-switch: Support one issues registry per contract
facuspagnuolo Apr 30, 2019
1f17dcb
kill-switch: drop custom programmatic handling
facuspagnuolo Apr 30, 2019
130c343
kill-switch: integrate with aragonOS components
facuspagnuolo May 1, 2019
3c9eb50
kill-switch: place mocks and test files in corresponding dirs
facuspagnuolo May 2, 2019
ffff170
kill-switch: minor tweaks based on @bingen's feedback
facuspagnuolo May 3, 2019
76f8521
kill-switch: skip gas costs test for coverage
facuspagnuolo May 3, 2019
2db3e46
kill-switch: rename 'ignore' action to 'allow'
facuspagnuolo May 3, 2019
d28cfee
kill-switch: rename issues registry `entry` by `implementation`
facuspagnuolo May 7, 2019
dba7e42
kill-switch: rename test files to follow naming convention
facuspagnuolo May 7, 2019
60268b2
kill-switch: improve authentication params
facuspagnuolo May 7, 2019
8f91fec
kill-switch: fix app id constant value
facuspagnuolo May 7, 2019
1ce842d
kill-switch: improve gas costs and support current kernel versions
facuspagnuolo May 7, 2019
a13d491
kill-switch: move kernel initialization logic to DAO factory
facuspagnuolo May 7, 2019
77b8b2e
kill-switch: rename issues registry `isSeverityFor` to `hasSeverity`
facuspagnuolo May 7, 2019
d499267
kill-switch: rename `killSwitched` modifier to `killSwitchProtected`
facuspagnuolo May 7, 2019
17922cc
kill-switch: improve settings to customize different scenarios
facuspagnuolo May 8, 2019
0f288ce
kill-switch: add `DAOFactory` tests
facuspagnuolo May 8, 2019
a5f6bfe
kill-switch: test non compliant kernel versions
facuspagnuolo May 8, 2019
7f73ac3
kill-switch: allow core instances by default
facuspagnuolo May 8, 2019
5230a86
kill-switch: optimize tests
facuspagnuolo May 8, 2019
fd6348c
kill-switch: parameterize instance being called in kernel
facuspagnuolo May 9, 2019
6ab5f69
Merge branch 'dev' of github.com:aragon/aragonOS into application_kil…
facuspagnuolo May 9, 2019
ae617f6
kill-switch: apply suggestions from @izqui
facuspagnuolo May 17, 2019
ce7a29b
kill-switch: extract `killSwitchProtected` modifier to function
facuspagnuolo May 17, 2019
08f1a8b
kill-switch: address feedback from @sohkai
facuspagnuolo May 20, 2019
5817661
kill-switch: small tweaks and optimizations
facuspagnuolo May 21, 2019
3113233
kill-switch: split unit and integration tests
facuspagnuolo May 21, 2019
c057f0d
kill-switch: rename mocks dir and improve inline doc
facuspagnuolo Jun 10, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions contracts/apps/AragonApp.sol
Original file line number Diff line number Diff line change
@@ -20,6 +20,8 @@ import "../evmscript/EVMScriptRunner.sol";
// are included so that they are automatically usable by subclassing contracts
contract AragonApp is AppStorage, Autopetrified, VaultRecoverable, ReentrancyGuard, EVMScriptRunner, ACLSyntaxSugar {
string private constant ERROR_AUTH_FAILED = "APP_AUTH_FAILED";
string private constant ERROR_CONTRACT_CALL_NOT_ALLOWED = "APP_CONTRACT_CALL_NOT_ALLOWED";
string private constant ERROR_UNEXPECTED_KERNEL_RESPONSE = "APP_UNEXPECTED_KERNEL_RESPONSE";

modifier auth(bytes32 _role) {
require(canPerform(msg.sender, _role, new uint256[](0)), ERROR_AUTH_FAILED);
@@ -31,6 +33,31 @@ contract AragonApp is AppStorage, Autopetrified, VaultRecoverable, ReentrancyGua
_;
}

modifier killSwitchProtected {
IKernel _kernel = kernel();
bytes4 selector = _kernel.shouldDenyCallingContract.selector;
bytes memory callData = abi.encodeWithSelector(selector, appId(), address(this));
bool success = address(_kernel).call(callData);

// perform a check only if kernel supports "shouldDenyCallingApp" method
if (success) {
uint256 _outputLength;
assembly { _outputLength := returndatasize }
// we expect 32 bytes length returned value here
require(_outputLength == 32, ERROR_UNEXPECTED_KERNEL_RESPONSE);

bool _shouldDenyCall;
assembly {
let ptr := mload(0x40) // get next free memory pointer
mstore(0x40, add(ptr, returndatasize)) // set next free memory pointer
returndatacopy(ptr, 0, returndatasize) // copy call return value
_shouldDenyCall := mload(ptr) // read data
}
require(!_shouldDenyCall, ERROR_CONTRACT_CALL_NOT_ALLOWED);
}
_;
}

/**
* @dev Check whether an action can be performed by a sender for a particular role on this app
* @param _sender Sender of the call
160 changes: 126 additions & 34 deletions contracts/factory/DAOFactory.sol
Original file line number Diff line number Diff line change
@@ -1,33 +1,46 @@
pragma solidity 0.4.24;

import "../acl/IACL.sol";
import "../acl/ACL.sol";
import "../kernel/IKernel.sol";
import "../kernel/Kernel.sol";
import "../kernel/KernelProxy.sol";

import "../acl/IACL.sol";
import "../acl/ACL.sol";

import "../kill_switch/KillSwitch.sol";
import "../kill_switch/IssuesRegistry.sol";
import "./EVMScriptRegistryFactory.sol";


contract DAOFactory {
string private constant ERROR_MISSING_BASE_KILL_SWITCH = "DF_MISSING_BASE_KILL_SWITCH";

IKernel public baseKernel;
IACL public baseACL;
EVMScriptRegistryFactory public regFactory;
KillSwitch public baseKillSwitch;
EVMScriptRegistryFactory public scriptsRegistryFactory;

event DeployDAO(address dao);
event DeployEVMScriptRegistry(address reg);
event DeployEVMScriptRegistry(address registry);

/**
* @notice Create a new DAOFactory, creating DAOs with Kernels proxied to `_baseKernel`, ACLs proxied to `_baseACL`, and new EVMScriptRegistries created from `_regFactory`.
* @notice Create a new DAOFactory, creating DAOs with Kernels proxied to `_baseKernel`, ACLs proxied to `_baseACL`, and new EVMScriptRegistries created from `_scriptsRegistryFactory`.
* @param _baseKernel Base Kernel
* @param _baseACL Base ACL
* @param _regFactory EVMScriptRegistry factory
* @param _scriptsRegistryFactory EVMScriptRegistry factory
*/
constructor(IKernel _baseKernel, IACL _baseACL, EVMScriptRegistryFactory _regFactory) public {
constructor(
IKernel _baseKernel,
IACL _baseACL,
KillSwitch _baseKillSwitch,
EVMScriptRegistryFactory _scriptsRegistryFactory
)
public
{
// No need to init as it cannot be killed by devops199
if (address(_regFactory) != address(0)) {
regFactory = _regFactory;
if (address(_scriptsRegistryFactory) != address(0)) {
scriptsRegistryFactory = _scriptsRegistryFactory;
}
if (address(_baseKillSwitch) != address(0)) {
baseKillSwitch = _baseKillSwitch;
}

baseKernel = _baseKernel;
@@ -40,38 +53,117 @@ contract DAOFactory {
* @return Newly created DAO
*/
function newDAO(address _root) public returns (Kernel) {
Kernel dao = Kernel(new KernelProxy(baseKernel));
if (address(scriptsRegistryFactory) == address(0)) {
return _createDAO(_root);
}

if (address(regFactory) == address(0)) {
dao.initialize(baseACL, _root);
Kernel dao = _createDAO(address(this));
_setupNewDaoPermissions(dao, _root);
return dao;
}

/**
* @notice Create a new DAO with `_root` set as the initial admin and `_issuesRegistry` as the source of truth for kill-switch purpose
* @param _root Address that will be granted control to setup DAO permissions
* @param _issuesRegistry Address of the registry of issues that will be used in case of critical situations by the kill switch
* @return Newly created DAO
*/
function newDAOWithKillSwitch(address _root, IssuesRegistry _issuesRegistry) public returns (Kernel) {
require(address(baseKillSwitch) != address(0), ERROR_MISSING_BASE_KILL_SWITCH);

Kernel dao = _createDAO(address(this));
_createKillSwitch(dao, _issuesRegistry);

if (address(scriptsRegistryFactory) == address(0)) {
_transferCreatePermissionsRole(dao, address(this), _root);
} else {
dao.initialize(baseACL, this);
_setupNewDaoPermissions(dao, _root);
}

ACL acl = ACL(dao.acl());
bytes32 permRole = acl.CREATE_PERMISSIONS_ROLE();
bytes32 appManagerRole = dao.APP_MANAGER_ROLE();
return dao;
}

acl.grantPermission(regFactory, acl, permRole);
function _createDAO(address _permissionsCreator) internal returns (Kernel) {
Kernel dao = Kernel(new KernelProxy(baseKernel));
dao.initialize(baseACL, _permissionsCreator);
emit DeployDAO(address(dao));
return dao;
}

acl.createPermission(regFactory, dao, appManagerRole, this);
function _createKillSwitch(Kernel _dao, IssuesRegistry _issuesRegistry) internal {
// create app manager role for this
_createAppManagerRole(_dao, address(this));

EVMScriptRegistry reg = regFactory.newEVMScriptRegistry(dao);
emit DeployEVMScriptRegistry(address(reg));
// create kill switch instance and set it as default
bytes32 killSwitchAppID = _dao.DEFAULT_KILL_SWITCH_APP_ID();
bytes memory initializeData = abi.encodeWithSelector(baseKillSwitch.initialize.selector, _issuesRegistry);
_dao.newAppInstance(killSwitchAppID, baseKillSwitch, initializeData, true);
_allowKillSwitchCoreInstances(_dao);

// Clean up permissions
// First, completely reset the APP_MANAGER_ROLE
acl.revokePermission(regFactory, dao, appManagerRole);
acl.removePermissionManager(dao, appManagerRole);
// remove app manager role permissions from this
_removeAppManagerRole(_dao, address(this));
}

// Then, make root the only holder and manager of CREATE_PERMISSIONS_ROLE
acl.revokePermission(regFactory, acl, permRole);
acl.revokePermission(this, acl, permRole);
acl.grantPermission(_root, acl, permRole);
acl.setPermissionManager(_root, acl, permRole);
}
function _allowKillSwitchCoreInstances(Kernel _dao) internal {
KillSwitch killSwitch = KillSwitch(_dao.killSwitch());

emit DeployDAO(address(dao));
// create allow instances role for this
ACL acl = ACL(_dao.acl());
bytes32 setAllowedInstancesRole = killSwitch.SET_ALLOWED_INSTANCES_ROLE();
acl.createPermission(address(this), killSwitch, setAllowedInstancesRole, address(this));

return dao;
// allow calls to core instances: kill switch, acl and kernel
killSwitch.setAllowedInstance(address(_dao), true);
killSwitch.setAllowedInstance(address(acl), true);
killSwitch.setAllowedInstance(address(killSwitch), true);

// remove allow instances role from this
acl.revokePermission(address(this), killSwitch, setAllowedInstancesRole);
acl.removePermissionManager(killSwitch, setAllowedInstancesRole);
}

function _setupNewDaoPermissions(Kernel _dao, address _root) internal {
// grant permissions to script registry factory
_grantCreatePermissionsRole(_dao, scriptsRegistryFactory);
_createAppManagerRole(_dao, scriptsRegistryFactory);

// create evm scripts registry
EVMScriptRegistry scriptsRegistry = scriptsRegistryFactory.newEVMScriptRegistry(_dao);
emit DeployEVMScriptRegistry(address(scriptsRegistry));

// remove permissions from scripts registry factory and transfer to root address
_removeAppManagerRole(_dao, scriptsRegistryFactory);
_transferCreatePermissionsRole(_dao, scriptsRegistryFactory, _root);
}

function _grantCreatePermissionsRole(Kernel _dao, address _to) internal {
ACL acl = ACL(_dao.acl());
bytes32 createPermissionsRole = acl.CREATE_PERMISSIONS_ROLE();
acl.grantPermission(_to, acl, createPermissionsRole);
}

function _createAppManagerRole(Kernel _dao, address _to) internal {
ACL acl = ACL(_dao.acl());
bytes32 appManagerRole = _dao.APP_MANAGER_ROLE();
acl.createPermission(_to, _dao, appManagerRole, address(this));
}

function _removeAppManagerRole(Kernel _dao, address _from) internal {
ACL acl = ACL(_dao.acl());
bytes32 appManagerRole = _dao.APP_MANAGER_ROLE();
acl.revokePermission(_from, _dao, appManagerRole);
acl.removePermissionManager(_dao, appManagerRole);
}

function _transferCreatePermissionsRole(Kernel _dao, address _from, address _to) internal {
ACL acl = ACL(_dao.acl());
bytes32 createPermissionsRole = acl.CREATE_PERMISSIONS_ROLE();
acl.revokePermission(_from, acl, createPermissionsRole);
if (_from != address(this)) {
acl.revokePermission(address(this), acl, createPermissionsRole);
}

acl.grantPermission(_to, acl, createPermissionsRole);
acl.setPermissionManager(_to, acl, createPermissionsRole);
}
}
2 changes: 2 additions & 0 deletions contracts/kernel/IKernel.sol
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
pragma solidity ^0.4.24;

import "../acl/IACL.sol";
import "../kill_switch/IKillSwitch.sol";
import "../common/IVaultRecoverable.sol";


@@ -20,4 +21,5 @@ contract IKernel is IKernelEvents, IVaultRecoverable {

function setApp(bytes32 namespace, bytes32 appId, address app) public;
function getApp(bytes32 namespace, bytes32 appId) public view returns (address);
function shouldDenyCallingContract(bytes32 appId, address _instance) public returns (bool);
}
28 changes: 27 additions & 1 deletion contracts/kernel/Kernel.sol
Original file line number Diff line number Diff line change
@@ -10,6 +10,8 @@ import "../common/IsContract.sol";
import "../common/Petrifiable.sol";
import "../common/VaultRecoverable.sol";
import "../factory/AppProxyFactory.sol";
import "../kill_switch/IKillSwitch.sol";
import "../kill_switch/IIssuesRegistry.sol";
import "../lib/misc/ERCProxy.sol";


@@ -20,9 +22,9 @@ contract Kernel is IKernel, KernelStorage, KernelAppIds, KernelNamespaceConstant
*/
bytes32 public constant APP_MANAGER_ROLE = 0xb6d92708f3d4817afc106147d969e229ced5c46e65e0a5002a0d391287762bd0;

string private constant ERROR_AUTH_FAILED = "KERNEL_AUTH_FAILED";
string private constant ERROR_APP_NOT_CONTRACT = "KERNEL_APP_NOT_CONTRACT";
string private constant ERROR_INVALID_APP_CHANGE = "KERNEL_INVALID_APP_CHANGE";
string private constant ERROR_AUTH_FAILED = "KERNEL_AUTH_FAILED";

/**
* @dev Constructor that allows the deployer to choose if the base instance should be petrified immediately.
@@ -162,13 +164,29 @@ contract Kernel is IKernel, KernelStorage, KernelAppIds, KernelNamespaceConstant
recoveryVaultAppId = _recoveryVaultAppId;
}

/**
* @dev Tells whether a call to an instance of an app should be denied or not based on the kill-switch settings
* @param _appId Identifier for app to be checked
* @return True is the given call should be denied, false otherwise
*/
function shouldDenyCallingContract(bytes32 _appId, address _instance) public returns (bool) {
IKillSwitch _killSwitch = killSwitch();
if (address(_killSwitch) == address(0)) {
return false;
}

address _baseApp = getApp(KERNEL_APP_BASES_NAMESPACE, _appId);
return _killSwitch.shouldDenyCallingApp(_appId, _baseApp, _instance);
}

// External access to default app id and namespace constants to mimic default getters for constants
/* solium-disable function-order, mixedcase */
function CORE_NAMESPACE() external pure returns (bytes32) { return KERNEL_CORE_NAMESPACE; }
function APP_BASES_NAMESPACE() external pure returns (bytes32) { return KERNEL_APP_BASES_NAMESPACE; }
function APP_ADDR_NAMESPACE() external pure returns (bytes32) { return KERNEL_APP_ADDR_NAMESPACE; }
function KERNEL_APP_ID() external pure returns (bytes32) { return KERNEL_CORE_APP_ID; }
function DEFAULT_ACL_APP_ID() external pure returns (bytes32) { return KERNEL_DEFAULT_ACL_APP_ID; }
function DEFAULT_KILL_SWITCH_APP_ID() external pure returns (bytes32) { return KERNEL_DEFAULT_KILL_SWITCH_APP_ID; }
/* solium-enable function-order, mixedcase */

/**
@@ -197,6 +215,14 @@ contract Kernel is IKernel, KernelStorage, KernelAppIds, KernelNamespaceConstant
return IACL(getApp(KERNEL_APP_ADDR_NAMESPACE, KERNEL_DEFAULT_ACL_APP_ID));
}

/**
* @dev Get the installed KillSwitch app
* @return KillSwitch app
*/
function killSwitch() public view returns (IKillSwitch) {
return IKillSwitch(getApp(KERNEL_APP_ADDR_NAMESPACE, KERNEL_DEFAULT_KILL_SWITCH_APP_ID));
}

/**
* @dev Function called by apps to check ACL on kernel or to check permission status
* @param _who Sender of the original call
2 changes: 2 additions & 0 deletions contracts/kernel/KernelConstants.sol
Original file line number Diff line number Diff line change
@@ -10,10 +10,12 @@ contract KernelAppIds {
bytes32 internal constant KERNEL_CORE_APP_ID = apmNamehash("kernel");
bytes32 internal constant KERNEL_DEFAULT_ACL_APP_ID = apmNamehash("acl");
bytes32 internal constant KERNEL_DEFAULT_VAULT_APP_ID = apmNamehash("vault");
bytes32 internal constant KERNEL_DEFAULT_KILL_SWITCH_APP_ID = apmNamehash("kill-switch");
*/
bytes32 internal constant KERNEL_CORE_APP_ID = 0x3b4bf6bf3ad5000ecf0f989d5befde585c6860fea3e574a4fab4c49d1c177d9c;
bytes32 internal constant KERNEL_DEFAULT_ACL_APP_ID = 0xe3262375f45a6e2026b7e7b18c2b807434f2508fe1a2a3dfb493c7df8f4aad6a;
bytes32 internal constant KERNEL_DEFAULT_VAULT_APP_ID = 0x7e852e0fcfce6551c13800f1e7476f982525c2b5277ba14b24339c68416336d1;
bytes32 internal constant KERNEL_DEFAULT_KILL_SWITCH_APP_ID = 0x498cc0d31a6b7824a695121dd7e3f77a2b8f1108ed5a3367ec52d064799ee9cc;
}


14 changes: 14 additions & 0 deletions contracts/kill_switch/IIssuesRegistry.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
pragma solidity 0.4.24;


contract IIssuesRegistry {
enum Severity { None, Low, Mid, High, Critical }

event SeveritySet(address indexed implementation, Severity severity, address indexed sender);

function setSeverityFor(address implementation, Severity severity) external;

function hasSeverity(address implementation) public view returns (bool);

function getSeverityFor(address implementation) public view returns (Severity);
}
8 changes: 8 additions & 0 deletions contracts/kill_switch/IKillSwitch.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
pragma solidity 0.4.24;

import "./IIssuesRegistry.sol";


contract IKillSwitch {
function shouldDenyCallingApp(bytes32 _appId, address _base, address _proxy) external returns (bool);
}
31 changes: 31 additions & 0 deletions contracts/kill_switch/IssuesRegistry.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
pragma solidity 0.4.24;

import "../apps/AragonApp.sol";
import "./IIssuesRegistry.sol";


contract IssuesRegistry is IIssuesRegistry, AragonApp {
bytes32 constant public SET_SEVERITY_ROLE = keccak256("SET_SEVERITY_ROLE");

mapping (address => Severity) internal issuesSeverity;

function initialize() external onlyInit {
initialized();
}

function setSeverityFor(address implementation, Severity severity)
external
authP(SET_SEVERITY_ROLE, arr(implementation, uint256(severity)))
{
issuesSeverity[implementation] = severity;
emit SeveritySet(implementation, severity, msg.sender);
}

function hasSeverity(address implementation) public view isInitialized returns (bool) {
return issuesSeverity[implementation] != Severity.None;
}

function getSeverityFor(address implementation) public view isInitialized returns (Severity) {
return issuesSeverity[implementation];
}
}
Loading