Incomplete object-capability system

[hu55a1n1]

Summary of Bug

The object-cap system in the IBC module is incomplete and needs improvement. The idea is to have the handlers receive the capabilities that are obtained using the lookup_module_by_{port,channel}() methods.

Version

master

Acceptance Criteria

• Handlers must be able to authenticate obj-caps.
• Make it easier for users to get type-safe obj-caps from the capability system. without having to deal with capability identifiers.
• Update mock tests

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate milestone (priority) applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[hu55a1n1]

Here's a minimal impl that leverages Rust's type system and visibility. The two most important properties of object capabilities are non-forgeability and ability to uniquely associate/identify a resource. ->

pub mod ibc {
    // Use atomics for static mut cap index
    use core::sync::atomic::AtomicUsize;
    static CAP_INDEX: AtomicUsize = AtomicUsize::new(0);

    // Define capability trait that cannot be implemented by types outside this module
    pub trait Capability: private::Sealed {
        fn index(&self) -> usize;
    }
    mod private {
        pub trait Sealed {}
    }

    // blanket impls so we don't have to impl Capability for ref types
    impl<T: Capability> Capability for &'_ T {
        fn index(&self) -> usize {
            (**self).index()
        }
    }
    impl<T: Capability> private::Sealed for &'_ T {}

    // A resource that the capability provides access to. eg. Channel/Port path
    // This must be serializable to a string so it can be persisted on storage.
    pub trait Resource: ToString {}

    #[derive(Debug)]
    pub struct Error;

    pub trait CapabilityKeeper {
        /// Store a new capability with the given name.
        /// Return an error if the capability was already taken.
        fn new_capability<C: Capability>(
            &mut self,
            _name: impl Resource,
            capability: Option<C>,
        ) -> Result<C, Error> {
            Ok(capability.unwrap())
        }

        /// Claim the specified capability using the specified name.
        /// Return an error if the capability was already taken.
        fn claim_capability(&mut self, _name: impl Resource, _capability: &impl Capability) {}

        /// Release a previously claimed or created capability
        fn release_capability(&mut self, _name: impl Resource, _capability: impl Capability) {}
    }

    pub trait CapabilityReader {
        /// Fetch a capability which was previously claimed by specified name
        fn get_capability<C: Capability>(&self, _name: &impl Resource) -> Result<C, Error> {
            todo!()
        }

        /// Authenticate a given capability and name. Lookup the capability from the internal store and
        /// check against the provided name.
        fn authenticate_capability(
            &self,
            _name: &impl Resource,
            _capability: &impl Capability,
        ) -> Result<(), Error> {
            Ok(())
        }
    }

    pub mod channel {
        // A private concrete capability type.
        struct ChannelCapability(usize);
        impl super::private::Sealed for ChannelCapability {}
        impl super::Capability for ChannelCapability {
            fn index(&self) -> usize {
                self.0
            }
        }

        // The resource that is protected by this capability.
        struct PortId(String);
        struct ChannelId(String);
        // "capabilities/ports/{port-id}/channels/{channel-id}"
        pub struct ChannelCapabilityPath(PortId, ChannelId);
        impl core::str::FromStr for ChannelCapabilityPath {
            type Err = ();
            fn from_str(_: &str) -> Result<Self, Self::Err> {
                Ok(Self(
                    PortId("transfer".to_string()),
                    ChannelId("chan-0".to_string()),
                ))
            }
        }
        impl ToString for ChannelCapabilityPath {
            fn to_string(&self) -> String {
                todo!()
            }
        }
        impl super::Resource for ChannelCapabilityPath {}

        // A handler method that creates a new capability
        pub fn chan_open_init(ck: &mut impl super::CapabilityKeeper) -> impl super::Capability {
            use core::sync::atomic::Ordering;
            use std::str::FromStr;

            let index = super::CAP_INDEX.fetch_add(1, Ordering::Relaxed);
            let chan_cap = ChannelCapability(index);
            let path =
                ChannelCapabilityPath::from_str("capabilities/ports/transfer/channels/chan-0}")
                    .unwrap();
            let chan_cap = ck.new_capability(path, Some(chan_cap)).unwrap();
            chan_cap
        }

        // handler method that checks that the caller has access to or owns the capability
        pub fn chan_use<C: super::Capability + 'static>(
            cr: &impl super::CapabilityReader,
            chan_cap: C,
        ) {
            use core::any::TypeId;
            use std::str::FromStr;

            if TypeId::of::<C>() != TypeId::of::<ChannelCapability>() {
                panic!("Not allowed");
            }
            let path =
                ChannelCapabilityPath::from_str("capabilities/ports/transfer/channels/chan-0}")
                    .unwrap();
            cr.authenticate_capability(&path, &chan_cap).unwrap();
            // use channel
        }
    }
}

pub mod application {
    // Untrusted code that is using the capability system
    pub fn on_chan_open_init<CKR: super::ibc::CapabilityKeeper + super::ibc::CapabilityReader>(
        ckr: &mut CKR,
        chan_cap: impl super::ibc::Capability + 'static,
    ) {
        use std::str::FromStr;

        let path = super::ibc::channel::ChannelCapabilityPath::from_str(
            "capabilities/ports/transfer/channels/chan-0}",
        )
        .unwrap();
        ckr.claim_capability(path, &chan_cap);

        // This code cannot create capabilities! 
        // struct UserDefinedCapability;
        // impl super::ibc::Capability for UserDefinedCapability {
        // fn index(&self) -> usize {
        // 		0
        // 	}
        // }
        // impl super::ibc::private::Sealed for UserDefinedCapability {}

        // It also cannot forge capabilties using unsafe code.
        // let _cap = unsafe {
        // 	std::mem::transmute::<(), ibc::channel::ChannelCapability>(())
        // };

        super::ibc::channel::chan_use(ckr, chan_cap);
    }
}

fn main() {
    struct OCapSys;
    impl ibc::CapabilityReader for OCapSys {}
    impl ibc::CapabilityKeeper for OCapSys {}

    let mut ocap_sys = OCapSys;
    let chan_cap = ibc::channel::chan_open_init(&mut ocap_sys);
    application::on_chan_open_init(&mut ocap_sys, chan_cap);
}

[hu55a1n1]

Here's a minimal impl that leverages Rust's type system and visibility.

Noticed a flaw in the design while trying to implement this. The CapabilityReader::get_capability() method needs to know the concrete capability type while deserializing it from the store, but the security model is based on the assumption that the concrete type is kept private and inaccessible. I think one way to solve this would be to use downcasting with core::any::Any. Another radically different solution would be to model the API such that it is clear that the Capability{Keeper,Reader}s for the core and apps must be different types.

[plafer]

From what I understand, there are 2 main ways to implement capabilities at the programming language level: either by using

1. references: the only way for modules/functions to access a restricted object is if it is given a reference to it,
2. a secret: the module/function is given a secret, and every time it requests access to a restricted object, it must give that secret.

I don't think using references is a viable option in Rust because they are forgeable; unsafe rust lets you instantiate references at any address and do pointer arithmetic. Therefore what is left for us is using a secret. And I think we can do it similar to how ibc-go does it, except we don't return a reference to a dummy object as the capability but rather we return a randomly generated number using a reliable source of randomness.

What do you think?

EDIT: Thinking about it some more, I'm really not sure how we'll implement "in-process" capabilities in Rust because the language technically lets you access the whole address space from anywhere (due to unsafe rust features), and so I don't see how we can ever be certain that a hacker won't figure out a way to find the secret somewhere in memory. It can even potentially be quite easy given knowledge of the memory allocator algorithm. The only way I can currently think of would be to run untrusted modules in their own process...

[adizere]

Copying replies from Slack

Marko: This doesn't sound like an issue because the attacker would need to upgrade the chain too

Carlos: Right. We work under the assumption that modules deployed on a chain are not malicious, correct?

Marko: Yes, the modules can be malicious but then the developer is making a conscious decision to do so and we can't prevent that

Sam: VM modules need special consideration ofc

[romac]

@hu55a1n1 Shall we close this now that we've decided not go forward with the OCap model for the time being?