feature: add Landlock support #6078

kmk3 · 2023-11-04T17:18:12Z

Based on 5315 by ChrysoliteAzalea. It is based on the same underlying structure, but with a lot of refactoring/simplification and with bugfixes and improvements. Co-authored-by: Kelvin M. Klann <[email protected]> Co-authored-by: Азалия Смарагдова <[email protected]>

Apply rules in the sandbox thread before the application is started.

And ignore landlock-related commands if Landlock is unsupported at runtime.

topimiettinen · 2023-11-20T19:40:59Z

src/firejail/landlock.c

+ attr.handled_access_fs =
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_BLOCK |
+ LANDLOCK_ACCESS_FS_MAKE_CHAR |


Wouldn't it be possible to forbid making block and char devices?

I'll grab all of them!

@topimiettinen on Nov 20:

Wouldn't it be possible to forbid making block and char devices?

Indeed, I also don't see why an unprivileged program would need that.

I think I'll comment those flags after #6125.

Alternatively, how about moving them from landlock.special into a new
command, landlock.dev?

Similarly to nodev in mount(8), which forbids block and char devices.

src/firejail/landlock.c

src/firejail/usage.c

src/man/firejail-profile.5.in

src/man/firejail.1.in

src/zsh_completion/_firejail.in

netblue30 · 2023-12-04T14:18:02Z

Merging in!

Originally from PR #5359. Relates to #6078.

Fix formatting and wrong/outdated information. This amends commit 6d0559d ("landlock: update README.md, small fix in man firejal; update profile stats in README.md", 2023-12-04). Relates to #6078.

This amends commit 13b2c56 ("feature: add Landlock support", 2023-10-24) / PR #6078.

This amends commit d10bf15 ("landlock: detect support at runtime", 2023-11-06) / PR #6078.

For consistency with the other functions that have no paramters. This amends commit 13b2c56 ("feature: add Landlock support", 2023-10-24) / PR #6078.

To avoid confusion, only return a new ruleset and let the caller set the global one. This amends commit 13b2c56 ("feature: add Landlock support", 2023-10-24) / PR #6078.

This amends commit 520508d ("landlock: avoid parsing landlock commands twice", 2023-11-02) / PR #6078.

When a new landlock entry is parsed from a profile, the first entry in the `cfg.lprofile` list is being set as the next/second entry and the new entry is being set as the first entry in the list, so all entries are being processed from last to first. This commit makes the behavior of ll_add_profile() match the one from profile_add() in src/firejail/profile.c so that the entries are processed in the same order that they are parsed. This amends commit b94cc75 ("landlock: apply rules in sandbox before app start", 2023-10-26) / PR #6078.

Avoid checking if Landlock is supported in ll_add_profile(), as it may result in a warning being printed in ll_is_supported() in the next commit. Relates to netblue30#6078.

Changes: * Move commands from --landlock and --landlock.proc= into etc/inc/landlock-common.inc * Remove --landlock and --landlock.proc= * Add --landlock.enforce Instead of hard-coding the default commands (and having a separate command just for /proc), move them into a dedicated profile to make it easier for users to interact with the entries (view, copy, add ignore entries, etc). Only enforce the Landlock commands if --landlock.enforce is supplied. This allows safely adding Landlock commands to (upstream) profiles while keeping their enforcement opt-in. It also makes it simpler to effectively disable all Landlock commands, by using `--ignore=landlock.enforce`. Relates to netblue30#6078.

Relates to #2097 #5876 #6032 #6078 #6109 #6115 #6125.

@topimiettinen

As discussed with @topimiettinen[1], it is unlikely that an unprivileged process would need to directly create block or character devices. So move the permission to create them from `landlock.special` into a new `landlock.dev` command. Misc: The name is based on `nodev` from mount(8), which makes it not interpret block and character devices. Relates to netblue30#6078. [1] netblue30#6078 (review)

@topimiettinen

As discussed with @topimiettinen[1], it is unlikely that an unprivileged process would need to directly create block or character devices. Also, `landlock.special` is not very descriptive of what it allows. So split `landlock.special` into: * `landlock.makeipc`: allow creating named pipes and sockets (which are usually used for inter-process communication) * `landlock.makedev`: allow creating block and character devices Misc: The `makedev` name is based on `nodev` from mount(8), which makes mount not interpret block and character devices. `ipc` was suggested by @rusty-snake[2]. Relates to netblue30#6078. [1] netblue30#6078 (review) [2] netblue30#6187 (comment)

This amends commit bf5a993 ("landlock: add support for PATH macro", 2023-12-22). Relates to #6078.

