feature: add Landlock support #6078

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

Relates to:

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.

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.

Sort commands and sections in firejail.1.in and sync the result with firejail-profile.5.in. * Commands: `--dbus-system.*`, `--dbus-user.*`, `--icmptrace`, `--ip=none`, `memory-deny-write-execute`, `--noinput` * Sections: "LANDLOCK", "NAME VALIDATION" Relates to netblue30#3190 netblue30#3406 netblue30#4209 netblue30#5856 netblue30#6078.

Added on commit 13b2c56 ("feature: add Landlock support", 2023-10-24) / PR #6078. Relates to #6451.

Sort commands and sections in firejail.1.in and sync the result with firejail-profile.5.in. * Commands: `--dbus-system.*`, `--dbus-user.*`, `--icmptrace`, `--ip=none`, `memory-deny-write-execute`, `--noinput` * Sections: "LANDLOCK", "NAME VALIDATION" Relates to #3190 #3406 #4209 #5856 #6078.

Reset the bold right after each command/argument. Command used to check for issues: git grep -E ' \\fR' -- src/man/*.in Related commits: * e91b9ff ("Deprecate --nodbus option", 2020-04-07) / PR netblue30#3265 * 5a61202 ("rename noautopulse to keep-config-pulse", 2021-05-13) / PR netblue30#4278 * d79547c ("docs: warn about limitations of landlock", 2024-03-31) / PR netblue30#6302 This is a follow-up to netblue30#6451. Relates to netblue30#6078.

Reset the bold right after each command/argument. Command used to check for issues: git grep -E ' \\fR' -- src/man/*.in Related commits: * e91b9ff ("Deprecate --nodbus option", 2020-04-07) / PR #3265 * 5a61202 ("rename noautopulse to keep-config-pulse", 2021-05-13) / PR #4278 * d79547c ("docs: warn about limitations of landlock", 2024-03-31) / PR #6302 This is a follow-up to #6451. Relates to #6078.

kmk3 added the WIP: DON'T MERGE A PR that is still being worked on label 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