[PATCH v3 08/23] landlock: Log ptrace denials
Mickaël Salaün
mic at digikod.net
Tue Dec 24 14:48:20 UTC 2024
On Fri, Dec 20, 2024 at 03:36:14PM +0100, Francis Laniel wrote:
> Le vendredi 22 novembre 2024, 15:33:38 CET Mickaël Salaün a écrit :
> > 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 <gnoack at google.com>
> > Cc: Paul Moore <paul at paul-moore.com>
> > Signed-off-by: Mickaël Salaün <mic at digikod.net>
> > Link: https://lore.kernel.org/r/20241122143353.59367-9-mic@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
> > * 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 */
> >
> > #define AUDIT_FIRST_KERN_ANOM_MSG 1700
> > #define AUDIT_LAST_KERN_ANOM_MSG 1799
> > diff --git a/security/landlock/Makefile b/security/landlock/Makefile
> > index e1777abbc413..31512ee4b041 100644
> > --- a/security/landlock/Makefile
> > +++ b/security/landlock/Makefile
> > @@ -5,4 +5,4 @@ landlock-y := setup.o syscalls.o object.o ruleset.o \
> >
> > landlock-$(CONFIG_INET) += net.o
> >
> > -landlock-$(CONFIG_AUDIT) += id.o
> > +landlock-$(CONFIG_AUDIT) += id.o domain.o audit.o
> > diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> > new file mode 100644
> > index 000000000000..eab6b3a8a231
> > --- /dev/null
> > +++ b/security/landlock/audit.c
> > @@ -0,0 +1,137 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Landlock LSM - Audit helpers
> > + *
> > + * Copyright © 2023-2024 Microsoft Corporation
> > + */
> > +
> > +#include <kunit/test.h>
> > +#include <linux/audit.h>
> > +#include <linux/lsm_audit.h>
> > +
> > +#include "audit.h"
> > +#include "domain.h"
> > +#include "ruleset.h"
> > +
> > +static const char *get_blocker(const enum landlock_request_type type)
> > +{
> > + switch (type) {
> > + case LANDLOCK_REQUEST_PTRACE:
> > + return "ptrace";
> > + }
> > +
> > + WARN_ON_ONCE(1);
> > + return "unknown";
> > +}
> > +
> > +static void log_blockers(struct audit_buffer *const ab,
> > + const enum landlock_request_type type)
> > +{
> > + audit_log_format(ab, "%s", get_blocker(type));
> > +}
> > +
> > +static struct landlock_hierarchy *
> > +get_hierarchy(const struct landlock_ruleset *const domain, const size_t
> > layer) +{
> > + struct landlock_hierarchy *node = domain->hierarchy;
> > + ssize_t i;
> > +
> > + if (WARN_ON_ONCE(layer >= domain->num_layers))
> > + return node;
> > +
> > + for (i = domain->num_layers - 1; i > layer; i--) {
> > + if (WARN_ON_ONCE(!node->parent))
> > + break;
> > +
> > + node = node->parent;
> > + }
> > +
> > + return node;
> > +}
> > +
> > +#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> > +
> > +static void test_get_hierarchy(struct kunit *const test)
> > +{
> > + struct landlock_hierarchy dom0_node = {
> > + .id = 10,
> > + };
> > + struct landlock_hierarchy dom1_node = {
> > + .parent = &dom0_node,
> > + .id = 20,
> > + };
> > + struct landlock_hierarchy dom2_node = {
> > + .parent = &dom1_node,
> > + .id = 30,
> > + };
> > + struct landlock_ruleset dom2 = {
> > + .hierarchy = &dom2_node,
> > + .num_layers = 3,
> > + };
> > +
> > + KUNIT_EXPECT_EQ(test, 10, get_hierarchy(&dom2, 0)->id);
> > + KUNIT_EXPECT_EQ(test, 20, get_hierarchy(&dom2, 1)->id);
> > + KUNIT_EXPECT_EQ(test, 30, get_hierarchy(&dom2, 2)->id);
> > + KUNIT_EXPECT_EQ(test, 30, get_hierarchy(&dom2, -1)->id);
> > +}
> > +
> > +#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> > +
> > +static bool is_valid_request(const struct landlock_request *const request)
> > +{
> > + if (WARN_ON_ONCE(!request->layer_plus_one))
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +/**
> > + * landlock_log_denial - Create audit records related to a denial
> > + *
> > + * @domain: The domain denying an action.
> > + * @request: Detail of the user space request.
> > + */
> > +void landlock_log_denial(const struct landlock_ruleset *const domain,
> > + const struct landlock_request *const request)
> > +{
> > + struct audit_buffer *ab;
> > + struct landlock_hierarchy *youngest_denied;
> > +
> > + if (WARN_ON_ONCE(!domain || !domain->hierarchy || !request))
> > + return;
> > +
> > + if (!is_valid_request(request))
> > + return;
> > +
> > + if (!audit_enabled)
> > + return;
> > +
> > + ab = audit_log_start(audit_context(), GFP_ATOMIC | __GFP_NOWARN,
> > + AUDIT_LANDLOCK_DENY);
> > + if (!ab)
> > + return;
> > +
> > + youngest_denied = get_hierarchy(domain, request->layer_plus_one - 1);
> > + audit_log_format(ab, "domain=%llx blockers=", youngest_denied->id);
> > + log_blockers(ab, request->type);
> > + audit_log_lsm_data(ab, &request->audit);
> > + audit_log_end(ab);
> > +}
> > +
> > +#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> > +
> > +static struct kunit_case test_cases[] = {
> > + /* clang-format off */
> > + KUNIT_CASE(test_get_hierarchy),
> > + {}
> > + /* clang-format on */
> > +};
> > +
> > +static struct kunit_suite test_suite = {
> > + .name = "landlock_audit",
> > + .test_cases = test_cases,
> > +};
> > +
> > +kunit_test_suite(test_suite);
> > +
> > +#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> > diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> > new file mode 100644
> > index 000000000000..f33095afcd2f
> > --- /dev/null
> > +++ b/security/landlock/audit.h
> > @@ -0,0 +1,52 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Landlock LSM - Audit helpers
> > + *
> > + * Copyright © 2023-2024 Microsoft Corporation
> > + */
> > +
> > +#ifndef _SECURITY_LANDLOCK_AUDIT_H
> > +#define _SECURITY_LANDLOCK_AUDIT_H
> > +
> > +#include <linux/audit.h>
> > +#include <linux/lsm_audit.h>
> > +
> > +#include "ruleset.h"
> > +
> > +enum landlock_request_type {
> > + LANDLOCK_REQUEST_PTRACE = 1,
> > +};
> > +
> > +/*
> > + * We should be careful to only use a variable of this type for
> > + * landlock_log_denial(). This way, the compiler can remove it entirely if
> > + * CONFIG_AUDIT is not set.
> > + */
> > +struct landlock_request {
> > + /* Mandatory fields. */
> > + enum landlock_request_type type;
> > + struct common_audit_data audit;
> > +
> > + /**
> > + * layer_plus_one: First layer level that denies the request + 1. The
> > + * extra one is useful to detect uninitialized field.
> > + */
> > + size_t layer_plus_one;
> > +};
> > +
> > +#ifdef CONFIG_AUDIT
> > +
> > +void landlock_log_denial(const struct landlock_ruleset *const domain,
> > + const struct landlock_request *const request);
> > +
> > +#else /* CONFIG_AUDIT */
> > +
> > +static inline void
> > +landlock_log_denial(const struct landlock_ruleset *const domain,
> > + const struct landlock_request *const request)
> > +{
> > +}
> > +
> > +#endif /* CONFIG_AUDIT */
> > +
> > +#endif /* _SECURITY_LANDLOCK_AUDIT_H */
> > diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> > new file mode 100644
> > index 000000000000..965e4dc21975
> > --- /dev/null
> > +++ b/security/landlock/domain.c
> > @@ -0,0 +1,21 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Landlock LSM - Domain management
> > + *
> > + * Copyright © 2024 Microsoft Corporation
> > + */
> > +
> > +#include "domain.h"
> > +#include "id.h"
> > +
> > +/**
> > + * landlock_init_current_hierarchy - Partially initialize
> > landlock_hierarchy + *
> > + * @hierarchy: The hierarchy to initialize.
> > + *
> > + * @hierarchy->parent and @hierarchy->usage should already be set.
> > + */
> > +void landlock_init_current_hierarchy(struct landlock_hierarchy *const
> > hierarchy) +{
> > + hierarchy->id = landlock_get_id(1);
> > +}
> > diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> > index 015d61fd81ec..f82d831ca0a7 100644
> > --- a/security/landlock/domain.h
> > +++ b/security/landlock/domain.h
> > @@ -4,6 +4,7 @@
> > *
> > * Copyright © 2016-2020 Mickaël Salaün <mic at digikod.net>
> > * Copyright © 2018-2020 ANSSI
> > + * Copyright © 2024 Microsoft Corporation
> > */
> >
> > #ifndef _SECURITY_LANDLOCK_DOMAIN_H
> > @@ -26,6 +27,13 @@ struct landlock_hierarchy {
> > * domain.
> > */
> > refcount_t usage;
> > +
> > +#ifdef CONFIG_AUDIT
> > + /**
> > + * @id: Landlock domain ID, sets once at domain creation time.
> > + */
> > + u64 id;
> > +#endif /* CONFIG_AUDIT */
> > };
> >
> > static inline void
> > @@ -45,4 +53,13 @@ static inline void landlock_put_hierarchy(struct
> > landlock_hierarchy *hierarchy) }
> > }
> >
> > +#ifdef CONFIG_AUDIT
> > +void landlock_init_current_hierarchy(struct landlock_hierarchy *const
> > hierarchy); +#else /* CONFIG_AUDIT */
> > +static inline void
> > +landlock_init_current_hierarchy(struct landlock_hierarchy *const hierarchy)
> > +{
> > +}
> > +#endif /* CONFIG_AUDIT */
> > +
> > #endif /* _SECURITY_LANDLOCK_DOMAIN_H */
> > diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> > index 4b3dfa3e6fcb..7a88fd9b2473 100644
> > --- a/security/landlock/ruleset.c
> > +++ b/security/landlock/ruleset.c
> > @@ -21,6 +21,7 @@
> > #include <linux/workqueue.h>
> >
> > #include "access.h"
> > +#include "audit.h"
> > #include "domain.h"
> > #include "limits.h"
> > #include "object.h"
> > @@ -503,6 +504,7 @@ static void free_ruleset_work(struct work_struct *const
> > work) free_ruleset(ruleset);
> > }
> >
> > +/* Only called by hook_cred_free(). */
> > void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset)
> > {
> > if (ruleset && refcount_dec_and_test(&ruleset->usage)) {
> > @@ -562,6 +564,7 @@ landlock_merge_ruleset(struct landlock_ruleset *const
> > parent, if (err)
> > goto out_put_dom;
> >
> > + landlock_init_current_hierarchy(new_dom->hierarchy);
> > return new_dom;
> >
> > out_put_dom:
> > diff --git a/security/landlock/task.c b/security/landlock/task.c
> > index 98894ad1abc7..1decd6f114e8 100644
> > --- a/security/landlock/task.c
> > +++ b/security/landlock/task.c
> > @@ -10,12 +10,14 @@
> > #include <linux/cred.h>
> > #include <linux/errno.h>
> > #include <linux/kernel.h>
> > +#include <linux/lsm_audit.h>
> > #include <linux/lsm_hooks.h>
> > #include <linux/rcupdate.h>
> > #include <linux/sched.h>
> > #include <net/af_unix.h>
> > #include <net/sock.h>
> >
> > +#include "audit.h"
> > #include "common.h"
> > #include "cred.h"
> > #include "domain.h"
> > @@ -38,41 +40,29 @@ static bool domain_scope_le(const struct
> > landlock_ruleset *const parent, {
> > const struct landlock_hierarchy *walker;
> >
> > + /* Quick return for non-landlocked tasks. */
> > if (!parent)
> > return true;
> > +
> > if (!child)
> > return false;
> > +
> > for (walker = child->hierarchy; walker; walker = walker->parent) {
> > if (walker == parent->hierarchy)
> > /* @parent is in the scoped hierarchy of @child. */
> > return true;
> > }
> > +
> > /* There is no relationship between @parent and @child. */
> > return false;
> > }
> >
> > -static bool task_is_scoped(const struct task_struct *const parent,
> > - const struct task_struct *const child)
> > -{
> > - bool is_scoped;
> > - const struct landlock_ruleset *dom_parent, *dom_child;
> > -
> > - rcu_read_lock();
> > - dom_parent = landlock_get_task_domain(parent);
> > - dom_child = landlock_get_task_domain(child);
> > - is_scoped = domain_scope_le(dom_parent, dom_child);
> > - rcu_read_unlock();
> > - return is_scoped;
> > -}
> > -
> > -static int task_ptrace(const struct task_struct *const parent,
> > - const struct task_struct *const child)
> > +static int domain_ptrace(const struct landlock_ruleset *const parent,
> > + const struct landlock_ruleset *const child)
> > {
> > - /* Quick return for non-landlocked tasks. */
> > - if (!landlocked(parent))
> > - return 0;
> > - if (task_is_scoped(parent, child))
> > + if (domain_scope_le(parent, child))
> > return 0;
> > +
> > return -EPERM;
> > }
> >
> > @@ -92,7 +82,36 @@ static int task_ptrace(const struct task_struct *const
> > parent, static int hook_ptrace_access_check(struct task_struct *const
> > child, const unsigned int mode)
> > {
> > - return task_ptrace(current, child);
> > + const struct landlock_ruleset *parent_dom, *child_dom;
> > + struct landlock_request request = {
> > + .type = LANDLOCK_REQUEST_PTRACE,
> > + .audit = {
> > + .type = LSM_AUDIT_DATA_TASK,
> > + .u.tsk = child,
> > + },
> > + };
> > + int err;
> > +
> > + /* Quick return for non-landlocked tasks. */
> > + parent_dom = landlock_get_current_domain();
> > + if (!parent_dom)
> > + return 0;
> > +
> > + rcu_read_lock();
> > + child_dom = landlock_get_task_domain(child);
> > + err = domain_ptrace(parent_dom, child_dom);
> > + rcu_read_unlock();
> > +
> > + /*
> > + * For the ptrace_access_check case, we log the current/parent domain
> > + * and the child task.
> > + */
> > + if (err && !(mode & PTRACE_MODE_NOAUDIT)) {
> > + request.layer_plus_one = parent_dom->num_layers;
> > + landlock_log_denial(parent_dom, &request);
> > + }
> > +
> > + return err;
> > }
> >
> > /**
> > @@ -109,7 +128,35 @@ static int hook_ptrace_access_check(struct task_struct
> > *const child, */
> > static int hook_ptrace_traceme(struct task_struct *const parent)
> > {
> > - return task_ptrace(parent, current);
> > + const struct landlock_ruleset *parent_dom, *child_dom;
> > + struct landlock_request request = {
> > + .type = LANDLOCK_REQUEST_PTRACE,
> > + .audit = {
> > + .type = LSM_AUDIT_DATA_TASK,
> > + .u.tsk = parent,
> > + },
> > + };
> > + int err;
> > +
> > + child_dom = landlock_get_current_domain();
> > + rcu_read_lock();
> > + parent_dom = landlock_get_task_domain(parent);
> > + err = domain_ptrace(parent_dom, child_dom);
> > +
> > + /*
> > + * For the ptrace_traceme case, we log the domain which is the cause
> of
> > + * the denial, which means the parent domain instead of the current
> > + * domain. This may look weird because the ptrace_traceme action is a
> > + * request to be traced, but the semantic is consistent with
> > + * hook_ptrace_access_check().
> > + */
> > + if (err) {
> > + request.layer_plus_one = parent_dom->num_layers;
> > + landlock_log_denial(parent_dom, &request);
> > + }
> > +
> > + rcu_read_unlock();
>
> Nit: in the above function, you do the rcu_read_unlock() before the if.
This is because the RCU read-side critical section is needed for the
"other" task's domain reference, which is parent_dom here, and child_dom
in hook_ptrace_access_check().
>
> > + return err;
> > }
> >
> > /**
>
>
>
>
More information about the Linux-security-module-archive
mailing list