bsips
bsips copied to clipboard
bitshares
BSIP 65: fix locked accounts / recursive account permissions
Earlier discussion is here: https://github.com/bitshares/bitshares-core/issues/269
We do need to identify the locked accounts first.
After identified, an approach to fix them is reverting the last account_update_operation done in the accounts.
What does it mean to identify? My account is "dayman32" - and I have all the keys to it.
- We should not modify accounts without the explicit permission of the account owners (which is tricky obviously, because they can't prove ownership in the first place). Proving ownership of previous keys may be sufficient.
- Does this BSIP include funding of required development, or only the permission to modify the code?
- This is a recurring subject. A BSIP should not only fix accounts, but try to mitigate the issue for the future.
@pmconrad good points.
This is an issue but not a formal BSIP document. What should we put in the BSIP is to be discussed.
Would it make sense to start integrating the account recovery feature that steem has? That way, he could challenge the new owner role and have the previous installed again ..
I feel this Issue needs a BSIP drafted. It will be a consensus change, so December at the earliest for release (unlikely). Next opportunity would be June. Let's get an abstract defined here, then begin the specification within a PR. This BSIP drafting process is still TBD.
Possible approach for preventing future accidents:
- add a flag to account_update, if set the account_update operation must be authorized by both the old and the new authorities
add a flag to account_update, if set the account_update operation must be authorized by both the old and the new authorities
An account_update operation with the account itself in authorities or with an impossible loop after updated will still pass current authorization check in bitshares-core, because authorization is only verified before applying the changes (https://github.com/bitshares/bitshares-core/issues/1373 is related).
Actually, I think it's possible to check whether an account is locked after applied the update. I think YOYOW has implemented it or almost there, related code is here.
Do you guys recall the account recovery stuff in steem? Basically, one can challange the change of the authory (by paying a fee), then (one of) the previous permissions can call a claim for the account and replace the authority. That could help in this case too.
IIRC there's a limit to how long you can claim back an account, so that wouldn't really help the locked accounts. And I'm not sure if it's a good idea to retroactively introduce this feature. Suppose you have bought an account from someone, assuming that it's yours now, and on return you find that it has been claimed back by him...
And I'm not sure if it's a good idea to retroactively introduce this feature. Suppose you have bought an account from someone, assuming that it's yours now, and on return you find that it has been claimed back by him...
Good point.
Perhaps a way for current auth to sign intention to revoke legacy auths. This would make it incumbent on a new owner to perform this step as part of account transfer. In case the account is locked, a prior auth could request restoration (because current auth was unable to sign the revocation).
IMHO how to recover stolen account is out of this issue's scope.
@ryanRfox your proposal about "current auth to sign intention to revoke legacy auths" is exactly current behavior. If you changed authorities, new authorities would gain control, old authorities would lose control.
For locked accounts, my proposal would be simply undo the last account_update operation with a hard fork, no exception, no request required. As mentioned above, likely we'll be able to detect and avoid further account locking up with a hard fork. So all will be settled.
Adding to (next) Protocol Release for further research and discussion. Not clear where the best place to discuss this will be. It seems identifying the impacted accounts is required step. Shall we begin with an effort to do this?
We agree with @abitmore:
For locked accounts, my proposal would be simply undo the last account_update operation with a hard fork, no exception, no request required. As mentioned above, likely we'll be able to detect and avoid further account locking up with a hard fork. So all will be settled.
Also, we offer to add functionality preventing create or update account authority with circular dependencies.
The toolkit/script of identification of the impacted users should be elaborated before hard-fork.
Yes, @OpenLedgerApp please do some research to identify the locked accounts. Suggesting from @oxarbitrage is to use elasticsearch plugin. Next, would be some effort to identify the accounts in a deterministic way. Please add an effort estimate in this Issue. Thanks
actually in regards to elasticsearch what needed is the es_objects plugin.
here are all the accounts created in bitshares:
www.bitshares-kibana.info:5601/app/kibana#/discover?_g=(refreshInterval:(display:Off,pause:!f,value:0),time:(from:now-5y,mode:quick,to:now))&_a=(columns:!(_source),index:debd10a0-20b6-11e9-82af-8dfbd16c55a9,interval:auto,query:(language:lucene,query:'_index:"objects-account"'),sort:!(block_time,desc))
However, to get the locked accounts we need to among other things, do a query on top of that to compare if owner.account_auths == name.
Will get us the locked accounts that added their own account into the owner auth. Other checks must also be done like if active.account_auths == name, etc.
Unfortunately, it seems that this kind of checks cant be done by kibana(lucene syntax) but can be done by elasticsearch script queries, something like: https://stackoverflow.com/questions/28845052/query-for-one-field-doesnt-equal-another-field-in-elasticsearch
I dont want to go further into that as maybe @OpenLedgerApp already have its own way to identify them, can be also by done by the bitshares api with python or whatever by pulling all the accounts and make the checks there; 1,2 million accounts to check should not be a huge performance issue.
Regarding the scope of this BSIP: Should we limit to only owner.account_auths == name or expand to include active.account_auths == name? I tend toward least intervention. If active is currently "locked" it can be reset with owner.
Are we interested to learn if any additional "locked accounts" exist because their authority references a locked account? This gets into recursion and I'm not suggesting we dig too deep. If the remedy is to unlock the parent account, the children will become unlocked without further action required by this BSIP.
Inside owner or active we have account_auths and also key_auths and address_auths.
Unsure if you can also block your own account with thoses but worth discussion.
We have made research to identify the locked accounts: At the first step, we collected potential accounts authorities with circular dependencies using Elastic. Then we found authorities with circular dependencies using a python script.
@OpenLedgerApp please create a PR with your findings thus far. It will be most beneficial if the ES queries and Python code can be posted for review and evaluation by other reviewers. We can use the results to continue refining a BSIP draft.
By the way, the following url can be used to make direct queries to ES with es_objects data:
alfredo@alfredo-Inspiron-5559 ~ $ curl -X GET 'https://elasticsearch.bitshares-kibana.info/objects-account/data/_count?pretty=true' -H 'Content-Type: application/json' -d '
{
"query" : {
"bool" : { "must" : [{"match_all": {}}] }
}
}
'
{
"count" : 1303561,
"_shards" : {
"total" : 5,
"successful" : 5,
"skipped" : 0,
"failed" : 0
}
}
alfredo@alfredo-Inspiron-5559 ~ $
Also, the kibana url posted above changed to https://bitshares-kibana.info/ instead of www.bitshares-kibana.info:5601
We have created a branch with our results - https://github.com/openledger/bitshares-core/tree/94-fix-locked-accounts/tests/locked_accounts The branch contains ES queries, Python code, and instructions. You can use instructions in order to check our investigation results.
IMHO, we need consensus code (C++) in bitshares-core to avoid future circuits, so python scripts or ES queries which are only able to detect existing circuits (after damage has been done) are not enough, although they are indeed helpful for testing.
We don't need to put the list of already locked accounts into the BSIP, although we can take some of them as examples.
I may be able to write the C++ code, but I don't have time in the near future, if OpenLedgerDev can get it done, that's great.
Of course identify them is not enough, i think it is just to do this part of the issue: "We do need to identify the locked accounts first."
My understanding is @OpenLedgerApp will do the c++ code as well.
IMHO, we need consensus code (C++) in bitshares-core to avoid future circuits, so python scripts or ES queries which are only able to detect existing circuits (after damage has been done) are not enough, although they are indeed helpful for testing.
it's a good point and we understand it. We wanted to share our investigation results with the community and found out how many locked accounts there are. Also, our python code can be adopted into C++ code which fix locked accounts.
After internal discussions, we are seeing the solution of this issue into 2 steps: First, we offer to add additional functionality "avoid circular dependencies" (@abitmore -c) to the nearest hard fork. This functionality will prevent to create or update account authority with circular dependencies in the future.
The second and final step, we can develop C++ code which will be executed in the maintenance interval. This C++ code will find all locked accounts with circular dependencies and undo the last account_update operation. However, this step in comparison with the first one should be discussed with community. Maybe we should publish such accounts and ask the approval for this step for every owner.
What do you think?
The same code that forbids circular dependencies after a hardfork can detect these before the hardfork and collect them for fixing them at maintenance after the hardfork.
Both steps are a change of consensus and require specification in BSIP that is particularly specific on the detection logic.
IMO owner approval from individual accounts is not needed. If someone did this on purpose to make their account unusable there are different ways to achieve this again.