Skip to content
This repository has been archived by the owner on Jan 29, 2024. It is now read-only.

Verify caller type #2

Open
cryptoAtwill opened this issue Oct 26, 2022 · 17 comments
Open

Verify caller type #2

cryptoAtwill opened this issue Oct 26, 2022 · 17 comments
Labels

Comments

@cryptoAtwill
Copy link
Collaborator

cryptoAtwill commented Oct 26, 2022

When the actor is invoked, it should perform a caller type checker so that when the caller is not satisfied, can throw error.

The proposed solution is to build a simple role management on fvm actor. We can follow existing evm role management system with some simplification. We just need two roles: caller and admin. For admin role, it can perform the CRUD of other roles while caller can just call the actor methods.

Role and address are many-to-many relationships and it can be persisted in a HAMT.

@adlrocha
Copy link
Contributor

adlrocha commented Nov 1, 2022

For fine-grained access-right management that may work, the problem is that we also want to limit the access to specific methods to specific types of actors, i.e. actors with a specific code_cid or implementing a specific interface. One way we could work around this, is forcing actors we want to check the type from to implement a type() method that returns its code_cid or, if we don´t have aces to it, it can return the ABI of the contract (to see the methods available). Of course, while the code_cid can not be forged, depending on how the method is implemented, an actor could be forging its ABI (breaking the type check).

@cryptoAtwill
Copy link
Collaborator Author

Maybe this can be of help? ethereum/EIPs#881

@adlrocha
Copy link
Contributor

adlrocha commented Nov 8, 2022

Hey, @cryptoAtwill, I was reading through ethereum/EIPs#881 and I think we can include something similar for our actors for now (at least until there's some kind of standard for the FVM --if it ever is--). This is an idea from the top of my head.

