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

Add Landlock support #5269

Closed
ChrysoliteAzalea opened this issue Jul 22, 2022 · 10 comments
Closed

Add Landlock support #5269

ChrysoliteAzalea opened this issue Jul 22, 2022 · 10 comments
Labels
enhancement New feature request

Comments

@ChrysoliteAzalea
Copy link
Collaborator

Is your feature request related to a problem? Please describe.

Hello everyone! I've been using Firejail for a long time. I like it sandboxing features that use namespaces, seccomp and AppArmor policies. However, as far as I understand, Firejail doesn't construct a file hierarchy for a mount namespace -- it uses the host FS instead, remounting protected directories with tmpfs or empty/inaccessible directories. It also has limited support for AppArmor, because it can only transition to a pre-defined profile.

Describe the solution you'd like

I propose adding Landlock support to enhance security of the sandbox. Landlock is a Linux security module that implements file access self-restriction in the way that seccomp implements syscall access self-restriction. It can be used to restrict access to sensitive paths. I'm thinking of implementing it for Firejail, however, currently I don't know how to do it better in order not to interfere with normal operation.

Describe alternatives you've considered

One of the alternatives is to reconstruct the filesystem hierarchy in the new namespace "from scratch" -- however, that would require serious changes in Firejail source code that would need to be thoroughly tested. Another alternative is to use AppArmor -- however, loading AppArmor requires superuser privileges, and it can lead to vulnerabilities.

Additional context

I've created an experimental patch for Bubblewrap that implements Landlock self-restriction in order to enhance sandbox security.

@osevan

This comment was marked as off-topic.

@topimiettinen
Copy link
Collaborator

Ideally all allow/whitelisting, deny/blacklisting, noexec, read-only, private-bin etc. operations specified in the existing profiles should be translated to corresponding Landlock restrictions (read, write, execute) automatically. That way the profiles probably would not need any changes. Do you think that would be possible?

I think Landlock could one day replace bind mounting (which needs superuser privileges) and then perhaps Firejail could work without being SUID root.

@ChrysoliteAzalea
Copy link
Collaborator Author

Ideally all allow/whitelisting, deny/blacklisting, noexec, read-only, private-bin etc. operations specified in the existing profiles should be translated to corresponding Landlock restrictions (read, write, execute) automatically. That way the profiles probably would not need any changes. Do you think that would be possible?

I think it's definitely possible. However, as far as I know, these operations differ in details, so I think that it's worth providing both options (namespacing and Landlock).

I think Landlock could one day replace bind mounting (which needs superuser privileges) and then perhaps Firejail could work without being SUID root.

A process doesn't need actual superuser in order to bind mount -- it just needs a separate user namespace with local CAP_SYS_ADMIN permission (not needing the system-wide CAP_SYS_ADMIN permission) and a separate mount namespace. Bubblewrap does that.

@rusty-snake
Copy link
Collaborator

rusty-snake commented Jul 27, 2022

FWIW: #5157


@topimiettinen this sound good to me in general. However it can become complicated as the filesystem layout is the results of procedural statement which can overlap if you translate them to landlock rules. Nevermind I thing this is a good approach to start with landlick in FJ.

More lanlock (only) based feature can be implemented later then #3992 (comment).

@ChrysoliteAzalea
Copy link
Collaborator Author

I only want to ask -- in order to simplify working with Landlock rulesets, I've made a library called tinyLL. I'd like to ask, is it a good idea to use it? Or would it be better to add Landlock-related functions to Firejail itself?

@rusty-snake
Copy link
Collaborator

Will it support v2 ABI? (FYI: https://codeberg.org/crabjail/crablock/commit/86d769797cfb83761e2e23fcd130e47b01324ac1)

Adding support for an external library in the build system sounds to complex to me (give that it has only ~100 LoC). However adding it to the source tree can be considered.

@ChrysoliteAzalea
Copy link
Collaborator Author

Will it support v2 ABI?

I didn't test it yet (and I don't know whether v2 is in the mainline kernel or not), however, I think I'll try to add support for it. I currently tested tinyLL for Landlock v1. I think I'll add support when my distribution (Linux Mint) start supporting it, or earlier if I manage to upgrade the kernel sooner.

@ChrysoliteAzalea
Copy link
Collaborator Author

Will it support v2 ABI? (FYI: https://codeberg.org/crabjail/crablock/commit/86d769797cfb83761e2e23fcd130e47b01324ac1)

Adding support for an external library in the build system sounds to complex to me (give that it has only ~100 LoC). However adding it to the source tree can be considered.

I'd like to ask, how to add it to the source tree correctly. I've created a pull request, and it works for me, but it seems that currently, it's like libapparmor -- the Firejail builds with Landlock only if tinyLL is already present in the system. I wonder, can it be built and linked automatically, the way it's done with libraries that are in src/lib

@smitsohu
Copy link
Collaborator

One of the alternatives is to reconstruct the filesystem hierarchy in the new namespace "from scratch" -- however, that would require serious changes in Firejail source code

Currently Firejail modifies the chroot it is running in (as a setuid tool that starts other privileged tools), and this complicates reasoning about code correctness a lot. It would be much easier, if Firejail would build a chroot and then pivot_root into that environment, like some other tools do.

In particular, this would simplify whitelist and some private-* code by a significant margin.

@kmk3
Copy link
Collaborator

kmk3 commented Jan 30, 2024

Closing, as it was added in #6078.

@kmk3 kmk3 closed this as completed Jan 30, 2024
kmk3 added a commit that referenced this issue Jan 30, 2024
@kmk3 kmk3 changed the title I propose adding Landlock support to Firejail Add Landlock support Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request
Projects
Status: Done (on RELNOTES)
Development

No branches or pull requests

6 participants