Ideas for a higher level notification API

[rusty-snake]

Discussed in #163

^{Originally posted by rusty-snake June 5, 2022}
This are my experiments for a saner unotify API.

The API will be build around the notify-fd. Currently you pass the fd as an integer to the receive/response function. This has little type safety and is a bit odd because you can only have one notify fd.

use std::error::Error as StdError;
use crate::{
    notify_id_valid, Result, ScmpArch, ScmpFd, ScmpNotifReq, ScmpNotifResp, ScmpSyscall,
    NOTIF_FLAG_CONTINUE,
};

//#[derive(Debug)]
pub struct Rotify {
    fd: ScmpFd,
    actions: Vec<RotifAction>, // ignore this filed for now
}

You then use receive/response methods on this type:

impl Rotify {
    pub fn receive(&self) -> Result<RotifReq> {
        let req = ScmpNotifReq::receive(self.fd)?;
        Ok(RotifReq {
            id: RotifCookie(req.id, self.fd),
            pid: RotifPid(req.pid),
            data: RotifData {
                syscall: req.data.syscall,
                arch: req.data.arch,
                instr_pointer: req.data.instr_pointer,
                args: req.data.args,
            },
        })
    }

    // or send_response()?
    pub fn respond(&self, id: RotifCookie, resp: RotifResp) -> Result<()> {
        match resp {
            RotifResp::Continue => ScmpNotifResp::new(id.0, 0, 0, NOTIF_FLAG_CONTINUE),
            RotifResp::Val(val) => ScmpNotifResp::new(id.0, val as i64, 0, 0),
            RotifResp::Error(error) => ScmpNotifResp::new(id.0, 0, -(error as i32), 0),
        }
        .respond(self.fd)
    }
}

The response type is an enum instead of an struct with logical invariants:

#[derive(Debug)]
#[non_exhaustive]
pub enum RotifResp {
    Continue,
    Val(i64),   // Are negative values supported? If not change this to u32.
    Error(u16), // u16 -> i32 -> *= -1
}

The request type looks close to the current one but the types of the fields are structs. This allows more type-safety and to implement helper methods.

#[derive(Debug)]
#[non_exhaustive]
pub struct RotifReq {
    pub id: RotifCookie,
    pub pid: RotifPid,
    pub data: RotifData,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct RotifCookie(u64, ScmpFd);
impl RotifCookie {
    pub fn is_valid(&self) -> bool {
        notify_id_valid(self.1, self.0).is_ok()
    }

    pub fn ensure_valid(&self) -> std::result::Result<(), &str> {
        if self.is_valid() {
            Ok(())
        } else {
            Err("This notification id has been invalidated.")
        }
    }
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct RotifPid(u32);
/*
impl RotifPid {
    fn helper_functions_like() -> File {
        let mem = File::open(&format!("/proc/{}/mem", self.0))?;
        self.id.is_valid().then(|| Some(mem))
    }
}
*/
// PartialEq<u32>; From<u32>; Into<u32>

#[derive(Debug)]
pub struct RotifData {
    pub syscall: ScmpSyscall,
    pub arch: ScmpArch,
    pub instr_pointer: u64,
    pub args: [u64; 6],
}

---

As a second step the usual "receive, take action and decide, send response"-loop can be abstracted too:

impl Rotify {
    // todo: cancelable
    pub fn receive_and_handle_loop<F>(&self, mut handle_error: F)
    where
        F: FnMut(&mut dyn StdError)
    {
        loop {
            if let Err(mut err) = self.receive_and_handle() {
                handle_error(&mut err);
            }
        }
    }
    
    fn receive_and_handle(&self) -> Result<()> {
        let req = self.receive()?;
        for action in &self.actions {
            if action.should_handle(&req) {
                if let Some(resp) = (action.func)(&req) {
                    self.respond(req.id, resp)?;
                }
            }
        }

        Ok(())
    }

    pub fn add_action<T, F>(&mut self, filter: Option<T>, f: F) -> &mut Self
    where
        //T: RotifFilter + 'static,
        T: Into<Box<dyn RotifFilter>>,
        F: Fn(&RotifReq) -> Option<RotifResp> + 'static,
    {
        self.actions.push(RotifAction {
            //filter: filter.map(|fltr| Box::new(fltr) as _),
            filter: filter.map(Into::into),
            func: Box::new(f),
        });
        self
    }
}

//#[derive(Debug)]
pub struct RotifAction {
    filter: Option<Box<dyn RotifFilter>>,
    func: Box<dyn Fn(&RotifReq) -> Option<RotifResp>>,
}
impl RotifAction {
    fn should_handle(&self, req: &RotifReq) -> bool {
        if let Some(filter) = &self.filter {
            filter.should_handle(req)
        } else {
            true
        }
    }
}

pub trait RotifFilter {
    fn should_handle(&self, req: &RotifReq) -> bool;
}
impl RotifFilter for ScmpSyscall {
    fn should_handle(&self, req: &RotifReq) -> bool {
        *self == req.data.syscall
    }
}
impl RotifFilter for &[ScmpSyscall] {
    fn should_handle(&self, req: &RotifReq) -> bool {
        self.contains(&req.data.syscall)
    }
}
impl<F> RotifFilter for F
where
    F: Fn(&RotifReq) -> bool,
{
    fn should_handle(&self, req: &RotifReq) -> bool {
        (self)(req)
    }
}

---

---

---

• io_safety
• Add a ScmpNotifCookie type #152

[rusty-snake]

Note: io_safety for ScmpFd.

[rusty-snake]

The correct type for ScmpFd would be BorrowedFd<'static> I think. Even if 'static is wrong, it is a BorrowedFd with a long lifetime.

[ManaSugi]

The correct type for ScmpFd would be BorrowedFd<'static> I think. Even if 'static is wrong, it is a BorrowedFd with a long lifetime.

It means that for safety you want to prevent users from accidentally closing the fd, isn't it?

[rusty-snake]

It we use OwnedFd it will be closed when OwnedFd gets dropped. Which would be the wrong think for a shared FD.

[ManaSugi]

Okay, is it necessary for us to make the lifetime 'static?
I think there seems to be no problem even if the lifetime is same as struct Rotify.

[not-jan]

Hi, I've been working on a similar functionality and since I'm depending on tokio I've published it as an external crate: https://github.com/not-jan/seccomp-stream

Maybe you can take some inspiration / ideas from my implementation. I ended up implementing a non-blocking version as opposed to the normal blocking version by utilizing epoll.

If epoll signals that the fd is readable it guarantees that the ioctl won't block. This way you can also react to the fd closing because epoll will send a EPOLLHUP event.

Most of the actual epoll implementation is hidden within tokio / mio so it might not be super easy to extract this.