From: Mickaël Salaün <redacted>
Process's credentials point to a Landlock domain, which is underneath
implemented with a ruleset. In the following commits, this domain is
used to check and enforce the ptrace and filesystem security policies.
A domain is inherited from a parent to its child the same way a thread
inherits a seccomp policy.
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
Reviewed-by: Jann Horn <jannh@google.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-4-mic@digikod.net
---
Changes since v28:
* Add Acked-by Serge Hallyn.
Changes since v25:
* Rename function to landlock_add_cred_hooks().
Changes since v23:
* Add an early check for the current domain in hook_cred_free() to avoid
superfluous call.
* Cosmetic cleanup to make the code more readable.
Changes since v22:
* Add Reviewed-by Jann Horn.
Changes since v21:
* Fix copyright dates.
Changes since v17:
* Constify returned domain pointers from landlock_get_current_domain()
and landlock_get_task_domain() helpers.
Changes since v15:
* Optimize landlocked() for current thread.
* Display the greeting message when everything is initialized.
Changes since v14:
* Uses pr_fmt from common.h .
* Constify variables.
* Remove useless NULL initialization.
Changes since v13:
* totally get ride of the seccomp dependency
* only keep credential management and LSM setup.
Previous changes:
https://lore.kernel.org/lkml/20191104172146.30797-4-mic@digikod.net/
---
security/Kconfig | 10 +++----
security/landlock/Makefile | 3 +-
security/landlock/common.h | 20 +++++++++++++
security/landlock/cred.c | 46 ++++++++++++++++++++++++++++++
security/landlock/cred.h | 58 ++++++++++++++++++++++++++++++++++++++
security/landlock/setup.c | 31 ++++++++++++++++++++
security/landlock/setup.h | 16 +++++++++++
7 files changed, 178 insertions(+), 6 deletions(-)
create mode 100644 security/landlock/common.h
create mode 100644 security/landlock/cred.c
create mode 100644 security/landlock/cred.h
create mode 100644 security/landlock/setup.c
create mode 100644 security/landlock/setup.h
@@ -278,11 +278,11 @@ endchoiceconfigLSMstring"Ordered list of enabled LSMs"-default"lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf"ifDEFAULT_SECURITY_SMACK-default"lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf"ifDEFAULT_SECURITY_APPARMOR-default"lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf"ifDEFAULT_SECURITY_TOMOYO-default"lockdown,yama,loadpin,safesetid,integrity,bpf"ifDEFAULT_SECURITY_DAC-default"lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"+default"landlock,lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf"ifDEFAULT_SECURITY_SMACK+default"landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf"ifDEFAULT_SECURITY_APPARMOR+default"landlock,lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf"ifDEFAULT_SECURITY_TOMOYO+default"landlock,lockdown,yama,loadpin,safesetid,integrity,bpf"ifDEFAULT_SECURITY_DAC+default"landlock,lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"helpAcomma-separatedlistofLSMs,ininitializationorder.AnyLSMsleftoffthislistwillbeignored.Thiscanbe
From: Mickaël Salaün <redacted>
A Landlock ruleset is mainly a red-black tree with Landlock rules as
nodes. This enables quick update and lookup to match a requested
access, e.g. to a file. A ruleset is usable through a dedicated file
descriptor (cf. following commit implementing syscalls) which enables a
process to create and populate a ruleset with new rules.
A domain is a ruleset tied to a set of processes. This group of rules
defines the security policy enforced on these processes and their future
children. A domain can transition to a new domain which is the
intersection of all its constraints and those of a ruleset provided by
the current process. This modification only impact the current process.
This means that a process can only gain more constraints (i.e. lose
accesses) over time.
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-3-mic@digikod.net
---
Changes since v28:
* Add Acked-by Serge Hallyn.
* Clean up comment.
Changes since v27:
* Fix domains with layers of non-overlapping access rights.
* Add stricter limit checks (same semantic).
* Change the grow direction of a rule layer stack to make it the same as
the new ruleset fs_access_masks stack (cosmetic change).
* Cosmetic fix for a comment block.
Changes since v26:
* Fix spelling.
Changes since v25:
* Add build-time checks for the num_layers and num_rules variables
according to LANDLOCK_MAX_NUM_LAYERS and LANDLOCK_MAX_NUM_RULES, and
move these limits to a dedicated file.
* Cosmetic variable renames.
Changes since v24:
* Update struct landlock_rule with a layer stack. This reverts "Always
intersect access rights" from v24 and also adds the ability to tie
access rights with their policy layer. As noted by Jann Horn, always
intersecting access rights made some use cases uselessly more
difficult to handle in user space. Thanks to this new stack, we still
have a deterministic policy behavior whatever their level in the stack
of policies, while using a "union" of accesses when building a
ruleset. The implementation use a FAM to keep the access checks quick
and memory efficient (4 bytes per layer per inode). Update
insert_rule() accordingly.
Changes since v23:
* Always intersect access rights. Following the filesystem change
logic, make ruleset updates more consistent by always intersecting
access rights (boolean AND) instead of combining them (boolean OR) for
the same layer. This defensive approach could also help avoid user
space to inadvertently allow multiple access rights for the same
object (e.g. write and execute access on a path hierarchy) instead of
dealing with such inconsistency. This can happen when there is no
deduplication of objects (e.g. paths and underlying inodes) whereas
they get different access rights with landlock_add_rule(2).
* Add extra checks to make sure that:
- there is always an (allocated) object in each used rules;
- when updating a ruleset with a new rule (i.e. not merging two
rulesets), the ruleset doesn't contain multiple layers.
* Hide merge parameter from the public landlock_insert_rule() API. This
helps avoid misuse of this function.
* Replace a remaining hardcoded 1 with SINGLE_DEPTH_NESTING.
Changes since v22:
* Explicitely use RB_ROOT and SINGLE_DEPTH_NESTING (suggested by Jann
Horn).
* Improve comments and fix spelling (suggested by Jann Horn).
Changes since v21:
* Add and clean up comments.
Changes since v18:
* Account rulesets to kmemcg.
* Remove struct holes.
* Cosmetic changes.
Changes since v17:
* Move include/uapi/linux/landlock.h and _LANDLOCK_ACCESS_FS_* to a
following patch.
Changes since v16:
* Allow enforcement of empty ruleset, which enables deny-all policies.
Changes since v15:
* Replace layer_levels and layer_depth with a bitfield of layers, cf.
filesystem commit.
* Rename the LANDLOCK_ACCESS_FS_{UNLINK,RMDIR} with
LANDLOCK_ACCESS_FS_REMOVE_{FILE,DIR} because it makes sense to use
them for the action of renaming a file or a directory, which may lead
to the removal of the source file or directory. Removes the
LANDLOCK_ACCESS_FS_{LINK_TO,RENAME_FROM,RENAME_TO} which are now
replaced with LANDLOCK_ACCESS_FS_REMOVE_{FILE,DIR} and
LANDLOCK_ACCESS_FS_MAKE_* .
* Update the documentation accordingly and highlight how the access
rights are taken into account.
* Change nb_rules from atomic_t to u32 because it is not use anymore by
show_fdinfo().
* Add safeguard for level variables types.
* Check max number of rules.
* Replace struct landlock_access (self and beneath bitfields) with one
bitfield.
* Remove useless variable.
* Add comments.
Changes since v14:
* Simplify the object, rule and ruleset management at the expense of a
less aggressive memory freeing (contributed by Jann Horn, with
additional modifications):
- Make a domain immutable (remove the opportunistic cleaning).
- Remove RCU pointers.
- Merge struct landlock_ref and struct landlock_ruleset_elem into
landlock_rule: get ride of rule's RCU.
- Adjust union.
- Remove the landlock_insert_rule() check about a new object with the
same address as a previously disabled one, because it is not
possible to disable a rule anymore.
Cf. https://lore.kernel.org/lkml/CAG48ez21bEn0wL1bbmTiiu8j9jP5iEWtHOwz4tURUJ+ki0ydYw@mail.gmail.com/
* Fix nested domains by implementing a notion of layer level and depth:
- Update landlock_insert_rule() to manage such layers.
- Add an inherit_ruleset() helper to properly create a new domain.
- Rename landlock_find_access() to landlock_find_rule() and return a
full rule reference.
- Add a layer_level and a layer_depth fields to struct landlock_rule.
- Add a top_layer_level field to struct landlock_ruleset.
* Remove access rights that may be required for FD-only requests:
truncate, getattr, lock, chmod, chown, chgrp, ioctl. This will be
handle in a future evolution of Landlock, but right now the goal is to
lighten the code to ease review.
* Remove LANDLOCK_ACCESS_FS_OPEN and rename
LANDLOCK_ACCESS_FS_{READ,WRITE} with a FILE suffix.
* Rename LANDLOCK_ACCESS_FS_READDIR to match the *_FILE pattern.
* Remove LANDLOCK_ACCESS_FS_MAP which was useless.
* Fix memory leak in put_hierarchy() (reported by Jann Horn).
* Fix user-after-free and rename free_ruleset() (reported by Jann Horn).
* Replace the for loops with rbtree_postorder_for_each_entry_safe().
* Constify variables.
* Only use refcount_inc() through getter helpers.
* Change Landlock_insert_ruleset_access() to
Landlock_insert_ruleset_rule().
* Rename landlock_put_ruleset_enqueue() to landlock_put_ruleset_deferred().
* Improve kernel documentation and add a warning about the unhandled
access/syscall families.
* Move ABI check to syscall.c .
Changes since v13:
* New implementation, inspired by the previous inode eBPF map, but
agnostic to the underlying kernel object.
Previous changes:
https://lore.kernel.org/lkml/20190721213116.23476-7-mic@digikod.net/
---
security/landlock/Makefile | 2 +-
security/landlock/limits.h | 17 ++
security/landlock/ruleset.c | 469 ++++++++++++++++++++++++++++++++++++
security/landlock/ruleset.h | 165 +++++++++++++
4 files changed, 652 insertions(+), 1 deletion(-)
create mode 100644 security/landlock/limits.h
create mode 100644 security/landlock/ruleset.c
create mode 100644 security/landlock/ruleset.h
From: Mickaël Salaün <redacted>
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 cannot 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 these 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 objects. 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.
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
Reviewed-by: Jann Horn <jannh@google.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-2-mic@digikod.net
---
Changes since v28:
* Improve Kconfig description (suggested by Serge Hallyn).
* Add Acked-by Serge Hallyn.
* Clean up comment.
Changes since v27:
* Update Kconfig for landlock_restrict_self(2).
* Cosmetic fixes: use 80 columns in Kconfig and align Makefile
declarations.
Changes since v26:
* Update Kconfig for landlock_enforce_ruleset_self(2).
* Fix spelling.
Changes since v24:
* Fix typo in comment (spotted by Jann Horn).
* Add Reviewed-by: Jann Horn [off-list ref]
Changes since v23:
* Update landlock_create_object() to return error codes instead of NULL.
This help error handling in callers.
* When using make oldconfig with a previous configuration already
including the CONFIG_LSM variable, no question is asked to update its
content. Update the Kconfig help to warn about LSM stacking
configuration.
* Constify variable (spotted by Vincent Dagonneau).
Changes since v22:
* Fix spelling (spotted by Jann Horn).
Changes since v21:
* Update Kconfig help.
* Clean up comments.
Changes since v18:
* Account objects to kmemcg.
Changes since v14:
* Simplify the object, rule and ruleset management at the expense of a
less aggressive memory freeing (contributed by Jann Horn, with
additional modifications):
- Remove object->list aggregating the rules tied to an object.
- Remove landlock_get_object(), landlock_drop_object(),
{get,put}_object_cleaner() and landlock_rule_is_disabled().
- Rewrite landlock_put_object() to use a more simple mechanism
(no tricky RCU).
- Replace enum landlock_object_type and landlock_release_object() with
landlock_object_underops->release()
- Adjust unions and Sparse annotations.
Cf. https://lore.kernel.org/lkml/CAG48ez21bEn0wL1bbmTiiu8j9jP5iEWtHOwz4tURUJ+ki0ydYw@mail.gmail.com/
* Merge struct landlock_rule into landlock_ruleset_elem to simplify the
rule management.
* Constify variables.
* Improve kernel documentation.
* Cosmetic variable renames.
* Remove the "default" in the Kconfig (suggested by Jann Horn).
* Only use refcount_inc() through getter helpers.
* Update Kconfig description.
Changes since v13:
* New dedicated implementation, removing the need for eBPF.
Previous changes:
https://lore.kernel.org/lkml/20190721213116.23476-6-mic@digikod.net/
---
MAINTAINERS | 10 +++++
security/Kconfig | 1 +
security/Makefile | 2 +
security/landlock/Kconfig | 21 +++++++++
security/landlock/Makefile | 3 ++
security/landlock/object.c | 67 ++++++++++++++++++++++++++++
security/landlock/object.h | 91 ++++++++++++++++++++++++++++++++++++++
7 files changed, 195 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
From: Mickaël Salaün <redacted>
Using ptrace(2) and related debug features on a target process can lead
to a privilege escalation. Indeed, ptrace(2) can be used by an attacker
to impersonate another task and to remain undetected while performing
malicious activities. Thanks to ptrace_may_access(), various part of
the kernel can check if a tracer is more privileged than a tracee.
A landlocked process has fewer privileges than a non-landlocked process
and must then be subject to additional restrictions when manipulating
processes. To be allowed to use ptrace(2) and related syscalls on a
target process, a landlocked process must have a subset of the target
process's rules (i.e. the tracee must be in a sub-domain of the tracer).
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
Reviewed-by: Jann Horn <jannh@google.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-5-mic@digikod.net
---
Changes since v28:
* Add Acked-by Serge Hallyn.
Changes since v25:
* Rename function to landlock_add_ptrace_hooks().
Changes since v22:
* Add Reviewed-by: Jann Horn [off-list ref]
Changes since v21:
* Fix copyright dates.
Changes since v14:
* Constify variables.
Changes since v13:
* Make the ptrace restriction mandatory, like in the v10.
* Remove the eBPF dependency.
Previous changes:
https://lore.kernel.org/lkml/20191104172146.30797-5-mic@digikod.net/
---
security/landlock/Makefile | 2 +-
security/landlock/ptrace.c | 120 +++++++++++++++++++++++++++++++++++++
security/landlock/ptrace.h | 14 +++++
security/landlock/setup.c | 2 +
4 files changed, 137 insertions(+), 1 deletion(-)
create mode 100644 security/landlock/ptrace.c
create mode 100644 security/landlock/ptrace.h
From: Mickaël Salaün <redacted>
The sb_delete security hook is called when shutting down a superblock,
which may be useful to release kernel objects tied to the superblock's
lifetime (e.g. inodes).
This new hook is needed by Landlock to release (ephemerally) tagged
struct inodes. This comes from the unprivileged nature of Landlock
described in the next commit.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
Reviewed-by: Jann Horn <jannh@google.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-7-mic@digikod.net
---
Changes since v28:
* Extend hook description (suggested by Serge Hallyn).
* Add Acked-by Serge Hallyn.
Changes since v22:
* Add Reviewed-by: Jann Horn [off-list ref]
Changes since v17:
* Initial patch to replace the direct call to landlock_release_inodes()
(requested by James Morris).
https://lore.kernel.org/lkml/alpine.LRH.2.21.2005150536440.7929@namei.org/
---
fs/super.c | 1 +
include/linux/lsm_hook_defs.h | 1 +
include/linux/lsm_hooks.h | 3 +++
include/linux/security.h | 4 ++++
security/security.c | 5 +++++
5 files changed, 14 insertions(+)
@@ -454,6 +454,7 @@ void generic_shutdown_super(struct super_block *sb)evict_inodes(sb);/* only nonzero refcount inodes can have marks */fsnotify_sb_delete(sb);+security_sb_delete(sb);if(sb->s_dio_done_wq){destroy_workqueue(sb->s_dio_done_wq);
From: Mickaël Salaün <redacted>
These 3 system calls are designed to be used by unprivileged processes
to sandbox themselves:
* landlock_create_ruleset(2): Creates a ruleset and returns its file
descriptor.
* landlock_add_rule(2): Adds a rule (e.g. file hierarchy access) to a
ruleset, identified by the dedicated file descriptor.
* landlock_restrict_self(2): Enforces a ruleset on the calling thread
and its future children (similar to seccomp). This syscall has the
same usage restrictions as seccomp(2): the caller must have the
no_new_privs attribute set or have CAP_SYS_ADMIN in the current user
namespace.
All these syscalls have a "flags" argument (not currently used) to
enable extensibility.
Here are the motivations for these new syscalls:
* A sandboxed process may not have access to file systems, including
/dev, /sys or /proc, but it should still be able to add more
restrictions to itself.
* Neither prctl(2) nor seccomp(2) (which was used in a previous version)
fit well with the current definition of a Landlock security policy.
All passed structs (attributes) are checked at build time to ensure that
they don't contain holes and that they are aligned the same way for each
architecture.
See the user and kernel documentation for more details (provided by a
following commit):
* Documentation/userspace-api/landlock.rst
* Documentation/security/landlock.rst
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-9-mic@digikod.net
---
Changes since v29:
* Rebase on v5.12-rc3 and fix trivial conflict with mount_setattr(2).
Changes since v28:
* Add Acked-by Serge Hallyn.
* Add extra check to be sure to not include ruleset FDs in a dependency
of a ruleset (e.g. a path_beneath rule). This case was already
forbiden thanks to FMODE_PATH and other flags check, but it makes it
explicit now.
Changes since v27:
* Forbid creation of rules with an empty allowed_access value because
they are now ignored (since v26) in path walks.
* Rename landlock_enforce_ruleset_self(2) to landlock_restrict_self(2):
shorter and consistent with the two other syscalls (i.e. verb + direct
object).
* Update ruleset access check according to the new access stack.
* Improve landlock_add_rule(2) documentation.
* Fix comment.
* Remove Reviewed-by Jann Horn because of the above changes.
Changes since v26:
* Rename landlock_enforce_ruleset_current(2) to
landlock_enforce_ruleset_self(2). "current" makes sense for a kernel
developer, but much less from a user space developer stand point.
"self" is widely used to refer to the current task (e.g. /proc/self).
"current" may refer to temporal properties, which could be added later
to this syscall flags (cf. /proc/self/attr/{current,exec}).
* Simplify build_check_abi().
* Rename syscall.c to syscalls.c .
* Use less ambiguous comments.
* Fix spelling.
Changes since v25:
* Revert build_check_abi() as non-inline to trigger a warning if it is
not called.
* Use the new limit names.
Changes since v24:
* Add Reviewed-by: Jann Horn [off-list ref]
* Set build_check_abi() as inline.
Changes since v23:
* Rewrite get_ruleset_from_fd() to please the 0-DAY CI Kernel Test
Service that reported an uninitialized variable (false positive):
https://lore.kernel.org/linux-security-module/202011101854.zGbWwusK-lkp@intel.com/
Anyway, it is cleaner like this.
* Add a comment about E2BIG which can be returned by
landlock_enforce_ruleset_current(2) when there is no more room for
another stacked ruleset (i.e. domain).
Changes since v22:
* Replace security_capable() with ns_capable_noaudit() (suggested by
Jann Horn) and explicitly return EPERM.
* Fix landlock_enforce_ruleset_current(2)'s out_put_creds (spotted by
Jann Horn).
* Add __always_inline to copy_min_struct_from_user() to make its
BUILD_BUG_ON() checks reliable (suggested by Jann Horn).
* Simplify path assignation in get_path_from_fd() (suggested by Jann
Horn).
* Fix spelling (spotted by Jann Horn).
Changes since v21:
* Fix and improve comments.
Changes since v20:
* Remove two arguments to landlock_enforce_ruleset(2) (requested by Arnd
Bergmann) and rename it to landlock_enforce_ruleset_current(2): remove
the enum landlock_target_type and the target file descriptor (not used
for now). A ruleset can only be enforced on the current thread.
* Remove the size argument in landlock_add_rule() (requested by Arnd
Bergmann).
* Remove landlock_get_features(2) (suggested by Arnd Bergmann).
* Simplify and rename copy_struct_if_any_from_user() to
copy_min_struct_from_user().
* Rename "options" to "flags" to allign with current syscalls.
* Rename some types and variables in a more consistent way.
* Fix missing type declarations in syscalls.h .
Changes since v19:
* Replace the landlock(2) syscall with 4 syscalls (one for each
command): landlock_get_features(2), landlock_create_ruleset(2),
landlock_add_rule(2) and landlock_enforce_ruleset(2) (suggested by
Arnd Bergmann).
https://lore.kernel.org/lkml/56d15841-e2c1-2d58-59b8-3a6a09b23b4a@digikod.net/
* Return EOPNOTSUPP (instead of ENOPKG) when Landlock is disabled.
* Add two new fields to landlock_attr_features to fit with the new
syscalls: last_rule_type and last_target_type. This enable to easily
identify which types are supported.
* Pack landlock_attr_path_beneath struct because of the removed
ruleset_fd.
* Update documentation and fix spelling.
Changes since v18:
* Remove useless include.
* Remove LLATTR_SIZE() which was only used to shorten lines. Cf. commit
bdc48fa11e46 ("checkpatch/coding-style: deprecate 80-column warning").
Changes since v17:
* Synchronize syscall declaration.
* Fix comment.
Changes since v16:
* Add a size_attr_features field to struct landlock_attr_features for
self-introspection, and move the access_fs field to be more
consistent.
* Replace __aligned_u64 types of attribute fields with __u16, __s32,
__u32 and __u64, and check at build time that these structures does
not contain hole and that they are aligned the same way (8-bits) on
all architectures. This shrinks the size of the userspace ABI, which
may be appreciated especially for struct landlock_attr_features which
could grow a lot in the future. For instance, struct
landlock_attr_features shrinks from 72 bytes to 32 bytes. This change
also enables to remove 64-bits to 32-bits conversion checks.
* Switch syscall attribute pointer and size arguments to follow similar
syscall argument order (e.g. bpf, clone3, openat2).
* Set LANDLOCK_OPT_* types to 32-bits.
* Allow enforcement of empty ruleset, which enables deny-all policies.
* Fix documentation inconsistency.
Changes since v15:
* Do not add file descriptors referring to internal filesystems (e.g.
nsfs) in a ruleset.
* Replace is_user_mountable() with in-place clean checks.
* Replace EBADR with EBADFD in get_ruleset_from_fd() and
get_path_from_fd().
* Remove ruleset's show_fdinfo() for now.
Changes since v14:
* Remove the security_file_open() check in get_path_from_fd(): an
opened FD should not be restricted here, and even less with this hook.
As a result, it is now allowed to add a path to a ruleset even if the
access to this path is not allowed (without O_PATH). This doesn't
change the fact that enforcing a ruleset can't grant any right, only
remove some rights. The new layer levels add more consistent
restrictions.
* Check minimal landlock_attr_* size/content. This fix the case when
no data was provided and e.g., FD 0 was interpreted as ruleset_fd.
Now this leads to a returned -EINVAL.
* Fix credential double-free error case.
* Complete struct landlock_attr_size with size_attr_enforce.
* Fix undefined reference to syscall when Landlock is not selected.
* Remove f.file->f_path.mnt check (suggested by Al Viro).
* Add build-time checks.
* Move ABI checks from fs.c .
* Constify variables.
* Fix spelling.
* Add comments.
Changes since v13:
* New implementation, replacing the dependency on seccomp(2) and bpf(2).
---
include/linux/syscalls.h | 7 +
include/uapi/linux/landlock.h | 53 ++++
kernel/sys_ni.c | 5 +
security/landlock/Makefile | 2 +-
security/landlock/syscalls.c | 445 ++++++++++++++++++++++++++++++++++
5 files changed, 511 insertions(+), 1 deletion(-)
create mode 100644 security/landlock/syscalls.c
@@ -1041,6 +1043,11 @@ asmlinkage long sys_pidfd_send_signal(int pidfd, int sig,siginfo_t__user*info,unsignedintflags);asmlinkagelongsys_pidfd_getfd(intpidfd,intfd,unsignedintflags);+asmlinkagelongsys_landlock_create_ruleset(conststructlandlock_ruleset_attr__user*attr,+size_tsize,__u32flags);+asmlinkagelongsys_landlock_add_rule(intruleset_fd,enumlandlock_rule_typerule_type,+constvoid__user*rule_attr,__u32flags);+asmlinkagelongsys_landlock_restrict_self(intruleset_fd,__u32flags);/**Architecture-specificsystemcalls
From: Mickaël Salaün <redacted>
Wire up the following system calls for all architectures:
* landlock_create_ruleset(2)
* landlock_add_rule(2)
* landlock_restrict_self(2)
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <redacted>
Link: https://lore.kernel.org/r/20210316204252.427806-10-mic@digikod.net
---
Changes since v29:
* Rebase on v5.12-rc3 and fix trivial conflict with mount_setattr(2).
* Synchronize syscall numbers with -next, which are the same as for
v5.12-rc3.
Changes since v27:
* Rename landlock_enforce_ruleset_self(2) to landlock_restrict_self(2).
* Cosmetic fix: align TBL enries.
Changes since v26:
* Rename landlock_enforce_ruleset_current(2) to
landlock_enforce_ruleset_self(2).
Changes since v25:
* Rebase and leave space for the new epoll_pwait2(2) and memfd_secret(2)
from -next.
Changes since v21:
* Rebase and leave space for watch_mount(2) from -next.
Changes since v20:
* Remove landlock_get_features(2).
* Decrease syscall numbers to stick to process_madvise(2) in -next.
* Rename landlock_enforce_ruleset(2) to
landlock_enforce_ruleset_current(2).
Changes since v19:
* Increase syscall numbers by 4 to leave space for new ones (in
linux-next): watch_mount(2), watch_sb(2), fsinfo(2) and
process_madvise(2) (requested by Arnd Bergmann).
* Replace the previous multiplexor landlock(2) with 4 syscalls:
landlock_get_features(2), landlock_create_ruleset(2),
landlock_add_rule(2) and landlock_enforce_ruleset(2).
Changes since v18:
* Increase the syscall number because of the new faccessat2(2).
Changes since v14:
* Add all architectures.
Changes since v13:
* New implementation.
---
arch/alpha/kernel/syscalls/syscall.tbl | 3 +++
arch/arm/tools/syscall.tbl | 3 +++
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 6 ++++++
arch/ia64/kernel/syscalls/syscall.tbl | 3 +++
arch/m68k/kernel/syscalls/syscall.tbl | 3 +++
arch/microblaze/kernel/syscalls/syscall.tbl | 3 +++
arch/mips/kernel/syscalls/syscall_n32.tbl | 3 +++
arch/mips/kernel/syscalls/syscall_n64.tbl | 3 +++
arch/mips/kernel/syscalls/syscall_o32.tbl | 3 +++
arch/parisc/kernel/syscalls/syscall.tbl | 3 +++
arch/powerpc/kernel/syscalls/syscall.tbl | 3 +++
arch/s390/kernel/syscalls/syscall.tbl | 3 +++
arch/sh/kernel/syscalls/syscall.tbl | 3 +++
arch/sparc/kernel/syscalls/syscall.tbl | 3 +++
arch/x86/entry/syscalls/syscall_32.tbl | 3 +++
arch/x86/entry/syscalls/syscall_64.tbl | 3 +++
arch/xtensa/kernel/syscalls/syscall.tbl | 3 +++
include/uapi/asm-generic/unistd.h | 8 +++++++-
19 files changed, 62 insertions(+), 2 deletions(-)
@@ -482,3 +482,6 @@ 550 common process_madvise sys_process_madvise 551 common epoll_pwait2 sys_epoll_pwait2 552 common mount_setattr sys_mount_setattr+553 common landlock_create_ruleset sys_landlock_create_ruleset+554 common landlock_add_rule sys_landlock_add_rule+555 common landlock_restrict_self sys_landlock_restrict_self
@@ -456,3 +456,6 @@ 440 common process_madvise sys_process_madvise 441 common epoll_pwait2 sys_epoll_pwait2 442 common mount_setattr sys_mount_setattr+443 common landlock_create_ruleset sys_landlock_create_ruleset+444 common landlock_add_rule sys_landlock_add_rule+445 common landlock_restrict_self sys_landlock_restrict_self
@@ -363,3 +363,6 @@ 440 common process_madvise sys_process_madvise 441 common epoll_pwait2 sys_epoll_pwait2 442 common mount_setattr sys_mount_setattr+443 common landlock_create_ruleset sys_landlock_create_ruleset+444 common landlock_add_rule sys_landlock_add_rule+445 common landlock_restrict_self sys_landlock_restrict_self
@@ -442,3 +442,6 @@ 440 common process_madvise sys_process_madvise 441 common epoll_pwait2 sys_epoll_pwait2 442 common mount_setattr sys_mount_setattr+443 common landlock_create_ruleset sys_landlock_create_ruleset+444 common landlock_add_rule sys_landlock_add_rule+445 common landlock_restrict_self sys_landlock_restrict_self
@@ -448,3 +448,6 @@ 440 common process_madvise sys_process_madvise 441 common epoll_pwait2 sys_epoll_pwait2 442 common mount_setattr sys_mount_setattr+443 common landlock_create_ruleset sys_landlock_create_ruleset+444 common landlock_add_rule sys_landlock_add_rule+445 common landlock_restrict_self sys_landlock_restrict_self
@@ -440,3 +440,6 @@ 440 common process_madvise sys_process_madvise 441 common epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2 442 common mount_setattr sys_mount_setattr+443 common landlock_create_ruleset sys_landlock_create_ruleset+444 common landlock_add_rule sys_landlock_add_rule+445 common landlock_restrict_self sys_landlock_restrict_self
@@ -522,3 +522,6 @@ 440 common process_madvise sys_process_madvise 441 common epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2 442 common mount_setattr sys_mount_setattr+443 common landlock_create_ruleset sys_landlock_create_ruleset+444 common landlock_add_rule sys_landlock_add_rule+445 common landlock_restrict_self sys_landlock_restrict_self
@@ -445,3 +445,6 @@ 440 common process_madvise sys_process_madvise 441 common epoll_pwait2 sys_epoll_pwait2 442 common mount_setattr sys_mount_setattr+443 common landlock_create_ruleset sys_landlock_create_ruleset+444 common landlock_add_rule sys_landlock_add_rule+445 common landlock_restrict_self sys_landlock_restrict_self
@@ -488,3 +488,6 @@ 440 common process_madvise sys_process_madvise 441 common epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2 442 common mount_setattr sys_mount_setattr+443 common landlock_create_ruleset sys_landlock_create_ruleset+444 common landlock_add_rule sys_landlock_add_rule+445 common landlock_restrict_self sys_landlock_restrict_self
@@ -364,6 +364,9 @@ 440 common process_madvise sys_process_madvise 441 common epoll_pwait2 sys_epoll_pwait2 442 common mount_setattr sys_mount_setattr+443 common landlock_create_ruleset sys_landlock_create_ruleset+444 common landlock_add_rule sys_landlock_add_rule+445 common landlock_restrict_self sys_landlock_restrict_self # # Due to a historical design error, certain syscalls are numbered differently
@@ -413,3 +413,6 @@ 440 common process_madvise sys_process_madvise 441 common epoll_pwait2 sys_epoll_pwait2 442 common mount_setattr sys_mount_setattr+443 common landlock_create_ruleset sys_landlock_create_ruleset+444 common landlock_add_rule sys_landlock_add_rule+445 common landlock_restrict_self sys_landlock_restrict_self
From: Casey Schaufler <casey@schaufler-ca.com>
Move management of the superblock->sb_security blob out of the
individual security modules and into the security infrastructure.
Instead of allocating the blobs from within the modules, the modules
tell the infrastructure how much space is required, and the space is
allocated there.
Cc: Kees Cook <redacted>
Cc: John Johansen <john.johansen@canonical.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Mickaël Salaün <redacted>
Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-6-mic@digikod.net
---
Changes since v28:
* Add Acked-by Serge Hallyn.
Changes since v26:
* Rebase on commit b159e86b5a2a ("selinux: drop super_block backpointer
from superblock_security_struct"). No change in the patch itself,
only a trivial conflict because of an updated nearby line in
selinux_set_mnt_opts() variable declarations.
Changes since v20:
* Remove all Reviewed-by except Stephen Smalley:
https://lore.kernel.org/lkml/CAEjxPJ7ARJO57MBW66=xsBzMMRb=9uLgqocK5eskHCaiVMx7Vw@mail.gmail.com/
* Cosmetic fix in the commit message.
Changes since v17:
* Rebase the original LSM stacking patch from v5.3 to v5.7: I fixed some
diff conflicts caused by code moves and function renames in
selinux/include/objsec.h and selinux/hooks.c . I checked that it
builds but I didn't test the changes for SELinux nor SMACK.
https://lore.kernel.org/r/20190829232935.7099-2-casey@schaufler-ca.com
---
include/linux/lsm_hooks.h | 1 +
security/security.c | 46 ++++++++++++++++++++----
security/selinux/hooks.c | 58 ++++++++++++-------------------
security/selinux/include/objsec.h | 6 ++++
security/selinux/ss/services.c | 3 +-
security/smack/smack.h | 6 ++++
security/smack/smack_lsm.c | 35 +++++--------------
7 files changed, 85 insertions(+), 70 deletions(-)
From: Mickaël Salaün <redacted>
Using Landlock objects and ruleset, it is possible to tag inodes
according to a process's domain. To enable an unprivileged process to
express a file hierarchy, it first needs to open a directory (or a file)
and pass this file descriptor to the kernel through
landlock_add_rule(2). When checking if a file access request is
allowed, we walk from the requested dentry to the real root, following
the different mount layers. The access to each "tagged" inodes are
collected according to their rule layer level, and ANDed to create
access to the requested file hierarchy. This makes possible to identify
a lot of files without tagging every inodes nor modifying the
filesystem, while still following the view and understanding the user
has from the filesystem.
Add a new ARCH_EPHEMERAL_INODES for UML because it currently does not
keep the same struct inodes for the same inodes whereas these inodes are
in use.
This commit adds a minimal set of supported filesystem access-control
which doesn't enable to restrict all file-related actions. This is the
result of multiple discussions to minimize the code of Landlock to ease
review. Thanks to the Landlock design, extending this access-control
without breaking user space will not be a problem. Moreover, seccomp
filters can be used to restrict the use of syscall families which may
not be currently handled by Landlock.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Jeff Dike <redacted>
Cc: Kees Cook <redacted>
Cc: Richard Weinberger <richard@nod.at>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <redacted>
Link: https://lore.kernel.org/r/20210316204252.427806-8-mic@digikod.net
---
Changes since v29:
* Remove a useless unlock/lock for the first loop walk in
hook_sb_delete(). This also makes the code clearer but doesn't change
the garantees for iput().
* Rename iput_inode to prev_inode, which shows its origin.
Changes since v28:
* Fix race conditions that could be caused by concurrent calls to
release_inode() and hook_sb_delete().
* Avoid livelock when a lot of inodes are tagged.
* Improve concurrency locking and add comments to explain the specific
lock rules.
* Add an inode_free_security hook to check that release_inode() and
hook_sb_delete() do their job.
* Add early return to check_access_path() to check if the access request
is empty. This doesn't change the semantic.
* Reword the first description sentence (suggested by Serge Hallyn).
Changes since v27:
* Fix domains with layers of non-overlapping access rights (cf.
layout1.non_overlapping_accesses test) thanks to a stack of access
rights per layer (replacing ORed access rights). This avoids
too-restrictive domains.
* Cosmetic fixes and updates in comments and Kconfig.
Changes since v26:
* Check each rule of a path to enable a more permissive and pragmatic
access control per layer. Suggested by Jann Horn:
https://lore.kernel.org/lkml/CAG48ez1O0VTwEiRd3KqexoF78WR+cmP5bGk5Kh5Cs7aPepiDVg@mail.gmail.com/
* Rename check_access_path_continue() to unmask_layers() and make it
return the new layer mask.
* Avoid double domain check in hook_file_open().
* In the documentation, add utime(2) as another example of unhandled
syscalls. Indeed, using `touch` to test write access may be tempting.
* Remove outdated comment about OverlayFS.
* Rename the landlock.h ifdef to align with most similar files.
* Fix spelling.
Changes since v25:
* Move build_check_layer() to ruleset.c, and add built-time checks for
the fs_access_mask and access variables according to
_LANDLOCK_ACCESS_FS_MASK.
* Move limits to a dedicated file and rename them:
_LANDLOCK_ACCESS_FS_LAST and _LANDLOCK_ACCESS_FS_MASK.
* Set build_check_layer() as non-inline to trigger a warning if it is
not called.
* Use BITS_PER_TYPE() macro.
* Rename function to landlock_add_fs_hooks().
* Cosmetic variable renames.
Changes since v24:
* Use the new struct landlock_rule and landlock_layer to not mix
accesses from different layers. Revert "Enforce deterministic
interleaved path rules" from v24, and fix the layer check. This
enables to follow a sane semantic: an access is granted if, for each
policy layer, at least one rule encountered on the pathwalk grants the
access, regardless of their position in the layer stack (suggested by
Jann Horn). See layout1.interleaved_masked_accesses tests from
tools/testing/selftests/landlock/fs_test.c for corner cases.
* Add build-time checks for layers.
* Use the new landlock_insert_rule() API.
Changes since v23:
* Enforce deterministic interleaved path rules. To have consistent
layered rules, granting access to a path implies that all accesses
tied to inodes, from the requested file to the real root, must be
checked. Otherwise, stacked rules may result to overzealous
restrictions. By excluding the ability to add exceptions in the same
layer (e.g. /a allowed, /a/b denied, and /a/b/c allowed), we get
deterministic interleaved path rules. This removes an optimization
which could be replaced by a proper cache mechanism. This also
further simplifies and explain check_access_path_continue().
* Fix memory allocation error handling in landlock_create_object()
calls. This prevent to inadvertently hold an inode.
* In get_inode_object(), improve comments, make code more readable and
move kfree() call out of the lock window.
* Use the simplified landlock_insert_rule() API.
Changes since v22:
* Simplify check_access_path_continue() (suggested by Jann Horn).
* Remove prefetch() call for now (suggested by Jann Horn).
* Fix spelling and remove superfluous comment (spotted by Jann Horn).
* Cosmetic variable renaming.
Changes since v21:
* Rename ARCH_EPHEMERAL_STATES to ARCH_EPHEMERAL_INODES (suggested by
James Morris).
* Remove the LANDLOCK_ACCESS_FS_CHROOT right because chroot(2) (which
requires CAP_SYS_CHROOT) doesn't enable to bypass Landlock (as tests
demonstrate it), and because it is often used by sandboxes, it would
be counterproductive to forbid it. This also reduces the code size.
* Clean up documentation.
Changes since v19:
* Fix spelling (spotted by Randy Dunlap).
Changes since v18:
* Remove useless include.
* Fix spelling.
Changes since v17:
* Replace landlock_release_inodes() with security_sb_delete() (requested
by James Morris).
* Replace struct super_block->s_landlock_inode_refs with the LSM
infrastructure management of the superblock (requested by James
Morris).
* Fix mknod restriction with a zero mode (spotted by Vincent Dagonneau).
* Minimize executed code in path_mknod and file_open hooks when the
current tasks is not sandboxed.
* Remove useless checks on the file pointer and inode in
hook_file_open() .
* Constify domain pointers.
* Rename inode_landlock() to landlock_inode().
* Import include/uapi/linux/landlock.h and _LANDLOCK_ACCESS_FS_* from
the ruleset and domain management patch.
* Explain the rational of this minimal set of access-control.
https://lore.kernel.org/lkml/f646e1c7-33cf-333f-070c-0a40ad0468cd@digikod.net/
Changes since v16:
* Add ARCH_EPHEMERAL_STATES and enable it for UML.
Changes since v15:
* Replace layer_levels and layer_depth with a bitfield of layers: this
enables to properly manage superset and subset of access rights,
whatever their order in the stack of layers.
Cf. https://lore.kernel.org/lkml/e07fe473-1801-01cc-12ae-b3167f95250e@digikod.net/
* Allow to open pipes and similar special files through /proc/self/fd/.
* Properly handle internal filesystems such as nsfs: always allow these
kind of roots because disconnected path cannot be evaluated.
* Remove the LANDLOCK_ACCESS_FS_LINK_TO and
LANDLOCK_ACCESS_FS_RENAME_{TO,FROM}, but use the
LANDLOCK_ACCESS_FS_REMOVE_{FILE,DIR} and LANDLOCK_ACCESS_FS_MAKE_*
instead. Indeed, it is not possible for now (and not really useful)
to express the semantic of a source and a destination.
* Check access rights to remove a directory or a file with rename(2).
* Forbid reparenting when linking or renaming. This is needed to easily
protect against possible privilege escalation by changing the place of
a file or directory in relation to an enforced access policy (from the
set of layers). This will be relaxed in the future.
* Update hooks to take into account replacement of the object's self and
beneath access bitfields with one. Simplify the code.
* Check file related access rights.
* Check d_is_negative() instead of !d_backing_inode() in
check_access_path_continue(), and continue the path walk while there
is no mapped inode e.g., with rename(2).
* Check private inode in check_access_path().
* Optimize get_file_access() when dealing with a directory.
* Add missing atomic.h .
Changes since v14:
* Simplify the object, rule and ruleset management at the expense of a
less aggressive memory freeing (contributed by Jann Horn, with
additional modifications):
- Rewrite release_inode() to use inode->sb->s_landlock_inode_refs.
- Remove useless checks in landlock_release_inodes(), clean object
pointer according to the new struct landlock_object and wait for all
iput() to complete.
- Rewrite get_inode_object() according to the new struct
landlock_object. If there is a race-condition when cleaning up an
object, we retry until the concurrent thread finished the object
cleaning.
Cf. https://lore.kernel.org/lkml/CAG48ez21bEn0wL1bbmTiiu8j9jP5iEWtHOwz4tURUJ+ki0ydYw@mail.gmail.com/
* Fix nested domains by implementing a notion of layer level and depth:
- Check for matching level ranges when walking through a file path.
- Only allow access if every layer granted the access request.
* Handles files without mount points (e.g. pipes).
* Hardens path walk by checking inode pointer values.
* Prefetches d_parent when walking to the root directory.
* Remove useless inode_alloc_security hook() (suggested by Jann Horn):
already initialized by lsm_inode_alloc().
* Remove the inode_free_security hook.
* Remove access checks that may be required for FD-only requests:
truncate, getattr, lock, chmod, chown, chgrp, ioctl. This will be
handle in a future evolution of Landlock, but right now the goal is to
lighten the code to ease review.
* Constify variables.
* Move ABI checks into syscall.c .
* Cosmetic variable renames.
Changes since v11:
* Add back, revamp and make a fully working filesystem access-control
based on paths and inodes.
* Remove the eBPF dependency.
Previous changes:
https://lore.kernel.org/lkml/20190721213116.23476-6-mic@digikod.net/
---
MAINTAINERS | 1 +
arch/Kconfig | 7 +
arch/um/Kconfig | 1 +
include/uapi/linux/landlock.h | 75 ++++
security/landlock/Kconfig | 2 +-
security/landlock/Makefile | 2 +-
security/landlock/fs.c | 687 ++++++++++++++++++++++++++++++++++
security/landlock/fs.h | 56 +++
security/landlock/limits.h | 4 +
security/landlock/ruleset.c | 4 +
security/landlock/setup.c | 7 +
security/landlock/setup.h | 2 +
12 files changed, 846 insertions(+), 2 deletions(-)
create mode 100644 include/uapi/linux/landlock.h
create mode 100644 security/landlock/fs.c
create mode 100644 security/landlock/fs.h
@@ -217,9 +219,11 @@ static void build_check_layer(void){conststructlandlock_layerlayer={.level=~0,+.access=~0,};BUILD_BUG_ON(layer.level<LANDLOCK_MAX_NUM_LAYERS);+BUILD_BUG_ON(layer.access<LANDLOCK_MASK_ACCESS_FS);}/* @ruleset must be locked by the caller. */
From: Mickaël Salaün <redacted>
Add a basic sandbox tool to launch a command which can only access a
list of file hierarchies in a read-only or read-write way.
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <redacted>
Reviewed-by: Jann Horn <jannh@google.com>
Link: https://lore.kernel.org/r/20210316204252.427806-12-mic@digikod.net
---
Changes since v28:
* Simplify Kconfig option title.
Changes since v27:
* Add samples/landlock/ to MAINTAINERS.
* Update landlock_restrict_self(2).
* Tweak Kconfig title and description.
Changes since v25:
* Improve comments and fix help (suggested by Jann Horn).
* Add a safeguard for errno check (suggested by Jann Horn).
* Allows users to not use all possible restrictions (e.g. use LL_FS_RO
without LL_FS_RW).
* Update syscall names.
* Improve Makefile:
- Replace hostprogs/always-y with userprogs-always-y, available since
commit faabed295ccc ("kbuild: introduce hostprogs-always-y and
userprogs-always-y").
- Depends on CC_CAN_LINK.
* Add Reviewed-by Jann Horn.
Changes since v25:
* Remove useless errno set in the syscall wrappers.
* Cosmetic variable renames.
Changes since v23:
* Re-add hints to help users understand the required kernel
configuration. This was removed with the removal of
landlock_get_features(2).
Changes since v21:
* Remove LANDLOCK_ACCESS_FS_CHROOT.
* Clean up help.
Changes since v20:
* Update with new syscalls and type names.
* Update errno check for EOPNOTSUPP.
* Use the full syscall interfaces: explicitely set the "flags" field to
zero.
Changes since v19:
* Update with the new Landlock syscalls.
* Comply with commit 5f2fb52fac15 ("kbuild: rename hostprogs-y/always to
hostprogs/always-y").
Changes since v16:
* Switch syscall attribute pointer and size arguments.
Changes since v15:
* Update access right names.
* Properly assign access right to files according to the new related
syscall restriction.
* Replace "select" with "depends on" HEADERS_INSTALL (suggested by Randy
Dunlap).
Changes since v14:
* Fix Kconfig dependency.
* Remove access rights that may be required for FD-only requests:
mmap, truncate, getattr, lock, chmod, chown, chgrp, ioctl.
* Fix useless hardcoded syscall number.
* Use execvpe().
* Follow symlinks.
* Extend help with common file paths.
* Constify variables.
* Clean up comments.
* Improve error message.
Changes since v11:
* Add back the filesystem sandbox manager and update it to work with the
new Landlock syscall.
Previous changes:
https://lore.kernel.org/lkml/20190721213116.23476-9-mic@digikod.net/
---
MAINTAINERS | 1 +
samples/Kconfig | 7 ++
samples/Makefile | 1 +
samples/landlock/.gitignore | 1 +
samples/landlock/Makefile | 13 ++
samples/landlock/sandboxer.c | 238 +++++++++++++++++++++++++++++++++++
6 files changed, 261 insertions(+)
create mode 100644 samples/landlock/.gitignore
create mode 100644 samples/landlock/Makefile
create mode 100644 samples/landlock/sandboxer.c
From: Mickaël Salaün <redacted>
This documentation can be built with the Sphinx framework.
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <redacted>
Reviewed-by: Vincent Dagonneau <redacted>
Link: https://lore.kernel.org/r/20210316204252.427806-13-mic@digikod.net
---
Changes since v28:
* Reorder subsections by importance in the "Current limitations"
section.
Changes since v27:
* Update landlock_restrict_self(2).
* Update date and copyright.
Changes since v25:
* Explain the behavior of layered access rights.
* Explain how bind mounts and overayfs mounts are handled by Landlock:
merged overlayfs mount points have their own inodes, which makes these
hierarchies independent from its upper and lower layers, unlike bind
mounts which share the same inodes between the source hierarchy and
the mount point hierarchy.
New overlayfs mount and bind mount tests check these behaviors.
* Synchronize with the new syscalls.c file and update syscall names.
* Fix spelling.
* Remove Reviewed-by Jann Horn because of the above changes.
Changes since v24:
* Add Reviewed-by Jann Horn.
* Add a paragraph to explain how the ruleset layers work.
* Bump date.
Changes since v23:
* Explain limitations for the maximum number of stacked ruleset, and the
memory usage restrictions.
Changes since v22:
* Fix spelling and remove obsolete sentence (spotted by Jann Horn).
* Bump date.
Changes since v21:
* Move the user space documentation to userspace-api/landlock.rst and
the kernel documentation to security/landlock.rst .
* Add license headers.
* Add last update dates.
* Update MAINTAINERS file.
* Add (back) links to git.kernel.org .
* Fix spelling.
Changes since v20:
* Update examples and documentation with the new syscalls.
Changes since v19:
* Update examples and documentation with the new syscalls.
Changes since v15:
* Add current limitations.
Changes since v14:
* Fix spelling (contributed by Randy Dunlap).
* Extend documentation about inheritance and explain layer levels.
* Remove the use of now-removed access rights.
* Use GitHub links.
* Improve kernel documentation.
* Add section for tests.
* Update example.
Changes since v13:
* Rewrote the documentation according to the major revamp.
Previous changes:
https://lore.kernel.org/lkml/20191104172146.30797-8-mic@digikod.net/
---
Documentation/security/index.rst | 1 +
Documentation/security/landlock.rst | 79 ++++++
Documentation/userspace-api/index.rst | 1 +
Documentation/userspace-api/landlock.rst | 307 +++++++++++++++++++++++
MAINTAINERS | 2 +
5 files changed, 390 insertions(+)
create mode 100644 Documentation/security/landlock.rst
create mode 100644 Documentation/userspace-api/landlock.rst
From: James Morris <jmorris@namei.org> Date: 2021-03-18 23:30:10
I've queued this patchset here:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git landlock_lsm
and pulled it into next-testing, which will get it coverage in linux-next.
All going well, I'll aim to push this to Linus in the next merge window.
More review and testing during that time will be helpful.
--
James Morris
[off-list ref]
I've queued this patchset here:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git landlock_lsm
and pulled it into next-testing, which will get it coverage in linux-next.
All going well, I'll aim to push this to Linus in the next merge window.
More review and testing during that time will be helpful.
On Tue, Mar 16, 2021 at 09:42:45PM +0100, Mickaël Salaün wrote:
From: Casey Schaufler <casey@schaufler-ca.com>
Move management of the superblock->sb_security blob out of the
individual security modules and into the security infrastructure.
Instead of allocating the blobs from within the modules, the modules
tell the infrastructure how much space is required, and the space is
allocated there.
Cc: Kees Cook <redacted>
Cc: John Johansen <john.johansen@canonical.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
On Tue, Mar 16, 2021 at 09:42:46PM +0100, Mickaël Salaün wrote:
From: Mickaël Salaün <redacted>
The sb_delete security hook is called when shutting down a superblock,
which may be useful to release kernel objects tied to the superblock's
lifetime (e.g. inodes).
This new hook is needed by Landlock to release (ephemerally) tagged
struct inodes. This comes from the unprivileged nature of Landlock
described in the next commit.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
On Tue, Mar 16, 2021 at 09:42:51PM +0100, Mickaël Salaün wrote:
From: Mickaël Salaün <redacted>
Add a basic sandbox tool to launch a command which can only access a
list of file hierarchies in a read-only or read-write way.
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <redacted>
I'm very happy to see any example!
Reviewed-by: Kees Cook <redacted>
--
Kees Cook
On Tue, Mar 16, 2021 at 09:42:52PM +0100, Mickaël Salaün wrote:
From: Mickaël Salaün <redacted>
This documentation can be built with the Sphinx framework.
Well, yes. :) Maybe describe what the documentation covers instead here.
Regardless: yay docs! This is great.
[...]
+Bind mounts and OverlayFS
+-------------------------
+
+Landlock enables to restrict access to file hierarchies, which means that these
+access rights can be propagated with bind mounts (cf.
+:doc:`/filesystems/sharedsubtree`) but not with :doc:`/filesystems/overlayfs`.
+
+A bind mount mirrors a source file hierarchy to a destination. The destination
+hierarchy is then composed of the exact same files, on which Landlock rules can
+be tied, either via the source or the destination path. These rules restrict
+access when they are encountered on a path, which means that they can restrict
+access to multiple file hierarchies at the same time, whether these hierarchies
+are the result of bind mounts or not.
+
+An OverlayFS mount point consists of upper and lower layers. These layers are
+combined in a merge directory, result of the mount point. This merge hierarchy
+may include files from the upper and lower layers, but modifications performed
+on the merge hierarchy only reflects on the upper layer. From a Landlock
+policy point of view, each OverlayFS layers and merge hierarchies are
+standalone and contains their own set of files and directories, which is
+different from bind mounts. A policy restricting an OverlayFS layer will not
+restrict the resulted merged hierarchy, and vice versa.
Can you include some examples about what a user of landlock should do?
i.e. what are some examples of unexpected results when trying to write
policy that runs on top of overlayfs, etc?
[...]
+File renaming and linking
+-------------------------
+
+Because Landlock targets unprivileged access controls, it is needed to properly
+handle composition of rules. Such property also implies rules nesting.
+Properly handling multiple layers of ruleset, each one of them able to restrict
+access to files, also implies to inherit the ruleset restrictions from a parent
+to its hierarchy. Because files are identified and restricted by their
+hierarchy, moving or linking a file from one directory to another implies to
+propagate the hierarchy constraints. To protect against privilege escalations
+through renaming or linking, and for the sack of simplicity, Landlock currently
typo: sack -> sake
[...]
+Special filesystems
+-------------------
+
+Access to regular files and directories can be restricted by Landlock,
+according to the handled accesses of a ruleset. However, files that do not
+come from a user-visible filesystem (e.g. pipe, socket), but can still be
+accessed through /proc/self/fd/, cannot currently be restricted. Likewise,
+some special kernel filesystems such as nsfs, which can be accessed through
+/proc/self/ns/, cannot currently be restricted. For now, these kind of special
+paths are then always allowed. Future Landlock evolutions will enable to
+restrict such paths with dedicated ruleset flags.
With this series, can /proc (at the top level) be blocked? (i.e. can a
landlock user avoid the weirdness by making /proc/$pid/ unavailable?)
+Ruleset layers
+--------------
+
+There is a limit of 64 layers of stacked rulesets. This can be an issue for a
+task willing to enforce a new ruleset in complement to its 64 inherited
+rulesets. Once this limit is reached, sys_landlock_restrict_self() returns
+E2BIG. It is then strongly suggested to carefully build rulesets once in the
+life of a thread, especially for applications able to launch other applications
+that may also want to sandbox themselves (e.g. shells, container managers,
+etc.).
On Tue, Mar 16, 2021 at 09:42:41PM +0100, Mickaël Salaün wrote:
quoted hunk
From: Mickaël Salaün <redacted>
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 cannot 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 these 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 objects. 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.
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
Reviewed-by: Jann Horn <jannh@google.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-2-mic@digikod.net
---
Changes since v28:
* Improve Kconfig description (suggested by Serge Hallyn).
* Add Acked-by Serge Hallyn.
* Clean up comment.
Changes since v27:
* Update Kconfig for landlock_restrict_self(2).
* Cosmetic fixes: use 80 columns in Kconfig and align Makefile
declarations.
Changes since v26:
* Update Kconfig for landlock_enforce_ruleset_self(2).
* Fix spelling.
Changes since v24:
* Fix typo in comment (spotted by Jann Horn).
* Add Reviewed-by: Jann Horn [off-list ref]
Changes since v23:
* Update landlock_create_object() to return error codes instead of NULL.
This help error handling in callers.
* When using make oldconfig with a previous configuration already
including the CONFIG_LSM variable, no question is asked to update its
content. Update the Kconfig help to warn about LSM stacking
configuration.
* Constify variable (spotted by Vincent Dagonneau).
Changes since v22:
* Fix spelling (spotted by Jann Horn).
Changes since v21:
* Update Kconfig help.
* Clean up comments.
Changes since v18:
* Account objects to kmemcg.
Changes since v14:
* Simplify the object, rule and ruleset management at the expense of a
less aggressive memory freeing (contributed by Jann Horn, with
additional modifications):
- Remove object->list aggregating the rules tied to an object.
- Remove landlock_get_object(), landlock_drop_object(),
{get,put}_object_cleaner() and landlock_rule_is_disabled().
- Rewrite landlock_put_object() to use a more simple mechanism
(no tricky RCU).
- Replace enum landlock_object_type and landlock_release_object() with
landlock_object_underops->release()
- Adjust unions and Sparse annotations.
Cf. https://lore.kernel.org/lkml/CAG48ez21bEn0wL1bbmTiiu8j9jP5iEWtHOwz4tURUJ+ki0ydYw@mail.gmail.com/
* Merge struct landlock_rule into landlock_ruleset_elem to simplify the
rule management.
* Constify variables.
* Improve kernel documentation.
* Cosmetic variable renames.
* Remove the "default" in the Kconfig (suggested by Jann Horn).
* Only use refcount_inc() through getter helpers.
* Update Kconfig description.
Changes since v13:
* New dedicated implementation, removing the need for eBPF.
Previous changes:
https://lore.kernel.org/lkml/20190721213116.23476-6-mic@digikod.net/
---
MAINTAINERS | 10 +++++
security/Kconfig | 1 +
security/Makefile | 2 +
security/landlock/Kconfig | 21 +++++++++
security/landlock/Makefile | 3 ++
security/landlock/object.c | 67 ++++++++++++++++++++++++++++
security/landlock/object.h | 91 ++++++++++++++++++++++++++++++++++++++
7 files changed, 195 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
Is there any benefit to using a dedicated kmem_cache instead of kmalloc?
I see later that you end up with variable-sized allocations, so this
might be fine as-is.
quoted hunk
+ if (!new_object)
+ return ERR_PTR(-ENOMEM);
+ refcount_set(&new_object->usage, 1);
+ spin_lock_init(&new_object->lock);
+ new_object->underops = underops;
+ new_object->underobj = underobj;
+ return new_object;
+}
+
+/*
+ * The caller must own the object (i.e. thanks to object->usage) to safely put
+ * it.
+ */
+void landlock_put_object(struct landlock_object *const object)
+{
+ /*
+ * The call to @object->underops->release(object) might sleep, e.g.
+ * because of iput().
+ */
+ might_sleep();
+ if (!object)
+ return;
+
+ /*
+ * If the @object's refcount cannot drop to zero, we can just decrement
+ * the refcount without holding a lock. Otherwise, the decrement must
+ * happen under @object->lock for synchronization with things like
+ * get_inode_object().
+ */
+ if (refcount_dec_and_lock(&object->usage, &object->lock)) {
+ __acquire(&object->lock);
+ /*
+ * With @object->lock initially held, remove the reference from
+ * @object->underobj to @object (if it still exists).
+ */
+ object->underops->release(object);
+ kfree_rcu(object, rcu_free);
+ }
+}
On Tue, Mar 16, 2021 at 09:42:42PM +0100, Mickaël Salaün wrote:
From: Mickaël Salaün <redacted>
A Landlock ruleset is mainly a red-black tree with Landlock rules as
nodes. This enables quick update and lookup to match a requested
access, e.g. to a file. A ruleset is usable through a dedicated file
descriptor (cf. following commit implementing syscalls) which enables a
process to create and populate a ruleset with new rules.
A domain is a ruleset tied to a set of processes. This group of rules
defines the security policy enforced on these processes and their future
children. A domain can transition to a new domain which is the
intersection of all its constraints and those of a ruleset provided by
the current process. This modification only impact the current process.
This means that a process can only gain more constraints (i.e. lose
accesses) over time.
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-3-mic@digikod.net
(Aside: you appear to be self-adding your Link: tags -- AIUI, this is
normally done by whoever pulls your series. I've only seen Link: tags
added when needing to refer to something else not included in the
series.)
This is checking that the largest possible stored value is correctly
within the LANDLOCK_MAX_* macro value?
[...]
The locking all looks right, and given your test coverage and syzkaller
work, it's hard for me to think of ways to prove it out any better. :)
Reviewed-by: Kees Cook <redacted>
--
Kees Cook
On Tue, Mar 16, 2021 at 09:42:43PM +0100, Mickaël Salaün wrote:
config LSM
string "Ordered list of enabled LSMs"
- default "lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
- default "lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
- default "lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
- default "lockdown,yama,loadpin,safesetid,integrity,bpf" if DEFAULT_SECURITY_DAC
- default "lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"
+ default "landlock,lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
+ default "landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
+ default "landlock,lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
+ default "landlock,lockdown,yama,loadpin,safesetid,integrity,bpf" if DEFAULT_SECURITY_DAC
+ default "landlock,lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"
help
A comma-separated list of LSMs, in initialization order.
Any LSMs left off this list will be ignored. This can be
There was some discussion long ago about landlock needing to be last
in the list because it was unprivileged. Is that no longer true? (And
what is the justification for its position in the list?)
On Tue, Mar 16, 2021 at 09:42:44PM +0100, Mickaël Salaün wrote:
From: Mickaël Salaün <redacted>
Using ptrace(2) and related debug features on a target process can lead
to a privilege escalation. Indeed, ptrace(2) can be used by an attacker
to impersonate another task and to remain undetected while performing
malicious activities. Thanks to ptrace_may_access(), various part of
the kernel can check if a tracer is more privileged than a tracee.
A landlocked process has fewer privileges than a non-landlocked process
and must then be subject to additional restrictions when manipulating
processes. To be allowed to use ptrace(2) and related syscalls on a
target process, a landlocked process must have a subset of the target
process's rules (i.e. the tracee must be in a sub-domain of the tracer).
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
On Tue, Mar 16, 2021 at 09:42:52PM +0100, Mickaël Salaün wrote:
quoted
From: Mickaël Salaün <redacted>
This documentation can be built with the Sphinx framework.
Well, yes. :) Maybe describe what the documentation covers instead here.
Regardless: yay docs! This is great.
Well, I don't know what to describe other than the subject, the rest is
in the patch. :)
quoted
[...]
+Bind mounts and OverlayFS
+-------------------------
+
+Landlock enables to restrict access to file hierarchies, which means that these
+access rights can be propagated with bind mounts (cf.
+:doc:`/filesystems/sharedsubtree`) but not with :doc:`/filesystems/overlayfs`.
+
+A bind mount mirrors a source file hierarchy to a destination. The destination
+hierarchy is then composed of the exact same files, on which Landlock rules can
+be tied, either via the source or the destination path. These rules restrict
+access when they are encountered on a path, which means that they can restrict
+access to multiple file hierarchies at the same time, whether these hierarchies
+are the result of bind mounts or not.
+
+An OverlayFS mount point consists of upper and lower layers. These layers are
+combined in a merge directory, result of the mount point. This merge hierarchy
+may include files from the upper and lower layers, but modifications performed
+on the merge hierarchy only reflects on the upper layer. From a Landlock
+policy point of view, each OverlayFS layers and merge hierarchies are
+standalone and contains their own set of files and directories, which is
+different from bind mounts. A policy restricting an OverlayFS layer will not
+restrict the resulted merged hierarchy, and vice versa.
Can you include some examples about what a user of landlock should do?
i.e. what are some examples of unexpected results when trying to write
policy that runs on top of overlayfs, etc?
Landlock works well with overlayfs, at least from my point of view. It
may be a bit disturbing with bind mount though, but it is the same with
other access-controls (e.g. DAC). Landlock users should just think about
file hierarchies and create their policies accordingly. A user should
not try to adapt a policy according to his/her understanding of
overlayfs, but just think about file hierarchies.
quoted
[...]
+File renaming and linking
+-------------------------
+
+Because Landlock targets unprivileged access controls, it is needed to properly
+handle composition of rules. Such property also implies rules nesting.
+Properly handling multiple layers of ruleset, each one of them able to restrict
+access to files, also implies to inherit the ruleset restrictions from a parent
+to its hierarchy. Because files are identified and restricted by their
+hierarchy, moving or linking a file from one directory to another implies to
+propagate the hierarchy constraints. To protect against privilege escalations
+through renaming or linking, and for the sack of simplicity, Landlock currently
typo: sack -> sake
Indeed
quoted
[...]
+Special filesystems
+-------------------
+
+Access to regular files and directories can be restricted by Landlock,
+according to the handled accesses of a ruleset. However, files that do not
+come from a user-visible filesystem (e.g. pipe, socket), but can still be
+accessed through /proc/self/fd/, cannot currently be restricted. Likewise,
+some special kernel filesystems such as nsfs, which can be accessed through
+/proc/self/ns/, cannot currently be restricted. For now, these kind of special
+paths are then always allowed. Future Landlock evolutions will enable to
+restrict such paths with dedicated ruleset flags.
With this series, can /proc (at the top level) be blocked? (i.e. can a
landlock user avoid the weirdness by making /proc/$pid/ unavailable?)
/proc can be blocked, but not /proc/*/ns/* because of disconnected
roots. I plan to address this.
quoted
+Ruleset layers
+--------------
+
+There is a limit of 64 layers of stacked rulesets. This can be an issue for a
+task willing to enforce a new ruleset in complement to its 64 inherited
+rulesets. Once this limit is reached, sys_landlock_restrict_self() returns
+E2BIG. It is then strongly suggested to carefully build rulesets once in the
+life of a thread, especially for applications able to launch other applications
+that may also want to sandbox themselves (e.g. shells, container managers,
+etc.).
How was this value (64) chosen?
It was first an implementation constraint to limit the memory allocated
for each rule, but it could now be increased if needed. The
implementation still uses u64 for the sake of simplicity, but we could
switch to a bitmap type.
On Tue, Mar 16, 2021 at 09:42:47PM +0100, Mickaël Salaün wrote:
From: Mickaël Salaün <redacted>
Using Landlock objects and ruleset, it is possible to tag inodes
according to a process's domain. To enable an unprivileged process to
express a file hierarchy, it first needs to open a directory (or a file)
and pass this file descriptor to the kernel through
landlock_add_rule(2). When checking if a file access request is
allowed, we walk from the requested dentry to the real root, following
the different mount layers. The access to each "tagged" inodes are
collected according to their rule layer level, and ANDed to create
access to the requested file hierarchy. This makes possible to identify
a lot of files without tagging every inodes nor modifying the
filesystem, while still following the view and understanding the user
has from the filesystem.
Add a new ARCH_EPHEMERAL_INODES for UML because it currently does not
keep the same struct inodes for the same inodes whereas these inodes are
in use.
This commit adds a minimal set of supported filesystem access-control
which doesn't enable to restrict all file-related actions. This is the
result of multiple discussions to minimize the code of Landlock to ease
review. Thanks to the Landlock design, extending this access-control
without breaking user space will not be a problem. Moreover, seccomp
filters can be used to restrict the use of syscall families which may
not be currently handled by Landlock.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Jeff Dike <redacted>
Cc: Kees Cook <redacted>
Cc: Richard Weinberger <richard@nod.at>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <redacted>
Link: https://lore.kernel.org/r/20210316204252.427806-8-mic@digikod.net
[...]
+ spin_lock(&sb->s_inode_list_lock);
+ list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
+ struct landlock_object *object;
+
+ /* Only handles referenced inodes. */
+ if (!atomic_read(&inode->i_count))
+ continue;
+
+ /*
+ * Checks I_FREEING and I_WILL_FREE to protect against a race
+ * condition when release_inode() just called iput(), which
+ * could lead to a NULL dereference of inode->security or a
+ * second call to iput() for the same Landlock object. Also
+ * checks I_NEW because such inode cannot be tied to an object.
+ */
+ spin_lock(&inode->i_lock);
+ if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
+ spin_unlock(&inode->i_lock);
+ continue;
+ }
This (and elsewhere here) seems like a lot of inode internals getting
exposed. Can any of this be repurposed into helpers? I see this test
scattered around the kernel a fair bit:
$ git grep I_FREEING | grep I_WILL_FREE | grep I_NEW | wc -l
9
+static inline u32 get_mode_access(const umode_t mode)
+{
+ switch (mode & S_IFMT) {
+ case S_IFLNK:
+ return LANDLOCK_ACCESS_FS_MAKE_SYM;
+ case 0:
+ /* A zero mode translates to S_IFREG. */
+ case S_IFREG:
+ return LANDLOCK_ACCESS_FS_MAKE_REG;
+ case S_IFDIR:
+ return LANDLOCK_ACCESS_FS_MAKE_DIR;
+ case S_IFCHR:
+ return LANDLOCK_ACCESS_FS_MAKE_CHAR;
+ case S_IFBLK:
+ return LANDLOCK_ACCESS_FS_MAKE_BLOCK;
+ case S_IFIFO:
+ return LANDLOCK_ACCESS_FS_MAKE_FIFO;
+ case S_IFSOCK:
+ return LANDLOCK_ACCESS_FS_MAKE_SOCK;
+ default:
+ WARN_ON_ONCE(1);
+ return 0;
+ }
I'm assuming this won't be reachable from userspace.
I think this landlock_initialized is logically separate from the optional
DEFINE_LSM "enabled" variable, but I thought I'd double check. :)
It seems like it's used here to avoid releasing superblocks before
landlock_init() is called? What is the scenario where that happens?
The locking and inode semantics are pretty complex, but since, again,
it's got significant test and syzkaller coverage, it looks good to me.
With the inode helper cleanup:
Reviewed-by: Kees Cook <redacted>
--
Kees Cook
On Tue, Mar 16, 2021 at 09:42:41PM +0100, Mickaël Salaün wrote:
quoted
From: Mickaël Salaün <redacted>
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 cannot 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 these 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 objects. 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.
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
Reviewed-by: Jann Horn <jannh@google.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-2-mic@digikod.net
---
Changes since v28:
* Improve Kconfig description (suggested by Serge Hallyn).
* Add Acked-by Serge Hallyn.
* Clean up comment.
Changes since v27:
* Update Kconfig for landlock_restrict_self(2).
* Cosmetic fixes: use 80 columns in Kconfig and align Makefile
declarations.
Changes since v26:
* Update Kconfig for landlock_enforce_ruleset_self(2).
* Fix spelling.
Changes since v24:
* Fix typo in comment (spotted by Jann Horn).
* Add Reviewed-by: Jann Horn [off-list ref]
Changes since v23:
* Update landlock_create_object() to return error codes instead of NULL.
This help error handling in callers.
* When using make oldconfig with a previous configuration already
including the CONFIG_LSM variable, no question is asked to update its
content. Update the Kconfig help to warn about LSM stacking
configuration.
* Constify variable (spotted by Vincent Dagonneau).
Changes since v22:
* Fix spelling (spotted by Jann Horn).
Changes since v21:
* Update Kconfig help.
* Clean up comments.
Changes since v18:
* Account objects to kmemcg.
Changes since v14:
* Simplify the object, rule and ruleset management at the expense of a
less aggressive memory freeing (contributed by Jann Horn, with
additional modifications):
- Remove object->list aggregating the rules tied to an object.
- Remove landlock_get_object(), landlock_drop_object(),
{get,put}_object_cleaner() and landlock_rule_is_disabled().
- Rewrite landlock_put_object() to use a more simple mechanism
(no tricky RCU).
- Replace enum landlock_object_type and landlock_release_object() with
landlock_object_underops->release()
- Adjust unions and Sparse annotations.
Cf. https://lore.kernel.org/lkml/CAG48ez21bEn0wL1bbmTiiu8j9jP5iEWtHOwz4tURUJ+ki0ydYw@mail.gmail.com/
* Merge struct landlock_rule into landlock_ruleset_elem to simplify the
rule management.
* Constify variables.
* Improve kernel documentation.
* Cosmetic variable renames.
* Remove the "default" in the Kconfig (suggested by Jann Horn).
* Only use refcount_inc() through getter helpers.
* Update Kconfig description.
Changes since v13:
* New dedicated implementation, removing the need for eBPF.
Previous changes:
https://lore.kernel.org/lkml/20190721213116.23476-6-mic@digikod.net/
---
MAINTAINERS | 10 +++++
security/Kconfig | 1 +
security/Makefile | 2 +
security/landlock/Kconfig | 21 +++++++++
security/landlock/Makefile | 3 ++
security/landlock/object.c | 67 ++++++++++++++++++++++++++++
security/landlock/object.h | 91 ++++++++++++++++++++++++++++++++++++++
7 files changed, 195 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
Is there any benefit to using a dedicated kmem_cache instead of kmalloc?
I see later that you end up with variable-sized allocations, so this
might be fine as-is.
I though about that, but for the sake of simplicity I choose to not use
that in this series. It may comes with a future update if there is a
visible performance improvement though.
quoted
+ if (!new_object)
+ return ERR_PTR(-ENOMEM);
+ refcount_set(&new_object->usage, 1);
+ spin_lock_init(&new_object->lock);
+ new_object->underops = underops;
+ new_object->underobj = underobj;
+ return new_object;
+}
+
+/*
+ * The caller must own the object (i.e. thanks to object->usage) to safely put
+ * it.
+ */
+void landlock_put_object(struct landlock_object *const object)
+{
+ /*
+ * The call to @object->underops->release(object) might sleep, e.g.
+ * because of iput().
+ */
+ might_sleep();
+ if (!object)
+ return;
+
+ /*
+ * If the @object's refcount cannot drop to zero, we can just decrement
+ * the refcount without holding a lock. Otherwise, the decrement must
+ * happen under @object->lock for synchronization with things like
+ * get_inode_object().
+ */
+ if (refcount_dec_and_lock(&object->usage, &object->lock)) {
+ __acquire(&object->lock);
+ /*
+ * With @object->lock initially held, remove the reference from
+ * @object->underobj to @object (if it still exists).
+ */
+ object->underops->release(object);
+ kfree_rcu(object, rcu_free);
+ }
+}
On Tue, Mar 16, 2021 at 09:42:42PM +0100, Mickaël Salaün wrote:
quoted
From: Mickaël Salaün <redacted>
A Landlock ruleset is mainly a red-black tree with Landlock rules as
nodes. This enables quick update and lookup to match a requested
access, e.g. to a file. A ruleset is usable through a dedicated file
descriptor (cf. following commit implementing syscalls) which enables a
process to create and populate a ruleset with new rules.
A domain is a ruleset tied to a set of processes. This group of rules
defines the security policy enforced on these processes and their future
children. A domain can transition to a new domain which is the
intersection of all its constraints and those of a ruleset provided by
the current process. This modification only impact the current process.
This means that a process can only gain more constraints (i.e. lose
accesses) over time.
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-3-mic@digikod.net
(Aside: you appear to be self-adding your Link: tags -- AIUI, this is
normally done by whoever pulls your series. I've only seen Link: tags
added when needing to refer to something else not included in the
series.)
On Tue, Mar 16, 2021 at 09:42:48PM +0100, Mickaël Salaün wrote:
From: Mickaël Salaün <redacted>
These 3 system calls are designed to be used by unprivileged processes
to sandbox themselves:
* landlock_create_ruleset(2): Creates a ruleset and returns its file
descriptor.
* landlock_add_rule(2): Adds a rule (e.g. file hierarchy access) to a
ruleset, identified by the dedicated file descriptor.
* landlock_restrict_self(2): Enforces a ruleset on the calling thread
and its future children (similar to seccomp). This syscall has the
same usage restrictions as seccomp(2): the caller must have the
no_new_privs attribute set or have CAP_SYS_ADMIN in the current user
namespace.
All these syscalls have a "flags" argument (not currently used) to
enable extensibility.
For the new-style extensible syscalls, you want only a "size" argument;
"flags" should be within the argument structure.
(And to this end, why 3 syscalls instead of 1, if you're going to use
extensibility anyway?)
+/**
+ * copy_min_struct_from_user - Safe future-proof argument copying
+ *
+ * Extend copy_struct_from_user() to check for consistent user buffer.
+ *
+ * @dst: Kernel space pointer or NULL.
+ * @ksize: Actual size of the data pointed to by @dst.
+ * @ksize_min: Minimal required size to be copied.
+ * @src: User space pointer or NULL.
+ * @usize: (Alleged) size of the data pointed to by @src.
+ */
+static __always_inline int copy_min_struct_from_user(void *const dst,
+ const size_t ksize, const size_t ksize_min,
+ const void __user *const src, const size_t usize)
+{
+ /* Checks buffer inconsistencies. */
+ BUILD_BUG_ON(!dst);
+ if (!src)
+ return -EFAULT;
+
+ /* Checks size ranges. */
+ BUILD_BUG_ON(ksize <= 0);
+ BUILD_BUG_ON(ksize < ksize_min);
+ if (usize < ksize_min)
+ return -EINVAL;
+ if (usize > PAGE_SIZE)
+ return -E2BIG;
+
+ /* Copies user buffer and fills with zeros. */
+ return copy_struct_from_user(dst, ksize, src, usize);
+}
I still wish this was built into copy_struct_from_user(). :) But yes,
this appears to be the way to protect one's self when using
copy_struct_from_user().
+/**
+ * sys_landlock_create_ruleset - Create a new ruleset
+ *
+ * @attr: Pointer to a &struct landlock_ruleset_attr identifying the scope of
+ * the new ruleset.
+ * @size: Size of the pointed &struct landlock_ruleset_attr (needed for
+ * backward and forward compatibility).
+ * @flags: Must be 0.
+ *
+ * This system call enables to create a new Landlock ruleset, and returns the
+ * related file descriptor on success.
+ *
+ * Possible returned errors are:
+ *
+ * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
+ * - EINVAL: @flags is not 0, or unknown access, or too small @size;
+ * - E2BIG or EFAULT: @attr or @size inconsistencies;
+ * - ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
+ */
+SYSCALL_DEFINE3(landlock_create_ruleset,
+ const struct landlock_ruleset_attr __user *const, attr,
+ const size_t, size, const __u32, flags)
+{
+ struct landlock_ruleset_attr ruleset_attr;
+ struct landlock_ruleset *ruleset;
+ int err, ruleset_fd;
+
+ /* Build-time checks. */
+ build_check_abi();
+
+ if (!landlock_initialized)
+ return -EOPNOTSUPP;
+
+ /* No flag for now. */
+ if (flags)
+ return -EINVAL;
+
+ /* Copies raw user space buffer. */
+ err = copy_min_struct_from_user(&ruleset_attr, sizeof(ruleset_attr),
+ offsetofend(typeof(ruleset_attr), handled_access_fs),
The use of offsetofend() here appears to be kind of the "V1", "V2", ...
sizes used in other extensible syscall implementations?
+ attr, size);
+ if (err)
+ return err;
+
+ /* Checks content (and 32-bits cast). */
+ if ((ruleset_attr.handled_access_fs | LANDLOCK_MASK_ACCESS_FS) !=
+ LANDLOCK_MASK_ACCESS_FS)
+ return -EINVAL;
+
+ /* Checks arguments and transforms to kernel struct. */
+ ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs);
+ if (IS_ERR(ruleset))
+ return PTR_ERR(ruleset);
+
+ /* Creates anonymous FD referring to the ruleset. */
+ ruleset_fd = anon_inode_getfd("landlock-ruleset", &ruleset_fops,
+ ruleset, O_RDWR | O_CLOEXEC);
+ if (ruleset_fd < 0)
+ landlock_put_ruleset(ruleset);
+ return ruleset_fd;
+}
+
+/*
+ * Returns an owned ruleset from a FD. It is thus needed to call
+ * landlock_put_ruleset() on the return value.
+ */
+static struct landlock_ruleset *get_ruleset_from_fd(const int fd,
+ const fmode_t mode)
+{
+ struct fd ruleset_f;
+ struct landlock_ruleset *ruleset;
+
+ ruleset_f = fdget(fd);
+ if (!ruleset_f.file)
+ return ERR_PTR(-EBADF);
+
+ /* Checks FD type and access right. */
+ if (ruleset_f.file->f_op != &ruleset_fops) {
+ ruleset = ERR_PTR(-EBADFD);
+ goto out_fdput;
+ }
+ if (!(ruleset_f.file->f_mode & mode)) {
+ ruleset = ERR_PTR(-EPERM);
+ goto out_fdput;
+ }
+ ruleset = ruleset_f.file->private_data;
+ if (WARN_ON_ONCE(ruleset->num_layers != 1)) {
+ ruleset = ERR_PTR(-EINVAL);
+ goto out_fdput;
+ }
+ landlock_get_ruleset(ruleset);
+
+out_fdput:
+ fdput(ruleset_f);
+ return ruleset;
+}
+
+/* Path handling */
+
+/*
+ * @path: Must call put_path(@path) after the call if it succeeded.
+ */
+static int get_path_from_fd(const s32 fd, struct path *const path)
+{
+ struct fd f;
+ int err = 0;
+
+ BUILD_BUG_ON(!__same_type(fd,
+ ((struct landlock_path_beneath_attr *)NULL)->parent_fd));
+
+ /* Handles O_PATH. */
+ f = fdget_raw(fd);
+ if (!f.file)
+ return -EBADF;
+ /*
+ * Only allows O_PATH file descriptor: enables to restrict ambient
+ * filesystem access without requiring to open and risk leaking or
+ * misusing a file descriptor. Forbids ruleset FDs, internal
+ * filesystems (e.g. nsfs), including pseudo filesystems that will
+ * never be mountable (e.g. sockfs, pipefs).
+ */
+ if (!(f.file->f_mode & FMODE_PATH) ||
+ (f.file->f_op == &ruleset_fops) ||
+ (f.file->f_path.mnt->mnt_flags & MNT_INTERNAL) ||
+ (f.file->f_path.dentry->d_sb->s_flags & SB_NOUSER) ||
+ d_is_negative(f.file->f_path.dentry) ||
+ IS_PRIVATE(d_backing_inode(f.file->f_path.dentry))) {
+ err = -EBADFD;
+ goto out_fdput;
+ }
+ *path = f.file->f_path;
+ path_get(path);
+
+out_fdput:
+ fdput(f);
+ return err;
+}
+
+/**
+ * sys_landlock_add_rule - Add a new rule to a ruleset
+ *
+ * @ruleset_fd: File descriptor tied to the ruleset that should be extended
+ * with the new rule.
+ * @rule_type: Identify the structure type pointed to by @rule_attr (only
+ * LANDLOCK_RULE_PATH_BENEATH for now).
+ * @rule_attr: Pointer to a rule (only of type &struct
+ * landlock_path_beneath_attr for now).
+ * @flags: Must be 0.
+ *
+ * This system call enables to define a new rule and add it to an existing
+ * ruleset.
+ *
+ * Possible returned errors are:
+ *
+ * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
+ * - EINVAL: @flags is not 0, or inconsistent access in the rule (i.e.
+ * &landlock_path_beneath_attr.allowed_access is not a subset of the rule's
+ * accesses);
+ * - ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access);
+ * - EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
+ * member of @rule_attr is not a file descriptor as expected;
+ * - EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
+ * @rule_attr is not the expected file descriptor type (e.g. file open
+ * without O_PATH);
+ * - EPERM: @ruleset_fd has no write access to the underlying ruleset;
+ * - EFAULT: @rule_attr inconsistency.
+ */
+SYSCALL_DEFINE4(landlock_add_rule,
+ const int, ruleset_fd, const enum landlock_rule_type, rule_type,
+ const void __user *const, rule_attr, const __u32, flags)
+{
If this is an extensible syscall, I'd expect a structure holding
rule_type, rule_attr, and flags.
+ struct landlock_path_beneath_attr path_beneath_attr;
+ struct path path;
+ struct landlock_ruleset *ruleset;
+ int res, err;
+
+ if (!landlock_initialized)
+ return -EOPNOTSUPP;
+
+ /* No flag for now. */
+ if (flags)
+ return -EINVAL;
+
+ if (rule_type != LANDLOCK_RULE_PATH_BENEATH)
+ return -EINVAL;
+
+ /* Copies raw user space buffer, only one type for now. */
+ res = copy_from_user(&path_beneath_attr, rule_attr,
+ sizeof(path_beneath_attr));
+ if (res)
+ return -EFAULT;
+
+ /* Gets and checks the ruleset. */
+ ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_WRITE);
+ if (IS_ERR(ruleset))
+ return PTR_ERR(ruleset);
+
+ /*
+ * Informs about useless rule: empty allowed_access (i.e. deny rules)
+ * are ignored in path walks.
+ */
+ if (!path_beneath_attr.allowed_access) {
+ err = -ENOMSG;
+ goto out_put_ruleset;
+ }
+ /*
+ * Checks that allowed_access matches the @ruleset constraints
+ * (ruleset->fs_access_masks[0] is automatically upgraded to 64-bits).
+ */
+ if ((path_beneath_attr.allowed_access | ruleset->fs_access_masks[0]) !=
+ ruleset->fs_access_masks[0]) {
+ err = -EINVAL;
+ goto out_put_ruleset;
+ }
+
+ /* Gets and checks the new rule. */
+ err = get_path_from_fd(path_beneath_attr.parent_fd, &path);
+ if (err)
+ goto out_put_ruleset;
+
+ /* Imports the new rule. */
+ err = landlock_append_fs_rule(ruleset, &path,
+ path_beneath_attr.allowed_access);
+ path_put(&path);
+
+out_put_ruleset:
+ landlock_put_ruleset(ruleset);
+ return err;
+}
+
+/* Enforcement */
+
+/**
+ * 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.
+ *
+ * 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
+ * namespace or is running with no_new_privs. This avoids scenarios where
+ * unprivileged tasks can affect the behavior of privileged children.
+ *
+ * Possible returned errors are:
+ *
+ * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
+ * - EINVAL: @flags is not 0.
+ * - EBADF: @ruleset_fd is not a file descriptor for the current thread;
+ * - EBADFD: @ruleset_fd is not a ruleset file descriptor;
+ * - EPERM: @ruleset_fd has no read access to the underlying ruleset, or the
+ * current thread is not running with no_new_privs, or it doesn't have
+ * CAP_SYS_ADMIN in its namespace.
+ * - E2BIG: The maximum number of stacked rulesets is reached for the current
+ * thread.
+ */
+SYSCALL_DEFINE2(landlock_restrict_self,
+ const int, ruleset_fd, const __u32, flags)
+{
Same observation about new style syscalls.
+ struct landlock_ruleset *new_dom, *ruleset;
+ struct cred *new_cred;
+ struct landlock_cred_security *new_llcred;
+ int err;
+
+ if (!landlock_initialized)
+ return -EOPNOTSUPP;
+
+ /* No flag for now. */
+ if (flags)
+ return -EINVAL;
+
+ /*
+ * Similar checks as for seccomp(2), except that an -EPERM may be
+ * returned.
+ */
+ if (!task_no_new_privs(current) &&
+ !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN))
+ return -EPERM;
+
+ /* Gets and checks the ruleset. */
+ ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_READ);
+ if (IS_ERR(ruleset))
+ return PTR_ERR(ruleset);
+
+ /* Prepares new credentials. */
+ new_cred = prepare_creds();
+ if (!new_cred) {
+ err = -ENOMEM;
+ goto out_put_ruleset;
+ }
+ new_llcred = landlock_cred(new_cred);
+
+ /*
+ * There is no possible race condition while copying and manipulating
+ * the current credentials because they are dedicated per thread.
+ */
+ new_dom = landlock_merge_ruleset(new_llcred->domain, ruleset);
+ if (IS_ERR(new_dom)) {
+ err = PTR_ERR(new_dom);
+ goto out_put_creds;
+ }
+
+ /* Replaces the old (prepared) domain. */
+ landlock_put_ruleset(new_llcred->domain);
+ new_llcred->domain = new_dom;
+
+ landlock_put_ruleset(ruleset);
+ return commit_creds(new_cred);
+
+out_put_creds:
+ abort_creds(new_cred);
+
+out_put_ruleset:
+ landlock_put_ruleset(ruleset);
+ return err;
+}
--
2.30.2
On Tue, Mar 16, 2021 at 09:42:43PM +0100, Mickaël Salaün wrote:
quoted
config LSM
string "Ordered list of enabled LSMs"
- default "lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
- default "lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
- default "lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
- default "lockdown,yama,loadpin,safesetid,integrity,bpf" if DEFAULT_SECURITY_DAC
- default "lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"
+ default "landlock,lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
+ default "landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
+ default "landlock,lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
+ default "landlock,lockdown,yama,loadpin,safesetid,integrity,bpf" if DEFAULT_SECURITY_DAC
+ default "landlock,lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"
help
A comma-separated list of LSMs, in initialization order.
Any LSMs left off this list will be ignored. This can be
There was some discussion long ago about landlock needing to be last
in the list because it was unprivileged. Is that no longer true? (And
what is the justification for its position in the list?)
Indeed, I wanted to put Landlock last because it was an unprivileged
programmable access-control, which could lead to side-channel attacks
against other access-controls (e.g. to infer enforced policies). This is
not valid anymore because Landlock is not using eBPF, only the BPF LSM
does that (which is not the only reason why it is the last stacked).
I'd expect this to be named "release" rather than "put" since it doesn't
do any lifetime reference counting.
It does decrement rule->object->usage .
Well, landlock_put_object() decrements rule->object's lifetime. It seems
"rule" doesn't have a lifetime. (There is no refcounter on rule.) I just
find it strange to see "put" without a matching "get". Not a big deal.
--
Kees Cook
On Tue, Mar 16, 2021 at 09:42:47PM +0100, Mickaël Salaün wrote:
quoted
From: Mickaël Salaün <redacted>
Using Landlock objects and ruleset, it is possible to tag inodes
according to a process's domain. To enable an unprivileged process to
express a file hierarchy, it first needs to open a directory (or a file)
and pass this file descriptor to the kernel through
landlock_add_rule(2). When checking if a file access request is
allowed, we walk from the requested dentry to the real root, following
the different mount layers. The access to each "tagged" inodes are
collected according to their rule layer level, and ANDed to create
access to the requested file hierarchy. This makes possible to identify
a lot of files without tagging every inodes nor modifying the
filesystem, while still following the view and understanding the user
has from the filesystem.
Add a new ARCH_EPHEMERAL_INODES for UML because it currently does not
keep the same struct inodes for the same inodes whereas these inodes are
in use.
This commit adds a minimal set of supported filesystem access-control
which doesn't enable to restrict all file-related actions. This is the
result of multiple discussions to minimize the code of Landlock to ease
review. Thanks to the Landlock design, extending this access-control
without breaking user space will not be a problem. Moreover, seccomp
filters can be used to restrict the use of syscall families which may
not be currently handled by Landlock.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Jeff Dike <redacted>
Cc: Kees Cook <redacted>
Cc: Richard Weinberger <richard@nod.at>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <redacted>
Link: https://lore.kernel.org/r/20210316204252.427806-8-mic@digikod.net
[...]
+ spin_lock(&sb->s_inode_list_lock);
+ list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
+ struct landlock_object *object;
+
+ /* Only handles referenced inodes. */
+ if (!atomic_read(&inode->i_count))
+ continue;
+
+ /*
+ * Checks I_FREEING and I_WILL_FREE to protect against a race
+ * condition when release_inode() just called iput(), which
+ * could lead to a NULL dereference of inode->security or a
+ * second call to iput() for the same Landlock object. Also
+ * checks I_NEW because such inode cannot be tied to an object.
+ */
+ spin_lock(&inode->i_lock);
+ if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
+ spin_unlock(&inode->i_lock);
+ continue;
+ }
This (and elsewhere here) seems like a lot of inode internals getting
exposed. Can any of this be repurposed into helpers? I see this test
scattered around the kernel a fair bit:
$ git grep I_FREEING | grep I_WILL_FREE | grep I_NEW | wc -l
9
Dealing with the filesystem is complex. Some helpers could probably be
added, but with a series dedicated to the filesystem. I can work on that
once this series is merged.
quoted
+static inline u32 get_mode_access(const umode_t mode)
+{
+ switch (mode & S_IFMT) {
+ case S_IFLNK:
+ return LANDLOCK_ACCESS_FS_MAKE_SYM;
+ case 0:
+ /* A zero mode translates to S_IFREG. */
+ case S_IFREG:
+ return LANDLOCK_ACCESS_FS_MAKE_REG;
+ case S_IFDIR:
+ return LANDLOCK_ACCESS_FS_MAKE_DIR;
+ case S_IFCHR:
+ return LANDLOCK_ACCESS_FS_MAKE_CHAR;
+ case S_IFBLK:
+ return LANDLOCK_ACCESS_FS_MAKE_BLOCK;
+ case S_IFIFO:
+ return LANDLOCK_ACCESS_FS_MAKE_FIFO;
+ case S_IFSOCK:
+ return LANDLOCK_ACCESS_FS_MAKE_SOCK;
+ default:
+ WARN_ON_ONCE(1);
+ return 0;
+ }
I'm assuming this won't be reachable from userspace.
The locking and inode semantics are pretty complex, but since, again,
it's got significant test and syzkaller coverage, it looks good to me.
With the inode helper cleanup:
Reviewed-by: Kees Cook <redacted>
On Tue, Mar 16, 2021 at 09:42:48PM +0100, Mickaël Salaün wrote:
quoted
From: Mickaël Salaün <redacted>
These 3 system calls are designed to be used by unprivileged processes
to sandbox themselves:
* landlock_create_ruleset(2): Creates a ruleset and returns its file
descriptor.
* landlock_add_rule(2): Adds a rule (e.g. file hierarchy access) to a
ruleset, identified by the dedicated file descriptor.
* landlock_restrict_self(2): Enforces a ruleset on the calling thread
and its future children (similar to seccomp). This syscall has the
same usage restrictions as seccomp(2): the caller must have the
no_new_privs attribute set or have CAP_SYS_ADMIN in the current user
namespace.
All these syscalls have a "flags" argument (not currently used) to
enable extensibility.
For the new-style extensible syscalls, you want only a "size" argument;
"flags" should be within the argument structure.
Not necessarily, we should avoid complexity as much as possible. A flag
argument is simple, and extensible arguments should be avoided as much
as possible. Here it is only used for an extensible list of properties
that can't be expressed otherwise.
(And to this end, why 3 syscalls instead of 1, if you're going to use
extensibility anyway?)
+/**
+ * copy_min_struct_from_user - Safe future-proof argument copying
+ *
+ * Extend copy_struct_from_user() to check for consistent user buffer.
+ *
+ * @dst: Kernel space pointer or NULL.
+ * @ksize: Actual size of the data pointed to by @dst.
+ * @ksize_min: Minimal required size to be copied.
+ * @src: User space pointer or NULL.
+ * @usize: (Alleged) size of the data pointed to by @src.
+ */
+static __always_inline int copy_min_struct_from_user(void *const dst,
+ const size_t ksize, const size_t ksize_min,
+ const void __user *const src, const size_t usize)
+{
+ /* Checks buffer inconsistencies. */
+ BUILD_BUG_ON(!dst);
+ if (!src)
+ return -EFAULT;
+
+ /* Checks size ranges. */
+ BUILD_BUG_ON(ksize <= 0);
+ BUILD_BUG_ON(ksize < ksize_min);
+ if (usize < ksize_min)
+ return -EINVAL;
+ if (usize > PAGE_SIZE)
+ return -E2BIG;
+
+ /* Copies user buffer and fills with zeros. */
+ return copy_struct_from_user(dst, ksize, src, usize);
+}
I still wish this was built into copy_struct_from_user(). :) But yes,
this appears to be the way to protect one's self when using
copy_struct_from_user().
quoted
+/**
+ * sys_landlock_create_ruleset - Create a new ruleset
+ *
+ * @attr: Pointer to a &struct landlock_ruleset_attr identifying the scope of
+ * the new ruleset.
+ * @size: Size of the pointed &struct landlock_ruleset_attr (needed for
+ * backward and forward compatibility).
+ * @flags: Must be 0.
+ *
+ * This system call enables to create a new Landlock ruleset, and returns the
+ * related file descriptor on success.
+ *
+ * Possible returned errors are:
+ *
+ * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
+ * - EINVAL: @flags is not 0, or unknown access, or too small @size;
+ * - E2BIG or EFAULT: @attr or @size inconsistencies;
+ * - ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
+ */
+SYSCALL_DEFINE3(landlock_create_ruleset,
+ const struct landlock_ruleset_attr __user *const, attr,
+ const size_t, size, const __u32, flags)
+{
+ struct landlock_ruleset_attr ruleset_attr;
+ struct landlock_ruleset *ruleset;
+ int err, ruleset_fd;
+
+ /* Build-time checks. */
+ build_check_abi();
+
+ if (!landlock_initialized)
+ return -EOPNOTSUPP;
+
+ /* No flag for now. */
+ if (flags)
+ return -EINVAL;
+
+ /* Copies raw user space buffer. */
+ err = copy_min_struct_from_user(&ruleset_attr, sizeof(ruleset_attr),
+ offsetofend(typeof(ruleset_attr), handled_access_fs),
The use of offsetofend() here appears to be kind of the "V1", "V2", ...
sizes used in other extensible syscall implementations?
ruleset_attr is an extensible argument.
quoted
+ attr, size);
+ if (err)
+ return err;
+
+ /* Checks content (and 32-bits cast). */
+ if ((ruleset_attr.handled_access_fs | LANDLOCK_MASK_ACCESS_FS) !=
+ LANDLOCK_MASK_ACCESS_FS)
+ return -EINVAL;
+
+ /* Checks arguments and transforms to kernel struct. */
+ ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs);
+ if (IS_ERR(ruleset))
+ return PTR_ERR(ruleset);
+
+ /* Creates anonymous FD referring to the ruleset. */
+ ruleset_fd = anon_inode_getfd("landlock-ruleset", &ruleset_fops,
+ ruleset, O_RDWR | O_CLOEXEC);
+ if (ruleset_fd < 0)
+ landlock_put_ruleset(ruleset);
+ return ruleset_fd;
+}
+
+/*
+ * Returns an owned ruleset from a FD. It is thus needed to call
+ * landlock_put_ruleset() on the return value.
+ */
+static struct landlock_ruleset *get_ruleset_from_fd(const int fd,
+ const fmode_t mode)
+{
+ struct fd ruleset_f;
+ struct landlock_ruleset *ruleset;
+
+ ruleset_f = fdget(fd);
+ if (!ruleset_f.file)
+ return ERR_PTR(-EBADF);
+
+ /* Checks FD type and access right. */
+ if (ruleset_f.file->f_op != &ruleset_fops) {
+ ruleset = ERR_PTR(-EBADFD);
+ goto out_fdput;
+ }
+ if (!(ruleset_f.file->f_mode & mode)) {
+ ruleset = ERR_PTR(-EPERM);
+ goto out_fdput;
+ }
+ ruleset = ruleset_f.file->private_data;
+ if (WARN_ON_ONCE(ruleset->num_layers != 1)) {
+ ruleset = ERR_PTR(-EINVAL);
+ goto out_fdput;
+ }
+ landlock_get_ruleset(ruleset);
+
+out_fdput:
+ fdput(ruleset_f);
+ return ruleset;
+}
+
+/* Path handling */
+
+/*
+ * @path: Must call put_path(@path) after the call if it succeeded.
+ */
+static int get_path_from_fd(const s32 fd, struct path *const path)
+{
+ struct fd f;
+ int err = 0;
+
+ BUILD_BUG_ON(!__same_type(fd,
+ ((struct landlock_path_beneath_attr *)NULL)->parent_fd));
+
+ /* Handles O_PATH. */
+ f = fdget_raw(fd);
+ if (!f.file)
+ return -EBADF;
+ /*
+ * Only allows O_PATH file descriptor: enables to restrict ambient
+ * filesystem access without requiring to open and risk leaking or
+ * misusing a file descriptor. Forbids ruleset FDs, internal
+ * filesystems (e.g. nsfs), including pseudo filesystems that will
+ * never be mountable (e.g. sockfs, pipefs).
+ */
+ if (!(f.file->f_mode & FMODE_PATH) ||
+ (f.file->f_op == &ruleset_fops) ||
+ (f.file->f_path.mnt->mnt_flags & MNT_INTERNAL) ||
+ (f.file->f_path.dentry->d_sb->s_flags & SB_NOUSER) ||
+ d_is_negative(f.file->f_path.dentry) ||
+ IS_PRIVATE(d_backing_inode(f.file->f_path.dentry))) {
+ err = -EBADFD;
+ goto out_fdput;
+ }
+ *path = f.file->f_path;
+ path_get(path);
+
+out_fdput:
+ fdput(f);
+ return err;
+}
+
+/**
+ * sys_landlock_add_rule - Add a new rule to a ruleset
+ *
+ * @ruleset_fd: File descriptor tied to the ruleset that should be extended
+ * with the new rule.
+ * @rule_type: Identify the structure type pointed to by @rule_attr (only
+ * LANDLOCK_RULE_PATH_BENEATH for now).
+ * @rule_attr: Pointer to a rule (only of type &struct
+ * landlock_path_beneath_attr for now).
+ * @flags: Must be 0.
+ *
+ * This system call enables to define a new rule and add it to an existing
+ * ruleset.
+ *
+ * Possible returned errors are:
+ *
+ * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
+ * - EINVAL: @flags is not 0, or inconsistent access in the rule (i.e.
+ * &landlock_path_beneath_attr.allowed_access is not a subset of the rule's
+ * accesses);
+ * - ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access);
+ * - EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
+ * member of @rule_attr is not a file descriptor as expected;
+ * - EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
+ * @rule_attr is not the expected file descriptor type (e.g. file open
+ * without O_PATH);
+ * - EPERM: @ruleset_fd has no write access to the underlying ruleset;
+ * - EFAULT: @rule_attr inconsistency.
+ */
+SYSCALL_DEFINE4(landlock_add_rule,
+ const int, ruleset_fd, const enum landlock_rule_type, rule_type,
+ const void __user *const, rule_attr, const __u32, flags)
+{
If this is an extensible syscall, I'd expect a structure holding
rule_type, rule_attr, and flags.
This does not use an extensible argument. rule_type specifies a type and
rule_attr specifies the content of this type. This is simpler and
sufficient.
quoted
+ struct landlock_path_beneath_attr path_beneath_attr;
+ struct path path;
+ struct landlock_ruleset *ruleset;
+ int res, err;
+
+ if (!landlock_initialized)
+ return -EOPNOTSUPP;
+
+ /* No flag for now. */
+ if (flags)
+ return -EINVAL;
+
+ if (rule_type != LANDLOCK_RULE_PATH_BENEATH)
+ return -EINVAL;
+
+ /* Copies raw user space buffer, only one type for now. */
+ res = copy_from_user(&path_beneath_attr, rule_attr,
+ sizeof(path_beneath_attr));
+ if (res)
+ return -EFAULT;
+
+ /* Gets and checks the ruleset. */
+ ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_WRITE);
+ if (IS_ERR(ruleset))
+ return PTR_ERR(ruleset);
+
+ /*
+ * Informs about useless rule: empty allowed_access (i.e. deny rules)
+ * are ignored in path walks.
+ */
+ if (!path_beneath_attr.allowed_access) {
+ err = -ENOMSG;
+ goto out_put_ruleset;
+ }
+ /*
+ * Checks that allowed_access matches the @ruleset constraints
+ * (ruleset->fs_access_masks[0] is automatically upgraded to 64-bits).
+ */
+ if ((path_beneath_attr.allowed_access | ruleset->fs_access_masks[0]) !=
+ ruleset->fs_access_masks[0]) {
+ err = -EINVAL;
+ goto out_put_ruleset;
+ }
+
+ /* Gets and checks the new rule. */
+ err = get_path_from_fd(path_beneath_attr.parent_fd, &path);
+ if (err)
+ goto out_put_ruleset;
+
+ /* Imports the new rule. */
+ err = landlock_append_fs_rule(ruleset, &path,
+ path_beneath_attr.allowed_access);
+ path_put(&path);
+
+out_put_ruleset:
+ landlock_put_ruleset(ruleset);
+ return err;
+}
+
+/* Enforcement */
+
+/**
+ * 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.
+ *
+ * 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
+ * namespace or is running with no_new_privs. This avoids scenarios where
+ * unprivileged tasks can affect the behavior of privileged children.
+ *
+ * Possible returned errors are:
+ *
+ * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
+ * - EINVAL: @flags is not 0.
+ * - EBADF: @ruleset_fd is not a file descriptor for the current thread;
+ * - EBADFD: @ruleset_fd is not a ruleset file descriptor;
+ * - EPERM: @ruleset_fd has no read access to the underlying ruleset, or the
+ * current thread is not running with no_new_privs, or it doesn't have
+ * CAP_SYS_ADMIN in its namespace.
+ * - E2BIG: The maximum number of stacked rulesets is reached for the current
+ * thread.
+ */
+SYSCALL_DEFINE2(landlock_restrict_self,
+ const int, ruleset_fd, const __u32, flags)
+{
Same observation about new style syscalls.
This design is simpler.
quoted
+ struct landlock_ruleset *new_dom, *ruleset;
+ struct cred *new_cred;
+ struct landlock_cred_security *new_llcred;
+ int err;
+
+ if (!landlock_initialized)
+ return -EOPNOTSUPP;
+
+ /* No flag for now. */
+ if (flags)
+ return -EINVAL;
+
+ /*
+ * Similar checks as for seccomp(2), except that an -EPERM may be
+ * returned.
+ */
+ if (!task_no_new_privs(current) &&
+ !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN))
+ return -EPERM;
+
+ /* Gets and checks the ruleset. */
+ ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_READ);
+ if (IS_ERR(ruleset))
+ return PTR_ERR(ruleset);
+
+ /* Prepares new credentials. */
+ new_cred = prepare_creds();
+ if (!new_cred) {
+ err = -ENOMEM;
+ goto out_put_ruleset;
+ }
+ new_llcred = landlock_cred(new_cred);
+
+ /*
+ * There is no possible race condition while copying and manipulating
+ * the current credentials because they are dedicated per thread.
+ */
+ new_dom = landlock_merge_ruleset(new_llcred->domain, ruleset);
+ if (IS_ERR(new_dom)) {
+ err = PTR_ERR(new_dom);
+ goto out_put_creds;
+ }
+
+ /* Replaces the old (prepared) domain. */
+ landlock_put_ruleset(new_llcred->domain);
+ new_llcred->domain = new_dom;
+
+ landlock_put_ruleset(ruleset);
+ return commit_creds(new_cred);
+
+out_put_creds:
+ abort_creds(new_cred);
+
+out_put_ruleset:
+ landlock_put_ruleset(ruleset);
+ return err;
+}
--
2.30.2
On Tue, Mar 16, 2021 at 9:43 PM Mickaël Salaün [off-list ref] wrote:
A Landlock ruleset is mainly a red-black tree with Landlock rules as
nodes. This enables quick update and lookup to match a requested
access, e.g. to a file. A ruleset is usable through a dedicated file
descriptor (cf. following commit implementing syscalls) which enables a
process to create and populate a ruleset with new rules.
A domain is a ruleset tied to a set of processes. This group of rules
defines the security policy enforced on these processes and their future
children. A domain can transition to a new domain which is the
intersection of all its constraints and those of a ruleset provided by
the current process. This modification only impact the current process.
This means that a process can only gain more constraints (i.e. lose
accesses) over time.
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Kees Cook <redacted>
Signed-off-by: Mickaël Salaün <redacted>
Acked-by: Serge Hallyn <serge@hallyn.com>
Link: https://lore.kernel.org/r/20210316204252.427806-3-mic@digikod.net
On Tue, Mar 16, 2021 at 9:43 PM Mickaël Salaün [off-list ref] wrote:
Using Landlock objects and ruleset, it is possible to tag inodes
according to a process's domain.
[...]
+static void release_inode(struct landlock_object *const object)
+ __releases(object->lock)
+{
+ struct inode *const inode = object->underobj;
+ struct super_block *sb;
+
+ if (!inode) {
+ spin_unlock(&object->lock);
+ return;
+ }
+
+ /*
+ * Protects against concurrent use by hook_sb_delete() of the reference
+ * to the underlying inode.
+ */
+ object->underobj = NULL;
+ /*
+ * Makes sure that if the filesystem is concurrently unmounted,
+ * hook_sb_delete() will wait for us to finish iput().
+ */
+ sb = inode->i_sb;
+ atomic_long_inc(&landlock_superblock(sb)->inode_refs);
+ spin_unlock(&object->lock);
+ /*
+ * Because object->underobj was not NULL, hook_sb_delete() and
+ * get_inode_object() guarantee that it is safe to reset
+ * landlock_inode(inode)->object while it is not NULL. It is therefore
+ * not necessary to lock inode->i_lock.
+ */
+ rcu_assign_pointer(landlock_inode(inode)->object, NULL);
+ /*
+ * Now, new rules can safely be tied to @inode with get_inode_object().
+ */
+
+ iput(inode);
+ if (atomic_long_dec_and_test(&landlock_superblock(sb)->inode_refs))
+ wake_up_var(&landlock_superblock(sb)->inode_refs);
+}
[...]
+static struct landlock_object *get_inode_object(struct inode *const inode)
+{
+ struct landlock_object *object, *new_object;
+ struct landlock_inode_security *inode_sec = landlock_inode(inode);
+
+ rcu_read_lock();
+retry:
+ object = rcu_dereference(inode_sec->object);
+ if (object) {
+ if (likely(refcount_inc_not_zero(&object->usage))) {
+ rcu_read_unlock();
+ return object;
+ }
+ /*
+ * We are racing with release_inode(), the object is going
+ * away. Wait for release_inode(), then retry.
+ */
+ spin_lock(&object->lock);
+ spin_unlock(&object->lock);
+ goto retry;
+ }
+ rcu_read_unlock();
+
+ /*
+ * If there is no object tied to @inode, then create a new one (without
+ * holding any locks).
+ */
+ new_object = landlock_create_object(&landlock_fs_underops, inode);
+ if (IS_ERR(new_object))
+ return new_object;
+
+ /* Protects against concurrent get_inode_object() calls. */
+ spin_lock(&inode->i_lock);
+ object = rcu_dereference_protected(inode_sec->object,
+ lockdep_is_held(&inode->i_lock));
rcu_dereference_protected() requires that inode_sec->object is not
concurrently changed, but I think another thread could call
get_inode_object() while we're in landlock_create_object(), and then
we could race with the NULL write in release_inode() here? (It
wouldn't actually be a UAF though because we're not actually accessing
`object` here.) Or am I missing a lock that prevents this?
In v28 this wasn't an issue because release_inode() was holding
inode->i_lock (and object->lock) during the NULL store; but in v29 and
this version the NULL store in release_inode() moved out of the locked
region. I think you could just move the NULL store in release_inode()
back up (and maybe add a comment explaining the locking rules for
landlock_inode(...)->object)?
(Or alternatively you could use rcu_dereference_raw() with a comment
explaining that the read pointer is only used to check for NULL-ness,
and that it is guaranteed that the pointer can't change if it is NULL
and we're holding the lock. But that'd be needlessly complicated, I
think.)
+ if (unlikely(object)) {
+ /* Someone else just created the object, bail out and retry. */
+ spin_unlock(&inode->i_lock);
+ kfree(new_object);
+
+ rcu_read_lock();
+ goto retry;
+ }
+
+ rcu_assign_pointer(inode_sec->object, new_object);
+ /*
+ * @inode will be released by hook_sb_delete() on its superblock
+ * shutdown.
+ */
+ ihold(inode);
+ spin_unlock(&inode->i_lock);
+ return new_object;
+}
On Tue, Mar 16, 2021 at 9:43 PM Mickaël Salaün [off-list ref] wrote:
quoted
Using Landlock objects and ruleset, it is possible to tag inodes
according to a process's domain.
[...]
quoted
+static void release_inode(struct landlock_object *const object)
+ __releases(object->lock)
+{
+ struct inode *const inode = object->underobj;
+ struct super_block *sb;
+
+ if (!inode) {
+ spin_unlock(&object->lock);
+ return;
+ }
+
+ /*
+ * Protects against concurrent use by hook_sb_delete() of the reference
+ * to the underlying inode.
+ */
+ object->underobj = NULL;
+ /*
+ * Makes sure that if the filesystem is concurrently unmounted,
+ * hook_sb_delete() will wait for us to finish iput().
+ */
+ sb = inode->i_sb;
+ atomic_long_inc(&landlock_superblock(sb)->inode_refs);
+ spin_unlock(&object->lock);
+ /*
+ * Because object->underobj was not NULL, hook_sb_delete() and
+ * get_inode_object() guarantee that it is safe to reset
+ * landlock_inode(inode)->object while it is not NULL. It is therefore
+ * not necessary to lock inode->i_lock.
+ */
+ rcu_assign_pointer(landlock_inode(inode)->object, NULL);
+ /*
+ * Now, new rules can safely be tied to @inode with get_inode_object().
+ */
+
+ iput(inode);
+ if (atomic_long_dec_and_test(&landlock_superblock(sb)->inode_refs))
+ wake_up_var(&landlock_superblock(sb)->inode_refs);
+}
[...]
quoted
+static struct landlock_object *get_inode_object(struct inode *const inode)
+{
+ struct landlock_object *object, *new_object;
+ struct landlock_inode_security *inode_sec = landlock_inode(inode);
+
+ rcu_read_lock();
+retry:
+ object = rcu_dereference(inode_sec->object);
+ if (object) {
+ if (likely(refcount_inc_not_zero(&object->usage))) {
+ rcu_read_unlock();
+ return object;
+ }
+ /*
+ * We are racing with release_inode(), the object is going
+ * away. Wait for release_inode(), then retry.
+ */
+ spin_lock(&object->lock);
+ spin_unlock(&object->lock);
+ goto retry;
+ }
+ rcu_read_unlock();
+
+ /*
+ * If there is no object tied to @inode, then create a new one (without
+ * holding any locks).
+ */
+ new_object = landlock_create_object(&landlock_fs_underops, inode);
+ if (IS_ERR(new_object))
+ return new_object;
+
+ /* Protects against concurrent get_inode_object() calls. */
+ spin_lock(&inode->i_lock);
+ object = rcu_dereference_protected(inode_sec->object,
+ lockdep_is_held(&inode->i_lock));
rcu_dereference_protected() requires that inode_sec->object is not
concurrently changed, but I think another thread could call
get_inode_object() while we're in landlock_create_object(), and then
we could race with the NULL write in release_inode() here? (It
wouldn't actually be a UAF though because we're not actually accessing
`object` here.) Or am I missing a lock that prevents this?
In v28 this wasn't an issue because release_inode() was holding
inode->i_lock (and object->lock) during the NULL store; but in v29 and
this version the NULL store in release_inode() moved out of the locked
region. I think you could just move the NULL store in release_inode()
back up (and maybe add a comment explaining the locking rules for
landlock_inode(...)->object)?
(Or alternatively you could use rcu_dereference_raw() with a comment
explaining that the read pointer is only used to check for NULL-ness,
and that it is guaranteed that the pointer can't change if it is NULL
and we're holding the lock. But that'd be needlessly complicated, I
think.)
To reach rcu_assign_pointer(landlock_inode(inode)->object, NULL) in
release_inode() or in hook_sb_delete(), the
landlock_inode(inode)->object need to be non-NULL, which implies that a
call to get_inode_object(inode) either "retry" (because release_inode is
only called by landlock_put_object, which set object->usage to 0) until
it creates a new object, or reuses the existing referenced object (and
increments object->usage). The worse case would be if
get_inode_object(inode) is called just before the
rcu_assign_pointer(landlock_inode(inode)->object, NULL) from
hook_sb_delete(), which would result in an object with a NULL underobj,
which is the expected behavior (and checked by release_inode).
The line rcu_assign_pointer(inode_sec->object, new_object) from
get_inode_object() can only be reached if the underlying inode doesn't
reference an object, in which case hook_sb_delete() will not reach the
rcu_assign_pointer(landlock_inode(inode)->object, NULL) line for this
same inode.
This works because get_inode_object(inode) is mutually exclusive to
itself with the same inode (i.e. an inode can only point to an object
that references this same inode).
I tried to explain this with the comment "Protects against concurrent
get_inode_object() calls" in get_inode_object(), and the comments just
before both rcu_assign_pointer(landlock_inode(inode)->object, NULL).
quoted
+ if (unlikely(object)) {
+ /* Someone else just created the object, bail out and retry. */
+ spin_unlock(&inode->i_lock);
+ kfree(new_object);
+
+ rcu_read_lock();
+ goto retry;
+ }
+
+ rcu_assign_pointer(inode_sec->object, new_object);
+ /*
+ * @inode will be released by hook_sb_delete() on its superblock
+ * shutdown.
+ */
+ ihold(inode);
+ spin_unlock(&inode->i_lock);
+ return new_object;
+}
On Tue, Mar 23, 2021 at 4:54 PM Mickaël Salaün [off-list ref] wrote:
On 23/03/2021 01:13, Jann Horn wrote:
quoted
On Tue, Mar 16, 2021 at 9:43 PM Mickaël Salaün [off-list ref] wrote:
quoted
Using Landlock objects and ruleset, it is possible to tag inodes
according to a process's domain.
[...]
quoted
+static void release_inode(struct landlock_object *const object)
+ __releases(object->lock)
+{
+ struct inode *const inode = object->underobj;
+ struct super_block *sb;
+
+ if (!inode) {
+ spin_unlock(&object->lock);
+ return;
+ }
+
+ /*
+ * Protects against concurrent use by hook_sb_delete() of the reference
+ * to the underlying inode.
+ */
+ object->underobj = NULL;
+ /*
+ * Makes sure that if the filesystem is concurrently unmounted,
+ * hook_sb_delete() will wait for us to finish iput().
+ */
+ sb = inode->i_sb;
+ atomic_long_inc(&landlock_superblock(sb)->inode_refs);
+ spin_unlock(&object->lock);
+ /*
+ * Because object->underobj was not NULL, hook_sb_delete() and
+ * get_inode_object() guarantee that it is safe to reset
+ * landlock_inode(inode)->object while it is not NULL. It is therefore
+ * not necessary to lock inode->i_lock.
+ */
+ rcu_assign_pointer(landlock_inode(inode)->object, NULL);
+ /*
+ * Now, new rules can safely be tied to @inode with get_inode_object().
+ */
+
+ iput(inode);
+ if (atomic_long_dec_and_test(&landlock_superblock(sb)->inode_refs))
+ wake_up_var(&landlock_superblock(sb)->inode_refs);
+}
[...]
quoted
+static struct landlock_object *get_inode_object(struct inode *const inode)
+{
+ struct landlock_object *object, *new_object;
+ struct landlock_inode_security *inode_sec = landlock_inode(inode);
+
+ rcu_read_lock();
+retry:
+ object = rcu_dereference(inode_sec->object);
+ if (object) {
+ if (likely(refcount_inc_not_zero(&object->usage))) {
+ rcu_read_unlock();
+ return object;
+ }
+ /*
+ * We are racing with release_inode(), the object is going
+ * away. Wait for release_inode(), then retry.
+ */
+ spin_lock(&object->lock);
+ spin_unlock(&object->lock);
+ goto retry;
+ }
+ rcu_read_unlock();
+
+ /*
+ * If there is no object tied to @inode, then create a new one (without
+ * holding any locks).
+ */
+ new_object = landlock_create_object(&landlock_fs_underops, inode);
+ if (IS_ERR(new_object))
+ return new_object;
+
+ /* Protects against concurrent get_inode_object() calls. */
+ spin_lock(&inode->i_lock);
+ object = rcu_dereference_protected(inode_sec->object,
+ lockdep_is_held(&inode->i_lock));
rcu_dereference_protected() requires that inode_sec->object is not
concurrently changed, but I think another thread could call
get_inode_object() while we're in landlock_create_object(), and then
we could race with the NULL write in release_inode() here? (It
wouldn't actually be a UAF though because we're not actually accessing
`object` here.) Or am I missing a lock that prevents this?
In v28 this wasn't an issue because release_inode() was holding
inode->i_lock (and object->lock) during the NULL store; but in v29 and
this version the NULL store in release_inode() moved out of the locked
region. I think you could just move the NULL store in release_inode()
back up (and maybe add a comment explaining the locking rules for
landlock_inode(...)->object)?
(Or alternatively you could use rcu_dereference_raw() with a comment
explaining that the read pointer is only used to check for NULL-ness,
and that it is guaranteed that the pointer can't change if it is NULL
and we're holding the lock. But that'd be needlessly complicated, I
think.)
To reach rcu_assign_pointer(landlock_inode(inode)->object, NULL) in
release_inode() or in hook_sb_delete(), the
landlock_inode(inode)->object need to be non-NULL,
Yes.
which implies that a
call to get_inode_object(inode) either "retry" (because release_inode is
only called by landlock_put_object, which set object->usage to 0) until
it creates a new object, or reuses the existing referenced object (and
increments object->usage).
But it can be that landlock_inode(inode)->object only becomes non-NULL
after get_inode_object() has checked
rcu_dereference(inode_sec->object).
The worse case would be if
get_inode_object(inode) is called just before the
rcu_assign_pointer(landlock_inode(inode)->object, NULL) from
hook_sb_delete(), which would result in an object with a NULL underobj,
which is the expected behavior (and checked by release_inode).
The scenario I'm talking about doesn't involve hook_sb_delete().
The line rcu_assign_pointer(inode_sec->object, new_object) from
get_inode_object() can only be reached if the underlying inode doesn't
reference an object,
Yes.
in which case hook_sb_delete() will not reach the
rcu_assign_pointer(landlock_inode(inode)->object, NULL) line for this
same inode.
This works because get_inode_object(inode) is mutually exclusive to
itself with the same inode (i.e. an inode can only point to an object
that references this same inode).
To clarify: You can concurrently call get_inode_object() multiple
times on the same inode, right? There are no locks held on entry to
that function.
I tried to explain this with the comment "Protects against concurrent
get_inode_object() calls" in get_inode_object(), and the comments just
before both rcu_assign_pointer(landlock_inode(inode)->object, NULL).
The scenario I'm talking about is:
Initially the inode does not have an associated landlock_object. There
are two threads A and B. Thread A is going to execute
get_inode_object(). Thread B is going to execute get_inode_object()
followed immediately by landlock_put_object().
thread A: enters get_inode_object()
thread A: rcu_dereference(inode_sec->object) returns NULL
thread A: enters landlock_create_object()
thread B: enters get_inode_object()
thread B: rcu_dereference(inode_sec->object) returns NULL
thread B: calls landlock_create_object()
thread B: sets inode_sec->object while holding inode->i_lock
thread B: leaves get_inode_object()
thread B: enters landlock_put_object()
thread B: object->usage drops to 0, object->lock is taken
thread B: calls release_inode()
thread B: drops object->lock
thread A: returns from landlock_create_object()
thread A: takes inode->i_lock
At this point, thread B will run:
rcu_assign_pointer(landlock_inode(inode)->object, NULL);
while thread A runs:
rcu_dereference_protected(inode_sec->object,
lockdep_is_held(&inode->i_lock));
meaning there is a (theoretical) data race, since
rcu_dereference_protected() doesn't use READ_ONCE().
quoted
quoted
+ if (unlikely(object)) {
+ /* Someone else just created the object, bail out and retry. */
+ spin_unlock(&inode->i_lock);
+ kfree(new_object);
+
+ rcu_read_lock();
+ goto retry;
+ }
+
+ rcu_assign_pointer(inode_sec->object, new_object);
+ /*
+ * @inode will be released by hook_sb_delete() on its superblock
+ * shutdown.
+ */
+ ihold(inode);
+ spin_unlock(&inode->i_lock);
+ return new_object;
+}
On Tue, Mar 23, 2021 at 4:54 PM Mickaël Salaün [off-list ref] wrote:
quoted
On 23/03/2021 01:13, Jann Horn wrote:
quoted
On Tue, Mar 16, 2021 at 9:43 PM Mickaël Salaün [off-list ref] wrote:
quoted
Using Landlock objects and ruleset, it is possible to tag inodes
according to a process's domain.
[...]
quoted
+static void release_inode(struct landlock_object *const object)
+ __releases(object->lock)
+{
+ struct inode *const inode = object->underobj;
+ struct super_block *sb;
+
+ if (!inode) {
+ spin_unlock(&object->lock);
+ return;
+ }
+
+ /*
+ * Protects against concurrent use by hook_sb_delete() of the reference
+ * to the underlying inode.
+ */
+ object->underobj = NULL;
+ /*
+ * Makes sure that if the filesystem is concurrently unmounted,
+ * hook_sb_delete() will wait for us to finish iput().
+ */
+ sb = inode->i_sb;
+ atomic_long_inc(&landlock_superblock(sb)->inode_refs);
+ spin_unlock(&object->lock);
+ /*
+ * Because object->underobj was not NULL, hook_sb_delete() and
+ * get_inode_object() guarantee that it is safe to reset
+ * landlock_inode(inode)->object while it is not NULL. It is therefore
+ * not necessary to lock inode->i_lock.
+ */
+ rcu_assign_pointer(landlock_inode(inode)->object, NULL);
+ /*
+ * Now, new rules can safely be tied to @inode with get_inode_object().
+ */
+
+ iput(inode);
+ if (atomic_long_dec_and_test(&landlock_superblock(sb)->inode_refs))
+ wake_up_var(&landlock_superblock(sb)->inode_refs);
+}
[...]
quoted
+static struct landlock_object *get_inode_object(struct inode *const inode)
+{
+ struct landlock_object *object, *new_object;
+ struct landlock_inode_security *inode_sec = landlock_inode(inode);
+
+ rcu_read_lock();
+retry:
+ object = rcu_dereference(inode_sec->object);
+ if (object) {
+ if (likely(refcount_inc_not_zero(&object->usage))) {
+ rcu_read_unlock();
+ return object;
+ }
+ /*
+ * We are racing with release_inode(), the object is going
+ * away. Wait for release_inode(), then retry.
+ */
+ spin_lock(&object->lock);
+ spin_unlock(&object->lock);
+ goto retry;
+ }
+ rcu_read_unlock();
+
+ /*
+ * If there is no object tied to @inode, then create a new one (without
+ * holding any locks).
+ */
+ new_object = landlock_create_object(&landlock_fs_underops, inode);
+ if (IS_ERR(new_object))
+ return new_object;
+
+ /* Protects against concurrent get_inode_object() calls. */
+ spin_lock(&inode->i_lock);
+ object = rcu_dereference_protected(inode_sec->object,
+ lockdep_is_held(&inode->i_lock));
rcu_dereference_protected() requires that inode_sec->object is not
concurrently changed, but I think another thread could call
get_inode_object() while we're in landlock_create_object(), and then
we could race with the NULL write in release_inode() here? (It
wouldn't actually be a UAF though because we're not actually accessing
`object` here.) Or am I missing a lock that prevents this?
In v28 this wasn't an issue because release_inode() was holding
inode->i_lock (and object->lock) during the NULL store; but in v29 and
this version the NULL store in release_inode() moved out of the locked
region. I think you could just move the NULL store in release_inode()
back up (and maybe add a comment explaining the locking rules for
landlock_inode(...)->object)?
(Or alternatively you could use rcu_dereference_raw() with a comment
explaining that the read pointer is only used to check for NULL-ness,
and that it is guaranteed that the pointer can't change if it is NULL
and we're holding the lock. But that'd be needlessly complicated, I
think.)
To reach rcu_assign_pointer(landlock_inode(inode)->object, NULL) in
release_inode() or in hook_sb_delete(), the
landlock_inode(inode)->object need to be non-NULL,
Yes.
quoted
which implies that a
call to get_inode_object(inode) either "retry" (because release_inode is
only called by landlock_put_object, which set object->usage to 0) until
it creates a new object, or reuses the existing referenced object (and
increments object->usage).
But it can be that landlock_inode(inode)->object only becomes non-NULL
after get_inode_object() has checked
rcu_dereference(inode_sec->object).
quoted
The worse case would be if
get_inode_object(inode) is called just before the
rcu_assign_pointer(landlock_inode(inode)->object, NULL) from
hook_sb_delete(), which would result in an object with a NULL underobj,
which is the expected behavior (and checked by release_inode).
The scenario I'm talking about doesn't involve hook_sb_delete().
quoted
The line rcu_assign_pointer(inode_sec->object, new_object) from
get_inode_object() can only be reached if the underlying inode doesn't
reference an object,
Yes.
quoted
in which case hook_sb_delete() will not reach the
rcu_assign_pointer(landlock_inode(inode)->object, NULL) line for this
same inode.
This works because get_inode_object(inode) is mutually exclusive to
itself with the same inode (i.e. an inode can only point to an object
that references this same inode).
To clarify: You can concurrently call get_inode_object() multiple
times on the same inode, right? There are no locks held on entry to
that function.
quoted
I tried to explain this with the comment "Protects against concurrent
get_inode_object() calls" in get_inode_object(), and the comments just
before both rcu_assign_pointer(landlock_inode(inode)->object, NULL).
The scenario I'm talking about is:
Initially the inode does not have an associated landlock_object. There
are two threads A and B. Thread A is going to execute
get_inode_object(). Thread B is going to execute get_inode_object()
followed immediately by landlock_put_object().
thread A: enters get_inode_object()
thread A: rcu_dereference(inode_sec->object) returns NULL
thread A: enters landlock_create_object()
thread B: enters get_inode_object()
thread B: rcu_dereference(inode_sec->object) returns NULL
thread B: calls landlock_create_object()
thread B: sets inode_sec->object while holding inode->i_lock
thread B: leaves get_inode_object()
thread B: enters landlock_put_object()
thread B: object->usage drops to 0, object->lock is taken
thread B: calls release_inode()
thread B: drops object->lock
thread A: returns from landlock_create_object()
thread A: takes inode->i_lock
At this point, thread B will run:
rcu_assign_pointer(landlock_inode(inode)->object, NULL);
while thread A runs:
rcu_dereference_protected(inode_sec->object,
lockdep_is_held(&inode->i_lock));
meaning there is a (theoretical) data race, since
rcu_dereference_protected() doesn't use READ_ONCE().
Hum, I see, that is what I was missing. And that explain why there is
(in practice) no impact on winning the race.
I would prefer to use rcu_access_pointer() instead of
rcu_dereference_protected() to avoid pitfall, and it reflects what I was
expecting:
*get_inode_object(struct inode *const inode)
/* Protects against concurrent get_inode_object() calls. */
spin_lock(&inode->i_lock);
- object = rcu_dereference_protected(inode_sec->object,
- lockdep_is_held(&inode->i_lock));
- if (unlikely(object)) {
+ if (unlikely(rcu_access_pointer(inode_sec->object))) {
/* Someone else just created the object, bail out and
retry. */
spin_unlock(&inode->i_lock);
kfree(new_object);
But I'm not sure about your proposition to move the NULL store in
release_inode() back up. Do you mean to add back the inode lock in
release_inode() like this?
*const object)
* Makes sure that if the filesystem is concurrently unmounted,
* hook_sb_delete() will wait for us to finish iput().
*/
+ spin_lock(&inode->i_lock);
sb = inode->i_sb;
atomic_long_inc(&landlock_superblock(sb)->inode_refs);
spin_unlock(&object->lock);
- /*
- * Because object->underobj was not NULL, hook_sb_delete() and
- * get_inode_object() guarantee that it is safe to reset
- * landlock_inode(inode)->object while it is not NULL. It is therefore
- * not necessary to lock inode->i_lock.
- */
rcu_assign_pointer(landlock_inode(inode)->object, NULL);
+ spin_unlock(&inode->i_lock);
/*
* Now, new rules can safely be tied to @inode with get_inode_object().
*/
I would prefer to avoid nested locks if it is not necessary though.
quoted
quoted
quoted
+ if (unlikely(object)) {
+ /* Someone else just created the object, bail out and retry. */
+ spin_unlock(&inode->i_lock);
+ kfree(new_object);
+
+ rcu_read_lock();
+ goto retry;
+ }
+
+ rcu_assign_pointer(inode_sec->object, new_object);
+ /*
+ * @inode will be released by hook_sb_delete() on its superblock
+ * shutdown.
+ */
+ ihold(inode);
+ spin_unlock(&inode->i_lock);
+ return new_object;
+}
On Tue, Mar 16, 2021 at 09:42:52PM +0100, Mickaël Salaün wrote:
quoted
From: Mickaël Salaün <redacted>
This documentation can be built with the Sphinx framework.
Well, yes. :) Maybe describe what the documentation covers instead here.
Regardless: yay docs! This is great.
What about that?
Add a first document describing userspace API: how to define and enforce
a Landlock security policy. This is explained with a simple example.
The Landlock system calls are described with their expected behavior and
current limitations.
Another document is dedicated to kernel developers, describing guiding
principles and some important kernel structures.
This documentation can be built with the Sphinx framework.
quoted
quoted
[...]
+Bind mounts and OverlayFS
+-------------------------
+
+Landlock enables to restrict access to file hierarchies, which means that these
+access rights can be propagated with bind mounts (cf.
+:doc:`/filesystems/sharedsubtree`) but not with :doc:`/filesystems/overlayfs`.
+
+A bind mount mirrors a source file hierarchy to a destination. The destination
+hierarchy is then composed of the exact same files, on which Landlock rules can
+be tied, either via the source or the destination path. These rules restrict
+access when they are encountered on a path, which means that they can restrict
+access to multiple file hierarchies at the same time, whether these hierarchies
+are the result of bind mounts or not.
+
+An OverlayFS mount point consists of upper and lower layers. These layers are
+combined in a merge directory, result of the mount point. This merge hierarchy
+may include files from the upper and lower layers, but modifications performed
+on the merge hierarchy only reflects on the upper layer. From a Landlock
+policy point of view, each OverlayFS layers and merge hierarchies are
+standalone and contains their own set of files and directories, which is
+different from bind mounts. A policy restricting an OverlayFS layer will not
+restrict the resulted merged hierarchy, and vice versa.
Can you include some examples about what a user of landlock should do?
i.e. what are some examples of unexpected results when trying to write
policy that runs on top of overlayfs, etc?
Landlock works well with overlayfs, at least from my point of view. It
may be a bit disturbing with bind mount though, but it is the same with
other access-controls (e.g. DAC). Landlock users should just think about
file hierarchies and create their policies accordingly. A user should
not try to adapt a policy according to his/her understanding of
overlayfs, but just think about file hierarchies.
quoted
quoted
[...]
+File renaming and linking
+-------------------------
+
+Because Landlock targets unprivileged access controls, it is needed to properly
+handle composition of rules. Such property also implies rules nesting.
+Properly handling multiple layers of ruleset, each one of them able to restrict
+access to files, also implies to inherit the ruleset restrictions from a parent
+to its hierarchy. Because files are identified and restricted by their
+hierarchy, moving or linking a file from one directory to another implies to
+propagate the hierarchy constraints. To protect against privilege escalations
+through renaming or linking, and for the sack of simplicity, Landlock currently
typo: sack -> sake
Indeed
quoted
quoted
[...]
+Special filesystems
+-------------------
+
+Access to regular files and directories can be restricted by Landlock,
+according to the handled accesses of a ruleset. However, files that do not
+come from a user-visible filesystem (e.g. pipe, socket), but can still be
+accessed through /proc/self/fd/, cannot currently be restricted. Likewise,
+some special kernel filesystems such as nsfs, which can be accessed through
+/proc/self/ns/, cannot currently be restricted. For now, these kind of special
+paths are then always allowed. Future Landlock evolutions will enable to
+restrict such paths with dedicated ruleset flags.
With this series, can /proc (at the top level) be blocked? (i.e. can a
landlock user avoid the weirdness by making /proc/$pid/ unavailable?)
/proc can be blocked, but not /proc/*/ns/* because of disconnected
roots. I plan to address this.
quoted
quoted
+Ruleset layers
+--------------
+
+There is a limit of 64 layers of stacked rulesets. This can be an issue for a
+task willing to enforce a new ruleset in complement to its 64 inherited
+rulesets. Once this limit is reached, sys_landlock_restrict_self() returns
+E2BIG. It is then strongly suggested to carefully build rulesets once in the
+life of a thread, especially for applications able to launch other applications
+that may also want to sandbox themselves (e.g. shells, container managers,
+etc.).
How was this value (64) chosen?
It was first an implementation constraint to limit the memory allocated
for each rule, but it could now be increased if needed. The
implementation still uses u64 for the sake of simplicity, but we could
switch to a bitmap type.
On Tue, Mar 16, 2021 at 09:42:47PM +0100, Mickaël Salaün wrote:
quoted
From: Mickaël Salaün <redacted>
Using Landlock objects and ruleset, it is possible to tag inodes
according to a process's domain. To enable an unprivileged process to
express a file hierarchy, it first needs to open a directory (or a file)
and pass this file descriptor to the kernel through
landlock_add_rule(2). When checking if a file access request is
allowed, we walk from the requested dentry to the real root, following
the different mount layers. The access to each "tagged" inodes are
collected according to their rule layer level, and ANDed to create
access to the requested file hierarchy. This makes possible to identify
a lot of files without tagging every inodes nor modifying the
filesystem, while still following the view and understanding the user
has from the filesystem.
Add a new ARCH_EPHEMERAL_INODES for UML because it currently does not
keep the same struct inodes for the same inodes whereas these inodes are
in use.
This commit adds a minimal set of supported filesystem access-control
which doesn't enable to restrict all file-related actions. This is the
result of multiple discussions to minimize the code of Landlock to ease
review. Thanks to the Landlock design, extending this access-control
without breaking user space will not be a problem. Moreover, seccomp
filters can be used to restrict the use of syscall families which may
not be currently handled by Landlock.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jann Horn <jannh@google.com>
Cc: Jeff Dike <redacted>
Cc: Kees Cook <redacted>
Cc: Richard Weinberger <richard@nod.at>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <redacted>
Link: https://lore.kernel.org/r/20210316204252.427806-8-mic@digikod.net
[...]
+ spin_lock(&sb->s_inode_list_lock);
+ list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
+ struct landlock_object *object;
+
+ /* Only handles referenced inodes. */
+ if (!atomic_read(&inode->i_count))
+ continue;
+
+ /*
+ * Checks I_FREEING and I_WILL_FREE to protect against a race
+ * condition when release_inode() just called iput(), which
+ * could lead to a NULL dereference of inode->security or a
+ * second call to iput() for the same Landlock object. Also
+ * checks I_NEW because such inode cannot be tied to an object.
+ */
+ spin_lock(&inode->i_lock);
+ if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
+ spin_unlock(&inode->i_lock);
+ continue;
+ }
This (and elsewhere here) seems like a lot of inode internals getting
exposed. Can any of this be repurposed into helpers? I see this test
scattered around the kernel a fair bit:
$ git grep I_FREEING | grep I_WILL_FREE | grep I_NEW | wc -l
9
Dealing with the filesystem is complex. Some helpers could probably be
added, but with a series dedicated to the filesystem. I can work on that
once this series is merged.
quoted
quoted
+static inline u32 get_mode_access(const umode_t mode)
+{
+ switch (mode & S_IFMT) {
+ case S_IFLNK:
+ return LANDLOCK_ACCESS_FS_MAKE_SYM;
+ case 0:
+ /* A zero mode translates to S_IFREG. */
+ case S_IFREG:
+ return LANDLOCK_ACCESS_FS_MAKE_REG;
+ case S_IFDIR:
+ return LANDLOCK_ACCESS_FS_MAKE_DIR;
+ case S_IFCHR:
+ return LANDLOCK_ACCESS_FS_MAKE_CHAR;
+ case S_IFBLK:
+ return LANDLOCK_ACCESS_FS_MAKE_BLOCK;
+ case S_IFIFO:
+ return LANDLOCK_ACCESS_FS_MAKE_FIFO;
+ case S_IFSOCK:
+ return LANDLOCK_ACCESS_FS_MAKE_SOCK;
+ default:
+ WARN_ON_ONCE(1);
+ return 0;
+ }
I'm assuming this won't be reachable from userspace.
The locking and inode semantics are pretty complex, but since, again,
it's got significant test and syzkaller coverage, it looks good to me.
With the inode helper cleanup:
I think the inode helper would have to be in a separate patch focused on
fs/ (like all matches of your greps, except Landlock). Are you OK if I
send a patch for that once Landlock is merged?
On Tue, Mar 23, 2021 at 8:22 PM Mickaël Salaün [off-list ref] wrote:
quoted hunk
On 23/03/2021 18:49, Jann Horn wrote:
quoted
On Tue, Mar 23, 2021 at 4:54 PM Mickaël Salaün [off-list ref] wrote:
quoted
On 23/03/2021 01:13, Jann Horn wrote:
quoted
On Tue, Mar 16, 2021 at 9:43 PM Mickaël Salaün [off-list ref] wrote:
quoted
Using Landlock objects and ruleset, it is possible to tag inodes
according to a process's domain.
[...]
quoted
+static void release_inode(struct landlock_object *const object)
+ __releases(object->lock)
+{
+ struct inode *const inode = object->underobj;
+ struct super_block *sb;
+
+ if (!inode) {
+ spin_unlock(&object->lock);
+ return;
+ }
+
+ /*
+ * Protects against concurrent use by hook_sb_delete() of the reference
+ * to the underlying inode.
+ */
+ object->underobj = NULL;
+ /*
+ * Makes sure that if the filesystem is concurrently unmounted,
+ * hook_sb_delete() will wait for us to finish iput().
+ */
+ sb = inode->i_sb;
+ atomic_long_inc(&landlock_superblock(sb)->inode_refs);
+ spin_unlock(&object->lock);
+ /*
+ * Because object->underobj was not NULL, hook_sb_delete() and
+ * get_inode_object() guarantee that it is safe to reset
+ * landlock_inode(inode)->object while it is not NULL. It is therefore
+ * not necessary to lock inode->i_lock.
+ */
+ rcu_assign_pointer(landlock_inode(inode)->object, NULL);
+ /*
+ * Now, new rules can safely be tied to @inode with get_inode_object().
+ */
+
+ iput(inode);
+ if (atomic_long_dec_and_test(&landlock_superblock(sb)->inode_refs))
+ wake_up_var(&landlock_superblock(sb)->inode_refs);
+}
[...]
quoted
+static struct landlock_object *get_inode_object(struct inode *const inode)
+{
+ struct landlock_object *object, *new_object;
+ struct landlock_inode_security *inode_sec = landlock_inode(inode);
+
+ rcu_read_lock();
+retry:
+ object = rcu_dereference(inode_sec->object);
+ if (object) {
+ if (likely(refcount_inc_not_zero(&object->usage))) {
+ rcu_read_unlock();
+ return object;
+ }
+ /*
+ * We are racing with release_inode(), the object is going
+ * away. Wait for release_inode(), then retry.
+ */
+ spin_lock(&object->lock);
+ spin_unlock(&object->lock);
+ goto retry;
+ }
+ rcu_read_unlock();
+
+ /*
+ * If there is no object tied to @inode, then create a new one (without
+ * holding any locks).
+ */
+ new_object = landlock_create_object(&landlock_fs_underops, inode);
+ if (IS_ERR(new_object))
+ return new_object;
+
+ /* Protects against concurrent get_inode_object() calls. */
+ spin_lock(&inode->i_lock);
+ object = rcu_dereference_protected(inode_sec->object,
+ lockdep_is_held(&inode->i_lock));
rcu_dereference_protected() requires that inode_sec->object is not
concurrently changed, but I think another thread could call
get_inode_object() while we're in landlock_create_object(), and then
we could race with the NULL write in release_inode() here? (It
wouldn't actually be a UAF though because we're not actually accessing
`object` here.) Or am I missing a lock that prevents this?
In v28 this wasn't an issue because release_inode() was holding
inode->i_lock (and object->lock) during the NULL store; but in v29 and
this version the NULL store in release_inode() moved out of the locked
region. I think you could just move the NULL store in release_inode()
back up (and maybe add a comment explaining the locking rules for
landlock_inode(...)->object)?
(Or alternatively you could use rcu_dereference_raw() with a comment
explaining that the read pointer is only used to check for NULL-ness,
and that it is guaranteed that the pointer can't change if it is NULL
and we're holding the lock. But that'd be needlessly complicated, I
think.)
To reach rcu_assign_pointer(landlock_inode(inode)->object, NULL) in
release_inode() or in hook_sb_delete(), the
landlock_inode(inode)->object need to be non-NULL,
Yes.
quoted
which implies that a
call to get_inode_object(inode) either "retry" (because release_inode is
only called by landlock_put_object, which set object->usage to 0) until
it creates a new object, or reuses the existing referenced object (and
increments object->usage).
But it can be that landlock_inode(inode)->object only becomes non-NULL
after get_inode_object() has checked
rcu_dereference(inode_sec->object).
quoted
The worse case would be if
get_inode_object(inode) is called just before the
rcu_assign_pointer(landlock_inode(inode)->object, NULL) from
hook_sb_delete(), which would result in an object with a NULL underobj,
which is the expected behavior (and checked by release_inode).
The scenario I'm talking about doesn't involve hook_sb_delete().
quoted
The line rcu_assign_pointer(inode_sec->object, new_object) from
get_inode_object() can only be reached if the underlying inode doesn't
reference an object,
Yes.
quoted
in which case hook_sb_delete() will not reach the
rcu_assign_pointer(landlock_inode(inode)->object, NULL) line for this
same inode.
This works because get_inode_object(inode) is mutually exclusive to
itself with the same inode (i.e. an inode can only point to an object
that references this same inode).
To clarify: You can concurrently call get_inode_object() multiple
times on the same inode, right? There are no locks held on entry to
that function.
quoted
I tried to explain this with the comment "Protects against concurrent
get_inode_object() calls" in get_inode_object(), and the comments just
before both rcu_assign_pointer(landlock_inode(inode)->object, NULL).
The scenario I'm talking about is:
Initially the inode does not have an associated landlock_object. There
are two threads A and B. Thread A is going to execute
get_inode_object(). Thread B is going to execute get_inode_object()
followed immediately by landlock_put_object().
thread A: enters get_inode_object()
thread A: rcu_dereference(inode_sec->object) returns NULL
thread A: enters landlock_create_object()
thread B: enters get_inode_object()
thread B: rcu_dereference(inode_sec->object) returns NULL
thread B: calls landlock_create_object()
thread B: sets inode_sec->object while holding inode->i_lock
thread B: leaves get_inode_object()
thread B: enters landlock_put_object()
thread B: object->usage drops to 0, object->lock is taken
thread B: calls release_inode()
thread B: drops object->lock
thread A: returns from landlock_create_object()
thread A: takes inode->i_lock
At this point, thread B will run:
rcu_assign_pointer(landlock_inode(inode)->object, NULL);
while thread A runs:
rcu_dereference_protected(inode_sec->object,
lockdep_is_held(&inode->i_lock));
meaning there is a (theoretical) data race, since
rcu_dereference_protected() doesn't use READ_ONCE().
Hum, I see, that is what I was missing. And that explain why there is
(in practice) no impact on winning the race.
I would prefer to use rcu_access_pointer() instead of
rcu_dereference_protected() to avoid pitfall, and it reflects what I was
expecting:
*get_inode_object(struct inode *const inode)
/* Protects against concurrent get_inode_object() calls. */
spin_lock(&inode->i_lock);
- object = rcu_dereference_protected(inode_sec->object,
- lockdep_is_held(&inode->i_lock));
- if (unlikely(object)) {
+ if (unlikely(rcu_access_pointer(inode_sec->object))) {
/* Someone else just created the object, bail out and
retry. */
spin_unlock(&inode->i_lock);
kfree(new_object);
Ah, yeah, that should work. I had forgotten about rcu_access_pointer().
quoted hunk
But I'm not sure about your proposition to move the NULL store in
release_inode() back up. Do you mean to add back the inode lock in
release_inode() like this?
*const object)
* Makes sure that if the filesystem is concurrently unmounted,
* hook_sb_delete() will wait for us to finish iput().
*/
+ spin_lock(&inode->i_lock);
sb = inode->i_sb;
atomic_long_inc(&landlock_superblock(sb)->inode_refs);
spin_unlock(&object->lock);
- /*
- * Because object->underobj was not NULL, hook_sb_delete() and
- * get_inode_object() guarantee that it is safe to reset
- * landlock_inode(inode)->object while it is not NULL. It is therefore
- * not necessary to lock inode->i_lock.
- */
rcu_assign_pointer(landlock_inode(inode)->object, NULL);
+ spin_unlock(&inode->i_lock);
/*
* Now, new rules can safely be tied to @inode with get_inode_object().
*/
I would prefer to avoid nested locks if it is not necessary though.
Hm, yeah, you have a point there.
Doing it locklessly does make the locking rules a little complicated
though, and you'll have to update the comment inside struct
landlock_inode_security. At the moment, it says:
* @object: Weak pointer to an allocated object. All writes (i.e.
* creating a new object or removing one) are protected by the
* underlying inode->i_lock. Disassociating @object from the inode is
* additionally protected by @object->lock, from the time @object's
* usage refcount drops to zero to the time this pointer is nulled out.
which isn't true anymore.
On Tue, Mar 16, 2021 at 09:42:48PM +0100, Mickaël Salaün wrote:
quoted
From: Mickaël Salaün <redacted>
[...]
quoted
quoted
+/**
+ * sys_landlock_create_ruleset - Create a new ruleset
+ *
+ * @attr: Pointer to a &struct landlock_ruleset_attr identifying the scope of
+ * the new ruleset.
+ * @size: Size of the pointed &struct landlock_ruleset_attr (needed for
+ * backward and forward compatibility).
+ * @flags: Must be 0.
+ *
+ * This system call enables to create a new Landlock ruleset, and returns the
+ * related file descriptor on success.
+ *
+ * Possible returned errors are:
+ *
+ * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
+ * - EINVAL: @flags is not 0, or unknown access, or too small @size;
+ * - E2BIG or EFAULT: @attr or @size inconsistencies;
+ * - ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
+ */
+SYSCALL_DEFINE3(landlock_create_ruleset,
+ const struct landlock_ruleset_attr __user *const, attr,
+ const size_t, size, const __u32, flags)
+{
+ struct landlock_ruleset_attr ruleset_attr;
+ struct landlock_ruleset *ruleset;
+ int err, ruleset_fd;
+
+ /* Build-time checks. */
+ build_check_abi();
+
+ if (!landlock_initialized)
+ return -EOPNOTSUPP;
+
+ /* No flag for now. */
+ if (flags)
+ return -EINVAL;
+
+ /* Copies raw user space buffer. */
+ err = copy_min_struct_from_user(&ruleset_attr, sizeof(ruleset_attr),
+ offsetofend(typeof(ruleset_attr), handled_access_fs),
The use of offsetofend() here appears to be kind of the "V1", "V2", ...
sizes used in other extensible syscall implementations?
ruleset_attr is an extensible argument.
offsetofen() is used to set the minimum size of a valid argument. This
code will then not change with future extended ruleset_attr.
On Tue, Mar 16, 2021 at 09:42:52PM +0100, Mickaël Salaün wrote:
quoted
From: Mickaël Salaün <redacted>
[...]
quoted
quoted
[...]
+Special filesystems
+-------------------
+
+Access to regular files and directories can be restricted by Landlock,
+according to the handled accesses of a ruleset. However, files that do not
+come from a user-visible filesystem (e.g. pipe, socket), but can still be
+accessed through /proc/self/fd/, cannot currently be restricted. Likewise,
+some special kernel filesystems such as nsfs, which can be accessed through
+/proc/self/ns/, cannot currently be restricted. For now, these kind of special
+paths are then always allowed. Future Landlock evolutions will enable to
+restrict such paths with dedicated ruleset flags.
With this series, can /proc (at the top level) be blocked? (i.e. can a
landlock user avoid the weirdness by making /proc/$pid/ unavailable?)
/proc can be blocked, but not /proc/*/ns/* because of disconnected
roots. I plan to address this.
It is important to note that access to sensitive /proc files such as
ns/* and fd/* are automatically restricted according to domain
hierarchies. I'll add this detail to the documentation. :)
(Aside: you appear to be self-adding your Link: tags -- AIUI, this is
normally done by whoever pulls your series. I've only seen Link: tags
added when needing to refer to something else not included in the
series.)
It is an insurance to not lose history. :)
How will history be lost? The code is in the repo and discussions can
easily be found by searching for subjects or message IDs.
Is anyone else doing this self linking?
--
James Morris
[off-list ref]
(Aside: you appear to be self-adding your Link: tags -- AIUI, this is
normally done by whoever pulls your series. I've only seen Link: tags
added when needing to refer to something else not included in the
series.)
It is an insurance to not lose history. :)
How will history be lost? The code is in the repo and discussions can
easily be found by searching for subjects or message IDs.
The (full and ordered) history may be hard to find without any
Message-ID in commit messages. The Lore links keep that information (in
the commit message) and redirect to the related archived email thread,
which is very handy. For instance, Linus can rely on those links to
judge the quality of a patch:
https://lore.kernel.org/lkml/CAHk-=wh7xY3UF7zEc0BNVNjOox59jYBW-Gfi7=emm+BXPWc6nQ@mail.gmail.com/
Is anyone else doing this self linking?
I don't know, but it doesn't hurt. This way, if you're using git am
without b4 am -l (or forgot to add links manually), the history is still
pointed out by these self-reference links. I find it convenient and it
is a safeguard to not forget them, no matter who takes the patches.