Make the error message format in `ll_create_full_ruleset` match the other ones in landlock.c. This amends commit 01a9ddb ("landlock: improve logs for debugging", 2023-11-08). Misc: This was noticed on #6195. Relates to #6078.

Recently (as of Landlock ABI 4), the `handled_access_net` field was added to the `landlock_ruleset_attr` struct in the Linux kernel (in linux/landlock.h). In src/firejail/landlock.c, that field is not being set in the struct (as we currently do not use it) before passing it to the `landlock_create_full_ruleset` syscall, so it may contain random garbage when used, resulting in the syscall sometimes returning EINVAL (depending on whether the garbage is valid)[1]: ll_is_supported: Detected Landlock ABI version 4 ll_restrict: Starting Landlock restrict ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) ll_create_full_ruleset: Error: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for / Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /: Bad file descriptor ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) ll_create_full_ruleset: Error: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for /proc Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /proc: Bad file descriptor [...] So ensure that all structs in landlock.c are initialized to 0 before using them. Note: This currently affects Arch but not Artix, as the former packages a more recent version of the Linux headers (linux-api-headers 6.7-1 vs 6.4-1). Fixes netblue30#6195. Relates to netblue30#6078. [1] netblue30#6195 (comment)

Recently (as of Landlock ABI 4), the `handled_access_net` field was added to the `landlock_ruleset_attr` struct in the Linux kernel (in linux/landlock.h). In src/firejail/landlock.c, that field is not being set in the struct (as we currently do not use it) before passing it to the `landlock_create_full_ruleset` syscall, so it may contain random garbage when used, resulting in the syscall sometimes returning EINVAL (depending on whether the garbage is valid)[1]: ll_is_supported: Detected Landlock ABI version 4 ll_restrict: Starting Landlock restrict ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) ll_create_full_ruleset: Error: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for / Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /: Bad file descriptor ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) ll_create_full_ruleset: Error: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for /proc Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /proc: Bad file descriptor [...] So ensure that all structs in landlock.c are initialized to 0 before using them. Note: Arch has recently (2024-01-31) updated the linux-api-headers package from version 6.4-1 to 6.7-1[2]. The former version is not affected (as it does not contain the extra struct field in linux/landlock.h), while the latter is. Fixes netblue30#6195. Relates to netblue30#6078. [1] netblue30#6195 (comment) [2] https://gitlab.archlinux.org/archlinux/packaging/packages/linux-api-headers/-/commit/b4223b0c2bfba54c26acc4dc289415b81b15989f

@curiosityseeker

Recently (as of Landlock ABI 4), the `handled_access_net` field was added to the `landlock_ruleset_attr` struct in the Linux kernel (in linux/landlock.h). In src/firejail/landlock.c, that field is not being set in the struct (as we currently do not use it) before passing it to the `landlock_create_full_ruleset` syscall, so it is likely to contain random garbage when used, resulting in the syscall returning EINVAL: $ firejail --debug --profile=/etc/firejail/landlock-common.inc \ --landlock.enforce true [...] ll_is_supported: Detected Landlock ABI version 4 ll_restrict: Starting Landlock restrict ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) Error: ll_create_full_ruleset: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for / Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /: Bad file descriptor [...] Not enforcing Landlock So ensure that all structs in src/firejail/landlock.c are initialized to 0 before using them. Note: Arch has recently (2024-01-31) updated the linux-api-headers package from version 6.4-1 to 6.7-1[1]. The former version is not affected (as it does not contain the extra struct field in linux/landlock.h), while the latter is. Fixes netblue30#6195. Relates to netblue30#6078. [1] https://gitlab.archlinux.org/archlinux/packaging/packages/linux-api-headers/-/commit/b4223b0c2bfba54c26acc4dc289415b81b15989f Reported-by: @curiosityseeker

@curiosityseeker

Recently (as of Landlock ABI 4), the `handled_access_net` field was added to the `landlock_ruleset_attr` struct in the Linux kernel (in linux/landlock.h). In src/firejail/landlock.c, that field is not being set in the struct (as we currently do not use it) before passing it to the `landlock_create_full_ruleset` syscall, so it is likely to contain random garbage when used, resulting in the syscall returning EINVAL: $ firejail --debug --profile=/etc/firejail/landlock-common.inc \ --landlock.enforce true [...] ll_is_supported: Detected Landlock ABI version 4 ll_restrict: Starting Landlock restrict ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) Error: ll_create_full_ruleset: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for / Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /: Bad file descriptor [...] Not enforcing Landlock So ensure that all structs in src/firejail/landlock.c are initialized to 0 before using them. Note: Arch has recently (2024-01-31) updated the linux-api-headers package from version 6.4-1 to 6.7-1[1]. The former version is not affected (as it does not contain the extra struct field in linux/landlock.h), while the latter is. Fixes #6195. Relates to #6078. [1] https://gitlab.archlinux.org/archlinux/packaging/packages/linux-api-headers/-/commit/b4223b0c2bfba54c26acc4dc289415b81b15989f Reported-by: @curiosityseeker

