On Mon, May 26, 2025 at 07:37:41PM +0100, Tingmao Wang wrote: > On 5/23/25 17:57, Mickaël Salaün wrote: > > Hi, > > > > This series adds two tracepoints to Landlock, one tied to rule addition, > > and another to rule checking. With these new tracepoints, we can see > > all steps leading to an access decision. They can be directly used with > > /sys/kernel/tracing/events/landlock/* or attached by eBPF programs to > > get a more complete view of Landlock internals. > > > > This new feature is useful to trouble shoot policy issues, and it should > > also limit the need for custom debugging kernel code when developing new > > Landlock features. > > > > Landlock already has audit support, which enables us to log denied > > access requests. Audit is useful to identify security issues or sandbox > > misconfiguration. However, it might not be enough to debug Landlock > > policies. The main differences with audit events is that traces are > > disabled by default, can be very verbose, and can be filtered according > > to process and Landlock properties (e.g. domain ID). > > > > As for audit, this tracing feature may expose sensitive information and > > must then only be accessible to the system administrator. > > > > This RFC only fully supports filesystem rules but the next series will > > also support network rules. Tests are also missing for now. > > > > Regards, > > > > Mickaël Salaün (5): > > landlock: Rename landlock_id to landlock_rule_ref > > landlock: Merge landlock_find_rule() into landlock_unmask_layers() > > tracing: Add __print_untrusted_str() > > landlock: Add landlock_add_rule_fs tracepoint > > landlock: Add landlock_check_rule tracepoint > > > > MAINTAINERS | 1 + > > include/linux/trace_events.h | 3 + > > include/trace/events/landlock.h | 124 ++++++++++++++ > > include/trace/stages/stage3_trace_output.h | 4 + > > include/trace/stages/stage7_class_define.h | 1 + > > kernel/trace/trace_output.c | 40 +++++ > > security/landlock/Makefile | 11 +- > > security/landlock/fs.c | 178 +++++++++++++-------- > > security/landlock/fs.h | 3 + > > security/landlock/net.c | 18 +-- > > security/landlock/ruleset.c | 65 ++++---- > > security/landlock/ruleset.h | 15 +- > > security/landlock/trace.c | 15 ++ > > 13 files changed, 365 insertions(+), 113 deletions(-) > > create mode 100644 include/trace/events/landlock.h > > create mode 100644 security/landlock/trace.c > > > > > > base-commit: a5806cd506af5a7c19bcd596e4708b5c464bfd21 > > Tested-by: Tingmao Wang <m...@maowtm.org> > > Here is an example output I got with `trace-cmd record -e landlock`: > > landlock-add-ru-776 [001] 75134.554776: landlock_add_rule_fs: [FAILED TO > PARSE] ruleset=0xffff88811a459200 ref_key=18446612686420679296 allowed=57343 > dev=21 ino=53 pathname=/tmp/test > landlock-add-ru-776 [001] 75134.555336: landlock_check_rule: [FAILED TO > PARSE] domain_id=7838764077 access_request=4 ref_type=1 > ref_key=18446612686420679296 layers=ARRAY[ff, df] > I suggest adding some more events which I think shouldn't be too difficult > to implement and I can see them helping a lot with tracing / debugging > especially by BPF programs or someone new to landlock: > > - landlock_restrict_self: Currently we trace out the ruleset pointer in add > rule, and the domain ID in check rule. However there is no way for someone > just looking at this trace (or a BPF program for landlock) to relate which > rulesets are applied to which domains. I think a simple event we could add > that will help with this is, on landlock_restrict_self, prints out the > ruleset passed in as well as the domain ID newly created. Maybe also > num_rules and num_layers on the new ruleset since it's trivial to do so, and > could be informative to someone analyzing a Landlock thing.
Yep, that was my plan for v2. :) I was also wondering about adding a tracepoint for ruleset creation. This one would print the ruleset's attributes as an array. > > - landlock_check_fs: Distinct from landlock_check_rule, this will happen > once the outcome of the access check is determined (maybe at the end of > is_access_to_paths_allowed and collect_domain_accesses?). It would include > the pathname of the target file (only allocated if this event is enabled of > course), so something like: > > landlock_check_fs: domain_id=7838764077 access_request=4 > pathname=/tmp/test allowed=true > landlock_check_fs: domain_id=7838764077 access_request=4 > pathname=/tmp/test2 allowed=false > > We already produces audit logs for denied requests so it is a little bit > duplicating that, but I think this trace event shouldn't be too costly to > include. It has the benefit that > > 1. If an access is denied because no rules matched, we don't get any > landlock_check_rule traces, and so there's no way for someone looking at the > trace log to find out landlock denied something. That would indeed be useful to see final access decisions. It might be a bit tricky to generically handle all the allowed access but I'll try something. > > 2. Having an event that represents an access check makes it possible for BPF > programs to find out about all landlock access checks (most interestingly > denied ones but we could expose the allowed ones too), and potentially > relate the various `landlock_check_rule` events to an access check. > > Actually, maybe it's worth having a "landlock_check_fs_enter", emitted at > the beginning of is_access_to_paths_allowed and collect_domain_accesses? > This could be useful for performance measurements, and also makes it more Aren't kprobes/kretprobes enough for performance measurement? They can also be used with current kernels. > explicit which landlock_check_rule resulted from which access check. Maybe > the pathname and domain_id could be printed on the enter event, and the exit > event would just have the outcome? (Just an idea, I feel less certain about > this. "Enter"/"exit" naming taken from sys_enter_*/sys_exit_* but start/end > also works) This landlock_check_fs_enter event would only be called after the landlock_get_applicable_subject() check though.
