Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Dynamic whitelist of intrinsics #6437

Closed
arhag opened this issue Dec 6, 2018 · 4 comments
Closed

Dynamic whitelist of intrinsics #6437

arhag opened this issue Dec 6, 2018 · 4 comments
Labels
SNAPSHOTS Requires introducing a new version for snapshots and/or ends support for old versions of snapshots

Comments

@arhag
Copy link
Contributor

arhag commented Dec 6, 2018

This issue tracks the code changes needed to facilitate dynamic changes to the whitelist of intrinsics against which WebAssembly code in eosio::setcode actions are allowed to link.

Background

Currently, nodeos is configured with a static set of intrinsics that it recognizes and has an implementation for. This set of intrinsics can be called the "recognized intrinsics set".

The protocol maintains a set of intrinsics against which a WebAssembly contract is allowed to link. If a contract was set at a time when linking to a particular intrinsic was allowed, it is not appropriate to suddenly take away the ability to link to the intrinsic. In particular, it would break consensus if attempted naively, since when a contract is used for the first time after a restart of nodeos, its WebAssembly code must be instantiated and cached on that node which involves linking to the intrinsics.

However, it is perfectly acceptable to disallow any new contract or updates to an existing contract to link against certain intrinsics that are to no longer be supported. The appropriate place to check the linking to enable this kind of behavior is during a eosio::setcode action. One can think of a subset of the "recognized intrinsics set" called the "whitelisted intrinsics set" which is the set of intrinsics against which WebAssembly code in a eosio::setcode action is allowed to link.

The "whitelisted intrinsics set" is allowed to change over time (or over blocks), which includes both additions and removals to the set, without any consensus problems as long as the changes are deterministically derived from the block data. It should also at all times remain a subset of the "recognized intrinsics set".

Currently, nodeos does not support a dynamic "whitelisted intrinsics set". One can think of the current behavior as if an implicit "whitelisted intrinsics set" exists that remains equivalent to the "recognized intrinsics set".

Changes to the state database

To maintain a deterministically changed "whitelisted intrinsics set", that set must exist inside of the chain state. This requires either adding a new index into the ChainBase state database specifically for this set, or adding the set as a field to an existing global object in the state database, e.g. the global_property_object. Either way, this change will require a new chain_snapshot_header version (e.g. version 2).

Furthermore, in order to simplify implementation of the code, it will be best if the "whitelisted intrinsics set" is initialized at genesis to the original "recognized intrinsics set" prior to any additional intrinsics that may be added via consensus protocol upgrades. Then consensus protocol upgrade features can simply add new intrinsics by inserting the name of the new intrinsic into the "whitelisted intrinsics set" during the execution of the protocol feature's trigger function (see #6429 for details). However, this approach means that a replay from genesis is required to support the code changes tracked by this issue.

Changes to eosio::chain::webassembly::common::root_resolver:

Default constructor continues to set validating to false.

Another constructor that takes a const reference to a set of intrinsic name strings would set validating to true and keep a pointer to the set. The set to be passed in would need to be comprised of the names of the intrinsics in the current "whitelisted intrinsics set".

In the resolve method, if validating == true, it would check that the export_name is in the set before calling Runtime::IntrinsicResolver::singleton.resolve.

Changes to eosio::chain::wasm_interface::validate:

The line containing root_resolver resolver(true); should be modified to instead pass in the "whitelisted intrinsics set" (or at least the appropriate form of it).

@taokayan
Copy link
Contributor

taokayan commented Feb 25, 2019

I guess this can be done without the change to chainbase database structure. we can use the generic contract's multi_index table to maintain this dynamic list. for example:

[[eosio::table]] struct newfuncs {
  uint64_t index; // unique for each intrinsics
  string signature; // the string used to link with WebAssembly. 
};

This table should be created under "eosio" in such a way that it needs 2/3 of BP to modify it. In the beginning of each block we can go through this multi_index table then put all new intrinsic functions into std::map or some cache for fast validation.

because of all multi_index tables use the same int64 index in chainbase this doesn't require any structure change to chainbase.

@gleehokie
Copy link
Contributor

@arhag in the Background section the second paragraph, last sentence does not make sense and needs to be corrected (“In particular, it would break consensus if attempted naively, since after a restart of nodeos when a contract is first time a contract used its WebAssembly code must be instantiated and cached which involves linking to the intrinsics.”).

@arhag
Copy link
Contributor Author

arhag commented Feb 25, 2019

@taokayan:

Since these are core EOSIO features, I don't think the data structures needed to track these changes should belong in the contract tables. A contract, even a privileged one, should not be able to modify this data directly; changes should only be done through the privileged intrinsics that guarantee certain invariants.

@gleehokie:

Thanks. I fixed it.

@arhag
Copy link
Contributor Author

arhag commented Mar 26, 2019

Resolved by #6831.

@arhag arhag closed this as completed Mar 26, 2019
@wanderingbort wanderingbort mentioned this issue Apr 1, 2019
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
SNAPSHOTS Requires introducing a new version for snapshots and/or ends support for old versions of snapshots
Projects
None yet
Development

No branches or pull requests

3 participants