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

Remove port/channel capability authentication from ics 026 routed handlers #886

Closed
colin-axner opened this issue Nov 28, 2022 · 1 comment
Assignees
Labels
improvement Improvement or enhancement to make specs more comprehensible tao Transport, authentication, & ordering layer.

Comments

@colin-axner
Copy link
Contributor

colin-axner commented Nov 28, 2022

Summary

Remove all port/channel capability authentication checks in core handlers which are routed by ICS 026.

These checks do not add additional protection. ICS 026 will use the port or channel identifier to obtain the capability provided to the core handler. Then the core handler will verify the capability using the same ICS 026 module. The circular logic became evident during a code walk through.

Code example

Here is the call stack when this occurs in ibc-go.

  1. Initiate MsgChanOpenInit
  2. Obtain portCap
    - which calls capabilityKeeper.LookupModules
    - which calls GetCapability
  3. Authenticate portCap
    - which calls capabilityKeeper.AuthenticateCapability
    - which calls GetCapabilityName
    - which performs the opposite lookup as GetCapability

Thus, within practice, we are obtaining the capability via the reverse lookup (using IBC scoped keeper) and authenticating the capability using the forward lookup (using the same scoped keeper). Capability authentication should only occur when the capability provided is by another module (SendPacket for example)

Spec change example:

Remove the following check from chanOpenInit

abortTransactionUnless(authenticateCapability(portPath(portIdentifier), portCapability))

Note, I believe the spec isn't specifying how portCapability is obtained

@crodriguezvega
Copy link
Contributor

Superseded by #1133, which will remove the usage of capabilities completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement or enhancement to make specs more comprehensible tao Transport, authentication, & ordering layer.
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants