On Nov 22, 2024 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <m...@digikod.net> wrote: > > Add audit support to ptrace_access_check and ptrace_traceme hooks. > > Add a new AUDIT_LANDLOCK_DENY record type dedicated to any Landlock > denials. > > Log the domain ID restricting the action, the domain's blockers that are > missing to allow the requested access, and the target task. > > The blockers are implicit restrictions (e.g. ptrace), or explicit access > rights (e.g. filesystem), or explicit scopes (e.g. signal). > > For the ptrace_access_check case, we log the current/parent domain and > the child task. For the ptrace_traceme case, we log the parent domain > and the parent task. Indeed, the requester is the current task, but the > action would be performed by the parent task. > > The quick return for non-landlocked tasks is moved from task_ptrace() to > each LSM hooks. > > Audit event sample: > > type=LL_DENY msg=audit(1732186800.349:44): domain=195ba459b blockers=ptrace > opid=1 ocomm="systemd" > type=SYSCALL msg=audit(1732186800.349:44): arch=c000003e syscall=101 > success=no [...] pid=300 auid=0 > > Cc: Günther Noack <gno...@google.com> > Cc: Paul Moore <p...@paul-moore.com> > Signed-off-by: Mickaël Salaün <m...@digikod.net> > Link: https://lore.kernel.org/r/20241122143353.59367-9-...@digikod.net > --- > Changes since v2: > - Log domain IDs as hexadecimal number: this is a more compact notation > (i.e. at least one less digit), it improves alignment in logs, and it > makes most IDs start with 1 as leading digit (because of the 2^32 > minimal value). Do not use the "0x" prefix that would add useless > data to logs. > - Constify function arguments. > > Changes since v1: > - Move most audit code to this patch. > - Rebase on the TCP patch series. > - Don't log missing access right: simplify and make it generic for rule > types. > - Don't log errno and then don't wrap the error with > landlock_log_request(), as suggested by Jeff. > - Add a WARN_ON_ONCE() check to never dereference null pointers. > - Only log when audit is enabled. > - Don't log task's PID/TID with log_task() because it would be redundant > with the SYSCALL record. > - Move the "op" in front and rename "domain" to "denying_domain" to make > it more consistent with other entries. > - Don't update the request with the domain ID but add an helper to get > it from the layer masks (and in a following commit with a struct > file). > - Revamp get_domain_id_from_layer_masks() into > get_level_from_layer_masks(). > - For ptrace_traceme, log the parent domain instead of the current one. > - Add documentation. > - Rename AUDIT_LANDLOCK_DENIAL to AUDIT_LANDLOCK_DENY. > - Only log the domain ID and the target task. > - Log "blockers", which are either implicit restrictions (e.g. ptrace) > or explicit access rights (e.g. filesystem), or scopes (e.g. signal). > - Don't log LSM hook names/operations. > - Pick an audit event ID folling the IPE ones. > - Add KUnit tests. > --- > include/uapi/linux/audit.h | 3 +- > security/landlock/Makefile | 2 +- > security/landlock/audit.c | 137 ++++++++++++++++++++++++++++++++++++ > security/landlock/audit.h | 52 ++++++++++++++ > security/landlock/domain.c | 21 ++++++ > security/landlock/domain.h | 17 +++++ > security/landlock/ruleset.c | 3 + > security/landlock/task.c | 91 ++++++++++++++++++------ > 8 files changed, 302 insertions(+), 24 deletions(-) > create mode 100644 security/landlock/audit.c > create mode 100644 security/landlock/audit.h > create mode 100644 security/landlock/domain.c > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h > index 75e21a135483..60c909c396c0 100644 > --- a/include/uapi/linux/audit.h > +++ b/include/uapi/linux/audit.h > @@ -33,7 +33,7 @@ > * 1100 - 1199 user space trusted application messages > * 1200 - 1299 messages internal to the audit daemon > * 1300 - 1399 audit event messages > - * 1400 - 1499 SE Linux use > + * 1400 - 1499 access control messages
Thank you :) I'm also reminded once again that the original audit devs stubbornly used "SE Linux" instead of "SELinux" :/ > * 1500 - 1599 kernel LSPP events > * 1600 - 1699 kernel crypto events > * 1700 - 1799 kernel anomaly records > @@ -146,6 +146,7 @@ > #define AUDIT_IPE_ACCESS 1420 /* IPE denial or grant */ > #define AUDIT_IPE_CONFIG_CHANGE 1421 /* IPE config change */ > #define AUDIT_IPE_POLICY_LOAD 1422 /* IPE policy load */ > +#define AUDIT_LANDLOCK_DENY 1423 /* Landlock denial */ Generally speaking, we don't really encode denial/allowed verdicts into the audit record type, instead we ask that developers use a field like "access=" to indicate that an action was allowed or denied. How about AUDIT_LANDLOCK_ACCESS ? -- paul-moore.com
