Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow host chain to limit ICA functionality #511

Closed
3 tasks
seantking opened this issue Nov 1, 2021 · 8 comments
Closed
3 tasks

Allow host chain to limit ICA functionality #511

seantking opened this issue Nov 1, 2021 · 8 comments

Comments

@seantking
Copy link
Contributor

Summary

At present, an interchain account can execute any sdk message sent by the Controller chain, so long as the Host chain supports this functionality. During requirement gathering, several high-profile chains made it clear that they will not use interchain accounts unless there is built-in functionality for a host chain to specify a list of SDK messages that an interchain account cannot execute.

This is briefly mentioned in the specification but has not been outlined clearly with regards to the implementation details. I think this is open to discussion. My initial thoughts are that we can simply pass in a list of message types in app.go to the ICA keeper initialization as an optional parameter. Any SDK messages in this list will be not be executed by the host chain.

This is probably a nice to have but I think it is a security feature that a lot of people will want from day one.

@AdityaSripal @colin-axner @damiannolan @crodriguezvega thoughts?


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@AdityaSripal
Copy link
Member

Your proposal makes sense to me. Is the only request to bar messages based on message type? We don't need any more granular restrictions correct?

@seantking
Copy link
Contributor Author

Great point. The requests so far have just been based on the message type. I think anything more granular than that can be left for future versions (if it gets requested).

@AdityaSripal
Copy link
Member

Cool, makes sense to me. Your solution seems simple enough. This should just be passed in on the host side

@colin-axner
Copy link
Contributor

colin-axner commented Nov 1, 2021

Can we just make this an on-chain parameter? Otherwise we require coordinated upgrades to modify this list. The parameter can be a list of strings which are the proto URLs of the message types

@seantking
Copy link
Contributor Author

Agree with @colin-axner.

Reference for whoever picks this up: https://github.com/cosmos/ibc-go/blob/main/modules/apps/transfer/types/params.go

@andrey-kuprianov
Copy link

From the security perspective, I would make this feature to use a whitelist instead of a blacklist, i.e. to allow the host chain to specify a list of messages that it allows to execute.

Otherwise, in the blacklist approach (when a host chain specifies a list of SDK messages that an interchain account cannot execute), some new messages are bound to be added in the future, that the host chain will find undesirable, but which will slip through the blacklist sieve. This, on the one hand, can be a source of security vulnerabilities, on the other -- will probably require much more frequent updates to the list of unwanted messages.

@seantking
Copy link
Contributor Author

@andrey-kuprianov thanks for your input. @AdityaSripal voiced a similar concern lately. I agree with your approach 🤝 We will make this a whitelist rather than a blacklist.

@damiannolan
Copy link
Contributor

closed by #576

@crodriguezvega crodriguezvega moved this to Done in ibc-go Dec 30, 2021
faddat pushed a commit to notional-labs/ibc-go that referenced this issue Mar 1, 2022
* typo fix

* typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

5 participants