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

Signals control #8

Open
l0kod opened this issue Jan 18, 2024 · 10 comments
Open

Signals control #8

l0kod opened this issue Jan 18, 2024 · 10 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@l0kod
Copy link
Member

l0kod commented Jan 18, 2024

A sandboxed process is currently not restricted to send signals (e.g. SIGKILL) to processes outside the sandbox. A simple way to control that would be to scope signals the same way ptrace is restricted (but this time it would be opt-in).

See https://lore.kernel.org/all/[email protected]/

Approach similar to #7

v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/36958dbc486e1f975f4d4ecdfa51ae65c2c4ced0.1720213293.git.fahimitahera@gmail.com/

@l0kod l0kod added the enhancement New feature or request label Jan 18, 2024
@l0kod l0kod changed the title Signal control Signals control Jan 18, 2024
@l0kod l0kod added the good first issue Good for newcomers label Jan 30, 2024
@l0kod l0kod added this to Landlock Feb 1, 2024
@l0kod l0kod moved this to Ready in Landlock Feb 1, 2024
@maryagiamah
Copy link

Hello, can I work on this ?

@l0kod
Copy link
Member Author

l0kod commented Mar 6, 2024

Hi @maryagiamah! Yes, you can start working on signals control.

@l0kod l0kod moved this from Ready to In Progress in Landlock Mar 6, 2024
@l0kod l0kod moved this from In Progress to Ready in Landlock Mar 7, 2024
@l0kod
Copy link
Member Author

l0kod commented Mar 7, 2024

What is your plan to work on this?

@maryagiamah
Copy link

maryagiamah commented Mar 7, 2024 via email

@l0kod
Copy link
Member Author

l0kod commented Mar 7, 2024

I have little idea about what it entails as I have experimented with signals prior but I am still reading up on what the task is all about. Do you have any route or path I should go searching for?

You first need to familiarize yourself with the Linux kernel development as explained in the Outreachy documentation. Then, you can start tweaking the code and test it with the test tools. If you have questions about that, please send emails to the Outreachy mailing list (CCing me and @pcmoore).

Before patching the kernel, you can start by thinking about attack scenarios using signals, describing them, and write such tests in a new file here.

About the kernel changes, we need to add a new scoped field to the landlock_ruleset_attr struct. This field will optionally contain a LANDLOCK_RULESET_SCOPED_SIGNAL flag to specify that this ruleset will deny any signal from within the sandbox to its parents (i.e. any parent sandbox or not-sandboxed processes).

This kind of restriction should be pretty similar to the ptrace ones, but we need to keep in mind that not all layers (i.e. nested sandboxes) of a Landlock domain may have this restriction. It could be seen as a roadblock for signals.

There is an LSM hook dedicated to control signals, and the Landlock scope logic is defined in ptrace.c (being renamed to task.c).

This GitHub issue is dedicated to track progress on signal control. As a reminder, no Linux kernel code change nor review should happen on GitHub because kernel reviewers are only following the dedicated mailing lists.

@l0kod
Copy link
Member Author

l0kod commented Mar 27, 2024

Any news @maryagiamah?

@l0kod l0kod moved this from Ready to In review in Landlock Aug 8, 2024
l0kod pushed a commit that referenced this issue Sep 9, 2024
[ Upstream commit a699781 ]

A sysfs reader can race with a device reset or removal, attempting to
read device state when the device is not actually present. eg:

     [exception RIP: qed_get_current_link+17]
  #8 [ffffb9e4f2907c48] qede_get_link_ksettings at ffffffffc07a994a [qede]
  #9 [ffffb9e4f2907cd8] __rh_call_get_link_ksettings at ffffffff992b01a3
 #10 [ffffb9e4f2907d38] __ethtool_get_link_ksettings at ffffffff992b04e4
 #11 [ffffb9e4f2907d90] duplex_show at ffffffff99260300
 #12 [ffffb9e4f2907e38] dev_attr_show at ffffffff9905a01c
 #13 [ffffb9e4f2907e50] sysfs_kf_seq_show at ffffffff98e0145b
 #14 [ffffb9e4f2907e68] seq_read at ffffffff98d902e3
 #15 [ffffb9e4f2907ec8] vfs_read at ffffffff98d657d1
 #16 [ffffb9e4f2907f00] ksys_read at ffffffff98d65c3f
 #17 [ffffb9e4f2907f38] do_syscall_64 at ffffffff98a052fb

 crash> struct net_device.state ffff9a9d21336000
    state = 5,

