[RFC PATCH v2 04/14] landlock: Add unique ID generator
Francis Laniel
flaniel at linux.microsoft.com
Fri Oct 25 15:18:06 UTC 2024
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 at google.com>
> Cc: Paul Moore <paul at paul-moore.com>
> Signed-off-by: Mickaël Salaün <mic at digikod.net>
> Link: https://lore.kernel.org/r/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.
More information about the Linux-security-module-archive
mailing list