Mount control

[l0kod]

To avoid filesystem (FS) security policy bypass, a landlocked process with FS restrictions cannot do any FS topology changes (see d722036), which include any mount calls.

Even with FS restrictions, it would be useful for some use cases to be able to safely do new mount and umount.

The main issue I see is that we may want to allow a set of accesses on the newly mount points, independently from the existing path_beneath rules because the new mount point would overlap part of the initial file hierarchy. For this reason, I think we could have a new type of rule dedicated to mount access rights, something like LANDLOCK_RULE_MOUNT. With a dedicated attr struct, probably with landlock_path_beneath_attr's equivalent fields, we'll be able to configure a whole mount point and enforce specific options such as ro and noexec. I guess the current LSM hooks should be enough.

Because a mount would change the file hierarchy, we would also need a dedicated LANDLOCK_ACCESS_FS_MOUNT right to control this change. Everything beneath such mount point will get the source's LANDLOCK_RULE_MOUNT properties/restrictions.

For bind mounts, I think we can follow the same checks as for LANDLOCK_ACCESS_FS_REFER with the LANDLOCK_ACCESS_FS_MOUNT (which could then be used for source and destination).

This approach should also enable to allow a service to do mounts without giving it the right to access them.

I suggest to start with the bind mount case and then incrementally add support for the block device mount case (and the new related rule type).

I now think unmounts should never be denied though, so we may want to add a new flag at the ruleset level to only manage compatibility.

Related chromeOS CL: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/5077507

[supersonnic]

This is useful, thanks for the details. I am working on updating that patch to use the refer logic instead of naively allowing a bind mount. I will update as I make progress.

[supersonnic]

Hi @l0kod, I updated the patch to support the same checks as REFER for bind-mounts and wanted to get some feedback on the high-level design. One interesting side-effect is that for the bind-mount to succeed, the source's parent needs to carry the LANDLOCK_ACCESS_FS_MOUNT rule, in addition to the destination directory (i.e. mount point). I think this is OK, as it works the same way for rename and link. For example, if I want to move /parent/child, I need to have the REFER rule applied to /parent. Similarly, to mount /parent/child the MOUNT rule should be applied to /parent. Let me know if the patch is inline with what you envisioned. Thanks.

[l0kod]

This looks good overall.

Thinking more about it, we should probably (re)use LANDLOCK_ACCESS_FS_REFER (instead of LANDLOCK_ACCESS_FS_MOUNT) to check the right to "refer" a file/directory because the related Landlock constraints are the same. This is for the UAPI, but the kernel implementation may differ a bit because of different assumptions (e.g. EXDEV, EACCES, or EPERM error codes).

LANDLOCK_ACCESS_FS_MOUNT should still be required on the destination though, similarly to LANDLOCK_ACCESS_FS_MAKE_*.

I'm worried about a race conditions in hook_sb_mount() regarding dev_name. I think we need a new LSM hook for bind mounts to avoid this kind of issue.

With that in mind, and a clang-formatted code, you can send a first RFC patch series, we'll review it there. Please add at least a few tests for now and propose a set of test scenarios for when we'll be OK with the approach.

You can use https://github.com/landlock-lsm/landlock-test-tools on top of my next branch to test it, or at least on top of b60a173 to avoid the IOCTL patches.

[supersonnic]

LANDLOCK_ACCESS_FS_MOUNT should still be required on the destination though, similarly to LANDLOCK_ACCESS_FS_MAKE_*.

Doesn’t this mean in practice we are also requiring LANDLOCK_ACCESS_FS_MOUNT on the source? If the rule only exists on the destination, the source would gain a new privilege (mount privilege) through the bind mount operation so the operation would fail with EXDEV. If it is missing on the destination, we get EACCES. Is that the behavior we want?

I'm worried about a race conditions in hook_sb_mount() regarding dev_name. I think we need a new LSM hook for bind mounts to avoid this kind of issue.

I think I found a place where we can add a hook for bind mount after the path is evaluated to eliminate the race.

Will work on the tests and preparing for RFC patches.

[l0kod]

LANDLOCK_ACCESS_FS_MOUNT should still be required on the destination though, similarly to LANDLOCK_ACCESS_FS_MAKE_*.

Doesn’t this mean in practice we are also requiring LANDLOCK_ACCESS_FS_MOUNT on the source? If the rule only exists on the destination, the source would gain a new privilege (mount privilege) through the bind mount operation so the operation would fail with EXDEV. If it is missing on the destination, we get EACCES. Is that the behavior we want?

Because of the way mounts propagate by default, we should only allow bind mounts on private mounts if the source doesn't have LANDLOCK_ACCESS_FS_MOUNT. We should then be able to create a new bind mount on the destination directory and mark it as private, and then do another bind mount with the source directory on the destination one (covering the private mount point).

We should also be careful to only create mount point that are not less-restrictive than the source (e.g. removing ro).

For mount operation, no EXDEV error should be returned, only EPERM or EACCES (see mount(2)).

I'm worried about a race conditions in hook_sb_mount() regarding dev_name. I think we need a new LSM hook for bind mounts to avoid this kind of issue.

I think I found a place where we can add a hook for bind mount after the path is evaluated to eliminate the race.

Looking at it again with an up-to-date mount utility, the new mount syscalls (e.g. open_tree, move_mount) are now used and security_move_mount() may be enough.

Will work on the tests and preparing for RFC patches.

Great, we can continue the discussion there.

[l0kod]

Because of the way mounts propagate by default, we should only allow bind mounts on private mounts if the source doesn't have LANDLOCK_ACCESS_FS_MOUNT.

Well, to allow a bind mount on a shared mount point, it's not the source that should have LANDLOCK_ACCESS_FS_MOUNT but the destination (shared) mount point (not only the destination file hierarchy). collect_domain_accesses() may be useful to check that.

However, if the destination mount point is private, we can look for LANDLOCK_ACCESS_FS_MOUNT in its parent mount point and continue looking for these properties in the file hierarchy...