state 5 is __LINK_STATE_START (0b1) and __LINK_STATE_NOCARRIER (0b100).
The device is not present, note lack of __LINK_STATE_PRESENT (0b10).

This is the same sort of panic as observed in commit 4224cfd
("net-sysfs: add check for netdevice being present to speed_show").

There are many other callers of __ethtool_get_link_ksettings() which
don't have a device presence check.

Move this check into ethtool to protect all callers.

Fixes: d519e17 ("net: export device speed and duplex via sysfs")
Fixes: 4224cfd ("net-sysfs: add check for netdevice being present to speed_show")
Signed-off-by: Jamie Bainbridge <[email protected]>
Link: https://patch.msgid.link/8bae218864beaa44ed01628140475b9bf641c5b0.1724393671.git.jamie.bainbridge@gmail.com
Signed-off-by: Jakub Kicinski <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
mj22226 pushed a commit to mj22226/linux that referenced this issue Sep 17, 2024
Currently, a sandbox process is not restricted to sending a signal (e.g.
SIGKILL) to a process outside the sandbox environment.  The ability to
send a signal for a sandboxed process should be scoped the same way
abstract UNIX sockets are scoped. Therefore, we extend the "scoped"
field in a ruleset with LANDLOCK_SCOPE_SIGNAL to specify that a ruleset
will deny sending any signal from within a sandbox process to its parent
(i.e. any parent sandbox or non-sandboxed processes).

This patch adds file_set_fowner and file_free_security hooks to set and
release a pointer to the file owner's domain. This pointer, fown_domain
in landlock_file_security will be used in file_send_sigiotask to check
if the process can send a signal.

The ruleset_with_unknown_scope test is updated to support
LANDLOCK_SCOPE_SIGNAL.

This depends on two new changes:
- commit 1934b21 ("file: reclaim 24 bytes from f_owner"): replace
  container_of(fown, struct file, f_owner) with fown->file .
- commit 26f2043 ("fs: Fix file_set_fowner LSM hook
  inconsistencies"): lock before calling the hook.

Signed-off-by: Tahera Fahimi <[email protected]>
Closes: landlock-lsm#8
Link: https://lore.kernel.org/r/df2b4f880a2ed3042992689a793ea0951f6798a5.1725657727.git.fahimitahera@gmail.com
[mic: Update landlock_get_current_domain()'s return type, improve and
fix locking in hook_file_set_fowner(), simplify and fix sleepable call
and locking issue in hook_file_send_sigiotask() and rebase on the latest
VFS tree, simplify hook_task_kill() and quickly return when not
sandboxed, improve comments, rename LANDLOCK_SCOPED_SIGNAL]
Co-developed-by: Mickaël Salaün <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
@l0kod
Copy link
Member Author

l0kod commented Sep 25, 2024

Merged in e1b061b, but a test is still missing to align with the #7 tests.

@l0kod
Copy link
Member Author

l0kod commented Sep 30, 2024

@gnoack
Copy link

gnoack commented Nov 15, 2024

Related go-landlock PR: landlock-lsm/go-landlock#33

During this PR, I discovered that signal scoping is potentially at odds with the way how Landlock gets enforced in multithreaded applications. Because landlock_restrict_self is inherently a single-threaded operation, in go-landlock, we use libpsx. This library implements the enforcement for all threads, but it does that by registering a special signal handler on all threads and then letting all threads do landlock_restrict_self individually. As a consequence:

  • Landlock domains on different OS threads are technically speaking enforced separately from each other, and are different Landlock domains as far as the kernel is concerned. ==> When signal scopes are enabled, the separate OS threads of that process can not send each other signals any more.
  • Because libpsx also in itself uses signals between these threads to enforce Landlock rulesets, libpsx can not be used correctly any more, affecting operations like (a) further landlock enforcement and (b) other psx-based glibc functionality such as setuid(2) and friends (compare setuid(2) VERSIONS section and nptl(7))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Status: Done
Development

No branches or pull requests

4 participants