As we don't have a function selector in FVM, my suggestion would be:

  • To implement a method selector as the cid of its signature considering its method name and method number as a concatenation of strings as well as the signature of the function using the types of the arguments (cid(methodNum: methodName(arg_types) output_types), for instance cid(1: send(address, TokenAmount).
    • For this we could maybe write a simple macro as part of the fvm_utils to make it more accessible to all actors.
  • The cid of an interface is computed as the cid of all the methods cids cid([]cid_methods). Thus, for an interface of n methods we would have to perform n+1 cids. If we see that this is expensive in terms of hash we could maybe reduce to a single cid, and compute the interface cid by just concatenating all the methods: cid(1:method_name(args)output, 2:method_name(args)output).
  • Every actor supporting an interface would include for method num 2 the implementation of a function supportsInterface(cid) bool that verifies if the interface is implemented by the actor. supportsInterface returns true if the cid of an specific interface implemented by the actor is provided as an argument. With this, we provide actors a way of identifying themselves of a specific type.

@cryptoAtwill
Copy link
Collaborator Author

Sounds, good. I can draft some initial implementation, then we can discuss over that.

@sergefdrv
Copy link
Contributor

Please have a look at FIP-0049 and the corresponding discussion.

@sergefdrv
Copy link
Contributor

method num 1 the implementation of a function supportsInterface(cid) bool

The method number 1 is reserved for the constructor, isn't it?

@adlrocha
Copy link
Contributor

Please have a look at FIP-0049 and filecoin-project/FIPs#382.

We could jump right away to implement this approach (and even propose it as reference implementation to the FIP). My only concern is that is still on a draft state and I don't know to what extent it may change or when should we expect an implementation to be available. But if we think that this approach is better, let's go for it. It has been more extensively discussed.

The method number 1 is reserved for the constructor, isn't it?

Correct. I wanted to give an example, but the methodNum I chose is already reserved so it could be confusing (my bad). Already edited.

@cryptoAtwill
Copy link
Collaborator Author

If my understanding is correct, that means this interface cid is generated offline and passed to the actor during construction?

@sergefdrv
Copy link
Contributor

If my understanding is correct, that means this interface cid is generated offline and passed to the actor during construction?

@cryptoAtwill, what do you mean by "interface CID"?

@cryptoAtwill
Copy link
Collaborator Author

Here is a brief update on my ideas so far.

To-do

In fvm_utils, I'm planning to introduce the following:

  • Standardize SubnetActor interface.
  • Support interface method number

Standardize SubnetActor

Our current SubnetActor interface is defined as follows:

pub trait SubnetActor {
    /// Deploys subnet actor with the corresponding parameters.
    fn constructor<BS, RT>(rt: &mut RT, params: ConstructParams) -> Result<(), ActorError>
    where
        BS: Blockstore,
        RT: Runtime<BS>;
    /// Logic for new peers to join a subnet.
    fn join<BS, RT>(rt: &mut RT, params: JoinParams) -> Result<Option<RawBytes>, ActorError>
    where
        BS: Blockstore,
        RT: Runtime<BS>;
/// Other methods...

It actually has the Runtime and Blockstore in the interface, this seems to be too strict a requirement for interface as what ipc-gateway cares about is actually just join(params: JoinParam), Runtime and Blockstore should be implementation details. Also we should remove constructor from the interface. Actually what matters, based on current FVM implementation not sure if this will be changed, is really the method number (since we can only send raw bytes over). Then our interface is really just:

pub enum Method {
    Join = 2,
    Leave = 3,
    Kill = 4,
    SubmitCheckpoint = 5,
}

If we just need to calculate the interface id, we should just use method_num:method_name to perform hash, such as hash(2:join) XOR hash(3:leave) XOR .... We can choose this hash to be sha3 or blake2 or sth along this line. We can just calculate this value offline and hard coded into subnet actor.

Support interface method number

We will reserve a method number that is not commonly used by user-defined actor to be the support subnet actor interface checker, say 1024. It takes only raw bytes as input and returns bool only.

When subnet actor joins the the protocol, gateway actor send a cross actor call with the MethodNum: 1024 and raw bytes over to the subnet actor. If false then gateway will reject the transaction. Only when true, it would proceed.

Next Steps

With the above, the next steps are quite simple:

  • Choose a hash function and calculate the hash.
  • ipc-gateway send to join requested actor with method num 1024 and the above hash as raw bytes to check if the calling subnet actor supports the interface.

Security Concerns

I feel the above is only a voluntary check and can easily be fabricated. It can be a simple check only (maybe?). To perform a more thorough check, maybe we can try the below:

  • Oracle and whitelist based. When subnet-actor requests interact with gateway, we broadcast the request and let selected voters to perform checks and decide?

@adlrocha
Copy link
Contributor

adlrocha commented Dec 12, 2022

Hey, @cryptoAtwill. Your proposal makes sense. An improved proposal, what about:

  • We expose a is_interface method in well-defined method number (e.g. 1024 as you suggest) that actors that want to support interface identification need to implement.
    • This method accepts the list of method numbers that conform the interface the caller wants to identify.
    • The method computes the interface identifier that results for those method numbers. The identifier can be computed as the XOR of the hash of the concatenation of the varInt for the method number and the serialization of the method params for all the input methods provided in the call:
hash(varInt(1) + method1_params) XOR hash(varInt(2) + method2_params) XOR ...
  • Calling actors can get use this ID to determine if the actor implements the interface.

Is up to the actor to decide how to implement this is_interface method so it is gas efficient, but it could be something as simple as hard-coding the return value if the right input for all the interfaces supported is received, and return zero bytes otherwise. We could also perform this computation on-chain in the constructor to avoid intentional or unintentional forging of interface ids.

I would also suggest offering the convenient methods to implement this interface_id method and computing the interface id as part of fvm_utils for actors to integrate it, and then expose the subnet_actor interface as part of the ipc_gateway or wherever we choose IPC-related types to live (maybe as part of the ipc_sdk?).

Let me know if this design makes sense and I can extend a bit the description if it is not clear so we can start tinkering with a first prototype.

@sergefdrv
Copy link
Contributor

Guys, please have a look at this.

@sergefdrv
Copy link
Contributor

sergefdrv commented Dec 15, 2022

I suggest we use FIP-0042 to compute method numbers (here is a reference implementation). Then we could derive the interface ID simply XORing the method numbers (not imposing any particular order in the method set), similar to ERC-165.

@adlrocha
Copy link
Contributor

adlrocha commented Dec 15, 2022

I agree, we should use FIP0042 for method numbers, and XORing to detect that the methods are right, but this standard doesn't consider the types of the method params, right? We are trying to go one step further and not only check that the method name (and thus the method num) is correct, but also that it has the right arguments. //cc @cryptoAtwill

@sergefdrv
Copy link
Contributor

Overloading was considered in the FIP:

Overloading
We could overload method names by including a description of their parameter type in the hash payload. This is rejected for simplicity. There is little evidence that method overloading has been of great utility in, e.g., Solidity. Since payloads are expected to be IPLD structures, overloading would require defining a standard serialization of an arbitrary IPLD type schema.

Excluding the parameter type from the method names means that methods may receive dynamically typed parameter payloads. This permits a kind of overloading if some part of the payload is used for dispatch in addition to the method number. A future convention could define a standard for this.

@adlrocha
Copy link
Contributor

adlrocha commented Dec 15, 2022

Excluding the parameter type from the method names means that methods may receive dynamically typed parameter payloads. This permits a kind of overloading if some part of the payload is used for dispatch in addition to the method number. A future convention could define a standard for this.

I completely missed this. @cryptoAtwill, re. our sync discussion. You may probably use FIP0042 directly without any macro-related dance. Thank you, @sergefdrv for the pointer.

@adlrocha
Copy link
Contributor

adlrocha commented Jan 2, 2023

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants