Hi, I think unveil_flagmatch() isn't complete and/or has not the right semantic.
A bit of internals for starting (I will speak about ni_pledge, people that know what it is and how it works with pledge/unveil could go to "what is the problem" part). unveil(2) works with the syscall annotation introduced for pledge(2). When kernel needs to resolv a name to vnode, it used namei() function. For that it initializes a special structure used as namei() argument: `struct nameidata`. This struct has a field `ni_pledge` used to let vfs subsystem know what kind of syscalls called it. This way, underline subsystem doesn't have to know all syscalls, and could work on them by "group". For example, when you call open(2), depending the flags argument used, ni_pledge will contain PLEDGE_RPATH, PLEDGE_WPATH, or PLEDGE_CPATH. The subsystem has a quick and accurate view of the intented usage of the vnode. pledge(2) uses it to check that you have the promise of intent to use, and unveil(2) uses it too in a similar way, in particular in unveil_flagmatch() to check if the process has the accurate permission for this particular vnode. Now, what is the problem with unveil_flagmatch() ? If I exclude the PLEDGE_STAT stuff from the equation (I am not still convinced it is the right way to do it - but it isn't the question for now), unveil_flagmatch() has a default policy to allow the operation, and only check for some flags in ni_pledge to deny it. For simple syscall like open(2) there is no problem. The possible value of ni_pledge are composed with only PLEDGE_RPATH, PLEDGE_WPATH, or PLEDGE_CPATH. But for some others syscalls, it isn't the case. For example, chmod(2) will set ni_pledge to "PLEDGE_FATTR | PLEDGE_RPATH". For pledge(2), it means you must have "fattr" (capability to change attribute on the node) and "rpath" (capability to read the node) promise to use such syscall. As unveil(2) doesn't have the "fattr" concept, and unveil_flagmatch() will check only for PLEDGE_RPATH, PLEDGE_WPATH, PLEDGE_CPATH and PLEDGE_EXEC, we end with unsound policy: you could use chmod(2) with just "r" on unveiled part of the filesystem. Some others flags could occurs in ni_pledge: - PLEDGE_CHOWN: chown(2) family - PLEDGE_DPATH: mknod(2), mkfifo(2) - PLEDGE_FATTR: utimes(2) family, chmod(2) family, truncate(2), chflags(2) - PLEDGE_TTY: revoke(2) - PLEDGE_UNIX: bind(2), connect(2) unveil_flagmatch() has a special case for "" flag: it means deny. but as soon as you have "r", "w", "x" or "c", you have an allow policy by default. Check will be done only for a part of ni_pledge. I see basically two possibilities: - extending unveil(2) permissions to match pledge(2) flags - but I don't like it, unveil(2) should be kept simple. - having a separate namespace for unveil and pledge flags (to avoid confusion), and translating all pledge flags to unveil flags. PLEDGE_RPATH => UNVEIL_READ PLEDGE_WPATH => UNVEIL_WRITE PLEDGE_CPATH => UNVEIL_CREATE PLEDGE_EXEC => UNVEIL_EXEC PLEDGE_CHOWN => UNVEIL_WRITE PLEDGE_DPATH => UNVEIL_CREATE PLEDGE_FATTR => UNVEIL_WRITE PLEDGE_TTY => UNVEIL_WRITE PLEDGE_UNIX => UNVEIL_READ|UNVEIL_WRITE (requiring both) any others => panic(9) This way, we could be really exhaustive in unveil_flagmatch() without having to bother for future PLEDGE flag addition (as we will panic(9) if some developer doesn't add it where intented). Thanks. -- Sebastien Marie
