mount_fusefs: Implement the fusermount functionality

[arrowd]

Summary

The canonical application of FUSE is reimplementing some existing file system
in user space. We have NTFS and exFAT in Ports as example. These FUSE daemons
follow the same policy as the mount(8) command - the mounting is only allowed
for root, unless vfs.usermount is set 1, which poses certain security risks.

There are, however, other usages of FUSE that do not involve real file systems:

• kio-fuse, a KDE module that allows arbitrary non-KDE applications to access remote files via protocols supported by KIO (sftp, ftp, smb, etc.).
• AppImage, a "one app = one file" format for program's distribution. An AppImage is a tiny runtime code coupled with a squashfs blob that contains an actual application together with all its dependencies.
• xdg-document-portal, a D-Bus service that allows sandboxed applications to access files on the host system in a controlled way.

All these examples run as an unprivileged user, yet require mounting a FUSE
file system. As a solution, the libfuse project provides the fusermount
utility, which is a SUID variant of mount(8), but constrained to mounting
fusefs only.

On FreeBSD we already have mount_fusefs(8), which gets called even when
the libfuse code runs as root. This change implements the support necessary
for mount_fusefs to act in the "fusermount" mode:

• The program is now installed with SUID bit set.
• If we're running in the "fusermount" mode, perform various checks on the mount point.
• Add the "-u" flag to allow unmounting by unprivileged user.
• The "fusermount" mode is disabled if getuid() == 0 or vfs.usermount=1.

Test Plan

All tests were conducted on the hello_ll FUSE daemon that comes from libfuse/examples.
The chosen mount point is /tmp/mnt.
A branch with changes on the libfuse side is here: https://github.com/arrowd/libfuse/tree/kernel-bsd-auto-unmount

Tests done:

• Running as root, Ctrl+C. The FS gets unmounted as expected.
• Running as root, umount, while running. The daemon shutdowns itself as expected.
• Running as root, killall -KILL hello_ll. The mount point stays mounted as expected (no auto_unmount in play).

• Running as root with -o auto_unmount, Ctrl+C. The FS gets unmounted as expected, but then the unmounting is attempted again. This can be dangerous, see comments in the code.
• Running as root, umount, while running. The daemon shutdowns itself as expected, but also tries to unmount twice.
• Running as root, killall -KILL hello_ll. The mount point gets unmounted as expected.

• Running as unprivileged user, Ctrl+C. The FS gets unmounted as expected.
• Running as unprivileged user, mount_fusefs -u, while running. The daemon shutdowns itself as expected.
• Running as unprivileged user, killall -KILL hello_ll. The mount point stays mounted as expected (no auto_unmount in play).

• Running as unprivileged user with -o auto_unmount, Ctrl+C. The FS gets unmounted as expected. The unmounting happens twice, but we can't unmount anything except our own mount point, so this is safe.
• Running as unprivileged user with -o auto_unmount, mount_fusefs -u, while running. The daemon shutdowns itself as expected, but also tries to unmount twice.
• Running as unprivileged user with -o auto_unmount, killall -KILL hello_ll. The mount point gets unmounted as expected.

After setting sysctl vfs.usermount=1:

Tests under unprivileged user behave the same.

[arrowd]

• More TODOs

[arrowd]

I'm seriously afraid of introducing a security hole with this change, so I highlighted places in the code I'm not sure about.

A thorough review would be greatly appreciated.

sbin/mount_fusefs/mount_fusefs.c

165

What's the point in doing chdir() and then lstat(".") on a seemingly the same directory?

The original code is here: https://github.com/libfuse/libfuse/blob/22d0fcd4c757a86377bc258296e55e2900af2c3c/util/fusermount.c#L1114

391

Here we need to make sure to only allow the user to unmount the same FUSE file system that he mounted before.

This is achieved by passing the user_id=getuid() mount option during mounting, which is then get placed into f_owner field. During unmounting we compare this field against getuid().

This should be safe, because mounting is done with MNT_NOCOVER, so we should be able to accidentally unmount something else.

470

I blindly copied this from https://github.com/libfuse/libfuse/blob/22d0fcd4c757a86377bc258296e55e2900af2c3c/util/fusermount.c#L1669
Nor sure if this is needed or not.

478

fusermount also does path sanitization, but I failed to figure out what's exactly is being checked in https://github.com/libfuse/libfuse/blob/22d0fcd4c757a86377bc258296e55e2900af2c3c/lib/mount_util.c#L277

The code is cryptic to me, so I'm hoping that we're doing a correct thing already.

sys/fs/fuse/fuse_device.c

189

This looks a bit hackish and requires reviewers attention.

sys/fs/fuse/fuse_vfsops.c

447

The answer to this question depends on

1. whether it is possible for the caller to lie about user_id. From what I gather, it is not, because in usual circumstances mounting is only allowed for root and mount_fusefs in usermount mode does not allow caller to provide an arbitrary value.
2. how the f_owner is used in other places.

[asomers]

I think that this will be a nice feature. Have you considered how to test it? I think it could all be integrated into the existing fusefs test suite. Or alternatively, the negative tests at least could use atf-sh, with a suitable fuse binary installed from ports.

sbin/mount_fusefs/mount_fusefs.8

35

Don't forget to change the date.

sbin/mount_fusefs/mount_fusefs.c

131

165

Probably as a protection against TOCTOU attacks. Since that directory is specified as a path, it could be replaced after the first lstat and before the chdir. I think a better approach would be to open it first, then use fstat and fchdir. But the first lstat in fusermount.c isn't very useful, because it just checks whether the path is a directory. chdir is perfectly capable of performing that check by itself.

192

I'm not sure that strncmp has any advantage over strcmp here, since the len is fixed at compile time. If anything, you should use the length of the f_fstypename array.

sys/fs/fuse/fuse_vfsops.c

447

I think this is ok, and desirable. I don't see user_id used in very many other places, and I don't see any problems.

[kib]

As was discussed elsewhere, fuse server which times out the responses could cause lock cascades in VFS. This would have global consequences for the whole system.

Until the vnodes are locked around communication with userspace, I do not think this change is appropriate.