[RFC PATCH v2 00/14] Landlock audit support

[RFC PATCH v2 00/14] Landlock audit support

[Mickaël Salaün]

Hi,

This patch series adds audit support to Landlock.

Logging denied requests is useful for different use cases:
* app developers: to ease and speed up sandboxing support
* power users: to understand denials
* sysadmins: to look for users' issues
* security experts: to detect attack attempts

To make logs useful, they need to contain the most relevant Landlock
domain that denied an action, and the reason of such denial.  This
translates to the latest nested domain and the related blockers: missing
access rights or other kind of constraints (e.g. scoped domain).

# Changes from previous version

This second patch series brings a full implementation with a novel
design fitted to an unprivileged access control system.

The previous approach created log records for any Landlock syscall and
denials.  We now only create log records related to denied actions.

This series does not include documentation nor user space tests yet, but
KUnit tests are provided.

# Design

Log records are created for any denied actions caused by a Landlock
policy, which means that a well-sandboxed applications should not log
anything except for unattended access requests that might be the result
of attacks or bugs.

However, sandbox tools creating restricted environments could lead to
abundant log entries because the sandboxed processes may not be aware of
the related restrictions.  To avoid log spam, the
landlock_restrict_self(2) syscall gets a new
LANDLOCK_RESTRICT_SELF_LOGLESS flag to not log denials related to this
specific domain.  Except for well-understood exceptions, this flag
should not be set.  Indeed, applications sandboxing themselves should
only try to bypass their own sandbox if they are compromised, which
should ring a bell thanks to log events.

When an action is denied, the related Landlock domain ID is specified.
If this domain was not previously described in a log record, one is
created.  This record contains the domain ID, the domain ID of its parent
domain (or 0 if none), and informations about the process that enforced
the restriction (at the time of the call to landlock_restrict_self):
PID, UID, executable path, and name (comm).

This new approach also brings building blocks for an upcoming
unprivileged introspection interface.  The unique Landlock IDs will be
useful to tie audit log entries to running processes, and to get
properties of the related Landlock domains.  This will replace the
previously logged ruleset properties.

# Samples

Here are two examples of log events:

$ LL_FS_RO=/ LL_FS_RW=/ ./sandboxer sh -c "LL_FS_RO=/ LL_FS_RW=/tmp LL_SCOPED=s ./sandboxer kill 1"

  type=UNKNOWN[1423] msg=audit(1.102:31): domain=5264859566 blockers=scope_signal opid=1 ocomm="systemd"
  type=UNKNOWN[1424] msg=audit(1.102:31): domain=5264859566 parent=5264859553 pid=290 uid=0 exe="/root/sandboxer" comm="sandboxer"
  type=UNKNOWN[1424] msg=audit(1.102:31): domain=5264859553 parent=0 pid=290 uid=0 exe="/root/sandboxer" comm="sandboxer"
  type=SYSCALL msg=audit(1.102:31): arch=c000003e syscall=62 success=no exit=-1 ...
  type=PROCTITLE msg=audit(1.102:31): proctitle=...
  type=UNKNOWN[1425] msg=audit(1.158:32): domain=5264859566
  type=UNKNOWN[1425] msg=audit(1.182:33): domain=5264859553

$ LL_FS_RO=/ LL_FS_RW=/tmp ./sandboxer sh -c "echo > /etc/passwd"

  type=UNKNOWN[1423] msg=audit(2.832:37): domain=5264859570 blockers=fs_write_file path="/etc/passwd" dev="vda2" ino=143821
  type=UNKNOWN[1424] msg=audit(2.832:37): domain=5264859570 parent=0 pid=296 uid=0 exe="/root/sandboxer" comm="sandboxer"
  type=SYSCALL msg=audit(2.832:37): arch=c000003e syscall=257 success=no exit=-13 ...
  type=PROCTITLE msg=audit(2.832:37): proctitle=...
  type=UNKNOWN[1425] msg=audit(2.892:38): domain=5264859570

# Future changes

It would be interesting to enhance audit with the ability to filter on
the executable path that created a sandbox, or to filter on a Landlock
domain ID.


This series is based on my "next" branch, which includes these patches:
https://lore.kernel.org/r/20241022151144.872797-2-mic@digikod.net

Previous version:
v1: https://lore.kernel.org/r/20230921061641.273654-1-mic@digikod.net

Regards,

Mickaël Salaün (14):
  lsm: Only build lsm_audit.c if CONFIG_AUDIT is set
  lsm: Add audit_log_lsm_data() helper
  landlock: Factor out check_access_path()
  landlock: Add unique ID generator
  landlock: Move access types
  landlock: Move domain hierarchy management
  landlock: Log ptrace denials
  landlock: Log domain properties and release
  landlock: Log mount-related denials
  landlock: Log file-related denials
  landlock: Log truncate and ioctl denials
  landlock: Log TCP bind and connect denials
  landlock: Log scoped denials
  landlock: Control log events with LANDLOCK_RESTRICT_SELF_LOGLESS

 include/linux/lsm_audit.h                    |  22 +
 include/uapi/linux/audit.h                   |   5 +-
 include/uapi/linux/landlock.h                |  14 +
 security/Makefile                            |   2 +-
 security/landlock/.kunitconfig               |   2 +
 security/landlock/Makefile                   |   2 +
 security/landlock/access.h                   |  70 +++
 security/landlock/audit.c                    | 493 +++++++++++++++++++
 security/landlock/audit.h                    |  76 +++
 security/landlock/domain.c                   | 184 +++++++
 security/landlock/domain.h                   | 111 +++++
 security/landlock/fs.c                       | 210 ++++++--
 security/landlock/fs.h                       |  10 +
 security/landlock/id.c                       | 242 +++++++++
 security/landlock/id.h                       |  25 +
 security/landlock/net.c                      |  52 +-
 security/landlock/ruleset.c                  |  31 +-
 security/landlock/ruleset.h                  |  80 ++-
 security/landlock/setup.c                    |   2 +
 security/landlock/syscalls.c                 |  26 +-
 security/landlock/task.c                     | 150 +++++-
 security/lsm_audit.c                         |  27 +-
 tools/testing/kunit/configs/all_tests.config |   2 +
 23 files changed, 1692 insertions(+), 146 deletions(-)
 create mode 100644 security/landlock/access.h
 create mode 100644 security/landlock/audit.c
 create mode 100644 security/landlock/audit.h
 create mode 100644 security/landlock/domain.c
 create mode 100644 security/landlock/domain.h
 create mode 100644 security/landlock/id.c
 create mode 100644 security/landlock/id.h


base-commit: 2798d07e6d416164119e83c2cd1bb50160297ec8
-- 
2.47.0

[RFC PATCH v2 01/14] lsm: Only build lsm_audit.c if CONFIG_AUDIT is set

[Mickaël Salaün]

When CONFIG_AUDIT is set, its CONFIG_NET dependency is also set, and the
dev_get_by_index and init_net symbols (used by dump_common_audit_data)
are found by the linker.  dump_common_audit_data() should then failed to
build when CONFIG_NET is not set. However, because the compiler is
smart, it knows that audit_log_start() always return NULL when
!CONFIG_AUDIT, and it doesn't build the body of common_lsm_audit().  As
a side effect, dump_common_audit_data() is not built and the linker
doesn't error out because of missing symbols.

Let's only build lsm_audit.o when CONFIG_AUDIT is set.

ipv4_skb_to_auditdata() and ipv6_skb_to_auditdata() are only used by
Smack if CONFIG_AUDIT is set, so they don't need fake implementations.

Because common_lsm_audit() is used in multiple places without
CONFIG_AUDIT checks, add a fake implementation.

Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: James Morris <jmorris@namei.org>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-2-mic@digikod.net
---
 include/linux/lsm_audit.h | 14 ++++++++++++++
 security/Makefile         |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index 97a8b21eb033..c2b01380262c 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -116,14 +116,28 @@ struct common_audit_data {
 #define v4info fam.v4
 #define v6info fam.v6
 
+#ifdef CONFIG_AUDIT
+
 int ipv4_skb_to_auditdata(struct sk_buff *skb,
 		struct common_audit_data *ad, u8 *proto);
 
+#if IS_ENABLED(CONFIG_IPV6)
 int ipv6_skb_to_auditdata(struct sk_buff *skb,
 		struct common_audit_data *ad, u8 *proto);
+#endif /* IS_ENABLED(CONFIG_IPV6) */
 
 void common_lsm_audit(struct common_audit_data *a,
 	void (*pre_audit)(struct audit_buffer *, void *),
 	void (*post_audit)(struct audit_buffer *, void *));
 
+#else /* CONFIG_AUDIT */
+
+static inline void common_lsm_audit(struct common_audit_data *a,
+	void (*pre_audit)(struct audit_buffer *, void *),
+	void (*post_audit)(struct audit_buffer *, void *))
+{
+}
+
+#endif /* CONFIG_AUDIT */
+
 #endif
diff --git a/security/Makefile b/security/Makefile
index cc0982214b84..e25da79f55d3 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -15,7 +15,7 @@ obj-$(CONFIG_SECURITY)			+= security.o
 obj-$(CONFIG_SECURITYFS)		+= inode.o
 obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/
 obj-$(CONFIG_SECURITY_SMACK)		+= smack/
-obj-$(CONFIG_SECURITY)			+= lsm_audit.o
+obj-$(CONFIG_AUDIT)			+= lsm_audit.o
 obj-$(CONFIG_SECURITY_TOMOYO)		+= tomoyo/
 obj-$(CONFIG_SECURITY_APPARMOR)		+= apparmor/
 obj-$(CONFIG_SECURITY_YAMA)		+= yama/
-- 
2.47.0

[RFC PATCH v2 02/14] lsm: Add audit_log_lsm_data() helper

[Mickaël Salaün]

Extract code from dump_common_audit_data() into the audit_log_lsm_data()
helper. This helps reuse common LSM audit data while not abusing
AUDIT_AVC records because of the common_lsm_audit() helper.

Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: James Morris <jmorris@namei.org>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-3-mic@digikod.net
---

Changes since v1:
* Fix commit message (spotted by Paul).
* Constify dump_common_audit_data()'s and audit_log_lsm_data()'s "a"
  argument.
* Fix build without CONFIG_NET: see previous patch.
---
 include/linux/lsm_audit.h |  8 ++++++++
 security/lsm_audit.c      | 27 ++++++++++++++++++---------
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index c2b01380262c..b62769a7c5fa 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -130,6 +130,9 @@ void common_lsm_audit(struct common_audit_data *a,
 	void (*pre_audit)(struct audit_buffer *, void *),
 	void (*post_audit)(struct audit_buffer *, void *));
 
+void audit_log_lsm_data(struct audit_buffer *ab,
+			const struct common_audit_data *a);
+
 #else /* CONFIG_AUDIT */
 
 static inline void common_lsm_audit(struct common_audit_data *a,
@@ -138,6 +141,11 @@ static inline void common_lsm_audit(struct common_audit_data *a,
 {
 }
 
+static inline void audit_log_lsm_data(struct audit_buffer *ab,
+			const struct common_audit_data *a)
+{
+}
+
 #endif /* CONFIG_AUDIT */
 
 #endif
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 849e832719e2..de29ce8ff708 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -189,16 +189,13 @@ static inline void print_ipv4_addr(struct audit_buffer *ab, __be32 addr,
 }
 
 /**
- * dump_common_audit_data - helper to dump common audit data
+ * audit_log_lsm_data - helper to log common LSM audit data
  * @ab : the audit buffer
  * @a : common audit data
- *
  */
-static void dump_common_audit_data(struct audit_buffer *ab,
-				   struct common_audit_data *a)
+void audit_log_lsm_data(struct audit_buffer *ab,
+			const struct common_audit_data *a)
 {
-	char comm[sizeof(current->comm)];
-
 	/*
 	 * To keep stack sizes in check force programmers to notice if they
 	 * start making this union too large!  See struct lsm_network_audit
@@ -206,9 +203,6 @@ static void dump_common_audit_data(struct audit_buffer *ab,
 	 */
 	BUILD_BUG_ON(sizeof(a->u) > sizeof(void *)*2);
 
-	audit_log_format(ab, " pid=%d comm=", task_tgid_nr(current));
-	audit_log_untrustedstring(ab, memcpy(comm, current->comm, sizeof(comm)));
-
 	switch (a->type) {
 	case LSM_AUDIT_DATA_NONE:
 		return;
@@ -428,6 +422,21 @@ static void dump_common_audit_data(struct audit_buffer *ab,
 	} /* switch (a->type) */
 }
 
+/**
+ * dump_common_audit_data - helper to dump common audit data
+ * @ab : the audit buffer
+ * @a : common audit data
+ */
+static void dump_common_audit_data(struct audit_buffer *ab,
+				   const struct common_audit_data *a)
+{
+	char comm[sizeof(current->comm)];
+
+	audit_log_format(ab, " pid=%d comm=", task_tgid_nr(current));
+	audit_log_untrustedstring(ab, memcpy(comm, current->comm, sizeof(comm)));
+	audit_log_lsm_data(ab, a);
+}
+
 /**
  * common_lsm_audit - generic LSM auditing function
  * @a:  auxiliary audit data
-- 
2.47.0

[RFC PATCH v2 03/14] landlock: Factor out check_access_path()

[Mickaël Salaün]

Merge check_access_path() into current_check_access_path() and make
hook_path_mknod() use it.

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-4-mic@digikod.net
---

Changes since v1:
* Rebased on the TCP patch series.
* Remove inlining removal which was merged.
---
 security/landlock/fs.c | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index dd9a7297ea4e..698a623a8184 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -907,28 +907,22 @@ static bool is_access_to_paths_allowed(
 	return allowed_parent1 && allowed_parent2;
 }
 
-static int check_access_path(const struct landlock_ruleset *const domain,
-			     const struct path *const path,
-			     access_mask_t access_request)
-{
-	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
-
-	access_request = landlock_init_layer_masks(
-		domain, access_request, &layer_masks, LANDLOCK_KEY_INODE);
-	if (is_access_to_paths_allowed(domain, path, access_request,
-				       &layer_masks, NULL, 0, NULL, NULL))
-		return 0;
-	return -EACCES;
-}
-
 static int current_check_access_path(const struct path *const path,
-				     const access_mask_t access_request)
+				     access_mask_t access_request)
 {
 	const struct landlock_ruleset *const dom = get_current_fs_domain();
+	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
 
 	if (!dom)
 		return 0;
-	return check_access_path(dom, path, access_request);
+
+	access_request = landlock_init_layer_masks(
+		dom, access_request, &layer_masks, LANDLOCK_KEY_INODE);
+	if (is_access_to_paths_allowed(dom, path, access_request, &layer_masks,
+				       NULL, 0, NULL, NULL))
+		return 0;
+
+	return -EACCES;
 }
 
 static access_mask_t get_mode_access(const umode_t mode)
@@ -1413,11 +1407,7 @@ static int hook_path_mknod(const struct path *const dir,
 			   struct dentry *const dentry, const umode_t mode,
 			   const unsigned int dev)
 {
-	const struct landlock_ruleset *const dom = get_current_fs_domain();
-
-	if (!dom)
-		return 0;
-	return check_access_path(dom, dir, get_mode_access(mode));
+	return current_check_access_path(dir, get_mode_access(mode));
 }
 
 static int hook_path_symlink(const struct path *const dir,
-- 
2.47.0

[RFC PATCH v2 04/14] landlock: Add unique ID generator

[Mickaël Salaün]

Landlock IDs can be generated to uniquely identify Landlock objects.
For now, only Landlock domains get an ID at creation time.

These IDs have important properties:
* They are unique during the lifetime of the running system thanks to
  the 64-bit values: at worse, 2^60 - 2*2^32 useful IDs.
* They are always greater than 2^32 and must then be stored in 64-bit
  integer types.
* The initial ID (at boot time) is randomly picked between 2^32 and
  2^33, which limits collisions in logs between different boots.
* IDs are sequential, which enables users to order them.
* IDs may not be consecutive but increase with a random 2^4 step, which
  limits side channels.

Such IDs can be exposed to unprivileged processes, even if it is not the
case with this audit patch series.  The domain IDs will be useful for
user space to identify sandboxes and get their properties.

Cc: Günther Noack <gnoack@google.com>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-5-mic@digikod.net
---

Changes since v1:
* New patch.
---
 security/landlock/.kunitconfig               |   2 +
 security/landlock/Makefile                   |   2 +
 security/landlock/id.c                       | 242 +++++++++++++++++++
 security/landlock/id.h                       |  25 ++
 security/landlock/setup.c                    |   2 +
 tools/testing/kunit/configs/all_tests.config |   2 +
 6 files changed, 275 insertions(+)
 create mode 100644 security/landlock/id.c
 create mode 100644 security/landlock/id.h

diff --git a/security/landlock/.kunitconfig b/security/landlock/.kunitconfig
index 03e119466604..f9423f01ac5b 100644
--- a/security/landlock/.kunitconfig
+++ b/security/landlock/.kunitconfig
@@ -1,4 +1,6 @@
+CONFIG_AUDIT=y
 CONFIG_KUNIT=y
+CONFIG_NET=y
 CONFIG_SECURITY=y
 CONFIG_SECURITY_LANDLOCK=y
 CONFIG_SECURITY_LANDLOCK_KUNIT_TEST=y
diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index b4538b7cf7d2..e1777abbc413 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -4,3 +4,5 @@ landlock-y := setup.o syscalls.o object.o ruleset.o \
 	cred.o task.o fs.o
 
 landlock-$(CONFIG_INET) += net.o
+
+landlock-$(CONFIG_AUDIT) += id.o
diff --git a/security/landlock/id.c b/security/landlock/id.c
new file mode 100644
index 000000000000..5d0b7743c308
--- /dev/null
+++ b/security/landlock/id.c
@@ -0,0 +1,242 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - Unique identification number generator
+ *
+ * Copyright © 2024 Microsoft Corporation
+ */
+
+#include <kunit/test.h>
+#include <linux/atomic.h>
+#include <linux/random.h>
+#include <linux/spinlock.h>
+
+#include "common.h"
+#include "id.h"
+
+#define COUNTER_PRE_INIT 0
+
+static atomic64_t global_counter = ATOMIC64_INIT(COUNTER_PRE_INIT);
+
+static void __init init_id(atomic64_t *const counter, const u32 random_32bits)
+{
+	u64 init;
+
+	/*
+	 * Ensures sure 64-bit values are always used by user space (or may
+	 * fail with -EOVERFLOW), and makes this testable.
+	 */
+	init = 1ULL << 32;
+
+	/*
+	 * Makes a large (2^32) boot-time value to limit ID collision in logs
+	 * from different boots, and to limit info leak about the number of
+	 * initially (relative to the reader) created elements (e.g. domains).
+	 */
+	init += random_32bits;
+
+	/* Sets first or ignores.  This will be the first ID. */
+	atomic64_cmpxchg(counter, COUNTER_PRE_INIT, init);
+}
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+static void test_init_min(struct kunit *const test)
+{
+	atomic64_t counter = ATOMIC64_INIT(COUNTER_PRE_INIT);
+
+	init_id(&counter, 0);
+	KUNIT_EXPECT_EQ(test, atomic64_read(&counter), 1ULL + U32_MAX);
+}
+
+static void test_init_max(struct kunit *const test)
+{
+	atomic64_t counter = ATOMIC64_INIT(COUNTER_PRE_INIT);
+
+	init_id(&counter, ~0);
+	KUNIT_EXPECT_EQ(test, atomic64_read(&counter), 1 + (2ULL * U32_MAX));
+}
+
+static void test_init_once(struct kunit *const test)
+{
+	const u64 first_init = 1ULL + U32_MAX;
+	atomic64_t counter = ATOMIC64_INIT(COUNTER_PRE_INIT);
+
+	init_id(&counter, 0);
+	KUNIT_EXPECT_EQ(test, atomic64_read(&counter), first_init);
+
+	init_id(&counter, ~0);
+	KUNIT_EXPECT_EQ(test, atomic64_read(&counter), first_init);
+}
+
+#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
+
+void __init landlock_init_id(void)
+{
+	return init_id(&global_counter, get_random_u32());
+}
+
+/*
+ * It's not worth it to try to hide the monotonic counter because it can still
+ * be inferred (with N counter ranges), and if we are allowed to read the inode
+ * number we should also be allowed to read the time creation anyway, and it
+ * can be handy to store and sort domain IDs for user space.
+ *
+ * Returns the value of global_counter and increment it to let some space for
+ * the next one.
+ */
+static u64 get_id(size_t number_of_ids, atomic64_t *const counter,
+		  u8 random_4bits)
+{
+	u64 id, step;
+
+	/*
+	 * We should return at least 1 ID, and we may need a set of consecutive
+	 * ones (e.g. to generate a set of inodes).
+	 */
+	if (WARN_ON_ONCE(number_of_ids <= 0))
+		number_of_ids = 1;
+
+	/*
+	 * Blurs the next ID guess with 1/16 ratio.  We get 2^(64 - 4) -
+	 * (2 * 2^32), so a bit less than 2^60 available IDs, which should be
+	 * much more than enough considering the number of CPU cycles required
+	 * to get a new ID (e.g. a full landlock_restrict_self() call), and the
+	 * cost of draining all available IDs during the system's uptime.
+	 */
+	random_4bits = random_4bits % (1 << 4);
+	step = number_of_ids + random_4bits;
+
+	/* It is safe to cast a signed atomic to an unsigned value. */
+	id = atomic64_fetch_add(step, counter);
+
+	/* Warns if landlock_init_id() was not called. */
+	WARN_ON_ONCE(id == COUNTER_PRE_INIT);
+	return id;
+}
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+static void test_range1_rand0(struct kunit *const test)
+{
+	atomic64_t counter;
+	u64 init;
+
+	init = get_random_u32();
+	atomic64_set(&counter, init);
+	KUNIT_EXPECT_EQ(test, get_id(1, &counter, 0), init);
+	KUNIT_EXPECT_EQ(test,
+			get_id(get_random_u8(), &counter, get_random_u8()),
+			init + 1);
+}
+
+static void test_range1_rand1(struct kunit *const test)
+{
+	atomic64_t counter;
+	u64 init;
+
+	init = get_random_u32();
+	atomic64_set(&counter, init);
+	KUNIT_EXPECT_EQ(test, get_id(1, &counter, 1), init);
+	KUNIT_EXPECT_EQ(test,
+			get_id(get_random_u8(), &counter, get_random_u8()),
+			init + 2);
+}
+
+static void test_range1_rand16(struct kunit *const test)
+{
+	atomic64_t counter;
+	u64 init;
+
+	init = get_random_u32();
+	atomic64_set(&counter, init);
+	KUNIT_EXPECT_EQ(test, get_id(1, &counter, 16), init);
+	KUNIT_EXPECT_EQ(test,
+			get_id(get_random_u8(), &counter, get_random_u8()),
+			init + 1);
+}
+
+static void test_range2_rand0(struct kunit *const test)
+{
+	atomic64_t counter;
+	u64 init;
+
+	init = get_random_u32();
+	atomic64_set(&counter, init);
+	KUNIT_EXPECT_EQ(test, get_id(2, &counter, 0), init);
+	KUNIT_EXPECT_EQ(test,
+			get_id(get_random_u8(), &counter, get_random_u8()),
+			init + 2);
+}
+
+static void test_range2_rand1(struct kunit *const test)
+{
+	atomic64_t counter;
+	u64 init;
+
+	init = get_random_u32();
+	atomic64_set(&counter, init);
+	KUNIT_EXPECT_EQ(test, get_id(2, &counter, 1), init);
+	KUNIT_EXPECT_EQ(test,
+			get_id(get_random_u8(), &counter, get_random_u8()),
+			init + 3);
+}
+
+static void test_range2_rand2(struct kunit *const test)
+{
+	atomic64_t counter;
+	u64 init;
+
+	init = get_random_u32();
+	atomic64_set(&counter, init);
+	KUNIT_EXPECT_EQ(test, get_id(2, &counter, 2), init);
+	KUNIT_EXPECT_EQ(test,
+			get_id(get_random_u8(), &counter, get_random_u8()),
+			init + 4);
+}
+
+static void test_range2_rand16(struct kunit *const test)
+{
+	atomic64_t counter;
+	u64 init;
+
+	init = get_random_u32();
+	atomic64_set(&counter, init);
+	KUNIT_EXPECT_EQ(test, get_id(2, &counter, 16), init);
+	KUNIT_EXPECT_EQ(test,
+			get_id(get_random_u8(), &counter, get_random_u8()),
+			init + 2);
+}
+
+#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
+
+u64 landlock_get_id(size_t number_of_ids)
+{
+	return get_id(number_of_ids, &global_counter, get_random_u8());
+}
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+static struct kunit_case test_cases[] = {
+	/* clang-format off */
+	KUNIT_CASE(test_init_min),
+	KUNIT_CASE(test_init_max),
+	KUNIT_CASE(test_init_once),
+	KUNIT_CASE(test_range1_rand0),
+	KUNIT_CASE(test_range1_rand1),
+	KUNIT_CASE(test_range1_rand16),
+	KUNIT_CASE(test_range2_rand0),
+	KUNIT_CASE(test_range2_rand1),
+	KUNIT_CASE(test_range2_rand2),
+	KUNIT_CASE(test_range2_rand16),
+	{}
+	/* clang-format on */
+};
+
+static struct kunit_suite test_suite = {
+	.name = "landlock_id",
+	.test_cases = test_cases,
+};
+
+kunit_test_suite(test_suite);
+
+#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
diff --git a/security/landlock/id.h b/security/landlock/id.h
new file mode 100644
index 000000000000..689ba7607472
--- /dev/null
+++ b/security/landlock/id.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - Unique identification number generator
+ *
+ * Copyright © 2024 Microsoft Corporation
+ */
+
+#ifndef _SECURITY_LANDLOCK_ID_H
+#define _SECURITY_LANDLOCK_ID_H
+
+#ifdef CONFIG_AUDIT
+
+void __init landlock_init_id(void);
+
+u64 landlock_get_id(size_t number_of_ids);
+
+#else /* CONFIG_AUDIT */
+
+static inline void __init landlock_init_id(void)
+{
+}
+
+#endif /* CONFIG_AUDIT */
+
+#endif /* _SECURITY_LANDLOCK_ID_H */
diff --git a/security/landlock/setup.c b/security/landlock/setup.c
index 28519a45b11f..d297083efcb1 100644
--- a/security/landlock/setup.c
+++ b/security/landlock/setup.c
@@ -13,6 +13,7 @@
 #include "common.h"
 #include "cred.h"
 #include "fs.h"
+#include "id.h"
 #include "net.h"
 #include "setup.h"
 #include "task.h"
@@ -33,6 +34,7 @@ const struct lsm_id landlock_lsmid = {
 
 static int __init landlock_init(void)
 {
+	landlock_init_id();
 	landlock_add_cred_hooks();
 	landlock_add_task_hooks();
 	landlock_add_fs_hooks();
diff --git a/tools/testing/kunit/configs/all_tests.config b/tools/testing/kunit/configs/all_tests.config
index b3b00269a52a..ea1f824ae70f 100644
--- a/tools/testing/kunit/configs/all_tests.config
+++ b/tools/testing/kunit/configs/all_tests.config
@@ -44,6 +44,8 @@ CONFIG_DAMON_DBGFS_DEPRECATED=y
 
 CONFIG_REGMAP_BUILD=y
 
+CONFIG_AUDIT=y
+
 CONFIG_SECURITY=y
 CONFIG_SECURITY_APPARMOR=y
 CONFIG_SECURITY_LANDLOCK=y
-- 
2.47.0

[RFC PATCH v2 05/14] landlock: Move access types

[Mickaël Salaün]

Move ACCESS_FS_OPTIONAL, access_mask_t, struct access_mask, and struct
access_masks_all to a dedicated access.h file.

This file will be extended with a following commit, and it will help to
avoid dependency loops.

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-6-mic@digikod.net
---

Changes since v1:
* New patch
---
 security/landlock/access.h  | 53 +++++++++++++++++++++++++++++++++++++
 security/landlock/fs.c      |  1 +
 security/landlock/fs.h      |  1 +
 security/landlock/ruleset.h | 31 +---------------------
 4 files changed, 56 insertions(+), 30 deletions(-)
 create mode 100644 security/landlock/access.h

diff --git a/security/landlock/access.h b/security/landlock/access.h
new file mode 100644
index 000000000000..2659fd9b4aaf
--- /dev/null
+++ b/security/landlock/access.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - Access types and helpers
+ *
+ * Copyright © 2016-2020 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2020 ANSSI
+ * Copyright © 2024 Microsoft Corporation
+ */
+
+#ifndef _SECURITY_LANDLOCK_ACCESS_H
+#define _SECURITY_LANDLOCK_ACCESS_H
+
+#include <uapi/linux/landlock.h>
+
+#include "limits.h"
+
+/* clang-format off */
+#define ACCESS_FS_OPTIONAL ( \
+	LANDLOCK_ACCESS_FS_TRUNCATE | \
+	LANDLOCK_ACCESS_FS_IOCTL_DEV)
+/* clang-format on */
+
+typedef u16 access_mask_t;
+/* Makes sure all filesystem access rights can be stored. */
+static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
+/* Makes sure all network access rights can be stored. */
+static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET);
+/* Makes sure all scoped rights can be stored. */
+static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_SCOPE);
+/* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
+static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
+
+/* Ruleset access masks. */
+struct access_masks {
+	access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
+	access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
+	access_mask_t scope : LANDLOCK_NUM_SCOPE;
+};
+
+union access_masks_all {
+	struct access_masks masks;
+	u32 all;
+};
+
+/* Makes sure all fields are covered. */
+static_assert(sizeof(((union access_masks_all *)NULL)->masks) ==
+	      sizeof(((union access_masks_all *)NULL)->all));
+
+typedef u16 layer_mask_t;
+/* Makes sure all layers can be checked. */
+static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
+
+#endif /* _SECURITY_LANDLOCK_ACCESS_H */
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 698a623a8184..e0e5775b75ae 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -36,6 +36,7 @@
 #include <uapi/linux/fiemap.h>
 #include <uapi/linux/landlock.h>
 
+#include "access.h"
 #include "common.h"
 #include "cred.h"
 #include "fs.h"
diff --git a/security/landlock/fs.h b/security/landlock/fs.h
index 1487e1f023a1..d445f411c26a 100644
--- a/security/landlock/fs.h
+++ b/security/landlock/fs.h
@@ -13,6 +13,7 @@
 #include <linux/init.h>
 #include <linux/rcupdate.h>
 
+#include "access.h"
 #include "ruleset.h"
 #include "setup.h"
 
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index e00edcb38c5b..7921bbe01344 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -17,6 +17,7 @@
 #include <linux/workqueue.h>
 #include <uapi/linux/landlock.h>
 
+#include "access.h"
 #include "limits.h"
 #include "object.h"
 
@@ -30,36 +31,6 @@
 	LANDLOCK_ACCESS_FS_REFER)
 /* clang-format on */
 
-typedef u16 access_mask_t;
-/* Makes sure all filesystem access rights can be stored. */
-static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
-/* Makes sure all network access rights can be stored. */
-static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET);
-/* Makes sure all scoped rights can be stored. */
-static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_SCOPE);
-/* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
-static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
-
-/* Ruleset access masks. */
-struct access_masks {
-	access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
-	access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
-	access_mask_t scope : LANDLOCK_NUM_SCOPE;
-};
-
-union access_masks_all {
-	struct access_masks masks;
-	u32 all;
-};
-
-/* Makes sure all fields are covered. */
-static_assert(sizeof(((union access_masks_all *)NULL)->masks) ==
-	      sizeof(((union access_masks_all *)NULL)->all));
-
-typedef u16 layer_mask_t;
-/* Makes sure all layers can be checked. */
-static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
-
 /**
  * struct landlock_layer - Access rights for a given layer
  */
-- 
2.47.0

[RFC PATCH v2 06/14] landlock: Move domain hierarchy management

[Mickaël Salaün]

Create a new domain.h file containing the struct landlock_hierarchy
definition and helpers.  This type will grow with audit support.  This
also prepares for a new domain type.

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-7-mic@digikod.net
---

Changes since v1:
* New patch.
---
 security/landlock/domain.h  | 48 +++++++++++++++++++++++++++++++++++++
 security/landlock/ruleset.c | 21 +++-------------
 security/landlock/ruleset.h | 17 +------------
 security/landlock/task.c    |  1 +
 4 files changed, 53 insertions(+), 34 deletions(-)
 create mode 100644 security/landlock/domain.h

diff --git a/security/landlock/domain.h b/security/landlock/domain.h
new file mode 100644
index 000000000000..015d61fd81ec
--- /dev/null
+++ b/security/landlock/domain.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - Domain management
+ *
+ * Copyright © 2016-2020 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2020 ANSSI
+ */
+
+#ifndef _SECURITY_LANDLOCK_DOMAIN_H
+#define _SECURITY_LANDLOCK_DOMAIN_H
+
+#include <linux/mm.h>
+#include <linux/refcount.h>
+
+/**
+ * struct landlock_hierarchy - Node in a domain hierarchy
+ */
+struct landlock_hierarchy {
+	/**
+	 * @parent: Pointer to the parent node, or NULL if it is a root
+	 * Landlock domain.
+	 */
+	struct landlock_hierarchy *parent;
+	/**
+	 * @usage: Number of potential children domains plus their parent
+	 * domain.
+	 */
+	refcount_t usage;
+};
+
+static inline void
+landlock_get_hierarchy(struct landlock_hierarchy *const hierarchy)
+{
+	if (hierarchy)
+		refcount_inc(&hierarchy->usage);
+}
+
+static inline void landlock_put_hierarchy(struct landlock_hierarchy *hierarchy)
+{
+	while (hierarchy && refcount_dec_and_test(&hierarchy->usage)) {
+		const struct landlock_hierarchy *const freeme = hierarchy;
+
+		hierarchy = hierarchy->parent;
+		kfree(freeme);
+	}
+}
+
+#endif /* _SECURITY_LANDLOCK_DOMAIN_H */
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index a93bdbf52fff..57cb7dcd6333 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -20,6 +20,7 @@
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 
+#include "domain.h"
 #include "limits.h"
 #include "object.h"
 #include "ruleset.h"
@@ -304,22 +305,6 @@ int landlock_insert_rule(struct landlock_ruleset *const ruleset,
 	return insert_rule(ruleset, id, &layers, ARRAY_SIZE(layers));
 }
 
-static void get_hierarchy(struct landlock_hierarchy *const hierarchy)
-{
-	if (hierarchy)
-		refcount_inc(&hierarchy->usage);
-}
-
-static void put_hierarchy(struct landlock_hierarchy *hierarchy)
-{
-	while (hierarchy && refcount_dec_and_test(&hierarchy->usage)) {
-		const struct landlock_hierarchy *const freeme = hierarchy;
-
-		hierarchy = hierarchy->parent;
-		kfree(freeme);
-	}
-}
-
 static int merge_tree(struct landlock_ruleset *const dst,
 		      struct landlock_ruleset *const src,
 		      const enum landlock_key_type key_type)
@@ -473,7 +458,7 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
 		err = -EINVAL;
 		goto out_unlock;
 	}
-	get_hierarchy(parent->hierarchy);
+	landlock_get_hierarchy(parent->hierarchy);
 	child->hierarchy->parent = parent->hierarchy;
 
 out_unlock:
@@ -497,7 +482,7 @@ static void free_ruleset(struct landlock_ruleset *const ruleset)
 		free_rule(freeme, LANDLOCK_KEY_NET_PORT);
 #endif /* IS_ENABLED(CONFIG_INET) */
 
-	put_hierarchy(ruleset->hierarchy);
+	landlock_put_hierarchy(ruleset->hierarchy);
 	kfree(ruleset);
 }
 
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 7921bbe01344..d0a60a5f7cd9 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -18,6 +18,7 @@
 #include <uapi/linux/landlock.h>
 
 #include "access.h"
+#include "domain.h"
 #include "limits.h"
 #include "object.h"
 
@@ -119,22 +120,6 @@ struct landlock_rule {
 	struct landlock_layer layers[] __counted_by(num_layers);
 };
 
-/**
- * struct landlock_hierarchy - Node in a ruleset hierarchy
- */
-struct landlock_hierarchy {
-	/**
-	 * @parent: Pointer to the parent node, or NULL if it is a root
-	 * Landlock domain.
-	 */
-	struct landlock_hierarchy *parent;
-	/**
-	 * @usage: Number of potential children domains plus their parent
-	 * domain.
-	 */
-	refcount_t usage;
-};
-
 /**
  * struct landlock_ruleset - Landlock ruleset
  *
diff --git a/security/landlock/task.c b/security/landlock/task.c
index e7f45af87ff5..23b56b3772c6 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -18,6 +18,7 @@
 
 #include "common.h"
 #include "cred.h"
+#include "domain.h"
 #include "fs.h"
 #include "ruleset.h"
 #include "setup.h"
-- 
2.47.0

[RFC PATCH v2 07/14] landlock: Log ptrace denials

[Mickaël Salaün]

Add audit support to ptrace_access_check and ptrace_traceme hooks.

Add a new AUDIT_LANDLOCK_DENY event dedicated to any Landlock denials.

Log the domain ID restricting the action, the domain's blockers that are
missing to allow the access, and the target task.

The blockers are either 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 move from task_ptrace() to
each LSM hooks.

Audit record sample:

  DENY:    domain=4533720530 blockers=ptrace opid=1 ocomm="systemd"
  SYSCALL: arch=c000003e syscall=101 success=no exit=-1 ...

Cc: Günther Noack <gnoack@google.com>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-8-mic@digikod.net
---

Changes since v2:
* 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.

Changes since v1:
* Move most audit code to this patch.
* Rebased 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 AUDIT_LANDLOCK_DENIAL event.
* Add a WARN_ON_ONCE() check to never dereference null pointers.
* Add and use get_domain_index() helper, which will also be useful in
  a following patch.
* 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).
* Rename "domain=" to domain_index=" for domain denial record.
* 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 "ptrace" operation to "ptrace_access".
* Rename AUDIT_LANDLOCK_DENIAL into AUDIT_LANDLOCK_DENY_ACCESS.
---
 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..3c3f831f828f
--- /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, 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=%llu 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@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 57cb7dcd6333..752dbadbe5ca 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -20,6 +20,7 @@
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 
+#include "audit.h"
 #include "domain.h"
 #include "limits.h"
 #include "object.h"
@@ -501,6 +502,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)) {
@@ -560,6 +562,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 23b56b3772c6..8c4468fb86bf 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 looks 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();
+	return err;
 }
 
 /**
-- 
2.47.0

[RFC PATCH v2 08/14] landlock: Log domain properties and release

[Mickaël Salaün]

Log domain informations when it denies an access.  This minimize the
amount of generated logs, which makes it possible to always log denials
since they should not happen (except with the new
LANDLOCK_RESTRICT_SELF_LOGLESS flag).  These records are identified by
AUDIT_LANDLOCK_DOM_INFO.

Domain's properties include their parent domains, which means that if a
domain was not logged before, it will be logged if one of its children
denies an access.  This means that a first denial by a domain A will
generate a record for the denial, another for the domain which is the
cause of this denial, and potentially a set of other records for each
parent domains if they were not logged before.

Log domain deletion with the AUDIT_LANDLOCK_DROP_DOMAIN record type when
a domain was previously logged.  This makes it possible for user space
to free potential resources when a domain ID will never show again.

The logged domain properties include:
- the domain ID
- its parent domain ID (or 0 if it is a root)
- its creator's PID
- its creator's UID
- its creator's executable path (exe)
- its creator's command line (comm)

This require each domain to save some task properties at creation time:
PID, credentials, exe path, and comm.

Audit record sample for a first denial:

  DENY:     domain=4533720530 blockers=ptrace opid=1 ocomm="systemd"
  DOM_INFO: domain=4533720530 parent=4533720526 pid=282 uid=0 exe="/root/sandboxer" comm="sandboxer"
  DOM_INFO: domain=4533720526 parent=0 pid=282 uid=0 exe="/root/sandboxer" comm="sandboxer"
  SYSCALL:  arch=c000003e syscall=101 success=no exit=-1 ...

Audit record sample for logged domains deletion:

  DROP: domain=4533720530
  DROP: domain=4533720526

Cc: Günther Noack <gnoack@google.com>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-9-mic@digikod.net
---

Questions:
* Should we also log the task's loginuid?
* Should we also log the task's sessionid?

Changes since v2:
* Do not log domain creation but log first domain use instead.
* Save task's properties that sandbox themselves.

Changes since v1:
* Add a ruleset's version for atomic logs.
* Rebased on the TCP patch series.
* Rename operation using "_" instead of "-".
* Rename AUDIT_LANDLOCK to AUDIT_LANDLOCK_RULESET.
* Only log when audit is enabled, but always set domain IDs.
* Don't log task's PID/TID with log_task() because it would be redundant
  with the SYSCALL record.
* Remove race condition when logging ruleset creation and logging
  ruleset modification while the related file descriptor was already
  registered but the ruleset creation not logged yet.
* Fix domain drop logs.
* Move the domain drop record from the previous patch into this one.
---
 include/uapi/linux/audit.h   |  2 +
 security/landlock/audit.c    | 78 ++++++++++++++++++++++++++++++++++++
 security/landlock/audit.h    |  7 ++++
 security/landlock/domain.c   | 35 ++++++++++++++++
 security/landlock/domain.h   | 37 +++++++++++++++++
 security/landlock/ruleset.c  |  7 ++++
 security/landlock/ruleset.h  |  1 +
 security/landlock/syscalls.c |  1 +
 8 files changed, 168 insertions(+)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 60c909c396c0..a72f7b3403be 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -147,6 +147,8 @@
 #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_LANDLOCK_DOM_INFO	1424	/* Landlock domain properties */
+#define AUDIT_LANDLOCK_DOM_DROP	1425	/* Landlock domain release */
 
 #define AUDIT_FIRST_KERN_ANOM_MSG   1700
 #define AUDIT_LAST_KERN_ANOM_MSG    1799
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index 3c3f831f828f..f5860f396044 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -8,8 +8,11 @@
 #include <kunit/test.h>
 #include <linux/audit.h>
 #include <linux/lsm_audit.h>
+#include <linux/pid.h>
+#include <linux/uidgid.h>
 
 #include "audit.h"
+#include "cred.h"
 #include "domain.h"
 #include "ruleset.h"
 
@@ -30,6 +33,44 @@ static void log_blockers(struct audit_buffer *const ab,
 	audit_log_format(ab, "%s", get_blocker(type));
 }
 
+static void log_node(struct landlock_hierarchy *const node)
+{
+	struct audit_buffer *ab;
+
+	ab = audit_log_start(audit_context(), GFP_ATOMIC,
+			     AUDIT_LANDLOCK_DOM_INFO);
+	if (!ab)
+		return;
+
+	WARN_ON_ONCE(node->id == 0);
+	audit_log_format(ab, "domain=%llu parent=%llu pid=%d uid=%u", node->id,
+			 node->parent ? node->parent->id : 0, pid_nr(node->pid),
+			 from_kuid(&init_user_ns, node->cred->uid));
+	audit_log_d_path(ab, " exe=", &node->exe);
+	audit_log_format(ab, " comm=");
+	audit_log_untrustedstring(ab, node->comm);
+	audit_log_end(ab);
+
+	/*
+	 * There may be race condition leading to logging of the same domain
+	 * several times but that is OK.
+	 */
+	WRITE_ONCE(node->log_status, LANDLOCK_LOG_RECORDED);
+}
+
+static void log_hierarchy(struct landlock_hierarchy *const hierarchy)
+{
+	struct landlock_hierarchy *walker;
+
+	if (WARN_ON_ONCE(!hierarchy))
+		return;
+
+	for (walker = hierarchy;
+	     walker && (READ_ONCE(walker->log_status) == LANDLOCK_LOG_PENDING);
+	     walker = walker->parent)
+		log_node(walker);
+}
+
 static struct landlock_hierarchy *
 get_hierarchy(const struct landlock_ruleset *const domain, size_t layer)
 {
@@ -116,6 +157,43 @@ void landlock_log_denial(const struct landlock_ruleset *const domain,
 	log_blockers(ab, request->type);
 	audit_log_lsm_data(ab, &request->audit);
 	audit_log_end(ab);
+
+	/* Logs this domain's hierarchy if it is the first time. */
+	log_hierarchy(youngest_denied);
+}
+
+/**
+ *
+ * landlock_log_drop_domain - Create an audit record when a domain is deleted
+ *
+ * Only domains which previously appeared in the audit logs are logged again.
+ * This is useful to know when a domain will never show again in the audit log.
+ *
+ * This record is not directly tied to a syscall entry.
+ *
+ * Called by the cred_free() hook, in an uninterruptible context.
+ */
+void landlock_log_drop_domain(const struct landlock_ruleset *const domain)
+{
+	struct audit_buffer *ab;
+
+	if (WARN_ON_ONCE(!domain->hierarchy))
+		return;
+
+	if (!audit_enabled)
+		return;
+
+	/* Ignores domains that were not logged.  */
+	if (READ_ONCE(domain->hierarchy->log_status) != LANDLOCK_LOG_RECORDED)
+		return;
+
+	ab = audit_log_start(audit_context(), GFP_ATOMIC,
+			     AUDIT_LANDLOCK_DOM_DROP);
+	if (!ab)
+		return;
+
+	audit_log_format(ab, "domain=%llu", domain->hierarchy->id);
+	audit_log_end(ab);
 }
 
 #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index f33095afcd2f..7a1b1652f21b 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -36,11 +36,18 @@ struct landlock_request {
 
 #ifdef CONFIG_AUDIT
 
+void landlock_log_drop_domain(const struct landlock_ruleset *const domain);
+
 void landlock_log_denial(const struct landlock_ruleset *const domain,
 			 const struct landlock_request *const request);
 
 #else /* CONFIG_AUDIT */
 
+static inline void
+landlock_log_drop_domain(const struct landlock_ruleset *const domain)
+{
+}
+
 static inline void
 landlock_log_denial(const struct landlock_ruleset *const domain,
 		    const struct landlock_request *const request)
diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index 965e4dc21975..f57a0242e6b1 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -5,17 +5,52 @@
  * Copyright © 2024 Microsoft Corporation
  */
 
+#include <linux/cred.h>
+#include <linux/file.h>
+#include <linux/mm.h>
+#include <linux/path.h>
+#include <linux/pid.h>
+#include <linux/sched.h>
+
 #include "domain.h"
 #include "id.h"
 
+static struct path get_current_exe(void)
+{
+	struct path exe_path = {};
+	struct file *exe_file;
+	struct mm_struct *mm = current->mm;
+
+	if (!mm)
+		return exe_path;
+
+	exe_file = get_mm_exe_file(mm);
+	if (!exe_file)
+		return exe_path;
+
+	exe_path = exe_file->f_path;
+	path_get(&exe_path);
+	fput(exe_file);
+	return exe_path;
+}
+
 /**
  * landlock_init_current_hierarchy - Partially initialize landlock_hierarchy
  *
  * @hierarchy: The hierarchy to initialize.
  *
+ * The current task is referenced as the domain creator.  The subjective
+ * credentials must not be in an overridden state.
+ *
  * @hierarchy->parent and @hierarchy->usage should already be set.
  */
 void landlock_init_current_hierarchy(struct landlock_hierarchy *const hierarchy)
 {
+	hierarchy->log_status = LANDLOCK_LOG_PENDING;
 	hierarchy->id = landlock_get_id(1);
+	WARN_ON_ONCE(current_cred() != current_real_cred());
+	hierarchy->cred = get_current_cred();
+	hierarchy->pid = get_pid(task_pid(current));
+	hierarchy->exe = get_current_exe();
+	get_task_comm(hierarchy->comm, current);
 }
diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index f82d831ca0a7..f397a73ea0c7 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -10,8 +10,17 @@
 #ifndef _SECURITY_LANDLOCK_DOMAIN_H
 #define _SECURITY_LANDLOCK_DOMAIN_H
 
+#include <linux/cred.h>
 #include <linux/mm.h>
+#include <linux/path.h>
+#include <linux/pid.h>
 #include <linux/refcount.h>
+#include <linux/sched.h>
+
+enum landlock_log_status {
+	LANDLOCK_LOG_PENDING = 0,
+	LANDLOCK_LOG_RECORDED,
+};
 
 /**
  * struct landlock_hierarchy - Node in a domain hierarchy
@@ -29,10 +38,32 @@ struct landlock_hierarchy {
 	refcount_t usage;
 
 #ifdef CONFIG_AUDIT
+	/**
+	 * @log_status: Whether this domain should be logged or not.  Because
+	 * concurrent log entries may be created at the same time, it is still
+	 * possible to have several domain records of the same domain.
+	 */
+	enum landlock_log_status log_status;
 	/**
 	 * @id: Landlock domain ID, sets once at domain creation time.
 	 */
 	u64 id;
+	/**
+	 * @cred: Credential of the domain's creator.
+	 */
+	const struct cred *cred;
+	/**
+	 * pid: Creator's PID.
+	 */
+	struct pid *pid;
+	/**
+	 * exe: Creator's exe path.
+	 */
+	struct path exe;
+	/**
+	 * comm: Command line of the domain's creator at creation time.
+	 */
+	char comm[TASK_COMM_LEN];
 #endif /* CONFIG_AUDIT */
 };
 
@@ -48,6 +79,12 @@ static inline void landlock_put_hierarchy(struct landlock_hierarchy *hierarchy)
 	while (hierarchy && refcount_dec_and_test(&hierarchy->usage)) {
 		const struct landlock_hierarchy *const freeme = hierarchy;
 
+#ifdef CONFIG_AUDIT
+		put_cred(hierarchy->cred);
+		put_pid(hierarchy->pid);
+		path_put(&hierarchy->exe);
+#endif /* CONFIG_AUDIT */
+
 		hierarchy = hierarchy->parent;
 		kfree(freeme);
 	}
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 752dbadbe5ca..0a7f9d13b6f1 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -506,6 +506,9 @@ static void free_ruleset_work(struct work_struct *const work)
 void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset)
 {
 	if (ruleset && refcount_dec_and_test(&ruleset->usage)) {
+		/* Logs with the current context. */
+		landlock_log_drop_domain(ruleset);
+
 		INIT_WORK(&ruleset->work_free, free_ruleset_work);
 		schedule_work(&ruleset->work_free);
 	}
@@ -517,6 +520,10 @@ void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset)
  * @parent: Parent domain.
  * @ruleset: New ruleset to be merged.
  *
+ * The current task is referenced as the domain creator.  The subjective
+ * credentials must not be in an overridden state (see
+ * landlock_init_current_hierarchy).
+ *
  * Returns the intersection of @parent and @ruleset, or returns @parent if
  * @ruleset is empty, or returns a duplicate of @ruleset if @parent is empty.
  */
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index d0a60a5f7cd9..1fe88027404b 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -14,6 +14,7 @@
 #include <linux/mutex.h>
 #include <linux/rbtree.h>
 #include <linux/refcount.h>
+#include <linux/sched.h>
 #include <linux/workqueue.h>
 #include <uapi/linux/landlock.h>
 
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index c097d356fa45..335067e36feb 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -26,6 +26,7 @@
 #include <linux/uaccess.h>
 #include <uapi/linux/landlock.h>
 
+#include "audit.h"
 #include "cred.h"
 #include "fs.h"
 #include "limits.h"
-- 
2.47.0

[RFC PATCH v2 09/14] landlock: Log mount-related denials

[Mickaël Salaün]

Add audit support for sb_mount, move_mount, sb_umount, sb_remount, and
sb_pivot_root hooks.

Add and use a new landlock_match_layer_level() helper.

Audit record sample:

  DENY:    blockers=fs_change_layout name="/" dev="tmpfs" ino=1
  SYSCALL: arch=c000003e syscall=166 success=no exit=-1 ...

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-10-mic@digikod.net
---

Changes since v2:
* Log the domain that denied the action because not all layers block FS
  layout changes.

Changes since v1:
* Rebased on the TCP patch series.
* Don't log missing permissions, only domain layer, and then remove the
  permission word (suggested by Günther)
---
 security/landlock/audit.c   |  3 ++
 security/landlock/audit.h   |  1 +
 security/landlock/fs.c      | 64 ++++++++++++++++++++++++++++++++++---
 security/landlock/ruleset.h | 31 ++++++++++++++++++
 4 files changed, 94 insertions(+), 5 deletions(-)

diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index f5860f396044..4cd9407459d2 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -21,6 +21,9 @@ static const char *get_blocker(const enum landlock_request_type type)
 	switch (type) {
 	case LANDLOCK_REQUEST_PTRACE:
 		return "ptrace";
+
+	case LANDLOCK_REQUEST_FS_CHANGE_LAYOUT:
+		return "fs_change_layout";
 	}
 
 	WARN_ON_ONCE(1);
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 7a1b1652f21b..6f5ad04b83c2 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -15,6 +15,7 @@
 
 enum landlock_request_type {
 	LANDLOCK_REQUEST_PTRACE = 1,
+	LANDLOCK_REQUEST_FS_CHANGE_LAYOUT,
 };
 
 /*
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index e0e5775b75ae..a099167d2347 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -23,6 +23,7 @@
 #include <linux/kernel.h>
 #include <linux/limits.h>
 #include <linux/list.h>
+#include <linux/lsm_audit.h>
 #include <linux/lsm_hooks.h>
 #include <linux/mount.h>
 #include <linux/namei.h>
@@ -37,6 +38,7 @@
 #include <uapi/linux/landlock.h>
 
 #include "access.h"
+#include "audit.h"
 #include "common.h"
 #include "cred.h"
 #include "fs.h"
@@ -1308,6 +1310,38 @@ static void hook_sb_delete(struct super_block *const sb)
 		       !atomic_long_read(&landlock_superblock(sb)->inode_refs));
 }
 
+static void
+log_fs_change_layout_path(const struct landlock_ruleset *const domain,
+			  const struct path *const path)
+{
+	const struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_FS_CHANGE_LAYOUT,
+		.audit = {
+			.type = LSM_AUDIT_DATA_PATH,
+			.u.path = *path,
+		},
+		.layer_plus_one = landlock_match_layer_level(domain, any_fs) + 1,
+	};
+
+	landlock_log_denial(domain, &request);
+}
+
+static void
+log_fs_change_layout_dentry(const struct landlock_ruleset *const domain,
+			    struct dentry *const dentry)
+{
+	const struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_FS_CHANGE_LAYOUT,
+		.audit = {
+			.type = LSM_AUDIT_DATA_DENTRY,
+			.u.dentry = dentry,
+		},
+		.layer_plus_one = landlock_match_layer_level(domain, any_fs) + 1,
+	};
+
+	landlock_log_denial(domain, &request);
+}
+
 /*
  * Because a Landlock security policy is defined according to the filesystem
  * topology (i.e. the mount namespace), changing it may grant access to files
@@ -1330,16 +1364,24 @@ static int hook_sb_mount(const char *const dev_name,
 			 const struct path *const path, const char *const type,
 			 const unsigned long flags, void *const data)
 {
-	if (!get_current_fs_domain())
+	const struct landlock_ruleset *const dom = get_current_fs_domain();
+
+	if (!dom)
 		return 0;
+
+	log_fs_change_layout_path(dom, path);
 	return -EPERM;
 }
 
 static int hook_move_mount(const struct path *const from_path,
 			   const struct path *const to_path)
 {
-	if (!get_current_fs_domain())
+	const struct landlock_ruleset *const dom = get_current_fs_domain();
+
+	if (!dom)
 		return 0;
+
+	log_fs_change_layout_path(dom, to_path);
 	return -EPERM;
 }
 
@@ -1349,15 +1391,23 @@ static int hook_move_mount(const struct path *const from_path,
  */
 static int hook_sb_umount(struct vfsmount *const mnt, const int flags)
 {
-	if (!get_current_fs_domain())
+	const struct landlock_ruleset *const dom = get_current_fs_domain();
+
+	if (!dom)
 		return 0;
+
+	log_fs_change_layout_dentry(dom, mnt->mnt_root);
 	return -EPERM;
 }
 
 static int hook_sb_remount(struct super_block *const sb, void *const mnt_opts)
 {
-	if (!get_current_fs_domain())
+	const struct landlock_ruleset *const dom = get_current_fs_domain();
+
+	if (!dom)
 		return 0;
+
+	log_fs_change_layout_dentry(dom, sb->s_root);
 	return -EPERM;
 }
 
@@ -1372,8 +1422,12 @@ static int hook_sb_remount(struct super_block *const sb, void *const mnt_opts)
 static int hook_sb_pivotroot(const struct path *const old_path,
 			     const struct path *const new_path)
 {
-	if (!get_current_fs_domain())
+	const struct landlock_ruleset *const dom = get_current_fs_domain();
+
+	if (!dom)
 		return 0;
+
+	log_fs_change_layout_path(dom, new_path);
 	return -EPERM;
 }
 
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 1fe88027404b..c463ff9a6615 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -278,6 +278,37 @@ landlock_match_ruleset(const struct landlock_ruleset *const ruleset,
 	return NULL;
 }
 
+/**
+ * landlock_match_layer_level - Return the layer level restricting @masks
+ *
+ * @ruleset: Landlock ruleset (used as a domain)
+ * @masks: access masks
+ *
+ * Returns: the number of the layer restricting/handling any right of @access,
+ * or return 0 (i.e. first layer) otherwise.
+ */
+static inline size_t
+landlock_match_layer_level(const struct landlock_ruleset *const ruleset,
+			   const struct access_masks masks)
+{
+	const union access_masks_all masks_all = {
+		.masks = masks,
+	};
+	size_t layer_level;
+
+	for (layer_level = ruleset->num_layers; layer_level >= 0;
+	     layer_level--) {
+		union access_masks_all layer = {
+			.masks = ruleset->access_masks[layer_level],
+		};
+
+		if (masks_all.all & layer.all)
+			return layer_level;
+	}
+
+	return 0;
+}
+
 static inline void
 landlock_add_fs_access_mask(struct landlock_ruleset *const ruleset,
 			    const access_mask_t fs_access_mask,
-- 
2.47.0

[RFC PATCH v2 10/14] landlock: Log file-related denials

[Mickaël Salaün]

Add audit support for path_mkdir, path_mknod, path_symlink, path_unlink,
path_rmdir, path_truncate, path_link, path_rename, and file_open hooks.

Audit record sample for a link action:

  DENY:     domain=4533720568 blockers=fs_refer path="/usr/bin" dev="vda2" ino=351
  DOM_INFO: domain=4533720568 parent=0 pid=325 uid=0 exe="/root/sandboxer" comm="sandboxer"
  DENY:     domain=4533720568 blockers=fs_make_reg,fs_refer path="/usr/local" dev="vda2" ino=365
  SYSCALL:  arch=c000003e syscall=265 success=no exit=-13 ...

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-11-mic@digikod.net
---

Changes since v2:
* Revamp logging and support the path_link and path_rename hooks.
* Add KUnit tests.

Changes since v1:
* Move audit code to the ptrace patch.
---
 security/landlock/audit.c | 173 ++++++++++++++++++++++++++++++++++++--
 security/landlock/audit.h |   9 ++
 security/landlock/fs.c    |  64 +++++++++++---
 3 files changed, 229 insertions(+), 17 deletions(-)

diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index 4cd9407459d2..9c8b6c246884 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -7,23 +7,55 @@
 
 #include <kunit/test.h>
 #include <linux/audit.h>
+#include <linux/bitops.h>
 #include <linux/lsm_audit.h>
 #include <linux/pid.h>
 #include <linux/uidgid.h>
+#include <uapi/linux/landlock.h>
 
 #include "audit.h"
+#include "common.h"
 #include "cred.h"
 #include "domain.h"
 #include "ruleset.h"
 
-static const char *get_blocker(const enum landlock_request_type type)
+static const char *const fs_access_strings[] = {
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = "fs_execute",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = "fs_write_file",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = "fs_read_file",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = "fs_read_dir",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = "fs_remove_dir",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_FILE)] = "fs_remove_file",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_CHAR)] = "fs_make_char",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_DIR)] = "fs_make_dir",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_REG)] = "fs_make_reg",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_SOCK)] = "fs_make_sock",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_FIFO)] = "fs_make_fifo",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_BLOCK)] = "fs_make_block",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_SYM)] = "fs_make_sym",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs_refer",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs_truncate",
+	[BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs_ioctl_dev",
+};
+static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
+
+static __attribute_const__ const char *
+get_blocker(const enum landlock_request_type type,
+	    const unsigned long access_bit)
 {
 	switch (type) {
 	case LANDLOCK_REQUEST_PTRACE:
+		WARN_ON_ONCE(access_bit != -1);
 		return "ptrace";
 
 	case LANDLOCK_REQUEST_FS_CHANGE_LAYOUT:
+		WARN_ON_ONCE(access_bit != -1);
 		return "fs_change_layout";
+
+	case LANDLOCK_REQUEST_FS_ACCESS:
+		if (WARN_ON_ONCE(access_bit >= ARRAY_SIZE(fs_access_strings)))
+			return "unknown";
+		return fs_access_strings[access_bit];
 	}
 
 	WARN_ON_ONCE(1);
@@ -31,9 +63,20 @@ static const char *get_blocker(const enum landlock_request_type type)
 }
 
 static void log_blockers(struct audit_buffer *const ab,
-			 const enum landlock_request_type type)
+			 const enum landlock_request_type type,
+			 const access_mask_t access)
 {
-	audit_log_format(ab, "%s", get_blocker(type));
+	const unsigned long access_mask = access;
+	unsigned long access_bit;
+	size_t i = 0;
+
+	for_each_set_bit(access_bit, &access_mask, BITS_PER_TYPE(access)) {
+		audit_log_format(ab, "%s%s", (i == 0) ? "" : ",",
+				 get_blocker(type, access_bit));
+		i++;
+	}
+	if (i == 0)
+		audit_log_format(ab, "%s", get_blocker(type, -1));
 }
 
 static void log_node(struct landlock_hierarchy *const node)
@@ -121,9 +164,110 @@ static void test_get_hierarchy(struct kunit *const test)
 
 #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
 
+static size_t get_denied_layer(const struct landlock_ruleset *const domain,
+			       access_mask_t *const access_request,
+			       const layer_mask_t (*const layer_masks)[],
+			       const size_t layer_masks_size)
+{
+	const unsigned long access_req = *access_request;
+	unsigned long access_bit;
+	access_mask_t missing = 0;
+	long youngest_layer = -1;
+
+	for_each_set_bit(access_bit, &access_req, layer_masks_size) {
+		const access_mask_t mask = (*layer_masks)[access_bit];
+		long layer;
+
+		if (!mask)
+			continue;
+
+		/* __fls(1) == 0 */
+		layer = __fls(mask);
+		if (layer > youngest_layer) {
+			youngest_layer = layer;
+			missing = BIT(access_bit);
+		} else if (layer == youngest_layer) {
+			missing |= BIT(access_bit);
+		}
+	}
+
+	*access_request = missing;
+	if (youngest_layer == -1)
+		return domain->num_layers - 1;
+
+	return youngest_layer;
+}
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+static void test_get_denied_layer(struct kunit *const test)
+{
+	const struct landlock_ruleset dom = {
+		.num_layers = 5,
+	};
+	const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
+		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT(0),
+		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT(1),
+		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = BIT(1) | BIT(0),
+		[BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = BIT(2),
+	};
+	access_mask_t access;
+
+	access = LANDLOCK_ACCESS_FS_EXECUTE;
+	KUNIT_EXPECT_EQ(test, 0,
+			get_denied_layer(&dom, &access, &layer_masks,
+					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_EXECUTE);
+
+	access = LANDLOCK_ACCESS_FS_READ_FILE;
+	KUNIT_EXPECT_EQ(test, 1,
+			get_denied_layer(&dom, &access, &layer_masks,
+					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_FILE);
+
+	access = LANDLOCK_ACCESS_FS_READ_DIR;
+	KUNIT_EXPECT_EQ(test, 1,
+			get_denied_layer(&dom, &access, &layer_masks,
+					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
+
+	access = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR;
+	KUNIT_EXPECT_EQ(test, 1,
+			get_denied_layer(&dom, &access, &layer_masks,
+					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, access,
+			LANDLOCK_ACCESS_FS_READ_FILE |
+				LANDLOCK_ACCESS_FS_READ_DIR);
+
+	access = LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_READ_DIR;
+	KUNIT_EXPECT_EQ(test, 1,
+			get_denied_layer(&dom, &access, &layer_masks,
+					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
+
+	access = LANDLOCK_ACCESS_FS_WRITE_FILE;
+	KUNIT_EXPECT_EQ(test, 4,
+			get_denied_layer(&dom, &access, &layer_masks,
+					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, access, 0);
+}
+
+#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))
+	if (WARN_ON_ONCE(!(!!request->layer_plus_one ^ !!request->access)))
+		return false;
+
+	if (request->access) {
+		if (WARN_ON_ONCE(!request->layer_masks))
+			return false;
+	} else {
+		if (WARN_ON_ONCE(request->layer_masks))
+			return false;
+	}
+
+	if (WARN_ON_ONCE(!!request->layer_masks ^ !!request->layer_masks_size))
 		return false;
 
 	return true;
@@ -140,6 +284,7 @@ void landlock_log_denial(const struct landlock_ruleset *const domain,
 {
 	struct audit_buffer *ab;
 	struct landlock_hierarchy *youngest_denied;
+	access_mask_t missing;
 
 	if (WARN_ON_ONCE(!domain || !domain->hierarchy || !request))
 		return;
@@ -155,9 +300,24 @@ void landlock_log_denial(const struct landlock_ruleset *const domain,
 	if (!ab)
 		return;
 
-	youngest_denied = get_hierarchy(domain, request->layer_plus_one - 1);
+	missing = request->access;
+	if (missing) {
+		size_t youngest_layer;
+
+		/* Gets the nearest domain that denies the request. */
+		if (request->layer_masks) {
+			youngest_layer = get_denied_layer(
+				domain, &missing, request->layer_masks,
+				request->layer_masks_size);
+		}
+		youngest_denied = get_hierarchy(domain, youngest_layer);
+	} else {
+		youngest_denied =
+			get_hierarchy(domain, request->layer_plus_one - 1);
+	}
+
 	audit_log_format(ab, "domain=%llu blockers=", youngest_denied->id);
-	log_blockers(ab, request->type);
+	log_blockers(ab, request->type, missing);
 	audit_log_lsm_data(ab, &request->audit);
 	audit_log_end(ab);
 
@@ -204,6 +364,7 @@ void landlock_log_drop_domain(const struct landlock_ruleset *const domain)
 static struct kunit_case test_cases[] = {
 	/* clang-format off */
 	KUNIT_CASE(test_get_hierarchy),
+	KUNIT_CASE(test_get_denied_layer),
 	{}
 	/* clang-format on */
 };
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 6f5ad04b83c2..25fc8333cddc 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -11,11 +11,13 @@
 #include <linux/audit.h>
 #include <linux/lsm_audit.h>
 
+#include "access.h"
 #include "ruleset.h"
 
 enum landlock_request_type {
 	LANDLOCK_REQUEST_PTRACE = 1,
 	LANDLOCK_REQUEST_FS_CHANGE_LAYOUT,
+	LANDLOCK_REQUEST_FS_ACCESS,
 };
 
 /*
@@ -33,6 +35,13 @@ struct landlock_request {
 	 * extra one is useful to detect uninitialized field.
 	 */
 	size_t layer_plus_one;
+
+	/* Required field for configurable access control. */
+	access_mask_t access;
+
+	/* Required fields for requests with layer masks. */
+	const layer_mask_t (*layer_masks)[];
+	size_t layer_masks_size;
 };
 
 #ifdef CONFIG_AUDIT
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index a099167d2347..7f69bed9e095 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -730,6 +730,7 @@ static void test_is_eacces_with_write(struct kunit *const test)
  *     those identified by @access_request_parent1).  This matrix can
  *     initially refer to domain layer masks and, when the accesses for the
  *     destination and source are the same, to requested layer masks.
+ * @log_request_parent1: Audit request to fill if the related access is denied.
  * @dentry_child1: Dentry to the initial child of the parent1 path.  This
  *     pointer must be NULL for non-refer actions (i.e. not link nor rename).
  * @access_request_parent2: Similar to @access_request_parent1 but for a
@@ -738,6 +739,7 @@ static void test_is_eacces_with_write(struct kunit *const test)
  *     the source.  Must be set to 0 when using a simple path request.
  * @layer_masks_parent2: Similar to @layer_masks_parent1 but for a refer
  *     action.  This must be NULL otherwise.
+ * @log_request_parent2: Audit request to fill if the related access is denied.
  * @dentry_child2: Dentry to the initial child of the parent2 path.  This
  *     pointer is only set for RENAME_EXCHANGE actions and must be NULL
  *     otherwise.
@@ -757,10 +759,12 @@ static bool is_access_to_paths_allowed(
 	const struct path *const path,
 	const access_mask_t access_request_parent1,
 	layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
-	const struct dentry *const dentry_child1,
+	struct landlock_request *const log_request_parent1,
+	struct dentry *const dentry_child1,
 	const access_mask_t access_request_parent2,
 	layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
-	const struct dentry *const dentry_child2)
+	struct landlock_request *const log_request_parent2,
+	struct dentry *const dentry_child2)
 {
 	bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check,
 	     child1_is_directory = true, child2_is_directory = true;
@@ -907,6 +911,24 @@ static bool is_access_to_paths_allowed(
 	}
 	path_put(&walker_path);
 
+	if (!allowed_parent1 && log_request_parent1) {
+		log_request_parent1->type = LANDLOCK_REQUEST_FS_ACCESS,
+		log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH,
+		log_request_parent1->audit.u.path = *path;
+		log_request_parent1->access = access_request_parent1;
+		log_request_parent1->layer_masks = layer_masks_parent1;
+		log_request_parent1->layer_masks_size =
+			ARRAY_SIZE(*layer_masks_parent1);
+	}
+	if (!allowed_parent2 && log_request_parent2) {
+		log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS,
+		log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH,
+		log_request_parent2->audit.u.path = *path;
+		log_request_parent2->access = access_request_parent2;
+		log_request_parent2->layer_masks = layer_masks_parent2;
+		log_request_parent2->layer_masks_size =
+			ARRAY_SIZE(*layer_masks_parent2);
+	}
 	return allowed_parent1 && allowed_parent2;
 }
 
@@ -915,6 +937,7 @@ static int current_check_access_path(const struct path *const path,
 {
 	const struct landlock_ruleset *const dom = get_current_fs_domain();
 	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+	struct landlock_request request = {};
 
 	if (!dom)
 		return 0;
@@ -922,9 +945,10 @@ static int current_check_access_path(const struct path *const path,
 	access_request = landlock_init_layer_masks(
 		dom, access_request, &layer_masks, LANDLOCK_KEY_INODE);
 	if (is_access_to_paths_allowed(dom, path, access_request, &layer_masks,
-				       NULL, 0, NULL, NULL))
+				       &request, NULL, 0, NULL, NULL, NULL))
 		return 0;
 
+	landlock_log_denial(dom, &request);
 	return -EACCES;
 }
 
@@ -1093,6 +1117,7 @@ static int current_check_refer_path(struct dentry *const old_dentry,
 	struct dentry *old_parent;
 	layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {},
 		     layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {};
+	struct landlock_request request1 = {}, request2 = {};
 
 	if (!dom)
 		return 0;
@@ -1124,10 +1149,13 @@ static int current_check_refer_path(struct dentry *const old_dentry,
 		access_request_parent1 = landlock_init_layer_masks(
 			dom, access_request_parent1 | access_request_parent2,
 			&layer_masks_parent1, LANDLOCK_KEY_INODE);
-		if (is_access_to_paths_allowed(
-			    dom, new_dir, access_request_parent1,
-			    &layer_masks_parent1, NULL, 0, NULL, NULL))
+		if (is_access_to_paths_allowed(dom, new_dir,
+					       access_request_parent1,
+					       &layer_masks_parent1, &request1,
+					       NULL, 0, NULL, NULL, NULL))
 			return 0;
+
+		landlock_log_denial(dom, &request1);
 		return -EACCES;
 	}
 
@@ -1162,12 +1190,22 @@ static int current_check_refer_path(struct dentry *const old_dentry,
 	 * parent access rights.  This will be useful to compare with the
 	 * destination parent access rights.
 	 */
-	if (is_access_to_paths_allowed(
-		    dom, &mnt_dir, access_request_parent1, &layer_masks_parent1,
-		    old_dentry, access_request_parent2, &layer_masks_parent2,
-		    exchange ? new_dentry : NULL))
+	if (is_access_to_paths_allowed(dom, &mnt_dir, access_request_parent1,
+				       &layer_masks_parent1, &request1,
+				       old_dentry, access_request_parent2,
+				       &layer_masks_parent2, &request2,
+				       exchange ? new_dentry : NULL))
 		return 0;
 
+	if (request1.access) {
+		request1.audit.u.path.dentry = old_parent;
+		landlock_log_denial(dom, &request1);
+	}
+	if (request2.access) {
+		request2.audit.u.path.dentry = new_dir->dentry;
+		landlock_log_denial(dom, &request2);
+	}
+
 	/*
 	 * This prioritizes EACCES over EXDEV for all actions, including
 	 * renames with RENAME_EXCHANGE.
@@ -1546,6 +1584,7 @@ static int hook_file_open(struct file *const file)
 		optional_access;
 	const struct landlock_ruleset *const dom = landlock_match_ruleset(
 		landlock_cred(file->f_cred)->domain, any_fs);
+	struct landlock_request request = {};
 
 	if (!dom)
 		return 0;
@@ -1571,7 +1610,7 @@ static int hook_file_open(struct file *const file)
 		    dom, &file->f_path,
 		    landlock_init_layer_masks(dom, full_access_request,
 					      &layer_masks, LANDLOCK_KEY_INODE),
-		    &layer_masks, NULL, 0, NULL, NULL)) {
+		    &layer_masks, &request, NULL, 0, NULL, NULL, NULL)) {
 		allowed_access = full_access_request;
 	} else {
 		unsigned long access_bit;
@@ -1601,6 +1640,9 @@ static int hook_file_open(struct file *const file)
 	if ((open_access_request & allowed_access) == open_access_request)
 		return 0;
 
+	/* Sets access to reflect the actual request. */
+	request.access = open_access_request;
+	landlock_log_denial(dom, &request);
 	return -EACCES;
 }
 
-- 
2.47.0

[RFC PATCH v2 11/14] landlock: Log truncate and ioctl denials

[Mickaël Salaün]

Add audit support to the file_truncate and file_ioctl hooks.

Add a deny_masks_t type and related helpers to store the domain's layer
level per optional access rights (i.e. FS_TRUNCATE and FS_IOCTL_DEV)
when opening a file, which cannot be inferred later.

Add KUnit tests.

Audit record sample:

  DENY:    domain=4533720577 blockers=fs_ioctl_dev path="/dev/tty" dev="devtmpfs" ino=9
  SYSCALL: arch=c000003e syscall=16 success=no exit=-13

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-12-mic@digikod.net
---
 security/landlock/access.h |  17 +++++
 security/landlock/audit.c  |  96 +++++++++++++++++++++++++++-
 security/landlock/audit.h  |   4 ++
 security/landlock/domain.c | 128 +++++++++++++++++++++++++++++++++++++
 security/landlock/domain.h |   8 +++
 security/landlock/fs.c     |  51 +++++++++++++++
 security/landlock/fs.h     |   9 +++
 7 files changed, 311 insertions(+), 2 deletions(-)

diff --git a/security/landlock/access.h b/security/landlock/access.h
index 2659fd9b4aaf..57bf53e01e69 100644
--- a/security/landlock/access.h
+++ b/security/landlock/access.h
@@ -50,4 +50,21 @@ typedef u16 layer_mask_t;
 /* Makes sure all layers can be checked. */
 static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
 
+/*
+ * Tracks domains responsible of a denied access.  This is required to avoid
+ * storing in each object the full layer_masks[] required by update_request().
+ */
+typedef u8 deny_masks_t;
+
+/*
+ * Makes sure all optional access rights can be tied to a layer index (cf.
+ * get_deny_mask).
+ */
+static_assert(BITS_PER_TYPE(deny_masks_t) >=
+	      (HWEIGHT(LANDLOCK_MAX_NUM_LAYERS - 1) *
+	       HWEIGHT(ACCESS_FS_OPTIONAL)));
+
+/* LANDLOCK_MAX_NUM_LAYERS must be a power of two (cf. deny_masks_t assert). */
+static_assert(HWEIGHT(LANDLOCK_MAX_NUM_LAYERS) == 1);
+
 #endif /* _SECURITY_LANDLOCK_ACCESS_H */
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index 9c8b6c246884..898c95ebe847 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -13,10 +13,12 @@
 #include <linux/uidgid.h>
 #include <uapi/linux/landlock.h>
 
+#include "access.h"
 #include "audit.h"
 #include "common.h"
 #include "cred.h"
 #include "domain.h"
+#include "fs.h"
 #include "ruleset.h"
 
 static const char *const fs_access_strings[] = {
@@ -254,22 +256,107 @@ static void test_get_denied_layer(struct kunit *const test)
 
 #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
 
+static size_t
+get_level_from_deny_masks(access_mask_t *const access_request,
+			  const access_mask_t all_existing_optional_access,
+			  const deny_masks_t deny_masks)
+{
+	const unsigned long access_opt = all_existing_optional_access;
+	const unsigned long access_req = *access_request;
+	access_mask_t missing = 0;
+	size_t youngest_layer = 0;
+	size_t access_index = 0;
+	unsigned long access_bit;
+
+	/* This will require change with new object types. */
+	WARN_ON_ONCE(access_opt != ACCESS_FS_OPTIONAL);
+
+	for_each_set_bit(access_bit, &access_opt,
+			 BITS_PER_TYPE(access_mask_t)) {
+		if (access_req & BIT(access_bit)) {
+			const size_t layer =
+				(deny_masks >> (access_index * 4)) &
+				(LANDLOCK_MAX_NUM_LAYERS - 1);
+
+			if (layer > youngest_layer) {
+				youngest_layer = layer;
+				missing = BIT(access_bit);
+			} else if (layer == youngest_layer) {
+				missing |= BIT(access_bit);
+			}
+		}
+		access_index++;
+	}
+
+	*access_request = missing;
+	return youngest_layer;
+}
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+static void test_get_level_from_deny_masks(struct kunit *const test)
+{
+	deny_masks_t deny_mask;
+	access_mask_t access;
+
+	/* truncate:0 ioctl_dev:2 */
+	deny_mask = 0x20;
+
+	access = LANDLOCK_ACCESS_FS_TRUNCATE;
+	KUNIT_EXPECT_EQ(test, 0,
+			get_level_from_deny_masks(&access, ACCESS_FS_OPTIONAL,
+						  deny_mask));
+	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+
+	access = LANDLOCK_ACCESS_FS_TRUNCATE | LANDLOCK_ACCESS_FS_IOCTL_DEV;
+	KUNIT_EXPECT_EQ(test, 2,
+			get_level_from_deny_masks(&access, ACCESS_FS_OPTIONAL,
+						  deny_mask));
+	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
+
+	/* truncate:15 ioctl_dev:15 */
+	deny_mask = 0xff;
+
+	access = LANDLOCK_ACCESS_FS_TRUNCATE;
+	KUNIT_EXPECT_EQ(test, 15,
+			get_level_from_deny_masks(&access, ACCESS_FS_OPTIONAL,
+						  deny_mask));
+	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
+
+	access = LANDLOCK_ACCESS_FS_TRUNCATE | LANDLOCK_ACCESS_FS_IOCTL_DEV;
+	KUNIT_EXPECT_EQ(test, 15,
+			get_level_from_deny_masks(&access, ACCESS_FS_OPTIONAL,
+						  deny_mask));
+	KUNIT_EXPECT_EQ(test, access,
+			LANDLOCK_ACCESS_FS_TRUNCATE |
+				LANDLOCK_ACCESS_FS_IOCTL_DEV);
+}
+
+#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 ^ !!request->access)))
 		return false;
 
 	if (request->access) {
-		if (WARN_ON_ONCE(!request->layer_masks))
+		if (WARN_ON_ONCE(!(!!request->layer_masks ^
+				   !!request->all_existing_optional_access)))
 			return false;
 	} else {
-		if (WARN_ON_ONCE(request->layer_masks))
+		if (WARN_ON_ONCE(request->layer_masks ||
+				 request->all_existing_optional_access))
 			return false;
 	}
 
 	if (WARN_ON_ONCE(!!request->layer_masks ^ !!request->layer_masks_size))
 		return false;
 
+	if (request->deny_masks) {
+		if (WARN_ON_ONCE(!request->all_existing_optional_access))
+			return false;
+	}
+
 	return true;
 }
 
@@ -309,6 +396,10 @@ void landlock_log_denial(const struct landlock_ruleset *const domain,
 			youngest_layer = get_denied_layer(
 				domain, &missing, request->layer_masks,
 				request->layer_masks_size);
+		} else {
+			youngest_layer = get_level_from_deny_masks(
+				&missing, request->all_existing_optional_access,
+				request->deny_masks);
 		}
 		youngest_denied = get_hierarchy(domain, youngest_layer);
 	} else {
@@ -365,6 +456,7 @@ static struct kunit_case test_cases[] = {
 	/* clang-format off */
 	KUNIT_CASE(test_get_hierarchy),
 	KUNIT_CASE(test_get_denied_layer),
+	KUNIT_CASE(test_get_level_from_deny_masks),
 	{}
 	/* clang-format on */
 };
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 25fc8333cddc..320394fd6b84 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -42,6 +42,10 @@ struct landlock_request {
 	/* Required fields for requests with layer masks. */
 	const layer_mask_t (*layer_masks)[];
 	size_t layer_masks_size;
+
+	/* Required fields for requests with deny masks. */
+	const access_mask_t all_existing_optional_access;
+	deny_masks_t deny_masks;
 };
 
 #ifdef CONFIG_AUDIT
diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index f57a0242e6b1..36cd81c1b542 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -5,6 +5,9 @@
  * Copyright © 2024 Microsoft Corporation
  */
 
+#include <kunit/test.h>
+#include <linux/bitops.h>
+#include <linux/bits.h>
 #include <linux/cred.h>
 #include <linux/file.h>
 #include <linux/mm.h>
@@ -12,6 +15,8 @@
 #include <linux/pid.h>
 #include <linux/sched.h>
 
+#include "access.h"
+#include "common.h"
 #include "domain.h"
 #include "id.h"
 
@@ -54,3 +59,126 @@ void landlock_init_current_hierarchy(struct landlock_hierarchy *const hierarchy)
 	hierarchy->exe = get_current_exe();
 	get_task_comm(hierarchy->comm, current);
 }
+
+static deny_masks_t
+get_layer_deny_mask(const access_mask_t all_existing_optional_access,
+		    const unsigned long access_bit, const size_t layer)
+{
+	unsigned long access_weight;
+
+	/* This may require change with new object types. */
+	WARN_ON_ONCE(all_existing_optional_access != ACCESS_FS_OPTIONAL);
+
+	if (WARN_ON_ONCE(layer >= LANDLOCK_MAX_NUM_LAYERS))
+		return 0;
+
+	access_weight = hweight_long(all_existing_optional_access &
+				     GENMASK(access_bit, 0));
+	if (WARN_ON_ONCE(access_weight < 1))
+		return 0;
+
+	return layer
+	       << ((access_weight - 1) * HWEIGHT(LANDLOCK_MAX_NUM_LAYERS - 1));
+}
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+static void test_get_layer_deny_mask(struct kunit *const test)
+{
+	const unsigned long truncate = BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE);
+	const unsigned long ioctl_dev = BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV);
+
+	KUNIT_EXPECT_EQ(test, 0,
+			get_layer_deny_mask(ACCESS_FS_OPTIONAL, truncate, 0));
+	KUNIT_EXPECT_EQ(test, 0x3,
+			get_layer_deny_mask(ACCESS_FS_OPTIONAL, truncate, 3));
+
+	KUNIT_EXPECT_EQ(test, 0,
+			get_layer_deny_mask(ACCESS_FS_OPTIONAL, ioctl_dev, 0));
+	KUNIT_EXPECT_EQ(test, 0xf0,
+			get_layer_deny_mask(ACCESS_FS_OPTIONAL, ioctl_dev, 15));
+}
+
+#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
+
+deny_masks_t
+landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
+			const access_mask_t optional_access,
+			const layer_mask_t (*const layer_masks)[],
+			const size_t layer_masks_size)
+{
+	const unsigned long access_opt = optional_access;
+	unsigned long access_bit;
+	deny_masks_t deny_masks = 0;
+
+	/* This may require change with new object types. */
+	WARN_ON_ONCE(access_opt !=
+		     (optional_access & all_existing_optional_access));
+
+	if (WARN_ON_ONCE(!layer_masks))
+		return 0;
+
+	if (WARN_ON_ONCE(!access_opt))
+		return 0;
+
+	for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
+		const layer_mask_t mask = (*layer_masks)[access_bit];
+
+		if (!mask)
+			continue;
+
+		/* __fls(1) == 0 */
+		deny_masks |= get_layer_deny_mask(all_existing_optional_access,
+						  access_bit, __fls(mask));
+	}
+	return deny_masks;
+}
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+static void test_landlock_get_deny_masks(struct kunit *const test)
+{
+	const layer_mask_t layers1[BITS_PER_TYPE(access_mask_t)] = {
+		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
+							  BIT_ULL(9),
+		[BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = BIT_ULL(1),
+		[BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = BIT_ULL(2) |
+							    BIT_ULL(0),
+	};
+
+	KUNIT_EXPECT_EQ(test, 0x1,
+			landlock_get_deny_masks(ACCESS_FS_OPTIONAL,
+						LANDLOCK_ACCESS_FS_TRUNCATE,
+						&layers1, ARRAY_SIZE(layers1)));
+	KUNIT_EXPECT_EQ(test, 0x20,
+			landlock_get_deny_masks(ACCESS_FS_OPTIONAL,
+						LANDLOCK_ACCESS_FS_IOCTL_DEV,
+						&layers1, ARRAY_SIZE(layers1)));
+	KUNIT_EXPECT_EQ(
+		test, 0x21,
+		landlock_get_deny_masks(ACCESS_FS_OPTIONAL,
+					LANDLOCK_ACCESS_FS_TRUNCATE |
+						LANDLOCK_ACCESS_FS_IOCTL_DEV,
+					&layers1, ARRAY_SIZE(layers1)));
+}
+
+#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+static struct kunit_case test_cases[] = {
+	/* clang-format off */
+	KUNIT_CASE(test_get_layer_deny_mask),
+	KUNIT_CASE(test_landlock_get_deny_masks),
+	{}
+	/* clang-format on */
+};
+
+static struct kunit_suite test_suite = {
+	.name = "landlock_domain",
+	.test_cases = test_cases,
+};
+
+kunit_test_suite(test_suite);
+
+#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index f397a73ea0c7..1374497d9a9b 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -17,6 +17,8 @@
 #include <linux/refcount.h>
 #include <linux/sched.h>
 
+#include "access.h"
+
 enum landlock_log_status {
 	LANDLOCK_LOG_PENDING = 0,
 	LANDLOCK_LOG_RECORDED,
@@ -90,6 +92,12 @@ static inline void landlock_put_hierarchy(struct landlock_hierarchy *hierarchy)
 	}
 }
 
+deny_masks_t
+landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
+			const access_mask_t optional_access,
+			const layer_mask_t (*const layer_masks)[],
+			size_t layer_masks_size);
+
 #ifdef CONFIG_AUDIT
 void landlock_init_current_hierarchy(struct landlock_hierarchy *const hierarchy);
 #else /* CONFIG_AUDIT */
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 7f69bed9e095..3724f9bff7d8 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -246,6 +246,17 @@ is_masked_device_ioctl_compat(const unsigned int cmd)
 	}
 }
 
+static void
+update_request(struct landlock_request *const request,
+	       const struct landlock_file_security *const file_security,
+	       const access_mask_t access)
+{
+	request->access = access;
+#ifdef CONFIG_AUDIT
+	request->deny_masks = file_security->deny_masks;
+#endif /* CONFIG_AUDIT */
+}
+
 /* Ruleset management */
 
 static struct landlock_object *get_inode_object(struct inode *const inode)
@@ -1636,6 +1647,11 @@ static int hook_file_open(struct file *const file)
 	 * file access rights in the opened struct file.
 	 */
 	landlock_file(file)->allowed_access = allowed_access;
+#ifdef CONFIG_AUDIT
+	landlock_file(file)->deny_masks =
+		landlock_get_deny_masks(ACCESS_FS_OPTIONAL, optional_access,
+					&layer_masks, ARRAY_SIZE(layer_masks));
+#endif /* CONFIG_AUDIT */
 
 	if ((open_access_request & allowed_access) == open_access_request)
 		return 0;
@@ -1648,6 +1664,15 @@ static int hook_file_open(struct file *const file)
 
 static int hook_file_truncate(struct file *const file)
 {
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_FS_ACCESS,
+		.audit = {
+			.type = LSM_AUDIT_DATA_FILE,
+			.u.file = file,
+		},
+		.all_existing_optional_access = ACCESS_FS_OPTIONAL,
+	};
+
 	/*
 	 * Allows truncation if the truncate right was available at the time of
 	 * opening the file, to get a consistent access check as for read, write
@@ -1660,12 +1685,24 @@ static int hook_file_truncate(struct file *const file)
 	 */
 	if (landlock_file(file)->allowed_access & LANDLOCK_ACCESS_FS_TRUNCATE)
 		return 0;
+
+	update_request(&request, landlock_file(file),
+		       LANDLOCK_ACCESS_FS_TRUNCATE);
+	landlock_log_denial(landlock_cred(file->f_cred)->domain, &request);
 	return -EACCES;
 }
 
 static int hook_file_ioctl(struct file *file, unsigned int cmd,
 			   unsigned long arg)
 {
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_FS_ACCESS,
+		.audit = {
+			.type = LSM_AUDIT_DATA_FILE,
+			.u.file = file,
+		},
+		.all_existing_optional_access = ACCESS_FS_OPTIONAL,
+	};
 	access_mask_t allowed_access = landlock_file(file)->allowed_access;
 
 	/*
@@ -1683,12 +1720,23 @@ static int hook_file_ioctl(struct file *file, unsigned int cmd,
 	if (is_masked_device_ioctl(cmd))
 		return 0;
 
+	update_request(&request, landlock_file(file),
+		       LANDLOCK_ACCESS_FS_IOCTL_DEV);
+	landlock_log_denial(landlock_cred(file->f_cred)->domain, &request);
 	return -EACCES;
 }
 
 static int hook_file_ioctl_compat(struct file *file, unsigned int cmd,
 				  unsigned long arg)
 {
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_FS_ACCESS,
+		.audit = {
+			.type = LSM_AUDIT_DATA_FILE,
+			.u.file = file,
+		},
+		.all_existing_optional_access = ACCESS_FS_OPTIONAL,
+	};
 	access_mask_t allowed_access = landlock_file(file)->allowed_access;
 
 	/*
@@ -1706,6 +1754,9 @@ static int hook_file_ioctl_compat(struct file *file, unsigned int cmd,
 	if (is_masked_device_ioctl_compat(cmd))
 		return 0;
 
+	update_request(&request, landlock_file(file),
+		       LANDLOCK_ACCESS_FS_IOCTL_DEV);
+	landlock_log_denial(landlock_cred(file->f_cred)->domain, &request);
 	return -EACCES;
 }
 
diff --git a/security/landlock/fs.h b/security/landlock/fs.h
index d445f411c26a..d36cf70a7daa 100644
--- a/security/landlock/fs.h
+++ b/security/landlock/fs.h
@@ -53,6 +53,15 @@ struct landlock_file_security {
 	 * needed to authorize later operations on the open file.
 	 */
 	access_mask_t allowed_access;
+
+#ifdef CONFIG_AUDIT
+	/**
+	 * @deny_masks: Domain layer levels that deny an optional access (see
+	 * ACCESS_FS_OPTIONAL).
+	 */
+	deny_masks_t deny_masks;
+#endif /* CONFIG_AUDIT */
+
 	/**
 	 * @fown_domain: Domain of the task that set the PID that may receive a
 	 * signal e.g., SIGURG when writing MSG_OOB to the related socket.
-- 
2.47.0

[RFC PATCH v2 12/14] landlock: Log TCP bind and connect denials

[Mickaël Salaün]

Add audit support to socket_bind and socket_connect hooks.

Audit record sample:

  DENY:    domain=4533720601 blockers=net_connect_tcp daddr=127.0.0.1 dest=80
  SYSCALL: arch=c000003e syscall=42 success=no exit=-13 ...

Cc: Günther Noack <gnoack@google.com>
Cc: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
Cc: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-13-mic@digikod.net
---
 security/landlock/audit.c | 11 +++++++++
 security/landlock/audit.h |  1 +
 security/landlock/net.c   | 52 ++++++++++++++++++++++++++++++++++++---
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index 898c95ebe847..c31a4a8719ee 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -41,6 +41,12 @@ static const char *const fs_access_strings[] = {
 };
 static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
 
+static const char *const net_access_strings[] = {
+	[BIT_INDEX(LANDLOCK_ACCESS_NET_BIND_TCP)] = "net_bind_tcp",
+	[BIT_INDEX(LANDLOCK_ACCESS_NET_CONNECT_TCP)] = "net_connect_tcp",
+};
+static_assert(ARRAY_SIZE(net_access_strings) == LANDLOCK_NUM_ACCESS_NET);
+
 static __attribute_const__ const char *
 get_blocker(const enum landlock_request_type type,
 	    const unsigned long access_bit)
@@ -58,6 +64,11 @@ get_blocker(const enum landlock_request_type type,
 		if (WARN_ON_ONCE(access_bit >= ARRAY_SIZE(fs_access_strings)))
 			return "unknown";
 		return fs_access_strings[access_bit];
+
+	case LANDLOCK_REQUEST_NET_ACCESS:
+		if (WARN_ON_ONCE(access_bit >= ARRAY_SIZE(net_access_strings)))
+			return "unknown";
+		return net_access_strings[access_bit];
 	}
 
 	WARN_ON_ONCE(1);
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 320394fd6b84..1075b0c8401f 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -18,6 +18,7 @@ enum landlock_request_type {
 	LANDLOCK_REQUEST_PTRACE = 1,
 	LANDLOCK_REQUEST_FS_CHANGE_LAYOUT,
 	LANDLOCK_REQUEST_FS_ACCESS,
+	LANDLOCK_REQUEST_NET_ACCESS,
 };
 
 /*
diff --git a/security/landlock/net.c b/security/landlock/net.c
index 27872d0f7e11..c21afd6e0b4d 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -7,10 +7,12 @@
  */
 
 #include <linux/in.h>
+#include <linux/lsm_audit.h>
 #include <linux/net.h>
 #include <linux/socket.h>
 #include <net/ipv6.h>
 
+#include "audit.h"
 #include "common.h"
 #include "cred.h"
 #include "limits.h"
@@ -56,6 +58,10 @@ static int current_check_access_socket(struct socket *const sock,
 	};
 	const struct landlock_ruleset *const dom =
 		landlock_match_ruleset(landlock_get_current_domain(), any_net);
+	struct lsm_network_audit audit_net = {};
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_NET_ACCESS,
+	};
 
 	if (!dom)
 		return 0;
@@ -72,18 +78,49 @@ static int current_check_access_socket(struct socket *const sock,
 
 	switch (address->sa_family) {
 	case AF_UNSPEC:
-	case AF_INET:
+	case AF_INET: {
+		const struct sockaddr_in *addr4;
+
 		if (addrlen < sizeof(struct sockaddr_in))
 			return -EINVAL;
-		port = ((struct sockaddr_in *)address)->sin_port;
+
+		addr4 = (struct sockaddr_in *)address;
+		port = addr4->sin_port;
+
+		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
+			audit_net.dport = port;
+			audit_net.v4info.daddr = addr4->sin_addr.s_addr;
+		} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
+			audit_net.sport = port;
+			audit_net.v4info.saddr = addr4->sin_addr.s_addr;
+		} else {
+			WARN_ON_ONCE(1);
+		}
 		break;
+	}
 
 #if IS_ENABLED(CONFIG_IPV6)
-	case AF_INET6:
+	case AF_INET6: {
+		const struct sockaddr_in6 *addr6;
+
 		if (addrlen < SIN6_LEN_RFC2133)
 			return -EINVAL;
-		port = ((struct sockaddr_in6 *)address)->sin6_port;
+
+		addr6 = (struct sockaddr_in6 *)address;
+		port = addr6->sin6_port;
+		audit_net.v6info.saddr = addr6->sin6_addr;
+
+		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
+			audit_net.dport = port;
+			audit_net.v6info.daddr = addr6->sin6_addr;
+		} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
+			audit_net.sport = port;
+			audit_net.v6info.saddr = addr6->sin6_addr;
+		} else {
+			WARN_ON_ONCE(1);
+		}
 		break;
+	}
 #endif /* IS_ENABLED(CONFIG_IPV6) */
 
 	default:
@@ -152,6 +189,13 @@ static int current_check_access_socket(struct socket *const sock,
 				   ARRAY_SIZE(layer_masks)))
 		return 0;
 
+	audit_net.family = address->sa_family;
+	request.audit.type = LSM_AUDIT_DATA_NET;
+	request.audit.u.net = &audit_net;
+	request.access = access_request;
+	request.layer_masks = &layer_masks;
+	request.layer_masks_size = ARRAY_SIZE(layer_masks);
+	landlock_log_denial(dom, &request);
 	return -EACCES;
 }
 
-- 
2.47.0

[RFC PATCH v2 13/14] landlock: Log scoped denials

[Mickaël Salaün]

Add audit support for unix_stream_connect, unix_may_send, task_kill, and
file_send_sigiotask hooks.

Audit record sample:

  DENY:    domain=4533720578 blockers=scope_abstract_unix_socket path=00666F6F
  SYSCALL: arch=c000003e syscall=42 success=no exit=-1 ...

Cc: Günther Noack <gnoack@google.com>
Cc: Tahera Fahimi <fahimitahera@gmail.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-14-mic@digikod.net
---
 security/landlock/audit.c |  8 ++++++
 security/landlock/audit.h |  2 ++
 security/landlock/task.c  | 58 ++++++++++++++++++++++++++++++++++++---
 3 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index c31a4a8719ee..b551812b8bc9 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -69,6 +69,14 @@ get_blocker(const enum landlock_request_type type,
 		if (WARN_ON_ONCE(access_bit >= ARRAY_SIZE(net_access_strings)))
 			return "unknown";
 		return net_access_strings[access_bit];
+
+	case LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET:
+		WARN_ON_ONCE(access_bit != -1);
+		return "scope_abstract_unix_socket";
+
+	case LANDLOCK_REQUEST_SCOPE_SIGNAL:
+		WARN_ON_ONCE(access_bit != -1);
+		return "scope_signal";
 	}
 
 	WARN_ON_ONCE(1);
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 1075b0c8401f..1e0a9164bacc 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -19,6 +19,8 @@ enum landlock_request_type {
 	LANDLOCK_REQUEST_FS_CHANGE_LAYOUT,
 	LANDLOCK_REQUEST_FS_ACCESS,
 	LANDLOCK_REQUEST_NET_ACCESS,
+	LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
+	LANDLOCK_REQUEST_SCOPE_SIGNAL,
 };
 
 /*
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 8c4468fb86bf..ddcb993bd53a 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -262,13 +262,27 @@ static int hook_unix_stream_connect(struct sock *const sock,
 {
 	const struct landlock_ruleset *const dom = landlock_match_ruleset(
 		landlock_get_current_domain(), unix_scope);
+	struct lsm_network_audit audit_net = {
+		.sk = other,
+	};
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
+		.audit = {
+			.type = LSM_AUDIT_DATA_NET,
+			.u.net = &audit_net,
+		},
+	};
 
 	/* Quick return for non-landlocked tasks. */
 	if (!dom)
 		return 0;
 
-	if (is_abstract_socket(other) && sock_is_scoped(other, dom))
+	if (is_abstract_socket(other) && sock_is_scoped(other, dom)) {
+		request.layer_plus_one =
+			landlock_match_layer_level(dom, unix_scope) + 1;
+		landlock_log_denial(dom, &request);
 		return -EPERM;
+	}
 
 	return 0;
 }
@@ -278,6 +292,16 @@ static int hook_unix_may_send(struct socket *const sock,
 {
 	const struct landlock_ruleset *const dom = landlock_match_ruleset(
 		landlock_get_current_domain(), unix_scope);
+	struct lsm_network_audit audit_net = {
+		.sk = other->sk,
+	};
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
+		.audit = {
+			.type = LSM_AUDIT_DATA_NET,
+			.u.net = &audit_net,
+		},
+	};
 
 	if (!dom)
 		return 0;
@@ -289,8 +313,12 @@ static int hook_unix_may_send(struct socket *const sock,
 	if (unix_peer(sock->sk) == other->sk)
 		return 0;
 
-	if (is_abstract_socket(other->sk) && sock_is_scoped(other->sk, dom))
+	if (is_abstract_socket(other->sk) && sock_is_scoped(other->sk, dom)) {
+		request.layer_plus_one =
+			landlock_match_layer_level(dom, unix_scope) + 1;
+		landlock_log_denial(dom, &request);
 		return -EPERM;
+	}
 
 	return 0;
 }
@@ -305,6 +333,13 @@ static int hook_task_kill(struct task_struct *const p,
 {
 	bool is_scoped;
 	const struct landlock_ruleset *dom;
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_SCOPE_SIGNAL,
+		.audit = {
+			.type = LSM_AUDIT_DATA_TASK,
+			.u.tsk = p,
+		},
+	};
 
 	if (cred) {
 		/* Dealing with USB IO. */
@@ -322,8 +357,12 @@ static int hook_task_kill(struct task_struct *const p,
 	is_scoped = domain_is_scoped(dom, landlock_get_task_domain(p),
 				     LANDLOCK_SCOPE_SIGNAL);
 	rcu_read_unlock();
-	if (is_scoped)
+	if (is_scoped) {
+		request.layer_plus_one =
+			landlock_match_layer_level(dom, signal_scope) + 1;
+		landlock_log_denial(dom, &request);
 		return -EPERM;
+	}
 
 	return 0;
 }
@@ -332,6 +371,13 @@ static int hook_file_send_sigiotask(struct task_struct *tsk,
 				    struct fown_struct *fown, int signum)
 {
 	const struct landlock_ruleset *dom;
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_SCOPE_SIGNAL,
+		.audit = {
+			.type = LSM_AUDIT_DATA_TASK,
+			.u.tsk = tsk,
+		},
+	};
 	bool is_scoped = false;
 
 	/* Lock already held by send_sigio() and send_sigurg(). */
@@ -347,8 +393,12 @@ static int hook_file_send_sigiotask(struct task_struct *tsk,
 	is_scoped = domain_is_scoped(dom, landlock_get_task_domain(tsk),
 				     LANDLOCK_SCOPE_SIGNAL);
 	rcu_read_unlock();
-	if (is_scoped)
+	if (is_scoped) {
+		request.layer_plus_one =
+			landlock_match_layer_level(dom, signal_scope) + 1;
+		landlock_log_denial(dom, &request);
 		return -EPERM;
+	}
 
 	return 0;
 }
-- 
2.47.0

[RFC PATCH v2 14/14] landlock: Control log events with LANDLOCK_RESTRICT_SELF_LOGLESS

[Mickaël Salaün]

Most of the time we want to log denied access because they should not
happen and such information helps diagnose issues.  However, when
sandboxing processes that we know will try to access denied resources
(e.g. unknown, bogus, or malicious binary), we might want to not log
related access requests that might fill up logs.

To disable any log for a specific Landlock domain, add a
LANDLOCK_RESTRICT_SELF_LOGLESS optional flag to the
landlock_restrict_self() system call.

Because this flag is set for a specific Landlock domain, it makes it
possible to selectively mask some access requests that would be logged
by a parent domain, which might be handy for unprivileged processes to
limit logs.  However, system administrators should still use the audit
filtering mechanism.

There is intentionally no audit nor sysctl configuration to re-enable
these logless domains.  This is delegated to the user space program.

Increment the Landlock ABI version to reflect this interface change.

Cc: Günther Noack <gnoack@google.com>
Cc: Paul Moore <paul@paul-moore.com>
Closes: https://github.com/landlock-lsm/linux/issues/3
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-15-mic@digikod.net
---

We could export and use audit_filter() to avoid computing the youngest
denied layer, but I'm not sure it's worth it.

We need to patch the samples/landlock/sandboxer to use
LANDLOCK_RESTRICT_SELF_LOGLESS because it is a sandboxer, but at the
same time it is useful to test this patch series without this flag.
---
 include/uapi/linux/landlock.h | 14 ++++++++++++++
 security/landlock/audit.c     | 13 ++++++++-----
 security/landlock/domain.h    |  1 +
 security/landlock/syscalls.c  | 25 ++++++++++++++++++++-----
 4 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 33745642f787..3b31d373ef74 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -62,6 +62,20 @@ struct landlock_ruleset_attr {
 #define LANDLOCK_CREATE_RULESET_VERSION			(1U << 0)
 /* clang-format on */
 
+/*
+ * sys_landlock_restrict_self() flags:
+ *
+ * - %LANDLOCK_RESTRICT_SELF_LOGLESS: Do not create any log related to the
+ *   enforced restrictions.  This should only be set by tools launching unknown
+ *   or untrusted programs (e.g. a sandbox tool, container runtime, system
+ *   service manager).  Because programs sandboxing themselves should fix any
+ *   denied access, they should not set this flag to be aware of potential
+ *   issues reported by system's logs (i.e. audit).
+ */
+/* clang-format off */
+#define LANDLOCK_RESTRICT_SELF_LOGLESS			(1U << 0)
+/* clang-format on */
+
 /**
  * enum landlock_rule_type - Landlock rule type
  *
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index b551812b8bc9..9235590997d7 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -401,11 +401,6 @@ void landlock_log_denial(const struct landlock_ruleset *const domain,
 	if (!audit_enabled)
 		return;
 
-	ab = audit_log_start(audit_context(), GFP_ATOMIC | __GFP_NOWARN,
-			     AUDIT_LANDLOCK_DENY);
-	if (!ab)
-		return;
-
 	missing = request->access;
 	if (missing) {
 		size_t youngest_layer;
@@ -426,6 +421,14 @@ void landlock_log_denial(const struct landlock_ruleset *const domain,
 			get_hierarchy(domain, request->layer_plus_one - 1);
 	}
 
+	if (READ_ONCE(youngest_denied->log_status) == LANDLOCK_LOG_DISABLED)
+		return;
+
+	ab = audit_log_start(audit_context(), GFP_ATOMIC | __GFP_NOWARN,
+			     AUDIT_LANDLOCK_DENY);
+	if (!ab)
+		return;
+
 	audit_log_format(ab, "domain=%llu blockers=", youngest_denied->id);
 	log_blockers(ab, request->type, missing);
 	audit_log_lsm_data(ab, &request->audit);
diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index 1374497d9a9b..765d5689fbb0 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -22,6 +22,7 @@
 enum landlock_log_status {
 	LANDLOCK_LOG_PENDING = 0,
 	LANDLOCK_LOG_RECORDED,
+	LANDLOCK_LOG_DISABLED,
 };
 
 /**
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 335067e36feb..48c26ed8c099 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -151,7 +151,12 @@ static const struct file_operations ruleset_fops = {
 	.write = fop_dummy_write,
 };
 
-#define LANDLOCK_ABI_VERSION 6
+/*
+ * The Landlock ABI version should be incremented for each new Landlock-related
+ * user space visible change (e.g. Landlock syscalls).  Only increment this
+ * version once per Linux release.
+ */
+#define LANDLOCK_ABI_VERSION 7
 
 /**
  * sys_landlock_create_ruleset - Create a new ruleset
@@ -452,7 +457,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
  * sys_landlock_restrict_self - Enforce a ruleset on the calling thread
  *
  * @ruleset_fd: File descriptor tied to the ruleset to merge with the target.
- * @flags: Must be 0.
+ * @flags: Supported value: %LANDLOCK_RESTRICT_SELF_LOGLESS.
  *
  * This system call enables to enforce a Landlock ruleset on the current
  * thread.  Enforcing a ruleset requires that the task has %CAP_SYS_ADMIN in its
@@ -478,6 +483,7 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
 	struct cred *new_cred;
 	struct landlock_cred_security *new_llcred;
 	int err;
+	bool is_logless = false;
 
 	if (!is_initialized())
 		return -EOPNOTSUPP;
@@ -490,9 +496,12 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
 	    !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN))
 		return -EPERM;
 
-	/* No flag for now. */
-	if (flags)
-		return -EINVAL;
+	if (flags) {
+		if (flags == LANDLOCK_RESTRICT_SELF_LOGLESS)
+			is_logless = true;
+		else
+			return -EINVAL;
+	}
 
 	/* Gets and checks the ruleset. */
 	ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_READ);
@@ -517,6 +526,12 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
 		goto out_put_creds;
 	}
 
+	if (is_logless) {
+#ifdef CONFIG_AUDIT
+		new_dom->hierarchy->log_status = LANDLOCK_LOG_DISABLED;
+#endif /* CONFIG_AUDIT */
+	}
+
 	/* Replaces the old (prepared) domain. */
 	landlock_put_ruleset(new_llcred->domain);
 	new_llcred->domain = new_dom;
-- 
2.47.0

Re: [RFC PATCH v2 00/14] Landlock audit support

[Mickaël Salaün]

On Tue, Oct 22, 2024 at 06:09:55PM +0200, Mickaël Salaün wrote:
> Hi,
> 
> This patch series adds audit support to Landlock.

> This series is based on my "next" branch, which includes these patches:
> https://lore.kernel.org/r/20241022151144.872797-2-mic@digikod.net

To ease testing, I pushed this series here:
https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git/log/?h=landlock-audit

Re: [RFC PATCH v2 01/14] lsm: Only build lsm_audit.c if CONFIG_AUDIT is set

[Paul Moore]

On Tue, Oct 22, 2024 at 12:10 PM Mickaël Salaün <mic@digikod.net> wrote:
>
> When CONFIG_AUDIT is set, its CONFIG_NET dependency is also set, and the
> dev_get_by_index and init_net symbols (used by dump_common_audit_data)
> are found by the linker.  dump_common_audit_data() should then failed to
> build when CONFIG_NET is not set. However, because the compiler is
> smart, it knows that audit_log_start() always return NULL when
> !CONFIG_AUDIT, and it doesn't build the body of common_lsm_audit().  As
> a side effect, dump_common_audit_data() is not built and the linker
> doesn't error out because of missing symbols.
>
> Let's only build lsm_audit.o when CONFIG_AUDIT is set.
>
> ipv4_skb_to_auditdata() and ipv6_skb_to_auditdata() are only used by
> Smack if CONFIG_AUDIT is set, so they don't need fake implementations.
>
> Because common_lsm_audit() is used in multiple places without
> CONFIG_AUDIT checks, add a fake implementation.
>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Serge E. Hallyn <serge@hallyn.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241022161009.982584-2-mic@digikod.net
> ---
>  include/linux/lsm_audit.h | 14 ++++++++++++++
>  security/Makefile         |  2 +-
>  2 files changed, 15 insertions(+), 1 deletion(-)

I think this fix is the right thing to do regardless of the rest of
the patchset so I've merged it into lsm/dev, if anyone objects please
speak up.

Thanks!

-- 
paul-moore.com

Re: [RFC PATCH v2 02/14] lsm: Add audit_log_lsm_data() helper

[Paul Moore]

On Tue, Oct 22, 2024 at 12:10 PM Mickaël Salaün <mic@digikod.net> wrote:
>
> Extract code from dump_common_audit_data() into the audit_log_lsm_data()
> helper. This helps reuse common LSM audit data while not abusing
> AUDIT_AVC records because of the common_lsm_audit() helper.
>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Serge E. Hallyn <serge@hallyn.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241022161009.982584-3-mic@digikod.net
> ---
>
> Changes since v1:
> * Fix commit message (spotted by Paul).
> * Constify dump_common_audit_data()'s and audit_log_lsm_data()'s "a"
>   argument.
> * Fix build without CONFIG_NET: see previous patch.
> ---
>  include/linux/lsm_audit.h |  8 ++++++++
>  security/lsm_audit.c      | 27 ++++++++++++++++++---------
>  2 files changed, 26 insertions(+), 9 deletions(-)

While not a fix like 1/14, reducing AUDIT_AVC reuse is a reasonable
goal.  Merged into lsm/dev, thanks!

-- 
paul-moore.com

Re: [RFC PATCH v2 01/14] lsm: Only build lsm_audit.c if CONFIG_AUDIT is set

[Guenter Roeck]

On Tue, Oct 22, 2024 at 06:09:56PM +0200, Mickaël Salaün wrote:
> When CONFIG_AUDIT is set, its CONFIG_NET dependency is also set, and the
> dev_get_by_index and init_net symbols (used by dump_common_audit_data)
> are found by the linker.  dump_common_audit_data() should then failed to
> build when CONFIG_NET is not set. However, because the compiler is
> smart, it knows that audit_log_start() always return NULL when
> !CONFIG_AUDIT, and it doesn't build the body of common_lsm_audit().  As
> a side effect, dump_common_audit_data() is not built and the linker
> doesn't error out because of missing symbols.
> 
> Let's only build lsm_audit.o when CONFIG_AUDIT is set.
> 

CONFIG_AUDIT and CONFIG_SECURITY are independent of each other.
With CONFIG_SECURITY=n and CONFIG_AUDIT=y, we now get:

Error log:
arm-linux-gnueabi-ld: security/lsm_audit.o: in function `audit_log_lsm_data':
security/lsm_audit.c:417:(.text+0x5e4): undefined reference to `lockdown_reasons'
arm-linux-gnueabi-ld: security/lsm_audit.c:417:(.text+0x5e8): undefined reference to `lockdown_reasons'
make[3]: *** [scripts/Makefile.vmlinux:78: vmlinux] Error 1
make[2]: *** [Makefile:1178: vmlinux] Error 2
make[1]: *** [Makefile:224: __sub-make] Error 2
make: *** [Makefile:224: __sub-make] Error 2

Guenter

Re: [RFC PATCH v2 01/14] lsm: Only build lsm_audit.c if CONFIG_AUDIT is set

[Paul Moore]

On Wed, Oct 23, 2024 at 2:51 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Tue, Oct 22, 2024 at 06:09:56PM +0200, Mickaël Salaün wrote:
> > When CONFIG_AUDIT is set, its CONFIG_NET dependency is also set, and the
> > dev_get_by_index and init_net symbols (used by dump_common_audit_data)
> > are found by the linker.  dump_common_audit_data() should then failed to
> > build when CONFIG_NET is not set. However, because the compiler is
> > smart, it knows that audit_log_start() always return NULL when
> > !CONFIG_AUDIT, and it doesn't build the body of common_lsm_audit().  As
> > a side effect, dump_common_audit_data() is not built and the linker
> > doesn't error out because of missing symbols.
> >
> > Let's only build lsm_audit.o when CONFIG_AUDIT is set.
>
> CONFIG_AUDIT and CONFIG_SECURITY are independent of each other.
> With CONFIG_SECURITY=n and CONFIG_AUDIT=y, we now get:

Yes, unfortunately the error was seen during linux-next testing too.
I'm removing patch 1/14 from lsm/dev now.

> Error log:
> arm-linux-gnueabi-ld: security/lsm_audit.o: in function `audit_log_lsm_data':
> security/lsm_audit.c:417:(.text+0x5e4): undefined reference to `lockdown_reasons'
> arm-linux-gnueabi-ld: security/lsm_audit.c:417:(.text+0x5e8): undefined reference to `lockdown_reasons'
> make[3]: *** [scripts/Makefile.vmlinux:78: vmlinux] Error 1
> make[2]: *** [Makefile:1178: vmlinux] Error 2
> make[1]: *** [Makefile:224: __sub-make] Error 2
> make: *** [Makefile:224: __sub-make] Error 2

-- 
paul-moore.com

Re: [RFC PATCH v2 02/14] lsm: Add audit_log_lsm_data() helper

[Paul Moore]

On Tue, Oct 22, 2024 at 8:07 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Tue, Oct 22, 2024 at 12:10 PM Mickaël Salaün <mic@digikod.net> wrote:
> >
> > Extract code from dump_common_audit_data() into the audit_log_lsm_data()
> > helper. This helps reuse common LSM audit data while not abusing
> > AUDIT_AVC records because of the common_lsm_audit() helper.
> >
> > Cc: Casey Schaufler <casey@schaufler-ca.com>
> > Cc: James Morris <jmorris@namei.org>
> > Cc: Paul Moore <paul@paul-moore.com>
> > Cc: Serge E. Hallyn <serge@hallyn.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > Link: https://lore.kernel.org/r/20241022161009.982584-3-mic@digikod.net
> > ---
> >
> > Changes since v1:
> > * Fix commit message (spotted by Paul).
> > * Constify dump_common_audit_data()'s and audit_log_lsm_data()'s "a"
> >   argument.
> > * Fix build without CONFIG_NET: see previous patch.
> > ---
> >  include/linux/lsm_audit.h |  8 ++++++++
> >  security/lsm_audit.c      | 27 ++++++++++++++++++---------
> >  2 files changed, 26 insertions(+), 9 deletions(-)
>
> While not a fix like 1/14, reducing AUDIT_AVC reuse is a reasonable
> goal.  Merged into lsm/dev, thanks!

I'm also going to have to remove this patch from lsm/dev due to
problems uncovered by the kernel test robot.

-- 
paul-moore.com

Re: [RFC PATCH v2 04/14] landlock: Add unique ID generator

[Francis Laniel]

Hi!

Le mardi 22 octobre 2024, 18:09:59 CEST Mickaël Salaün a écrit :
> Landlock IDs can be generated to uniquely identify Landlock objects.
> For now, only Landlock domains get an ID at creation time.
> 
> These IDs have important properties:
> * They are unique during the lifetime of the running system thanks to
>   the 64-bit values: at worse, 2^60 - 2*2^32 useful IDs.
> * They are always greater than 2^32 and must then be stored in 64-bit
>   integer types.
> * The initial ID (at boot time) is randomly picked between 2^32 and
>   2^33, which limits collisions in logs between different boots.
> * IDs are sequential, which enables users to order them.
> * IDs may not be consecutive but increase with a random 2^4 step, which
>   limits side channels.
> 
> Such IDs can be exposed to unprivileged processes, even if it is not the
> case with this audit patch series.  The domain IDs will be useful for
> user space to identify sandboxes and get their properties.
> 
> Cc: Günther Noack <gnoack@google.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241022161009.982584-5-mic@digikod.net
> ---
> 
> Changes since v1:
> * New patch.
> ---
>  security/landlock/.kunitconfig               |   2 +
>  security/landlock/Makefile                   |   2 +
>  security/landlock/id.c                       | 242 +++++++++++++++++++
>  security/landlock/id.h                       |  25 ++
>  security/landlock/setup.c                    |   2 +
>  tools/testing/kunit/configs/all_tests.config |   2 +
>  6 files changed, 275 insertions(+)
>  create mode 100644 security/landlock/id.c
>  create mode 100644 security/landlock/id.h
> 
> diff --git a/security/landlock/.kunitconfig b/security/landlock/.kunitconfig
> index 03e119466604..f9423f01ac5b 100644
> --- a/security/landlock/.kunitconfig
> +++ b/security/landlock/.kunitconfig
> @@ -1,4 +1,6 @@
> +CONFIG_AUDIT=y
>  CONFIG_KUNIT=y
> +CONFIG_NET=y
>  CONFIG_SECURITY=y
>  CONFIG_SECURITY_LANDLOCK=y
>  CONFIG_SECURITY_LANDLOCK_KUNIT_TEST=y
> diff --git a/security/landlock/Makefile b/security/landlock/Makefile
> index b4538b7cf7d2..e1777abbc413 100644
> --- a/security/landlock/Makefile
> +++ b/security/landlock/Makefile
> @@ -4,3 +4,5 @@ landlock-y := setup.o syscalls.o object.o ruleset.o \
>  	cred.o task.o fs.o
> 
>  landlock-$(CONFIG_INET) += net.o
> +
> +landlock-$(CONFIG_AUDIT) += id.o
> diff --git a/security/landlock/id.c b/security/landlock/id.c
> new file mode 100644
> index 000000000000..5d0b7743c308
> --- /dev/null
> +++ b/security/landlock/id.c
> @@ -0,0 +1,242 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Landlock LSM - Unique identification number generator
> + *
> + * Copyright © 2024 Microsoft Corporation
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/atomic.h>
> +#include <linux/random.h>
> +#include <linux/spinlock.h>
> +
> +#include "common.h"
> +#include "id.h"
> +
> +#define COUNTER_PRE_INIT 0
> +
> +static atomic64_t global_counter = ATOMIC64_INIT(COUNTER_PRE_INIT);
> +
> +static void __init init_id(atomic64_t *const counter, const u32
> random_32bits) +{
> +	u64 init;
> +
> +	/*
> +	 * Ensures sure 64-bit values are always used by user space (or may
> +	 * fail with -EOVERFLOW), and makes this testable.
> +	 */
> +	init = 1ULL << 32;
> +
> +	/*
> +	 * Makes a large (2^32) boot-time value to limit ID collision in logs
> +	 * from different boots, and to limit info leak about the number of
> +	 * initially (relative to the reader) created elements (e.g. domains).
> +	 */
> +	init += random_32bits;
> +
> +	/* Sets first or ignores.  This will be the first ID. */
> +	atomic64_cmpxchg(counter, COUNTER_PRE_INIT, init);
> +}
> +
> +#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> +
> +static void test_init_min(struct kunit *const test)
> +{
> +	atomic64_t counter = ATOMIC64_INIT(COUNTER_PRE_INIT);
> +
> +	init_id(&counter, 0);
> +	KUNIT_EXPECT_EQ(test, atomic64_read(&counter), 1ULL + U32_MAX);
> +}
> +
> +static void test_init_max(struct kunit *const test)
> +{
> +	atomic64_t counter = ATOMIC64_INIT(COUNTER_PRE_INIT);
> +
> +	init_id(&counter, ~0);
> +	KUNIT_EXPECT_EQ(test, atomic64_read(&counter), 1 + (2ULL * U32_MAX));
> +}
> +
> +static void test_init_once(struct kunit *const test)
> +{
> +	const u64 first_init = 1ULL + U32_MAX;
> +	atomic64_t counter = ATOMIC64_INIT(COUNTER_PRE_INIT);
> +
> +	init_id(&counter, 0);
> +	KUNIT_EXPECT_EQ(test, atomic64_read(&counter), first_init);
> +
> +	init_id(&counter, ~0);
> +	KUNIT_EXPECT_EQ(test, atomic64_read(&counter), first_init);
> +}
> +
> +#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> +
> +void __init landlock_init_id(void)
> +{
> +	return init_id(&global_counter, get_random_u32());
> +}
> +
> +/*
> + * It's not worth it to try to hide the monotonic counter because it can
> still + * be inferred (with N counter ranges), and if we are allowed to
> read the inode + * number we should also be allowed to read the time
> creation anyway, and it + * can be handy to store and sort domain IDs for
> user space.
> + *
> + * Returns the value of global_counter and increment it to let some space
> for + * the next one.
> + */
> +static u64 get_id(size_t number_of_ids, atomic64_t *const counter,
> +		  u8 random_4bits)
> +{
> +	u64 id, step;
> +
> +	/*
> +	 * We should return at least 1 ID, and we may need a set of 
consecutive
> +	 * ones (e.g. to generate a set of inodes).
> +	 */
> +	if (WARN_ON_ONCE(number_of_ids <= 0))
> +		number_of_ids = 1;
> +
> +	/*
> +	 * Blurs the next ID guess with 1/16 ratio.  We get 2^(64 - 4) -
> +	 * (2 * 2^32), so a bit less than 2^60 available IDs, which should be
> +	 * much more than enough considering the number of CPU cycles required
> +	 * to get a new ID (e.g. a full landlock_restrict_self() call), and 
the
> +	 * cost of draining all available IDs during the system's uptime.
> +	 */
> +	random_4bits = random_4bits % (1 << 4);
> +	step = number_of_ids + random_4bits;
> +
> +	/* It is safe to cast a signed atomic to an unsigned value. */
> +	id = atomic64_fetch_add(step, counter);
> +
> +	/* Warns if landlock_init_id() was not called. */
> +	WARN_ON_ONCE(id == COUNTER_PRE_INIT);
> +	return id;
> +}
> +
> +#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> +
> +static void test_range1_rand0(struct kunit *const test)
> +{
> +	atomic64_t counter;
> +	u64 init;
> +
> +	init = get_random_u32();
> +	atomic64_set(&counter, init);
> +	KUNIT_EXPECT_EQ(test, get_id(1, &counter, 0), init);
> +	KUNIT_EXPECT_EQ(test,
> +			get_id(get_random_u8(), &counter, get_random_u8()),
> +			init + 1);
> +}
> +
> +static void test_range1_rand1(struct kunit *const test)
> +{
> +	atomic64_t counter;
> +	u64 init;
> +
> +	init = get_random_u32();
> +	atomic64_set(&counter, init);
> +	KUNIT_EXPECT_EQ(test, get_id(1, &counter, 1), init);
> +	KUNIT_EXPECT_EQ(test,
> +			get_id(get_random_u8(), &counter, get_random_u8()),
> +			init + 2);
> +}
> +
> +static void test_range1_rand16(struct kunit *const test)
> +{
> +	atomic64_t counter;
> +	u64 init;
> +
> +	init = get_random_u32();
> +	atomic64_set(&counter, init);
> +	KUNIT_EXPECT_EQ(test, get_id(1, &counter, 16), init);
> +	KUNIT_EXPECT_EQ(test,
> +			get_id(get_random_u8(), &counter, get_random_u8()),
> +			init + 1);
> +}
> +
> +static void test_range2_rand0(struct kunit *const test)
> +{
> +	atomic64_t counter;
> +	u64 init;
> +
> +	init = get_random_u32();
> +	atomic64_set(&counter, init);
> +	KUNIT_EXPECT_EQ(test, get_id(2, &counter, 0), init);
> +	KUNIT_EXPECT_EQ(test,
> +			get_id(get_random_u8(), &counter, get_random_u8()),
> +			init + 2);
> +}
> +
> +static void test_range2_rand1(struct kunit *const test)
> +{
> +	atomic64_t counter;
> +	u64 init;
> +
> +	init = get_random_u32();
> +	atomic64_set(&counter, init);
> +	KUNIT_EXPECT_EQ(test, get_id(2, &counter, 1), init);
> +	KUNIT_EXPECT_EQ(test,
> +			get_id(get_random_u8(), &counter, get_random_u8()),
> +			init + 3);
> +}
> +
> +static void test_range2_rand2(struct kunit *const test)
> +{
> +	atomic64_t counter;
> +	u64 init;
> +
> +	init = get_random_u32();
> +	atomic64_set(&counter, init);
> +	KUNIT_EXPECT_EQ(test, get_id(2, &counter, 2), init);
> +	KUNIT_EXPECT_EQ(test,
> +			get_id(get_random_u8(), &counter, get_random_u8()),
> +			init + 4);
> +}
> +
> +static void test_range2_rand16(struct kunit *const test)
> +{
> +	atomic64_t counter;
> +	u64 init;
> +
> +	init = get_random_u32();
> +	atomic64_set(&counter, init);
> +	KUNIT_EXPECT_EQ(test, get_id(2, &counter, 16), init);
> +	KUNIT_EXPECT_EQ(test,
> +			get_id(get_random_u8(), &counter, get_random_u8()),
> +			init + 2);
> +}
> +
> +#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> +
> +u64 landlock_get_id(size_t number_of_ids)
> +{
> +	return get_id(number_of_ids, &global_counter, get_random_u8());
> +}
> +
> +#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> +
> +static struct kunit_case test_cases[] = {
> +	/* clang-format off */
> +	KUNIT_CASE(test_init_min),
> +	KUNIT_CASE(test_init_max),
> +	KUNIT_CASE(test_init_once),
> +	KUNIT_CASE(test_range1_rand0),
> +	KUNIT_CASE(test_range1_rand1),
> +	KUNIT_CASE(test_range1_rand16),
> +	KUNIT_CASE(test_range2_rand0),
> +	KUNIT_CASE(test_range2_rand1),
> +	KUNIT_CASE(test_range2_rand2),
> +	KUNIT_CASE(test_range2_rand16),
> +	{}
> +	/* clang-format on */
> +};
> +
> +static struct kunit_suite test_suite = {
> +	.name = "landlock_id",
> +	.test_cases = test_cases,
> +};
> +
> +kunit_test_suite(test_suite);
> +
> +#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> diff --git a/security/landlock/id.h b/security/landlock/id.h
> new file mode 100644
> index 000000000000..689ba7607472
> --- /dev/null
> +++ b/security/landlock/id.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Landlock LSM - Unique identification number generator
> + *
> + * Copyright © 2024 Microsoft Corporation
> + */
> +
> +#ifndef _SECURITY_LANDLOCK_ID_H
> +#define _SECURITY_LANDLOCK_ID_H
> +
> +#ifdef CONFIG_AUDIT
> +
> +void __init landlock_init_id(void);
> +
> +u64 landlock_get_id(size_t number_of_ids);
> +
> +#else /* CONFIG_AUDIT */
> +
> +static inline void __init landlock_init_id(void)
> +{
> +}

Should the function have the same signature than when CONFIG_AUDIT is set?

> +#endif /* CONFIG_AUDIT */
> +
> +#endif /* _SECURITY_LANDLOCK_ID_H */
> diff --git a/security/landlock/setup.c b/security/landlock/setup.c
> index 28519a45b11f..d297083efcb1 100644
> --- a/security/landlock/setup.c
> +++ b/security/landlock/setup.c
> @@ -13,6 +13,7 @@
>  #include "common.h"
>  #include "cred.h"
>  #include "fs.h"
> +#include "id.h"
>  #include "net.h"
>  #include "setup.h"
>  #include "task.h"
> @@ -33,6 +34,7 @@ const struct lsm_id landlock_lsmid = {
> 
>  static int __init landlock_init(void)
>  {
> +	landlock_init_id();
>  	landlock_add_cred_hooks();
>  	landlock_add_task_hooks();
>  	landlock_add_fs_hooks();
> diff --git a/tools/testing/kunit/configs/all_tests.config
> b/tools/testing/kunit/configs/all_tests.config index
> b3b00269a52a..ea1f824ae70f 100644
> --- a/tools/testing/kunit/configs/all_tests.config
> +++ b/tools/testing/kunit/configs/all_tests.config
> @@ -44,6 +44,8 @@ CONFIG_DAMON_DBGFS_DEPRECATED=y
> 
>  CONFIG_REGMAP_BUILD=y
> 
> +CONFIG_AUDIT=y
> +
>  CONFIG_SECURITY=y
>  CONFIG_SECURITY_APPARMOR=y
>  CONFIG_SECURITY_LANDLOCK=y

Best regards.

Re: [RFC PATCH v2 05/14] landlock: Move access types

[Francis Laniel]

Le mardi 22 octobre 2024, 18:10:00 CEST Mickaël Salaün a écrit :
> Move ACCESS_FS_OPTIONAL, access_mask_t, struct access_mask, and struct
> access_masks_all to a dedicated access.h file.
> 
> This file will be extended with a following commit, and it will help to
> avoid dependency loops.
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241022161009.982584-6-mic@digikod.net
> ---
> 
> Changes since v1:
> * New patch
> ---
>  security/landlock/access.h  | 53 +++++++++++++++++++++++++++++++++++++
>  security/landlock/fs.c      |  1 +
>  security/landlock/fs.h      |  1 +
>  security/landlock/ruleset.h | 31 +---------------------
>  4 files changed, 56 insertions(+), 30 deletions(-)
>  create mode 100644 security/landlock/access.h
> 
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> new file mode 100644
> index 000000000000..2659fd9b4aaf
> --- /dev/null
> +++ b/security/landlock/access.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Landlock LSM - Access types and helpers
> + *
> + * Copyright © 2016-2020 Mickaël Salaün <mic@digikod.net>
> + * Copyright © 2018-2020 ANSSI
> + * Copyright © 2024 Microsoft Corporation
> + */
> +
> +#ifndef _SECURITY_LANDLOCK_ACCESS_H
> +#define _SECURITY_LANDLOCK_ACCESS_H
> +
> +#include <uapi/linux/landlock.h>
> +
> +#include "limits.h"
> +
> +/* clang-format off */
> +#define ACCESS_FS_OPTIONAL ( \
> +	LANDLOCK_ACCESS_FS_TRUNCATE | \
> +	LANDLOCK_ACCESS_FS_IOCTL_DEV)

Nit: The patch message indicates this is moved from somewhere but I cannot find 
deletion for it.

> +/* clang-format on */
> +
> +typedef u16 access_mask_t;
> +/* Makes sure all filesystem access rights can be stored. */
> +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
> +/* Makes sure all network access rights can be stored. */
> +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET);
> +/* Makes sure all scoped rights can be stored. */
> +static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_SCOPE);
> +/* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
> +static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
> +
> +/* Ruleset access masks. */
> +struct access_masks {
> +	access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
> +	access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
> +	access_mask_t scope : LANDLOCK_NUM_SCOPE;
> +};
> +
> +union access_masks_all {
> +	struct access_masks masks;
> +	u32 all;
> +};
> +
> +/* Makes sure all fields are covered. */
> +static_assert(sizeof(((union access_masks_all *)NULL)->masks) ==
> +	      sizeof(((union access_masks_all *)NULL)->all));
> +
> +typedef u16 layer_mask_t;
> +/* Makes sure all layers can be checked. */
> +static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
> +
> +#endif /* _SECURITY_LANDLOCK_ACCESS_H */
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 698a623a8184..e0e5775b75ae 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -36,6 +36,7 @@
>  #include <uapi/linux/fiemap.h>
>  #include <uapi/linux/landlock.h>
> 
> +#include "access.h"
>  #include "common.h"
>  #include "cred.h"
>  #include "fs.h"
> diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> index 1487e1f023a1..d445f411c26a 100644
> --- a/security/landlock/fs.h
> +++ b/security/landlock/fs.h
> @@ -13,6 +13,7 @@
>  #include <linux/init.h>
>  #include <linux/rcupdate.h>
> 
> +#include "access.h"
>  #include "ruleset.h"
>  #include "setup.h"
> 
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index e00edcb38c5b..7921bbe01344 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -17,6 +17,7 @@
>  #include <linux/workqueue.h>
>  #include <uapi/linux/landlock.h>
> 
> +#include "access.h"
>  #include "limits.h"
>  #include "object.h"
> 
> @@ -30,36 +31,6 @@
>  	LANDLOCK_ACCESS_FS_REFER)
>  /* clang-format on */
> 
> -typedef u16 access_mask_t;
> -/* Makes sure all filesystem access rights can be stored. */
> -static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
> -/* Makes sure all network access rights can be stored. */
> -static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET);
> -/* Makes sure all scoped rights can be stored. */
> -static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_SCOPE);
> -/* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
> -static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
> -
> -/* Ruleset access masks. */
> -struct access_masks {
> -	access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
> -	access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
> -	access_mask_t scope : LANDLOCK_NUM_SCOPE;
> -};
> -
> -union access_masks_all {
> -	struct access_masks masks;
> -	u32 all;
> -};
> -
> -/* Makes sure all fields are covered. */
> -static_assert(sizeof(((union access_masks_all *)NULL)->masks) ==
> -	      sizeof(((union access_masks_all *)NULL)->all));
> -
> -typedef u16 layer_mask_t;
> -/* Makes sure all layers can be checked. */
> -static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
> -
>  /**
>   * struct landlock_layer - Access rights for a given layer
>   */

Re: [RFC PATCH v2 10/14] landlock: Log file-related denials

[Francis Laniel]

Le mardi 22 octobre 2024, 18:10:05 CEST Mickaël Salaün a écrit :
> Add audit support for path_mkdir, path_mknod, path_symlink, path_unlink,
> path_rmdir, path_truncate, path_link, path_rename, and file_open hooks.
> 
> Audit record sample for a link action:
> 
>   DENY:     domain=4533720568 blockers=fs_refer path="/usr/bin" dev="vda2"
> ino=351 DOM_INFO: domain=4533720568 parent=0 pid=325 uid=0
> exe="/root/sandboxer" comm="sandboxer" DENY:     domain=4533720568
> blockers=fs_make_reg,fs_refer path="/usr/local" dev="vda2" ino=365 SYSCALL:
>  arch=c000003e syscall=265 success=no exit=-13 ...
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241022161009.982584-11-mic@digikod.net
> ---
> 
> Changes since v2:
> * Revamp logging and support the path_link and path_rename hooks.
> * Add KUnit tests.
> 
> Changes since v1:
> * Move audit code to the ptrace patch.
> ---
>  security/landlock/audit.c | 173 ++++++++++++++++++++++++++++++++++++--
>  security/landlock/audit.h |   9 ++
>  security/landlock/fs.c    |  64 +++++++++++---
>  3 files changed, 229 insertions(+), 17 deletions(-)
> 
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index 4cd9407459d2..9c8b6c246884 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -7,23 +7,55 @@
> 
>  #include <kunit/test.h>
>  #include <linux/audit.h>
> +#include <linux/bitops.h>
>  #include <linux/lsm_audit.h>
>  #include <linux/pid.h>
>  #include <linux/uidgid.h>
> +#include <uapi/linux/landlock.h>
> 
>  #include "audit.h"
> +#include "common.h"
>  #include "cred.h"
>  #include "domain.h"
>  #include "ruleset.h"
> 
> -static const char *get_blocker(const enum landlock_request_type type)
> +static const char *const fs_access_strings[] = {
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = "fs_execute",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = "fs_write_file",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = "fs_read_file",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = "fs_read_dir",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = "fs_remove_dir",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_FILE)] = "fs_remove_file",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_CHAR)] = "fs_make_char",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_DIR)] = "fs_make_dir",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_REG)] = "fs_make_reg",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_SOCK)] = "fs_make_sock",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_FIFO)] = "fs_make_fifo",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_BLOCK)] = "fs_make_block",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_SYM)] = "fs_make_sym",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs_refer",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs_truncate",
> +	[BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs_ioctl_dev",
> +};
> +static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> +
> +static __attribute_const__ const char *
> +get_blocker(const enum landlock_request_type type,
> +	    const unsigned long access_bit)
>  {
>  	switch (type) {
>  	case LANDLOCK_REQUEST_PTRACE:
> +		WARN_ON_ONCE(access_bit != -1);
>  		return "ptrace";
> 
>  	case LANDLOCK_REQUEST_FS_CHANGE_LAYOUT:
> +		WARN_ON_ONCE(access_bit != -1);
>  		return "fs_change_layout";
> +
> +	case LANDLOCK_REQUEST_FS_ACCESS:
> +		if (WARN_ON_ONCE(access_bit >= ARRAY_SIZE(fs_access_strings)))
> +			return "unknown";
> +		return fs_access_strings[access_bit];
>  	}
> 
>  	WARN_ON_ONCE(1);
> @@ -31,9 +63,20 @@ static const char *get_blocker(const enum
> landlock_request_type type) }
> 
>  static void log_blockers(struct audit_buffer *const ab,
> -			 const enum landlock_request_type type)
> +			 const enum landlock_request_type type,
> +			 const access_mask_t access)
>  {
> -	audit_log_format(ab, "%s", get_blocker(type));
> +	const unsigned long access_mask = access;
> +	unsigned long access_bit;
> +	size_t i = 0;
> +
> +	for_each_set_bit(access_bit, &access_mask, BITS_PER_TYPE(access)) {
> +		audit_log_format(ab, "%s%s", (i == 0) ? "" : ",",
> +				 get_blocker(type, access_bit));
> +		i++;
> +	}
> +	if (i == 0)
> +		audit_log_format(ab, "%s", get_blocker(type, -1));
>  }
> 
>  static void log_node(struct landlock_hierarchy *const node)
> @@ -121,9 +164,110 @@ static void test_get_hierarchy(struct kunit *const
> test)
> 
>  #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> 
> +static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> +			       access_mask_t *const access_request,
> +			       const layer_mask_t (*const layer_masks)[],
> +			       const size_t layer_masks_size)
> +{
> +	const unsigned long access_req = *access_request;

Nit: should access_request being checked for not being NULL?

> +	unsigned long access_bit;
> +	access_mask_t missing = 0;
> +	long youngest_layer = -1;
> +
> +	for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> +		const access_mask_t mask = (*layer_masks)[access_bit];
> +		long layer;
> +
> +		if (!mask)
> +			continue;
> +
> +		/* __fls(1) == 0 */
> +		layer = __fls(mask);
> +		if (layer > youngest_layer) {
> +			youngest_layer = layer;
> +			missing = BIT(access_bit);
> +		} else if (layer == youngest_layer) {
> +			missing |= BIT(access_bit);
> +		}
> +	}
> +
> +	*access_request = missing;
> +	if (youngest_layer == -1)
> +		return domain->num_layers - 1;
> +
> +	return youngest_layer;
> +}
> +
> +#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> +
> +static void test_get_denied_layer(struct kunit *const test)
> +{
> +	const struct landlock_ruleset dom = {
> +		.num_layers = 5,
> +	};
> +	const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> +		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT(0),
> +		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT(1),
> +		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = BIT(1) | BIT(0),
> +		[BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = BIT(2),
> +	};
> +	access_mask_t access;
> +
> +	access = LANDLOCK_ACCESS_FS_EXECUTE;
> +	KUNIT_EXPECT_EQ(test, 0,
> +			get_denied_layer(&dom, &access, &layer_masks,
> +					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_EXECUTE);
> +
> +	access = LANDLOCK_ACCESS_FS_READ_FILE;
> +	KUNIT_EXPECT_EQ(test, 1,
> +			get_denied_layer(&dom, &access, &layer_masks,
> +					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_FILE);
> +
> +	access = LANDLOCK_ACCESS_FS_READ_DIR;
> +	KUNIT_EXPECT_EQ(test, 1,
> +			get_denied_layer(&dom, &access, &layer_masks,
> +					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
> +
> +	access = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR;
> +	KUNIT_EXPECT_EQ(test, 1,
> +			get_denied_layer(&dom, &access, &layer_masks,
> +					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, access,
> +			LANDLOCK_ACCESS_FS_READ_FILE |
> +				LANDLOCK_ACCESS_FS_READ_DIR);
> +
> +	access = LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_READ_DIR;
> +	KUNIT_EXPECT_EQ(test, 1,
> +			get_denied_layer(&dom, &access, &layer_masks,
> +					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
> +
> +	access = LANDLOCK_ACCESS_FS_WRITE_FILE;
> +	KUNIT_EXPECT_EQ(test, 4,
> +			get_denied_layer(&dom, &access, &layer_masks,
> +					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, access, 0);
> +}
> +
> +#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))
> +	if (WARN_ON_ONCE(!(!!request->layer_plus_one ^ !!request->access)))
> +		return false;
> +
> +	if (request->access) {
> +		if (WARN_ON_ONCE(!request->layer_masks))
> +			return false;
> +	} else {
> +		if (WARN_ON_ONCE(request->layer_masks))
> +			return false;
> +	}
> +
> +	if (WARN_ON_ONCE(!!request->layer_masks ^ !!request-
>layer_masks_size))
>  		return false;
> 
>  	return true;
> @@ -140,6 +284,7 @@ void landlock_log_denial(const struct landlock_ruleset
> *const domain, {
>  	struct audit_buffer *ab;
>  	struct landlock_hierarchy *youngest_denied;
> +	access_mask_t missing;
> 
>  	if (WARN_ON_ONCE(!domain || !domain->hierarchy || !request))
>  		return;
> @@ -155,9 +300,24 @@ void landlock_log_denial(const struct landlock_ruleset
> *const domain, if (!ab)
>  		return;
> 
> -	youngest_denied = get_hierarchy(domain, request->layer_plus_one - 1);
> +	missing = request->access;
> +	if (missing) {
> +		size_t youngest_layer;
> +
> +		/* Gets the nearest domain that denies the request. */
> +		if (request->layer_masks) {
> +			youngest_layer = get_denied_layer(
> +				domain, &missing, request->layer_masks,
> +				request->layer_masks_size);
> +		}
> +		youngest_denied = get_hierarchy(domain, youngest_layer);

If request->layer_masks is 0, it is possible to call get_hierarchy() with
uninitialized youngest_layer, is this wanted?

> +	} else {
> +		youngest_denied =
> +			get_hierarchy(domain, request->layer_plus_one - 1);
> +	}
> +
>  	audit_log_format(ab, "domain=%llu blockers=", youngest_denied->id);
> -	log_blockers(ab, request->type);
> +	log_blockers(ab, request->type, missing);
>  	audit_log_lsm_data(ab, &request->audit);
>  	audit_log_end(ab);
> 
> @@ -204,6 +364,7 @@ void landlock_log_drop_domain(const struct
> landlock_ruleset *const domain) static struct kunit_case test_cases[] = {
>  	/* clang-format off */
>  	KUNIT_CASE(test_get_hierarchy),
> +	KUNIT_CASE(test_get_denied_layer),
>  	{}
>  	/* clang-format on */
>  };
> diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> index 6f5ad04b83c2..25fc8333cddc 100644
> --- a/security/landlock/audit.h
> +++ b/security/landlock/audit.h
> @@ -11,11 +11,13 @@
>  #include <linux/audit.h>
>  #include <linux/lsm_audit.h>
> 
> +#include "access.h"
>  #include "ruleset.h"
> 
>  enum landlock_request_type {
>  	LANDLOCK_REQUEST_PTRACE = 1,
>  	LANDLOCK_REQUEST_FS_CHANGE_LAYOUT,
> +	LANDLOCK_REQUEST_FS_ACCESS,
>  };
> 
>  /*
> @@ -33,6 +35,13 @@ struct landlock_request {
>  	 * extra one is useful to detect uninitialized field.
>  	 */
>  	size_t layer_plus_one;
> +
> +	/* Required field for configurable access control. */
> +	access_mask_t access;
> +
> +	/* Required fields for requests with layer masks. */
> +	const layer_mask_t (*layer_masks)[];
> +	size_t layer_masks_size;
>  };
> 
>  #ifdef CONFIG_AUDIT
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index a099167d2347..7f69bed9e095 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -730,6 +730,7 @@ static void test_is_eacces_with_write(struct kunit
> *const test) *     those identified by @access_request_parent1).  This
> matrix can *     initially refer to domain layer masks and, when the
> accesses for the *     destination and source are the same, to requested
> layer masks. + * @log_request_parent1: Audit request to fill if the related
> access is denied. * @dentry_child1: Dentry to the initial child of the
> parent1 path.  This *     pointer must be NULL for non-refer actions (i.e.
> not link nor rename). * @access_request_parent2: Similar to
> @access_request_parent1 but for a @@ -738,6 +739,7 @@ static void
> test_is_eacces_with_write(struct kunit *const test) *     the source.  Must
> be set to 0 when using a simple path request. * @layer_masks_parent2:
> Similar to @layer_masks_parent1 but for a refer *     action.  This must be
> NULL otherwise.
> + * @log_request_parent2: Audit request to fill if the related access is
> denied. * @dentry_child2: Dentry to the initial child of the parent2 path. 
> This *     pointer is only set for RENAME_EXCHANGE actions and must be NULL
> *     otherwise.
> @@ -757,10 +759,12 @@ static bool is_access_to_paths_allowed(
>  	const struct path *const path,
>  	const access_mask_t access_request_parent1,
>  	layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
> -	const struct dentry *const dentry_child1,
> +	struct landlock_request *const log_request_parent1,
> +	struct dentry *const dentry_child1,
>  	const access_mask_t access_request_parent2,
>  	layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
> -	const struct dentry *const dentry_child2)
> +	struct landlock_request *const log_request_parent2,
> +	struct dentry *const dentry_child2)
>  {
>  	bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check,
>  	     child1_is_directory = true, child2_is_directory = true;
> @@ -907,6 +911,24 @@ static bool is_access_to_paths_allowed(
>  	}
>  	path_put(&walker_path);
> 
> +	if (!allowed_parent1 && log_request_parent1) {
> +		log_request_parent1->type = LANDLOCK_REQUEST_FS_ACCESS,
> +		log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH,
> +		log_request_parent1->audit.u.path = *path;
> +		log_request_parent1->access = access_request_parent1;
> +		log_request_parent1->layer_masks = layer_masks_parent1;
> +		log_request_parent1->layer_masks_size =
> +			ARRAY_SIZE(*layer_masks_parent1);
> +	}
> +	if (!allowed_parent2 && log_request_parent2) {
> +		log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS,
> +		log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH,
> +		log_request_parent2->audit.u.path = *path;
> +		log_request_parent2->access = access_request_parent2;
> +		log_request_parent2->layer_masks = layer_masks_parent2;
> +		log_request_parent2->layer_masks_size =
> +			ARRAY_SIZE(*layer_masks_parent2);
> +	}

You may want to add a function to set these fields in log_request.

>  	return allowed_parent1 && allowed_parent2;
>  }
> 
> @@ -915,6 +937,7 @@ static int current_check_access_path(const struct path
> *const path, {
>  	const struct landlock_ruleset *const dom = get_current_fs_domain();
>  	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> +	struct landlock_request request = {};
> 
>  	if (!dom)
>  		return 0;
> @@ -922,9 +945,10 @@ static int current_check_access_path(const struct path
> *const path, access_request = landlock_init_layer_masks(
>  		dom, access_request, &layer_masks, LANDLOCK_KEY_INODE);
>  	if (is_access_to_paths_allowed(dom, path, access_request, 
&layer_masks,
> -				       NULL, 0, NULL, NULL))
> +				       &request, NULL, 0, NULL, NULL, NULL))
>  		return 0;
> 
> +	landlock_log_denial(dom, &request);
>  	return -EACCES;
>  }
> 
> @@ -1093,6 +1117,7 @@ static int current_check_refer_path(struct dentry
> *const old_dentry, struct dentry *old_parent;
>  	layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {},
>  		     layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {};
> +	struct landlock_request request1 = {}, request2 = {};
> 
>  	if (!dom)
>  		return 0;
> @@ -1124,10 +1149,13 @@ static int current_check_refer_path(struct dentry
> *const old_dentry, access_request_parent1 = landlock_init_layer_masks(
>  			dom, access_request_parent1 | access_request_parent2,
>  			&layer_masks_parent1, LANDLOCK_KEY_INODE);
> -		if (is_access_to_paths_allowed(
> -			    dom, new_dir, access_request_parent1,
> -			    &layer_masks_parent1, NULL, 0, NULL, NULL))
> +		if (is_access_to_paths_allowed(dom, new_dir,
> +					       access_request_parent1,
> +					       &layer_masks_parent1, &request1,
> +					       NULL, 0, NULL, NULL, NULL))
>  			return 0;
> +
> +		landlock_log_denial(dom, &request1);
>  		return -EACCES;
>  	}
> 
> @@ -1162,12 +1190,22 @@ static int current_check_refer_path(struct dentry
> *const old_dentry, * parent access rights.  This will be useful to compare
> with the * destination parent access rights.
>  	 */
> -	if (is_access_to_paths_allowed(
> -		    dom, &mnt_dir, access_request_parent1, 
&layer_masks_parent1,
> -		    old_dentry, access_request_parent2, &layer_masks_parent2,
> -		    exchange ? new_dentry : NULL))
> +	if (is_access_to_paths_allowed(dom, &mnt_dir, access_request_parent1,
> +				       &layer_masks_parent1, &request1,
> +				       old_dentry, access_request_parent2,
> +				       &layer_masks_parent2, &request2,
> +				       exchange ? new_dentry : NULL))
>  		return 0;
> 
> +	if (request1.access) {
> +		request1.audit.u.path.dentry = old_parent;
> +		landlock_log_denial(dom, &request1);
> +	}
> +	if (request2.access) {
> +		request2.audit.u.path.dentry = new_dir->dentry;
> +		landlock_log_denial(dom, &request2);
> +	}
> +
>  	/*
>  	 * This prioritizes EACCES over EXDEV for all actions, including
>  	 * renames with RENAME_EXCHANGE.
> @@ -1546,6 +1584,7 @@ static int hook_file_open(struct file *const file)
>  		optional_access;
>  	const struct landlock_ruleset *const dom = landlock_match_ruleset(
>  		landlock_cred(file->f_cred)->domain, any_fs);
> +	struct landlock_request request = {};
> 
>  	if (!dom)
>  		return 0;
> @@ -1571,7 +1610,7 @@ static int hook_file_open(struct file *const file)
>  		    dom, &file->f_path,
>  		    landlock_init_layer_masks(dom, full_access_request,
>  					      &layer_masks, LANDLOCK_KEY_INODE),
> -		    &layer_masks, NULL, 0, NULL, NULL)) {
> +		    &layer_masks, &request, NULL, 0, NULL, NULL, NULL)) {
>  		allowed_access = full_access_request;
>  	} else {
>  		unsigned long access_bit;
> @@ -1601,6 +1640,9 @@ static int hook_file_open(struct file *const file)
>  	if ((open_access_request & allowed_access) == open_access_request)
>  		return 0;
> 
> +	/* Sets access to reflect the actual request. */
> +	request.access = open_access_request;
> +	landlock_log_denial(dom, &request);
>  	return -EACCES;
>  }

Re: [RFC PATCH v2 12/14] landlock: Log TCP bind and connect denials

[Francis Laniel]

Le mardi 22 octobre 2024, 18:10:07 CEST Mickaël Salaün a écrit :
> Add audit support to socket_bind and socket_connect hooks.
> 
> Audit record sample:
> 
>   DENY:    domain=4533720601 blockers=net_connect_tcp daddr=127.0.0.1
> dest=80 SYSCALL: arch=c000003e syscall=42 success=no exit=-13 ...
> 
> Cc: Günther Noack <gnoack@google.com>
> Cc: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> Cc: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241022161009.982584-13-mic@digikod.net
> ---
>  security/landlock/audit.c | 11 +++++++++
>  security/landlock/audit.h |  1 +
>  security/landlock/net.c   | 52 ++++++++++++++++++++++++++++++++++++---
>  3 files changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index 898c95ebe847..c31a4a8719ee 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -41,6 +41,12 @@ static const char *const fs_access_strings[] = {
>  };
>  static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> 
> +static const char *const net_access_strings[] = {
> +	[BIT_INDEX(LANDLOCK_ACCESS_NET_BIND_TCP)] = "net_bind_tcp",
> +	[BIT_INDEX(LANDLOCK_ACCESS_NET_CONNECT_TCP)] = "net_connect_tcp",
> +};
> +static_assert(ARRAY_SIZE(net_access_strings) == LANDLOCK_NUM_ACCESS_NET);
> +
>  static __attribute_const__ const char *
>  get_blocker(const enum landlock_request_type type,
>  	    const unsigned long access_bit)
> @@ -58,6 +64,11 @@ get_blocker(const enum landlock_request_type type,
>  		if (WARN_ON_ONCE(access_bit >= ARRAY_SIZE(fs_access_strings)))
>  			return "unknown";
>  		return fs_access_strings[access_bit];
> +
> +	case LANDLOCK_REQUEST_NET_ACCESS:
> +		if (WARN_ON_ONCE(access_bit >= ARRAY_SIZE(net_access_strings)))
> +			return "unknown";
> +		return net_access_strings[access_bit];
>  	}
> 
>  	WARN_ON_ONCE(1);
> diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> index 320394fd6b84..1075b0c8401f 100644
> --- a/security/landlock/audit.h
> +++ b/security/landlock/audit.h
> @@ -18,6 +18,7 @@ enum landlock_request_type {
>  	LANDLOCK_REQUEST_PTRACE = 1,
>  	LANDLOCK_REQUEST_FS_CHANGE_LAYOUT,
>  	LANDLOCK_REQUEST_FS_ACCESS,
> +	LANDLOCK_REQUEST_NET_ACCESS,
>  };
> 
>  /*
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index 27872d0f7e11..c21afd6e0b4d 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -7,10 +7,12 @@
>   */
> 
>  #include <linux/in.h>
> +#include <linux/lsm_audit.h>
>  #include <linux/net.h>
>  #include <linux/socket.h>
>  #include <net/ipv6.h>
> 
> +#include "audit.h"
>  #include "common.h"
>  #include "cred.h"
>  #include "limits.h"
> @@ -56,6 +58,10 @@ static int current_check_access_socket(struct socket
> *const sock, };
>  	const struct landlock_ruleset *const dom =
>  		landlock_match_ruleset(landlock_get_current_domain(), any_net);
> +	struct lsm_network_audit audit_net = {};
> +	struct landlock_request request = {
> +		.type = LANDLOCK_REQUEST_NET_ACCESS,
> +	};
> 
>  	if (!dom)
>  		return 0;
> @@ -72,18 +78,49 @@ static int current_check_access_socket(struct socket
> *const sock,
> 
>  	switch (address->sa_family) {
>  	case AF_UNSPEC:
> -	case AF_INET:
> +	case AF_INET: {
> +		const struct sockaddr_in *addr4;
> +
>  		if (addrlen < sizeof(struct sockaddr_in))
>  			return -EINVAL;
> -		port = ((struct sockaddr_in *)address)->sin_port;
> +
> +		addr4 = (struct sockaddr_in *)address;
> +		port = addr4->sin_port;
> +
> +		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
> +			audit_net.dport = port;
> +			audit_net.v4info.daddr = addr4->sin_addr.s_addr;
> +		} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
> +			audit_net.sport = port;
> +			audit_net.v4info.saddr = addr4->sin_addr.s_addr;
> +		} else {
> +			WARN_ON_ONCE(1);
> +		}
>  		break;
> +	}
> 
>  #if IS_ENABLED(CONFIG_IPV6)
> -	case AF_INET6:
> +	case AF_INET6: {
> +		const struct sockaddr_in6 *addr6;
> +
>  		if (addrlen < SIN6_LEN_RFC2133)
>  			return -EINVAL;
> -		port = ((struct sockaddr_in6 *)address)->sin6_port;
> +
> +		addr6 = (struct sockaddr_in6 *)address;
> +		port = addr6->sin6_port;
> +		audit_net.v6info.saddr = addr6->sin6_addr;

You set this for all access_request, but not for IPv4, is this done on 
purpose?

> +
> +		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
> +			audit_net.dport = port;
> +			audit_net.v6info.daddr = addr6->sin6_addr;
> +		} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
> +			audit_net.sport = port;
> +			audit_net.v6info.saddr = addr6->sin6_addr;
> +		} else {
> +			WARN_ON_ONCE(1);
> +		}
>  		break;
> +	}
>  #endif /* IS_ENABLED(CONFIG_IPV6) */
> 
>  	default:
> @@ -152,6 +189,13 @@ static int current_check_access_socket(struct socket
> *const sock, ARRAY_SIZE(layer_masks)))
>  		return 0;
> 
> +	audit_net.family = address->sa_family;
> +	request.audit.type = LSM_AUDIT_DATA_NET;
> +	request.audit.u.net = &audit_net;
> +	request.access = access_request;
> +	request.layer_masks = &layer_masks;
> +	request.layer_masks_size = ARRAY_SIZE(layer_masks);
> +	landlock_log_denial(dom, &request);
>  	return -EACCES;
>  }

Re: [RFC PATCH v2 04/14] landlock: Add unique ID generator

[Mickaël Salaün]

On Fri, Oct 25, 2024 at 05:18:06PM +0200, Francis Laniel wrote:
> Hi!
> 
> Le mardi 22 octobre 2024, 18:09:59 CEST Mickaël Salaün a écrit :
> > Landlock IDs can be generated to uniquely identify Landlock objects.
> > For now, only Landlock domains get an ID at creation time.
> > 
> > These IDs have important properties:
> > * They are unique during the lifetime of the running system thanks to
> >   the 64-bit values: at worse, 2^60 - 2*2^32 useful IDs.
> > * They are always greater than 2^32 and must then be stored in 64-bit
> >   integer types.
> > * The initial ID (at boot time) is randomly picked between 2^32 and
> >   2^33, which limits collisions in logs between different boots.
> > * IDs are sequential, which enables users to order them.
> > * IDs may not be consecutive but increase with a random 2^4 step, which
> >   limits side channels.
> > 
> > Such IDs can be exposed to unprivileged processes, even if it is not the
> > case with this audit patch series.  The domain IDs will be useful for
> > user space to identify sandboxes and get their properties.
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: Paul Moore <paul@paul-moore.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > Link: https://lore.kernel.org/r/20241022161009.982584-5-mic@digikod.net
> > ---

> > diff --git a/security/landlock/id.h b/security/landlock/id.h
> > new file mode 100644
> > index 000000000000..689ba7607472
> > --- /dev/null
> > +++ b/security/landlock/id.h
> > @@ -0,0 +1,25 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Landlock LSM - Unique identification number generator
> > + *
> > + * Copyright © 2024 Microsoft Corporation
> > + */
> > +
> > +#ifndef _SECURITY_LANDLOCK_ID_H
> > +#define _SECURITY_LANDLOCK_ID_H
> > +
> > +#ifdef CONFIG_AUDIT
> > +
> > +void __init landlock_init_id(void);
> > +
> > +u64 landlock_get_id(size_t number_of_ids);
> > +
> > +#else /* CONFIG_AUDIT */
> > +
> > +static inline void __init landlock_init_id(void)
> > +{
> > +}
> 
> Should the function have the same signature than when CONFIG_AUDIT is set?

The API is the same, only the static inline changes, which is what we
need to do in a header file.

Re: [RFC PATCH v2 05/14] landlock: Move access types

[Mickaël Salaün]

On Fri, Oct 25, 2024 at 05:20:39PM +0200, Francis Laniel wrote:
> Le mardi 22 octobre 2024, 18:10:00 CEST Mickaël Salaün a écrit :
> > Move ACCESS_FS_OPTIONAL, access_mask_t, struct access_mask, and struct
> > access_masks_all to a dedicated access.h file.
> > 
> > This file will be extended with a following commit, and it will help to
> > avoid dependency loops.
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > Link: https://lore.kernel.org/r/20241022161009.982584-6-mic@digikod.net
> > ---
> > 
> > Changes since v1:
> > * New patch
> > ---
> >  security/landlock/access.h  | 53 +++++++++++++++++++++++++++++++++++++
> >  security/landlock/fs.c      |  1 +
> >  security/landlock/fs.h      |  1 +
> >  security/landlock/ruleset.h | 31 +---------------------
> >  4 files changed, 56 insertions(+), 30 deletions(-)
> >  create mode 100644 security/landlock/access.h
> > 
> > diff --git a/security/landlock/access.h b/security/landlock/access.h
> > new file mode 100644
> > index 000000000000..2659fd9b4aaf
> > --- /dev/null
> > +++ b/security/landlock/access.h
> > @@ -0,0 +1,53 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Landlock LSM - Access types and helpers
> > + *
> > + * Copyright © 2016-2020 Mickaël Salaün <mic@digikod.net>
> > + * Copyright © 2018-2020 ANSSI
> > + * Copyright © 2024 Microsoft Corporation
> > + */
> > +
> > +#ifndef _SECURITY_LANDLOCK_ACCESS_H
> > +#define _SECURITY_LANDLOCK_ACCESS_H
> > +
> > +#include <uapi/linux/landlock.h>
> > +
> > +#include "limits.h"
> > +
> > +/* clang-format off */
> > +#define ACCESS_FS_OPTIONAL ( \
> > +	LANDLOCK_ACCESS_FS_TRUNCATE | \
> > +	LANDLOCK_ACCESS_FS_IOCTL_DEV)
> 
> Nit: The patch message indicates this is moved from somewhere but I cannot find 
> deletion for it.

Correct, I'll move this define to the following patch introducing
deny_masks_t.

Re: [RFC PATCH v2 10/14] landlock: Log file-related denials

[Mickaël Salaün]

On Fri, Oct 25, 2024 at 05:23:48PM +0200, Francis Laniel wrote:
> Le mardi 22 octobre 2024, 18:10:05 CEST Mickaël Salaün a écrit :
> > Add audit support for path_mkdir, path_mknod, path_symlink, path_unlink,
> > path_rmdir, path_truncate, path_link, path_rename, and file_open hooks.
> > 
> > Audit record sample for a link action:
> > 
> >   DENY:     domain=4533720568 blockers=fs_refer path="/usr/bin" dev="vda2"
> > ino=351 DOM_INFO: domain=4533720568 parent=0 pid=325 uid=0
> > exe="/root/sandboxer" comm="sandboxer" DENY:     domain=4533720568
> > blockers=fs_make_reg,fs_refer path="/usr/local" dev="vda2" ino=365 SYSCALL:
> >  arch=c000003e syscall=265 success=no exit=-13 ...
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > Link: https://lore.kernel.org/r/20241022161009.982584-11-mic@digikod.net
> > ---
> > 
> > Changes since v2:
> > * Revamp logging and support the path_link and path_rename hooks.
> > * Add KUnit tests.
> > 
> > Changes since v1:
> > * Move audit code to the ptrace patch.
> > ---
> >  security/landlock/audit.c | 173 ++++++++++++++++++++++++++++++++++++--
> >  security/landlock/audit.h |   9 ++
> >  security/landlock/fs.c    |  64 +++++++++++---
> >  3 files changed, 229 insertions(+), 17 deletions(-)
> > 
> > diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> > index 4cd9407459d2..9c8b6c246884 100644
> > --- a/security/landlock/audit.c
> > +++ b/security/landlock/audit.c
> > @@ -7,23 +7,55 @@
> > 
> >  #include <kunit/test.h>
> >  #include <linux/audit.h>
> > +#include <linux/bitops.h>
> >  #include <linux/lsm_audit.h>
> >  #include <linux/pid.h>
> >  #include <linux/uidgid.h>
> > +#include <uapi/linux/landlock.h>
> > 
> >  #include "audit.h"
> > +#include "common.h"
> >  #include "cred.h"
> >  #include "domain.h"
> >  #include "ruleset.h"
> > 
> > -static const char *get_blocker(const enum landlock_request_type type)
> > +static const char *const fs_access_strings[] = {
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = "fs_execute",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = "fs_write_file",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = "fs_read_file",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = "fs_read_dir",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = "fs_remove_dir",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_FILE)] = "fs_remove_file",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_CHAR)] = "fs_make_char",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_DIR)] = "fs_make_dir",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_REG)] = "fs_make_reg",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_SOCK)] = "fs_make_sock",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_FIFO)] = "fs_make_fifo",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_BLOCK)] = "fs_make_block",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_SYM)] = "fs_make_sym",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs_refer",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs_truncate",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs_ioctl_dev",
> > +};
> > +static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> > +
> > +static __attribute_const__ const char *
> > +get_blocker(const enum landlock_request_type type,
> > +	    const unsigned long access_bit)
> >  {
> >  	switch (type) {
> >  	case LANDLOCK_REQUEST_PTRACE:
> > +		WARN_ON_ONCE(access_bit != -1);
> >  		return "ptrace";
> > 
> >  	case LANDLOCK_REQUEST_FS_CHANGE_LAYOUT:
> > +		WARN_ON_ONCE(access_bit != -1);
> >  		return "fs_change_layout";
> > +
> > +	case LANDLOCK_REQUEST_FS_ACCESS:
> > +		if (WARN_ON_ONCE(access_bit >= ARRAY_SIZE(fs_access_strings)))
> > +			return "unknown";
> > +		return fs_access_strings[access_bit];
> >  	}
> > 
> >  	WARN_ON_ONCE(1);
> > @@ -31,9 +63,20 @@ static const char *get_blocker(const enum
> > landlock_request_type type) }
> > 
> >  static void log_blockers(struct audit_buffer *const ab,
> > -			 const enum landlock_request_type type)
> > +			 const enum landlock_request_type type,
> > +			 const access_mask_t access)
> >  {
> > -	audit_log_format(ab, "%s", get_blocker(type));
> > +	const unsigned long access_mask = access;
> > +	unsigned long access_bit;
> > +	size_t i = 0;
> > +
> > +	for_each_set_bit(access_bit, &access_mask, BITS_PER_TYPE(access)) {
> > +		audit_log_format(ab, "%s%s", (i == 0) ? "" : ",",
> > +				 get_blocker(type, access_bit));
> > +		i++;
> > +	}
> > +	if (i == 0)
> > +		audit_log_format(ab, "%s", get_blocker(type, -1));
> >  }
> > 
> >  static void log_node(struct landlock_hierarchy *const node)
> > @@ -121,9 +164,110 @@ static void test_get_hierarchy(struct kunit *const
> > test)
> > 
> >  #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> > 
> > +static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> > +			       access_mask_t *const access_request,
> > +			       const layer_mask_t (*const layer_masks)[],
> > +			       const size_t layer_masks_size)
> > +{
> > +	const unsigned long access_req = *access_request;
> 
> Nit: should access_request being checked for not being NULL?

This is not necessary because this helper is private and the pointer is
always refering to the stack.

> 
> > +	unsigned long access_bit;
> > +	access_mask_t missing = 0;
> > +	long youngest_layer = -1;
> > +
> > +	for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> > +		const access_mask_t mask = (*layer_masks)[access_bit];
> > +		long layer;
> > +
> > +		if (!mask)
> > +			continue;
> > +
> > +		/* __fls(1) == 0 */
> > +		layer = __fls(mask);
> > +		if (layer > youngest_layer) {
> > +			youngest_layer = layer;
> > +			missing = BIT(access_bit);
> > +		} else if (layer == youngest_layer) {
> > +			missing |= BIT(access_bit);
> > +		}
> > +	}
> > +
> > +	*access_request = missing;
> > +	if (youngest_layer == -1)
> > +		return domain->num_layers - 1;
> > +
> > +	return youngest_layer;
> > +}
> > +
> > +#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> > +
> > +static void test_get_denied_layer(struct kunit *const test)
> > +{
> > +	const struct landlock_ruleset dom = {
> > +		.num_layers = 5,
> > +	};
> > +	const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> > +		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT(0),
> > +		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT(1),
> > +		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = BIT(1) | BIT(0),
> > +		[BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = BIT(2),
> > +	};
> > +	access_mask_t access;
> > +
> > +	access = LANDLOCK_ACCESS_FS_EXECUTE;
> > +	KUNIT_EXPECT_EQ(test, 0,
> > +			get_denied_layer(&dom, &access, &layer_masks,
> > +					 sizeof(layer_masks)));
> > +	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_EXECUTE);
> > +
> > +	access = LANDLOCK_ACCESS_FS_READ_FILE;
> > +	KUNIT_EXPECT_EQ(test, 1,
> > +			get_denied_layer(&dom, &access, &layer_masks,
> > +					 sizeof(layer_masks)));
> > +	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_FILE);
> > +
> > +	access = LANDLOCK_ACCESS_FS_READ_DIR;
> > +	KUNIT_EXPECT_EQ(test, 1,
> > +			get_denied_layer(&dom, &access, &layer_masks,
> > +					 sizeof(layer_masks)));
> > +	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
> > +
> > +	access = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR;
> > +	KUNIT_EXPECT_EQ(test, 1,
> > +			get_denied_layer(&dom, &access, &layer_masks,
> > +					 sizeof(layer_masks)));
> > +	KUNIT_EXPECT_EQ(test, access,
> > +			LANDLOCK_ACCESS_FS_READ_FILE |
> > +				LANDLOCK_ACCESS_FS_READ_DIR);
> > +
> > +	access = LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_READ_DIR;
> > +	KUNIT_EXPECT_EQ(test, 1,
> > +			get_denied_layer(&dom, &access, &layer_masks,
> > +					 sizeof(layer_masks)));
> > +	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
> > +
> > +	access = LANDLOCK_ACCESS_FS_WRITE_FILE;
> > +	KUNIT_EXPECT_EQ(test, 4,
> > +			get_denied_layer(&dom, &access, &layer_masks,
> > +					 sizeof(layer_masks)));
> > +	KUNIT_EXPECT_EQ(test, access, 0);
> > +}
> > +
> > +#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))
> > +	if (WARN_ON_ONCE(!(!!request->layer_plus_one ^ !!request->access)))
> > +		return false;
> > +
> > +	if (request->access) {
> > +		if (WARN_ON_ONCE(!request->layer_masks))
> > +			return false;
> > +	} else {
> > +		if (WARN_ON_ONCE(request->layer_masks))
> > +			return false;
> > +	}
> > +
> > +	if (WARN_ON_ONCE(!!request->layer_masks ^ !!request-
> >layer_masks_size))
> >  		return false;
> > 
> >  	return true;
> > @@ -140,6 +284,7 @@ void landlock_log_denial(const struct landlock_ruleset
> > *const domain, {
> >  	struct audit_buffer *ab;
> >  	struct landlock_hierarchy *youngest_denied;
> > +	access_mask_t missing;
> > 
> >  	if (WARN_ON_ONCE(!domain || !domain->hierarchy || !request))
> >  		return;
> > @@ -155,9 +300,24 @@ void landlock_log_denial(const struct landlock_ruleset
> > *const domain, if (!ab)
> >  		return;
> > 
> > -	youngest_denied = get_hierarchy(domain, request->layer_plus_one - 1);
> > +	missing = request->access;
> > +	if (missing) {
> > +		size_t youngest_layer;
> > +
> > +		/* Gets the nearest domain that denies the request. */
> > +		if (request->layer_masks) {
> > +			youngest_layer = get_denied_layer(
> > +				domain, &missing, request->layer_masks,
> > +				request->layer_masks_size);
> > +		}
> > +		youngest_denied = get_hierarchy(domain, youngest_layer);
> 
> If request->layer_masks is 0, it is possible to call get_hierarchy() with
> uninitialized youngest_layer, is this wanted?

Well spotted. This patch seems indeed buggy because I created several
patches touching the same function, but the final code (with all the
patches applied) always initializes youngest_denied.  The current calls
to landlock_log_denial() also always set request->layer_mask, but I'll
fix this patch to avoid confusion.

> 
> > +	} else {
> > +		youngest_denied =
> > +			get_hierarchy(domain, request->layer_plus_one - 1);
> > +	}
> > +
> >  	audit_log_format(ab, "domain=%llu blockers=", youngest_denied->id);
> > -	log_blockers(ab, request->type);
> > +	log_blockers(ab, request->type, missing);
> >  	audit_log_lsm_data(ab, &request->audit);
> >  	audit_log_end(ab);
> > 
> > @@ -204,6 +364,7 @@ void landlock_log_drop_domain(const struct
> > landlock_ruleset *const domain) static struct kunit_case test_cases[] = {
> >  	/* clang-format off */
> >  	KUNIT_CASE(test_get_hierarchy),
> > +	KUNIT_CASE(test_get_denied_layer),
> >  	{}
> >  	/* clang-format on */
> >  };
> > diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> > index 6f5ad04b83c2..25fc8333cddc 100644
> > --- a/security/landlock/audit.h
> > +++ b/security/landlock/audit.h
> > @@ -11,11 +11,13 @@
> >  #include <linux/audit.h>
> >  #include <linux/lsm_audit.h>
> > 
> > +#include "access.h"
> >  #include "ruleset.h"
> > 
> >  enum landlock_request_type {
> >  	LANDLOCK_REQUEST_PTRACE = 1,
> >  	LANDLOCK_REQUEST_FS_CHANGE_LAYOUT,
> > +	LANDLOCK_REQUEST_FS_ACCESS,
> >  };
> > 
> >  /*
> > @@ -33,6 +35,13 @@ struct landlock_request {
> >  	 * extra one is useful to detect uninitialized field.
> >  	 */
> >  	size_t layer_plus_one;
> > +
> > +	/* Required field for configurable access control. */
> > +	access_mask_t access;
> > +
> > +	/* Required fields for requests with layer masks. */
> > +	const layer_mask_t (*layer_masks)[];
> > +	size_t layer_masks_size;
> >  };
> > 
> >  #ifdef CONFIG_AUDIT
> > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > index a099167d2347..7f69bed9e095 100644
> > --- a/security/landlock/fs.c
> > +++ b/security/landlock/fs.c
> > @@ -730,6 +730,7 @@ static void test_is_eacces_with_write(struct kunit
> > *const test) *     those identified by @access_request_parent1).  This
> > matrix can *     initially refer to domain layer masks and, when the
> > accesses for the *     destination and source are the same, to requested
> > layer masks. + * @log_request_parent1: Audit request to fill if the related
> > access is denied. * @dentry_child1: Dentry to the initial child of the
> > parent1 path.  This *     pointer must be NULL for non-refer actions (i.e.
> > not link nor rename). * @access_request_parent2: Similar to
> > @access_request_parent1 but for a @@ -738,6 +739,7 @@ static void
> > test_is_eacces_with_write(struct kunit *const test) *     the source.  Must
> > be set to 0 when using a simple path request. * @layer_masks_parent2:
> > Similar to @layer_masks_parent1 but for a refer *     action.  This must be
> > NULL otherwise.
> > + * @log_request_parent2: Audit request to fill if the related access is
> > denied. * @dentry_child2: Dentry to the initial child of the parent2 path. 
> > This *     pointer is only set for RENAME_EXCHANGE actions and must be NULL
> > *     otherwise.
> > @@ -757,10 +759,12 @@ static bool is_access_to_paths_allowed(
> >  	const struct path *const path,
> >  	const access_mask_t access_request_parent1,
> >  	layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
> > -	const struct dentry *const dentry_child1,
> > +	struct landlock_request *const log_request_parent1,
> > +	struct dentry *const dentry_child1,
> >  	const access_mask_t access_request_parent2,
> >  	layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
> > -	const struct dentry *const dentry_child2)
> > +	struct landlock_request *const log_request_parent2,
> > +	struct dentry *const dentry_child2)
> >  {
> >  	bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check,
> >  	     child1_is_directory = true, child2_is_directory = true;
> > @@ -907,6 +911,24 @@ static bool is_access_to_paths_allowed(
> >  	}
> >  	path_put(&walker_path);
> > 
> > +	if (!allowed_parent1 && log_request_parent1) {
> > +		log_request_parent1->type = LANDLOCK_REQUEST_FS_ACCESS,
> > +		log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH,
> > +		log_request_parent1->audit.u.path = *path;
> > +		log_request_parent1->access = access_request_parent1;
> > +		log_request_parent1->layer_masks = layer_masks_parent1;
> > +		log_request_parent1->layer_masks_size =
> > +			ARRAY_SIZE(*layer_masks_parent1);
> > +	}
> > +	if (!allowed_parent2 && log_request_parent2) {
> > +		log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS,
> > +		log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH,
> > +		log_request_parent2->audit.u.path = *path;
> > +		log_request_parent2->access = access_request_parent2;
> > +		log_request_parent2->layer_masks = layer_masks_parent2;
> > +		log_request_parent2->layer_masks_size =
> > +			ARRAY_SIZE(*layer_masks_parent2);
> > +	}
> 
> You may want to add a function to set these fields in log_request.

There would only be two calls to this function and with at least four
arguments, so for simplicity, I don't think it's worth it.

Re: [RFC PATCH v2 12/14] landlock: Log TCP bind and connect denials

[Mickaël Salaün]

On Fri, Oct 25, 2024 at 05:25:45PM +0200, Francis Laniel wrote:
> Le mardi 22 octobre 2024, 18:10:07 CEST Mickaël Salaün a écrit :
> > Add audit support to socket_bind and socket_connect hooks.
> > 
> > Audit record sample:
> > 
> >   DENY:    domain=4533720601 blockers=net_connect_tcp daddr=127.0.0.1
> > dest=80 SYSCALL: arch=c000003e syscall=42 success=no exit=-13 ...
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> > Cc: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > Link: https://lore.kernel.org/r/20241022161009.982584-13-mic@digikod.net
> > ---
> >  security/landlock/audit.c | 11 +++++++++
> >  security/landlock/audit.h |  1 +
> >  security/landlock/net.c   | 52 ++++++++++++++++++++++++++++++++++++---
> >  3 files changed, 60 insertions(+), 4 deletions(-)
> > 
> > diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> > index 898c95ebe847..c31a4a8719ee 100644
> > --- a/security/landlock/audit.c
> > +++ b/security/landlock/audit.c
> > @@ -41,6 +41,12 @@ static const char *const fs_access_strings[] = {
> >  };
> >  static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> > 
> > +static const char *const net_access_strings[] = {
> > +	[BIT_INDEX(LANDLOCK_ACCESS_NET_BIND_TCP)] = "net_bind_tcp",
> > +	[BIT_INDEX(LANDLOCK_ACCESS_NET_CONNECT_TCP)] = "net_connect_tcp",
> > +};
> > +static_assert(ARRAY_SIZE(net_access_strings) == LANDLOCK_NUM_ACCESS_NET);
> > +
> >  static __attribute_const__ const char *
> >  get_blocker(const enum landlock_request_type type,
> >  	    const unsigned long access_bit)
> > @@ -58,6 +64,11 @@ get_blocker(const enum landlock_request_type type,
> >  		if (WARN_ON_ONCE(access_bit >= ARRAY_SIZE(fs_access_strings)))
> >  			return "unknown";
> >  		return fs_access_strings[access_bit];
> > +
> > +	case LANDLOCK_REQUEST_NET_ACCESS:
> > +		if (WARN_ON_ONCE(access_bit >= ARRAY_SIZE(net_access_strings)))
> > +			return "unknown";
> > +		return net_access_strings[access_bit];
> >  	}
> > 
> >  	WARN_ON_ONCE(1);
> > diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> > index 320394fd6b84..1075b0c8401f 100644
> > --- a/security/landlock/audit.h
> > +++ b/security/landlock/audit.h
> > @@ -18,6 +18,7 @@ enum landlock_request_type {
> >  	LANDLOCK_REQUEST_PTRACE = 1,
> >  	LANDLOCK_REQUEST_FS_CHANGE_LAYOUT,
> >  	LANDLOCK_REQUEST_FS_ACCESS,
> > +	LANDLOCK_REQUEST_NET_ACCESS,
> >  };
> > 
> >  /*
> > diff --git a/security/landlock/net.c b/security/landlock/net.c
> > index 27872d0f7e11..c21afd6e0b4d 100644
> > --- a/security/landlock/net.c
> > +++ b/security/landlock/net.c
> > @@ -7,10 +7,12 @@
> >   */
> > 
> >  #include <linux/in.h>
> > +#include <linux/lsm_audit.h>
> >  #include <linux/net.h>
> >  #include <linux/socket.h>
> >  #include <net/ipv6.h>
> > 
> > +#include "audit.h"
> >  #include "common.h"
> >  #include "cred.h"
> >  #include "limits.h"
> > @@ -56,6 +58,10 @@ static int current_check_access_socket(struct socket
> > *const sock, };
> >  	const struct landlock_ruleset *const dom =
> >  		landlock_match_ruleset(landlock_get_current_domain(), any_net);
> > +	struct lsm_network_audit audit_net = {};
> > +	struct landlock_request request = {
> > +		.type = LANDLOCK_REQUEST_NET_ACCESS,
> > +	};
> > 
> >  	if (!dom)
> >  		return 0;
> > @@ -72,18 +78,49 @@ static int current_check_access_socket(struct socket
> > *const sock,
> > 
> >  	switch (address->sa_family) {
> >  	case AF_UNSPEC:
> > -	case AF_INET:
> > +	case AF_INET: {
> > +		const struct sockaddr_in *addr4;
> > +
> >  		if (addrlen < sizeof(struct sockaddr_in))
> >  			return -EINVAL;
> > -		port = ((struct sockaddr_in *)address)->sin_port;
> > +
> > +		addr4 = (struct sockaddr_in *)address;
> > +		port = addr4->sin_port;
> > +
> > +		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
> > +			audit_net.dport = port;
> > +			audit_net.v4info.daddr = addr4->sin_addr.s_addr;
> > +		} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
> > +			audit_net.sport = port;
> > +			audit_net.v4info.saddr = addr4->sin_addr.s_addr;
> > +		} else {
> > +			WARN_ON_ONCE(1);
> > +		}
> >  		break;
> > +	}
> > 
> >  #if IS_ENABLED(CONFIG_IPV6)
> > -	case AF_INET6:
> > +	case AF_INET6: {
> > +		const struct sockaddr_in6 *addr6;
> > +
> >  		if (addrlen < SIN6_LEN_RFC2133)
> >  			return -EINVAL;
> > -		port = ((struct sockaddr_in6 *)address)->sin6_port;
> > +
> > +		addr6 = (struct sockaddr_in6 *)address;
> > +		port = addr6->sin6_port;
> > +		audit_net.v6info.saddr = addr6->sin6_addr;
> 
> You set this for all access_request, but not for IPv4, is this done on 
> purpose?

Indeed, this is a bug, I'll remove this line.  Thanks!

> 
> > +
> > +		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
> > +			audit_net.dport = port;
> > +			audit_net.v6info.daddr = addr6->sin6_addr;
> > +		} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
> > +			audit_net.sport = port;
> > +			audit_net.v6info.saddr = addr6->sin6_addr;
> > +		} else {
> > +			WARN_ON_ONCE(1);
> > +		}
> >  		break;
> > +	}
> >  #endif /* IS_ENABLED(CONFIG_IPV6) */
> > 
> >  	default: