Abstract unix socket control

[l0kod]

It would be nice to be able to scope access to abstract unix sockets the same way ptrace is restricted (but this time it would be opt-in).

See https://lore.kernel.org/all/20231023.ahphah4Wii4v@digikod.net/ and https://lore.kernel.org/all/20231102.MaeWaepav8nu@digikod.net/

Approach similar to #8

[tahifahimi]

Hi @l0kod, I start working on this, and read almost all of the related docs & explanations :) quick question, besides following the landlock logic from task.c (which is ptrace.c in staging branch) and scope changes that should apply to net.c, do we need to modify any other file?

[l0kod]

Hi @tahifahimi,

Hi @l0kod, I start working on this, and read almost all of the related docs & explanations :)

Great!

besides following the landlock logic from task.c (which is ptrace.c in staging branch) and scope changes that should apply to net.c, do we need to modify any other file?

I think most changes should be in task.c/ptrace.c. The net.c file is dedicated to network rule management (only TCP bind/connect for now), but the abstract unix socket restrictions should be implemented as scopes like for ptrace restrictions (e.g. no new Landlock rule type but only a ruleset's flag, no sockaddr parsing...). Moreover, I consider abstract unix sockets to be more an IPC mechanism rather than a network feature. task.c contains the main helpers that should be used (and extended): task_is_scoped() and domain_scope_le().

About the flag design, see https://github.com/landlock-lsm/linux/issues/8#issuecomment-1983910849. Updating this interface should lead you to the other files that will need some changes.

[tahifahimi]

Thanks for your comments! Based on you suggested, I had a look; please correct me if I am wrong. First I have to add a struct field to the landlock_ruleset in landlock/ruleset.h (a struct field same as hierarchy which is used for ptrace). Then I should edit the "landlock/ptrace.c" and add landlock_hooks for unix_stream_connect and unix_may_send and write the respective helpers (same thing we have for ptrace_traceme and ptrace_access_check for ptrace).

[l0kod]

Looks good! One important thing to keep in mind is that because abstract unix socket scoping will be optional, you might need to implement new checks in the current domain_scope_le() helper and related data structures/calls.

BTW, the rename of ptrace.c to task.c is now merged: 35e886e88c803920644c9d3abb45a9ecb7f1e761

[l0kod]

I'm assigning this issue to you. Please regularly update it with your progress. This doesn't need to be long but just enough to make sure you are not blocked.

[tahifahimi]

@l0kod, I have a quick question: Since the input of hook_unix_stream_connect are struct sock *const sock, struct sock *const other, and struct sock *const newsk, how can we map a sock struct to a task_struct? (because it is specified in here that we want to scope the abstract unix socket according to a landlock domain, and we can get the landlock_ruleset through landlock_get_task_domainfunction)

[l0kod]

If you take a look at unix_listen(), you'll see that the credential of the current task (the listener) is stored in the socket. In the same file, you'll find useful helpers to access different fields. You can then infer the Landlock domain from this credential. You'll also find examples of such use in sk_getsockopt().

For performance reasons, we should think about caching the result somehow. You can take a look at other LSMs that implement these hooks. The sk_security field can be used to label sockets, but for that to be useful we need a way to implement a partial ordering algorithm using raw values (instead of walking through the landlock_hierarchy for each hook call). Anyway, you can think about that but you should first work on the simple approach.

[tahifahimi]

Quick update: Since the inputs of security_unix_stream_connect are three struct sock data types, we can get the credentials of the sock's peer the same as sk_get_peer_cred() in net/core/socket.c and then we can get the landlock_cred_security of a cred through landlock_cred in landlock/cred.h

I am working on a bpf file that hooks to "unix_stream_connect" lsm. so instead of changing the task.c file for now, we apply changes to the bpf file and ensure that the changes in the hook work.

[l0kod]

Quick update: Since the inputs of security_unix_stream_connect are three struct sock data types, we can get the credentials of the sock's peer the same as sk_get_peer_cred() in net/core/socket.c and then we can get the landlock_cred_security of a cred through landlock_cred in landlock/cred.h

OK

I am working on a bpf file that hooks to "unix_stream_connect" lsm. so instead of changing the task.c file for now, we apply changes to the bpf file and ensure that the changes in the hook work.

Why would BPF be involved here? Even for experimenting, I don't think it would be a good idea to spend some time with BPF for this task.

[tahifahimi]

Why would BPF be involved here? Even for experimenting, I don't think it would be a good idea to spend some time with BPF for this task.

So, as a first step, I was thinking of implementing the scoping mechanism inside a BPF function that hooks into the unix_stream_connect instead of actually modifying the task.c file to implement the unix_stream_connect functionality. When I ensure that the BPF works, I can modify the task.c file with the proper helpers and functions.

[l0kod]

Why would BPF be involved here? Even for experimenting, I don't think it would be a good idea to spend some time with BPF for this task.

So, as a first step, I was thinking of implementing the scoping mechanism inside a BPF function that hooks into the unix_stream_connect instead of actually modifying the task.c file to implement the unix_stream_connect functionality. When I ensure that the BPF works, I can modify the task.c file with the proper helpers and functions.

I don't think it would help to go with BPF first because that would mean to spend a lot of time tweaking BPF-related code that would not be useful after that.

[l0kod]

For reference: the related draft patch.

[tahifahimi]

For reference: the first patch