This amends commit 760f50f ("landlock: move commands into profile and add landlock.enforce", 2023-11-17) / PR #6125. Misc: This was noticed on #6203. Relates to #6078.

Relates to netblue30#6078.

Since Landlock ABI v4 it is possible to restrict actions related to the network and potentially more areas will be added in the future. So use `landlock.fs.` as the prefix in the current filesystem-related commands (and later `landlock.net.` for the network-related commands) to keep them organized and to match what is used in the kernel. Examples of filesystem and network access flags: * `LANDLOCK_ACCESS_FS_EXECUTE`: Execute a file. * `LANDLOCK_ACCESS_FS_READ_DIR`: Open a directory or list its content. * `LANDLOCK_ACCESS_NET_BIND_TCP`: Bind a TCP socket to a local port. * `LANDLOCK_ACCESS_NET_CONNECT_TCP`: Connect an active TCP socket to a remote port. Relates to netblue30#6078.

To reduce duplication. Support for it was added on commit bf5a993 ("landlock: add support for PATH macro", 2023-12-22). See also commit 19e1082 ("landlock: expand simple macros in commands", 2023-11-11) / PR netblue30#6125. Relates to netblue30#6078.

To reduce duplication. Support for it was added on commit bf5a993 ("landlock: add support for PATH macro", 2023-12-22). See also commit 19e1082 ("landlock: expand simple macros in commands", 2023-11-11) / PR #6125. Relates to #6078.

And mark it as experimental. Relates to netblue30#6078.

Changes: * Always declare public landlock functions, regardless of `HAVE_LANDLOCK` * Make the other public landlock functions (besides `ll_add_profile`) also be empty when `HAVE_LANDLOCK` is not defined * Clarify related comments This amends commit 8259f66 ("landlock fix for old kernel versions", 2024-04-06). For clarity, landlock-common.inc is included by default.profile and the issue that the aforementioned commit fixes is that if profile.c is built without the part that parses landlock commands (that is, when `HAVE_LANDLOCK` is not defined), using default.profile would cause firejail to abort due to "invalid lines". Note that the issue would only occur when firejail is built with an older kernel (or with --disable-landlock), not when simply running on an older kernel. See also commit b02a7a3 ("landlock: remove empty functions", 2023-12-07). Relates to netblue30#6078.

And mark it as experimental. Relates to netblue30#6078.

landlock.h may not be available on the system (such as with older versions of Linux API headers), so only try to include it if `HAVE_LANDLOCK` is defined. This fixes the following error from `build_debian_package` (which uses `debian:buster`) on GitLab CI[1]: $ ./mkdeb.sh --enable-fatal-warnings [...] gcc [...] -c ../../src/firejail/landlock.c -o ../../src/firejail/landlock.o ../../src/firejail/landlock.c:22:10: fatal error: linux/landlock.h: No such file or directory #include <linux/landlock.h> ^~~~~~~~~~~~~~~~~~ compilation terminated. This amends commit a05ae97 ("landlock: amend empty functions and comments", 2024-04-08) / PR netblue30#6305. Relates to netblue30#6078. [1] https://gitlab.com/Firejail/firejail_ci/-/jobs/6743161059

This amends commit bf5a993 ("landlock: add support for PATH macro", 2023-12-22). Relates to #6078.

kmk3 added the WIP: DON'T MERGE A PR that is still being worked on label Nov 4, 2023

kmk3 added this to In progress in Release 0.9.74 via automation Nov 4, 2023

kmk3 force-pushed the landlock_v3 branch from e51efbc to 4d5cb9d Compare November 5, 2023 11:08

cleanup

5456ec6

kmk3 force-pushed the landlock_v3 branch 2 times, most recently from 05ce614 to cf4f361 Compare November 7, 2023 19:57

netblue30 and others added 5 commits November 7, 2023 17:55

landlock: new filesystem for --landlock command

abc1edc

landlock: apply rules in sandbox before app start

b94cc75

Apply rules in the sandbox thread before the application is started.

landlock: avoid parsing landlock commands twice

520508d

landlock: detect support at runtime

d10bf15

And ignore landlock-related commands if Landlock is unsupported at runtime.

kmk3 force-pushed the landlock_v3 branch from cf4f361 to d10bf15 Compare November 7, 2023 21:01