Hi,
This series, improvement of the previous RFC [1], is a proof of concept to fill
some missing part of seccomp as the ability to check syscall argument pointers
or creating more dynamic security policies. The goal of this new stackable
Linux Security Module (LSM) called Landlock is to allow any process, including
unprivileged ones, to create powerful security sandboxes comparable to the
Seatbelt/XNU Sandbox or the OpenBSD Pledge. This kind of sandbox help to
mitigate the security impact of bugs or unexpected/malicious behaviors in
userland applications.
# Landlock LSM
This series is mainly focused on cgroups while keeping the possibility to
enforce access rules through seccomp. eBPF programs are used to create a
security rule. They are very limited (i.e. can only call a whitelist of
functions) and can not do a denial of service (i.e. no loop). A new dedicated
eBPF map allows to collect and compare Landlock handles with system resources
(e.g. files or network connections).
The approach taken is to add the minimum amount of code while still allowing
the userland to create quite complex access rules. A dedicated security policy
language such as used by SELinux, AppArmor and other major LSMs is a lot of
code and dedicated to a trusted process (i.e. root/administrator).
# eBPF
To get an expressive language while still being safe and small, Landlock is
based on eBPF. Landlock should be usable by untrusted processes and must then
expose a minimal attack surface. The eBPF bytecode is minimal while powerful,
widely used and thought to be used by not so trusted application. Reusing this
code allows to not reproduce the same mistakes and minimize new code while
still taking a generic approach. There is only some new features like a new
kind of arraymap and few dedicated eBPF functions.
An eBPF program have access to an eBPF context which contains the LSM hook
arguments (as does seccomp-bpf with syscall arguments). They can be used
directly or passed to helper functions according to their types. It is then
possible to do complex access checks without race conditions nor inconsistent
evaluation (i.e. incorrect mirroring of the OS code and state [2]).
There is one eBPF program subtype per LSM hook. This allow to statically check
which context access is performed by an eBPF program. This is needed to deny
kernel address leak and ensure the right use of LSM hook arguments with eBPF
functions. Moreover, this safe pointer handling remove the need for runtime
check or abstract data, which improve performances. Any user can add multiple
Landlock eBPF programs per LSM hook. They are stacked and evaluated one after
the other (cf. seccomp-bpf).
# LSM hooks
Contrary to syscalls, LSM hooks are security checkpoints and are not
architecture dependant. They are designed to match a security need reflected by
a security policy (e.g. access to a file). Exposing parts of some LSM hooks
instead of using the syscall API for sandboxing should help to avoid bugs and
hacks as encountered by the first RFC. Instead of redoing the work of the LSM
hooks through syscalls, we should use and expose them as does policies of
access control LSM.
Only a subset of the hooks are meaningful for an unprivileged sandbox mechanism
(e.g. file system or network access control). Landlock use an abstraction of
raw LSM hooks, which allow to deal with possible future API changes of the LSM
hook API. Moreover, thanks to the ePBF program typing (per LSM hook) used by
Landlock, it should not be hard to make such evolutions backward compatible.
# Use case scenario
First, a process need to create a new dedicated eBPF map containing handles.
This handles are references to system resources (e.g. file or directory) and
grouped in one or multiple maps to be efficiently managed and checked in
batches. This kind of map can be passed to Landlock eBPF functions to compare,
for example, with a file access request. The handles are only accessible from
the eBPF programs created by the same thread.
The loaded Landlock eBPF programs can be triggered by a seccomp filter
returning RET_LANDLOCK. In addition, a cookie (16-bit value) can be passed from
a seccomp filter to eBPF programs. This allow flexible security policies
between seccomp and Landlock.
Another way to enforce a Landlock security policy is to attach Landlock
programs to a dedicated cgroup. All the processes in this cgroup will then be
subject to this policy. For unprivileged processes, this can be done thanks to
cgroup delegation.
A triggered Landlock eBPF program can allows or deny an access, according to
its subtype (i.e. LSM hook), thanks to errno return values.
# Sandbox example with process hierarchy sandboxing (seccomp)
$ ls /home
user1
$ LANDLOCK_ALLOWED='/bin:/lib:/usr:/tmp:/proc/self/fd/0' \
./samples/landlock/sandbox /bin/sh -i
Launching a new sandboxed process.
$ ls /home
ls: cannot open directory '/home': Permission denied
# Sandbox example with conditional access control depending on a cgroup
$ mkdir /sys/fs/cgroup/sandboxed
$ ls /home
user1
$ LANDLOCK_CGROUPS='/sys/fs/cgroup/sandboxed' \
LANDLOCK_ALLOWED='/bin:/lib:/usr:/tmp:/proc/self/fd/0' \
./samples/landlock/sandbox
Ready to sandbox with cgroups.
$ ls /home
user1
$ echo $$ > /sys/fs/cgroup/sandboxed/cgroup.procs
$ ls /home
ls: cannot open directory '/home': Permission denied
# Current limitations and possible improvements
For now, eBPF programs can only return an errno code. It may be interesting to
be able to do other actions like seccomp-filter does (e.g. kill process). Such
features can easily be implemented but the main advantage of the current
approach is to be able to only execute eBPF programs until one return an errno
code instead of executing all programs like seccomp-filter does.
It is quite easy to add new eBPF functions to extend Landlock. The main concern
should be about the ability to leak information from the current process to
another one (e.g. through maps) to not reproduce the same security sensitive
behavior as ptrace.
This design does not seems too intrusive but is flexible enough to allow a
powerful sandbox mechanism accessible by any process on Linux. The use of
seccomp and Landlock is more suitable with the help of a userland library (e.g.
libseccomp) that could help to specify a high-level language to express a
security policy instead of raw eBPF programs. Moreover, thanks to LLVM, it is
possible to express an eBPF program with a subset of C.
# FAQ
## Why not use a policy language like used by SElinux or AppArmor?
This kind of LSMs are dedicated to administrators. They already manage the
system and are not a threat to the system security. However, seccomp, and
Landlock too, should be available to anyone, which potentially include
untrusted users and processes. To reduce the attack surface, Landlock should
expose the minimum amount of code, hence minimal complexity. Moreover, another
threat is to make accessible to a malicious code a new way to gain more
information. For example, Landlock features should not allow a program to get
the file owner if the directory containing this file is not readable. This data
could then be exfiltrated thanks to the access result. Thus, we should limit
the expressiveness of the available checks. The current approach is to do the
checks in such a way that only a comparison with an already accessed resource
(e.g. file descriptor) is possible. This allow to have a reference to compare
with, without exposing much information.
## Why a new LSM? Does SELinux, AppArmor, Smack or Tomoyo are not good enough?
The current access control LSMs are fine for their purpose which is to give the
*root* the ability to enforce a security policy for the *system*. What is
missing is a way to enforce a security policy for any applications by its
developer and *unprivileged user* as seccomp can do for raw syscall filtering.
Moreover, Landlock handles stacked hook programs from different users. It must
then ensure there is no possible malicious interactions between this programs.
Difference with other (access control) LSMs:
* not only dedicated to administrators (i.e. no_new_priv);
* limited kernel attack surface (e.g. policy parsing);
* helpers to compare complex objects (path/FD), no access to internal kernel
data (do not leak addresses);
* constraint policy rules/programs (no DoS: deterministic execution time);
* do not leak more information than the loader process can legitimately have
access to (minimize metadata inference): must compare from an already allowed
file (through a handle).
## Why does seccomp-filter is not enough?
A seccomp filter can access to raw syscall arguments which means that it is not
possible to filter according to pointed data as a file path. As demonstrated
the first version of this patch series, filtering at the syscall level is
complicated (e.g. need to take care of race conditions). This is mainly because
the access control checkpoints of the kernel are not at this high-level but
more underneath, at LSM hooks level. The LSM hooks are designed to handle this
kind of checks. This series use this approach to leverage the ability of
unprivileged users to limit themselves.
Cf. "What it isn't?" in Documentation/prctl/seccomp_filter.txt
## As a developer, why do I need this feature?
Landlock's goal is to help userland to limit its attack surface.
Security-conscious developers would like to protect users from a security bug
in their applications and the third-party dependencies they are using. Such a
bug can compromise all the user data and help an attacker to perform a
privilege escalation. Using an *unprivileged sandbox* feature such as Landlock
empower the developer with the ability to properly compartmentalize its
software and limit the impact of being compromised.
## As a user, why do I need a this feature?
Any user can already use seccomp-filter to whitelist a set of syscalls to
reduce the kernel attack surface for a set of processes. However an
unprivileged user can't create a security policy as the root user can thanks to
SELinux and other access control LSMs. Landlock allows any unprivileged user to
protect their data from being accessed by any process they run but only an
identified subset. User tools can be created to help create such a high-level
access control policy. This policy may not be powerful enough to express the
same policies as the current access control LSMs, because of the threat an
unprivileged user can be to the system, but it should be enough for most
use-cases (e.g. blacklist or whitelist a set of file hierarchies).
## Does Landlock can limit network access or other resources?
Limiting network access is obviously in the scope of Landlock but it is not yet
implemented. The main goal now is to get feedback about the whole concept, the
API and the file access control part. More access control types could be
implemented in the future.
## Why using the seccomp(2) syscall?
Landlock use the same semantic as seccomp to apply access rule restrictions. It
add a new layer of security for the current process which is inherited by its
childs. It make sense to use an unique access-restricting syscall (that should
be allowed by seccomp-filter rules) which can only drop privileges. Moreover, a
Landlock eBPF program could come from outside a process (e.g. passed through a
UNIX socket). It is then useful to differentiate the creation/load of Landlock
eBPF programs via bpf(2), from rule enforcing via seccomp(2).
## Why using cgroups?
cgroups are designed to handle groups of processes. One use case is to manage
containers. Sandboxing based on process hierarchy (seccomp) is design to handle
immutable security policies, which is a good security property but does not
match all use cases. A user can attach Landlock rules to a cgroup. Doing so,
all the processes in that cgroup will be subject to the security policy.
However, if the user is allowed to manage this cgroup, it could dynamically
move this group of processes to a cgroup with another security policy (or
none). Landlock rules can be applied either on a process hierarchy (e.g.
application with built-in sandboxing) or a group of processes (e.g. container
sandboxing). Both approaches can be combined for the same process.
# Changes since RFC v2
* revamp cgroup handling:
* use Daniel Mack's patches "Add eBPF hooks for cgroups" v5 [3]
* remove bpf_landlock_cmp_cgroup_beneath()
* make BPF_PROG_ATTACH usable with delegated cgroups
* add a new CGRP_NO_NEW_PRIVS flag for safe cgroups
* handle Landlock sandboxing for cgroups hierarchy
* allow unprivileged processes to attach Landlock eBPF program to cgroups
* add subtype to eBPF programs:
* replace Landlock hook identification by custom eBPF program types with a
dedicated subtype field
* manage fine-grained privileged Landlock programs
* register Landlock programs for dedicated trigger origins (e.g. syscall,
return from seccomp filter and/or interruption)
* performance and memory optimizations: use an array to access Landlock hooks
directly but do not duplicated it for each thread (seccomp-based)
* allow running Landlock programs without seccomp filter
* fix seccomp-related issues
* remove extra errno bounding check for Landlock programs
* add some examples for optional eBPF functions or context access (network
related) according to security checks to allow more features for privileged
programs (e.g. Checmate)
# Changes since RFC v1
* focus on the LSM hooks, not the syscalls:
* much more simple implementation
* does not need audit cache tricks to avoid race conditions
* more simple to use and more generic because using the LSM hook abstraction
directly
* more efficient because only checking in LSM hooks
* architecture agnostic
* switch from cBPF to eBPF:
* new eBPF program types dedicated to Landlock
* custom functions used by the eBPF program
* gain some new features (e.g. 10 registers, can load values of different
size, LLVM translator) but only a few functions allowed and a dedicated map
type
* new context: LSM hook ID, cookie and LSM hook arguments
* need to set the sysctl kernel.unprivileged_bpf_disable to 0 (default value)
to be able to load hook filters as unprivileged users
* smaller and simpler:
* no more checker groups but dedicated arraymap of handles
* simpler userland structs thanks to eBPF functions
* distinctive name: Landlock
This series can be applied on top of the Daniel Mack's patches for
BPF_PROG_ATTACH v5 [3] on Linux next-20160913 (commit
562d4a2d7fa26b11d995f418951f3396a5d0f550). This can be
tested with CONFIG_SECURITY_LANDLOCK, CONFIG_SECCOMP_FILTER and
CONFIG_CGROUP_BPF. I would really appreciate constructive comments on the
usability, architecture, code and userland API of Landlock LSM.
[1] https://lkml.kernel.org/r/1472121165-29071-1-git-send-email-mic@digikod.net
[2] https://crypto.stanford.edu/cs155/papers/traps.pdf
[3] https://lkml.kernel.org/r/1473696735-11269-1-git-send-email-daniel@zonque.org
Regards,
Mickaël Salaün (22):
landlock: Add Kconfig
bpf: Move u64_to_ptr() to BPF headers and inline it
bpf,landlock: Add a new arraymap type to deal with (Landlock) handles
bpf: Set register type according to is_valid_access()
bpf,landlock: Add eBPF program subtype and is_valid_subtype() verifier
landlock: Add LSM hooks
landlock: Handle file comparisons
seccomp: Fix documentation for struct seccomp_filter
seccomp: Move struct seccomp_filter in seccomp.h
seccomp: Split put_seccomp_filter() with put_seccomp()
seccomp,landlock: Handle Landlock hooks per process hierarchy
bpf: Cosmetic change for bpf_prog_attach()
bpf/cgroup: Replace struct bpf_prog with union bpf_object
bpf/cgroup: Make cgroup_bpf_update() return an error code
bpf/cgroup: Move capability check
bpf/cgroup,landlock: Handle Landlock hooks per cgroup
cgroup: Add access check for cgroup_get_from_fd()
cgroup,landlock: Add CGRP_NO_NEW_PRIVS to handle unprivileged hooks
landlock: Add interrupted origin
landlock: Add update and debug access flags
bpf,landlock: Add optional skb pointer in the Landlock context
samples/landlock: Add sandbox example
include/linux/bpf-cgroup.h | 19 +-
include/linux/bpf.h | 45 +++-
include/linux/cgroup-defs.h | 9 +
include/linux/cgroup.h | 2 +-
include/linux/filter.h | 1 +
include/linux/landlock.h | 86 ++++++++
include/linux/lsm_hooks.h | 5 +
include/linux/seccomp.h | 58 +++++-
include/uapi/linux/bpf.h | 122 +++++++++++
include/uapi/linux/seccomp.h | 2 +
kernel/bpf/arraymap.c | 226 ++++++++++++++++++++-
kernel/bpf/cgroup.c | 78 ++++---
kernel/bpf/syscall.c | 78 ++++---
kernel/bpf/verifier.c | 47 ++++-
kernel/cgroup.c | 66 +++++-
kernel/fork.c | 25 ++-
kernel/seccomp.c | 107 +++++++---
kernel/trace/bpf_trace.c | 12 +-
net/core/filter.c | 21 +-
samples/Makefile | 2 +-
samples/landlock/.gitignore | 1 +
samples/landlock/Makefile | 16 ++
samples/landlock/sandbox.c | 307 ++++++++++++++++++++++++++++
security/Kconfig | 1 +
security/Makefile | 2 +
security/landlock/Kconfig | 23 +++
security/landlock/Makefile | 3 +
security/landlock/checker_fs.c | 179 ++++++++++++++++
security/landlock/checker_fs.h | 20 ++
security/landlock/common.h | 27 +++
security/landlock/lsm.c | 451 +++++++++++++++++++++++++++++++++++++++++
security/landlock/manager.c | 281 +++++++++++++++++++++++++
security/security.c | 1 +
33 files changed, 2194 insertions(+), 129 deletions(-)
create mode 100644 include/linux/landlock.h
create mode 100644 samples/landlock/.gitignore
create mode 100644 samples/landlock/Makefile
create mode 100644 samples/landlock/sandbox.c
create mode 100644 security/landlock/Kconfig
create mode 100644 security/landlock/Makefile
create mode 100644 security/landlock/checker_fs.c
create mode 100644 security/landlock/checker_fs.h
create mode 100644 security/landlock/common.h
create mode 100644 security/landlock/lsm.c
create mode 100644 security/landlock/manager.c
--
2.9.3
This fix a pointer leak when an unprivileged eBPF program read a pointer
value from the context. Even if is_valid_access() returns a pointer
type, the eBPF verifier replace it with UNKNOWN_VALUE. The register
value containing an address is then allowed to leak. Moreover, this
prevented unprivileged eBPF programs to use functions with (legitimate)
pointer arguments.
This bug was not a problem until now because the only unprivileged eBPF
program allowed is of type BPF_PROG_TYPE_SOCKET_FILTER and all the types
from its context are UNKNOWN_VALUE.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Fixes: 969bf05eb3ce ("bpf: direct packet access")
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
kernel/bpf/verifier.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
Set struct seccomp_filter public because of the next use of
the new field thread_prev added for Landlock LSM.
Signed-off-by: Mickaël Salaün <redacted>
Cc: Kees Cook <redacted>
Cc: Andy Lutomirski <redacted>
Cc: Will Drewry <redacted>
---
include/linux/seccomp.h | 27 ++++++++++++++++++++++++++-
kernel/seccomp.c | 26 --------------------------
2 files changed, 26 insertions(+), 27 deletions(-)
@@ -33,32 +33,6 @@#include<linux/tracehook.h>#include<linux/uaccess.h>-/**-*structseccomp_filter-containerforseccompBPFprograms-*-*@usage:referencecounttomanagetheobjectlifetime.-*get/puthelpersshouldbeusedwhenaccessinganinstance-*outsideofalifetime-guardedsection.Ingeneral,this-*isonlyneededforhandlingfilterssharedacrosstasks.-*@prev:pointstoapreviouslyinstalled,orinherited,filter-*@prog:theBPFprogramtoevaluate-*-*seccomp_filterobjectsareorganizedinatreelinkedviathe@prev-*pointer.Foranytask,itappearstobeasingly-linkedliststarting-*withcurrent->seccomp.filter,themostrecentlyattachedorinheritedfilter.-*However,multiplefiltersmaysharea@prevnode,bywayoffork(),which-*resultsinaunidirectionaltreeexistinginmemory.Thisissimilarto-*hownamespaceswork.-*-*seccomp_filterobjectsshouldneverbemodifiedafterbeingattached-*toatask_struct(otherthan@usage).-*/-structseccomp_filter{-atomic_tusage;-structseccomp_filter*prev;-structbpf_prog*prog;-};-/* Limit any path through the tree to 256KB worth of instructions. */#define MAX_INSNS_PER_PATH ((1 << 18) / sizeof(struct sock_filter))
This allows to add new eBPF programs to Landlock hooks dedicated to a
cgroup thanks to the BPF_PROG_ATTACH command. Like for socket eBPF
programs, the Landlock hooks attached to a cgroup are propagated to the
nested cgroups. However, when a new Landlock program is attached to one
of this nested cgroup, this cgroup hierarchy fork the Landlock hooks.
This design is simple and match the current CONFIG_BPF_CGROUP
inheritance. The difference lie in the fact that Landlock programs can
only be stacked but not removed. This match the append-only seccomp
behavior. Userland is free to handle Landlock hooks attached to a cgroup
in more complicated ways (e.g. continuous inheritance), but care should
be taken to properly handle error cases (e.g. memory allocation errors).
Changes since v2:
* new design based on BPF_PROG_ATTACH (suggested by Alexei Starovoitov)
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Daniel Mack <daniel@zonque.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Cc: Tejun Heo <tj@kernel.org>
Link: https://lkml.kernel.org/r/20160826021432.GA8291@ast-mbp.thefacebook.com
Link: https://lkml.kernel.org/r/20160827204307.GA43714@ast-mbp.thefacebook.com
---
include/linux/bpf-cgroup.h | 7 +++++++
include/linux/cgroup-defs.h | 2 ++
include/linux/landlock.h | 9 +++++++++
include/uapi/linux/bpf.h | 1 +
kernel/bpf/cgroup.c | 33 ++++++++++++++++++++++++++++++---
kernel/bpf/syscall.c | 11 +++++++++++
security/landlock/lsm.c | 40 +++++++++++++++++++++++++++++++++++++++-
security/landlock/manager.c | 32 ++++++++++++++++++++++++++++++++
8 files changed, 131 insertions(+), 4 deletions(-)
@@ -301,8 +301,10 @@ struct cgroup {/* used to schedule release agent */structwork_structrelease_agent_work;+#ifdef CONFIG_CGROUP_BPF/* used to store eBPF programs */structcgroup_bpfbpf;+#endif /* CONFIG_CGROUP_BPF *//* ids of the ancestors at each level including self */intancestor_ids[];
@@ -240,3 +251,24 @@ int landlock_seccomp_set_hook(unsigned int flags, const char __user *user_bpf_fdreturn0;}#endif /* CONFIG_SECCOMP_FILTER */++/**+*landlock_cgroup_set_hook-attachaLandlockprogramtoacgroup+*+*Mustbecalledwithcgroup_mutexheld.+*+*@crgp:non-NULLcgrouppointertoattachto+*@prog:Landlockprogrampointer+*/+#ifdef CONFIG_CGROUP_BPF+structlandlock_hooks*landlock_cgroup_set_hook(structcgroup*cgrp,+structbpf_prog*prog)+{+if(!prog)+returnERR_PTR(-EINVAL);++/* copy the inherited hooks and append a new one */+returnlandlock_set_hook(cgrp->bpf.effective[BPF_CGROUP_LANDLOCK].hooks,+prog,NULL);+}+#endif /* CONFIG_CGROUP_BPF */
For now, the update and debug accesses are only accessible to a process
with CAP_SYS_ADMIN. This could change in the future.
The capability check is statically done when loading an eBPF program,
according to the current process. If the process has enough rights and
set the appropriate access flags, then the dedicated functions or data
will be accessible.
With the update access, the following functions are available:
* bpf_map_lookup_elem
* bpf_map_update_elem
* bpf_map_delete_elem
* bpf_tail_call
With the debug access, the following functions are available:
* bpf_trace_printk
* bpf_get_prandom_u32
* bpf_get_current_pid_tgid
* bpf_get_current_uid_gid
* bpf_get_current_comm
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Cc: Sargun Dhillon <redacted>
---
include/uapi/linux/bpf.h | 4 +++-
security/landlock/lsm.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+), 1 deletion(-)
Add a basic sandbox tool to create a process isolated from some part of
the system. This can depend of the current cgroup.
Example with the current process hierarchy (seccomp):
$ ls /home
user1
$ LANDLOCK_ALLOWED='/bin:/lib:/usr:/tmp:/proc/self/fd/0' \
./samples/landlock/sandbox /bin/sh -i
Launching a new sandboxed process.
$ ls /home
ls: cannot open directory '/home': Permission denied
Example with a cgroup:
$ mkdir /sys/fs/cgroup/sandboxed
$ ls /home
user1
$ LANDLOCK_CGROUPS='/sys/fs/cgroup/sandboxed' \
LANDLOCK_ALLOWED='/bin:/lib:/usr:/tmp:/proc/self/fd/0' \
./samples/landlock/sandbox
Ready to sandbox with cgroups.
$ ls /home
user1
$ echo $$ > /sys/fs/cgroup/sandboxed/cgroup.procs
$ ls /home
ls: cannot open directory '/home': Permission denied
Changes since v2:
* use BPF_PROG_ATTACH for cgroup handling
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: James Morris <redacted>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
---
samples/Makefile | 2 +-
samples/landlock/.gitignore | 1 +
samples/landlock/Makefile | 16 +++
samples/landlock/sandbox.c | 307 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 325 insertions(+), 1 deletion(-)
create mode 100644 samples/landlock/.gitignore
create mode 100644 samples/landlock/Makefile
create mode 100644 samples/landlock/sandbox.c
@@ -0,0 +1,16 @@+# kbuild trick to avoid linker error. Can be omitted if a module is built.+obj-:=dummy.o++hostprogs-$(CONFIG_SECURITY_LANDLOCK):=sandbox+sandbox-objs:=sandbox.o++always:=$(hostprogs-y)++HOSTCFLAGS+=-I$(objtree)/usr/include++# Trick to allow make to be run from this directory+all:+$(MAKE)-C../../$$PWD/++clean:+$(MAKE)-C../../M=$$PWDclean
@@ -0,0 +1,307 @@+/*+*LandlockLSM-Sandboxexample+*+*Copyright(C)2016MickaëlSalaün<mic@digikod.net>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseversion3,as+*publishedbytheFreeSoftwareFoundation.+*/++#define _GNU_SOURCE+#include<errno.h>+#include<fcntl.h>/* open() */+#include<linux/bpf.h>+#include<linux/filter.h>+#include<linux/prctl.h>+#include<linux/seccomp.h>+#include<stddef.h>+#include<stdio.h>+#include<stdlib.h>+#include<string.h>+#include<sys/prctl.h>+#include<sys/syscall.h>+#include<unistd.h>++#include"../../tools/include/linux/filter.h"++#include"../bpf/libbpf.c"++#ifndef seccomp+staticintseccomp(unsignedintop,unsignedintflags,void*args)+{+errno=0;+returnsyscall(__NR_seccomp,op,flags,args);+}+#endif++staticintlandlock_prog_load(conststructbpf_insn*insns,intprog_len,+enumlandlock_hook_idhook_id,__u64access)+{+unionbpf_attrattr={+.prog_type=BPF_PROG_TYPE_LANDLOCK,+.insns=ptr_to_u64((void*)insns),+.insn_cnt=prog_len/sizeof(structbpf_insn),+.license=ptr_to_u64((void*)"GPL"),+.log_buf=ptr_to_u64(bpf_log_buf),+.log_size=LOG_BUF_SIZE,+.log_level=1,+.prog_subtype.landlock_hook={+.id=hook_id,+.origin=LANDLOCK_FLAG_ORIGIN_SECCOMP|+LANDLOCK_FLAG_ORIGIN_SYSCALL|+LANDLOCK_FLAG_ORIGIN_INTERRUPT,+.access=access,+},+};++/* assign one field outside of struct init to make sure any+*paddingiszeroinitialized+*/+attr.kern_version=0;++bpf_log_buf[0]=0;++returnsyscall(__NR_bpf,BPF_PROG_LOAD,&attr,sizeof(attr));+}++#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))++staticintapply_sandbox(constchar**allowed_paths,intpath_nb,constchar+**cgroup_paths,intcgroup_nb)+{+__u32key;+inti,ret=0,map_fs=-1,offset;++/* set up the test sandbox */+if(prctl(PR_SET_NO_NEW_PRIVS,1,0,0,0)){+perror("prctl(no_new_priv)");+return1;+}++/* register a new syscall filter */+structsock_filterfilter0[]={+/* pass a cookie containing 5 to the LSM hook filter */+BPF_STMT(BPF_RET|BPF_K,SECCOMP_RET_LANDLOCK|5),+};+structsock_fprogprog0={+.len=(unsignedshort)ARRAY_SIZE(filter0),+.filter=filter0,+};+if(!cgroup_nb){+if(seccomp(SECCOMP_SET_MODE_FILTER,0,&prog0)){+perror("seccomp(set_filter)");+return1;+}+}++if(path_nb){+map_fs=bpf_create_map(BPF_MAP_TYPE_LANDLOCK_ARRAY,+sizeof(key),sizeof(structlandlock_handle),+10,0);+if(map_fs<0){+fprintf(stderr,"bpf_create_map(fs): %s\n",+strerror(errno));+return1;+}+for(key=0;key<path_nb;key++){+intfd=open(allowed_paths[key],+O_RDONLY|O_CLOEXEC);+if(fd<0){+fprintf(stderr,"open(fs: \"%s\"): %s\n",+allowed_paths[key],+strerror(errno));+return1;+}+structlandlock_handlehandle={+.type=BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD,+.fd=(__u64)fd,+};++/* register a new LSM handle */+if(bpf_update_elem(map_fs,&key,&handle,BPF_ANY)){+fprintf(stderr,"bpf_update_elem(fs: \"%s\"): %s\n",+allowed_paths[key],+strerror(errno));+close(fd);+return1;+}+close(fd);+}+}++/* load a LSM filter hook (eBPF) */+structbpf_insnhook_pre[]={+/* save context */+BPF_MOV64_REG(BPF_REG_6,BPF_REG_1),++#if 0+/* check our cookie (not used in this example) */+BPF_LDX_MEM(BPF_H,BPF_REG_0,BPF_REG_6,offsetof(struct+landlock_data,cookie)),+BPF_JMP_IMM(BPF_JEQ,BPF_REG_0,5,2),+BPF_MOV32_IMM(BPF_REG_0,0),+BPF_EXIT_INSN(),+#endif+};+structbpf_insnhook_path[]={+/* specify an option, if any */+BPF_MOV32_IMM(BPF_REG_1,0),+/* handles to compare with */+BPF_LD_MAP_FD(BPF_REG_2,map_fs),+BPF_MOV64_IMM(BPF_REG_3,BPF_MAP_ARRAY_OP_OR),+/* hook argument (struct file) */+BPF_LDX_MEM(BPF_DW,BPF_REG_4,BPF_REG_6,offsetof(struct+landlock_data,args[0])),+/* checker function */+BPF_EMIT_CALL(BPF_FUNC_landlock_cmp_fs_beneath_with_struct_file),++/* if the checked path is beneath the handle */+BPF_JMP_IMM(BPF_JNE,BPF_REG_0,0,2),+BPF_MOV32_IMM(BPF_REG_0,0),+BPF_EXIT_INSN(),+/* allow anonymous mapping */+BPF_JMP_IMM(BPF_JNE,BPF_REG_0,-ENOENT,2),+BPF_MOV32_IMM(BPF_REG_0,0),+BPF_EXIT_INSN(),+/* deny by default, if any error */+BPF_JMP_IMM(BPF_JGE,BPF_REG_0,0,2),+BPF_MOV32_IMM(BPF_REG_0,EACCES),+BPF_EXIT_INSN(),+};+structbpf_insnhook_post[]={+BPF_MOV32_IMM(BPF_REG_0,EACCES),+BPF_EXIT_INSN(),+};++unsignedlonghook_size=sizeof(hook_pre)+sizeof(hook_path)*+(path_nb?1:0)+sizeof(hook_post);++structbpf_insn*hook0=malloc(hook_size);+if(!hook0){+perror("malloc");+ret=1;+gotoerr_alloc;+}+memcpy(hook0,hook_pre,sizeof(hook_pre));+offset=sizeof(hook_pre)/sizeof(hook0[0]);+if(path_nb){+memcpy(hook0+offset,hook_path,sizeof(hook_path));+offset+=sizeof(hook_path)/sizeof(hook0[0]);+}+memcpy(hook0+offset,hook_post,sizeof(hook_post));++/* TODO: handle inode_permission hook (e.g. chdir) */+enumlandlock_hook_idhooks[]={+LANDLOCK_HOOK_FILE_OPEN,+LANDLOCK_HOOK_FILE_PERMISSION,+LANDLOCK_HOOK_MMAP_FILE,+};+for(i=0;i<ARRAY_SIZE(hooks)&&!ret;i++){+intbpf0=landlock_prog_load(hook0,hook_size,hooks[i],0);+if(bpf0==-1){+perror("prog_load");+fprintf(stderr,"%s",bpf_log_buf);+ret=1;+break;+}+if(!cgroup_nb){+if(seccomp(SECCOMP_SET_LANDLOCK_HOOK,0,&bpf0)){+perror("seccomp(set_hook)");+ret=1;+}+}else{+for(key=0;key<cgroup_nb&&!ret;key++){+intfd=open(cgroup_paths[key],+O_DIRECTORY|O_CLOEXEC);+if(fd<0){+fprintf(stderr,"open(cgroup: \"%s\"): %s\n",+cgroup_paths[key],strerror(errno));+ret=1;+break;+}+if(bpf_prog_attach(bpf0,fd,BPF_CGROUP_LANDLOCK)){+fprintf(stderr,"bpf_prog_attach(cgroup: \"%s\"): %s\n",+cgroup_paths[key],strerror(errno));+ret=1;+}+close(fd);+}+}+close(bpf0);+}++free(hook0);+err_alloc:+if(path_nb){+close(map_fs);+}+returnret;+}++#define ENV_FS_PATH_NAME "LANDLOCK_ALLOWED"+#define ENV_CGROUP_PATH_NAME "LANDLOCK_CGROUPS"+#define ENV_PATH_TOKEN ":"++staticintparse_path(char*env_path,constchar***path_list)+{+inti,path_nb=0;++if(env_path){+path_nb++;+for(i=0;env_path[i];i++){+if(env_path[i]==ENV_PATH_TOKEN[0]){+path_nb++;+}+}+}+*path_list=malloc(path_nb*sizeof(**path_list));+for(i=0;i<path_nb;i++){+(*path_list)[i]=strsep(&env_path,ENV_PATH_TOKEN);+}++returnpath_nb;+}++intmain(intargc,char*constargv[],char*const*envp)+{+char*cmd_path;+char*env_path_allowed,*env_path_cgroup;+intpath_nb,cgroup_nb;+constchar**sb_paths=NULL;+constchar**cg_paths=NULL;+char*const*cmd_argv;++env_path_allowed=getenv(ENV_FS_PATH_NAME);+if(env_path_allowed)+env_path_allowed=strdup(env_path_allowed);+env_path_cgroup=getenv(ENV_CGROUP_PATH_NAME);+if(env_path_cgroup)+env_path_cgroup=strdup(env_path_cgroup);++path_nb=parse_path(env_path_allowed,&sb_paths);+cgroup_nb=parse_path(env_path_cgroup,&cg_paths);+if(argc<2&&!cgroup_nb){+fprintf(stderr,"usage: %s <cmd> [args]...\n\n",argv[0]);+fprintf(stderr,"Environment variables containing paths, each separated by a colon:\n");+fprintf(stderr,"* %s (whitelist of allowed files and directories)\n",+ENV_FS_PATH_NAME);+fprintf(stderr,"* %s (optional cgroup paths for which the sandbox is enabled)\n",+ENV_CGROUP_PATH_NAME);+fprintf(stderr,"\nexample:\n%s='/bin:/lib:/usr:/tmp:/proc/self/fd/0' %s /bin/sh -i\n",+ENV_FS_PATH_NAME,argv[0]);+return1;+}+if(apply_sandbox(sb_paths,path_nb,cg_paths,cgroup_nb))+return1;+if(!cgroup_nb){+cmd_path=argv[1];+cmd_argv=argv+1;+fprintf(stderr,"Launching a new sandboxed process.\n");+execve(cmd_path,cmd_argv,envp);+perror("execve");+return1;+}+fprintf(stderr,"Ready to sandbox with cgroups.\n");+return0;+}
Add a new flag CGRP_NO_NEW_PRIVS for each cgroup. This flag is initially
set for all cgroup except the root. The flag is clear when a new process
without the no_new_privs flags is attached to the cgroup.
If a cgroup is landlocked, then any new attempt, from an unprivileged
process, to attach a process without no_new_privs to this cgroup will
be denied.
This allows to safely manage Landlock rules with cgroup delegation as
with seccomp.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Daniel Mack <daniel@zonque.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Cc: Tejun Heo <tj@kernel.org>
---
include/linux/cgroup-defs.h | 7 +++++++
kernel/bpf/syscall.c | 7 ++++---
kernel/cgroup.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
security/landlock/manager.c | 7 +++++++
4 files changed, 60 insertions(+), 5 deletions(-)
@@ -1985,6 +1986,7 @@ static void init_cgroup_root(struct cgroup_root *root,strcpy(root->name,opts->name);if(opts->cpuset_clone_children)set_bit(CGRP_CPUSET_CLONE_CHILDREN,&root->cgrp.flags);+/* no CGRP_NO_NEW_PRIVS flag for the root */}staticintcgroup_setup_root(structcgroup_root*root,u16ss_mask)
@@ -4345,9 +4368,22 @@ int cgroup_transfer_tasks(struct cgroup *to, struct cgroup *from)return-EBUSY;mutex_lock(&cgroup_mutex);-percpu_down_write(&cgroup_threadgroup_rwsem);+#if defined(CONFIG_CGROUP_BPF) && defined(CONFIG_SECURITY_LANDLOCK)+if(!(from->flags&BIT_ULL(CGRP_NO_NEW_PRIVS))){+if(to->bpf.pinned[BPF_CGROUP_LANDLOCK].hooks&&+security_capable_noaudit(current_cred(),+current_user_ns(),CAP_SYS_ADMIN)!=0){+pr_warn("%s: EPERM\n",__func__);+ret=-EPERM;+gotoout_unlock;+}+pr_warn("%s: no EPERM\n",__func__);+clear_bit(CGRP_NO_NEW_PRIVS,&to->flags);+}+#endif /* CONFIG_CGROUP_BPF && CONFIG_SECURITY_LANDLOCK */+/* all tasks in @from are being moved, all csets are source */spin_lock_irq(&css_set_lock);list_for_each_entry(link,&from->cset_links,cset_link)
@@ -267,6 +268,12 @@ struct landlock_hooks *landlock_cgroup_set_hook(struct cgroup *cgrp,if(!prog)returnERR_PTR(-EINVAL);+/* check no_new_privs for tasks in the cgroup */+if(!(cgrp->flags&BIT_ULL(CGRP_NO_NEW_PRIVS))&&+security_capable_noaudit(current_cred(),+current_user_ns(),CAP_SYS_ADMIN)!=0)+returnERR_PTR(-EPERM);+/* copy the inherited hooks and append a new one */returnlandlock_set_hook(cgrp->bpf.effective[BPF_CGROUP_LANDLOCK].hooks,prog,NULL);
Add security access check for cgroup backed FD. The "cgroup.procs" file
of the corresponding cgroup must be readable to identify the cgroup, and
writable to prove that the current process can manage this cgroup (e.g.
through delegation). This is similar to the check done by
cgroup_procs_write_permission().
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Daniel Mack <daniel@zonque.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Cc: Tejun Heo <tj@kernel.org>
---
include/linux/cgroup.h | 2 +-
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/syscall.c | 6 +++---
kernel/cgroup.c | 16 +++++++++++++++-
4 files changed, 20 insertions(+), 6 deletions(-)
@@ -863,7 +864,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)if(IS_ERR(prog))returnPTR_ERR(prog);-cgrp=cgroup_get_from_fd(attr->target_fd);+cgrp=cgroup_get_from_fd(attr->target_fd,MAY_WRITE);if(IS_ERR(cgrp)){bpf_prog_put(prog);returnPTR_ERR(cgrp);
@@ -891,10 +892,9 @@ static int bpf_prog_detach(const union bpf_attr *attr)if(!capable(CAP_NET_ADMIN))return-EPERM;-cgrp=cgroup_get_from_fd(attr->target_fd);+cgrp=cgroup_get_from_fd(attr->target_fd,MAY_WRITE);if(IS_ERR(cgrp))returnPTR_ERR(cgrp);-result=cgroup_bpf_update(cgrp,NULL,attr->attach_type);cgroup_put(cgrp);break;
This is a proof of concept to expose optional values that could depend
of the process access rights.
There is two dedicated flags: LANDLOCK_FLAG_ACCESS_SKB_READ and
LANDLOCK_FLAG_ACCESS_SKB_WRITE. Each of them can be activated to access
eBPF functions manipulating a skb in a read or write way.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Cc: Sargun Dhillon <redacted>
---
include/linux/bpf.h | 2 ++
include/uapi/linux/bpf.h | 7 ++++++-
kernel/bpf/verifier.c | 6 ++++++
security/landlock/lsm.c | 26 ++++++++++++++++++++++++++
4 files changed, 40 insertions(+), 1 deletion(-)
@@ -88,6 +88,7 @@ enum bpf_arg_type {ARG_PTR_TO_STRUCT_FILE,/* pointer to struct file */ARG_CONST_PTR_TO_LANDLOCK_HANDLE_FS,/* pointer to Landlock FS handle */+ARG_PTR_TO_STRUCT_SKB,/* pointer to struct skb */};/* type of values returned from helper functions */
A Landlock program will be triggered according to its subtype/origin
bitfield. The LANDLOCK_FLAG_ORIGIN_SECCOMP value will trigger the
Landlock program when a seccomp filter will return RET_LANDLOCK.
Moreover, it is possible to return a 16-bit cookie which will be
readable by the Landlock programs in its context.
Only seccomp filters loaded from the same thread and before a Landlock
program can trigger it through LANDLOCK_FLAG_ORIGIN_SECCOMP. Multiple
Landlock programs can be triggered by one or more seccomp filters. This
way, each RET_LANDLOCK (with specific cookie) will trigger all the
allowed Landlock programs once.
Changes since v2:
* Landlock programs can now be run without seccomp filter but for any
syscall (from the process) or interruption
* move Landlock related functions and structs into security/landlock/*
(to manage cgroups as well)
* fix seccomp filter handling: run Landlock programs for each of their
legitimate seccomp filter
* properly clean up all seccomp results
* cosmetic changes to ease the understanding
* fix some ifdef
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Kees Cook <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Will Drewry <wad@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
include/linux/landlock.h | 77 ++++++++++++++
include/linux/seccomp.h | 26 +++++
include/uapi/linux/seccomp.h | 2 +
kernel/fork.c | 23 +++-
kernel/seccomp.c | 68 +++++++++++-
security/landlock/Makefile | 2 +-
security/landlock/common.h | 27 +++++
security/landlock/lsm.c | 96 ++++++++++++++++-
security/landlock/manager.c | 242 +++++++++++++++++++++++++++++++++++++++++++
9 files changed, 552 insertions(+), 11 deletions(-)
create mode 100644 include/linux/landlock.h
create mode 100644 security/landlock/common.h
create mode 100644 security/landlock/manager.c
@@ -1284,7 +1289,7 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)return0;}-staticvoidcopy_seccomp(structtask_struct*p)+staticintcopy_seccomp(structtask_struct*p){#ifdef CONFIG_SECCOMP/*
@@ -1297,7 +1302,14 @@ static void copy_seccomp(struct task_struct *p)/* Ref-count the new filter user, and assign it. */get_seccomp_filter(current);-p->seccomp=current->seccomp;+p->seccomp.mode=current->seccomp.mode;+p->seccomp.filter=current->seccomp.filter;+#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_SECURITY_LANDLOCK)+/* No copy for thread_filter nor landlock_ret. */+p->seccomp.landlock_hooks=current->seccomp.landlock_hooks;+if(p->seccomp.landlock_hooks)+atomic_inc(&p->seccomp.landlock_hooks->usage);+#endif /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK *//**Explicitlyenableno_new_privshereincaseitgotset
@@ -32,12 +34,11 @@#include<linux/security.h>#include<linux/tracehook.h>#include<linux/uaccess.h>+#include<linux/landlock.h>/* Limit any path through the tree to 256KB worth of instructions. */#define MAX_INSNS_PER_PATH ((1 << 18) / sizeof(struct sock_filter))-staticvoidput_seccomp_filter(structseccomp_filter*filter);-/**EndiannessisexplicitlyignoredandleftforBPFprogramauthorstomanage*asperthespecificarchitecture.
@@ -424,6 +467,11 @@ static long seccomp_attach_filter(unsigned int flags,*/filter->prev=current->seccomp.filter;current->seccomp.filter=filter;+#ifdef CONFIG_SECURITY_LANDLOCK+/* Chain the filters from the same thread, if any. */+filter->thread_prev=current->seccomp.thread_filter;+current->seccomp.thread_filter=filter;+#endif /* CONFIG_SECURITY_LANDLOCK *//* Now that the new filter is in place, synchronize to all threads. */if(flags&SECCOMP_FILTER_FLAG_TSYNC)
@@ -451,14 +499,16 @@ static inline void seccomp_filter_free(struct seccomp_filter *filter)}/* put_seccomp_filter - decrements the ref count of a filter */-staticvoidput_seccomp_filter(structseccomp_filter*filter)+voidput_seccomp_filter(structseccomp_filter*filter){structseccomp_filter*orig=filter;/* Clean up single-reference branches iteratively. */while(orig&&atomic_dec_and_test(&orig->usage)){structseccomp_filter*freeme=orig;+orig=orig->prev;+/* must not put orig->thread_prev */seccomp_filter_free(freeme);}}
@@ -612,6 +666,8 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,return0;+caseSECCOMP_RET_LANDLOCK:+/* fall through */caseSECCOMP_RET_ALLOW:return0;
@@ -770,6 +826,10 @@ static long do_seccomp(unsigned int op, unsigned int flags,returnseccomp_set_mode_strict();caseSECCOMP_SET_MODE_FILTER:returnseccomp_set_mode_filter(flags,uargs);+#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_SECURITY_LANDLOCK)+caseSECCOMP_SET_LANDLOCK_HOOK:+returnlandlock_seccomp_set_hook(flags,uargs);+#endif /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK */default:return-EINVAL;}
@@ -62,10 +65,99 @@#define LANDLOCK_HOOK_INIT(NAME) LSM_HOOK_INIT(NAME, landlock_hook_##NAME)+/**+*landlock_run_prog_for_syscall-runLandlockprogramforasyscall+*+*@hook_idx:hookindexintherulesarray+*@ctx:non-NULLeBPFcontext;the"origin"fieldwillbeupdated+*@hooks:Landlockhookspointer+*/+staticu32landlock_run_prog_for_syscall(u32hook_idx,+structlandlock_data*ctx,structlandlock_hooks*hooks)+{+structlandlock_rule*rule;+u32cur_ret=0,ret=0;++if(!hooks)+return0;++for(rule=hooks->rules[hook_idx];rule&&!ret;rule=rule->prev){+if(!(rule->prog->subtype.landlock_hook.origin&ctx->origin))+continue;+cur_ret=BPF_PROG_RUN(rule->prog,(void*)ctx);+if(cur_ret>MAX_ERRNO)+ret=MAX_ERRNO;+else+ret=cur_ret;+}+returnret;+}staticintlandlock_run_prog(enumlandlock_hook_idhook_id,__u64args[6]){-return0;+u32cur_ret=0,ret=0;+#ifdef CONFIG_SECCOMP_FILTER+structlandlock_seccomp_ret*lr;+#endif /* CONFIG_SECCOMP_FILTER */+structlandlock_rule*rule;+u32hook_idx=get_index(hook_id);++structlandlock_datactx={+.hook=hook_id,+.cookie=0,+.args[0]=args[0],+.args[1]=args[1],+.args[2]=args[2],+.args[3]=args[3],+.args[4]=args[4],+.args[5]=args[5],+};++/* TODO: use lockless_dereference()? */++#ifdef CONFIG_SECCOMP_FILTER+/* seccomp triggers and landlock_ret cleanup */+ctx.origin=LANDLOCK_FLAG_ORIGIN_SECCOMP;+for(lr=current->seccomp.landlock_ret;lr;lr=lr->prev){+if(!lr->triggered)+continue;+lr->triggered=false;+/* clean up all seccomp results */+if(ret)+continue;+ctx.cookie=lr->cookie;+for(rule=current->seccomp.landlock_hooks->rules[hook_idx];+rule&&!ret;rule=rule->prev){+structseccomp_filter*filter;++if(!(rule->prog->subtype.landlock_hook.origin&+ctx.origin))+continue;+for(filter=rule->thread_filter;filter;+filter=filter->thread_prev){+if(rule->thread_filter!=lr->filter)+continue;+cur_ret=BPF_PROG_RUN(rule->prog,(void*)&ctx);+if(cur_ret>MAX_ERRNO)+ret=MAX_ERRNO;+else+ret=cur_ret;+/* walk to the next program */+break;+}+}+}+if(ret)+return-ret;+ctx.cookie=0;++/* syscall trigger */+ctx.origin=LANDLOCK_FLAG_ORIGIN_SYSCALL;+ret=landlock_run_prog_for_syscall(hook_idx,&ctx,+current->seccomp.landlock_hooks);+#endif /* CONFIG_SECCOMP_FILTER */++return-ret;}staticconststructbpf_func_proto*bpf_landlock_func_proto(
@@ -152,7 +244,7 @@ static struct security_hook_list landlock_hooks[] = {void__initlandlock_add_hooks(void){-pr_info("landlock: Becoming ready for sandboxing\n");+pr_info("landlock: Becoming ready to sandbox with seccomp\n");security_add_hooks(landlock_hooks,ARRAY_SIZE(landlock_hooks));}
@@ -175,7 +188,7 @@ static int landlock_run_prog(enum landlock_hook_id hook_id, __u64 args[6])#ifdef CONFIG_CGROUP_BPF/* syscall trigger */if(cgroup_bpf_enabled){-ctx.origin=LANDLOCK_FLAG_ORIGIN_SYSCALL;+ctx.origin=current_call;/* get the default cgroup associated with the current thread */cgrp=task_css_set(current)->dfl_cgrp;ret=landlock_run_prog_for_syscall(hook_idx,&ctx,
Add LSM hooks which can be used by userland through Landlock (eBPF)
programs. This programs are limited to a whitelist of functions (cf.
next commit). The eBPF program context is depicted by the struct
landlock_data (cf. include/uapi/linux/bpf.h):
* hook: LSM hook ID
* origin: what triggered this Landlock program (syscall, dedicated
seccomp return or interruption)
* cookie: the 16-bit value from the seccomp filter that triggered this
Landlock program
* args[6]: array of some LSM hook arguments
The LSM hook arguments can contain raw values as integers or
(unleakable) pointers. The only way to use the pointers are to pass them
to an eBPF function according to their types (e.g. the
bpf_landlock_cmp_fs_beneath_with_struct_file function can use a struct
file pointer).
For each Landlock program, the subtype allows to specify for which LSM
hook the program is dedicated thanks to the "id" field. The "origin"
field must contains each triggers for which the Landlock program will
be called (e.g. every syscall or/and seccomp filters returning
RET_LANDLOCK). The "access" bitfield can be used to allow a program to
access a specific feature from a Landlock hook (i.e. context value or
function). The flag guarding this feature may only be enabled according
to the capabilities of the process loading the program.
For now, there is three hooks for file system access control:
* file_open
* file_permission
* mmap_file
Changes since v2:
* use subtypes instead of dedicated eBPF program types for each hook
(suggested by Alexei Starovoitov)
* replace convert_ctx_access() with subtype check
* use an array of Landlock program list instead of a single list
* handle running Landlock programs without needing a seccomp filter
* use, check and expose "origin" to Landlock programs
* mask the unused struct cred * (suggested by Andy Lutomirski)
Changes since v1:
* revamp access control from a syscall-based to a LSM hooks-based
* do not use audit cache
* no race conditions by design
* architecture agnostic
* switch from cBPF to eBPF (suggested by Daniel Borkmann)
* new BPF context
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: James Morris <redacted>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Will Drewry <wad@chromium.org>
Link: https://lkml.kernel.org/r/20160827205559.GA43880@ast-mbp.thefacebook.com
Link: https://lkml.kernel.org/r/20160827180642.GA38754@ast-mbp.thefacebook.com
Link: https://lkml.kernel.org/r/CALCETrUK1umtXMEXXKzMAccNQCVTPA8_XNDf01B5=gAZuJwvsQ@mail.gmail.com
Link: https://lkml.kernel.org/r/20160827204307.GA43714@ast-mbp.thefacebook.com
---
include/linux/bpf.h | 5 +
include/linux/lsm_hooks.h | 5 +
include/uapi/linux/bpf.h | 37 ++++++++
kernel/bpf/syscall.c | 10 +-
kernel/bpf/verifier.c | 6 ++
security/Makefile | 2 +
security/landlock/Makefile | 3 +
security/landlock/lsm.c | 222 +++++++++++++++++++++++++++++++++++++++++++++
security/security.c | 1 +
9 files changed, 289 insertions(+), 2 deletions(-)
create mode 100644 security/landlock/Makefile
create mode 100644 security/landlock/lsm.c
@@ -85,6 +85,8 @@ enum bpf_arg_type {ARG_PTR_TO_CTX,/* pointer to context */ARG_ANYTHING,/* any (initialized) argument is ok */++ARG_PTR_TO_STRUCT_FILE,/* pointer to struct file */};/* type of values returned from helper functions */
Add eBPF functions to compare file system access with a Landlock file
system handle:
* bpf_landlock_cmp_fs_prop_with_struct_file(prop, map, map_op, file)
This function allows to compare the dentry, inode, device or mount
point of the currently accessed file, with a reference handle.
* bpf_landlock_cmp_fs_beneath_with_struct_file(opt, map, map_op, file)
This function allows an eBPF program to check if the current accessed
file is the same or in the hierarchy of a reference handle.
The goal of file system handle is to abstract kernel objects such as a
struct file or a struct inode. Userland can create this kind of handle
thanks to the BPF_MAP_UPDATE_ELEM command. The element is a struct
landlock_handle containing the handle type (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) and a file descriptor. This could
also be any descriptions able to match a struct file or a struct inode
(e.g. path or glob string).
Changes since v2:
* add MNT_INTERNAL check to only add file handle from user-visible FS
(e.g. no anonymous inode)
* replace struct file* with struct path* in map_landlock_handle
* add BPF protos
* fix bpf_landlock_cmp_fs_prop_with_struct_file()
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: James Morris <redacted>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com
---
include/linux/bpf.h | 10 +++
include/uapi/linux/bpf.h | 49 +++++++++++
kernel/bpf/arraymap.c | 21 +++++
kernel/bpf/verifier.c | 8 ++
security/landlock/Makefile | 2 +-
security/landlock/checker_fs.c | 179 +++++++++++++++++++++++++++++++++++++++++
security/landlock/checker_fs.h | 20 +++++
security/landlock/lsm.c | 6 ++
8 files changed, 294 insertions(+), 1 deletion(-)
create mode 100644 security/landlock/checker_fs.c
create mode 100644 security/landlock/checker_fs.h
@@ -87,6 +87,7 @@ enum bpf_arg_type {ARG_ANYTHING,/* any (initialized) argument is ok */ARG_PTR_TO_STRUCT_FILE,/* pointer to struct file */+ARG_CONST_PTR_TO_LANDLOCK_HANDLE_FS,/* pointer to Landlock FS handle */};/* type of values returned from helper functions */
@@ -650,8 +657,22 @@ static inline long landlock_store_handle(struct map_landlock_handle *dst,structlandlock_handle*handle){enumbpf_map_handle_typehandle_type=handle->type;+structfile*handle_file;++/* access control already done for the FD */switch(handle_type){+caseBPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD:+FGET_OR_RET(handle_file,handle->fd);+/* check if the FD is tied to a user mount point */+if(unlikely(handle_file->f_path.mnt->mnt_flags&MNT_INTERNAL)){+fput(handle_file);+return-EINVAL;+}+path_get(&handle_file->f_path);+dst->path=handle_file->f_path;+fput(handle_file);+break;caseBPF_MAP_HANDLE_TYPE_UNSPEC:default:WARN_ON(1);
@@ -0,0 +1,179 @@+/*+*LandlockLSM-FileSystemCheckers+*+*Copyright(C)2016MickaëlSalaün<mic@digikod.net>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseversion2,as+*publishedbytheFreeSoftwareFoundation.+*/++#include<linux/bpf.h>/* enum bpf_map_array_op */+#include<linux/errno.h>+#include<linux/fs.h>/* path_is_under() */+#include<linux/path.h>/* struct path */++#include"checker_fs.h"++#define EQUAL_NOT_NULL(a, b) (a && a == b)++/*+*bpf_landlock_cmp_fs_prop_with_struct_file+*+*Cf.include/uapi/linux/bpf.h+*/+staticinlineu64bpf_landlock_cmp_fs_prop_with_struct_file(u64r1_property,+u64r2_map,u64r3_map_op,u64r4_file,u64r5)+{+u8property=(u8)r1_property;+structbpf_map*map=(structbpf_map*)(unsignedlong)r2_map;+enumbpf_map_array_opmap_op=r3_map_op;+structfile*file=(structfile*)(unsignedlong)r4_file;+structbpf_array*array=container_of(map,structbpf_array,map);+structpath*p1,*p2;+structmap_landlock_handle*handle;+inti;++/* ARG_CONST_PTR_TO_LANDLOCK_HANDLE_FS is an arraymap */+if(unlikely(!map)){+WARN_ON(1);+return-EFAULT;+}+if(unlikely(!file))+return-ENOENT;+if(unlikely((property|_LANDLOCK_FLAG_FS_MASK)!=_LANDLOCK_FLAG_FS_MASK))+return-EINVAL;++/* for now, only handle OP_OR */+switch(map_op){+caseBPF_MAP_ARRAY_OP_OR:+break;+caseBPF_MAP_ARRAY_OP_UNSPEC:+caseBPF_MAP_ARRAY_OP_AND:+caseBPF_MAP_ARRAY_OP_XOR:+default:+return-EINVAL;+}+p2=&file->f_path;++synchronize_rcu();++for(i=0;i<array->n_entries;i++){+boolresult_dentry=!(property&LANDLOCK_FLAG_FS_DENTRY);+boolresult_inode=!(property&LANDLOCK_FLAG_FS_INODE);+boolresult_device=!(property&LANDLOCK_FLAG_FS_DEVICE);+boolresult_mount=!(property&LANDLOCK_FLAG_FS_MOUNT);++handle=(structmap_landlock_handle*)+(array->value+array->elem_size*i);++if(handle->type!=BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD){+WARN_ON(1);+return-EFAULT;+}+p1=&handle->path;++if(!result_dentry&&p1->dentry==p2->dentry)+result_dentry=true;+/* TODO: use d_inode_rcu() instead? */+if(!result_inode+&&EQUAL_NOT_NULL(d_inode(p1->dentry)->i_ino,+d_inode(p2->dentry)->i_ino))+result_inode=true;+/* check superblock instead of device major/minor */+if(!result_device+&&EQUAL_NOT_NULL(d_inode(p1->dentry)->i_sb,+d_inode(p2->dentry)->i_sb))+result_device=true;+if(!result_mount&&EQUAL_NOT_NULL(p1->mnt,p2->mnt))+result_mount=true;+if(result_dentry&&result_inode&&result_device&&result_mount)+return0;+}+return1;+}++conststructbpf_func_protobpf_landlock_cmp_fs_prop_with_struct_file_proto={+.func=bpf_landlock_cmp_fs_prop_with_struct_file,+.gpl_only=true,+.ret_type=RET_INTEGER,+.arg1_type=ARG_ANYTHING,+.arg2_type=ARG_CONST_PTR_TO_LANDLOCK_HANDLE_FS,+.arg3_type=ARG_ANYTHING,+.arg4_type=ARG_PTR_TO_STRUCT_FILE,+};++/*+*bpf_landlock_cmp_fs_beneath_with_struct_file+*+*Cf.include/uapi/linux/bpf.h+*/+staticinlineu64bpf_landlock_cmp_fs_beneath_with_struct_file(u64r1_option,+u64r2_map,u64r3_map_op,u64r4_file,u64r5)+{+u8option=(u8)r1_option;+structbpf_map*map=(structbpf_map*)(unsignedlong)r2_map;+enumbpf_map_array_opmap_op=r3_map_op;+structfile*file=(structfile*)(unsignedlong)r4_file;+structbpf_array*array=container_of(map,structbpf_array,map);+structpath*p1,*p2;+structmap_landlock_handle*handle;+inti;++/* ARG_CONST_PTR_TO_LANDLOCK_HANDLE_FS is an arraymap */+if(unlikely(!map)){+WARN_ON(1);+return-EFAULT;+}+/* @file can be null for anonymous mmap */+if(unlikely(!file))+return-ENOENT;+if(unlikely((option|_LANDLOCK_FLAG_OPT_MASK)!=_LANDLOCK_FLAG_OPT_MASK))+return-EINVAL;++/* for now, only handle OP_OR */+switch(map_op){+caseBPF_MAP_ARRAY_OP_OR:+break;+caseBPF_MAP_ARRAY_OP_UNSPEC:+caseBPF_MAP_ARRAY_OP_AND:+caseBPF_MAP_ARRAY_OP_XOR:+default:+return-EINVAL;+}+/* p1 and p2 will be set correctly in the loop */+p1=&file->f_path;+p2=p1;++synchronize_rcu();++for(i=0;i<array->n_entries;i++){+handle=(structmap_landlock_handle*)+(array->value+array->elem_size*i);++/* protected by the proto types, should not happen */+if(unlikely(handle->type!=BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD)){+WARN_ON(1);+return-EINVAL;+}++if(option&LANDLOCK_FLAG_OPT_REVERSE)+p2=&handle->path;+else+p1=&handle->path;++if(path_is_under(p2,p1))+return0;+}+return1;+}++conststructbpf_func_protobpf_landlock_cmp_fs_beneath_with_struct_file_proto={+.func=bpf_landlock_cmp_fs_beneath_with_struct_file,+.gpl_only=true,+.ret_type=RET_INTEGER,+.arg1_type=ARG_ANYTHING,+.arg2_type=ARG_CONST_PTR_TO_LANDLOCK_HANDLE_FS,+.arg3_type=ARG_ANYTHING,+.arg4_type=ARG_PTR_TO_STRUCT_FILE,+};
This allows CONFIG_CGROUP_BPF to manage different type of pointers
instead of only eBPF programs. This will be useful for the next commits
to support Landlock with cgroups.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Daniel Mack <daniel@zonque.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Tejun Heo <tj@kernel.org>
---
include/linux/bpf-cgroup.h | 8 ++++++--
kernel/bpf/cgroup.c | 44 +++++++++++++++++++++++---------------------
2 files changed, 29 insertions(+), 23 deletions(-)
@@ -87,32 +88,33 @@ void __cgroup_bpf_update(struct cgroup *cgrp,structbpf_prog*prog,enumbpf_attach_typetype){-structbpf_prog*old_prog,*effective;+unionbpf_objectobj,old_pinned,effective;structcgroup_subsys_state*pos;-old_prog=xchg(cgrp->bpf.prog+type,prog);+obj.prog=prog;+old_pinned=xchg(cgrp->bpf.pinned+type,obj);-effective=(!prog&&parent)?-rcu_dereference_protected(parent->bpf.effective[type],+effective.prog=(!obj.prog&&parent)?+rcu_dereference_protected(parent->bpf.effective[type].prog,lockdep_is_held(&cgroup_mutex)):-prog;+obj.prog;css_for_each_descendant_pre(pos,&cgrp->self){structcgroup*desc=container_of(pos,structcgroup,self);/* skip the subtree if the descendant has its own program */-if(desc->bpf.prog[type]&&desc!=cgrp)+if(desc->bpf.pinned[type].prog&&desc!=cgrp)pos=css_rightmost_descendant(pos);else-rcu_assign_pointer(desc->bpf.effective[type],-effective);+rcu_assign_pointer(desc->bpf.effective[type].prog,+effective.prog);}-if(prog)+if(obj.prog)static_branch_inc(&cgroup_bpf_enabled_key);-if(old_prog){-bpf_prog_put(old_prog);+if(old_pinned.prog){+bpf_prog_put(old_pinned.prog);static_branch_dec(&cgroup_bpf_enabled_key);}}
@@ -151,7 +153,7 @@ int __cgroup_bpf_run_filter(struct sock *sk,rcu_read_lock();-prog=rcu_dereference(cgrp->bpf.effective[type]);+prog=rcu_dereference(cgrp->bpf.effective[type].prog);if(prog){unsignedintoffset=skb->data-skb_mac_header(skb);
This new arraymap looks like a set and brings new properties:
* strong typing of entries: the eBPF functions get the array type of
elements instead of CONST_PTR_TO_MAP (e.g.
CONST_PTR_TO_LANDLOCK_HANDLE_FS);
* force sequential filling (i.e. replace or append-only update), which
allow quick browsing of all entries.
This strong typing is useful to statically check if the content of a map
can be passed to an eBPF function. For example, Landlock use it to store
and manage kernel objects (e.g. struct file) instead of dealing with
userland raw data. This improve efficiency and ensure that an eBPF
program can only call functions with the right high-level arguments.
The enum bpf_map_handle_type list low-level types (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) which are identified when
updating a map entry (handle). This handle types are used to infer a
high-level arraymap type which are listed in enum bpf_map_array_type
(e.g. BPF_MAP_ARRAY_TYPE_LANDLOCK_FS).
For now, this new arraymap is only used by Landlock LSM (cf. next
commits) but it could be useful for other needs.
Changes since v2:
* add a RLIMIT_NOFILE-based limit to the maximum number of arraymap
handle entries (suggested by Andy Lutomirski)
* remove useless checks
Changes since v1:
* arraymap of handles replace custom checker groups
* simpler userland API
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com
---
include/linux/bpf.h | 14 ++++
include/uapi/linux/bpf.h | 18 +++++
kernel/bpf/arraymap.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/verifier.c | 12 ++-
4 files changed, 246 insertions(+), 1 deletion(-)
@@ -187,6 +192,9 @@ struct bpf_array {*/enumbpf_prog_typeowner_prog_type;boolowner_jited;+#ifdef CONFIG_SECURITY_LANDLOCK+u32n_entries;/* number of entries in a handle array */+#endif /* CONFIG_SECURITY_LANDLOCK */union{charvalue[0]__aligned(8);void*ptrs[0]__aligned(8);
@@ -580,3 +587,199 @@ static int __init register_cgroup_array_map(void)}late_initcall(register_cgroup_array_map);#endif++#ifdef CONFIG_SECURITY_LANDLOCK+staticstructbpf_map*landlock_array_map_alloc(unionbpf_attr*attr)+{+if(attr->value_size!=sizeof(structlandlock_handle))+returnERR_PTR(-EINVAL);+attr->value_size=sizeof(structmap_landlock_handle);++returnarray_map_alloc(attr);+}++staticvoidlandlock_put_handle(structmap_landlock_handle*handle)+{+enumbpf_map_handle_typehandle_type=handle->type;++switch(handle_type){+caseBPF_MAP_HANDLE_TYPE_UNSPEC:+default:+WARN_ON(1);+}+/* safeguard */+handle->type=BPF_MAP_HANDLE_TYPE_UNSPEC;+}++staticvoidlandlock_array_map_free(structbpf_map*map)+{+structbpf_array*array=container_of(map,structbpf_array,map);+inti;++synchronize_rcu();++for(i=0;i<array->n_entries;i++)+landlock_put_handle((structmap_landlock_handle*)+(array->value+array->elem_size*i));+kvfree(array);+}++staticenumbpf_map_array_typelandlock_get_array_type(+enumbpf_map_handle_typehandle_type)+{+switch(handle_type){+caseBPF_MAP_HANDLE_TYPE_UNSPEC:+default:+return-EINVAL;+}+}++#define FGET_OR_RET(file, fd) { \+file=fget(fd);\+if(unlikely(IS_ERR(file)))\+returnPTR_ERR(file);\+}++/**+*landlock_store_handle-storeanuserhandleinanarraymapentry+*+*@dst:non-NULLkernel-sideLandlockhandledestination+*@handle:non-NULLuser-sideLandlockhandlesource+*/+staticinlinelonglandlock_store_handle(structmap_landlock_handle*dst,+structlandlock_handle*handle)+{+enumbpf_map_handle_typehandle_type=handle->type;++switch(handle_type){+caseBPF_MAP_HANDLE_TYPE_UNSPEC:+default:+WARN_ON(1);+return-EINVAL;+}+dst->type=handle_type;+return0;+}++staticvoid*nop_map_lookup_elem(structbpf_map*map,void*key)+{+returnERR_PTR(-EINVAL);+}++/* called from syscall or from eBPF program */+staticintlandlock_array_map_update_elem(structbpf_map*map,void*key,+void*value,u64map_flags)+{+structbpf_array*array=container_of(map,structbpf_array,map);+u32index=*(u32*)key;+enumbpf_map_array_typearray_type;+intret;+structlandlock_handle*khandle=(structlandlock_handle*)value;+structmap_landlock_handle*handle,handle_new;++if(unlikely(map_flags>BPF_EXIST))+/* unknown flags */+return-EINVAL;++/*+*Limitnumberofentriesinanarraymapofhandlestothemaximum+*numberofopenfilesforthecurrentprocess.Themaximumnumberof+*handleentries(includingallarraymaps)foraprocessisthen+*(RLIMIT_NOFILE-1)*RLIMIT_NOFILE.Iftheprocess'RLIMIT_NOFILE+*is0,thenanyentryupdateisforbidden.+*+*AneBPFprogramcaninheritallthearraymapFD.Theworsecaseis+*tofillabunchofarraymaps,createaneBPFprogram,closethe+*arraymapFDs,andstartagain.Themaximumnumberofarraymap+*entriescanthenbeclosetoRLIMIT_NOFILE^3.+*+*FIXME:Thisshouldbeimproved...anyidea?+*/+if(unlikely(index>=rlimit(RLIMIT_NOFILE)))+return-EMFILE;++if(unlikely(index>=array->map.max_entries))+/* all elements were pre-allocated, cannot insert a new one */+return-E2BIG;++/* FIXME: add lock */+if(unlikely(index>array->n_entries))+/* only replace an existing entry or append a new one */+return-EINVAL;++/* TODO: handle all flags, not only BPF_ANY */+if(unlikely(map_flags==BPF_NOEXIST))+/* all elements already exist */+return-EEXIST;++if(unlikely(!khandle))+return-EINVAL;++array_type=landlock_get_array_type(khandle->type);+if(array_type<0)+returnarray_type;++if(!map->map_array_type){+/* set the initial set type */+map->map_array_type=array_type;+}elseif(map->map_array_type!=array_type){+return-EINVAL;+}++ret=landlock_store_handle(&handle_new,khandle);+if(!ret){+/* map->value_size == sizeof(struct map_landlock_handle) */+handle=(structmap_landlock_handle*)+(array->value+array->elem_size*index);+/* FIXME: make atomic update */+if(index<array->n_entries)+landlock_put_handle(handle);+*handle=handle_new;+/* TODO: use atomic_inc? */+if(index==array->n_entries)+array->n_entries++;+}+/* FIXME: unlock */++returnret;+}++/* called from syscall or from eBPF program */+staticintlandlock_array_map_delete_elem(structbpf_map*map,void*key)+{+structbpf_array*array=container_of(map,structbpf_array,map);+u32index=*(u32*)key;++/* only remove the last element */+/* TODO: use atomic_dec? */+if(array->n_entries&&index==array->n_entries-1){+array->n_entries--;+landlock_put_handle((structmap_landlock_handle*)+(array->value+array->elem_size*index));+return0;+}+return-EINVAL;+}++staticconststructbpf_map_opslandlock_array_ops={+.map_alloc=landlock_array_map_alloc,+.map_free=landlock_array_map_free,+.map_get_next_key=array_map_get_next_key,+.map_lookup_elem=nop_map_lookup_elem,+.map_update_elem=landlock_array_map_update_elem,+.map_delete_elem=landlock_array_map_delete_elem,+};++staticstructbpf_map_type_listlandlock_array_type__read_mostly={+.ops=&landlock_array_ops,+.type=BPF_MAP_TYPE_LANDLOCK_ARRAY,+};++staticint__initregister_landlock_array_map(void)+{+bpf_register_map_type(&landlock_array_type);+return0;+}++late_initcall(register_landlock_array_map);+#endif /* CONFIG_SECURITY_LANDLOCK */
@@ -1819,8 +1828,9 @@ static int check_ld_imm(struct verifier_env *env, struct bpf_insn *insn)/* replace_map_fd_with_map_ptr() should have caught bad ld_imm64 */BUG_ON(insn->src_reg!=BPF_PSEUDO_MAP_FD);-regs[insn->dst_reg].type=CONST_PTR_TO_MAP;regs[insn->dst_reg].map_ptr=ld_imm64_to_map_ptr(insn);+regs[insn->dst_reg].type=+bpf_reg_type_from_map(regs[insn->dst_reg].map_ptr);return0;}
@@ -843,23 +843,24 @@ static int bpf_prog_attach(const union bpf_attr *attr)caseBPF_CGROUP_INET_EGRESS:prog=bpf_prog_get_type(attr->attach_bpf_fd,BPF_PROG_TYPE_CGROUP_SOCKET);-if(IS_ERR(prog))-returnPTR_ERR(prog);--cgrp=cgroup_get_from_fd(attr->target_fd);-if(IS_ERR(cgrp)){-bpf_prog_put(prog);-returnPTR_ERR(cgrp);-}--cgroup_bpf_update(cgrp,prog,attr->attach_type);-cgroup_put(cgrp);break;default:return-EINVAL;}+if(IS_ERR(prog))+returnPTR_ERR(prog);++cgrp=cgroup_get_from_fd(attr->target_fd);+if(IS_ERR(cgrp)){+bpf_prog_put(prog);+returnPTR_ERR(cgrp);+}++cgroup_bpf_update(cgrp,prog,attr->attach_type);+cgroup_put(cgrp);+return0;}
@@ -36,6 +36,8 @@/* Limit any path through the tree to 256KB worth of instructions. */#define MAX_INSNS_PER_PATH ((1 << 18) / sizeof(struct sock_filter))+staticvoidput_seccomp_filter(structseccomp_filter*filter);+/**EndiannessisexplicitlyignoredandleftforBPFprogramauthorstomanage*asperthespecificarchitecture.
This will be useful to be able to add more BPF attach type with
different capability checks.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Daniel Mack <daniel@zonque.org>
---
kernel/bpf/syscall.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
@@ -833,15 +833,15 @@ static int bpf_prog_attach(const union bpf_attr *attr)structcgroup*cgrp;intresult;-if(!capable(CAP_NET_ADMIN))-return-EPERM;-if(CHECK_ATTR(BPF_PROG_ATTACH))return-EINVAL;switch(attr->attach_type){caseBPF_CGROUP_INET_INGRESS:caseBPF_CGROUP_INET_EGRESS:+if(!capable(CAP_NET_ADMIN))+return-EPERM;+prog=bpf_prog_get_type(attr->attach_bpf_fd,BPF_PROG_TYPE_CGROUP_SOCKET);break;
@@ -872,15 +872,15 @@ static int bpf_prog_detach(const union bpf_attr *attr)structcgroup*cgrp;intresult=0;-if(!capable(CAP_NET_ADMIN))-return-EPERM;-if(CHECK_ATTR(BPF_PROG_DETACH))return-EINVAL;switch(attr->attach_type){caseBPF_CGROUP_INET_INGRESS:caseBPF_CGROUP_INET_EGRESS:+if(!capable(CAP_NET_ADMIN))+return-EPERM;+cgrp=cgroup_get_from_fd(attr->target_fd);if(IS_ERR(cgrp))returnPTR_ERR(cgrp);
This will be useful to support Landlock for the next commits.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Daniel Mack <daniel@zonque.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Tejun Heo <tj@kernel.org>
---
include/linux/bpf-cgroup.h | 4 ++--
kernel/bpf/cgroup.c | 3 ++-
kernel/bpf/syscall.c | 10 ++++++----
kernel/cgroup.c | 6 ++++--
4 files changed, 14 insertions(+), 9 deletions(-)
@@ -831,6 +831,7 @@ static int bpf_prog_attach(const union bpf_attr *attr){structbpf_prog*prog;structcgroup*cgrp;+intresult;if(!capable(CAP_NET_ADMIN))return-EPERM;
@@ -858,10 +859,10 @@ static int bpf_prog_attach(const union bpf_attr *attr)returnPTR_ERR(cgrp);}-cgroup_bpf_update(cgrp,prog,attr->attach_type);+result=cgroup_bpf_update(cgrp,prog,attr->attach_type);cgroup_put(cgrp);-return0;+returnresult;}#define BPF_PROG_DETACH_LAST_FIELD attach_type
@@ -869,6 +870,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)staticintbpf_prog_detach(constunionbpf_attr*attr){structcgroup*cgrp;+intresult=0;if(!capable(CAP_NET_ADMIN))return-EPERM;
@@ -883,7 +885,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)if(IS_ERR(cgrp))returnPTR_ERR(cgrp);-cgroup_bpf_update(cgrp,NULL,attr->attach_type);+result=cgroup_bpf_update(cgrp,NULL,attr->attach_type);cgroup_put(cgrp);break;
@@ -891,7 +893,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)return-EINVAL;}-return0;+returnresult;}#endif /* CONFIG_CGROUP_BPF */
The program subtype goal is to be able to have different static
fine-grained verifications for a unique program type.
The struct bpf_verifier_ops gets a new optional function:
is_valid_subtype(). This new verifier is called at the begening of the
eBPF program verification to check if the (optional) program subtype is
valid.
For now, only Landlock eBPF programs are using a program subtype but
this could be used by other program types in the future.
Cf. the next commit to see how the subtype is used by Landlock LSM.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lkml.kernel.org/r/20160827205559.GA43880@ast-mbp.thefacebook.com
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
---
include/linux/bpf.h | 8 ++++++--
include/linux/filter.h | 1 +
include/uapi/linux/bpf.h | 9 +++++++++
kernel/bpf/syscall.c | 5 +++--
kernel/bpf/verifier.c | 9 +++++++--
kernel/trace/bpf_trace.c | 12 ++++++++----
net/core/filter.c | 21 +++++++++++++--------
7 files changed, 47 insertions(+), 18 deletions(-)
@@ -149,17 +149,21 @@ struct bpf_prog;structbpf_verifier_ops{/* return eBPF function prototype for verification */-conststructbpf_func_proto*(*get_func_proto)(enumbpf_func_idfunc_id);+conststructbpf_func_proto*(*get_func_proto)(enumbpf_func_idfunc_id,+unionbpf_prog_subtype*prog_subtype);/* return true if 'size' wide access at offset 'off' within bpf_context*with'type'(readorwrite)isallowed*/bool(*is_valid_access)(intoff,intsize,enumbpf_access_typetype,-enumbpf_reg_type*reg_type);+enumbpf_reg_type*reg_type,+unionbpf_prog_subtype*prog_subtype);u32(*convert_ctx_access)(enumbpf_access_typetype,intdst_reg,intsrc_reg,intctx_off,structbpf_insn*insn,structbpf_prog*prog);++bool(*is_valid_subtype)(unionbpf_prog_subtype*prog_subtype);};structbpf_prog_type_list{
@@ -406,6 +406,7 @@ struct bpf_prog {kmemcheck_bitfield_end(meta);u32len;/* Number of filter blocks */enumbpf_prog_typetype;/* Type of BPF program */+unionbpf_prog_subtypesubtype;/* For fine-grained verifications */structbpf_prog_aux*aux;/* Auxiliary fields */structsock_fprog_kern*orig_prog;/* Original BPF program */unsignedint(*bpf_func)(conststructsk_buff*skb,
@@ -127,6 +127,14 @@ enum bpf_attach_type {#define BPF_F_NO_PREALLOC (1U << 0)+unionbpf_prog_subtype{+struct{+__u32id;/* enum landlock_hook_id */+__u16origin;/* LANDLOCK_FLAG_ORIGIN_* */+__aligned_u64access;/* LANDLOCK_FLAG_ACCESS_* */+}landlock_hook;+}__attribute__((aligned(8)));+unionbpf_attr{struct{/* anonymous struct used by BPF_MAP_CREATE command */__u32map_type;/* one of enum bpf_map_type */
@@ -155,6 +163,7 @@ union bpf_attr {__u32log_size;/* size of user buffer */__aligned_u64log_buf;/* user supplied buffer */__u32kern_version;/* checked when prog_type=kprobe */+unionbpf_prog_subtypeprog_subtype;/* checked when prog_type=landlock */};struct{/* anonymous struct used by BPF_OBJ_* commands */
@@ -572,7 +572,7 @@ static void fixup_bpf_calls(struct bpf_prog *prog)continue;}-fn=prog->aux->ops->get_func_proto(insn->imm);+fn=prog->aux->ops->get_func_proto(insn->imm,&prog->subtype);/* all functions that have prototype and verifier allowed*programstocallthem,mustberealin-kernelfunctions*/
@@ -710,7 +710,7 @@ struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type)EXPORT_SYMBOL_GPL(bpf_prog_get_type);/* last field in 'union bpf_attr' used by this command */-#define BPF_PROG_LOAD_LAST_FIELD kern_version+#define BPF_PROG_LOAD_LAST_FIELD prog_subtypestaticintbpf_prog_load(unionbpf_attr*attr){
@@ -768,6 +768,7 @@ static int bpf_prog_load(union bpf_attr *attr)err=find_prog_type(type,prog);if(err<0)gotofree_prog;+prog->subtype=attr->prog_subtype;/* run eBPF verifier */err=bpf_check(&prog,attr);
@@ -684,7 +684,8 @@ static int check_ctx_access(struct verifier_env *env, int off, int size,enumbpf_access_typet,enumbpf_reg_type*reg_type){if(env->prog->aux->ops->is_valid_access&&-env->prog->aux->ops->is_valid_access(off,size,t,reg_type)){+env->prog->aux->ops->is_valid_access(off,size,t,reg_type,+&env->prog->subtype)){/* remember the offset of last byte accessed in ctx */if(env->prog->aux->max_ctx_offset<off+size)env->prog->aux->max_ctx_offset=off+size;
@@ -1173,7 +1174,7 @@ static int check_call(struct verifier_env *env, int func_id)}if(env->prog->aux->ops->get_func_proto)-fn=env->prog->aux->ops->get_func_proto(func_id);+fn=env->prog->aux->ops->get_func_proto(func_id,&env->prog->subtype);if(!fn){verbose("unknown func %d\n",func_id);
@@ -2768,6 +2769,10 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)if((*prog)->len<=0||(*prog)->len>BPF_MAXINSNS)return-E2BIG;+if((*prog)->aux->ops->is_valid_subtype&&+!(*prog)->aux->ops->is_valid_subtype(&(*prog)->subtype))+return-EINVAL;+/* 'struct verifier_env' can be global, but since it's not small,*allocate/freeiteverytimebpf_check()iscalled*/
From: David Laight <hidden> Date: 2016-09-14 14:39:02
From: Mickaël Salaün
Sent: 14 September 2016 08:24
...
## Why does seccomp-filter is not enough?
A seccomp filter can access to raw syscall arguments which means that it is not
possible to filter according to pointed data as a file path. As demonstrated
the first version of this patch series, filtering at the syscall level is
complicated (e.g. need to take care of race conditions). This is mainly because
the access control checkpoints of the kernel are not at this high-level but
more underneath, at LSM hooks level. The LSM hooks are designed to handle this
kind of checks. This series use this approach to leverage the ability of
unprivileged users to limit themselves.
You cannot validate file path parameters during syscall entry.
It can only be done after the user buffer has been read into kernel memory.
(ie you must only access the buffer once.)
This has nothing to do with where the kernel does any access checks,
and everything to do with the fact that another thread/process can
modify the buffer after you have validated it.
David
From: Andy Lutomirski <luto@amacapital.net> Date: 2016-09-14 18:29:55
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
This third origin of hook call should cover all possible trigger paths
(e.g. page fault). Landlock eBPF programs can then take decisions
accordingly.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Kees Cook <redacted>
---
+ if (unlikely(in_interrupt())) {
IMO security hooks have no business being called from interrupts.
Aren't they all synchronous things done by tasks? Interrupts are
driver things.
Are you trying to check for page faults and such?
--Andy
From: Andy Lutomirski <luto@amacapital.net> Date: 2016-09-14 18:33:41
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
Add a new flag CGRP_NO_NEW_PRIVS for each cgroup. This flag is initially
set for all cgroup except the root. The flag is clear when a new process
without the no_new_privs flags is attached to the cgroup.
If a cgroup is landlocked, then any new attempt, from an unprivileged
process, to attach a process without no_new_privs to this cgroup will
be denied.
Until and unless everyone can agree on a way to properly namespace,
delegate, etc cgroups, I think that trying to add unprivileged
semantics to cgroups is nuts. Given the big thread about cgroup v2,
no-internal-tasks, etc, I just don't see how this approach can be
viable.
Can we try to make landlock work completely independently of cgroups
so that it doesn't get stuck and so that programs can use it without
worrying about cgroup v1 vs v2, interactions with cgroup managers,
cgroup managers that (supposedly?) will start migrating processes
around piecemeal and almost certainly blowing up landlock in the
process, etc?
I have no problem with looking at prototypes for how landlock +
cgroups would work, but I can't imagine the result being mergeable.
From: Andy Lutomirski <luto@amacapital.net> Date: 2016-09-14 18:44:21
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
A Landlock program will be triggered according to its subtype/origin
bitfield. The LANDLOCK_FLAG_ORIGIN_SECCOMP value will trigger the
Landlock program when a seccomp filter will return RET_LANDLOCK.
Moreover, it is possible to return a 16-bit cookie which will be
readable by the Landlock programs in its context.
Are you envisioning that the filters will return RET_LANDLOCK most of
the time or rarely? If it's most of the time, then maybe this could
be simplified a bit by unconditionally calling the landlock filter and
letting the landlock filter access a struct seccomp_data if needed.
Only seccomp filters loaded from the same thread and before a Landlock
program can trigger it through LANDLOCK_FLAG_ORIGIN_SECCOMP. Multiple
Landlock programs can be triggered by one or more seccomp filters. This
way, each RET_LANDLOCK (with specific cookie) will trigger all the
allowed Landlock programs once.
This interface seems somewhat awkward. Should we not have a way to
atomicaly install a whole pile of landlock filters and associated
seccomp filter all at once?
--Andy
On Wed, Sep 14, 2016 at 09:23:56AM +0200, Mickaël Salaün wrote:
quoted hunk
This new arraymap looks like a set and brings new properties:
* strong typing of entries: the eBPF functions get the array type of
elements instead of CONST_PTR_TO_MAP (e.g.
CONST_PTR_TO_LANDLOCK_HANDLE_FS);
* force sequential filling (i.e. replace or append-only update), which
allow quick browsing of all entries.
This strong typing is useful to statically check if the content of a map
can be passed to an eBPF function. For example, Landlock use it to store
and manage kernel objects (e.g. struct file) instead of dealing with
userland raw data. This improve efficiency and ensure that an eBPF
program can only call functions with the right high-level arguments.
The enum bpf_map_handle_type list low-level types (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) which are identified when
updating a map entry (handle). This handle types are used to infer a
high-level arraymap type which are listed in enum bpf_map_array_type
(e.g. BPF_MAP_ARRAY_TYPE_LANDLOCK_FS).
For now, this new arraymap is only used by Landlock LSM (cf. next
commits) but it could be useful for other needs.
Changes since v2:
* add a RLIMIT_NOFILE-based limit to the maximum number of arraymap
handle entries (suggested by Andy Lutomirski)
* remove useless checks
Changes since v1:
* arraymap of handles replace custom checker groups
* simpler userland API
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com
---
include/linux/bpf.h | 14 ++++
include/uapi/linux/bpf.h | 18 +++++
kernel/bpf/arraymap.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/verifier.c | 12 ++-
4 files changed, 246 insertions(+), 1 deletion(-)
@@ -187,6 +192,9 @@ struct bpf_array {*/enumbpf_prog_typeowner_prog_type;boolowner_jited;+#ifdef CONFIG_SECURITY_LANDLOCK+u32n_entries;/* number of entries in a handle array */+#endif /* CONFIG_SECURITY_LANDLOCK */union{charvalue[0]__aligned(8);void*ptrs[0]__aligned(8);
missing something. why it has to be special to have it's own
fd array implementation?
Please take a look how BPF_MAP_TYPE_PERF_EVENT_ARRAY,
BPF_MAP_TYPE_CGROUP_ARRAY and BPF_MAP_TYPE_PROG_ARRAY are done.
The all store objects into array map that user space passes via FD.
I think the same model should apply here.
On Wed, Sep 14, 2016 at 09:24:00AM +0200, Mickaël Salaün wrote:
Add eBPF functions to compare file system access with a Landlock file
system handle:
* bpf_landlock_cmp_fs_prop_with_struct_file(prop, map, map_op, file)
This function allows to compare the dentry, inode, device or mount
point of the currently accessed file, with a reference handle.
* bpf_landlock_cmp_fs_beneath_with_struct_file(opt, map, map_op, file)
This function allows an eBPF program to check if the current accessed
file is the same or in the hierarchy of a reference handle.
Why is this safe? As far as I can tell, this is not in an RCU read-side
critical section (synchronize_rcu() was just called), and no lock has been
taken. What prevents someone from removing the arraymap entry while we're
looking at it? Am I missing something?
[...]
On Wed, Sep 14, 2016 at 09:24:00AM +0200, Mickaël Salaün wrote:
Add eBPF functions to compare file system access with a Landlock file
system handle:
* bpf_landlock_cmp_fs_prop_with_struct_file(prop, map, map_op, file)
This function allows to compare the dentry, inode, device or mount
point of the currently accessed file, with a reference handle.
* bpf_landlock_cmp_fs_beneath_with_struct_file(opt, map, map_op, file)
This function allows an eBPF program to check if the current accessed
file is the same or in the hierarchy of a reference handle.
The goal of file system handle is to abstract kernel objects such as a
struct file or a struct inode. Userland can create this kind of handle
thanks to the BPF_MAP_UPDATE_ELEM command. The element is a struct
landlock_handle containing the handle type (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) and a file descriptor. This could
also be any descriptions able to match a struct file or a struct inode
(e.g. path or glob string).
Changes since v2:
* add MNT_INTERNAL check to only add file handle from user-visible FS
(e.g. no anonymous inode)
* replace struct file* with struct path* in map_landlock_handle
* add BPF protos
* fix bpf_landlock_cmp_fs_prop_with_struct_file()
Signed-off-by: Mickaël Salaün <redacted>
Cc: Alexei Starovoitov <redacted>
Cc: Andy Lutomirski <redacted>
Cc: Daniel Borkmann <redacted>
Cc: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: James Morris <redacted>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <redacted>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
thanks for keeping the links to the previous discussion.
Long term it should help, though I worry we already at the point
where there are too many outstanding issues to resolve before we
can proceed with reasonable code review.
please use just added BPF_CALL_ macros. They will help readability of the above.
quoted hunk
+ struct bpf_array *array = container_of(map, struct bpf_array, map);+ struct path *p1, *p2;+ struct map_landlock_handle *handle;+ int i;++ /* ARG_CONST_PTR_TO_LANDLOCK_HANDLE_FS is an arraymap */+ if (unlikely(!map)) {+ WARN_ON(1);+ return -EFAULT;+ }+ if (unlikely(!file))+ return -ENOENT;+ if (unlikely((property | _LANDLOCK_FLAG_FS_MASK) != _LANDLOCK_FLAG_FS_MASK))+ return -EINVAL;++ /* for now, only handle OP_OR */+ switch (map_op) {+ case BPF_MAP_ARRAY_OP_OR:+ break;+ case BPF_MAP_ARRAY_OP_UNSPEC:+ case BPF_MAP_ARRAY_OP_AND:+ case BPF_MAP_ARRAY_OP_XOR:+ default:+ return -EINVAL;+ }+ p2 = &file->f_path;++ synchronize_rcu();
that is completely broken.
bpf programs are executing under rcu_lock.
please enable CONFIG_PROVE_RCU and retest everything.
I would suggest for the next RFC to do minimal 7 patches up to this point
with simple example that demonstrates the use case.
I would avoid all unpriv stuff and all of seccomp for the next RFC as well,
otherwise I don't think we can realistically make forward progress, since
there are too many issues raised in the subsequent patches.
The common part that is mergeable is prog's subtype extension to
the verifier that can be used for better tracing and is the common
piece of infra needed for both landlock and checmate LSMs
(which must be one LSM anyway)
On Wed, Sep 14, 2016 at 09:24:07AM +0200, Mickaël Salaün wrote:
This will be useful to support Landlock for the next commits.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Daniel Mack <daniel@zonque.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Tejun Heo <tj@kernel.org>
I think this is good to do regardless. Sooner or later cgroup_bpf_update
will start rejecting the prog attach. Like we discussed to have a flag
that would dissallow processeses lower in the cgroup hierarchy to install
their own bpf programs.
It will be minimal change to bpf_prog_attach() error handling,
but will greatly help Mickael to build stuff on top.
DanielM can you refactor your patch to do that from the start ?
Thanks!
@@ -831,6 +831,7 @@ static int bpf_prog_attach(const union bpf_attr *attr){structbpf_prog*prog;structcgroup*cgrp;+intresult;if(!capable(CAP_NET_ADMIN))return-EPERM;
@@ -858,10 +859,10 @@ static int bpf_prog_attach(const union bpf_attr *attr)returnPTR_ERR(cgrp);}-cgroup_bpf_update(cgrp,prog,attr->attach_type);+result=cgroup_bpf_update(cgrp,prog,attr->attach_type);cgroup_put(cgrp);-return0;+returnresult;}#define BPF_PROG_DETACH_LAST_FIELD attach_type
@@ -869,6 +870,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)staticintbpf_prog_detach(constunionbpf_attr*attr){structcgroup*cgrp;+intresult=0;if(!capable(CAP_NET_ADMIN))return-EPERM;
@@ -883,7 +885,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)if(IS_ERR(cgrp))returnPTR_ERR(cgrp);-cgroup_bpf_update(cgrp,NULL,attr->attach_type);+result=cgroup_bpf_update(cgrp,NULL,attr->attach_type);cgroup_put(cgrp);break;
@@ -891,7 +893,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)return-EINVAL;}-return0;+returnresult;}#endif /* CONFIG_CGROUP_BPF */
On Wed, Sep 14, 2016 at 09:24:14AM +0200, Mickaël Salaün wrote:
This is a proof of concept to expose optional values that could depend
of the process access rights.
There is two dedicated flags: LANDLOCK_FLAG_ACCESS_SKB_READ and
LANDLOCK_FLAG_ACCESS_SKB_WRITE. Each of them can be activated to access
eBPF functions manipulating a skb in a read or write way.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
@@ -619,12 +621,15 @@ struct landlock_handle { * @args: LSM hook arguments, see include/linux/lsm_hooks.h for there * description and the LANDLOCK_HOOK* definitions from * security/landlock/lsm.c for their types.+ * @opt_skb: optional skb pointer, accessible with the+ * LANDLOCK_FLAG_ACCESS_SKB_* flags for network-related hooks. */ struct landlock_data { __u32 hook; /* enum landlock_hook_id */ __u16 origin; /* LANDLOCK_FLAG_ORIGIN_* */ __u16 cookie; /* seccomp RET_LANDLOCK */ __u64 args[6];+ __u64 opt_skb; };
missing something here.
This patch doesn't make use of it.
That's something for the future?
How that field will be populated?
Why make it different vs the rest or args[6] ?
On Wed, Sep 14, 2016 at 09:24:15AM +0200, Mickaël Salaün wrote:
Add a basic sandbox tool to create a process isolated from some part of
the system. This can depend of the current cgroup.
Example with the current process hierarchy (seccomp):
$ ls /home
user1
$ LANDLOCK_ALLOWED='/bin:/lib:/usr:/tmp:/proc/self/fd/0' \
./samples/landlock/sandbox /bin/sh -i
Launching a new sandboxed process.
$ ls /home
ls: cannot open directory '/home': Permission denied
Example with a cgroup:
$ mkdir /sys/fs/cgroup/sandboxed
$ ls /home
user1
$ LANDLOCK_CGROUPS='/sys/fs/cgroup/sandboxed' \
LANDLOCK_ALLOWED='/bin:/lib:/usr:/tmp:/proc/self/fd/0' \
./samples/landlock/sandbox
Ready to sandbox with cgroups.
$ ls /home
user1
$ echo $$ > /sys/fs/cgroup/sandboxed/cgroup.procs
$ ls /home
ls: cannot open directory '/home': Permission denied
Changes since v2:
* use BPF_PROG_ATTACH for cgroup handling
Signed-off-by: Mickaël Salaün <mic@digikod.net>
...
quoted hunk
+ struct bpf_insn hook_path[] = {+ /* specify an option, if any */+ BPF_MOV32_IMM(BPF_REG_1, 0),+ /* handles to compare with */+ BPF_LD_MAP_FD(BPF_REG_2, map_fs),+ BPF_MOV64_IMM(BPF_REG_3, BPF_MAP_ARRAY_OP_OR),+ /* hook argument (struct file) */+ BPF_LDX_MEM(BPF_DW, BPF_REG_4, BPF_REG_6, offsetof(struct+ landlock_data, args[0])),+ /* checker function */+ BPF_EMIT_CALL(BPF_FUNC_landlock_cmp_fs_beneath_with_struct_file),
the example is sweet!
Since only that helper is used, could you skip the other one
from the patches (for now) ?
Add security access check for cgroup backed FD. The "cgroup.procs" file
of the corresponding cgroup must be readable to identify the cgroup, and
writable to prove that the current process can manage this cgroup (e.g.
through delegation). This is similar to the check done by
cgroup_procs_write_permission().
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Daniel Mack <daniel@zonque.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Cc: Tejun Heo <tj@kernel.org>
---
include/linux/cgroup.h | 2 +-
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/syscall.c | 6 +++---
kernel/cgroup.c | 16 +++++++++++++++-
4 files changed, 20 insertions(+), 6 deletions(-)
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
Add a new flag CGRP_NO_NEW_PRIVS for each cgroup. This flag is initially
set for all cgroup except the root. The flag is clear when a new process
without the no_new_privs flags is attached to the cgroup.
If a cgroup is landlocked, then any new attempt, from an unprivileged
process, to attach a process without no_new_privs to this cgroup will
be denied.
Until and unless everyone can agree on a way to properly namespace,
delegate, etc cgroups, I think that trying to add unprivileged
semantics to cgroups is nuts. Given the big thread about cgroup v2,
no-internal-tasks, etc, I just don't see how this approach can be
viable.
As far as I can tell, the no_new_privs flag of at task is not related to
namespaces. The CGRP_NO_NEW_PRIVS flag is only a cache to quickly access
the no_new_privs property of *tasks* in a cgroup. The semantic is unchanged.
Using cgroup is optional, any task could use the seccomp-based
landlocking instead. However, for those that want/need to manage a
security policy in a more dynamic way, using cgroups may make sense.
I though cgroup delegation was OK in the v2, isn't it the case? Do you
have some links?
Can we try to make landlock work completely independently of cgroups
so that it doesn't get stuck and so that programs can use it without
worrying about cgroup v1 vs v2, interactions with cgroup managers,
cgroup managers that (supposedly?) will start migrating processes
around piecemeal and almost certainly blowing up landlock in the
process, etc?
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
I have no problem with looking at prototypes for how landlock +
cgroups would work, but I can't imagine the result being mergeable.
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
This third origin of hook call should cover all possible trigger paths
(e.g. page fault). Landlock eBPF programs can then take decisions
accordingly.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Kees Cook <redacted>
---
quoted
+ if (unlikely(in_interrupt())) {
IMO security hooks have no business being called from interrupts.
Aren't they all synchronous things done by tasks? Interrupts are
driver things.
Are you trying to check for page faults and such?
Yes, that was the idea you did put in my mind. Not sure how to deal with
this.
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
A Landlock program will be triggered according to its subtype/origin
bitfield. The LANDLOCK_FLAG_ORIGIN_SECCOMP value will trigger the
Landlock program when a seccomp filter will return RET_LANDLOCK.
Moreover, it is possible to return a 16-bit cookie which will be
readable by the Landlock programs in its context.
Are you envisioning that the filters will return RET_LANDLOCK most of
the time or rarely? If it's most of the time, then maybe this could
be simplified a bit by unconditionally calling the landlock filter and
letting the landlock filter access a struct seccomp_data if needed.
Exposing seccomp_data in a Landlock context may be a good idea. The main
implication is that Landlock programs may then be architecture specific
(if dealing with data) as seccomp filters are. Another point is that it
remove any direct binding between seccomp filters and Landlock programs.
I will try this (more simple) approach.
quoted
Only seccomp filters loaded from the same thread and before a Landlock
program can trigger it through LANDLOCK_FLAG_ORIGIN_SECCOMP. Multiple
Landlock programs can be triggered by one or more seccomp filters. This
way, each RET_LANDLOCK (with specific cookie) will trigger all the
allowed Landlock programs once.
This interface seems somewhat awkward. Should we not have a way to
atomicaly install a whole pile of landlock filters and associated
seccomp filter all at once?
I can change the seccomp(2) use in this way: instead of loading a
Landlock program, (atomically) load an array of Landlock programs.
However, exposing seccomp_data to Landlock programs looks like a better
way to deal with it. This does not needs to manage an array of Landlock
programs.
Mickaël
On Wed, Sep 14, 2016 at 09:24:00AM +0200, Mickaël Salaün wrote:
quoted
Add eBPF functions to compare file system access with a Landlock file
system handle:
* bpf_landlock_cmp_fs_prop_with_struct_file(prop, map, map_op, file)
This function allows to compare the dentry, inode, device or mount
point of the currently accessed file, with a reference handle.
* bpf_landlock_cmp_fs_beneath_with_struct_file(opt, map, map_op, file)
This function allows an eBPF program to check if the current accessed
file is the same or in the hierarchy of a reference handle.
Why is this safe? As far as I can tell, this is not in an RCU read-side
critical section (synchronize_rcu() was just called), and no lock has been
taken. What prevents someone from removing the arraymap entry while we're
looking at it? Am I missing something?
On Wed, Sep 14, 2016 at 09:24:14AM +0200, Mickaël Salaün wrote:
quoted
This is a proof of concept to expose optional values that could depend
of the process access rights.
There is two dedicated flags: LANDLOCK_FLAG_ACCESS_SKB_READ and
LANDLOCK_FLAG_ACCESS_SKB_WRITE. Each of them can be activated to access
eBPF functions manipulating a skb in a read or write way.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
@@ -619,12 +621,15 @@ struct landlock_handle { * @args: LSM hook arguments, see include/linux/lsm_hooks.h for there * description and the LANDLOCK_HOOK* definitions from * security/landlock/lsm.c for their types.+ * @opt_skb: optional skb pointer, accessible with the+ * LANDLOCK_FLAG_ACCESS_SKB_* flags for network-related hooks. */ struct landlock_data { __u32 hook; /* enum landlock_hook_id */ __u16 origin; /* LANDLOCK_FLAG_ORIGIN_* */ __u16 cookie; /* seccomp RET_LANDLOCK */ __u64 args[6];+ __u64 opt_skb; };
missing something here.
This patch doesn't make use of it.
That's something for the future?
How that field will be populated?
Why make it different vs the rest or args[6] ?
I don't use this code, it's only purpose is to show how to deal with
fine-grained privileges of Landlock programs (to allow Sargun to add his
custom helpers from Checmate). However, this optional field may be part
of args[6].
On Wed, Sep 14, 2016 at 09:24:00AM +0200, Mickaël Salaün wrote:
quoted
Add eBPF functions to compare file system access with a Landlock file
system handle:
* bpf_landlock_cmp_fs_prop_with_struct_file(prop, map, map_op, file)
This function allows to compare the dentry, inode, device or mount
point of the currently accessed file, with a reference handle.
* bpf_landlock_cmp_fs_beneath_with_struct_file(opt, map, map_op, file)
This function allows an eBPF program to check if the current accessed
file is the same or in the hierarchy of a reference handle.
The goal of file system handle is to abstract kernel objects such as a
struct file or a struct inode. Userland can create this kind of handle
thanks to the BPF_MAP_UPDATE_ELEM command. The element is a struct
landlock_handle containing the handle type (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) and a file descriptor. This could
also be any descriptions able to match a struct file or a struct inode
(e.g. path or glob string).
Changes since v2:
* add MNT_INTERNAL check to only add file handle from user-visible FS
(e.g. no anonymous inode)
* replace struct file* with struct path* in map_landlock_handle
* add BPF protos
* fix bpf_landlock_cmp_fs_prop_with_struct_file()
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: James Morris <redacted>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com
thanks for keeping the links to the previous discussion.
Long term it should help, though I worry we already at the point
where there are too many outstanding issues to resolve before we
can proceed with reasonable code review.
please use just added BPF_CALL_ macros. They will help readability of the above.
quoted
+ struct bpf_array *array = container_of(map, struct bpf_array, map);+ struct path *p1, *p2;+ struct map_landlock_handle *handle;+ int i;++ /* ARG_CONST_PTR_TO_LANDLOCK_HANDLE_FS is an arraymap */+ if (unlikely(!map)) {+ WARN_ON(1);+ return -EFAULT;+ }+ if (unlikely(!file))+ return -ENOENT;+ if (unlikely((property | _LANDLOCK_FLAG_FS_MASK) != _LANDLOCK_FLAG_FS_MASK))+ return -EINVAL;++ /* for now, only handle OP_OR */+ switch (map_op) {+ case BPF_MAP_ARRAY_OP_OR:+ break;+ case BPF_MAP_ARRAY_OP_UNSPEC:+ case BPF_MAP_ARRAY_OP_AND:+ case BPF_MAP_ARRAY_OP_XOR:+ default:+ return -EINVAL;+ }+ p2 = &file->f_path;++ synchronize_rcu();
that is completely broken.
bpf programs are executing under rcu_lock.
please enable CONFIG_PROVE_RCU and retest everything.
Thanks for the tip. I will fix this.
I would suggest for the next RFC to do minimal 7 patches up to this point
with simple example that demonstrates the use case.
I would avoid all unpriv stuff and all of seccomp for the next RFC as well,
otherwise I don't think we can realistically make forward progress, since
there are too many issues raised in the subsequent patches.
I hope we will find a common agreement about seccomp vs cgroup… I think
both approaches have their advantages, can be complementary and nicely
combined.
Unprivileged sandboxing is the main goal of Landlock. This should not be
a problem, even for privileged features, thanks to the new subtype/access.
The common part that is mergeable is prog's subtype extension to
the verifier that can be used for better tracing and is the common
piece of infra needed for both landlock and checmate LSMs
(which must be one LSM anyway)
Agreed. With this RFC, the Checmate features (i.e. network helpers)
should be able to sit on top of Landlock.
On Wed, Sep 14, 2016 at 09:23:56AM +0200, Mickaël Salaün wrote:
quoted
This new arraymap looks like a set and brings new properties:
* strong typing of entries: the eBPF functions get the array type of
elements instead of CONST_PTR_TO_MAP (e.g.
CONST_PTR_TO_LANDLOCK_HANDLE_FS);
* force sequential filling (i.e. replace or append-only update), which
allow quick browsing of all entries.
This strong typing is useful to statically check if the content of a map
can be passed to an eBPF function. For example, Landlock use it to store
and manage kernel objects (e.g. struct file) instead of dealing with
userland raw data. This improve efficiency and ensure that an eBPF
program can only call functions with the right high-level arguments.
The enum bpf_map_handle_type list low-level types (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) which are identified when
updating a map entry (handle). This handle types are used to infer a
high-level arraymap type which are listed in enum bpf_map_array_type
(e.g. BPF_MAP_ARRAY_TYPE_LANDLOCK_FS).
For now, this new arraymap is only used by Landlock LSM (cf. next
commits) but it could be useful for other needs.
Changes since v2:
* add a RLIMIT_NOFILE-based limit to the maximum number of arraymap
handle entries (suggested by Andy Lutomirski)
* remove useless checks
Changes since v1:
* arraymap of handles replace custom checker groups
* simpler userland API
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com
---
include/linux/bpf.h | 14 ++++
include/uapi/linux/bpf.h | 18 +++++
kernel/bpf/arraymap.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/verifier.c | 12 ++-
4 files changed, 246 insertions(+), 1 deletion(-)
@@ -187,6 +192,9 @@ struct bpf_array {*/enumbpf_prog_typeowner_prog_type;boolowner_jited;+#ifdef CONFIG_SECURITY_LANDLOCK+u32n_entries;/* number of entries in a handle array */+#endif /* CONFIG_SECURITY_LANDLOCK */union{charvalue[0]__aligned(8);void*ptrs[0]__aligned(8);
missing something. why it has to be special to have it's own
fd array implementation?
Please take a look how BPF_MAP_TYPE_PERF_EVENT_ARRAY,
BPF_MAP_TYPE_CGROUP_ARRAY and BPF_MAP_TYPE_PROG_ARRAY are done.
The all store objects into array map that user space passes via FD.
I think the same model should apply here.
The idea is to have multiple way for userland to describe a resource
(e.g. an open file descriptor, a path or a glob pattern). The kernel
representation could then be a "struct path *" or dedicated types (e.g.
custom glob).
Another interesting point (that could replace
check_map_func_compatibility()) is that BPF_MAP_TYPE_LANDLOCK_ARRAY
translate to dedicated (abstract) types (instead of CONST_PTR_TO_MAP)
thanks to bpf_reg_type_from_map(). This is useful to abstract userland
(map) interface with kernel object(s) dealing with that type.
A third point is that BPF_MAP_TYPE_LANDLOCK_ARRAY is a kind of set. It
is optimized to quickly walk through all the elements in a sequential way.
On Thu, Sep 15, 2016 at 01:02:22AM +0200, Mickaël Salaün wrote:
quoted
I would suggest for the next RFC to do minimal 7 patches up to this point
with simple example that demonstrates the use case.
I would avoid all unpriv stuff and all of seccomp for the next RFC as well,
otherwise I don't think we can realistically make forward progress, since
there are too many issues raised in the subsequent patches.
I hope we will find a common agreement about seccomp vs cgroup… I think
both approaches have their advantages, can be complementary and nicely
combined.
I don't mind having both task based lsm and cgroup based as long as
infrastracture is not duplicated and scaling issues from earlier version
are resolved.
I'm proposing to do cgroup only for the next RFC, since mine and Sargun's
use case for this bpf+lsm+cgroup is _not_ security or sandboxing.
No need for unpriv, no_new_priv to cgroups are other things that Andy
is concerned about.
Unprivileged sandboxing is the main goal of Landlock. This should not be
a problem, even for privileged features, thanks to the new subtype/access.
yes. the point that unpriv stuff can come later after agreement is reached.
If we keep arguing about seccomp details this set won't go anywhere.
Even in basic part (which is cgroup+bpf+lsm) are plenty of questions
to be still agreed.
Agreed. With this RFC, the Checmate features (i.e. network helpers)
should be able to sit on top of Landlock.
I think neither of them should be called fancy names for no technical reason.
We will have only one bpf based lsm. That's it and it doesn't
need an obscure name. Directory name can be security/bpf/..stuff.c
On Thu, Sep 15, 2016 at 01:22:49AM +0200, Mickaël Salaün wrote:
On 14/09/2016 20:51, Alexei Starovoitov wrote:
quoted
On Wed, Sep 14, 2016 at 09:23:56AM +0200, Mickaël Salaün wrote:
quoted
This new arraymap looks like a set and brings new properties:
* strong typing of entries: the eBPF functions get the array type of
elements instead of CONST_PTR_TO_MAP (e.g.
CONST_PTR_TO_LANDLOCK_HANDLE_FS);
* force sequential filling (i.e. replace or append-only update), which
allow quick browsing of all entries.
This strong typing is useful to statically check if the content of a map
can be passed to an eBPF function. For example, Landlock use it to store
and manage kernel objects (e.g. struct file) instead of dealing with
userland raw data. This improve efficiency and ensure that an eBPF
program can only call functions with the right high-level arguments.
The enum bpf_map_handle_type list low-level types (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) which are identified when
updating a map entry (handle). This handle types are used to infer a
high-level arraymap type which are listed in enum bpf_map_array_type
(e.g. BPF_MAP_ARRAY_TYPE_LANDLOCK_FS).
For now, this new arraymap is only used by Landlock LSM (cf. next
commits) but it could be useful for other needs.
Changes since v2:
* add a RLIMIT_NOFILE-based limit to the maximum number of arraymap
handle entries (suggested by Andy Lutomirski)
* remove useless checks
Changes since v1:
* arraymap of handles replace custom checker groups
* simpler userland API
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com
---
include/linux/bpf.h | 14 ++++
include/uapi/linux/bpf.h | 18 +++++
kernel/bpf/arraymap.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/verifier.c | 12 ++-
4 files changed, 246 insertions(+), 1 deletion(-)
@@ -187,6 +192,9 @@ struct bpf_array {*/enumbpf_prog_typeowner_prog_type;boolowner_jited;+#ifdef CONFIG_SECURITY_LANDLOCK+u32n_entries;/* number of entries in a handle array */+#endif /* CONFIG_SECURITY_LANDLOCK */union{charvalue[0]__aligned(8);void*ptrs[0]__aligned(8);
missing something. why it has to be special to have it's own
fd array implementation?
Please take a look how BPF_MAP_TYPE_PERF_EVENT_ARRAY,
BPF_MAP_TYPE_CGROUP_ARRAY and BPF_MAP_TYPE_PROG_ARRAY are done.
The all store objects into array map that user space passes via FD.
I think the same model should apply here.
The idea is to have multiple way for userland to describe a resource
(e.g. an open file descriptor, a path or a glob pattern). The kernel
representation could then be a "struct path *" or dedicated types (e.g.
custom glob).
hmm. I think user space api should only deal with FD. Everything
else is user space job to encapsulate/hide.
Another interesting point (that could replace
check_map_func_compatibility()) is that BPF_MAP_TYPE_LANDLOCK_ARRAY
translate to dedicated (abstract) types (instead of CONST_PTR_TO_MAP)
thanks to bpf_reg_type_from_map(). This is useful to abstract userland
(map) interface with kernel object(s) dealing with that type.
I probably missing something. If user space interface is FD,
to the kernel they're different object types. Nothing else.
A third point is that BPF_MAP_TYPE_LANDLOCK_ARRAY is a kind of set. It
is optimized to quickly walk through all the elements in a sequential way.
From: Andy Lutomirski <luto@amacapital.net> Date: 2016-09-15 01:19:39
On Wed, Sep 14, 2016 at 3:14 PM, Mickaël Salaün [off-list ref] wrote:
On 14/09/2016 20:29, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
This third origin of hook call should cover all possible trigger paths
(e.g. page fault). Landlock eBPF programs can then take decisions
accordingly.
Signed-off-by: Mickaël Salaün <redacted>
Cc: Alexei Starovoitov <redacted>
Cc: Andy Lutomirski <redacted>
Cc: Daniel Borkmann <redacted>
Cc: Kees Cook <redacted>
---
quoted
+ if (unlikely(in_interrupt())) {
IMO security hooks have no business being called from interrupts.
Aren't they all synchronous things done by tasks? Interrupts are
driver things.
Are you trying to check for page faults and such?
Yes, that was the idea you did put in my mind. Not sure how to deal with
this.
It's not so easy, unfortunately. The easiest reliable way might be to
set a TS_ flag on all syscall entries when TIF_SECCOMP or similar is
set.
--Andy
From: Andy Lutomirski <luto@amacapital.net> Date: 2016-09-15 01:25:35
On Wed, Sep 14, 2016 at 3:11 PM, Mickaël Salaün [off-list ref] wrote:
On 14/09/2016 20:27, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
Add a new flag CGRP_NO_NEW_PRIVS for each cgroup. This flag is initially
set for all cgroup except the root. The flag is clear when a new process
without the no_new_privs flags is attached to the cgroup.
If a cgroup is landlocked, then any new attempt, from an unprivileged
process, to attach a process without no_new_privs to this cgroup will
be denied.
Until and unless everyone can agree on a way to properly namespace,
delegate, etc cgroups, I think that trying to add unprivileged
semantics to cgroups is nuts. Given the big thread about cgroup v2,
no-internal-tasks, etc, I just don't see how this approach can be
viable.
As far as I can tell, the no_new_privs flag of at task is not related to
namespaces. The CGRP_NO_NEW_PRIVS flag is only a cache to quickly access
the no_new_privs property of *tasks* in a cgroup. The semantic is unchanged.
Using cgroup is optional, any task could use the seccomp-based
landlocking instead. However, for those that want/need to manage a
security policy in a more dynamic way, using cgroups may make sense.
I though cgroup delegation was OK in the v2, isn't it the case? Do you
have some links?
quoted
Can we try to make landlock work completely independently of cgroups
so that it doesn't get stuck and so that programs can use it without
worrying about cgroup v1 vs v2, interactions with cgroup managers,
cgroup managers that (supposedly?) will start migrating processes
around piecemeal and almost certainly blowing up landlock in the
process, etc?
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
[1] https://lkml.kernel.org/g/<20160909225747.GA30105-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org
On Wed, Sep 14, 2016 at 06:25:07PM -0700, Andy Lutomirski wrote:
On Wed, Sep 14, 2016 at 3:11 PM, Mickaël Salaün [off-list ref] wrote:
quoted
On 14/09/2016 20:27, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
Add a new flag CGRP_NO_NEW_PRIVS for each cgroup. This flag is initially
set for all cgroup except the root. The flag is clear when a new process
without the no_new_privs flags is attached to the cgroup.
If a cgroup is landlocked, then any new attempt, from an unprivileged
process, to attach a process without no_new_privs to this cgroup will
be denied.
Until and unless everyone can agree on a way to properly namespace,
delegate, etc cgroups, I think that trying to add unprivileged
semantics to cgroups is nuts. Given the big thread about cgroup v2,
no-internal-tasks, etc, I just don't see how this approach can be
viable.
As far as I can tell, the no_new_privs flag of at task is not related to
namespaces. The CGRP_NO_NEW_PRIVS flag is only a cache to quickly access
the no_new_privs property of *tasks* in a cgroup. The semantic is unchanged.
Using cgroup is optional, any task could use the seccomp-based
landlocking instead. However, for those that want/need to manage a
security policy in a more dynamic way, using cgroups may make sense.
I though cgroup delegation was OK in the v2, isn't it the case? Do you
have some links?
quoted
Can we try to make landlock work completely independently of cgroups
so that it doesn't get stuck and so that programs can use it without
worrying about cgroup v1 vs v2, interactions with cgroup managers,
cgroup managers that (supposedly?) will start migrating processes
around piecemeal and almost certainly blowing up landlock in the
process, etc?
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
Ours and Sargun's use cases for cgroup+lsm+bpf is not for security
and not for sandboxing. So the above doesn't matter in such contexts.
lsm hooks + cgroups provide convenient scope and existing entry points.
Please see checmate examples how it's used.
From: Andy Lutomirski <luto@amacapital.net> Date: 2016-09-15 02:27:36
On Wed, Sep 14, 2016 at 7:19 PM, Alexei Starovoitov
[off-list ref] wrote:
On Wed, Sep 14, 2016 at 06:25:07PM -0700, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 3:11 PM, Mickaël Salaün [off-list ref] wrote:
quoted
On 14/09/2016 20:27, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
Add a new flag CGRP_NO_NEW_PRIVS for each cgroup. This flag is initially
set for all cgroup except the root. The flag is clear when a new process
without the no_new_privs flags is attached to the cgroup.
If a cgroup is landlocked, then any new attempt, from an unprivileged
process, to attach a process without no_new_privs to this cgroup will
be denied.
Until and unless everyone can agree on a way to properly namespace,
delegate, etc cgroups, I think that trying to add unprivileged
semantics to cgroups is nuts. Given the big thread about cgroup v2,
no-internal-tasks, etc, I just don't see how this approach can be
viable.
As far as I can tell, the no_new_privs flag of at task is not related to
namespaces. The CGRP_NO_NEW_PRIVS flag is only a cache to quickly access
the no_new_privs property of *tasks* in a cgroup. The semantic is unchanged.
Using cgroup is optional, any task could use the seccomp-based
landlocking instead. However, for those that want/need to manage a
security policy in a more dynamic way, using cgroups may make sense.
I though cgroup delegation was OK in the v2, isn't it the case? Do you
have some links?
quoted
Can we try to make landlock work completely independently of cgroups
so that it doesn't get stuck and so that programs can use it without
worrying about cgroup v1 vs v2, interactions with cgroup managers,
cgroup managers that (supposedly?) will start migrating processes
around piecemeal and almost certainly blowing up landlock in the
process, etc?
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
Ours and Sargun's use cases for cgroup+lsm+bpf is not for security
and not for sandboxing. So the above doesn't matter in such contexts.
lsm hooks + cgroups provide convenient scope and existing entry points.
Please see checmate examples how it's used.
To be clear: I'm not arguing at all that there shouldn't be
bpf+lsm+cgroup integration. I'm arguing that the unprivileged
landlock interface shouldn't expose any cgroup integration, at least
until the cgroup situation settles down a lot.
--Andy
On Wed, Sep 14, 2016 at 07:27:08PM -0700, Andy Lutomirski wrote:
quoted
quoted
quoted
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
Ours and Sargun's use cases for cgroup+lsm+bpf is not for security
and not for sandboxing. So the above doesn't matter in such contexts.
lsm hooks + cgroups provide convenient scope and existing entry points.
Please see checmate examples how it's used.
To be clear: I'm not arguing at all that there shouldn't be
bpf+lsm+cgroup integration. I'm arguing that the unprivileged
landlock interface shouldn't expose any cgroup integration, at least
until the cgroup situation settles down a lot.
ahh. yes. we're perfectly in agreement here.
I'm suggesting that the next RFC shouldn't include unpriv
and seccomp at all. Once bpf+lsm+cgroup is merged, we can
argue about unpriv with cgroups and even unpriv as a whole,
since it's not a given. Seccomp integration is also questionable.
I'd rather not have seccomp as a gate keeper for this lsm.
lsm and seccomp are orthogonal hook points. Syscalls and lsm hooks
don't have one to one relationship, so mixing them up is only
asking for trouble further down the road.
If we really need to carry some information from seccomp to lsm+bpf,
it's easier to add eBPF support to seccomp and let bpf side deal
with passing whatever information.
From: Andy Lutomirski <luto@amacapital.net> Date: 2016-09-15 04:09:22
On Wed, Sep 14, 2016 at 9:00 PM, Alexei Starovoitov
[off-list ref] wrote:
On Wed, Sep 14, 2016 at 07:27:08PM -0700, Andy Lutomirski wrote:
quoted
quoted
quoted
quoted
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
Ours and Sargun's use cases for cgroup+lsm+bpf is not for security
and not for sandboxing. So the above doesn't matter in such contexts.
lsm hooks + cgroups provide convenient scope and existing entry points.
Please see checmate examples how it's used.
To be clear: I'm not arguing at all that there shouldn't be
bpf+lsm+cgroup integration. I'm arguing that the unprivileged
landlock interface shouldn't expose any cgroup integration, at least
until the cgroup situation settles down a lot.
ahh. yes. we're perfectly in agreement here.
I'm suggesting that the next RFC shouldn't include unpriv
and seccomp at all. Once bpf+lsm+cgroup is merged, we can
argue about unpriv with cgroups and even unpriv as a whole,
since it's not a given. Seccomp integration is also questionable.
I'd rather not have seccomp as a gate keeper for this lsm.
lsm and seccomp are orthogonal hook points. Syscalls and lsm hooks
don't have one to one relationship, so mixing them up is only
asking for trouble further down the road.
If we really need to carry some information from seccomp to lsm+bpf,
it's easier to add eBPF support to seccomp and let bpf side deal
with passing whatever information.
As an argument for keeping seccomp (or an extended seccomp) as the
interface for an unprivileged bpf+lsm: seccomp already checks off most
of the boxes for safely letting unprivileged programs sandbox
themselves. Furthermore, to the extent that there are use cases for
unprivileged bpf+lsm that *aren't* expressible within the seccomp
hierarchy, I suspect that syscall filters have exactly the same
problem and that we should fix seccomp to cover it.
If I ever add a "seccomp monitor", which is something I want to do
eventually, I think it should work for lsm+bpf as well, which is
another argument for keeping it in seccomp.
--Andy
On Wed, Sep 14, 2016 at 09:08:57PM -0700, Andy Lutomirski wrote:
On Wed, Sep 14, 2016 at 9:00 PM, Alexei Starovoitov
[off-list ref] wrote:
quoted
On Wed, Sep 14, 2016 at 07:27:08PM -0700, Andy Lutomirski wrote:
quoted
quoted
quoted
quoted
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
Ours and Sargun's use cases for cgroup+lsm+bpf is not for security
and not for sandboxing. So the above doesn't matter in such contexts.
lsm hooks + cgroups provide convenient scope and existing entry points.
Please see checmate examples how it's used.
To be clear: I'm not arguing at all that there shouldn't be
bpf+lsm+cgroup integration. I'm arguing that the unprivileged
landlock interface shouldn't expose any cgroup integration, at least
until the cgroup situation settles down a lot.
ahh. yes. we're perfectly in agreement here.
I'm suggesting that the next RFC shouldn't include unpriv
and seccomp at all. Once bpf+lsm+cgroup is merged, we can
argue about unpriv with cgroups and even unpriv as a whole,
since it's not a given. Seccomp integration is also questionable.
I'd rather not have seccomp as a gate keeper for this lsm.
lsm and seccomp are orthogonal hook points. Syscalls and lsm hooks
don't have one to one relationship, so mixing them up is only
asking for trouble further down the road.
If we really need to carry some information from seccomp to lsm+bpf,
it's easier to add eBPF support to seccomp and let bpf side deal
with passing whatever information.
As an argument for keeping seccomp (or an extended seccomp) as the
interface for an unprivileged bpf+lsm: seccomp already checks off most
of the boxes for safely letting unprivileged programs sandbox
themselves.
you mean the attach part of seccomp syscall that deals with no_new_priv?
sure, that's reusable.
Furthermore, to the extent that there are use cases for
unprivileged bpf+lsm that *aren't* expressible within the seccomp
hierarchy, I suspect that syscall filters have exactly the same
problem and that we should fix seccomp to cover it.
not sure what you mean by 'seccomp hierarchy'. The normal process
hierarchy ?
imo the main deficiency of secccomp is inability to look into arguments.
One can argue that it's a blessing, since composite args
are not yet copied into the kernel memory.
But in a lot of cases the seccomp arguments are FDs pointing
to kernel objects and if programs could examine those objects
the sandboxing scope would be more precise.
lsm+bpf solves that part and I'd still argue that it's
orthogonal to seccomp's pass/reject flow.
I mean if seccomp says 'ok' the syscall should continue executing
as normal and whatever LSM hooks were triggered by it may have
their own lsm+bpf verdicts.
Furthermore in the process hierarchy different children
should be able to set their own lsm+bpf filters that are not
related to parallel seccomp+bpf hierarchy of programs.
seccomp syscall can be an interface to attach programs
to lsm hooks, but nothing more than that.
From: Andy Lutomirski <luto@amacapital.net> Date: 2016-09-15 04:38:42
On Wed, Sep 14, 2016 at 9:31 PM, Alexei Starovoitov
[off-list ref] wrote:
On Wed, Sep 14, 2016 at 09:08:57PM -0700, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 9:00 PM, Alexei Starovoitov
[off-list ref] wrote:
quoted
On Wed, Sep 14, 2016 at 07:27:08PM -0700, Andy Lutomirski wrote:
quoted
quoted
quoted
quoted
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
Ours and Sargun's use cases for cgroup+lsm+bpf is not for security
and not for sandboxing. So the above doesn't matter in such contexts.
lsm hooks + cgroups provide convenient scope and existing entry points.
Please see checmate examples how it's used.
To be clear: I'm not arguing at all that there shouldn't be
bpf+lsm+cgroup integration. I'm arguing that the unprivileged
landlock interface shouldn't expose any cgroup integration, at least
until the cgroup situation settles down a lot.
ahh. yes. we're perfectly in agreement here.
I'm suggesting that the next RFC shouldn't include unpriv
and seccomp at all. Once bpf+lsm+cgroup is merged, we can
argue about unpriv with cgroups and even unpriv as a whole,
since it's not a given. Seccomp integration is also questionable.
I'd rather not have seccomp as a gate keeper for this lsm.
lsm and seccomp are orthogonal hook points. Syscalls and lsm hooks
don't have one to one relationship, so mixing them up is only
asking for trouble further down the road.
If we really need to carry some information from seccomp to lsm+bpf,
it's easier to add eBPF support to seccomp and let bpf side deal
with passing whatever information.
As an argument for keeping seccomp (or an extended seccomp) as the
interface for an unprivileged bpf+lsm: seccomp already checks off most
of the boxes for safely letting unprivileged programs sandbox
themselves.
you mean the attach part of seccomp syscall that deals with no_new_priv?
sure, that's reusable.
quoted
Furthermore, to the extent that there are use cases for
unprivileged bpf+lsm that *aren't* expressible within the seccomp
hierarchy, I suspect that syscall filters have exactly the same
problem and that we should fix seccomp to cover it.
not sure what you mean by 'seccomp hierarchy'. The normal process
hierarchy ?
Kind of. I mean the filter layers that are inherited across fork(),
the TSYNC mechanism, etc.
imo the main deficiency of secccomp is inability to look into arguments.
One can argue that it's a blessing, since composite args
are not yet copied into the kernel memory.
But in a lot of cases the seccomp arguments are FDs pointing
to kernel objects and if programs could examine those objects
the sandboxing scope would be more precise.
lsm+bpf solves that part and I'd still argue that it's
orthogonal to seccomp's pass/reject flow.
I mean if seccomp says 'ok' the syscall should continue executing
as normal and whatever LSM hooks were triggered by it may have
their own lsm+bpf verdicts.
I agree with all of this...
Furthermore in the process hierarchy different children
should be able to set their own lsm+bpf filters that are not
related to parallel seccomp+bpf hierarchy of programs.
seccomp syscall can be an interface to attach programs
to lsm hooks, but nothing more than that.
I'm not sure what you mean. I mean that, logically, I think we should
be able to do:
seccomp(attach a syscall filter);
fork();
child does seccomp(attach some lsm filters);
I think that they *should* be related to the seccomp+bpf hierarchy of
programs in that they are entries in the same logical list of filter
layers installed. Some of those layers can be syscall filters and
some of the layers can be lsm filters. If we subsequently add a way
to attach a removable seccomp filter or a way to attach a seccomp
filter that logs failures to some fd watched by an outside monitor, I
think that should work for lsm, too, with more or less the same
interface.
If we need a way for a sandbox manager to opt different children into
different subsets of fancy filters, then I think that syscall filters
and lsm filters should use the same mechanism.
I think we might be on the same page here and just saying it different ways.
On Wed, Sep 14, 2016 at 09:38:16PM -0700, Andy Lutomirski wrote:
On Wed, Sep 14, 2016 at 9:31 PM, Alexei Starovoitov
[off-list ref] wrote:
quoted
On Wed, Sep 14, 2016 at 09:08:57PM -0700, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 9:00 PM, Alexei Starovoitov
[off-list ref] wrote:
quoted
On Wed, Sep 14, 2016 at 07:27:08PM -0700, Andy Lutomirski wrote:
quoted
quoted
quoted
quoted
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
Ours and Sargun's use cases for cgroup+lsm+bpf is not for security
and not for sandboxing. So the above doesn't matter in such contexts.
lsm hooks + cgroups provide convenient scope and existing entry points.
Please see checmate examples how it's used.
To be clear: I'm not arguing at all that there shouldn't be
bpf+lsm+cgroup integration. I'm arguing that the unprivileged
landlock interface shouldn't expose any cgroup integration, at least
until the cgroup situation settles down a lot.
ahh. yes. we're perfectly in agreement here.
I'm suggesting that the next RFC shouldn't include unpriv
and seccomp at all. Once bpf+lsm+cgroup is merged, we can
argue about unpriv with cgroups and even unpriv as a whole,
since it's not a given. Seccomp integration is also questionable.
I'd rather not have seccomp as a gate keeper for this lsm.
lsm and seccomp are orthogonal hook points. Syscalls and lsm hooks
don't have one to one relationship, so mixing them up is only
asking for trouble further down the road.
If we really need to carry some information from seccomp to lsm+bpf,
it's easier to add eBPF support to seccomp and let bpf side deal
with passing whatever information.
As an argument for keeping seccomp (or an extended seccomp) as the
interface for an unprivileged bpf+lsm: seccomp already checks off most
of the boxes for safely letting unprivileged programs sandbox
themselves.
you mean the attach part of seccomp syscall that deals with no_new_priv?
sure, that's reusable.
quoted
Furthermore, to the extent that there are use cases for
unprivileged bpf+lsm that *aren't* expressible within the seccomp
hierarchy, I suspect that syscall filters have exactly the same
problem and that we should fix seccomp to cover it.
not sure what you mean by 'seccomp hierarchy'. The normal process
hierarchy ?
Kind of. I mean the filter layers that are inherited across fork(),
the TSYNC mechanism, etc.
quoted
imo the main deficiency of secccomp is inability to look into arguments.
One can argue that it's a blessing, since composite args
are not yet copied into the kernel memory.
But in a lot of cases the seccomp arguments are FDs pointing
to kernel objects and if programs could examine those objects
the sandboxing scope would be more precise.
lsm+bpf solves that part and I'd still argue that it's
orthogonal to seccomp's pass/reject flow.
I mean if seccomp says 'ok' the syscall should continue executing
as normal and whatever LSM hooks were triggered by it may have
their own lsm+bpf verdicts.
I agree with all of this...
quoted
Furthermore in the process hierarchy different children
should be able to set their own lsm+bpf filters that are not
related to parallel seccomp+bpf hierarchy of programs.
seccomp syscall can be an interface to attach programs
to lsm hooks, but nothing more than that.
I'm not sure what you mean. I mean that, logically, I think we should
be able to do:
seccomp(attach a syscall filter);
fork();
child does seccomp(attach some lsm filters);
I think that they *should* be related to the seccomp+bpf hierarchy of
programs in that they are entries in the same logical list of filter
layers installed. Some of those layers can be syscall filters and
some of the layers can be lsm filters. If we subsequently add a way
to attach a removable seccomp filter or a way to attach a seccomp
filter that logs failures to some fd watched by an outside monitor, I
think that should work for lsm, too, with more or less the same
interface.
If we need a way for a sandbox manager to opt different children into
different subsets of fancy filters, then I think that syscall filters
and lsm filters should use the same mechanism.
I think we might be on the same page here and just saying it different ways.
Sounds like it :)
All of the above makes sense to me.
The 'orthogonal' part is that the user should be able to use
this seccomp-managed hierarchy without actually enabling
TIF_SECCOMP for the task and syscalls should still go through
fast path and all the way till lsm hooks as normal.
I don't want to pay _any_ performance penalty for this feature
for lsm hooks (and all syscalls) that don't have bpf programs attached.
On Wed, Sep 14, 2016 at 3:11 PM, Mickaël Salaün [off-list ref] wrote:
quoted
On 14/09/2016 20:27, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
Add a new flag CGRP_NO_NEW_PRIVS for each cgroup. This flag is initially
set for all cgroup except the root. The flag is clear when a new process
without the no_new_privs flags is attached to the cgroup.
If a cgroup is landlocked, then any new attempt, from an unprivileged
process, to attach a process without no_new_privs to this cgroup will
be denied.
Until and unless everyone can agree on a way to properly namespace,
delegate, etc cgroups, I think that trying to add unprivileged
semantics to cgroups is nuts. Given the big thread about cgroup v2,
no-internal-tasks, etc, I just don't see how this approach can be
viable.
As far as I can tell, the no_new_privs flag of at task is not related to
namespaces. The CGRP_NO_NEW_PRIVS flag is only a cache to quickly access
the no_new_privs property of *tasks* in a cgroup. The semantic is unchanged.
Using cgroup is optional, any task could use the seccomp-based
landlocking instead. However, for those that want/need to manage a
security policy in a more dynamic way, using cgroups may make sense.
I though cgroup delegation was OK in the v2, isn't it the case? Do you
have some links?
quoted
Can we try to make landlock work completely independently of cgroups
so that it doesn't get stuck and so that programs can use it without
worrying about cgroup v1 vs v2, interactions with cgroup managers,
cgroup managers that (supposedly?) will start migrating processes
around piecemeal and almost certainly blowing up landlock in the
process, etc?
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
[1] https://lkml.kernel.org/r/20160909225747.GA30105@mtj.duckdns.org
Please refer to "2-5. Delegation" of Documentation/cgroup-v2.txt.
Delegation on v1 is broken on both core and specific controller
behaviors and thus discouraged. On v2, delegation should work just
fine.
Tejun, could you please clarify if there is still a problem with cgroup
v2 delegation?
This patch only implement a cache mechanism with the CGRP_NO_NEW_PRIVS
flag. If cgroups can group processes correctly, I don't see any
(security) issue here. It's the administrator choice to delegate a part
of the cgroup management. It's then the delegatee responsibility to
correctly put processes in cgroups. This is comparable to a process
which is responsible to correctly call seccomp(2).
Mickaël
On Wed, Sep 14, 2016 at 09:38:16PM -0700, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 9:31 PM, Alexei Starovoitov
[off-list ref] wrote:
quoted
On Wed, Sep 14, 2016 at 09:08:57PM -0700, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 9:00 PM, Alexei Starovoitov
[off-list ref] wrote:
quoted
On Wed, Sep 14, 2016 at 07:27:08PM -0700, Andy Lutomirski wrote:
quoted
quoted
quoted
quoted
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
Ours and Sargun's use cases for cgroup+lsm+bpf is not for security
and not for sandboxing. So the above doesn't matter in such contexts.
lsm hooks + cgroups provide convenient scope and existing entry points.
Please see checmate examples how it's used.
To be clear: I'm not arguing at all that there shouldn't be
bpf+lsm+cgroup integration. I'm arguing that the unprivileged
landlock interface shouldn't expose any cgroup integration, at least
until the cgroup situation settles down a lot.
ahh. yes. we're perfectly in agreement here.
I'm suggesting that the next RFC shouldn't include unpriv
and seccomp at all. Once bpf+lsm+cgroup is merged, we can
argue about unpriv with cgroups and even unpriv as a whole,
since it's not a given. Seccomp integration is also questionable.
I'd rather not have seccomp as a gate keeper for this lsm.
lsm and seccomp are orthogonal hook points. Syscalls and lsm hooks
don't have one to one relationship, so mixing them up is only
asking for trouble further down the road.
If we really need to carry some information from seccomp to lsm+bpf,
it's easier to add eBPF support to seccomp and let bpf side deal
with passing whatever information.
As an argument for keeping seccomp (or an extended seccomp) as the
interface for an unprivileged bpf+lsm: seccomp already checks off most
of the boxes for safely letting unprivileged programs sandbox
themselves.
you mean the attach part of seccomp syscall that deals with no_new_priv?
sure, that's reusable.
quoted
Furthermore, to the extent that there are use cases for
unprivileged bpf+lsm that *aren't* expressible within the seccomp
hierarchy, I suspect that syscall filters have exactly the same
problem and that we should fix seccomp to cover it.
not sure what you mean by 'seccomp hierarchy'. The normal process
hierarchy ?
Kind of. I mean the filter layers that are inherited across fork(),
the TSYNC mechanism, etc.
quoted
imo the main deficiency of secccomp is inability to look into arguments.
One can argue that it's a blessing, since composite args
are not yet copied into the kernel memory.
But in a lot of cases the seccomp arguments are FDs pointing
to kernel objects and if programs could examine those objects
the sandboxing scope would be more precise.
lsm+bpf solves that part and I'd still argue that it's
orthogonal to seccomp's pass/reject flow.
I mean if seccomp says 'ok' the syscall should continue executing
as normal and whatever LSM hooks were triggered by it may have
their own lsm+bpf verdicts.
I agree with all of this...
quoted
Furthermore in the process hierarchy different children
should be able to set their own lsm+bpf filters that are not
related to parallel seccomp+bpf hierarchy of programs.
seccomp syscall can be an interface to attach programs
to lsm hooks, but nothing more than that.
I'm not sure what you mean. I mean that, logically, I think we should
be able to do:
seccomp(attach a syscall filter);
fork();
child does seccomp(attach some lsm filters);
I think that they *should* be related to the seccomp+bpf hierarchy of
programs in that they are entries in the same logical list of filter
layers installed. Some of those layers can be syscall filters and
some of the layers can be lsm filters. If we subsequently add a way
to attach a removable seccomp filter or a way to attach a seccomp
filter that logs failures to some fd watched by an outside monitor, I
think that should work for lsm, too, with more or less the same
interface.
If we need a way for a sandbox manager to opt different children into
different subsets of fancy filters, then I think that syscall filters
and lsm filters should use the same mechanism.
I think we might be on the same page here and just saying it different ways.
Sounds like it :)
All of the above makes sense to me.
The 'orthogonal' part is that the user should be able to use
this seccomp-managed hierarchy without actually enabling
TIF_SECCOMP for the task and syscalls should still go through
fast path and all the way till lsm hooks as normal.
I don't want to pay _any_ performance penalty for this feature
for lsm hooks (and all syscalls) that don't have bpf programs attached.
Yes, it seems that we are all on the same page here, and that match this
RFC implementation. So, using the seccomp(2) *interface* to attach
Landlock programs to a process hierarchy is still on track. :)
On Thu, Sep 15, 2016 at 01:02:22AM +0200, Mickaël Salaün wrote:
quoted
quoted
I would suggest for the next RFC to do minimal 7 patches up to this point
with simple example that demonstrates the use case.
I would avoid all unpriv stuff and all of seccomp for the next RFC as well,
otherwise I don't think we can realistically make forward progress, since
there are too many issues raised in the subsequent patches.
I hope we will find a common agreement about seccomp vs cgroup… I think
both approaches have their advantages, can be complementary and nicely
combined.
I don't mind having both task based lsm and cgroup based as long as
infrastracture is not duplicated and scaling issues from earlier version
are resolved.
It should be much better with this RFC.
I'm proposing to do cgroup only for the next RFC, since mine and Sargun's
use case for this bpf+lsm+cgroup is _not_ security or sandboxing.
Well, LSM purpose is to do security stuff. The main goal of Landlock is
to bring security features to userland, including unprivileged
processes, at least via the seccomp interface [1].
No need for unpriv, no_new_priv to cgroups are other things that Andy
is concerned about.
I'm concern about security too! :)
quoted
Unprivileged sandboxing is the main goal of Landlock. This should not be
a problem, even for privileged features, thanks to the new subtype/access.
yes. the point that unpriv stuff can come later after agreement is reached.
If we keep arguing about seccomp details this set won't go anywhere.
Even in basic part (which is cgroup+bpf+lsm) are plenty of questions
to be still agreed.
Agreed. With this RFC, the Checmate features (i.e. network helpers)
should be able to sit on top of Landlock.
I think neither of them should be called fancy names for no technical reason.
We will have only one bpf based lsm. That's it and it doesn't
need an obscure name. Directory name can be security/bpf/..stuff.c
I disagree on an LSM named "BPF". I first started with the "seccomp LSM"
name (first RFC) but I later realized that it is confusing because
seccomp is associated to its syscall and the underlying features. Same
thing goes for BPF. It is also artificially hard to grep on a name too
used in the kernel source tree.
Making an association between the generic eBPF mechanism and a security
centric approach (i.e. LSM) seems a bit reductive (for BPF). Moreover,
the seccomp interface [1] can still be used.
Landlock is a nice name to depict a sandbox as an enclave (i.e. a
landlocked country/state). I want to keep this name, which is simple,
express the goal of Landlock nicely and is comparable to other sandbox
mechanisms as Seatbelt or Pledge.
Landlock should not be confused with the underlying eBPF implementation.
Landlock could use more than only eBPF in the future and eBPF could be
used in other LSM as well.
Mickaël
On Thu, Sep 15, 2016 at 01:22:49AM +0200, Mickaël Salaün wrote:
quoted
On 14/09/2016 20:51, Alexei Starovoitov wrote:
quoted
On Wed, Sep 14, 2016 at 09:23:56AM +0200, Mickaël Salaün wrote:
quoted
This new arraymap looks like a set and brings new properties:
* strong typing of entries: the eBPF functions get the array type of
elements instead of CONST_PTR_TO_MAP (e.g.
CONST_PTR_TO_LANDLOCK_HANDLE_FS);
* force sequential filling (i.e. replace or append-only update), which
allow quick browsing of all entries.
This strong typing is useful to statically check if the content of a map
can be passed to an eBPF function. For example, Landlock use it to store
and manage kernel objects (e.g. struct file) instead of dealing with
userland raw data. This improve efficiency and ensure that an eBPF
program can only call functions with the right high-level arguments.
The enum bpf_map_handle_type list low-level types (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) which are identified when
updating a map entry (handle). This handle types are used to infer a
high-level arraymap type which are listed in enum bpf_map_array_type
(e.g. BPF_MAP_ARRAY_TYPE_LANDLOCK_FS).
For now, this new arraymap is only used by Landlock LSM (cf. next
commits) but it could be useful for other needs.
Changes since v2:
* add a RLIMIT_NOFILE-based limit to the maximum number of arraymap
handle entries (suggested by Andy Lutomirski)
* remove useless checks
Changes since v1:
* arraymap of handles replace custom checker groups
* simpler userland API
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com
---
include/linux/bpf.h | 14 ++++
include/uapi/linux/bpf.h | 18 +++++
kernel/bpf/arraymap.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/verifier.c | 12 ++-
4 files changed, 246 insertions(+), 1 deletion(-)
@@ -187,6 +192,9 @@ struct bpf_array {*/enumbpf_prog_typeowner_prog_type;boolowner_jited;+#ifdef CONFIG_SECURITY_LANDLOCK+u32n_entries;/* number of entries in a handle array */+#endif /* CONFIG_SECURITY_LANDLOCK */union{charvalue[0]__aligned(8);void*ptrs[0]__aligned(8);
missing something. why it has to be special to have it's own
fd array implementation?
Please take a look how BPF_MAP_TYPE_PERF_EVENT_ARRAY,
BPF_MAP_TYPE_CGROUP_ARRAY and BPF_MAP_TYPE_PROG_ARRAY are done.
The all store objects into array map that user space passes via FD.
I think the same model should apply here.
The idea is to have multiple way for userland to describe a resource
(e.g. an open file descriptor, a path or a glob pattern). The kernel
representation could then be a "struct path *" or dedicated types (e.g.
custom glob).
hmm. I think user space api should only deal with FD. Everything
else is user space job to encapsulate/hide.
How would you create a FD referring to a glob, a user or port ranges for
example ?
quoted
Another interesting point (that could replace
check_map_func_compatibility()) is that BPF_MAP_TYPE_LANDLOCK_ARRAY
translate to dedicated (abstract) types (instead of CONST_PTR_TO_MAP)
thanks to bpf_reg_type_from_map(). This is useful to abstract userland
(map) interface with kernel object(s) dealing with that type.
I probably missing something. If user space interface is FD,
to the kernel they're different object types. Nothing else.
On Thu, Sep 15, 2016 at 11:25:10PM +0200, Mickaël Salaün wrote:
quoted
quoted
Agreed. With this RFC, the Checmate features (i.e. network helpers)
should be able to sit on top of Landlock.
I think neither of them should be called fancy names for no technical reason.
We will have only one bpf based lsm. That's it and it doesn't
need an obscure name. Directory name can be security/bpf/..stuff.c
I disagree on an LSM named "BPF". I first started with the "seccomp LSM"
name (first RFC) but I later realized that it is confusing because
seccomp is associated to its syscall and the underlying features. Same
thing goes for BPF. It is also artificially hard to grep on a name too
used in the kernel source tree.
Making an association between the generic eBPF mechanism and a security
centric approach (i.e. LSM) seems a bit reductive (for BPF). Moreover,
the seccomp interface [1] can still be used.
agree with above.
Landlock is a nice name to depict a sandbox as an enclave (i.e. a
landlocked country/state). I want to keep this name, which is simple,
express the goal of Landlock nicely and is comparable to other sandbox
mechanisms as Seatbelt or Pledge.
Landlock should not be confused with the underlying eBPF implementation.
Landlock could use more than only eBPF in the future and eBPF could be
used in other LSM as well.
there will not be two bpf based LSMs.
Therefore unless you can convince Sargun to give up his 'checmate' name,
nothing goes in.
The features you both need are 90% the same, so they must be done
as part of single LSM whatever you both agree to call it.
I'm fine giving up the Checmate name. Landlock seems easy enough to
Google. I haven't gotten a chance to look through the entire patchset
yet, but it does seem like they are somewhat similar.
On Mon, Sep 19, 2016 at 5:12 PM, Alexei Starovoitov
[off-list ref] wrote:
On Thu, Sep 15, 2016 at 11:25:10PM +0200, Mickaël Salaün wrote:
quoted
quoted
quoted
Agreed. With this RFC, the Checmate features (i.e. network helpers)
should be able to sit on top of Landlock.
I think neither of them should be called fancy names for no technical reason.
We will have only one bpf based lsm. That's it and it doesn't
need an obscure name. Directory name can be security/bpf/..stuff.c
I disagree on an LSM named "BPF". I first started with the "seccomp LSM"
name (first RFC) but I later realized that it is confusing because
seccomp is associated to its syscall and the underlying features. Same
thing goes for BPF. It is also artificially hard to grep on a name too
used in the kernel source tree.
Making an association between the generic eBPF mechanism and a security
centric approach (i.e. LSM) seems a bit reductive (for BPF). Moreover,
the seccomp interface [1] can still be used.
agree with above.
quoted
Landlock is a nice name to depict a sandbox as an enclave (i.e. a
landlocked country/state). I want to keep this name, which is simple,
express the goal of Landlock nicely and is comparable to other sandbox
mechanisms as Seatbelt or Pledge.
Landlock should not be confused with the underlying eBPF implementation.
Landlock could use more than only eBPF in the future and eBPF could be
used in other LSM as well.
there will not be two bpf based LSMs.
Therefore unless you can convince Sargun to give up his 'checmate' name,
nothing goes in.
The features you both need are 90% the same, so they must be done
as part of single LSM whatever you both agree to call it.
On Thu, Sep 15, 2016 at 09:41:33PM +0200, Mickaël Salaün wrote:
On 15/09/2016 06:48, Alexei Starovoitov wrote:
quoted
On Wed, Sep 14, 2016 at 09:38:16PM -0700, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 9:31 PM, Alexei Starovoitov
[off-list ref] wrote:
quoted
On Wed, Sep 14, 2016 at 09:08:57PM -0700, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 9:00 PM, Alexei Starovoitov
[off-list ref] wrote:
quoted
On Wed, Sep 14, 2016 at 07:27:08PM -0700, Andy Lutomirski wrote:
quoted
quoted
quoted
quoted
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
Ours and Sargun's use cases for cgroup+lsm+bpf is not for security
and not for sandboxing. So the above doesn't matter in such contexts.
lsm hooks + cgroups provide convenient scope and existing entry points.
Please see checmate examples how it's used.
To be clear: I'm not arguing at all that there shouldn't be
bpf+lsm+cgroup integration. I'm arguing that the unprivileged
landlock interface shouldn't expose any cgroup integration, at least
until the cgroup situation settles down a lot.
ahh. yes. we're perfectly in agreement here.
I'm suggesting that the next RFC shouldn't include unpriv
and seccomp at all. Once bpf+lsm+cgroup is merged, we can
argue about unpriv with cgroups and even unpriv as a whole,
since it's not a given. Seccomp integration is also questionable.
I'd rather not have seccomp as a gate keeper for this lsm.
lsm and seccomp are orthogonal hook points. Syscalls and lsm hooks
don't have one to one relationship, so mixing them up is only
asking for trouble further down the road.
If we really need to carry some information from seccomp to lsm+bpf,
it's easier to add eBPF support to seccomp and let bpf side deal
with passing whatever information.
As an argument for keeping seccomp (or an extended seccomp) as the
interface for an unprivileged bpf+lsm: seccomp already checks off most
of the boxes for safely letting unprivileged programs sandbox
themselves.
you mean the attach part of seccomp syscall that deals with no_new_priv?
sure, that's reusable.
quoted
Furthermore, to the extent that there are use cases for
unprivileged bpf+lsm that *aren't* expressible within the seccomp
hierarchy, I suspect that syscall filters have exactly the same
problem and that we should fix seccomp to cover it.
not sure what you mean by 'seccomp hierarchy'. The normal process
hierarchy ?
Kind of. I mean the filter layers that are inherited across fork(),
the TSYNC mechanism, etc.
quoted
imo the main deficiency of secccomp is inability to look into arguments.
One can argue that it's a blessing, since composite args
are not yet copied into the kernel memory.
But in a lot of cases the seccomp arguments are FDs pointing
to kernel objects and if programs could examine those objects
the sandboxing scope would be more precise.
lsm+bpf solves that part and I'd still argue that it's
orthogonal to seccomp's pass/reject flow.
I mean if seccomp says 'ok' the syscall should continue executing
as normal and whatever LSM hooks were triggered by it may have
their own lsm+bpf verdicts.
I agree with all of this...
quoted
Furthermore in the process hierarchy different children
should be able to set their own lsm+bpf filters that are not
related to parallel seccomp+bpf hierarchy of programs.
seccomp syscall can be an interface to attach programs
to lsm hooks, but nothing more than that.
I'm not sure what you mean. I mean that, logically, I think we should
be able to do:
seccomp(attach a syscall filter);
fork();
child does seccomp(attach some lsm filters);
I think that they *should* be related to the seccomp+bpf hierarchy of
programs in that they are entries in the same logical list of filter
layers installed. Some of those layers can be syscall filters and
some of the layers can be lsm filters. If we subsequently add a way
to attach a removable seccomp filter or a way to attach a seccomp
filter that logs failures to some fd watched by an outside monitor, I
think that should work for lsm, too, with more or less the same
interface.
If we need a way for a sandbox manager to opt different children into
different subsets of fancy filters, then I think that syscall filters
and lsm filters should use the same mechanism.
I think we might be on the same page here and just saying it different ways.
Sounds like it :)
All of the above makes sense to me.
The 'orthogonal' part is that the user should be able to use
this seccomp-managed hierarchy without actually enabling
TIF_SECCOMP for the task and syscalls should still go through
fast path and all the way till lsm hooks as normal.
I don't want to pay _any_ performance penalty for this feature
for lsm hooks (and all syscalls) that don't have bpf programs attached.
Yes, it seems that we are all on the same page here, and that match this
RFC implementation. So, using the seccomp(2) *interface* to attach
Landlock programs to a process hierarchy is still on track. :)
So, I'm catching up on this after a little while away. I really like the
simplicity of the approach Daniel took with his patches. I began to have
difficulty reading your patchset once you got into using seccomp + unprivileged
mode. I would love to see a separate patchset that only have the verifier, and
lsm hook changes. Do you think you could decompose your patchset into an MVP?
I'm fine giving up the Checmate name. Landlock seems easy enough to
Google. I haven't gotten a chance to look through the entire patchset
yet, but it does seem like they are somewhat similar.
Excellent! I'm looking forward for your review.
On Mon, Sep 19, 2016 at 5:12 PM, Alexei Starovoitov
[off-list ref] wrote:
quoted
On Thu, Sep 15, 2016 at 11:25:10PM +0200, Mickaël Salaün wrote:
quoted
quoted
quoted
Agreed. With this RFC, the Checmate features (i.e. network helpers)
should be able to sit on top of Landlock.
I think neither of them should be called fancy names for no technical reason.
We will have only one bpf based lsm. That's it and it doesn't
need an obscure name. Directory name can be security/bpf/..stuff.c
I disagree on an LSM named "BPF". I first started with the "seccomp LSM"
name (first RFC) but I later realized that it is confusing because
seccomp is associated to its syscall and the underlying features. Same
thing goes for BPF. It is also artificially hard to grep on a name too
used in the kernel source tree.
Making an association between the generic eBPF mechanism and a security
centric approach (i.e. LSM) seems a bit reductive (for BPF). Moreover,
the seccomp interface [1] can still be used.
agree with above.
quoted
Landlock is a nice name to depict a sandbox as an enclave (i.e. a
landlocked country/state). I want to keep this name, which is simple,
express the goal of Landlock nicely and is comparable to other sandbox
mechanisms as Seatbelt or Pledge.
Landlock should not be confused with the underlying eBPF implementation.
Landlock could use more than only eBPF in the future and eBPF could be
used in other LSM as well.
there will not be two bpf based LSMs.
Therefore unless you can convince Sargun to give up his 'checmate' name,
nothing goes in.
The features you both need are 90% the same, so they must be done
as part of single LSM whatever you both agree to call it.
On Thu, Sep 15, 2016 at 09:41:33PM +0200, Mickaël Salaün wrote:
quoted
On 15/09/2016 06:48, Alexei Starovoitov wrote:
quoted
On Wed, Sep 14, 2016 at 09:38:16PM -0700, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 9:31 PM, Alexei Starovoitov
[off-list ref] wrote:
quoted
On Wed, Sep 14, 2016 at 09:08:57PM -0700, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 9:00 PM, Alexei Starovoitov
[off-list ref] wrote:
quoted
On Wed, Sep 14, 2016 at 07:27:08PM -0700, Andy Lutomirski wrote:
quoted
quoted
quoted
quoted
This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?
What I mean is: cgroup v2 delegation has a functionality problem.
Tejun says [1]:
We haven't had to face this decision because cgroup has never properly
supported delegating to applications and the in-use setups where this
happens are custom configurations where there is no boundary between
system and applications and adhoc trial-and-error is good enough a way
to find a working solution. That wiggle room goes away once we
officially open this up to individual applications.
Unless and until that changes, I think that landlock should stay away
from cgroups. Others could reasonably disagree with me.
Ours and Sargun's use cases for cgroup+lsm+bpf is not for security
and not for sandboxing. So the above doesn't matter in such contexts.
lsm hooks + cgroups provide convenient scope and existing entry points.
Please see checmate examples how it's used.
To be clear: I'm not arguing at all that there shouldn't be
bpf+lsm+cgroup integration. I'm arguing that the unprivileged
landlock interface shouldn't expose any cgroup integration, at least
until the cgroup situation settles down a lot.
ahh. yes. we're perfectly in agreement here.
I'm suggesting that the next RFC shouldn't include unpriv
and seccomp at all. Once bpf+lsm+cgroup is merged, we can
argue about unpriv with cgroups and even unpriv as a whole,
since it's not a given. Seccomp integration is also questionable.
I'd rather not have seccomp as a gate keeper for this lsm.
lsm and seccomp are orthogonal hook points. Syscalls and lsm hooks
don't have one to one relationship, so mixing them up is only
asking for trouble further down the road.
If we really need to carry some information from seccomp to lsm+bpf,
it's easier to add eBPF support to seccomp and let bpf side deal
with passing whatever information.
As an argument for keeping seccomp (or an extended seccomp) as the
interface for an unprivileged bpf+lsm: seccomp already checks off most
of the boxes for safely letting unprivileged programs sandbox
themselves.
you mean the attach part of seccomp syscall that deals with no_new_priv?
sure, that's reusable.
quoted
Furthermore, to the extent that there are use cases for
unprivileged bpf+lsm that *aren't* expressible within the seccomp
hierarchy, I suspect that syscall filters have exactly the same
problem and that we should fix seccomp to cover it.
not sure what you mean by 'seccomp hierarchy'. The normal process
hierarchy ?
Kind of. I mean the filter layers that are inherited across fork(),
the TSYNC mechanism, etc.
quoted
imo the main deficiency of secccomp is inability to look into arguments.
One can argue that it's a blessing, since composite args
are not yet copied into the kernel memory.
But in a lot of cases the seccomp arguments are FDs pointing
to kernel objects and if programs could examine those objects
the sandboxing scope would be more precise.
lsm+bpf solves that part and I'd still argue that it's
orthogonal to seccomp's pass/reject flow.
I mean if seccomp says 'ok' the syscall should continue executing
as normal and whatever LSM hooks were triggered by it may have
their own lsm+bpf verdicts.
I agree with all of this...
quoted
Furthermore in the process hierarchy different children
should be able to set their own lsm+bpf filters that are not
related to parallel seccomp+bpf hierarchy of programs.
seccomp syscall can be an interface to attach programs
to lsm hooks, but nothing more than that.
I'm not sure what you mean. I mean that, logically, I think we should
be able to do:
seccomp(attach a syscall filter);
fork();
child does seccomp(attach some lsm filters);
I think that they *should* be related to the seccomp+bpf hierarchy of
programs in that they are entries in the same logical list of filter
layers installed. Some of those layers can be syscall filters and
some of the layers can be lsm filters. If we subsequently add a way
to attach a removable seccomp filter or a way to attach a seccomp
filter that logs failures to some fd watched by an outside monitor, I
think that should work for lsm, too, with more or less the same
interface.
If we need a way for a sandbox manager to opt different children into
different subsets of fancy filters, then I think that syscall filters
and lsm filters should use the same mechanism.
I think we might be on the same page here and just saying it different ways.
Sounds like it :)
All of the above makes sense to me.
The 'orthogonal' part is that the user should be able to use
this seccomp-managed hierarchy without actually enabling
TIF_SECCOMP for the task and syscalls should still go through
fast path and all the way till lsm hooks as normal.
I don't want to pay _any_ performance penalty for this feature
for lsm hooks (and all syscalls) that don't have bpf programs attached.
Yes, it seems that we are all on the same page here, and that match this
RFC implementation. So, using the seccomp(2) *interface* to attach
Landlock programs to a process hierarchy is still on track. :)
So, I'm catching up on this after a little while away. I really like the
simplicity of the approach Daniel took with his patches. I began to have
difficulty reading your patchset once you got into using seccomp + unprivileged
mode. I would love to see a separate patchset that only have the verifier, and
lsm hook changes. Do you think you could decompose your patchset into an MVP?
OK, I'll try to split the common parts from the seccomp part, but there
is already a dedicated patch for the LSM hooks [06/22].
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted hunk
Add eBPF functions to compare file system access with a Landlock file
system handle:
* bpf_landlock_cmp_fs_prop_with_struct_file(prop, map, map_op, file)
This function allows to compare the dentry, inode, device or mount
point of the currently accessed file, with a reference handle.
* bpf_landlock_cmp_fs_beneath_with_struct_file(opt, map, map_op, file)
This function allows an eBPF program to check if the current accessed
file is the same or in the hierarchy of a reference handle.
The goal of file system handle is to abstract kernel objects such as a
struct file or a struct inode. Userland can create this kind of handle
thanks to the BPF_MAP_UPDATE_ELEM command. The element is a struct
landlock_handle containing the handle type (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) and a file descriptor. This could
also be any descriptions able to match a struct file or a struct inode
(e.g. path or glob string).
Changes since v2:
* add MNT_INTERNAL check to only add file handle from user-visible FS
(e.g. no anonymous inode)
* replace struct file* with struct path* in map_landlock_handle
* add BPF protos
* fix bpf_landlock_cmp_fs_prop_with_struct_file()
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: James Morris <redacted>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com
---
include/linux/bpf.h | 10 +++
include/uapi/linux/bpf.h | 49 +++++++++++
kernel/bpf/arraymap.c | 21 +++++
kernel/bpf/verifier.c | 8 ++
security/landlock/Makefile | 2 +-
security/landlock/checker_fs.c | 179 +++++++++++++++++++++++++++++++++++++++++
security/landlock/checker_fs.h | 20 +++++
security/landlock/lsm.c | 6 ++
8 files changed, 294 insertions(+), 1 deletion(-)
create mode 100644 security/landlock/checker_fs.c
create mode 100644 security/landlock/checker_fs.h
[...]
@@ -0,0 +1,179 @@+/*+*LandlockLSM-FileSystemCheckers+*+*Copyright(C)2016MickaëlSalaün<mic@digikod.net>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseversion2,as+*publishedbytheFreeSoftwareFoundation.+*/++#include<linux/bpf.h>/* enum bpf_map_array_op */+#include<linux/errno.h>+#include<linux/fs.h>/* path_is_under() */+#include<linux/path.h>/* struct path */++#include"checker_fs.h"++#define EQUAL_NOT_NULL(a, b) (a && a == b)++/*+*bpf_landlock_cmp_fs_prop_with_struct_file+*+*Cf.include/uapi/linux/bpf.h+*/+staticinlineu64bpf_landlock_cmp_fs_prop_with_struct_file(u64r1_property,+u64r2_map,u64r3_map_op,u64r4_file,u64r5)+{+u8property=(u8)r1_property;+structbpf_map*map=(structbpf_map*)(unsignedlong)r2_map;+enumbpf_map_array_opmap_op=r3_map_op;+structfile*file=(structfile*)(unsignedlong)r4_file;+structbpf_array*array=container_of(map,structbpf_array,map);+structpath*p1,*p2;+structmap_landlock_handle*handle;+inti;++/* ARG_CONST_PTR_TO_LANDLOCK_HANDLE_FS is an arraymap */+if(unlikely(!map)){+WARN_ON(1);+return-EFAULT;+}
Just some minor style/readability nits...
This is more readable as:
if (WARN_ON(!map))
return -EFAULT;
(WARN_ON already includes the unlikely() and passes through the test result.)
quoted hunk
+ if (unlikely(!file))+ return -ENOENT;+ if (unlikely((property | _LANDLOCK_FLAG_FS_MASK) != _LANDLOCK_FLAG_FS_MASK))+ return -EINVAL;++ /* for now, only handle OP_OR */+ switch (map_op) {+ case BPF_MAP_ARRAY_OP_OR:+ break;+ case BPF_MAP_ARRAY_OP_UNSPEC:+ case BPF_MAP_ARRAY_OP_AND:+ case BPF_MAP_ARRAY_OP_XOR:+ default:+ return -EINVAL;+ }+ p2 = &file->f_path;++ synchronize_rcu();++ for (i = 0; i < array->n_entries; i++) {+ bool result_dentry = !(property & LANDLOCK_FLAG_FS_DENTRY);+ bool result_inode = !(property & LANDLOCK_FLAG_FS_INODE);+ bool result_device = !(property & LANDLOCK_FLAG_FS_DEVICE);+ bool result_mount = !(property & LANDLOCK_FLAG_FS_MOUNT);++ handle = (struct map_landlock_handle *)+ (array->value + array->elem_size * i);++ if (handle->type != BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) {+ WARN_ON(1);+ return -EFAULT;+ }
Same here... and in the other function (much of which seems to repeat
-- can some of these checks be put into common functions?)
-Kees
--
Kees Cook
Nexus Security
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted hunk
This allows to add new eBPF programs to Landlock hooks dedicated to a
cgroup thanks to the BPF_PROG_ATTACH command. Like for socket eBPF
programs, the Landlock hooks attached to a cgroup are propagated to the
nested cgroups. However, when a new Landlock program is attached to one
of this nested cgroup, this cgroup hierarchy fork the Landlock hooks.
This design is simple and match the current CONFIG_BPF_CGROUP
inheritance. The difference lie in the fact that Landlock programs can
only be stacked but not removed. This match the append-only seccomp
behavior. Userland is free to handle Landlock hooks attached to a cgroup
in more complicated ways (e.g. continuous inheritance), but care should
be taken to properly handle error cases (e.g. memory allocation errors).
Changes since v2:
* new design based on BPF_PROG_ATTACH (suggested by Alexei Starovoitov)
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Daniel Mack <daniel@zonque.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Cc: Tejun Heo <tj@kernel.org>
Link: https://lkml.kernel.org/r/20160826021432.GA8291@ast-mbp.thefacebook.com
Link: https://lkml.kernel.org/r/20160827204307.GA43714@ast-mbp.thefacebook.com
---
include/linux/bpf-cgroup.h | 7 +++++++
include/linux/cgroup-defs.h | 2 ++
include/linux/landlock.h | 9 +++++++++
include/uapi/linux/bpf.h | 1 +
kernel/bpf/cgroup.c | 33 ++++++++++++++++++++++++++++++---
kernel/bpf/syscall.c | 11 +++++++++++
security/landlock/lsm.c | 40 +++++++++++++++++++++++++++++++++++++++-
security/landlock/manager.c | 32 ++++++++++++++++++++++++++++++++
8 files changed, 131 insertions(+), 4 deletions(-)
[...]
I get creeped out by type-controlled unions of pointers. :P I don't
have a suggestion to improve this, but I don't like seeing a pointer
type managed separately from the pointer itself as it tends to bypass
a lot of both static and dynamic checking. A union is better than a
cast of void *, but it still worries me. :)
-Kees
--
Kees Cook
Nexus Security
On Wed, Sep 14, 2016 at 6:19 PM, Andy Lutomirski [off-list ref] wrote:
On Wed, Sep 14, 2016 at 3:14 PM, Mickaël Salaün [off-list ref] wrote:
quoted
On 14/09/2016 20:29, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
This third origin of hook call should cover all possible trigger paths
(e.g. page fault). Landlock eBPF programs can then take decisions
accordingly.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Kees Cook <redacted>
---
quoted
+ if (unlikely(in_interrupt())) {
IMO security hooks have no business being called from interrupts.
Aren't they all synchronous things done by tasks? Interrupts are
driver things.
Are you trying to check for page faults and such?
Yes, that was the idea you did put in my mind. Not sure how to deal with
this.
It's not so easy, unfortunately. The easiest reliable way might be to
set a TS_ flag on all syscall entries when TIF_SECCOMP or similar is
set.
For making this series smaller, let's leave the idea idea of interrupt
hooks out -- the intention is for stricter syscall filtering, yes?
Once things are more well established and there's a use-case for this,
it can be added back in.
-Kees
--
Kees Cook
Nexus Security
On Wed, Sep 14, 2016 at 3:34 PM, Mickaël Salaün [off-list ref] wrote:
On 14/09/2016 20:43, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
A Landlock program will be triggered according to its subtype/origin
bitfield. The LANDLOCK_FLAG_ORIGIN_SECCOMP value will trigger the
Landlock program when a seccomp filter will return RET_LANDLOCK.
Moreover, it is possible to return a 16-bit cookie which will be
readable by the Landlock programs in its context.
Are you envisioning that the filters will return RET_LANDLOCK most of
the time or rarely? If it's most of the time, then maybe this could
be simplified a bit by unconditionally calling the landlock filter and
letting the landlock filter access a struct seccomp_data if needed.
Exposing seccomp_data in a Landlock context may be a good idea. The main
implication is that Landlock programs may then be architecture specific
(if dealing with data) as seccomp filters are. Another point is that it
remove any direct binding between seccomp filters and Landlock programs.
I will try this (more simple) approach.
Yeah, I would prefer that the seccomp code isn't doing list management
to identify the landlock hooks to trigger, etc. I think that's better
done on the LSM side. And since multiple seccomp filters could trigger
landlock, it may be best to just leave the low 16 bits unused
entirely. Then all state management is handled by the landlock eBPF
maps, not a value coming from seccomp that can get stomped on by new
filters, etc.
-Kees
--
Kees Cook
Nexus Security
On Wed, Sep 14, 2016 at 12:23 AM, Mickaël Salaün [off-list ref] wrote:
quoted hunk
This new arraymap looks like a set and brings new properties:
* strong typing of entries: the eBPF functions get the array type of
elements instead of CONST_PTR_TO_MAP (e.g.
CONST_PTR_TO_LANDLOCK_HANDLE_FS);
* force sequential filling (i.e. replace or append-only update), which
allow quick browsing of all entries.
This strong typing is useful to statically check if the content of a map
can be passed to an eBPF function. For example, Landlock use it to store
and manage kernel objects (e.g. struct file) instead of dealing with
userland raw data. This improve efficiency and ensure that an eBPF
program can only call functions with the right high-level arguments.
The enum bpf_map_handle_type list low-level types (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) which are identified when
updating a map entry (handle). This handle types are used to infer a
high-level arraymap type which are listed in enum bpf_map_array_type
(e.g. BPF_MAP_ARRAY_TYPE_LANDLOCK_FS).
For now, this new arraymap is only used by Landlock LSM (cf. next
commits) but it could be useful for other needs.
Changes since v2:
* add a RLIMIT_NOFILE-based limit to the maximum number of arraymap
handle entries (suggested by Andy Lutomirski)
* remove useless checks
Changes since v1:
* arraymap of handles replace custom checker groups
* simpler userland API
Signed-off-by: Mickaël Salaün <redacted>
Cc: Alexei Starovoitov <redacted>
Cc: Andy Lutomirski <redacted>
Cc: Daniel Borkmann <redacted>
Cc: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Kees Cook <redacted>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
---
include/linux/bpf.h | 14 ++++
include/uapi/linux/bpf.h | 18 +++++
kernel/bpf/arraymap.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/verifier.c | 12 ++-
4 files changed, 246 insertions(+), 1 deletion(-)
[...]
[...]
+ /*
+ * Limit number of entries in an arraymap of handles to the maximum
+ * number of open files for the current process. The maximum number of
+ * handle entries (including all arraymaps) for a process is then
+ * (RLIMIT_NOFILE - 1) * RLIMIT_NOFILE. If the process' RLIMIT_NOFILE
+ * is 0, then any entry update is forbidden.
+ *
+ * An eBPF program can inherit all the arraymap FD. The worse case is
+ * to fill a bunch of arraymaps, create an eBPF program, close the
+ * arraymap FDs, and start again. The maximum number of arraymap
+ * entries can then be close to RLIMIT_NOFILE^3.
+ *
+ * FIXME: This should be improved... any idea?
+ */
+ if (unlikely(index >= rlimit(RLIMIT_NOFILE)))
+ return -EMFILE;
I'm not sure what's best for resource management here. Landlock will
be holding open path structs, for example, but how are you expecting
to track things like network policies? An allowed IP address, for
example, doesn't have a handle outside of doing a full
socket()/connect() setup.
I think an explicit design for resource management should be
considered up front...
-Kees
--
Kees Cook
Nexus Security
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
This allows to add new eBPF programs to Landlock hooks dedicated to a
cgroup thanks to the BPF_PROG_ATTACH command. Like for socket eBPF
programs, the Landlock hooks attached to a cgroup are propagated to the
nested cgroups. However, when a new Landlock program is attached to one
of this nested cgroup, this cgroup hierarchy fork the Landlock hooks.
This design is simple and match the current CONFIG_BPF_CGROUP
inheritance. The difference lie in the fact that Landlock programs can
only be stacked but not removed. This match the append-only seccomp
behavior. Userland is free to handle Landlock hooks attached to a cgroup
in more complicated ways (e.g. continuous inheritance), but care should
be taken to properly handle error cases (e.g. memory allocation errors).
Changes since v2:
* new design based on BPF_PROG_ATTACH (suggested by Alexei Starovoitov)
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Daniel Mack <daniel@zonque.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Cc: Tejun Heo <tj@kernel.org>
Link: https://lkml.kernel.org/r/20160826021432.GA8291@ast-mbp.thefacebook.com
Link: https://lkml.kernel.org/r/20160827204307.GA43714@ast-mbp.thefacebook.com
---
include/linux/bpf-cgroup.h | 7 +++++++
include/linux/cgroup-defs.h | 2 ++
include/linux/landlock.h | 9 +++++++++
include/uapi/linux/bpf.h | 1 +
kernel/bpf/cgroup.c | 33 ++++++++++++++++++++++++++++++---
kernel/bpf/syscall.c | 11 +++++++++++
security/landlock/lsm.c | 40 +++++++++++++++++++++++++++++++++++++++-
security/landlock/manager.c | 32 ++++++++++++++++++++++++++++++++
8 files changed, 131 insertions(+), 4 deletions(-)
[...]
I get creeped out by type-controlled unions of pointers. :P I don't
have a suggestion to improve this, but I don't like seeing a pointer
type managed separately from the pointer itself as it tends to bypass
a lot of both static and dynamic checking. A union is better than a
cast of void *, but it still worries me. :)
This is not fully satisfactory for me neither but the other approach is
to use two distinct struct fields instead of a union.
Do you prefer if there is a "type" field in the "pinned" struct to
select the union?
Mickaël
On Wed, Sep 14, 2016 at 6:19 PM, Andy Lutomirski [off-list ref] wrote:
quoted
On Wed, Sep 14, 2016 at 3:14 PM, Mickaël Salaün [off-list ref] wrote:
quoted
On 14/09/2016 20:29, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
This third origin of hook call should cover all possible trigger paths
(e.g. page fault). Landlock eBPF programs can then take decisions
accordingly.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Kees Cook <redacted>
---
quoted
+ if (unlikely(in_interrupt())) {
IMO security hooks have no business being called from interrupts.
Aren't they all synchronous things done by tasks? Interrupts are
driver things.
Are you trying to check for page faults and such?
Yes, that was the idea you did put in my mind. Not sure how to deal with
this.
It's not so easy, unfortunately. The easiest reliable way might be to
set a TS_ flag on all syscall entries when TIF_SECCOMP or similar is
set.
For making this series smaller, let's leave the idea idea of interrupt
hooks out -- the intention is for stricter syscall filtering, yes?
Once things are more well established and there's a use-case for this,
it can be added back in.
On Wed, Sep 14, 2016 at 3:34 PM, Mickaël Salaün [off-list ref] wrote:
quoted
On 14/09/2016 20:43, Andy Lutomirski wrote:
quoted
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
A Landlock program will be triggered according to its subtype/origin
bitfield. The LANDLOCK_FLAG_ORIGIN_SECCOMP value will trigger the
Landlock program when a seccomp filter will return RET_LANDLOCK.
Moreover, it is possible to return a 16-bit cookie which will be
readable by the Landlock programs in its context.
Are you envisioning that the filters will return RET_LANDLOCK most of
the time or rarely? If it's most of the time, then maybe this could
be simplified a bit by unconditionally calling the landlock filter and
letting the landlock filter access a struct seccomp_data if needed.
Exposing seccomp_data in a Landlock context may be a good idea. The main
implication is that Landlock programs may then be architecture specific
(if dealing with data) as seccomp filters are. Another point is that it
remove any direct binding between seccomp filters and Landlock programs.
I will try this (more simple) approach.
Yeah, I would prefer that the seccomp code isn't doing list management
to identify the landlock hooks to trigger, etc. I think that's better
done on the LSM side. And since multiple seccomp filters could trigger
landlock, it may be best to just leave the low 16 bits unused
entirely. Then all state management is handled by the landlock eBPF
maps, not a value coming from seccomp that can get stomped on by new
filters, etc.
Right, this approach should be simpler, more efficient and independent
from seccomp. This will be in the next RFC.
On Wed, Oct 5, 2016 at 1:58 PM, Mickaël Salaün [off-list ref] wrote:
On 04/10/2016 01:43, Kees Cook wrote:
quoted
On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün [off-list ref] wrote:
quoted
This allows to add new eBPF programs to Landlock hooks dedicated to a
cgroup thanks to the BPF_PROG_ATTACH command. Like for socket eBPF
programs, the Landlock hooks attached to a cgroup are propagated to the
nested cgroups. However, when a new Landlock program is attached to one
of this nested cgroup, this cgroup hierarchy fork the Landlock hooks.
This design is simple and match the current CONFIG_BPF_CGROUP
inheritance. The difference lie in the fact that Landlock programs can
only be stacked but not removed. This match the append-only seccomp
behavior. Userland is free to handle Landlock hooks attached to a cgroup
in more complicated ways (e.g. continuous inheritance), but care should
be taken to properly handle error cases (e.g. memory allocation errors).
Changes since v2:
* new design based on BPF_PROG_ATTACH (suggested by Alexei Starovoitov)
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Daniel Mack <daniel@zonque.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Cc: Tejun Heo <tj@kernel.org>
Link: https://lkml.kernel.org/r/20160826021432.GA8291@ast-mbp.thefacebook.com
Link: https://lkml.kernel.org/r/20160827204307.GA43714@ast-mbp.thefacebook.com
---
include/linux/bpf-cgroup.h | 7 +++++++
include/linux/cgroup-defs.h | 2 ++
include/linux/landlock.h | 9 +++++++++
include/uapi/linux/bpf.h | 1 +
kernel/bpf/cgroup.c | 33 ++++++++++++++++++++++++++++++---
kernel/bpf/syscall.c | 11 +++++++++++
security/landlock/lsm.c | 40 +++++++++++++++++++++++++++++++++++++++-
security/landlock/manager.c | 32 ++++++++++++++++++++++++++++++++
8 files changed, 131 insertions(+), 4 deletions(-)
[...]
I get creeped out by type-controlled unions of pointers. :P I don't
have a suggestion to improve this, but I don't like seeing a pointer
type managed separately from the pointer itself as it tends to bypass
a lot of both static and dynamic checking. A union is better than a
cast of void *, but it still worries me. :)
This is not fully satisfactory for me neither but the other approach is
to use two distinct struct fields instead of a union.
Do you prefer if there is a "type" field in the "pinned" struct to
select the union?
Since memory usage isn't a huge deal for this, I'd actually prefer
there just be no union at all. Have a type field, and a distinct
pointer field for each type you're expecting to use. That way there
can never be confusion between types and you could even validate that
only a single field type has been populated, etc.
-Kees
--
Kees Cook
Nexus Security
On Wed, Sep 14, 2016 at 12:23 AM, Mickaël Salaün [off-list ref] wrote:
quoted
This new arraymap looks like a set and brings new properties:
* strong typing of entries: the eBPF functions get the array type of
elements instead of CONST_PTR_TO_MAP (e.g.
CONST_PTR_TO_LANDLOCK_HANDLE_FS);
* force sequential filling (i.e. replace or append-only update), which
allow quick browsing of all entries.
This strong typing is useful to statically check if the content of a map
can be passed to an eBPF function. For example, Landlock use it to store
and manage kernel objects (e.g. struct file) instead of dealing with
userland raw data. This improve efficiency and ensure that an eBPF
program can only call functions with the right high-level arguments.
The enum bpf_map_handle_type list low-level types (e.g.
BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) which are identified when
updating a map entry (handle). This handle types are used to infer a
high-level arraymap type which are listed in enum bpf_map_array_type
(e.g. BPF_MAP_ARRAY_TYPE_LANDLOCK_FS).
For now, this new arraymap is only used by Landlock LSM (cf. next
commits) but it could be useful for other needs.
Changes since v2:
* add a RLIMIT_NOFILE-based limit to the maximum number of arraymap
handle entries (suggested by Andy Lutomirski)
* remove useless checks
Changes since v1:
* arraymap of handles replace custom checker groups
* simpler userland API
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Kees Cook <redacted>
Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com
---
include/linux/bpf.h | 14 ++++
include/uapi/linux/bpf.h | 18 +++++
kernel/bpf/arraymap.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/verifier.c | 12 ++-
4 files changed, 246 insertions(+), 1 deletion(-)
[...]
[...]
+ /*
+ * Limit number of entries in an arraymap of handles to the maximum
+ * number of open files for the current process. The maximum number of
+ * handle entries (including all arraymaps) for a process is then
+ * (RLIMIT_NOFILE - 1) * RLIMIT_NOFILE. If the process' RLIMIT_NOFILE
+ * is 0, then any entry update is forbidden.
+ *
+ * An eBPF program can inherit all the arraymap FD. The worse case is
+ * to fill a bunch of arraymaps, create an eBPF program, close the
+ * arraymap FDs, and start again. The maximum number of arraymap
+ * entries can then be close to RLIMIT_NOFILE^3.
+ *
+ * FIXME: This should be improved... any idea?
+ */
+ if (unlikely(index >= rlimit(RLIMIT_NOFILE)))
+ return -EMFILE;
I'm not sure what's best for resource management here. Landlock will
be holding open path structs, for example, but how are you expecting
to track things like network policies? An allowed IP address, for
example, doesn't have a handle outside of doing a full
socket()/connect() setup.
Path and file references are hard to handle correctly but other things
should be simpler. External resources (i.e. not relative to the running
system as paths are) like network hosts or ports could simply be
expressed as raw values (like used for iptables rules). Moreover, for
network rules, relying on raw packet values (as
BPF_PROG_TYPE_SOCKET_FILTER have access to) may be more than enough.
I think an explicit design for resource management should be
considered up front...
I'm not really sure how to handle that part…
There is basically two ways to express a "kernel object": relative (with
an internal pointer to a struct, e.g. struct file) or absolute (a raw
value). Both of them use kernel memory. However, only the former may
impact other parts of the kernel (e.g. can force to hold a kernel object
like a struct dentry). The impact of this is not clear for me but it
looks like other resource managements for a process: number of open
files, number of network connections…
The more reasonable approach seems to charge the user for the (kernel)
memory dedicated to the user's policy. How can I do it? Maybe to
decrement the RLIMIT_NPROC and check the RLIMIT_AS (i.e. act like a
virtual process)?
There is no such limits with other eBPF maps (even those dealing with
FD), so this may be too much.
Mickaël
From: Thomas Graf <tgraf@suug.ch> Date: 2016-10-19 14:54:50
On 09/14/16 at 09:23am, Mickaël Salaün wrote:
This fix a pointer leak when an unprivileged eBPF program read a pointer
value from the context. Even if is_valid_access() returns a pointer
type, the eBPF verifier replace it with UNKNOWN_VALUE. The register
value containing an address is then allowed to leak. Moreover, this
prevented unprivileged eBPF programs to use functions with (legitimate)
pointer arguments.
This bug was not a problem until now because the only unprivileged eBPF
program allowed is of type BPF_PROG_TYPE_SOCKET_FILTER and all the types
from its context are UNKNOWN_VALUE.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Fixes: 969bf05eb3ce ("bpf: direct packet access")
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Can you post this fix separately? It's valid and needed outside of the
scope of this series.
From: Daniel Borkmann <daniel@iogearbox.net> Date: 2016-10-19 15:10:42
On 10/19/2016 04:54 PM, Thomas Graf wrote:
On 09/14/16 at 09:23am, Mickaël Salaün wrote:
quoted
This fix a pointer leak when an unprivileged eBPF program read a pointer
value from the context. Even if is_valid_access() returns a pointer
type, the eBPF verifier replace it with UNKNOWN_VALUE. The register
value containing an address is then allowed to leak. Moreover, this
prevented unprivileged eBPF programs to use functions with (legitimate)
pointer arguments.
This bug was not a problem until now because the only unprivileged eBPF
program allowed is of type BPF_PROG_TYPE_SOCKET_FILTER and all the types
from its context are UNKNOWN_VALUE.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Fixes: 969bf05eb3ce ("bpf: direct packet access")
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Can you post this fix separately? It's valid and needed outside of the
scope of this series.
@@ -85,6 +85,8 @@ enum bpf_arg_type {ARG_PTR_TO_CTX,/* pointer to context */ARG_ANYTHING,/* any (initialized) argument is ok */++ARG_PTR_TO_STRUCT_FILE,/* pointer to struct file */
This should go into patch 7 I guess?
quoted hunk
+void __init landlock_add_hooks(void)+{+ pr_info("landlock: Becoming ready for sandboxing\n");+ security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks));+}
Can we add the hooks when we load the first BPF program for a hook? That
would also allow to not make this conditional on a new config option
which all all distros have to enable anyway.
I would really like to see this patch split into the LSM part which
allows running BPF progs at LSM and your specific sandboxing use case
which requires the new BPF helpers, new reg type, etc.