threefoldtech / home

Starting point for the threefoldtech organization
https://threefold.io
Apache License 2.0
9 stars 4 forks source link

Restrict access to our development networks #1518

Open sameh-farouk opened 4 months ago

sameh-farouk commented 4 months ago

Currently, we are indulgent by letting anyone access our grid resources (DevNet, QANet) if he knows how to access them.

This practice has resulted in uninvited access to our development resources, and we have received reports of machines and public IP addresses being rented by unauthorized individuals. To mitigate these issues, we need to restrict access to our development networks, confining the accessibility to only our team members.

Suggestions made so far on a recent TG discussion:

in this case, we will also need to restrict TFTs on stellar testnet as it can be bridged to tfchain.

Let's discuss this in more detail. Additionally, please share any other ideas you may have.

sameh-farouk commented 2 months ago

I've had some time to think about this today and I've drafted an initial propsal that best aligns with our constraints.

What I try to achieve with this approach:

Off-Chain (faucet-like) Token Distribution Server for devnet and qanet:

Server Components:

Request Flow:

Security Considerations:

With this in place, we gain control over our resources and restrict new outsiders from accessing them. however, still, two issues remain:

  1. Existing unknown accounts with a lot of TFT. These accounts need to be reset. either by council motions if the account numbers are small. or via a simple one-time migration if the number of these accounts is more than what can be handled by multiple council motions (This might sound counterintuitive, but there won't be any runtime changes in this release except for a storage migration to reset these outsiders' accounts' balances. The release will be specifically deployed on the development and quality assurance networks, and afterward the migration code will be removed from all subsequent releases. this special release/runtime should never be deployed on test or main networks. We can also guard against accidental deployment by adding a guard condition on the migration code (by checking the genesis hash maybe?)).

  2. Stellar faucet service, if it can mint an unrestricted amount of TFT, which can be bridged to an account on the TFChain network, this could potentially provide a backdoor to the entire faucet system. As a result, measures will need to be taken to restrict this capability. I am unsure who currently maintains this stellar service or where the source code is located, so I would appreciate some feedback on this matter.

renauter commented 2 months ago

Great suggestion! I am just afraid we create another component and need to deploy and maintain it.

I was thinking about a faucet pallet that will be activated only for dev and qa env. With this we could customize whatever we want, to who we transfer (whitelist?), the frequency, the max amount etc

See examples here:

Just an idea but the key is to have something as "simple" as possible

renauter commented 2 months ago
  1. Existing unknown accounts with a lot of TFT. These accounts need to be reset. either by council motions if the account numbers are small. or via a simple one-time migration if the number of these accounts is more than what can be handled by multiple council motions (This might sound counterintuitive, but there won't be any runtime changes in this release except for a storage migration to reset these outsiders' accounts' balances. The release will be specifically deployed on the development and quality assurance networks, and afterward the migration code will be removed from all subsequent releases. this special release/runtime should never be deployed on test or main networks. We can also guard against accidental deployment by adding a guard condition on the migration code (by checking the genesis hash maybe?)).

For this purpose I have a suggestion:

(1) Adding a NetworkIdentifier storage in pallet tfgrid that could identify on which network we are Values would be "devnet" "qanet" "testnet" or "mainnet"

(2) To set the value it could be done via extrinsic set_network_identifier() requiring council approval origin

And once NetworkIdentifier is set by council approval for all networks:

(3) Resetting balances could be done like this:

OR

sameh-farouk commented 2 months ago

Good suggestion. I have notes regarding 1 and 2 .. the same outcome can be achieved by querying system.blockHash for block 0 (Genesis) Devnet genesis hash: 0xbb7f298ba5fca8802040061ddd0ca6406832238b7825ebfa66927cc25ec666be QAnet genesis hash: 0xffe36f258bba7f5ea972b6a74c094c96bf50fd36438143e28ed6a48783986562

So we can check against them without adding NetworkIdentifier or setting it by the council. however, your approach could be more flexible IMO.

regarding 3, I like option (b) but I'm concerned about the length of the call parameter. maybe we could use twinID instead of the account address? it can take an optional list for accounts twins' IDs to be left intact.

renauter commented 2 months ago

So we can check against them without adding NetworkIdentifier or setting it by the council. however, your approach could be more flexible IMO.

I am ok with both, I also like your option which do not require to add extra storage. If you think it is doable we could have some function like:

fn is_devnet() -> bool {
    return system.blockHash(0) == 0xbb7f298ba5fca8802040061ddd0ca6406832238b7825ebfa66927cc25ec666be
}

regarding 3, I like option (b) but I'm concerned about the length of the call parameter. maybe we could use twinID instead of the account address? it can take an optional list for accounts twins' IDs to be left intact.

I didn't think too much about the arguments but yes, twin_id is better and optional list to preserve also.