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

Clarification about ICA message signing requirements #3494

Closed
0xekez opened this issue Apr 19, 2023 · 3 comments
Closed

Clarification about ICA message signing requirements #3494

0xekez opened this issue Apr 19, 2023 · 3 comments
Labels
27-interchain-accounts question Further information is requested

Comments

@0xekez
Copy link

0xekez commented Apr 19, 2023

Moved from here: #3434 (comment)

ICS-027's spec seems to say that messages must be signed by the sender. I may be misunderstanding the SDK, but this does not seem to be the case to me.

To authenticate a TX in the ICA implementation, the signers are looped over and checked:

for _, signer := range msg.GetSigners() {
if interchainAccountAddr != signer.String() {
return errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "unexpected signer address: expected %s, got %s", interchainAccountAddr, signer.String())
}
}

This confuses me for two reasons:

  1. From what I have been told online, SDK modules can send ICA messages. SDK modules do not have keypairs (afik) so if this did indeed require a signed message, it seems to me that a module could not send ICA messages.
  2. My assumption is then that when a SDK module sends a message, the signers list is empty, so this loop is never entered.

It feels to me then that one of two things are true:

  1. SDK modules (and by extension smart contracts) can not send messages over ICA, or
  2. The signing verification logic is easily bypassed by sending a message with no signers.

I think its entirely possible that I'm misunderstanding something here so would love to be corrected. My question is basically: how can ICA both require that the sent message is signed by the account owner, and allow SDK modules / smart contracts to send messages (as modules and contracts don't have keypairs)?

Thanks for the help!

@crodriguezvega
Copy link
Contributor

crodriguezvega commented Apr 19, 2023

If the owner of the interchain account is a regular user account, then the transaction with MsgSendTx should be signed with the private key of the owner (and the same owner address should be filled in the field Owner of MsgSendTx). But ibc-go is not checking this; this is all verified by the SDK.

But if the owner is a generic authentication module like x/gov or x/group then the owner address that is filled in MsgSendTx is either the governance module address or a group policy address, respectively. In this case, the MsgSendTx included in the governance or group proposal is not signed with a private key, but the Owner address should be filled in with the owner of the interchain account (and this is the address returned by GetSigners implementation of MsgSendTx).

We have this tutorial here that shows how you can use x/group to register an interchain account and send messages to it. Basically you would set up a group policy and the group policy address becomes the owner of the interchain account address. Then you can submit proposals on the controller chain to execute MsgSendTx. The MsgSendTx in the proposal contains a list of messages to execute on the host chain (encoded in PacketData). In the tutorial you will see that the proposal includes one MsgSendTx where the owner is the group policy address. And in the PacketData of this MsgSendTx there is an x/bank MsgSend encoded where the FromAddress is filled in with the interchain account address, so that the validation that you link above passes (because GetSigners implementation of MsgSend returns the address in FromAddress).

We also have these 2 tests that show how x/gov and x/group can be used with ICA here and here.

The signing verification logic is easily bypassed by sending a message with no signers.

This is a good question. I am not sure if there's any validation in the SDK to make sure that GetSigners should not return an empty slice... But, assuming no malicious message implementations that return an empty slice, the messages (encoded in PacketData of MsgSendTx), even if they are sent by executing a proposal submitted by x/gov or x/group, should return the interchain account address when GetSigners is called.

Maybe the wording used in the spec is not exactly how it is implemented in ibc-go (i.e. the message to be executed by the interchain account is not signed by a private key), but the spec tries to be generic enough so that implementations can decide how to do the verification of the "signer" of the message. In the case of ibc-go we use SDK's GetSigners function, but other implementations might do something different.

I hope this helps! Let us know if you have further questions.

@crodriguezvega crodriguezvega added question Further information is requested 27-interchain-accounts labels Apr 23, 2023
@crodriguezvega
Copy link
Contributor

@0xekez Has the comment above helped to answers your questions? If it has, can we close this issue then?

@crodriguezvega
Copy link
Contributor

I will close the issue for now, @0xekez. Feel free to reopen if your question was not answered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
27-interchain-accounts question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants