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.

[kib]

sbin/mount_fusefs/mount_fusefs.c

93

Look at style(9) how the multiline comment should be formatted.

/*
  * text
  */

126

139

You must check that syscalls did not failed.

158

mnt_fd declaration should go into the corresponding block.
Do you want to set O_CLOEXEC there?

159

Blank lines are not needed.

180

static const char *fs_allowlist

Also should go into the decl block.

193

This is too fragile. I suggest to explicitly compare with "tmpfs" after the strcmp loop.

195

197

Does mnt_fd leak?

244

Why not bool?

251

Wrong indent.

397

'{' should be on the previous line

399

Blank line is needed after the block of locals declarations.

[arrowd]

• Address comments

[asomers]

In D56165#1285144, @kib wrote:

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.

I think kib means "unlocked around communication with userspace". But I do not see how such a change could possibly work.

Another thing: what is the rationale for prohibiting the usermount feature from working on top of certain file system types?

tests/sys/fs/fusefs/auto_unmount.sh

34

These two functions look at the global mount table. So they might fail if two tests are running in parallel. To fix that, you must mark the tests as exclusive. Or better yet, change these functions to they only look at a known mount point, with a command like df -T mnt.

44

I suggest unmount -f.

62

Using killall will also interfere with running tests in parallel. Is there anyway to kill by PID instead?

63

Instead of hard-coding a 1-second sleep, could you put a polling loop around df -T mnt?

tests/sys/fs/fusefs/usermount.sh

54

Instead of su -m nobody below, can you set require.user unprivileged here?

103–104

138

Is this part forbidden even if vfs.usermount=1 ? If not, then you should skip the test if that setting is true.

237

268

It would be very bad to do this in the cleanup if that setting wasn't in use system-wide before the test started.

[kib]

In D56165#1334568, @asomers wrote:

In D56165#1285144, @kib wrote:

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.

I think kib means "unlocked around communication with userspace". But I do not see how such a change could possibly work.

Fuse needs to make some way to ensure liveness of the inode (not vnode) around calls to the userspace.

Another thing: what is the rationale for prohibiting the usermount feature from working on top of certain file system types?

BTW, what prevents a malicious userspace to try to sneak a mount with a type outside the allowed list after the check, but before the mount?

[arrowd]

tests/sys/fs/fusefs/auto_unmount.sh

34

In the usermount.sh tests I do mount | grep nosuid to check that the FS is mounted with nosuid. It seems it isn't possible to do with df -T and this makes the whole test exclusive?

[arrowd]

Another thing: what is the rationale for prohibiting the usermount feature from working on top of certain file system types?

I don't know, to be honest. I followed what Linux fusermount does.
I tried searching for rationale, but the only clue I got is Linux overlay mounts.

[asomers]

In D56165#1334631, @kib wrote:

In D56165#1334568, @asomers wrote:

In D56165#1285144, @kib wrote:

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.

I think kib means "unlocked around communication with userspace". But I do not see how such a change could possibly work.

Fuse needs to make some way to ensure liveness of the inode (not vnode) around calls to the userspace.

Now I'm really confused. Are you saying that you want fusefs to hold the vnode lock, but yet allow other fusefs threads to access the same inode?

tests/sys/fs/fusefs/auto_unmount.sh

34

How about mount | grep "$MOUNTPOINT.*nosuid"?

[arrowd]

• Address comments

[arrowd]

In D56165#1334631, @kib wrote:

BTW, what prevents a malicious userspace to try to sneak a mount with a type outside the allowed list after the check, but before the mount?

Sneaking mount requires mounting or unmounting something, which means it should either be root or vfs.usermount be enabled. There is no sense to guard against root and when vfs.usermount is set to 1 the FUSE usermount feature is disabled.

The checking itself is sound, but it is still unclear why it is needed. I'd leave it in place, because it is always easier to disable some unneeded security measures than enabling them later when we bump into some security problem.

sbin/mount_fusefs/mount_fusefs.c

158

I now close this fd in the same function, so I guess O_CLOEXEC would be useless.

tests/sys/fs/fusefs/usermount.sh

54

Turns out this doesn't work, because we need to unmount in the cleanup and with require.user unprivileged the cleanup step is also ran as unprivileged user.

138

All these tests expect vfs.usermount=0 at their start. Should I put this assertion at the beginning of each test and also add allow_sysctl_side_effects to each test?

[asomers]

tests/sys/fs/fusefs/auto_unmount.sh

33

There needs to be some kind of sleep here, and in the other loop too.

tests/sys/fs/fusefs/usermount.sh

32

You removed the exclusivity requirement in common_cleanup . Can you do it here and in check_mounted too?

54

Ahh, that makes sense. Being so, I suggest you use user tests instead of nobody. That's what it's for.

138

You should definitely not assume that. You should skip at the beginning of each test if that sysctl is already 1. And yes, you should add allow_sysctl_side_effects too.

[arrowd]

• Address comments

[arrowd]

tests/sys/fs/fusefs/usermount.sh

32

Sorry, I failed to parse this. What do you want me to do?

[asomers]

tests/sys/fs/fusefs/usermount.sh

32

I want you to do exactly what you've already done, because I'm stupid and didn't read the code correctly when last I reviewed it.

208

This test doesn't need this setting, because it doesn't change any sysctls.

[arrowd]

tests/sys/fs/fusefs/usermount.sh

208

You said

And yes, you should add allow_sysctl_side_effects too.

My understanding is: since all these tests depend on vfs.usermount being 0 during the whole test, they must be marked allow_sysctl_side_effects, so that no any other test would change this sysctl.

[asomers]

tests/sys/fs/fusefs/usermount.sh

208

Not quite. Tests only need to set allow_sysctl_side_effects if they change the setting. They can also depend on it having one value, yet not change it.

[arrowd]

• Remove the allow_sysctl_side_effects requirement from tests that don't need it

tests/sys/fs/fusefs/usermount.sh

268

(this comment was about userunmount_negative_cases)

The test now gets skipped if vfs.usermount != 0. This means it should be safe to keep sysctl vfs.usermount=0 || true in the cleanup?

[asomers]

sbin/mount_fusefs/mount_fusefs.8

283

"could not" is past tense, and doesn't make sense here. Use "may not", "should not", or something like that.

sbin/mount_fusefs/mount_fusefs.c

250

Technically, this is a TOCTOU. You check the value of the sysctl here, but it might change before you mount or unmount anything later on. However, I don't think it will do damage. AFAICT the worst that will happen is that mount_fusefs -u might fail to unmount a mountpoint, if vfs.umount transitioned from 1 to 0 while the program was executing.

407–418

This is another TOCTOU. You stat the mountpoint on line 407, but don't use that information until line 418. The mountpoint may have changed in the meantime. I think the only safe way to do an operation like this would be to either do the permission check in the kernel, or implement some kind of unmountat syscall.

This TOCTOU is exploitable, too. For example:

• mkdir /tmp/mymountpoint
• ln -s /tmp/mymountpoint /tmp/mymountlink
• fuse-hello_ll /tmp/mymountpoint
• mount_fusefs -u /tmp/mymountpoint
  • statfs follows the symlink and returns information about /tmp/mymountpoint
  • after statfs returns, the user does ln -s / /tmp/mymountlink
• The checks pass, so mount_fusefs calls restore_privs, and then proceeds to unmount / with root privileges.

591

Another TOCTOU here. check_perm works by pathname, and it follows symlinks. So an attacker could create a symlink pointing to a directory that will satisfy the checks in check_perm. Then, after check_perm succeeds, he replaces that symlink with one point to somewhere else. Then the following nmount call will mount over top of that.

Using nocover and emptydir helps, by preventing the attacker from overwriting /etc. But those aren't good enough. For example, the attacker could use this method to mount over /usr/local/etc/some_daemon/ , if that directory were previously empty, providing his own config file for some_daemon.

Disabling allow_other and allow_root helps a lot. But it would still be possible for an attacker to replace a directory that should be empty with one that returns EPERM on access. So I'm still nervous.

672–673

You should document -o auto_unmount here, and maybe -u too.

sys/fs/fuse/fuse_device.c

181–192

If dounmount fails, you should restore mnt_cred->cr_uid

sys/fs/fuse/fuse_vfsops.c

446

Should we restrict the ability to set user_id to privileged processes?

tests/sys/fs/fusefs/auto_unmount.sh

32

You can simplify this expression.

37–38

You need to sleep within the loop, not after it.

41

And simplify here too.

46–47

You need to sleep within the loop, not after it.

69–71

Trying to check the existence of $pid after killing it is unreliable. The pid might still exist even though you killed if either:

• It hasn't been reaped yet, or
• A new process has been spawned with that same pid.

tests/sys/fs/fusefs/usermount.sh

79–81

163

If you think it's worth the effort, you could eliminate the special MOUNT_FUSEFS_TESTING code by using tarfs in this test.

304

If it's going to change global settings, it had better be exclusive.

420

If it's going to change global settings, it had better be exclusive.

435

You can delete this line, because it's redundant with the cleanup.

[asomers]

tests/sys/fs/fusefs/auto_unmount.sh

53

Simplify

58

Simplify

63

tests/sys/fs/fusefs/usermount.sh

53

Simplify

64–66

[arrowd]

• Address comments

[arrowd]

sbin/mount_fusefs/mount_fusefs.c

250

As far as I gather, yes. There are 4 combinations of these 2 conditions:

• We're mounting/unmounting
• We're starting with vfs.usermount=0/1 that later gets changed to vfs.usermount=1/0

Going through all 4 combinations:

1. (mounting, vfs.usermount=0->1). The mounting operation will [unexpectedly] succeed and it will skip additional usermount checks. But this is exactly the same when vfs.usermount=1 all the time.
2. (mounting, vfs.usermount=1->0). This will disable usermount mode and the mounting operation will [unexpectedly] fail without negative effects.
3. (unmounting, vfs.usermount=0->1). No effect on the operation.
4. (unmounting, vfs.usermount=1->0). We will get the "unmount flag only makes sense for usermount" error.

So yeah, this TOCTOU seems harmless. Should I put a comment into the code?

407–418

Thank you, that's a great catch. I hopefully fixed it by performing the unmounting by FSID.

sys/fs/fuse/fuse_vfsops.c

446

I was under impression that this is already the case. If we reached fuse_vfsop_mount then the caller is privileged enough to call nmount. Am I wrong here?

tests/sys/fs/fusefs/usermount.sh

163

That didn't work. tarfs is readonly, just like cd9660, so I get into "no write access to mountpoint" error before it checks fstype.

[arrowd]

sbin/mount_fusefs/mount_fusefs.c

591

Ugh, that's a valid concern indeed. Maybe we could teach nmount to mount by a directory fd? Otherwise, I don't know how to avoid this vulnerability.

[kib]

sbin/mount_fusefs/mount_fusefs.c

591

Yes, I pointed it out before as well. I do not think there is any way to avoid the problem other than to do the check after the future covered vnode is locked in the vfs_domount(). If really wanted to fix it, you can pass a vector of allowed fs types and required permissions as the iov arg to nmount().

I am not completely sure that we want to do this, but it is the only way.

[kib]

sbin/mount_fusefs/mount_fusefs.c

591

I realized that this can be significantly simplified. Instead of passing the checks to kernel, you could retrieve the fsid (see struct statfs f_fsid member) for the mount where the covered vnode belongs. Then pass this single parameter to the nmount(2). Keep the permission checks in usermode.

Kernel would re-check that the resolved covered vnode belongs to the mount point with the passed fsid. This closes the race.