EOSIO / eos

An open source smart contract platform
https://developers.eos.io/manuals/eos
MIT License
11.27k stars 3.6k forks source link

cleos/keosd enhancements to catch common user errors with updating permissions #4037

Closed arhag closed 4 years ago

arhag commented 6 years ago

There are several ways one can lock themselves out of their own account. One user (#4033) unfortunately came across one of these ways. In that particular case, it is obvious that no reasonable user would have ever intended for that permission update to occur. So ideally cleos (or any other wallet app) should have detected it and rejected. In fact these class of mistakes can even be detected and rejected by producer / API nodes that are running the appropriate plugin.

However, there is a legitimate need for users to be able to intentionally lock themselves out of their account. One example is deploying an immutable smart contract. Cleos should still support a few standard ways to lock out access of an account; although it should print a warning message and ask for confirmation again from the user.

Then there are nearly limitless ways that a user can lock themselves out of their account that may be perfectly legitimate actions but that the wallet app cannot tell if it is legitimate or a user mistake. The simplest example of this class of mistakes is changing an owner key to a public key that the user does not have the private key for.

One way to help the user catch mistakes is for cleos to warn and require confirmation from the user when modifying a permission to something that keosd cannot satisfy. But it should not disallow the change entirely because there are legitimate use cases for such actions, for example:

taokayan commented 6 years ago

Hi would you help to review #4807 ?

ValeryDubrava commented 6 years ago

Sometimes such cases is not mistake. E.g. we want to do locked account for smart contracts, which cannot change code or get funds and so on. So, if you are going to add the protection, please, add a flag like --force or --unprotect also to disable protection.

wanderingbort commented 5 years ago

@snstrand please take a look at this issue in a few stages:

1 . Determine classes of "bad" permissions. Producing guidance on these outside of code is valuable. For instance,

2 . Determine a plan for addressing some/all of these classes in our tooling (cleos) that we can discuss within the team.

3 . Implement / Create tests.

wanderingbort commented 5 years ago

see https://github.com/EOSIO/eos/blob/master/libraries/chain/authorization_manager.cpp and https://github.com/EOSIO/eos/blob/master/libraries/chain/include/eosio/chain/authorization_manager.hpp

aclark-b1 commented 4 years ago

In order to focus our efforts on issues that are currently creating difficulty for the community we are closing tickets that were created prior to the EOSIO 2.0 release. If you believe this issue is still relevant please feel free to reopen it or create a new one. Thank you for your continued support of EOSIO!