-
Notifications
You must be signed in to change notification settings - Fork 567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature: add Landlock support #6078
Conversation
05ce614
to
cf4f361
Compare
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 <kmk3.code@protonmail.com> Co-authored-by: Азалия Смарагдова <charming.flurry@yandex.ru>
Apply rules in the sandbox thread before the application is started.
And ignore landlock-related commands if Landlock is unsupported at runtime.
attr.handled_access_fs = | ||
LANDLOCK_ACCESS_FS_EXECUTE | | ||
LANDLOCK_ACCESS_FS_MAKE_BLOCK | | ||
LANDLOCK_ACCESS_FS_MAKE_CHAR | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be possible to forbid making block and char devices?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll grab all of them!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Merging in! |
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.
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
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
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
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.
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
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.
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.
Relates to: