> On Oct 11, 2024, at 2:22 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Fri, Oct 11, 2024 at 4:24 PM Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote: >> >> >> >>> On Oct 11, 2024, at 5:00 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >>> >>> Christian, >>> >>> These patches bring the NFS connectable file handles feature to >>> userspace servers. >>> >>> They rely on your and Aleksa's changes recently merged to v6.12. >>> >>> This v4 incorporates the review comments on Jeff and Jan (thanks!) >>> and there does not seem to be any objection for this new API, so >>> I think it is ready for staging. >>> >>> The API I chose for encoding conenctable file handles is pretty >>> conventional (AT_HANDLE_CONNECTABLE). >>> >>> open_by_handle_at(2) does not have AT_ flags argument, but also, I find >>> it more useful API that encoding a connectable file handle can mandate >>> the resolving of a connected fd, without having to opt-in for a >>> connected fd independently. >>> >>> I chose to implemnent this by using upper bits in the handle type field >>> It may be that out-of-tree filesystems return a handle type with upper >>> bits set, but AFAIK, no in-tree filesystem does that. >>> I added some warnings just in case we encouter that. >>> >>> I have written an fstest [4] and a man page draft [5] for the feature. >>> >>> Thanks, >>> Amir. >>> >>> Changes since v3 [3]: >>> - Relax WARN_ON in decode and replace with pr_warn in encode (Jeff) >>> - Loose the macro FILEID_USER_TYPE_IS_VALID() (Jan) >>> - Add explicit check for negative type values (Jan) >>> - Added fstest and man-page draft >>> >>> Changes since v2 [2]: >>> - Use bit arithmetics instead of bitfileds (Jeff) >>> - Add assertions about use of high type bits >>> >>> Changes since v1 [1]: >>> - Assert on encode for disconnected path (Jeff) >>> - Don't allow AT_HANDLE_CONNECTABLE with AT_EMPTY_PATH >>> - Drop the O_PATH mount_fd API hack (Jeff) >>> - Encode an explicit "connectable" flag in handle type >>> >>> [1] https://lore.kernel.org/linux-fsdevel/20240919140611.1771651-1-amir73il@xxxxxxxxx/ >>> [2] https://lore.kernel.org/linux-fsdevel/20240923082829.1910210-1-amir73il@xxxxxxxxx/ >>> [3] https://lore.kernel.org/linux-fsdevel/20241008152118.453724-1-amir73il@xxxxxxxxx/ >>> [4] https://github.com/amir73il/xfstests/commits/connectable-fh/ >>> [5] https://github.com/amir73il/man-pages/commits/connectable-fh/ >>> >>> Amir Goldstein (3): >>> fs: prepare for "explicit connectable" file handles >>> fs: name_to_handle_at() support for "explicit connectable" file >>> handles >>> fs: open_by_handle_at() support for decoding "explicit connectable" >>> file handles >>> >>> fs/exportfs/expfs.c | 17 ++++++++- >>> fs/fhandle.c | 75 +++++++++++++++++++++++++++++++++++--- >>> include/linux/exportfs.h | 13 +++++++ >>> include/uapi/linux/fcntl.h | 1 + >>> 4 files changed, 98 insertions(+), 8 deletions(-) >>> >>> -- >>> 2.34.1 >>> >> >> Acked-by: Chuck Lever <chuck.lever@xxxxxxxxxx <mailto:chuck.lever@xxxxxxxxxx>> >> >> Assuming this is going directly to Christian's tree. >> >> I'm a little concerned about how this new facility might be >> abused to get access to parts of the file system that a user >> is not authorized to access. > > That's exactly the sort of thing I would like to be reviewed, > but what makes you feel concerned? > > Are you concerned about handcrafted file handles? Yes; a user could construct a file handle that could bypass the usual authorization checks when it gets connected. It's a little hare-brained and hand-wavy because this is a new area for me. > Correct me if I am wrong, but I think that any parts of the filesystem > that could be accessed (by user with CAP_DAC_READ_SEARCH) > using a handcrafted connectable file handle, could have also been > accessed by the parent fid part before, so I do not see how connectable > file handles create new ways to get access? > >> But follow-up comments from Amir >> suggest that (with the current code) it is difficult or >> impossible to do. >> >> Are there self-tests or unit-tests for exportfs? > > There are fstests, particularly, the "exportfs" test group > and I added this one for connectable file handles: > > [4] https://github.com/amir73il/xfstests/commits/connectable-fh/ > > Did you mean another form of unit tests? No, actually; fstests wfm. That's something I could set up via kdevops and run on a regular basis. -- Chuck Lever
