Skip to content
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

Merged
merged 6 commits into from
Dec 4, 2023
Merged

feature: add Landlock support #6078

merged 6 commits into from
Dec 4, 2023

Conversation

kmk3
Copy link
Collaborator

@kmk3 kmk3 commented Nov 4, 2023

@kmk3 kmk3 added the WIP: DON'T MERGE A PR that is still being worked on label Nov 4, 2023
@kmk3 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
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 |
Copy link
Collaborator

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?

Copy link
Owner

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!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

@netblue30 netblue30 marked this pull request as ready for review December 4, 2023 14:17
@netblue30
Copy link
Owner

Merging in!

@netblue30 netblue30 merged commit 9ba5c8d into netblue30:master Dec 4, 2023
25 checks passed
kmk3 pushed a commit that referenced this pull request Dec 5, 2023
kmk3 added a commit that referenced this pull request Dec 5, 2023
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.
kmk3 added a commit that referenced this pull request Dec 5, 2023
This amends commit 13b2c56 ("feature: add Landlock support",
2023-10-24) / PR #6078.
kmk3 added a commit that referenced this pull request Dec 5, 2023
This amends commit d10bf15 ("landlock: detect support at runtime",
2023-11-06) / PR #6078.
kmk3 added a commit that referenced this pull request Dec 5, 2023
This amends commit d10bf15 ("landlock: detect support at runtime",
2023-11-06) / PR #6078.
kmk3 added a commit that referenced this pull request Dec 5, 2023
For consistency with the other functions that have no paramters.

This amends commit 13b2c56 ("feature: add Landlock support",
2023-10-24) / PR #6078.
kmk3 added a commit that referenced this pull request Dec 5, 2023
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.
kmk3 added a commit that referenced this pull request Dec 5, 2023
This amends commit 520508d ("landlock: avoid parsing landlock commands
twice", 2023-11-02) / PR #6078.
kmk3 added a commit that referenced this pull request Dec 5, 2023
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.
kmk3 added a commit that referenced this pull request Feb 6, 2024
This amends commit bf5a993 ("landlock: add support for PATH macro",
2023-12-22).

Relates to #6078.
kmk3 added a commit that referenced this pull request Feb 6, 2024
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.
kmk3 added a commit to kmk3/firejail that referenced this pull request Feb 7, 2024
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)
kmk3 added a commit to kmk3/firejail that referenced this pull request Feb 7, 2024
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
kmk3 added a commit to kmk3/firejail that referenced this pull request Feb 7, 2024
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
kmk3 added a commit that referenced this pull request Feb 8, 2024
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
kmk3 added a commit that referenced this pull request Feb 12, 2024
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.
kmk3 added a commit to kmk3/firejail that referenced this pull request Feb 28, 2024
kmk3 added a commit to kmk3/firejail that referenced this pull request Feb 28, 2024
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.
kmk3 added a commit to kmk3/firejail that referenced this pull request Mar 3, 2024
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.
kmk3 added a commit that referenced this pull request Mar 8, 2024
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.
kmk3 added a commit to kmk3/firejail that referenced this pull request Apr 5, 2024
And mark it as experimental.

Relates to netblue30#6078.
kmk3 added a commit to kmk3/firejail that referenced this pull request Apr 10, 2024
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.
kmk3 added a commit to kmk3/firejail that referenced this pull request Apr 11, 2024
And mark it as experimental.

Relates to netblue30#6078.
kmk3 added a commit to kmk3/firejail that referenced this pull request Apr 29, 2024
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
kmk3 added a commit that referenced this pull request May 12, 2024
This amends commit bf5a993 ("landlock: add support for PATH macro",
2023-12-22).

Relates to #6078.
kmk3 pushed a commit to glitsj16/firejail that referenced this pull request Sep 1, 2024
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.
@kmk3 kmk3 added the enhancement New feature request label Sep 4, 2024
kmk3 pushed a commit that referenced this pull request Sep 10, 2024
Added on commit 13b2c56 ("feature: add Landlock support", 2023-10-24)
/ PR #6078.

Relates to #6451.
kmk3 pushed a commit that referenced this pull request Sep 10, 2024
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.
kmk3 added a commit to kmk3/firejail that referenced this pull request Sep 10, 2024
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.
kmk3 added a commit that referenced this pull request Sep 12, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request WIP: DON'T MERGE A PR that is still being worked on
Projects
Status: Done (on RELNOTES)
Development

Successfully merging this pull request may close these issues.

3 participants