From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hillf Danton Subject: Re: [RFC PATCH v14 01/10] landlock: Add object and rule management Date: Thu, 27 Feb 2020 12:20:02 +0800 Message-ID: <20200227042002.3032-1-hdanton@sina.com> References: <20200224160215.4136-1-mic@digikod.net> Mime-Version: 1.0 Content-Type: text/plain; charset=y Content-Transfer-Encoding: 8bit Return-path: List-Post: List-Help: List-Unsubscribe: List-Subscribe: In-Reply-To: <20200224160215.4136-1-mic@digikod.net> To: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Cc: linux-kernel@vger.kernel.org, Al Viro , Andy Lutomirski , Arnd Bergmann , Casey Schaufler , Greg Kroah-Hartman , James Morris , Jann Horn , Jonathan Corbet , Kees Cook , Michael Kerrisk , =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= , "Serge E. Hallyn" , Shuah Khan , Vincent Dagonneau , kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-security-module@vger.kernel.org, x86@kernel.org List-Id: linux-arch.vger.kernel.org On Mon, 24 Feb 2020 17:02:06 +0100 Mickaël Salaün > A Landlock object enables to identify a kernel object (e.g. an inode). > A Landlock rule is a set of access rights allowed on an object. Rules > are grouped in rulesets that may be tied to a set of processes (i.e. > subjects) to enforce a scoped access-control (i.e. a domain). > > Because Landlock's goal is to empower any process (especially > unprivileged ones) to sandbox themselves, we can't rely on a system-wide > object identification such as file extended attributes. Indeed, we need > innocuous, composable and modular access-controls. > > The main challenge with this constraints is to identify kernel objects > while this identification is useful (i.e. when a security policy makes > use of this object). But this identification data should be freed once > no policy is using it. This ephemeral tagging should not and may not be > written in the filesystem. We then need to manage the lifetime of a > rule according to the lifetime of its object. To avoid a global lock, > this implementation make use of RCU and counters to safely reference > objects. > > A following commit uses this generic object management for inodes. > > Signed-off-by: Mickaël Salaün > Cc: Andy Lutomirski > Cc: James Morris > Cc: Kees Cook > Cc: Serge E. Hallyn > --- > > Changes since v13: > * New dedicated implementation, removing the need for eBPF. > > Previous version: > https://lore.kernel.org/lkml/20190721213116.23476-6-mic@digikod.net/ > --- > MAINTAINERS | 10 ++ > security/Kconfig | 1 + > security/Makefile | 2 + > security/landlock/Kconfig | 15 ++ > security/landlock/Makefile | 3 + > security/landlock/object.c | 339 +++++++++++++++++++++++++++++++++++++ > security/landlock/object.h | 134 +++++++++++++++ > 7 files changed, 504 insertions(+) > create mode 100644 security/landlock/Kconfig > create mode 100644 security/landlock/Makefile > create mode 100644 security/landlock/object.c > create mode 100644 security/landlock/object.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index fcd79fc38928..206f85768cd9 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9360,6 +9360,16 @@ F: net/core/skmsg.c > F: net/core/sock_map.c > F: net/ipv4/tcp_bpf.c > > +LANDLOCK SECURITY MODULE > +M: Mickaël Salaün > +L: linux-security-module@vger.kernel.org > +W: https://landlock.io > +T: git https://github.com/landlock-lsm/linux.git > +S: Supported > +F: security/landlock/ > +K: landlock > +K: LANDLOCK > + > LANTIQ / INTEL Ethernet drivers > M: Hauke Mehrtens > L: netdev@vger.kernel.org > diff --git a/security/Kconfig b/security/Kconfig > index 2a1a2d396228..9d9981394fb0 100644 > --- a/security/Kconfig > +++ b/security/Kconfig > @@ -238,6 +238,7 @@ source "security/loadpin/Kconfig" > source "security/yama/Kconfig" > source "security/safesetid/Kconfig" > source "security/lockdown/Kconfig" > +source "security/landlock/Kconfig" > > source "security/integrity/Kconfig" > > diff --git a/security/Makefile b/security/Makefile > index 746438499029..2472ef96d40a 100644 > --- a/security/Makefile > +++ b/security/Makefile > @@ -12,6 +12,7 @@ subdir-$(CONFIG_SECURITY_YAMA) += yama > subdir-$(CONFIG_SECURITY_LOADPIN) += loadpin > subdir-$(CONFIG_SECURITY_SAFESETID) += safesetid > subdir-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown > +subdir-$(CONFIG_SECURITY_LANDLOCK) += landlock > > # always enable default capabilities > obj-y += commoncap.o > @@ -29,6 +30,7 @@ obj-$(CONFIG_SECURITY_YAMA) += yama/ > obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/ > obj-$(CONFIG_SECURITY_SAFESETID) += safesetid/ > obj-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown/ > +obj-$(CONFIG_SECURITY_LANDLOCK) += landlock/ > obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o > > # Object integrity file lists > diff --git a/security/landlock/Kconfig b/security/landlock/Kconfig > new file mode 100644 > index 000000000000..4a321d5b3f67 > --- /dev/null > +++ b/security/landlock/Kconfig > @@ -0,0 +1,15 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +config SECURITY_LANDLOCK > + bool "Landlock support" > + depends on SECURITY > + default n > + help > + This selects Landlock, a safe sandboxing mechanism. It enables to > + restrict processes on the fly (i.e. enforce an access control policy), > + which can complement seccomp-bpf. The security policy is a set of access > + rights tied to an object, which could be a file, a socket or a process. > + > + See Documentation/security/landlock/ for further information. > + > + If you are unsure how to answer this question, answer N. > diff --git a/security/landlock/Makefile b/security/landlock/Makefile > new file mode 100644 > index 000000000000..cb6deefbf4c0 > --- /dev/null > +++ b/security/landlock/Makefile > @@ -0,0 +1,3 @@ > +obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o > + > +landlock-y := object.o > diff --git a/security/landlock/object.c b/security/landlock/object.c > new file mode 100644 > index 000000000000..38fbbb108120 > --- /dev/null > +++ b/security/landlock/object.c > @@ -0,0 +1,339 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Landlock LSM - Object and rule management > + * > + * Copyright © 2016-2020 Mickaël Salaün > + * Copyright © 2018-2020 ANSSI > + * > + * Principles and constraints of the object and rule management: > + * - Do not leak memory. > + * - Try as much as possible to free a memory allocation as soon as it is > + * unused. > + * - Do not use global lock. > + * - Do not charge processes other than the one requesting a Landlock > + * operation. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "object.h" > + > +struct landlock_object *landlock_create_object( > + const enum landlock_object_type type, void *underlying_object) > +{ > + struct landlock_object *object; > + > + if (WARN_ON_ONCE(!underlying_object)) > + return NULL; > + object = kzalloc(sizeof(*object), GFP_KERNEL); > + if (!object) > + return NULL; > + refcount_set(&object->usage, 1); > + refcount_set(&object->cleaners, 1); > + spin_lock_init(&object->lock); > + INIT_LIST_HEAD(&object->rules); > + object->type = type; > + WRITE_ONCE(object->underlying_object, underlying_object); > + return object; > +} > + > +struct landlock_object *landlock_get_object(struct landlock_object *object) > + __acquires(object->usage) > +{ > + __acquire(object->usage); > + /* > + * If @object->usage equal 0, then it will be ignored by writers, and > + * underlying_object->object may be replaced, but this is not an issue > + * for release_object(). > + */ > + if (object && refcount_inc_not_zero(&object->usage)) { > + /* > + * It should not be possible to get a reference to an object if > + * its underlying object is being terminated (e.g. with > + * landlock_release_object()), because an object is only > + * modifiable through such underlying object. This is not the > + * case with landlock_get_object_cleaner(). > + */ > + WARN_ON_ONCE(!READ_ONCE(object->underlying_object)); > + return object; > + } > + return NULL; > +} > + > +static struct landlock_object *get_object_cleaner( > + struct landlock_object *object) > + __acquires(object->cleaners) > +{ > + __acquire(object->cleaners); > + if (object && refcount_inc_not_zero(&object->cleaners)) > + return object; > + return NULL; > +} > + > +/* > + * There is two cases when an object should be free and the reference to the > + * underlying object should be put: > + * - when the last rule tied to this object is removed, which is handled by > + * landlock_put_rule() and then release_object(); > + * - when the object is being terminated (e.g. no more reference to an inode), > + * which is handled by landlock_put_object(). > + */ > +static void put_object_free(struct landlock_object *object) > + __releases(object->cleaners) > +{ > + __release(object->cleaners); > + if (!refcount_dec_and_test(&object->cleaners)) > + return; > + WARN_ON_ONCE(refcount_read(&object->usage)); > + /* > + * Ensures a safe use of @object in the RCU block from > + * landlock_put_rule(). > + */ > + kfree_rcu(object, rcu_free); > +} > + > +/* > + * Destroys a newly created and useless object. > + */ > +void landlock_drop_object(struct landlock_object *object) > +{ > + if (WARN_ON_ONCE(!refcount_dec_and_test(&object->usage))) > + return; > + __acquire(object->cleaners); > + put_object_free(object); > +} > + > +/* > + * Puts the underlying object (e.g. inode) if it is the first request to > + * release @object, without calling landlock_put_object(). > + * > + * Return true if this call effectively marks @object as released, false > + * otherwise. > + */ > +static bool release_object(struct landlock_object *object) > + __releases(&object->lock) > +{ > + void *underlying_object; > + > + lockdep_assert_held(&object->lock); > + > + underlying_object = xchg(&object->underlying_object, NULL); A one-line comment looks needed for xchg. > + spin_unlock(&object->lock); > + might_sleep(); Have trouble working out what might_sleep is put for. > + if (!underlying_object) > + return false; > + > + switch (object->type) { > + case LANDLOCK_OBJECT_INODE: > + break; > + default: > + WARN_ON_ONCE(1); > + } > + return true; > +} > + > +static void put_object_cleaner(struct landlock_object *object) > + __releases(object->cleaners) > +{ > + /* Let's try an early lockless check. */ > + if (list_empty(&object->rules) && > + READ_ONCE(object->underlying_object)) { > + /* > + * Puts @object if there is no rule tied to it and the > + * remaining user is the underlying object. This check is > + * atomic because @object->rules and @object->underlying_object > + * are protected by @object->lock. > + */ > + spin_lock(&object->lock); > + if (list_empty(&object->rules) && > + READ_ONCE(object->underlying_object) && > + refcount_dec_if_one(&object->usage)) { > + /* > + * Releases @object, in place of > + * landlock_release_object(). > + * > + * @object is already empty, implying that all its > + * previous rules are already disabled. > + * > + * Unbalance the @object->cleaners counter to reflect > + * the underlying object release. > + */ > + if (!WARN_ON_ONCE(!release_object(object))) { Two ! hurt more than help. > + __acquire(object->cleaners); > + put_object_free(object); Why put object more than once? > + } > + } else { > + spin_unlock(&object->lock); > + } > + } > + put_object_free(object); > +} > +