bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v13 0/7] Landlock LSM
@ 2019-11-04 17:21 Mickaël Salaün
  2019-11-04 17:21 ` [PATCH bpf-next v13 1/7] bpf,landlock: Define an eBPF program type for Landlock hooks Mickaël Salaün
                   ` (6 more replies)
  0 siblings, 7 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-04 17:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

Hi,

Following the previous series [1], this thirteenth series mainly clean
up and add safeguards to the domain management.  This series also
improves the documentation, add comments, and fixes some latent bugs.

This is the first step of the roadmap discussed at LPC [2].  While the
intended final goal is to allow unprivileged (or non-root) users to use
Landlock, this series allows only a process with global CAP_SYS_ADMIN to
load and enforce a rule.  This may help to get feedback and avoid
unexpected behaviors.

This series can be applied on top of bpf-next, commit e93d99180abd
("selftests/bpf: Restore $(OUTPUT)/test_stub.o rule").  This can be
tested with CONFIG_BPF_SYSCALL, CONFIG_SECCOMP_FILTER and
CONFIG_SECURITY_LANDLOCK.  This patch series can be found in a Git
repository here:
https://github.com/landlock-lsm/linux/commits/landlock-v13
I would really appreciate constructive comments on the design and the
code.


# Landlock LSM

Landlock is a stackable LSM [3] intended to be used as a low-level
framework to build custom access-control/audit systems or safe endpoint
security agents.  There is currently one Landlock hook dedicated to
check ptrace(2).  This hook accepts a dedicated eBPF program, called a
Landlock program, which can currently compare its position in the
hierarchy of similar programs tied to other processes.  This enables to
enforce programmatic scoped ptrace restrictions.

The final goal of this new Linux Security Module (LSM) called Landlock
is to allow any process, including unprivileged ones, to create powerful
security sandboxes comparable to XNU Sandbox, FreeBSD Capsicum or
OpenBSD Pledge (which could be implemented with Landlock).  This kind of
sandbox is expected to help mitigate the security impact of bugs or
unexpected/malicious behaviors in user-space applications.

The use of seccomp and Landlock is more suitable with the help of a
user-space 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 the LLVM front-end, it is quite easy to
write an eBPF program with a subset of the C language.

The documentation patch contains some kernel documentation, explanations
on how to use Landlock and a FAQ.  The compiled documentation and some
talks can be found here: https://landlock.io


# Frequently asked questions

## Why is seccomp-bpf not enough?

A seccomp filter can access only raw syscall arguments (i.e. the
register values) which means that it is not possible to filter according
to the value pointed to by an argument, such as a file pathname. As an
embryonic Landlock version demonstrated (i.e. seccomp-object), 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 the
LSM-hook level. The LSM hooks are designed to handle this kind of
checks.  Landlock abstracts this approach to leverage the ability of
unprivileged users to limit themselves.

Cf. section "What it isn't?" in
Documentation/userspace-api/seccomp_filter.rst


## Why use 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 children. It makes sense to use an unique
access-restricting syscall (that should be allowed by seccomp filters)
which can only drop privileges. Moreover, a Landlock rule 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 enforcement via seccomp(2).


## Why a new LSM? Are SELinux, AppArmor, Smack and Tomoyo 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
application by its developer and *unprivileged user* as seccomp can do
for raw syscall filtering.

Differences from other (access control) LSMs:
* not only dedicated to administrators (i.e. no_new_priv);
* limited kernel attack surface (e.g. policy parsing);
* constrained policy rules (no DoS: deterministic execution time);
* do not leak more information than the loader process can legitimately
  have access to (minimize metadata inference).


# Changes since v12

* make landlock_prepend_prog() more consistant by putting its caller in
  charge of the lifetime of the Landlock domain being updated
* always copy a domain being updated, which garantee its immutability,
  and behave as a copy-on-write thanks to commit_creds()
* fix preemption
* improve documentation and comments


# Changes since v11

* rework domain management
* minor fixes


# Changes since v10

* remove all the file system related features: program types, inode
  map and expected_attach_triggers
* replace the static ptrace security policy with a new and simpler
  ptrace program (attached) type and a task_landlock_ptrace_ancestor()
  eBPF helper
* do not rely on seccomp internal structure but use stacked credentials
  insdead
* extend ptrace tests
* add more documentation
* split and rename files/patches
* miscellaneous fixes


Previous changes can be found in a previous cover-letter [4].


[1] https://lore.kernel.org/lkml/20191031164445.29426-1-mic@digikod.net/
[2] https://lore.kernel.org/lkml/5828776A.1010104@digikod.net/
[3] https://lore.kernel.org/lkml/50db058a-7dde-441b-a7f9-f6837fe8b69f@schaufler-ca.com/
[4] https://lore.kernel.org/lkml/20190721213116.23476-1-mic@digikod.net/

Regards,

Mickaël Salaün (7):
  bpf,landlock: Define an eBPF program type for Landlock hooks
  landlock: Add the management of domains
  landlock,seccomp: Apply Landlock programs to process hierarchy
  landlock: Add ptrace LSM hooks
  bpf,landlock: Add task_landlock_ptrace_ancestor() helper
  bpf,landlock: Add tests for the Landlock ptrace program type
  landlock: Add user and kernel documentation for Landlock

 Documentation/security/index.rst              |   1 +
 Documentation/security/landlock/index.rst     |  22 ++
 Documentation/security/landlock/kernel.rst    | 166 ++++++++++
 Documentation/security/landlock/user.rst      | 153 ++++++++++
 MAINTAINERS                                   |   9 +
 include/linux/bpf.h                           |   3 +
 include/linux/bpf_types.h                     |   3 +
 include/linux/landlock.h                      |  25 ++
 include/linux/lsm_hooks.h                     |   1 +
 include/uapi/linux/bpf.h                      |  23 +-
 include/uapi/linux/landlock.h                 |  39 +++
 include/uapi/linux/seccomp.h                  |   1 +
 kernel/bpf/syscall.c                          |   9 +
 kernel/bpf/verifier.c                         |  11 +
 kernel/seccomp.c                              |   4 +
 scripts/bpf_helpers_doc.py                    |   1 +
 security/Kconfig                              |   1 +
 security/Makefile                             |   2 +
 security/landlock/Kconfig                     |  19 ++
 security/landlock/Makefile                    |   6 +
 security/landlock/bpf_ptrace.c                |  98 ++++++
 security/landlock/bpf_ptrace.h                |  17 ++
 security/landlock/bpf_run.c                   |  65 ++++
 security/landlock/bpf_run.h                   |  25 ++
 security/landlock/bpf_verify.c                |  87 ++++++
 security/landlock/common.h                    |  84 +++++
 security/landlock/domain_manage.c             | 152 +++++++++
 security/landlock/domain_manage.h             |  22 ++
 security/landlock/domain_syscall.c            |  93 ++++++
 security/landlock/hooks_cred.c                |  47 +++
 security/landlock/hooks_cred.h                |  14 +
 security/landlock/hooks_ptrace.c              | 114 +++++++
 security/landlock/hooks_ptrace.h              |  19 ++
 security/landlock/init.c                      |  32 ++
 security/security.c                           |  15 +
 tools/include/uapi/linux/bpf.h                |  23 +-
 tools/include/uapi/linux/landlock.h           |  22 ++
 tools/lib/bpf/libbpf_probes.c                 |   3 +
 tools/testing/selftests/bpf/config            |   3 +
 tools/testing/selftests/bpf/test_verifier.c   |   1 +
 .../testing/selftests/bpf/verifier/landlock.c |  56 ++++
 tools/testing/selftests/landlock/.gitignore   |   5 +
 tools/testing/selftests/landlock/Makefile     |  27 ++
 tools/testing/selftests/landlock/config       |   5 +
 tools/testing/selftests/landlock/test.h       |  48 +++
 tools/testing/selftests/landlock/test_base.c  |  24 ++
 .../testing/selftests/landlock/test_ptrace.c  | 289 ++++++++++++++++++
 47 files changed, 1887 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/security/landlock/index.rst
 create mode 100644 Documentation/security/landlock/kernel.rst
 create mode 100644 Documentation/security/landlock/user.rst
 create mode 100644 include/linux/landlock.h
 create mode 100644 include/uapi/linux/landlock.h
 create mode 100644 security/landlock/Kconfig
 create mode 100644 security/landlock/Makefile
 create mode 100644 security/landlock/bpf_ptrace.c
 create mode 100644 security/landlock/bpf_ptrace.h
 create mode 100644 security/landlock/bpf_run.c
 create mode 100644 security/landlock/bpf_run.h
 create mode 100644 security/landlock/bpf_verify.c
 create mode 100644 security/landlock/common.h
 create mode 100644 security/landlock/domain_manage.c
 create mode 100644 security/landlock/domain_manage.h
 create mode 100644 security/landlock/domain_syscall.c
 create mode 100644 security/landlock/hooks_cred.c
 create mode 100644 security/landlock/hooks_cred.h
 create mode 100644 security/landlock/hooks_ptrace.c
 create mode 100644 security/landlock/hooks_ptrace.h
 create mode 100644 security/landlock/init.c
 create mode 100644 tools/include/uapi/linux/landlock.h
 create mode 100644 tools/testing/selftests/bpf/verifier/landlock.c
 create mode 100644 tools/testing/selftests/landlock/.gitignore
 create mode 100644 tools/testing/selftests/landlock/Makefile
 create mode 100644 tools/testing/selftests/landlock/config
 create mode 100644 tools/testing/selftests/landlock/test.h
 create mode 100644 tools/testing/selftests/landlock/test_base.c
 create mode 100644 tools/testing/selftests/landlock/test_ptrace.c

-- 
2.23.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v13 1/7] bpf,landlock: Define an eBPF program type for Landlock hooks
  2019-11-04 17:21 [PATCH bpf-next v13 0/7] Landlock LSM Mickaël Salaün
@ 2019-11-04 17:21 ` Mickaël Salaün
  2019-11-04 17:21 ` [PATCH bpf-next v13 2/7] landlock: Add the management of domains Mickaël Salaün
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-04 17:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

Add a new type of eBPF program used by Landlock hooks.  The goal of this
type of program is to accept or deny a requested access from userspace
to a kernel object (e.g. ptrace a process).  This will be more useful
with the next commit adding a new eBPF helper.

The context of this program type contains two items of type PTR_TO_TASK,
one for the tracer and one for the tracee.  The underlying kernel
structure is currently a task_struct pointer, but it could seamlessly
evolve to a task wrapper with dedicated rights (i.e.  capability-based
security) to fit with different use cases (e.g. get and log the task's
PID).

This new BPF program type will be registered with the Landlock LSM
initialization.

Add an initial Landlock Kconfig and update the MAINTAINERS 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: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Will Drewry <wad@chromium.org>
---

Changes since v10:
* replace file system program types with a (simpler) ptrace program type
* add an eBPF task pointer type
* split files

Changes since v9:
* handle inode put and map put, which fix unmount (reported by Al Viro)
* replace subtype with expected_attach_type and expected_attach_triggers
* check eBPF program return code

Changes since v8:
* Remove the chaining concept from the eBPF program contexts (chain and
  cookie). We need to keep these subtypes this way to be able to make
  them evolve, though.
* remove bpf_landlock_put_extra() because there is no more a "previous"
  field to free (for now)

Changes since v7:
* cosmetic fixes
* rename LANDLOCK_SUBTYPE_* to LANDLOCK_*
* cleanup UAPI definitions and move them from bpf.h to landlock.h
  (suggested by Alexei Starovoitov)
* disable Landlock by default (suggested by Alexei Starovoitov)
* rename BPF_PROG_TYPE_LANDLOCK_{RULE,HOOK}
* update the Kconfig
* update the MAINTAINERS file
* replace the IOCTL, LOCK and FCNTL events with FS_PICK, FS_WALK and
  FS_GET hook types
* add the ability to chain programs with an eBPF program file descriptor
  (i.e. the "previous" field in a Landlock subtype) and keep a state
  with a "cookie" value available from the context
* add a "triggers" subtype bitfield to match specific actions (e.g.
  append, chdir, read...)

Changes since v6:
* add 3 more sub-events: IOCTL, LOCK, FCNTL
  https://lkml.kernel.org/r/2fbc99a6-f190-f335-bd14-04bdeed35571@digikod.net
* rename LANDLOCK_VERSION to LANDLOCK_ABI to better reflect its purpose,
  and move it from landlock.h to common.h
* rename BPF_PROG_TYPE_LANDLOCK to BPF_PROG_TYPE_LANDLOCK_RULE: an eBPF
  program could be used for something else than a rule
* simplify struct landlock_context by removing the arch and syscall_nr fields
* remove all eBPF map functions call, remove ABILITY_WRITE
* refactor bpf_landlock_func_proto() (suggested by Kees Cook)
* constify pointers
* fix doc inclusion

Changes since v5:
* rename file hooks.c to init.c
* fix spelling

Changes since v4:
* merge a minimal (not enabled) LSM code and Kconfig in this commit

Changes since v3:
* split commit
* revamp the landlock_context:
  * add arch, syscall_nr and syscall_cmd (ioctl, fcntl…) to be able to
    cross-check action with the event type
  * replace args array with dedicated fields to ease the addition of new
    fields
---
 MAINTAINERS                    |  8 ++++
 include/linux/bpf.h            |  1 +
 include/linux/bpf_types.h      |  3 ++
 include/uapi/linux/bpf.h       |  2 +
 include/uapi/linux/landlock.h  | 39 ++++++++++++++++
 kernel/bpf/syscall.c           |  9 ++++
 kernel/bpf/verifier.c          |  7 +++
 security/Kconfig               |  1 +
 security/Makefile              |  2 +
 security/landlock/Kconfig      | 19 ++++++++
 security/landlock/Makefile     |  4 ++
 security/landlock/bpf_ptrace.c | 30 ++++++++++++
 security/landlock/bpf_ptrace.h | 17 +++++++
 security/landlock/bpf_verify.c | 83 ++++++++++++++++++++++++++++++++++
 security/landlock/common.h     | 30 ++++++++++++
 15 files changed, 255 insertions(+)
 create mode 100644 include/uapi/linux/landlock.h
 create mode 100644 security/landlock/Kconfig
 create mode 100644 security/landlock/Makefile
 create mode 100644 security/landlock/bpf_ptrace.c
 create mode 100644 security/landlock/bpf_ptrace.h
 create mode 100644 security/landlock/bpf_verify.c
 create mode 100644 security/landlock/common.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 7fc074632eac..4cabb85ea52d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9146,6 +9146,14 @@ F:	net/core/skmsg.c
 F:	net/core/sock_map.c
 F:	net/ipv4/tcp_bpf.c
 
+LANDLOCK SECURITY MODULE
+M:	Mickaël Salaün <mic@digikod.net>
+S:	Supported
+F:	include/uapi/linux/landlock.h
+F:	security/landlock/
+K:	landlock
+K:	LANDLOCK
+
 LANTIQ / INTEL Ethernet drivers
 M:	Hauke Mehrtens <hauke@hauke-m.de>
 L:	netdev@vger.kernel.org
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 171be30fe0ae..819a3e207438 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -291,6 +291,7 @@ enum bpf_reg_type {
 	PTR_TO_TP_BUFFER,	 /* reg points to a writable raw tp's buffer */
 	PTR_TO_XDP_SOCK,	 /* reg points to struct xdp_sock */
 	PTR_TO_BTF_ID,		 /* reg points to kernel struct */
+	PTR_TO_TASK,		 /* reg points to struct task_struct */
 };
 
 /* The information passed from prog-specific *_is_valid_access
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 36a9c2325176..bddabc961a3b 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -38,6 +38,9 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
 #ifdef CONFIG_INET
 BPF_PROG_TYPE(BPF_PROG_TYPE_SK_REUSEPORT, sk_reuseport)
 #endif
+#ifdef CONFIG_SECURITY_LANDLOCK
+BPF_PROG_TYPE(BPF_PROG_TYPE_LANDLOCK_HOOK, landlock)
+#endif
 
 BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 4af8b0819a32..6e4147790f96 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -173,6 +173,7 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_CGROUP_SYSCTL,
 	BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE,
 	BPF_PROG_TYPE_CGROUP_SOCKOPT,
+	BPF_PROG_TYPE_LANDLOCK_HOOK,
 };
 
 enum bpf_attach_type {
@@ -199,6 +200,7 @@ enum bpf_attach_type {
 	BPF_CGROUP_UDP6_RECVMSG,
 	BPF_CGROUP_GETSOCKOPT,
 	BPF_CGROUP_SETSOCKOPT,
+	BPF_LANDLOCK_PTRACE,
 	__MAX_BPF_ATTACH_TYPE
 };
 
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
new file mode 100644
index 000000000000..3ffe3cbdbad6
--- /dev/null
+++ b/include/uapi/linux/landlock.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Landlock - UAPI headers
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#ifndef _UAPI__LINUX_LANDLOCK_H__
+#define _UAPI__LINUX_LANDLOCK_H__
+
+#include <linux/types.h>
+
+/**
+ * DOC: landlock_ret
+ *
+ * The return value of a landlock program is a bitmask that can allow or deny
+ * the action for which the program is run.
+ *
+ * In the future, this could be used to trigger an audit event as well.
+ *
+ * - %LANDLOCK_RET_ALLOW
+ * - %LANDLOCK_RET_DENY
+ */
+#define LANDLOCK_RET_ALLOW	0
+#define LANDLOCK_RET_DENY	1
+
+/**
+ * struct landlock_context_ptrace - context accessible to BPF_LANDLOCK_PTRACE
+ *
+ * @tracer: pointer to the task requesting to debug @tracee
+ * @tracee: pointer to the task being debugged
+ */
+struct landlock_context_ptrace {
+	__u64 tracer;
+	__u64 tracee;
+};
+
+#endif /* _UAPI__LINUX_LANDLOCK_H__ */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ff5225759553..5159e582a0d8 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1621,6 +1621,15 @@ bpf_prog_load_check_attach(enum bpf_prog_type prog_type,
 		default:
 			return -EINVAL;
 		}
+#ifdef CONFIG_SECURITY_LANDLOCK
+	case BPF_PROG_TYPE_LANDLOCK_HOOK:
+		switch (expected_attach_type) {
+		case BPF_LANDLOCK_PTRACE:
+			return 0;
+		default:
+			return -EINVAL;
+		}
+#endif
 	default:
 		return 0;
 	}
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c59778c0fc4d..ebf1991906b7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -421,6 +421,7 @@ static const char * const reg_type_str[] = {
 	[PTR_TO_TP_BUFFER]	= "tp_buffer",
 	[PTR_TO_XDP_SOCK]	= "xdp_sock",
 	[PTR_TO_BTF_ID]		= "ptr_",
+	[PTR_TO_TASK]		= "task",
 };
 
 static char slot_type_char[] = {
@@ -1878,6 +1879,7 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
 	case PTR_TO_TCP_SOCK:
 	case PTR_TO_TCP_SOCK_OR_NULL:
 	case PTR_TO_XDP_SOCK:
+	case PTR_TO_TASK:
 		return true;
 	default:
 		return false;
@@ -2600,6 +2602,9 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
 	case PTR_TO_XDP_SOCK:
 		pointer_desc = "xdp_sock ";
 		break;
+	case PTR_TO_TASK:
+		pointer_desc = "task ";
+		break;
 	default:
 		break;
 	}
@@ -4527,6 +4532,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
 	case PTR_TO_TCP_SOCK:
 	case PTR_TO_TCP_SOCK_OR_NULL:
 	case PTR_TO_XDP_SOCK:
+	case PTR_TO_TASK:
 		verbose(env, "R%d pointer arithmetic on %s prohibited\n",
 			dst, reg_type_str[ptr_reg->type]);
 		return -EACCES;
@@ -6278,6 +6284,7 @@ static int check_return_code(struct bpf_verifier_env *env)
 	case BPF_PROG_TYPE_CGROUP_DEVICE:
 	case BPF_PROG_TYPE_CGROUP_SYSCTL:
 	case BPF_PROG_TYPE_CGROUP_SOCKOPT:
+	case BPF_PROG_TYPE_LANDLOCK_HOOK:
 		break;
 	default:
 		return 0;
diff --git a/security/Kconfig b/security/Kconfig
index 2a1a2d396228..9d9981394fb0 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -238,6 +238,7 @@ source "security/loadpin/Kconfig"
 source "security/yama/Kconfig"
 source "security/safesetid/Kconfig"
 source "security/lockdown/Kconfig"
+source "security/landlock/Kconfig"
 
 source "security/integrity/Kconfig"
 
diff --git a/security/Makefile b/security/Makefile
index be1dd9d2cb2f..60b7f6f2fd30 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -12,6 +12,7 @@ subdir-$(CONFIG_SECURITY_YAMA)		+= yama
 subdir-$(CONFIG_SECURITY_LOADPIN)	+= loadpin
 subdir-$(CONFIG_SECURITY_SAFESETID)    += safesetid
 subdir-$(CONFIG_SECURITY_LOCKDOWN_LSM)	+= lockdown
+subdir-$(CONFIG_SECURITY_LANDLOCK)		+= landlock
 
 # always enable default capabilities
 obj-y					+= commoncap.o
@@ -29,6 +30,7 @@ obj-$(CONFIG_SECURITY_YAMA)		+= yama/
 obj-$(CONFIG_SECURITY_LOADPIN)		+= loadpin/
 obj-$(CONFIG_SECURITY_SAFESETID)       += safesetid/
 obj-$(CONFIG_SECURITY_LOCKDOWN_LSM)	+= lockdown/
+obj-$(CONFIG_SECURITY_LANDLOCK)	+= landlock/
 obj-$(CONFIG_CGROUP_DEVICE)		+= device_cgroup.o
 
 # Object integrity file lists
diff --git a/security/landlock/Kconfig b/security/landlock/Kconfig
new file mode 100644
index 000000000000..44921bd72380
--- /dev/null
+++ b/security/landlock/Kconfig
@@ -0,0 +1,19 @@
+config SECURITY_LANDLOCK
+	bool "Landlock support"
+	depends on SECURITY
+	depends on BPF_SYSCALL
+	depends on SECCOMP_FILTER
+	default n
+	help
+	  This selects Landlock, a programmatic access control.  It enables to
+	  restrict processes on the fly (i.e. create a sandbox) or log some
+	  actions.  The security policy is a set of eBPF programs, dedicated to
+	  allow or deny a list of actions on specific kernel objects (e.g.
+	  process).
+
+	  You need to enable seccomp filter to apply a security policy to a
+	  process hierarchy (e.g. application with built-in sandboxing).
+
+	  See Documentation/security/landlock/ for further information.
+
+	  If you are unsure how to answer this question, answer N.
diff --git a/security/landlock/Makefile b/security/landlock/Makefile
new file mode 100644
index 000000000000..682b798c6b76
--- /dev/null
+++ b/security/landlock/Makefile
@@ -0,0 +1,4 @@
+obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
+
+landlock-y := \
+	bpf_verify.o bpf_ptrace.o
diff --git a/security/landlock/bpf_ptrace.c b/security/landlock/bpf_ptrace.c
new file mode 100644
index 000000000000..2ec73078ad01
--- /dev/null
+++ b/security/landlock/bpf_ptrace.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - eBPF ptrace
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2019 ANSSI
+ */
+
+#include <linux/bpf.h>
+#include <uapi/linux/landlock.h>
+
+#include "bpf_ptrace.h"
+
+bool landlock_is_valid_access_ptrace(int off, enum bpf_access_type type,
+		enum bpf_reg_type *reg_type, int *max_size)
+{
+	if (type != BPF_READ)
+		return false;
+
+	switch (off) {
+	case offsetof(struct landlock_context_ptrace, tracer):
+		/* fall through */
+	case offsetof(struct landlock_context_ptrace, tracee):
+		*reg_type = PTR_TO_TASK;
+		*max_size = sizeof(u64);
+		return true;
+	default:
+		return false;
+	}
+}
diff --git a/security/landlock/bpf_ptrace.h b/security/landlock/bpf_ptrace.h
new file mode 100644
index 000000000000..85edce37b70a
--- /dev/null
+++ b/security/landlock/bpf_ptrace.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - eBPF ptrace headers
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2019 ANSSI
+ */
+
+#ifndef _SECURITY_LANDLOCK_BPF_PTRACE_H
+#define _SECURITY_LANDLOCK_BPF_PTRACE_H
+
+#include <linux/bpf.h>
+
+bool landlock_is_valid_access_ptrace(int off, enum bpf_access_type type,
+		enum bpf_reg_type *reg_type, int *max_size);
+
+#endif /* _SECURITY_LANDLOCK_BPF_PTRACE_H */
diff --git a/security/landlock/bpf_verify.c b/security/landlock/bpf_verify.c
new file mode 100644
index 000000000000..6ed921588178
--- /dev/null
+++ b/security/landlock/bpf_verify.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - eBPF program verifications
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <linux/bpf.h>
+#include <linux/filter.h>
+
+#include "common.h"
+#include "bpf_ptrace.h"
+
+static bool bpf_landlock_is_valid_access(int off, int size,
+		enum bpf_access_type type, const struct bpf_prog *prog,
+		struct bpf_insn_access_aux *info)
+{
+	enum bpf_reg_type reg_type = NOT_INIT;
+	int max_size = 0;
+
+	if (WARN_ON(!prog->expected_attach_type))
+		return false;
+
+	if (off < 0)
+		return false;
+	if (size <= 0 || size > sizeof(__u64))
+		return false;
+
+	/* set register type and max size */
+	switch (get_hook_type(prog)) {
+	case LANDLOCK_HOOK_PTRACE:
+		if (!landlock_is_valid_access_ptrace(off, type, &reg_type,
+					&max_size))
+			return false;
+		break;
+	}
+
+	/* check memory range access */
+	switch (reg_type) {
+	case NOT_INIT:
+		return false;
+	case SCALAR_VALUE:
+		/* allow partial raw value */
+		if (size > max_size)
+			return false;
+		info->ctx_field_size = max_size;
+		break;
+	default:
+		/* deny partial pointer */
+		if (size != max_size)
+			return false;
+	}
+
+	info->reg_type = reg_type;
+	return true;
+}
+
+static const struct bpf_func_proto *bpf_landlock_func_proto(
+		enum bpf_func_id func_id,
+		const struct bpf_prog *prog)
+{
+	if (WARN_ON(!prog->expected_attach_type))
+		return NULL;
+
+	switch (func_id) {
+	case BPF_FUNC_map_lookup_elem:
+		return &bpf_map_lookup_elem_proto;
+	case BPF_FUNC_map_update_elem:
+		return &bpf_map_update_elem_proto;
+	case BPF_FUNC_map_delete_elem:
+		return &bpf_map_delete_elem_proto;
+	default:
+		return NULL;
+	}
+}
+
+const struct bpf_verifier_ops landlock_verifier_ops = {
+	.get_func_proto	= bpf_landlock_func_proto,
+	.is_valid_access = bpf_landlock_is_valid_access,
+};
+
+const struct bpf_prog_ops landlock_prog_ops = {};
diff --git a/security/landlock/common.h b/security/landlock/common.h
new file mode 100644
index 000000000000..0234c4bc4acd
--- /dev/null
+++ b/security/landlock/common.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - private headers
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#ifndef _SECURITY_LANDLOCK_COMMON_H
+#define _SECURITY_LANDLOCK_COMMON_H
+
+#include <linux/bpf.h>
+#include <linux/filter.h>
+
+enum landlock_hook_type {
+	LANDLOCK_HOOK_PTRACE = 1,
+};
+
+static inline enum landlock_hook_type get_hook_type(const struct bpf_prog *prog)
+{
+	switch (prog->expected_attach_type) {
+	case BPF_LANDLOCK_PTRACE:
+		return LANDLOCK_HOOK_PTRACE;
+	default:
+		WARN_ON(1);
+		return BPF_LANDLOCK_PTRACE;
+	}
+}
+
+#endif /* _SECURITY_LANDLOCK_COMMON_H */
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v13 2/7] landlock: Add the management of domains
  2019-11-04 17:21 [PATCH bpf-next v13 0/7] Landlock LSM Mickaël Salaün
  2019-11-04 17:21 ` [PATCH bpf-next v13 1/7] bpf,landlock: Define an eBPF program type for Landlock hooks Mickaël Salaün
@ 2019-11-04 17:21 ` Mickaël Salaün
  2019-11-04 17:21 ` [PATCH bpf-next v13 3/7] landlock,seccomp: Apply Landlock programs to process hierarchy Mickaël Salaün
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-04 17:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

A Landlock domain is a set of eBPF programs.  There is a list for each
different program types that can be run on a specific Landlock hook
(e.g. ptrace).  A domain is tied to a set of subjects (i.e. tasks).  A
Landlock program should not try (nor be able) to infer which subject is
currently enforced, but to have a unique security policy for all
subjects tied to the same domain.  This make the reasoning much easier
and help avoid pitfalls.

The next commits tie a domain to a task's credentials thanks to
seccomp(2), but we could use cgroups or a security file-system to
enforce a sysadmin-defined policy .

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: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Will Drewry <wad@chromium.org>
---

Changes since v12:
* move the landlock_put_domain() call from landlock_prepend_prog() to
  domain_syscall.c:landlock_seccomp_prepend_prog() (next commit): keep
  the same semantic but don't conditionally own the domain (safeguard
  domain owning)
* always copy a domain to-be-modified which guarantee domain
  immutability: the callers of landlock_prepend_prog() are now fully in
  charge of concurrency handling, which is currently only handled by
  prepare_creds() and commit_creds() (next commit)
* simplify landlock_prepend_prog()
* use a consistent naming

Changes since v11:
* remove old code from previous refactoring (removing the program
  chaining concept) and simplify program prepending (reported by Serge
  E. Hallyn):
  * simplify landlock_prepend_prog() and merge it with
    store_landlock_prog()
  * add new_prog_list() and rework new_landlock_domain()
  * remove the extra page allocation checks, only rely on the eBPF
    program checks
* replace the -EINVAL for the duplicate program check with the -EEXIST

Changes since v10:
* rename files and names to clearly define a domain
* create a standalone patch to ease review
---
 security/landlock/Makefile        |   3 +-
 security/landlock/common.h        |  38 ++++++++
 security/landlock/domain_manage.c | 152 ++++++++++++++++++++++++++++++
 security/landlock/domain_manage.h |  22 +++++
 4 files changed, 214 insertions(+), 1 deletion(-)
 create mode 100644 security/landlock/domain_manage.c
 create mode 100644 security/landlock/domain_manage.h

diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index 682b798c6b76..dd5f70185778 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
 
 landlock-y := \
-	bpf_verify.o bpf_ptrace.o
+	bpf_verify.o bpf_ptrace.o \
+	domain_manage.o
diff --git a/security/landlock/common.h b/security/landlock/common.h
index 0234c4bc4acd..fb2990eb5fb4 100644
--- a/security/landlock/common.h
+++ b/security/landlock/common.h
@@ -11,11 +11,49 @@
 
 #include <linux/bpf.h>
 #include <linux/filter.h>
+#include <linux/refcount.h>
 
 enum landlock_hook_type {
 	LANDLOCK_HOOK_PTRACE = 1,
 };
 
+#define _LANDLOCK_HOOK_LAST LANDLOCK_HOOK_PTRACE
+
+struct landlock_prog_list {
+	struct landlock_prog_list *prev;
+	struct bpf_prog *prog;
+	refcount_t usage;
+};
+
+/**
+ * struct landlock_domain - Landlock programs enforced on a set of tasks
+ *
+ * When prepending a new program, if &struct landlock_domain is shared with
+ * other tasks, then duplicate it and prepend the program to this new &struct
+ * landlock_domain.
+ *
+ * @usage: reference count to manage the object lifetime. When a task needs to
+ *	   add Landlock programs and if @usage is greater than 1, then the
+ *	   task must duplicate &struct landlock_domain to not change the
+ *	   children's programs as well.
+ * @programs: array of non-NULL &struct landlock_prog_list pointers
+ */
+struct landlock_domain {
+	struct landlock_prog_list *programs[_LANDLOCK_HOOK_LAST];
+	refcount_t usage;
+};
+
+/**
+ * get_hook_index - get an index for the programs of struct landlock_prog_set
+ *
+ * @type: a Landlock hook type
+ */
+static inline size_t get_hook_index(enum landlock_hook_type type)
+{
+	/* type ID > 0 for loaded programs */
+	return type - 1;
+}
+
 static inline enum landlock_hook_type get_hook_type(const struct bpf_prog *prog)
 {
 	switch (prog->expected_attach_type) {
diff --git a/security/landlock/domain_manage.c b/security/landlock/domain_manage.c
new file mode 100644
index 000000000000..b65960611d09
--- /dev/null
+++ b/security/landlock/domain_manage.c
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - domain management
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/refcount.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include "common.h"
+#include "domain_manage.h"
+
+void landlock_get_domain(struct landlock_domain *domain)
+{
+	if (!domain)
+		return;
+	refcount_inc(&domain->usage);
+}
+
+static void put_prog_list(struct landlock_prog_list *prog_list)
+{
+	struct landlock_prog_list *orig = prog_list;
+
+	/* clean up single-reference branches iteratively */
+	while (orig && refcount_dec_and_test(&orig->usage)) {
+		struct landlock_prog_list *freeme = orig;
+
+		if (orig->prog)
+			bpf_prog_put(orig->prog);
+		orig = orig->prev;
+		kfree(freeme);
+	}
+}
+
+void landlock_put_domain(struct landlock_domain *domain)
+{
+	if (domain && refcount_dec_and_test(&domain->usage)) {
+		size_t i;
+
+		for (i = 0; i < ARRAY_SIZE(domain->programs); i++)
+			put_prog_list(domain->programs[i]);
+		kfree(domain);
+	}
+}
+
+static struct landlock_prog_list *create_prog_list(struct bpf_prog *prog)
+{
+	struct landlock_prog_list *new_list;
+
+	if (WARN_ON(IS_ERR_OR_NULL(prog)))
+		return ERR_PTR(-EFAULT);
+	if (prog->type != BPF_PROG_TYPE_LANDLOCK_HOOK)
+		return ERR_PTR(-EINVAL);
+	prog = bpf_prog_inc(prog);
+	if (IS_ERR(prog))
+		return ERR_CAST(prog);
+	new_list = kzalloc(sizeof(*new_list), GFP_KERNEL);
+	if (!new_list) {
+		bpf_prog_put(prog);
+		return ERR_PTR(-ENOMEM);
+	}
+	new_list->prog = prog;
+	refcount_set(&new_list->usage, 1);
+	return new_list;
+}
+
+static struct landlock_domain *create_domain(struct bpf_prog *prog)
+{
+	struct landlock_domain *new_dom;
+	struct landlock_prog_list *new_list;
+	size_t hook;
+
+	/* programs[] filled with NULL values */
+	new_dom = kzalloc(sizeof(*new_dom), GFP_KERNEL);
+	if (!new_dom)
+		return ERR_PTR(-ENOMEM);
+	refcount_set(&new_dom->usage, 1);
+	new_list = create_prog_list(prog);
+	if (IS_ERR(new_list)) {
+		kfree(new_dom);
+		return ERR_CAST(new_list);
+	}
+	hook = get_hook_index(get_hook_type(prog));
+	new_dom->programs[hook] = new_list;
+	return new_dom;
+}
+
+/**
+ * landlock_prepend_prog - extend a Landlock domain with an eBPF program
+ *
+ * Prepend @prog to @domain if @prog is not already in @domain.
+ *
+ * @domain: domain to copy and extend with @prog. This domain must not be
+ *          modified by another function than this one to guarantee domain
+ *          immutability.
+ * @prog: non-NULL Landlock program to prepend to a copy of @domain.  @prog
+ *        will be owned by landlock_prepend_prog(). You can then call
+ *        bpf_prog_put(@prog) after.
+ *
+ * Return a copy of @domain (with @prog prepended) when OK. Return a pointer
+ * error otherwise.
+ */
+struct landlock_domain *landlock_prepend_prog(struct landlock_domain *domain,
+		struct bpf_prog *prog)
+{
+	struct landlock_prog_list *walker;
+	struct landlock_domain *new_dom;
+	size_t i, hook;
+
+	if (WARN_ON(!prog))
+		return ERR_PTR(-EFAULT);
+	if (prog->type != BPF_PROG_TYPE_LANDLOCK_HOOK)
+		return ERR_PTR(-EINVAL);
+
+	if (!domain)
+		return create_domain(prog);
+
+	hook = get_hook_index(get_hook_type(prog));
+	/* check for similar program */
+	for (walker = domain->programs[hook]; walker;
+			walker = walker->prev) {
+		/* don't allow duplicate programs */
+		if (prog == walker->prog)
+			return ERR_PTR(-EEXIST);
+	}
+
+	new_dom = create_domain(prog);
+	if (IS_ERR(new_dom))
+		return new_dom;
+
+	/* copy @domain (which is guarantee to be immutable) */
+	for (i = 0; i < ARRAY_SIZE(new_dom->programs); i++) {
+		struct landlock_prog_list *current_list;
+		struct landlock_prog_list **new_list;
+
+		current_list = domain->programs[i];
+		if (!current_list)
+			continue;
+		refcount_inc(&current_list->usage);
+		new_list = &new_dom->programs[i];
+		if (*new_list)
+			new_list = &(*new_list)->prev;
+		/* do not increment usage */
+		*new_list = current_list;
+	}
+	return new_dom;
+}
diff --git a/security/landlock/domain_manage.h b/security/landlock/domain_manage.h
new file mode 100644
index 000000000000..d32c65c941c0
--- /dev/null
+++ b/security/landlock/domain_manage.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - domain management headers
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#ifndef _SECURITY_LANDLOCK_DOMAIN_MANAGE_H
+#define _SECURITY_LANDLOCK_DOMAIN_MANAGE_H
+
+#include <linux/filter.h>
+
+#include "common.h"
+
+void landlock_get_domain(struct landlock_domain *domain);
+void landlock_put_domain(struct landlock_domain *domain);
+
+struct landlock_domain *landlock_prepend_prog(struct landlock_domain *domain,
+		struct bpf_prog *prog);
+
+#endif /* _SECURITY_LANDLOCK_DOMAIN_MANAGE_H */
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v13 3/7] landlock,seccomp: Apply Landlock programs to process hierarchy
  2019-11-04 17:21 [PATCH bpf-next v13 0/7] Landlock LSM Mickaël Salaün
  2019-11-04 17:21 ` [PATCH bpf-next v13 1/7] bpf,landlock: Define an eBPF program type for Landlock hooks Mickaël Salaün
  2019-11-04 17:21 ` [PATCH bpf-next v13 2/7] landlock: Add the management of domains Mickaël Salaün
@ 2019-11-04 17:21 ` Mickaël Salaün
  2019-11-04 17:21 ` [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks Mickaël Salaün
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-04 17:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

The seccomp(2) syscall can be used by a task to apply a Landlock program
to itself. As a seccomp filter, a Landlock program is enforced for the
current task and all its future children. A program is immutable and a
task can only add new restricting programs to itself, forming a list of
programs.

A Landlock program is tied to a Landlock hook. If the action on a kernel
object is allowed by the other Linux security mechanisms (e.g. DAC,
capabilities, other LSM), then a Landlock hook related to this kind of
object is triggered. The list of programs for this hook is then
evaluated. Each program return a binary value which can deny the action
on a kernel object with a non-zero value. If every programs of the list
return zero, then the action on the object is allowed.

The next commit adds the LSM hooks to enforce the memory protection
programs on the appropriate process hierarchies.

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: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Will Drewry <wad@chromium.org>
Link: https://lore.kernel.org/lkml/c10a503d-5e35-7785-2f3d-25ed8dd63fab@digikod.net/
---

Changes since v12:
* move the landlock_put_domain() call from domain_manage.c: same
  semantic (for this call) but less error-prone and self-explanatory
  (only put a domain when it is effectively replaced)
* only handle copy of domains (cf. domain management changes)
* use a consistent naming
* extend comment about unprivileged enforcement

Changes since v11:
* fix build of seccomp without Landlock (reported by kbuild test robot)

Changes since v10:
* rewrite the Landlock program attaching mechanisme to not rely on
  internal seccomp structures but only on the (LSM-stacked) task's
  credentials:
  * make the use of seccomp (and task's credentials) optional if not
    relying on its syscall, which may be useful for domains defined by
    other means (e.g. cgroups or system-wide thanks to a dedicated
    securityfs)

Changes since v9:
* replace subtype with expected_attach_type and expected_attach_triggers

Changes since v8:
* Remove the chaining concept from the eBPF program contexts (chain and
  cookie). We need to keep these subtypes this way to be able to make
  them evolve, though.

Changes since v7:
* handle and verify program chains
* split and rename providers.c to enforce.c and enforce_seccomp.c
* rename LANDLOCK_SUBTYPE_* to LANDLOCK_*

Changes since v6:
* rename some functions with more accurate names to reflect that an eBPF
  program for Landlock could be used for something else than a rule
* reword rule "appending" to "prepending" and explain it
* remove the superfluous no_new_privs check, only check global
  CAP_SYS_ADMIN when prepending a Landlock rule (needed for containers)
* create and use {get,put}_seccomp_landlock() (suggested by Kees Cook)
* replace ifdef with static inlined function (suggested by Kees Cook)
* use get_user() (suggested by Kees Cook)
* replace atomic_t with refcount_t (requested by Kees Cook)
* move struct landlock_{rule,events} from landlock.h to common.h
* cleanup headers

Changes since v5:
* remove struct landlock_node and use a similar inheritance mechanisme
  as seccomp-bpf (requested by Andy Lutomirski)
* rename SECCOMP_ADD_LANDLOCK_RULE to SECCOMP_APPEND_LANDLOCK_RULE
* rename file manager.c to providers.c
* add comments
* typo and cosmetic fixes

Changes since v4:
* merge manager and seccomp patches
* return -EFAULT in seccomp(2) when user_bpf_fd is null to easely check
  if Landlock is supported
* only allow a process with the global CAP_SYS_ADMIN to use Landlock
  (will be lifted in the future)
* add an early check to exit as soon as possible if the current process
  does not have Landlock rules

Changes since v3:
* remove the hard link with seccomp (suggested by Andy Lutomirski and
  Kees Cook):
  * remove the cookie which could imply multiple evaluation of Landlock
    rules
  * remove the origin field in struct landlock_data
* remove documentation fix (merged upstream)
* rename the new seccomp command to SECCOMP_ADD_LANDLOCK_RULE
* internal renaming
* split commit
* new design to be able to inherit on the fly the parent rules

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
---
 MAINTAINERS                        |  1 +
 include/linux/landlock.h           | 25 ++++++++
 include/linux/lsm_hooks.h          |  1 +
 include/uapi/linux/seccomp.h       |  1 +
 kernel/seccomp.c                   |  4 ++
 security/landlock/Makefile         |  5 +-
 security/landlock/common.h         | 16 +++++
 security/landlock/domain_syscall.c | 93 ++++++++++++++++++++++++++++++
 security/landlock/hooks_cred.c     | 47 +++++++++++++++
 security/landlock/hooks_cred.h     | 14 +++++
 security/landlock/init.c           | 30 ++++++++++
 security/security.c                | 15 +++++
 12 files changed, 250 insertions(+), 2 deletions(-)
 create mode 100644 include/linux/landlock.h
 create mode 100644 security/landlock/domain_syscall.c
 create mode 100644 security/landlock/hooks_cred.c
 create mode 100644 security/landlock/hooks_cred.h
 create mode 100644 security/landlock/init.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4cabb85ea52d..32bfd88159b0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9149,6 +9149,7 @@ F:	net/ipv4/tcp_bpf.c
 LANDLOCK SECURITY MODULE
 M:	Mickaël Salaün <mic@digikod.net>
 S:	Supported
+F:	include/linux/landlock.h
 F:	include/uapi/linux/landlock.h
 F:	security/landlock/
 K:	landlock
diff --git a/include/linux/landlock.h b/include/linux/landlock.h
new file mode 100644
index 000000000000..ffbf2397c459
--- /dev/null
+++ b/include/linux/landlock.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Landlock LSM - public kernel headers
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#ifndef _LINUX_LANDLOCK_H
+#define _LINUX_LANDLOCK_H
+
+#include <linux/errno.h>
+
+#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_SECURITY_LANDLOCK)
+extern int landlock_seccomp_prepend_prog(unsigned int flags,
+		const int __user *user_bpf_fd);
+#else /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK */
+static inline int landlock_seccomp_prepend_prog(unsigned int flags,
+		const int __user *user_bpf_fd)
+{
+		return -EINVAL;
+}
+#endif /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK */
+
+#endif /* _LINUX_LANDLOCK_H */
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index a3763247547c..a8ba679b388a 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2106,6 +2106,7 @@ extern void security_add_hooks(struct security_hook_list *hooks, int count,
 enum lsm_order {
 	LSM_ORDER_FIRST = -1,	/* This is only for capabilities. */
 	LSM_ORDER_MUTABLE = 0,
+	LSM_ORDER_LAST = 1,	/* potentially-unprivileged LSM */
 };
 
 struct lsm_info {
diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index 90734aa5aa36..bce6534e7feb 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -16,6 +16,7 @@
 #define SECCOMP_SET_MODE_FILTER		1
 #define SECCOMP_GET_ACTION_AVAIL	2
 #define SECCOMP_GET_NOTIF_SIZES		3
+#define SECCOMP_PREPEND_LANDLOCK_PROG	4
 
 /* Valid flags for SECCOMP_SET_MODE_FILTER */
 #define SECCOMP_FILTER_FLAG_TSYNC		(1UL << 0)
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index dba52a7db5e8..0688a1439587 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -19,6 +19,7 @@
 #include <linux/compat.h>
 #include <linux/coredump.h>
 #include <linux/kmemleak.h>
+#include <linux/landlock.h>
 #include <linux/nospec.h>
 #include <linux/prctl.h>
 #include <linux/sched.h>
@@ -1397,6 +1398,9 @@ static long do_seccomp(unsigned int op, unsigned int flags,
 			return -EINVAL;
 
 		return seccomp_get_notif_sizes(uargs);
+	case SECCOMP_PREPEND_LANDLOCK_PROG:
+		return landlock_seccomp_prepend_prog(flags,
+				(const int __user *)uargs);
 	default:
 		return -EINVAL;
 	}
diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index dd5f70185778..0b291f2c027c 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
 
-landlock-y := \
+landlock-y := init.o \
 	bpf_verify.o bpf_ptrace.o \
-	domain_manage.o
+	domain_manage.o domain_syscall.o \
+	hooks_cred.o
diff --git a/security/landlock/common.h b/security/landlock/common.h
index fb2990eb5fb4..3ae8340a5b3d 100644
--- a/security/landlock/common.h
+++ b/security/landlock/common.h
@@ -10,9 +10,15 @@
 #define _SECURITY_LANDLOCK_COMMON_H
 
 #include <linux/bpf.h>
+#include <linux/cred.h>
 #include <linux/filter.h>
+#include <linux/lsm_hooks.h>
 #include <linux/refcount.h>
 
+#define LANDLOCK_NAME "landlock"
+
+extern struct lsm_blob_sizes landlock_blob_sizes;
+
 enum landlock_hook_type {
 	LANDLOCK_HOOK_PTRACE = 1,
 };
@@ -43,6 +49,16 @@ struct landlock_domain {
 	refcount_t usage;
 };
 
+struct landlock_cred_security {
+	struct landlock_domain *domain;
+};
+
+static inline struct landlock_cred_security *landlock_cred(
+		const struct cred *cred)
+{
+	return cred->security + landlock_blob_sizes.lbs_cred;
+}
+
 /**
  * get_hook_index - get an index for the programs of struct landlock_prog_set
  *
diff --git a/security/landlock/domain_syscall.c b/security/landlock/domain_syscall.c
new file mode 100644
index 000000000000..022393841a0a
--- /dev/null
+++ b/security/landlock/domain_syscall.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - seccomp syscall
+ *
+ * Copyright © 2016-2018 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#ifdef CONFIG_SECCOMP_FILTER
+
+#include <asm/current.h>
+#include <linux/bpf.h>
+#include <linux/capability.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/filter.h>
+#include <linux/landlock.h>
+#include <linux/refcount.h>
+#include <linux/sched.h>
+#include <linux/seccomp.h>
+#include <linux/uaccess.h>
+
+#include "common.h"
+#include "domain_manage.h"
+
+/**
+ * landlock_seccomp_prepend_prog - attach a Landlock program to the current
+ *                                 task
+ *
+ * current->cred->security[landlock]->domain is lazily allocated. When a new
+ * credential is created, only a pointer is copied.  When a new Landlock
+ * program is added by a task, if there is other references to this task's
+ * domain, then a new allocation is made to contain an array pointing to
+ * Landlock program lists.  This design enable low-performance impact and is
+ * memory efficient while keeping the property of prepend-only programs.
+ *
+ * For now, installing a Landlock program requires that the requesting task has
+ * the global CAP_SYS_ADMIN. We cannot force the use of no_new_privs to not
+ * exclude containers where a process may legitimately acquire more privileges
+ * thanks to an SUID binary.
+ *
+ * @flags: not used, must be 0
+ * @user_bpf_fd: file descriptor pointing to a loaded Landlock prog
+ */
+int landlock_seccomp_prepend_prog(unsigned int flags,
+		const int __user *user_bpf_fd)
+{
+	struct landlock_domain *new_dom;
+	struct cred *new_cred;
+	struct landlock_cred_security *new_llcred;
+	struct bpf_prog *prog;
+	int bpf_fd, err;
+
+	/*
+	 * It is planned to replaced the CAP_SYS_ADMIN check with a
+	 * no_new_privs check to allow unprivileged tasks to sandbox
+	 * themselves.  However, they may not be allowed to directly create an
+	 * eBPF program, but could received it from a privileged service.
+	 */
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+	/* enable to check if Landlock is supported with early EFAULT */
+	if (!user_bpf_fd)
+		return -EFAULT;
+	if (flags)
+		return -EINVAL;
+	err = get_user(bpf_fd, user_bpf_fd);
+	if (err)
+		return err;
+	prog = bpf_prog_get(bpf_fd);
+	if (IS_ERR(prog))
+		return PTR_ERR(prog);
+
+	new_cred = prepare_creds();
+	if (!new_cred) {
+		bpf_prog_put(prog);
+		return -ENOMEM;
+	}
+	new_llcred = landlock_cred(new_cred);
+	/* the new creds are an atomic copy of the current creds */
+	new_dom = landlock_prepend_prog(new_llcred->domain, prog);
+	bpf_prog_put(prog);
+	if (IS_ERR(new_dom)) {
+		abort_creds(new_cred);
+		return PTR_ERR(new_dom);
+	}
+	/* replace the old (prepared) domain */
+	landlock_put_domain(new_llcred->domain);
+	new_llcred->domain = new_dom;
+	return commit_creds(new_cred);
+}
+
+#endif /* CONFIG_SECCOMP_FILTER */
diff --git a/security/landlock/hooks_cred.c b/security/landlock/hooks_cred.c
new file mode 100644
index 000000000000..def8678019a0
--- /dev/null
+++ b/security/landlock/hooks_cred.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - credential hooks
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <linux/cred.h>
+#include <linux/lsm_hooks.h>
+
+#include "common.h"
+#include "domain_manage.h"
+#include "hooks_cred.h"
+
+static int hook_cred_prepare(struct cred *new, const struct cred *old,
+		gfp_t gfp)
+{
+	const struct landlock_cred_security *cred_old = landlock_cred(old);
+	struct landlock_cred_security *cred_new = landlock_cred(new);
+	struct landlock_domain *dom_old;
+
+	dom_old = cred_old->domain;
+	if (dom_old) {
+		landlock_get_domain(dom_old);
+		cred_new->domain = dom_old;
+	} else {
+		cred_new->domain = NULL;
+	}
+	return 0;
+}
+
+static void hook_cred_free(struct cred *cred)
+{
+	landlock_put_domain(landlock_cred(cred)->domain);
+}
+
+static struct security_hook_list landlock_hooks[] = {
+	LSM_HOOK_INIT(cred_prepare, hook_cred_prepare),
+	LSM_HOOK_INIT(cred_free, hook_cred_free),
+};
+
+__init void landlock_add_hooks_cred(void)
+{
+	security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
+			LANDLOCK_NAME);
+}
diff --git a/security/landlock/hooks_cred.h b/security/landlock/hooks_cred.h
new file mode 100644
index 000000000000..641d66f6bf9a
--- /dev/null
+++ b/security/landlock/hooks_cred.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - credential hooks headers
+ *
+ * Copyright © 2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2019 ANSSI
+ */
+
+#ifndef _SECURITY_LANDLOCK_HOOKS_CRED_H
+#define _SECURITY_LANDLOCK_HOOKS_CRED_H
+
+__init void landlock_add_hooks_cred(void);
+
+#endif /* _SECURITY_LANDLOCK_HOOKS_CRED_H */
diff --git a/security/landlock/init.c b/security/landlock/init.c
new file mode 100644
index 000000000000..8836ec4defd3
--- /dev/null
+++ b/security/landlock/init.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - initialization
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <linux/lsm_hooks.h>
+
+#include "common.h"
+#include "hooks_cred.h"
+
+static int __init landlock_init(void)
+{
+	pr_info(LANDLOCK_NAME ": Registering hooks\n");
+	landlock_add_hooks_cred();
+	return 0;
+}
+
+struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {
+	.lbs_cred = sizeof(struct landlock_cred_security),
+};
+
+DEFINE_LSM(LANDLOCK_NAME) = {
+	.name = LANDLOCK_NAME,
+	.order = LSM_ORDER_LAST,
+	.blobs = &landlock_blob_sizes,
+	.init = landlock_init,
+};
diff --git a/security/security.c b/security/security.c
index 1bc000f834e2..03c7dce9e014 100644
--- a/security/security.c
+++ b/security/security.c
@@ -264,6 +264,21 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
 		}
 	}
 
+	/*
+	 * In case of an unprivileged access-control, we don't want to give the
+	 * ability to any process to do some checks (e.g. through an eBPF
+	 * program) on kernel objects (e.g. files) if a privileged security
+	 * policy forbid their access.  We must then load
+	 * potentially-unprivileged security modules after all other LSMs.
+	 *
+	 * LSM_ORDER_LAST is always last and does not appear in the modifiable
+	 * ordered list of enabled LSMs.
+	 */
+	for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+		if (lsm->order == LSM_ORDER_LAST)
+			append_ordered_lsm(lsm, "last");
+	}
+
 	/* Disable all LSMs not in the ordered list. */
 	for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
 		if (exists_ordered_lsm(lsm))
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-04 17:21 [PATCH bpf-next v13 0/7] Landlock LSM Mickaël Salaün
                   ` (2 preceding siblings ...)
  2019-11-04 17:21 ` [PATCH bpf-next v13 3/7] landlock,seccomp: Apply Landlock programs to process hierarchy Mickaël Salaün
@ 2019-11-04 17:21 ` Mickaël Salaün
  2019-11-05 17:18   ` Alexei Starovoitov
  2019-11-04 17:21 ` [PATCH bpf-next v13 5/7] bpf,landlock: Add task_landlock_ptrace_ancestor() helper Mickaël Salaün
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-04 17:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

Add a first Landlock hook that can be used to enforce a security policy
or to audit some process activities.  For a sandboxing use-case, it is
needed to inform the kernel if a task can legitimately debug another.
ptrace(2) can also be used by an attacker to impersonate another task
and remain undetected while performing malicious activities.

Using ptrace(2) and related features on a target process can lead to a
privilege escalation.  A sandboxed task must then be able to tell the
kernel if another task is more privileged, via ptrace_may_access().

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: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Will Drewry <wad@chromium.org>
---

Changes since v12:
* ensure preemption is disabled before running BPF programs, cf. commit
  568f196756ad ("bpf: check that BPF programs run with preemption
  disabled")

Changes since v10:
* revamp and replace the static policy with a Landlock hook which may be
  used by the corresponding BPF_LANDLOCK_PTRACE program (attach) type
  and a dedicated process_cmp_landlock_ptrace() BPF helper
* check prog return value against LANDLOCK_RET_DENY (ret is a bitmask)

Changes since v6:
* factor out ptrace check
* constify pointers
* cleanup headers
* use the new security_add_hooks()
---
 security/landlock/Makefile       |   4 +-
 security/landlock/bpf_run.c      |  65 ++++++++++++++++++
 security/landlock/bpf_run.h      |  25 +++++++
 security/landlock/hooks_ptrace.c | 114 +++++++++++++++++++++++++++++++
 security/landlock/hooks_ptrace.h |  19 ++++++
 security/landlock/init.c         |   2 +
 6 files changed, 227 insertions(+), 2 deletions(-)
 create mode 100644 security/landlock/bpf_run.c
 create mode 100644 security/landlock/bpf_run.h
 create mode 100644 security/landlock/hooks_ptrace.c
 create mode 100644 security/landlock/hooks_ptrace.h

diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index 0b291f2c027c..93e4c2f31c8a 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -1,6 +1,6 @@
 obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
 
 landlock-y := init.o \
-	bpf_verify.o bpf_ptrace.o \
+	bpf_verify.o bpf_run.o bpf_ptrace.o \
 	domain_manage.o domain_syscall.o \
-	hooks_cred.o
+	hooks_cred.o hooks_ptrace.o
diff --git a/security/landlock/bpf_run.c b/security/landlock/bpf_run.c
new file mode 100644
index 000000000000..454cd4b6e99b
--- /dev/null
+++ b/security/landlock/bpf_run.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - eBPF program evaluation
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <asm/current.h>
+#include <linux/bpf.h>
+#include <linux/errno.h>
+#include <linux/filter.h>
+#include <linux/preempt.h>
+#include <linux/rculist.h>
+#include <uapi/linux/landlock.h>
+
+#include "bpf_run.h"
+#include "common.h"
+#include "hooks_ptrace.h"
+
+static const void *get_prog_ctx(struct landlock_hook_ctx *hook_ctx)
+{
+	switch (hook_ctx->type) {
+	case LANDLOCK_HOOK_PTRACE:
+		return landlock_get_ctx_ptrace(hook_ctx->ctx_ptrace);
+	}
+	WARN_ON(1);
+	return NULL;
+}
+
+/**
+ * landlock_access_denied - run Landlock programs tied to a hook
+ *
+ * @domain: Landlock domain pointer
+ * @hook_ctx: non-NULL valid eBPF context pointer
+ *
+ * Return true if at least one program return deny, false otherwise.
+ */
+bool landlock_access_denied(struct landlock_domain *domain,
+		struct landlock_hook_ctx *hook_ctx)
+{
+	struct landlock_prog_list *prog_list;
+	const size_t hook = get_hook_index(hook_ctx->type);
+
+	if (!domain)
+		return false;
+
+	for (prog_list = domain->programs[hook]; prog_list;
+			prog_list = prog_list->prev) {
+		u32 ret;
+		const void *prog_ctx;
+
+		prog_ctx = get_prog_ctx(hook_ctx);
+		if (!prog_ctx || WARN_ON(IS_ERR(prog_ctx)))
+			return true;
+		preempt_disable();
+		rcu_read_lock();
+		ret = BPF_PROG_RUN(prog_list->prog, prog_ctx);
+		rcu_read_unlock();
+		preempt_enable();
+		if (ret & LANDLOCK_RET_DENY)
+			return true;
+	}
+	return false;
+}
diff --git a/security/landlock/bpf_run.h b/security/landlock/bpf_run.h
new file mode 100644
index 000000000000..3461cbb8ec12
--- /dev/null
+++ b/security/landlock/bpf_run.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - eBPF program evaluation headers
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#ifndef _SECURITY_LANDLOCK_BPF_RUN_H
+#define _SECURITY_LANDLOCK_BPF_RUN_H
+
+#include "common.h"
+#include "hooks_ptrace.h"
+
+struct landlock_hook_ctx {
+	enum landlock_hook_type type;
+	union {
+		struct landlock_hook_ctx_ptrace *ctx_ptrace;
+	};
+};
+
+bool landlock_access_denied(struct landlock_domain *domain,
+		struct landlock_hook_ctx *hook_ctx);
+
+#endif /* _SECURITY_LANDLOCK_BPF_RUN_H */
diff --git a/security/landlock/hooks_ptrace.c b/security/landlock/hooks_ptrace.c
new file mode 100644
index 000000000000..8e518a472d04
--- /dev/null
+++ b/security/landlock/hooks_ptrace.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - ptrace hooks
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2019 ANSSI
+ */
+
+#include <asm/current.h>
+#include <linux/cred.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/lsm_hooks.h>
+#include <linux/sched.h>
+#include <uapi/linux/landlock.h>
+
+#include "bpf_run.h"
+#include "common.h"
+#include "hooks_ptrace.h"
+
+struct landlock_hook_ctx_ptrace {
+	struct landlock_context_ptrace prog_ctx;
+};
+
+const struct landlock_context_ptrace *landlock_get_ctx_ptrace(
+		const struct landlock_hook_ctx_ptrace *hook_ctx)
+{
+	if (WARN_ON(!hook_ctx))
+		return NULL;
+
+	return &hook_ctx->prog_ctx;
+}
+
+static int check_ptrace(struct landlock_domain *domain,
+		struct task_struct *tracer, struct task_struct *tracee)
+{
+	struct landlock_hook_ctx_ptrace ctx_ptrace = {
+		.prog_ctx = {
+			.tracer = (uintptr_t)tracer,
+			.tracee = (uintptr_t)tracee,
+		},
+	};
+	struct landlock_hook_ctx hook_ctx = {
+		.type = LANDLOCK_HOOK_PTRACE,
+		.ctx_ptrace = &ctx_ptrace,
+	};
+
+	return landlock_access_denied(domain, &hook_ctx) ? -EPERM : 0;
+}
+
+/**
+ * hook_ptrace_access_check - determine whether the current process may access
+ *			      another
+ *
+ * @child: the process to be accessed
+ * @mode: the mode of attachment
+ *
+ * If the current task (i.e. tracer) has one or multiple BPF_LANDLOCK_PTRACE
+ * programs, then run them with the `struct landlock_context_ptrace` context.
+ * If one of these programs return LANDLOCK_RET_DENY, then deny access with
+ * -EPERM, else allow it by returning 0.
+ */
+static int hook_ptrace_access_check(struct task_struct *child,
+		unsigned int mode)
+{
+	struct landlock_domain *dom_current;
+	const size_t hook = get_hook_index(LANDLOCK_HOOK_PTRACE);
+
+	dom_current = landlock_cred(current_cred())->domain;
+	if (!(dom_current && dom_current->programs[hook]))
+		return 0;
+	return check_ptrace(dom_current, current, child);
+}
+
+/**
+ * hook_ptrace_traceme - determine whether another process may trace the
+ *			 current one
+ *
+ * @parent: the task proposed to be the tracer
+ *
+ * If the parent task (i.e. tracer) has one or multiple BPF_LANDLOCK_PTRACE
+ * programs, then run them with the `struct landlock_context_ptrace` context.
+ * If one of these programs return LANDLOCK_RET_DENY, then deny access with
+ * -EPERM, else allow it by returning 0.
+ */
+static int hook_ptrace_traceme(struct task_struct *parent)
+{
+	struct landlock_domain *dom_parent;
+	const size_t hook = get_hook_index(LANDLOCK_HOOK_PTRACE);
+	int ret;
+
+	rcu_read_lock();
+	dom_parent = landlock_cred(__task_cred(parent))->domain;
+	if (!(dom_parent && dom_parent->programs[hook])) {
+		ret = 0;
+		goto put_rcu;
+	}
+	ret = check_ptrace(dom_parent, parent, current);
+
+put_rcu:
+	rcu_read_unlock();
+	return ret;
+}
+
+static struct security_hook_list landlock_hooks[] = {
+	LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check),
+	LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme),
+};
+
+__init void landlock_add_hooks_ptrace(void)
+{
+	security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
+			LANDLOCK_NAME);
+}
diff --git a/security/landlock/hooks_ptrace.h b/security/landlock/hooks_ptrace.h
new file mode 100644
index 000000000000..53fe651bdb3e
--- /dev/null
+++ b/security/landlock/hooks_ptrace.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - ptrace hooks headers
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2019 ANSSI
+ */
+
+#ifndef _SECURITY_LANDLOCK_HOOKS_PTRACE_H
+#define _SECURITY_LANDLOCK_HOOKS_PTRACE_H
+
+struct landlock_hook_ctx_ptrace;
+
+const struct landlock_context_ptrace *landlock_get_ctx_ptrace(
+		const struct landlock_hook_ctx_ptrace *hook_ctx);
+
+__init void landlock_add_hooks_ptrace(void);
+
+#endif /* _SECURITY_LANDLOCK_HOOKS_PTRACE_H */
diff --git a/security/landlock/init.c b/security/landlock/init.c
index 8836ec4defd3..541aad17418e 100644
--- a/security/landlock/init.c
+++ b/security/landlock/init.c
@@ -10,11 +10,13 @@
 
 #include "common.h"
 #include "hooks_cred.h"
+#include "hooks_ptrace.h"
 
 static int __init landlock_init(void)
 {
 	pr_info(LANDLOCK_NAME ": Registering hooks\n");
 	landlock_add_hooks_cred();
+	landlock_add_hooks_ptrace();
 	return 0;
 }
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v13 5/7] bpf,landlock: Add task_landlock_ptrace_ancestor() helper
  2019-11-04 17:21 [PATCH bpf-next v13 0/7] Landlock LSM Mickaël Salaün
                   ` (3 preceding siblings ...)
  2019-11-04 17:21 ` [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks Mickaël Salaün
@ 2019-11-04 17:21 ` Mickaël Salaün
  2019-11-04 17:21 ` [PATCH bpf-next v13 6/7] bpf,landlock: Add tests for the Landlock ptrace program type Mickaël Salaün
  2019-11-04 17:21 ` [PATCH bpf-next v13 7/7] landlock: Add user and kernel documentation for Landlock Mickaël Salaün
  6 siblings, 0 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-04 17:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

This new task_landlock_ptrace_ancestor() helper can be used to identify
if the Landlock domain tied to the current tracer is in the same
hierarchy as the domain of tracee.  This may be a way for user space to
programmatically defines that a set of tasks is less privileged than
another set of tasks.

Indeed, ptrace(2) can be used to impersonate an unsandboxed process and
lead to a privilege escalation.  A common use-case when sandboxing a
process is then to forbid it to debug a less-privileged process.  A
sandbox process (tracer) should only be allowed to trace another process
(tracee) if the tracee has fewer privileges than the tracer.  This
policy can be implemented with this helper.

More complex helpers could be added in the future to enable other ways
to check the relation between the tracer and the tracee (e.g. check
other program types or other hierarchies) if there is a use case.

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: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Will Drewry <wad@chromium.org>
---

Changes since v10:
* new patch taking inspiration from the previous static ptrace policy
---
 include/linux/bpf.h            |  2 +
 include/uapi/linux/bpf.h       | 21 ++++++++++-
 kernel/bpf/verifier.c          |  4 ++
 security/landlock/bpf_ptrace.c | 68 ++++++++++++++++++++++++++++++++++
 security/landlock/bpf_verify.c |  4 ++
 5 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 819a3e207438..67ec198a90cb 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -214,6 +214,7 @@ enum bpf_arg_type {
 	ARG_PTR_TO_LONG,	/* pointer to long */
 	ARG_PTR_TO_SOCKET,	/* pointer to bpf_sock (fullsock) */
 	ARG_PTR_TO_BTF_ID,	/* pointer to in-kernel struct */
+	ARG_PTR_TO_TASK,	/* pointer to task_struct */
 };
 
 /* type of values returned from helper functions */
@@ -1088,6 +1089,7 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto;
 extern const struct bpf_func_proto bpf_strtol_proto;
 extern const struct bpf_func_proto bpf_strtoul_proto;
 extern const struct bpf_func_proto bpf_tcp_sock_proto;
+extern const struct bpf_func_proto bpf_task_landlock_ptrace_ancestor_proto;
 
 /* Shared helpers among cBPF and eBPF. */
 void bpf_user_rnd_init_once(void);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 6e4147790f96..c88436b97163 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2777,6 +2777,24 @@ union bpf_attr {
  * 		restricted to raw_tracepoint bpf programs.
  * 	Return
  * 		0 on success, or a negative error in case of failure.
+ *
+ * int bpf_task_landlock_ptrace_ancestor(struct task_struct *parent, struct task_struct *child)
+ *	Description
+ *		Check the relation of a potentially parent task with a child
+ *		one, according to their Landlock ptrace hook programs.
+ *	Return
+ *		**-EINVAL** if the child's ptrace programs are not comparable
+ *		to the parent ones, i.e. one of them is an empty set.
+ *
+ *		**-ENOENT** if the parent's ptrace programs are either in a
+ *		separate hierarchy of the child ones, or if the parent's ptrace
+ *		programs are a superset of the child ones.
+ *
+ *		0 if the parent's ptrace programs are the same as the child
+ *		ones.
+ *
+ *		1 if the parent's ptrace programs are indeed a subset of the
+ *		child ones.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2890,7 +2908,8 @@ union bpf_attr {
 	FN(sk_storage_delete),		\
 	FN(send_signal),		\
 	FN(tcp_gen_syncookie),		\
-	FN(skb_output),
+	FN(skb_output),			\
+	FN(task_landlock_ptrace_ancestor),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ebf1991906b7..af8f1a777a2d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3492,6 +3492,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
 		    type != PTR_TO_MAP_VALUE &&
 		    type != expected_type)
 			goto err_type;
+	} else if (arg_type == ARG_PTR_TO_TASK) {
+		expected_type = PTR_TO_TASK;
+		if (type != expected_type)
+			goto err_type;
 	} else {
 		verbose(env, "unsupported arg_type %d\n", arg_type);
 		return -EFAULT;
diff --git a/security/landlock/bpf_ptrace.c b/security/landlock/bpf_ptrace.c
index 2ec73078ad01..0e1362951463 100644
--- a/security/landlock/bpf_ptrace.c
+++ b/security/landlock/bpf_ptrace.c
@@ -7,9 +7,13 @@
  */
 
 #include <linux/bpf.h>
+#include <linux/cred.h>
+#include <linux/kernel.h>
+#include <linux/rcupdate.h>
 #include <uapi/linux/landlock.h>
 
 #include "bpf_ptrace.h"
+#include "common.h"
 
 bool landlock_is_valid_access_ptrace(int off, enum bpf_access_type type,
 		enum bpf_reg_type *reg_type, int *max_size)
@@ -28,3 +32,67 @@ bool landlock_is_valid_access_ptrace(int off, enum bpf_access_type type,
 		return false;
 	}
 }
+
+/**
+ * domain_ptrace_ancestor - check domain ordering according to ptrace
+ *
+ * @parent: a parent domain
+ * @child: a potential child of @parent
+ *
+ * Check if the @parent domain is less or equal to (i.e. a subset of) the
+ * @child domain.
+ */
+static int domain_ptrace_ancestor(const struct landlock_domain *parent,
+		const struct landlock_domain *child)
+{
+	const struct landlock_prog_list *child_progs, *parent_progs;
+	const size_t hook = get_hook_index(LANDLOCK_HOOK_PTRACE);
+
+	if (!parent || !child)
+		/* @parent or @child has no ptrace restriction */
+		return -EINVAL;
+	parent_progs = parent->programs[hook];
+	child_progs = child->programs[hook];
+	if (!parent_progs || !child_progs)
+		/* @parent or @child has no ptrace restriction */
+		return -EINVAL;
+	if (child_progs == parent_progs)
+		/* @parent is at the same level as @child */
+		return 0;
+	for (child_progs = child_progs->prev; child_progs;
+			child_progs = child_progs->prev) {
+		if (child_progs == parent_progs)
+			/* @parent is one of the ancestors of @child */
+			return 1;
+	}
+	/*
+	 * Either there is no relationship between @parent and @child, or
+	 * @child is one of the ancestors of @parent.
+	 */
+	return -ENOENT;
+}
+
+/*
+ * Cf. include/uapi/linux/bpf.h - bpf_task_landlock_ptrace_ancestor
+ */
+BPF_CALL_2(bpf_task_landlock_ptrace_ancestor, const struct task_struct *,
+		parent, const struct task_struct *, child)
+{
+	const struct landlock_domain *dom_parent, *dom_child;
+
+	WARN_ON_ONCE(!rcu_read_lock_held());
+	if (WARN_ON(!parent || !child))
+		return -EFAULT;
+	dom_parent = landlock_cred(__task_cred(parent))->domain;
+	dom_child = landlock_cred(__task_cred(child))->domain;
+	return domain_ptrace_ancestor(dom_parent, dom_child);
+}
+
+const struct bpf_func_proto bpf_task_landlock_ptrace_ancestor_proto = {
+	.func		= bpf_task_landlock_ptrace_ancestor,
+	.gpl_only	= false,
+	.pkt_access	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_TASK,
+	.arg2_type	= ARG_PTR_TO_TASK,
+};
diff --git a/security/landlock/bpf_verify.c b/security/landlock/bpf_verify.c
index 6ed921588178..a1d2db75d51d 100644
--- a/security/landlock/bpf_verify.c
+++ b/security/landlock/bpf_verify.c
@@ -70,6 +70,10 @@ static const struct bpf_func_proto *bpf_landlock_func_proto(
 		return &bpf_map_update_elem_proto;
 	case BPF_FUNC_map_delete_elem:
 		return &bpf_map_delete_elem_proto;
+	case BPF_FUNC_task_landlock_ptrace_ancestor:
+		if (get_hook_type(prog) == LANDLOCK_HOOK_PTRACE)
+			return &bpf_task_landlock_ptrace_ancestor_proto;
+		return NULL;
 	default:
 		return NULL;
 	}
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v13 6/7] bpf,landlock: Add tests for the Landlock ptrace program type
  2019-11-04 17:21 [PATCH bpf-next v13 0/7] Landlock LSM Mickaël Salaün
                   ` (4 preceding siblings ...)
  2019-11-04 17:21 ` [PATCH bpf-next v13 5/7] bpf,landlock: Add task_landlock_ptrace_ancestor() helper Mickaël Salaün
@ 2019-11-04 17:21 ` Mickaël Salaün
  2019-11-04 17:21 ` [PATCH bpf-next v13 7/7] landlock: Add user and kernel documentation for Landlock Mickaël Salaün
  6 siblings, 0 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-04 17:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

Test eBPF program context access and ptrace hooks semantic.

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: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Will Drewry <wad@chromium.org>
---

Changes since v12:
* add documentation and diagrams (by Vincent Dagonneau)

Changes since v11:
* cosmetic fixes

Changes since v10:
* rework tests with new Landlock ptrace programs which restrict ptrace
  thanks to the task_landlock_ptrace_ancestor() helper
* simplify ptrace tests (make expect_ptrace implicit)
* add tests:
  * check a child process tracing its parent
  * check Landlock domain without ptrace enforcement (e.g. useful for
    audit/signaling purpose)
  * check inherited-only domains
  * check task pointer arithmetic
* fix flaky test for multi-core
* increase log size
* cosmetic renames
* update and improve the Makefile

Changes since v9:
* replace subtype with expected_attach_type and expected_attach_triggers
* rename inode_map_lookup() into inode_map_lookup_elem()
* check for inode map entry without value (which is now possible thanks
  to the pointer null check)
* use read-only inode map for Landlock programs

Changes since v8:
* update eBPF include path for macros
* use TEST_GEN_PROGS and use the generic "clean" target
* add more verbose errors
* update the bpf/verifier files
* remove chain tests (from landlock and bpf/verifier)
* replace the whitelist tests with blacklist tests (because of stateless
  Landlock programs): remove "dotdot" tests and other depth tests
* sync the landlock Makefile with its bpf sibling directory and use
  bpf_load_program_xattr()

Changes since v7:
* update tests and add new ones for filesystem hierarchy and Landlock
  chains.

Changes since v6:
* use the new kselftest_harness.h
* use const variables
* replace ASSERT_STEP with ASSERT_*
* rename BPF_PROG_TYPE_LANDLOCK to BPF_PROG_TYPE_LANDLOCK_RULE
* force sample library rebuild
* fix install target

Changes since v5:
* add subtype test
* add ptrace tests
* split and rename files
* cleanup and rebase
---
 scripts/bpf_helpers_doc.py                    |   1 +
 tools/include/uapi/linux/bpf.h                |  23 +-
 tools/include/uapi/linux/landlock.h           |  22 ++
 tools/lib/bpf/libbpf_probes.c                 |   3 +
 tools/testing/selftests/bpf/config            |   3 +
 tools/testing/selftests/bpf/test_verifier.c   |   1 +
 .../testing/selftests/bpf/verifier/landlock.c |  56 ++++
 tools/testing/selftests/landlock/.gitignore   |   5 +
 tools/testing/selftests/landlock/Makefile     |  27 ++
 tools/testing/selftests/landlock/config       |   5 +
 tools/testing/selftests/landlock/test.h       |  48 +++
 tools/testing/selftests/landlock/test_base.c  |  24 ++
 .../testing/selftests/landlock/test_ptrace.c  | 289 ++++++++++++++++++
 13 files changed, 506 insertions(+), 1 deletion(-)
 create mode 100644 tools/include/uapi/linux/landlock.h
 create mode 100644 tools/testing/selftests/bpf/verifier/landlock.c
 create mode 100644 tools/testing/selftests/landlock/.gitignore
 create mode 100644 tools/testing/selftests/landlock/Makefile
 create mode 100644 tools/testing/selftests/landlock/config
 create mode 100644 tools/testing/selftests/landlock/test.h
 create mode 100644 tools/testing/selftests/landlock/test_base.c
 create mode 100644 tools/testing/selftests/landlock/test_ptrace.c

diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
index 7548569e8076..8e4c0fe75663 100755
--- a/scripts/bpf_helpers_doc.py
+++ b/scripts/bpf_helpers_doc.py
@@ -466,6 +466,7 @@ class PrinterHelpers(Printer):
             'const struct sk_buff': 'const struct __sk_buff',
             'struct sk_msg_buff': 'struct sk_msg_md',
             'struct xdp_buff': 'struct xdp_md',
+            'struct task_struct': 'void',
     }
 
     def print_header(self):
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 4af8b0819a32..c88436b97163 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -173,6 +173,7 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_CGROUP_SYSCTL,
 	BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE,
 	BPF_PROG_TYPE_CGROUP_SOCKOPT,
+	BPF_PROG_TYPE_LANDLOCK_HOOK,
 };
 
 enum bpf_attach_type {
@@ -199,6 +200,7 @@ enum bpf_attach_type {
 	BPF_CGROUP_UDP6_RECVMSG,
 	BPF_CGROUP_GETSOCKOPT,
 	BPF_CGROUP_SETSOCKOPT,
+	BPF_LANDLOCK_PTRACE,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -2775,6 +2777,24 @@ union bpf_attr {
  * 		restricted to raw_tracepoint bpf programs.
  * 	Return
  * 		0 on success, or a negative error in case of failure.
+ *
+ * int bpf_task_landlock_ptrace_ancestor(struct task_struct *parent, struct task_struct *child)
+ *	Description
+ *		Check the relation of a potentially parent task with a child
+ *		one, according to their Landlock ptrace hook programs.
+ *	Return
+ *		**-EINVAL** if the child's ptrace programs are not comparable
+ *		to the parent ones, i.e. one of them is an empty set.
+ *
+ *		**-ENOENT** if the parent's ptrace programs are either in a
+ *		separate hierarchy of the child ones, or if the parent's ptrace
+ *		programs are a superset of the child ones.
+ *
+ *		0 if the parent's ptrace programs are the same as the child
+ *		ones.
+ *
+ *		1 if the parent's ptrace programs are indeed a subset of the
+ *		child ones.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2888,7 +2908,8 @@ union bpf_attr {
 	FN(sk_storage_delete),		\
 	FN(send_signal),		\
 	FN(tcp_gen_syncookie),		\
-	FN(skb_output),
+	FN(skb_output),			\
+	FN(task_landlock_ptrace_ancestor),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/tools/include/uapi/linux/landlock.h b/tools/include/uapi/linux/landlock.h
new file mode 100644
index 000000000000..3db2d190c4e7
--- /dev/null
+++ b/tools/include/uapi/linux/landlock.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Landlock - UAPI headers
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#ifndef _UAPI__LINUX_LANDLOCK_H__
+#define _UAPI__LINUX_LANDLOCK_H__
+
+#include <linux/types.h>
+
+#define LANDLOCK_RET_ALLOW	0
+#define LANDLOCK_RET_DENY	1
+
+struct landlock_context_ptrace {
+	__u64 tracer;
+	__u64 tracee;
+};
+
+#endif /* _UAPI__LINUX_LANDLOCK_H__ */
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index 4b0b0364f5fc..1e0d6346a7c7 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -78,6 +78,9 @@ probe_load(enum bpf_prog_type prog_type, const struct bpf_insn *insns,
 	case BPF_PROG_TYPE_KPROBE:
 		xattr.kern_version = get_kernel_version();
 		break;
+	case BPF_PROG_TYPE_LANDLOCK_HOOK:
+		xattr.expected_attach_type = BPF_LANDLOCK_PTRACE;
+		break;
 	case BPF_PROG_TYPE_UNSPEC:
 	case BPF_PROG_TYPE_SOCKET_FILTER:
 	case BPF_PROG_TYPE_SCHED_CLS:
diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 5dc109f4c097..3161a88a6059 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -35,3 +35,6 @@ CONFIG_MPLS_ROUTING=m
 CONFIG_MPLS_IPTUNNEL=m
 CONFIG_IPV6_SIT=m
 CONFIG_BPF_JIT=y
+CONFIG_SECCOMP_FILTER=y
+CONFIG_SECURITY=y
+CONFIG_SECURITY_LANDLOCK=y
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index d27fd929abb9..74f249dafc0b 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -30,6 +30,7 @@
 #include <linux/bpf.h>
 #include <linux/if_ether.h>
 #include <linux/btf.h>
+#include <linux/landlock.h>
 
 #include <bpf/bpf.h>
 #include <bpf/libbpf.h>
diff --git a/tools/testing/selftests/bpf/verifier/landlock.c b/tools/testing/selftests/bpf/verifier/landlock.c
new file mode 100644
index 000000000000..59cd333745dc
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/landlock.c
@@ -0,0 +1,56 @@
+{
+	"landlock/ptrace: always accept",
+	.prog_type = BPF_PROG_TYPE_LANDLOCK_HOOK,
+	.expected_attach_type = BPF_LANDLOCK_PTRACE,
+	.insns = {
+		BPF_MOV32_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+},
+{
+	"landlock/ptrace: forbid arbitrary return value",
+	.prog_type = BPF_PROG_TYPE_LANDLOCK_HOOK,
+	.expected_attach_type = BPF_LANDLOCK_PTRACE,
+	.insns = {
+		BPF_MOV32_IMM(BPF_REG_0, 2),
+		BPF_EXIT_INSN(),
+	},
+	.result = REJECT,
+	.errstr = "At program exit the register R0 has value (0x2; 0x0) should have been in (0x0; 0x1)",
+},
+{
+	"landlock/ptrace: read context and call dedicated helper",
+	.prog_type = BPF_PROG_TYPE_LANDLOCK_HOOK,
+	.expected_attach_type = BPF_LANDLOCK_PTRACE,
+	.insns = {
+		BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+		BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_6,
+			offsetof(struct landlock_context_ptrace, tracer)),
+		BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_6,
+			offsetof(struct landlock_context_ptrace, tracer)),
+		BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
+				BPF_FUNC_task_landlock_ptrace_ancestor),
+		BPF_MOV32_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+},
+{
+	"landlock/ptrace: forbid pointer arithmetic",
+	.prog_type = BPF_PROG_TYPE_LANDLOCK_HOOK,
+	.expected_attach_type = BPF_LANDLOCK_PTRACE,
+	.insns = {
+		BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+		BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_6,
+			offsetof(struct landlock_context_ptrace, tracer)),
+		BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
+		BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_6,
+			offsetof(struct landlock_context_ptrace, tracee)),
+		BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, 1),
+		BPF_MOV32_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+	},
+	.result = REJECT,
+	.errstr = "R1 pointer arithmetic on task prohibited",
+},
diff --git a/tools/testing/selftests/landlock/.gitignore b/tools/testing/selftests/landlock/.gitignore
new file mode 100644
index 000000000000..4c5c01d23fe0
--- /dev/null
+++ b/tools/testing/selftests/landlock/.gitignore
@@ -0,0 +1,5 @@
+/feature
+/fixdep
+/*libbpf*
+/test_base
+/test_ptrace
diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
new file mode 100644
index 000000000000..2da77c30e77f
--- /dev/null
+++ b/tools/testing/selftests/landlock/Makefile
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: GPL-2.0
+
+LIBDIR := $(abspath ../../../lib)
+BPFDIR := $(LIBDIR)/bpf
+TOOLSDIR := $(abspath ../../../include)
+APIDIR := $(TOOLSDIR)/uapi
+
+CFLAGS += -g -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(TOOLSDIR)
+LDLIBS += -lelf
+
+test_src = $(wildcard test_*.c)
+
+TEST_GEN_PROGS := $(test_src:.c=)
+
+include ../lib.mk
+
+BPFOBJ := $(OUTPUT)/libbpf.a
+
+$(TEST_GEN_PROGS): $(BPFOBJ) ../kselftest_harness.h
+
+.PHONY: force
+
+# force a rebuild of BPFOBJ when its dependencies are updated
+force:
+
+$(BPFOBJ): force
+	$(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/
diff --git a/tools/testing/selftests/landlock/config b/tools/testing/selftests/landlock/config
new file mode 100644
index 000000000000..fa5081b840ad
--- /dev/null
+++ b/tools/testing/selftests/landlock/config
@@ -0,0 +1,5 @@
+CONFIG_BPF=y
+CONFIG_BPF_SYSCALL=y
+CONFIG_SECCOMP_FILTER=y
+CONFIG_SECURITY=y
+CONFIG_SECURITY_LANDLOCK=y
diff --git a/tools/testing/selftests/landlock/test.h b/tools/testing/selftests/landlock/test.h
new file mode 100644
index 000000000000..836df68b6bb8
--- /dev/null
+++ b/tools/testing/selftests/landlock/test.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Landlock helpers
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2019 ANSSI
+ */
+
+#include <bpf/bpf.h>
+#include <errno.h>
+#include <linux/filter.h>
+#include <linux/landlock.h>
+#include <linux/seccomp.h>
+#include <sys/prctl.h>
+#include <sys/syscall.h>
+
+#include "../kselftest_harness.h"
+#include "../../../../samples/bpf/bpf_load.h"
+
+#ifndef SECCOMP_PREPEND_LANDLOCK_PROG
+#define SECCOMP_PREPEND_LANDLOCK_PROG	4
+#endif
+
+#ifndef seccomp
+static int __attribute__((unused)) seccomp(unsigned int op, unsigned int flags,
+		void *args)
+{
+	errno = 0;
+	return syscall(__NR_seccomp, op, flags, args);
+}
+#endif
+
+static int __attribute__((unused)) ll_bpf_load_program(
+		const struct bpf_insn *bpf_insns, size_t insns_len,
+		char *log_buf, size_t log_buf_sz,
+		const enum bpf_attach_type attach_type)
+{
+	struct bpf_load_program_attr load_attr;
+
+	memset(&load_attr, 0, sizeof(struct bpf_load_program_attr));
+	load_attr.prog_type = BPF_PROG_TYPE_LANDLOCK_HOOK;
+	load_attr.expected_attach_type = attach_type;
+	load_attr.insns = bpf_insns;
+	load_attr.insns_cnt = insns_len / sizeof(struct bpf_insn);
+	load_attr.license = "GPL";
+
+	return bpf_load_program_xattr(&load_attr, log_buf, log_buf_sz);
+}
diff --git a/tools/testing/selftests/landlock/test_base.c b/tools/testing/selftests/landlock/test_base.c
new file mode 100644
index 000000000000..db46f39048cb
--- /dev/null
+++ b/tools/testing/selftests/landlock/test_base.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Landlock tests - base
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+
+#include "test.h"
+
+TEST(seccomp_landlock)
+{
+	int ret;
+
+	ret = seccomp(SECCOMP_PREPEND_LANDLOCK_PROG, 0, NULL);
+	EXPECT_EQ(-1, ret);
+	EXPECT_EQ(EFAULT, errno) {
+		TH_LOG("Kernel does not support CONFIG_SECURITY_LANDLOCK");
+	}
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/landlock/test_ptrace.c b/tools/testing/selftests/landlock/test_ptrace.c
new file mode 100644
index 000000000000..7f4945a61758
--- /dev/null
+++ b/tools/testing/selftests/landlock/test_ptrace.c
@@ -0,0 +1,289 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Landlock tests - ptrace
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2019 ANSSI
+ */
+
+#define _GNU_SOURCE
+#include <signal.h>
+#include <sys/ptrace.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "test.h"
+
+#define LOG_SIZE 512
+
+static void create_domain(struct __test_metadata *_metadata,
+		bool scoped_ptrace, bool inherited_only)
+{
+	const struct bpf_insn prog_void[] = {
+		BPF_MOV32_IMM(BPF_REG_0, LANDLOCK_RET_ALLOW),
+		BPF_EXIT_INSN(),
+	};
+	const struct bpf_insn prog_check[] = {
+		BPF_ALU64_REG(BPF_MOV, BPF_REG_6, BPF_REG_1),
+		BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_6,
+			offsetof(struct landlock_context_ptrace, tracer)),
+		BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_6,
+			offsetof(struct landlock_context_ptrace, tracee)),
+		BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
+				BPF_FUNC_task_landlock_ptrace_ancestor),
+		/*
+		 * If @tracee is an ancestor or at the same level of @tracer,
+		 * then allow ptrace (warning: do not use BPF_JGE 0).
+		 */
+		BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, inherited_only ? 0 : 1, 2),
+		BPF_MOV32_IMM(BPF_REG_0, LANDLOCK_RET_DENY),
+		BPF_EXIT_INSN(),
+		BPF_MOV32_IMM(BPF_REG_0, LANDLOCK_RET_ALLOW),
+		BPF_EXIT_INSN(),
+	};
+	int prog;
+	char log[LOG_SIZE] = "";
+
+	if (scoped_ptrace)
+		prog = ll_bpf_load_program(prog_check, sizeof(prog_check),
+				log, sizeof(log), BPF_LANDLOCK_PTRACE);
+	else
+		prog = ll_bpf_load_program(prog_void, sizeof(prog_void),
+				log, sizeof(log), BPF_LANDLOCK_PTRACE);
+	ASSERT_NE(-1, prog) {
+		TH_LOG("Failed to load the %s program: %s\n%s",
+				scoped_ptrace ? "check" : "void",
+				strerror(errno), log);
+	}
+	ASSERT_EQ(0, seccomp(SECCOMP_PREPEND_LANDLOCK_PROG, 0, &prog)) {
+		TH_LOG("Failed to create a Landlock domain: %s",
+				strerror(errno));
+	}
+	EXPECT_EQ(0, close(prog));
+}
+
+/* test PTRACE_TRACEME and PTRACE_ATTACH for parent and child */
+static void _check_ptrace(struct __test_metadata *_metadata,
+		bool scoped_ptrace, bool domain_both,
+		bool domain_parent, bool domain_child)
+{
+	pid_t child, parent;
+	int status;
+	int pipe_child[2], pipe_parent[2];
+	char buf_parent;
+	const bool inherited_only = domain_both && !domain_parent &&
+		!domain_child;
+
+	parent = getpid();
+
+	ASSERT_EQ(0, pipe(pipe_child));
+	ASSERT_EQ(0, pipe(pipe_parent));
+	if (domain_both)
+		create_domain(_metadata, scoped_ptrace, inherited_only);
+
+	child = fork();
+	ASSERT_LE(0, child);
+	if (child == 0) {
+		char buf_child;
+
+		EXPECT_EQ(0, close(pipe_parent[1]));
+		EXPECT_EQ(0, close(pipe_child[0]));
+		if (domain_child)
+			create_domain(_metadata, scoped_ptrace, inherited_only);
+
+		/* sync #1 */
+		ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1)) {
+			TH_LOG("Failed to read() sync #1 from parent");
+		}
+		ASSERT_EQ('.', buf_child);
+
+		/* test the parent protection */
+		ASSERT_EQ((domain_child && scoped_ptrace) ? -1 : 0,
+				ptrace(PTRACE_ATTACH, parent, NULL, 0));
+		if (domain_child && scoped_ptrace) {
+			ASSERT_EQ(EPERM, errno);
+		} else {
+			ASSERT_EQ(parent, waitpid(parent, &status, 0));
+			ASSERT_EQ(1, WIFSTOPPED(status));
+			ASSERT_EQ(0, ptrace(PTRACE_DETACH, parent, NULL, 0));
+		}
+
+		/* sync #2 */
+		ASSERT_EQ(1, write(pipe_child[1], ".", 1)) {
+			TH_LOG("Failed to write() sync #2 to parent");
+		}
+
+		/* test traceme */
+		ASSERT_EQ((domain_parent && scoped_ptrace) ? -1 : 0,
+				ptrace(PTRACE_TRACEME));
+		if (domain_parent && scoped_ptrace) {
+			ASSERT_EQ(EPERM, errno);
+		} else {
+			ASSERT_EQ(0, raise(SIGSTOP));
+		}
+
+		/* sync #3 */
+		ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1)) {
+			TH_LOG("Failed to read() sync #3 from parent");
+		}
+		ASSERT_EQ('.', buf_child);
+		_exit(_metadata->passed ? EXIT_SUCCESS : EXIT_FAILURE);
+	}
+
+	EXPECT_EQ(0, close(pipe_child[1]));
+	EXPECT_EQ(0, close(pipe_parent[0]));
+	if (domain_parent)
+		create_domain(_metadata, scoped_ptrace, inherited_only);
+
+	/* sync #1 */
+	ASSERT_EQ(1, write(pipe_parent[1], ".", 1)) {
+		TH_LOG("Failed to write() sync #1 to child");
+	}
+
+	/* test the parent protection */
+	/* sync #2 */
+	ASSERT_EQ(1, read(pipe_child[0], &buf_parent, 1)) {
+		TH_LOG("Failed to read() sync #2 from child");
+	}
+	ASSERT_EQ('.', buf_parent);
+
+	/* test traceme */
+	if (!(domain_parent && scoped_ptrace)) {
+		ASSERT_EQ(child, waitpid(child, &status, 0));
+		ASSERT_EQ(1, WIFSTOPPED(status));
+		ASSERT_EQ(0, ptrace(PTRACE_DETACH, child, NULL, 0));
+	}
+	/* test attach */
+	ASSERT_EQ((domain_parent && scoped_ptrace) ? -1 : 0,
+			ptrace(PTRACE_ATTACH, child, NULL, 0));
+	if (domain_parent && scoped_ptrace) {
+		ASSERT_EQ(EPERM, errno);
+	} else {
+		ASSERT_EQ(child, waitpid(child, &status, 0));
+		ASSERT_EQ(1, WIFSTOPPED(status));
+		ASSERT_EQ(0, ptrace(PTRACE_DETACH, child, NULL, 0));
+	}
+
+	/* sync #3 */
+	ASSERT_EQ(1, write(pipe_parent[1], ".", 1)) {
+		TH_LOG("Failed to write() sync #3 to child");
+	}
+	ASSERT_EQ(child, waitpid(child, &status, 0));
+	if (WIFSIGNALED(status) || WEXITSTATUS(status))
+		_metadata->passed = 0;
+}
+
+/* keep the *_scoped order to check program inheritance */
+#define CHECK_PTRACE(name, domain_both, domain_parent, domain_child) \
+	TEST(name ## _unscoped) { \
+		_check_ptrace(_metadata, false, domain_both, domain_parent, \
+				domain_child); \
+	} \
+	TEST(name ## _scoped) { \
+		_check_ptrace(_metadata, false, domain_both, domain_parent, \
+				domain_child); \
+		_check_ptrace(_metadata, true, domain_both, domain_parent, \
+				domain_child); \
+	}
+
+/*
+ * Test multiple tracing combinations between a parent process P1 and a child
+ * process P2.
+ *
+ * Yama's scoped ptrace is presumed disabled.  If enabled, this additional
+ * restriction is enforced before any Landlock check, which means that all P2
+ * requests to trace P1 would be denied.
+ */
+
+/*
+ *        No domain
+ *
+ *   P1-.               P1 -> P2 : allow
+ *       \              P2 -> P1 : allow
+ *        'P2
+ */
+CHECK_PTRACE(allow_without_domain, false, false, false);
+
+/*
+ *        Child domain
+ *
+ *   P1--.              P1 -> P2 : allow
+ *        \             P2 -> P1 : deny
+ *        .'-----.
+ *        |  P2  |
+ *        '------'
+ */
+CHECK_PTRACE(allow_with_one_domain, false, false, true);
+
+/*
+ *        Parent domain
+ * .------.
+ * |  P1  --.           P1 -> P2 : deny
+ * '------'  \          P2 -> P1 : allow
+ *            '
+ *            P2
+ */
+CHECK_PTRACE(deny_with_parent_domain, false, true, false);
+
+/*
+ *        Parent + child domain (siblings)
+ * .------.
+ * |  P1  ---.          P1 -> P2 : deny
+ * '------'   \         P2 -> P1 : deny
+ *         .---'--.
+ *         |  P2  |
+ *         '------'
+ */
+CHECK_PTRACE(deny_with_sibling_domain, false, true, true);
+
+/*
+ *         Same domain (inherited)
+ * .-------------.
+ * | P1----.     |      P1 -> P2 : allow
+ * |        \    |      P2 -> P1 : allow
+ * |         '   |
+ * |         P2  |
+ * '-------------'
+ */
+CHECK_PTRACE(allow_sibling_domain, true, false, false);
+
+/*
+ *         Inherited + child domain
+ * .-----------------.
+ * |  P1----.        |  P1 -> P2 : allow
+ * |         \       |  P2 -> P1 : deny
+ * |        .-'----. |
+ * |        |  P2  | |
+ * |        '------' |
+ * '-----------------'
+ */
+CHECK_PTRACE(allow_with_nested_domain, true, false, true);
+
+/*
+ *         Inherited + parent domain
+ * .-----------------.
+ * |.------.         |  P1 -> P2 : deny
+ * ||  P1  ----.     |  P2 -> P1 : allow
+ * |'------'    \    |
+ * |             '   |
+ * |             P2  |
+ * '-----------------'
+ */
+CHECK_PTRACE(deny_with_nested_and_parent_domain, true, true, false);
+
+/*
+ *         Inherited + parent and child domain (siblings)
+ * .-----------------.
+ * | .------.        |  P1 -> P2 : deny
+ * | |  P1  .        |  P2 -> P1 : deny
+ * | '------'\       |
+ * |          \      |
+ * |        .--'---. |
+ * |        |  P2  | |
+ * |        '------' |
+ * '-----------------'
+ */
+CHECK_PTRACE(deny_with_forked_domain, true, true, true);
+
+TEST_HARNESS_MAIN
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH bpf-next v13 7/7] landlock: Add user and kernel documentation for Landlock
  2019-11-04 17:21 [PATCH bpf-next v13 0/7] Landlock LSM Mickaël Salaün
                   ` (5 preceding siblings ...)
  2019-11-04 17:21 ` [PATCH bpf-next v13 6/7] bpf,landlock: Add tests for the Landlock ptrace program type Mickaël Salaün
@ 2019-11-04 17:21 ` Mickaël Salaün
  6 siblings, 0 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-04 17:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mickaël Salaün, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

This documentation can be built with the Sphinx framework.

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: James Morris <jmorris@namei.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Will Drewry <wad@chromium.org>
---

Changes since v12:
* enhance the "unprivileged use" explanation
* add more explanation about the domain/credentials inheritance
* update and add self-reference Sphinx links
* more clearly explain the capability-based security principles for
  program context

Changes since v11:
* cosmetic improvements

Changes since v10:
* replace the filesystem hooks with the ptrace one
* remove the triggers
* update example
* add documenation for Landlock domains and seccomp interaction
* reference more kernel documenation (e.g. LSM hooks)

Changes since v9:
* update with expected attach type and expected attach triggers

Changes since v8:
* remove documentation related to chaining and tagging according to this
  patch series

Changes since v7:
* update documentation according to the Landlock revamp

Changes since v6:
* add a check for ctx->event
* rename BPF_PROG_TYPE_LANDLOCK to BPF_PROG_TYPE_LANDLOCK_RULE
* rename Landlock version to ABI to better reflect its purpose and add a
  dedicated changelog section
* update tables
* relax no_new_privs recommendations
* remove ABILITY_WRITE related functions
* reword rule "appending" to "prepending" and explain it
* cosmetic fixes

Changes since v5:
* update the rule hierarchy inheritance explanation
* briefly explain ctx->arg2
* add ptrace restrictions
* explain EPERM
* update example (subtype)
* use ":manpage:"
---
 Documentation/security/index.rst           |   1 +
 Documentation/security/landlock/index.rst  |  22 +++
 Documentation/security/landlock/kernel.rst | 166 +++++++++++++++++++++
 Documentation/security/landlock/user.rst   | 153 +++++++++++++++++++
 4 files changed, 342 insertions(+)
 create mode 100644 Documentation/security/landlock/index.rst
 create mode 100644 Documentation/security/landlock/kernel.rst
 create mode 100644 Documentation/security/landlock/user.rst

diff --git a/Documentation/security/index.rst b/Documentation/security/index.rst
index fc503dd689a7..4d213e76ddf4 100644
--- a/Documentation/security/index.rst
+++ b/Documentation/security/index.rst
@@ -15,3 +15,4 @@ Security Documentation
    self-protection
    siphash
    tpm/index
+   landlock/index
diff --git a/Documentation/security/landlock/index.rst b/Documentation/security/landlock/index.rst
new file mode 100644
index 000000000000..1eced757b05d
--- /dev/null
+++ b/Documentation/security/landlock/index.rst
@@ -0,0 +1,22 @@
+=========================================
+Landlock LSM: programmatic access control
+=========================================
+
+:Author: Mickaël Salaün
+
+Landlock is a stackable Linux Security Module (LSM) that makes it possible to
+create security sandboxes, programmable access-controls or safe endpoint
+security agents.  This kind of sandbox is expected to help mitigate the
+security impact of bugs or unexpected/malicious behaviors in user-space
+applications.  The current version allows only a process with the global
+CAP_SYS_ADMIN capability to create such sandboxes but the ultimate goal of
+Landlock is to empower any process, including unprivileged ones, to securely
+restrict themselves.  Landlock is inspired by seccomp-bpf but instead of
+filtering syscalls and their raw arguments, a Landlock rule can inspect the use
+of kernel objects like processes and hence make a decision according to the
+kernel semantic.
+
+.. toctree::
+
+    user
+    kernel
diff --git a/Documentation/security/landlock/kernel.rst b/Documentation/security/landlock/kernel.rst
new file mode 100644
index 000000000000..ec0109b17e6f
--- /dev/null
+++ b/Documentation/security/landlock/kernel.rst
@@ -0,0 +1,166 @@
+==============================
+Landlock: kernel documentation
+==============================
+
+eBPF properties
+===============
+
+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
+therefore expose a minimal attack surface. The eBPF bytecode is minimal,
+powerful, widely used and designed to be used by untrusted applications. Thus,
+reusing the eBPF support in the kernel enables a generic approach while
+minimizing new code.
+
+An eBPF program has access to an eBPF context containing some fields used to
+inspect the current object. These arguments may be used directly (e.g. raw
+value) or passed to helper functions according to their types (e.g. pointer).
+It is then possible to do complex access checks without race conditions or
+inconsistent evaluation (i.e.  `incorrect mirroring of the OS code and state
+<https://www.ndss-symposium.org/ndss2003/traps-and-pitfalls-practical-problems-system-call-interposition-based-security-tools/>`_).
+
+A Landlock hook describes a particular access type.  For now, there is one hook
+dedicated to ptrace related operations: ``BPF_LANDLOCK_PTRACE``.  A Landlock
+program is tied to one hook.  This makes it possible to statically check
+context accesses, potentially performed by such program, and hence prevents
+kernel address leaks and ensure the right use of hook arguments with eBPF
+functions.  Any user can add multiple Landlock programs per Landlock hook.
+They are stacked and evaluated one after the other, starting from the most
+recent program, as seccomp-bpf does with its filters.  Underneath, a hook is an
+abstraction over a set of LSM hooks.
+
+
+Guiding principles
+==================
+
+Unprivileged use
+----------------
+
+* As far as possible, Landlock helpers and contexts should be *designed* to be
+  usable by unprivileged programs while following the system security policy
+  enforced by other access control mechanisms (e.g. DAC, LSM).  Indeed, a
+  Landlock program shall not interfere with other access-controls enforced on
+  the system.
+
+Because one of the Landlock's goal is to create scoped access-control (i.e.
+sandboxing), it makes sense to make it possible to have access-control-safe
+programs.  This enables to avoid unneeded security risks when writing a
+security policy.  We should also keep in mind that a Landlock program may be
+written and loaded in the kernel by a trusted process, but applied by a
+non-root (and possibly malicious) process to sandbox itself e.g., using a
+sandboxer service.  This sandboxed process must not be able to leverage one of
+the Landlock program applied on itself to do a privilege escalation nor to
+infer data that should not be accessible otherwise (i.e. side-channels).
+
+However, when justified, it should be possible to have dedicated
+privileged-only program types e.g., to make a security decision based on
+properties inaccessible by unprivileged processes, or to log actions with
+additional metadata.  As explained above, these properties should not be
+inferable from the enforced access-control.  Care must be taken to not only
+focus on these programs' context or helpers to avoid putting everything in a
+root-only realm (cf. `CAP_SYS_ADMIN: the new root
+<https://lwn.net/Articles/486306/>`_).
+
+It should be noted that ``CAP_SYS_ADMIN`` is currently required for loading and
+for enforcing any Landlock programs, but more fine-grained rights may be
+discussed in the future.
+
+
+Landlock hook and context
+-------------------------
+
+* A Landlock hook shall be focused on access control on kernel objects instead
+  of syscall filtering (i.e. syscall arguments), which is the purpose of
+  seccomp-bpf.
+* A Landlock context provided by a hook shall express the minimal and more
+  generic interface to control an access for a kernel object.  This may be
+  implemented with kernel pointers used as security capabilities (i.e.
+  unforgeable token enabling actions on an object according to a set of
+  rights).
+* A hook shall guaranty that all the BPF function calls from a program are
+  safe.  Thus, the related Landlock context arguments shall always be of the
+  same type for a particular hook.  For example, a network hook could share
+  helpers with a file hook because of UNIX socket.  However, the same helpers
+  may not be compatible for a file system handle and a net handle.
+* Multiple hooks may use the same context interface.
+
+
+Landlock helpers
+----------------
+
+* Landlock helpers shall be as generic as possible while at the same time being
+  as simple as possible and following the syscall creation principles (cf.
+  :doc:`/process/adding-syscalls`).
+* The only behavior change allowed on a helper is to fix a (logical) bug to
+  match the initial semantic.
+* Helpers shall be reentrant, i.e. only take inputs from arguments (e.g. from
+  the BPF context), to enable a hook to use a cache.  Future program options
+  might change this cache behavior.
+* It is quite easy to add new helpers to extend Landlock.  The main concern
+  should be about the possibility to leak information from the kernel that may
+  not be accessible otherwise (i.e. side-channel attack).
+
+
+Landlock domain
+===============
+
+A Landlock domain is a set of eBPF programs.  There is a list for each
+different program types that can be run on a specific Landlock hook (e.g.
+ptrace).  A domain is tied to a set of subjects (i.e. tasks).
+
+A Landlock program should not try (nor be able) to infer which subject is
+currently enforced, but to have a unique security policy for all subjects tied
+to the same domain.  This make the reasoning much easier and help avoid
+pitfalls.
+
+.. kernel-doc:: security/landlock/common.h
+    :functions: landlock_domain
+
+.. kernel-doc:: security/landlock/domain_manage.c
+    :functions: landlock_prepend_prog
+
+
+Adding a Landlock program with seccomp
+--------------------------------------
+
+The :manpage:`seccomp(2)` syscall can be used with the
+``SECCOMP_PREPEND_LANDLOCK_PROG`` operation to prepend a Landlock program to
+the current task's domain.
+
+.. kernel-doc:: security/landlock/domain_syscall.c
+    :functions: landlock_seccomp_prepend_prog
+
+
+Running a list of Landlock programs
+-----------------------------------
+
+.. kernel-doc:: security/landlock/bpf_run.c
+    :functions: landlock_access_denied
+
+
+LSM hooks
+=========
+
+.. kernel-doc:: security/landlock/hooks_ptrace.c
+    :functions: hook_ptrace_access_check
+
+.. kernel-doc:: security/landlock/hooks_ptrace.c
+    :functions: hook_ptrace_traceme
+
+
+Questions and answers
+=====================
+
+Why a program does not return an errno or a kill code?
+------------------------------------------------------
+
+seccomp filters can return multiple kind of code, including an errno value or a
+kill signal, which may be convenient for access control.  Those return codes
+are hardwired in the userland ABI.  Instead, Landlock's approach is to return a
+bitmask to allow or deny an action, which is much simpler and more generic.
+Moreover, we do not really have a choice because, unlike to seccomp, Landlock
+programs are not enforced at the syscall entry point but may be executed at any
+point in the kernel (through LSM hooks) where an errno return code may not make
+sense.  However, with this simple ABI and with the ability to call helpers,
+Landlock may gain features similar to seccomp-bpf in the future while being
+compatible with previous programs.
diff --git a/Documentation/security/landlock/user.rst b/Documentation/security/landlock/user.rst
new file mode 100644
index 000000000000..ef48e7752f1b
--- /dev/null
+++ b/Documentation/security/landlock/user.rst
@@ -0,0 +1,153 @@
+=================================
+Landlock: userspace documentation
+=================================
+
+Landlock programs
+=================
+
+eBPF programs are used to create security programs.  They are contained and can
+call only a whitelist of dedicated functions. Moreover, they can only loop
+under strict conditions, which protects from denial of service.  More
+information on BPF can be found in :doc:`/bpf/index`.
+
+
+Writing a program
+-----------------
+
+To enforce a security policy, a thread first needs to create a Landlock
+program.  The easiest way to write an eBPF program depicting a security program
+is to write it in the C language.  As described in `samples/bpf/README.rst`_,
+LLVM can compile such programs.  A simple eBPF program can also be written by
+hand has done in `tools/testing/selftests/landlock/`_.
+
+Once the eBPF program is created, the next step is to create the metadata
+describing the Landlock program.  This metadata includes an expected attach
+type which contains the hook type to which the program is tied.
+
+A hook is a policy decision point which exposes the same context type for
+each program evaluation.
+
+A Landlock hook describes the kind of kernel object for which a program will be
+triggered to allow or deny an action.  For example, the hook
+``BPF_LANDLOCK_PTRACE`` can be triggered every time a landlocked thread
+performs a set of action related to debugging (cf. :manpage:`ptrace(2)`) or if
+the kernel needs to know if a process manipulation requested by something else
+is legitimate.
+
+The next step is to fill a :c:type:`struct bpf_load_program_attr
+<bpf_load_program_attr>` with ``BPF_PROG_TYPE_LANDLOCK_HOOK``, the expected
+attach type and other BPF program metadata.  This bpf_attr must then be passed
+to the :manpage:`bpf(2)` syscall alongside the ``BPF_PROG_LOAD`` command.  If
+everything is deemed correct by the kernel, the thread gets a file descriptor
+referring to this program.
+
+In the following code, the `insn` variable is an array of BPF instructions
+which can be extracted from an ELF file as is done in bpf_load_file() from
+`samples/bpf/bpf_load.c`_.
+
+.. code-block:: c
+
+    int prog_fd;
+    struct bpf_load_program_attr load_attr;
+
+    memset(&load_attr, 0, sizeof(struct bpf_load_program_attr));
+    load_attr.prog_type = BPF_PROG_TYPE_LANDLOCK_HOOK;
+    load_attr.expected_attach_type = BPF_LANDLOCK_PTRACE;
+    load_attr.insns = insns;
+    load_attr.insns_cnt = sizeof(insn) / sizeof(struct bpf_insn);
+    load_attr.license = "GPL";
+
+    prog_fd = bpf_load_program_xattr(&load_attr, log_buf, log_buf_sz);
+    if (prog_fd == -1)
+        exit(1);
+
+
+Enforcing a program
+-------------------
+
+Once the Landlock program has been created or received (e.g. through a UNIX
+socket), the thread willing to sandbox itself (and its future children) should
+perform the following two steps.
+
+The thread should first request to never be allowed to get new privileges with
+a call to :manpage:`prctl(2)` and the ``PR_SET_NO_NEW_PRIVS`` option.  More
+information can be found in :doc:`/userspace-api/no_new_privs`.
+
+.. code-block:: c
+
+    if (prctl(PR_SET_NO_NEW_PRIVS, 1, NULL, 0, 0))
+        exit(1);
+
+A thread can apply a program to itself by using the :manpage:`seccomp(2)`
+syscall.  The operation is ``SECCOMP_PREPEND_LANDLOCK_PROG``, the flags must be
+empty and the `args` argument must point to a valid Landlock program file
+descriptor.
+
+.. code-block:: c
+
+    if (seccomp(SECCOMP_PREPEND_LANDLOCK_PROG, 0, &fd))
+        exit(1);
+
+If the syscall succeeds, the program is now enforced on the calling thread and
+will be enforced on all its subsequently created children of the thread as
+well.  Once a thread is landlocked, there is no way to remove this security
+policy, only stacking more restrictions is allowed.  The program evaluation is
+performed from the newest to the oldest.
+
+When a syscall ask for an action on a kernel object, if this action is denied,
+then an ``EACCES`` errno code is returned through the syscall.
+
+
+.. _inherited_programs:
+
+Inherited programs
+------------------
+
+Every new thread resulting from a :manpage:`clone(2)` inherits Landlock program
+restrictions from its parent.  This is similar to the seccomp inheritance (cf.
+:doc:`/userspace-api/seccomp_filter`) or any other LSM dealing with task's
+:manpage:`credentials(7)`.  For instance, one process's thread may apply
+Landlock programs to itself, but they will not be automatically applied to
+other sibling threads (unlike POSIX thread credential changes, cf.
+:manpage:`nptl(7)`).
+
+
+Ptrace restrictions
+-------------------
+
+A sandboxed process has less privileges than a non-sandboxed process and must
+then be subject to additional restrictions when manipulating another process.
+To be allowed to use :manpage:`ptrace(2)` and related syscalls on a target
+process, a sandboxed process should have a subset of the target process
+programs.  This security policy can easily be implemented like in
+`tools/testing/selftests/landlock/test_ptrace.c`_.
+
+
+Landlock structures and constants
+=================================
+
+Contexts
+--------
+
+.. kernel-doc:: include/uapi/linux/landlock.h
+    :functions: landlock_context_ptrace
+
+
+Return types
+------------
+
+.. kernel-doc:: include/uapi/linux/landlock.h
+    :functions: landlock_ret
+
+
+Additional documentation
+========================
+
+See https://landlock.io
+
+
+.. Links
+.. _samples/bpf/README.rst: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/samples/bpf/README.rst
+.. _tools/testing/selftests/landlock/: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/tools/testing/selftests/landlock/
+.. _samples/bpf/bpf_load.c: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/samples/bpf/bpf_load.c
+.. _tools/testing/selftests/landlock/test_ptrace.c: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/tools/testing/selftests/landlock/test_ptrace.c
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-04 17:21 ` [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks Mickaël Salaün
@ 2019-11-05 17:18   ` Alexei Starovoitov
  2019-11-05 17:55     ` Casey Schaufler
  2019-11-05 18:01     ` Mickaël Salaün
  0 siblings, 2 replies; 26+ messages in thread
From: Alexei Starovoitov @ 2019-11-05 17:18 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

On Mon, Nov 04, 2019 at 06:21:43PM +0100, Mickaël Salaün wrote:
> Add a first Landlock hook that can be used to enforce a security policy
> or to audit some process activities.  For a sandboxing use-case, it is
> needed to inform the kernel if a task can legitimately debug another.
> ptrace(2) can also be used by an attacker to impersonate another task
> and remain undetected while performing malicious activities.
> 
> Using ptrace(2) and related features on a target process can lead to a
> privilege escalation.  A sandboxed task must then be able to tell the
> kernel if another task is more privileged, via ptrace_may_access().
> 
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
...
> +static int check_ptrace(struct landlock_domain *domain,
> +		struct task_struct *tracer, struct task_struct *tracee)
> +{
> +	struct landlock_hook_ctx_ptrace ctx_ptrace = {
> +		.prog_ctx = {
> +			.tracer = (uintptr_t)tracer,
> +			.tracee = (uintptr_t)tracee,
> +		},
> +	};

So you're passing two kernel pointers obfuscated as u64 into bpf program
yet claiming that the end goal is to make landlock unprivileged?!
The most basic security hole in the tool that is aiming to provide security.

I think the only way bpf-based LSM can land is both landlock and KRSI
developers work together on a design that solves all use cases. BPF is capable
to be a superset of all existing LSMs whereas landlock and KRSI propsals today
are custom solutions to specific security concerns. BPF subsystem was extended
with custom things in the past. In networking we have lwt, skb, tc, xdp, sk
program types with a lot of overlapping functionality. We couldn't figure out
how to generalize them into single 'networking' program. Now we can and we
should. Accepting two partially overlapping bpf-based LSMs would be repeating
the same mistake again.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-05 17:18   ` Alexei Starovoitov
@ 2019-11-05 17:55     ` Casey Schaufler
  2019-11-05 19:31       ` Alexei Starovoitov
  2019-11-05 18:01     ` Mickaël Salaün
  1 sibling, 1 reply; 26+ messages in thread
From: Casey Schaufler @ 2019-11-05 17:55 UTC (permalink / raw)
  To: Alexei Starovoitov, Mickaël Salaün
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski,
	Daniel Borkmann, David Drysdale, Florent Revest, James Morris,
	Jann Horn, John Johansen, Jonathan Corbet, Kees Cook, KP Singh,
	Michael Kerrisk, Mickaël Salaün, Paul Moore,
	Sargun Dhillon, Serge E . Hallyn, Shuah Khan, Stephen Smalley,
	Tejun Heo, Tetsuo Handa, Tycho Andersen, Will Drewry, bpf,
	kernel-hardening, linux-api, linux-security-module, netdev,
	casey

On 11/5/2019 9:18 AM, Alexei Starovoitov wrote:
> On Mon, Nov 04, 2019 at 06:21:43PM +0100, Mickaël Salaün wrote:
>> Add a first Landlock hook that can be used to enforce a security policy
>> or to audit some process activities.  For a sandboxing use-case, it is
>> needed to inform the kernel if a task can legitimately debug another.
>> ptrace(2) can also be used by an attacker to impersonate another task
>> and remain undetected while performing malicious activities.
>>
>> Using ptrace(2) and related features on a target process can lead to a
>> privilege escalation.  A sandboxed task must then be able to tell the
>> kernel if another task is more privileged, via ptrace_may_access().
>>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ...
>> +static int check_ptrace(struct landlock_domain *domain,
>> +		struct task_struct *tracer, struct task_struct *tracee)
>> +{
>> +	struct landlock_hook_ctx_ptrace ctx_ptrace = {
>> +		.prog_ctx = {
>> +			.tracer = (uintptr_t)tracer,
>> +			.tracee = (uintptr_t)tracee,
>> +		},
>> +	};
> So you're passing two kernel pointers obfuscated as u64 into bpf program
> yet claiming that the end goal is to make landlock unprivileged?!
> The most basic security hole in the tool that is aiming to provide security.
>
> I think the only way bpf-based LSM can land is both landlock and KRSI
> developers work together on a design that solves all use cases. BPF is capable
> to be a superset of all existing LSMs

I can't agree with this. Nope. There are many security models
for which BPF introduces excessive complexity. You don't need
or want the generality of a general purpose programming language
to implement Smack or TOMOYO. Or a simple Bell & LaPadula for
that matter. SELinux? I can't imagine anyone trying to do that
in eBPF, although I'm willing to be surprised. Being able to
enforce a policy isn't the only criteria for an LSM. It's got
to perform well and integrate with the rest of the system. I
see many issues with a BPF <-> vfs interface.

> whereas landlock and KRSI propsals today
> are custom solutions to specific security concerns.

Yes. As they should be. No one has every solved the entire
security problem, and no one ever will. The only hope we have
to address security issues is to have the flexibility to add
the mechanisms needed for the concerns of the day. Ideally,
we should be able to drop mechanisms when we decide that they
no longer add value.

> BPF subsystem was extended
> with custom things in the past. In networking we have lwt, skb, tc, xdp, sk
> program types with a lot of overlapping functionality. We couldn't figure out
> how to generalize them into single 'networking' program. Now we can and we
> should. Accepting two partially overlapping bpf-based LSMs would be repeating
> the same mistake again.

I don't get your analogy at all. You have a variety of programs because
you have a variety of protocols and administrative interfaces. Of course
you don't have a single 'networking" program. Security has a variety of
issues and policies. A single 'security' program makes no sense whatever.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-05 17:18   ` Alexei Starovoitov
  2019-11-05 17:55     ` Casey Schaufler
@ 2019-11-05 18:01     ` Mickaël Salaün
  2019-11-05 19:34       ` Alexei Starovoitov
  2019-11-06 10:15       ` KP Singh
  1 sibling, 2 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-05 18:01 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev


On 05/11/2019 18:18, Alexei Starovoitov wrote:
> On Mon, Nov 04, 2019 at 06:21:43PM +0100, Mickaël Salaün wrote:
>> Add a first Landlock hook that can be used to enforce a security policy
>> or to audit some process activities.  For a sandboxing use-case, it is
>> needed to inform the kernel if a task can legitimately debug another.
>> ptrace(2) can also be used by an attacker to impersonate another task
>> and remain undetected while performing malicious activities.
>>
>> Using ptrace(2) and related features on a target process can lead to a
>> privilege escalation.  A sandboxed task must then be able to tell the
>> kernel if another task is more privileged, via ptrace_may_access().
>>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ...
>> +static int check_ptrace(struct landlock_domain *domain,
>> +		struct task_struct *tracer, struct task_struct *tracee)
>> +{
>> +	struct landlock_hook_ctx_ptrace ctx_ptrace = {
>> +		.prog_ctx = {
>> +			.tracer = (uintptr_t)tracer,
>> +			.tracee = (uintptr_t)tracee,
>> +		},
>> +	};
> 
> So you're passing two kernel pointers obfuscated as u64 into bpf program
> yet claiming that the end goal is to make landlock unprivileged?!
> The most basic security hole in the tool that is aiming to provide security.

How could you used these pointers without dedicated BPF helpers? This
context items are typed as PTR_TO_TASK and can't be used without a
dedicated helper able to deal with ARG_PTR_TO_TASK. Moreover, pointer
arithmetic is explicitly forbidden (and I added tests for that). Did I
miss something?

> 
> I think the only way bpf-based LSM can land is both landlock and KRSI
> developers work together on a design that solves all use cases.

As I said in a previous cover letter [1], that would be great. I think
that the current Landlock bases (almost everything from this series
except the seccomp interface) should meet both needs, but I would like
to have the point of view of the KRSI developers.

[1] https://lore.kernel.org/lkml/20191029171505.6650-1-mic@digikod.net/

> BPF is capable
> to be a superset of all existing LSMs whereas landlock and KRSI propsals today
> are custom solutions to specific security concerns. BPF subsystem was extended
> with custom things in the past. In networking we have lwt, skb, tc, xdp, sk
> program types with a lot of overlapping functionality. We couldn't figure out
> how to generalize them into single 'networking' program. Now we can and we
> should. Accepting two partially overlapping bpf-based LSMs would be repeating
> the same mistake again.

I'll let the LSM maintainers comment on whether BPF could be a superset
of all LSM, but given the complexity of an access-control system, I have
some doubts though. Anyway, we need to start somewhere and then iterate.
This patch series is a first step.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-05 17:55     ` Casey Schaufler
@ 2019-11-05 19:31       ` Alexei Starovoitov
  2019-11-05 19:55         ` Casey Schaufler
  0 siblings, 1 reply; 26+ messages in thread
From: Alexei Starovoitov @ 2019-11-05 19:31 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: Mickaël Salaün, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

On Tue, Nov 05, 2019 at 09:55:42AM -0800, Casey Schaufler wrote:
> On 11/5/2019 9:18 AM, Alexei Starovoitov wrote:
> > On Mon, Nov 04, 2019 at 06:21:43PM +0100, Mickaël Salaün wrote:
> >> Add a first Landlock hook that can be used to enforce a security policy
> >> or to audit some process activities.  For a sandboxing use-case, it is
> >> needed to inform the kernel if a task can legitimately debug another.
> >> ptrace(2) can also be used by an attacker to impersonate another task
> >> and remain undetected while performing malicious activities.
> >>
> >> Using ptrace(2) and related features on a target process can lead to a
> >> privilege escalation.  A sandboxed task must then be able to tell the
> >> kernel if another task is more privileged, via ptrace_may_access().
> >>
> >> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ...
> >> +static int check_ptrace(struct landlock_domain *domain,
> >> +		struct task_struct *tracer, struct task_struct *tracee)
> >> +{
> >> +	struct landlock_hook_ctx_ptrace ctx_ptrace = {
> >> +		.prog_ctx = {
> >> +			.tracer = (uintptr_t)tracer,
> >> +			.tracee = (uintptr_t)tracee,
> >> +		},
> >> +	};
> > So you're passing two kernel pointers obfuscated as u64 into bpf program
> > yet claiming that the end goal is to make landlock unprivileged?!
> > The most basic security hole in the tool that is aiming to provide security.
> >
> > I think the only way bpf-based LSM can land is both landlock and KRSI
> > developers work together on a design that solves all use cases. BPF is capable
> > to be a superset of all existing LSMs
> 
> I can't agree with this. Nope. There are many security models
> for which BPF introduces excessive complexity. You don't need
> or want the generality of a general purpose programming language
> to implement Smack or TOMOYO. Or a simple Bell & LaPadula for
> that matter. SELinux? I can't imagine anyone trying to do that
> in eBPF, although I'm willing to be surprised. Being able to
> enforce a policy isn't the only criteria for an LSM. 

what are the other criteria?

> It's got
> to perform well and integrate with the rest of the system. 

what do you mean by that?

> I see many issues with a BPF <-> vfs interface.

There is no such interface today. What do you have in mind?

> the mechanisms needed for the concerns of the day. Ideally,
> we should be able to drop mechanisms when we decide that they
> no longer add value.

Exactly. bpf-based lsm must not add to kernel abi.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-05 18:01     ` Mickaël Salaün
@ 2019-11-05 19:34       ` Alexei Starovoitov
  2019-11-05 22:18         ` Mickaël Salaün
  2019-11-06 10:06         ` KP Singh
  2019-11-06 10:15       ` KP Singh
  1 sibling, 2 replies; 26+ messages in thread
From: Alexei Starovoitov @ 2019-11-05 19:34 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

On Tue, Nov 05, 2019 at 07:01:41PM +0100, Mickaël Salaün wrote:
> 
> On 05/11/2019 18:18, Alexei Starovoitov wrote:
> > On Mon, Nov 04, 2019 at 06:21:43PM +0100, Mickaël Salaün wrote:
> >> Add a first Landlock hook that can be used to enforce a security policy
> >> or to audit some process activities.  For a sandboxing use-case, it is
> >> needed to inform the kernel if a task can legitimately debug another.
> >> ptrace(2) can also be used by an attacker to impersonate another task
> >> and remain undetected while performing malicious activities.
> >>
> >> Using ptrace(2) and related features on a target process can lead to a
> >> privilege escalation.  A sandboxed task must then be able to tell the
> >> kernel if another task is more privileged, via ptrace_may_access().
> >>
> >> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ...
> >> +static int check_ptrace(struct landlock_domain *domain,
> >> +		struct task_struct *tracer, struct task_struct *tracee)
> >> +{
> >> +	struct landlock_hook_ctx_ptrace ctx_ptrace = {
> >> +		.prog_ctx = {
> >> +			.tracer = (uintptr_t)tracer,
> >> +			.tracee = (uintptr_t)tracee,
> >> +		},
> >> +	};
> > 
> > So you're passing two kernel pointers obfuscated as u64 into bpf program
> > yet claiming that the end goal is to make landlock unprivileged?!
> > The most basic security hole in the tool that is aiming to provide security.
> 
> How could you used these pointers without dedicated BPF helpers? This
> context items are typed as PTR_TO_TASK and can't be used without a
> dedicated helper able to deal with ARG_PTR_TO_TASK. Moreover, pointer
> arithmetic is explicitly forbidden (and I added tests for that). Did I
> miss something?

It's a pointer leak.

> 
> > 
> > I think the only way bpf-based LSM can land is both landlock and KRSI
> > developers work together on a design that solves all use cases.
> 
> As I said in a previous cover letter [1], that would be great. I think
> that the current Landlock bases (almost everything from this series
> except the seccomp interface) should meet both needs, but I would like
> to have the point of view of the KRSI developers.
> 
> [1] https://lore.kernel.org/lkml/20191029171505.6650-1-mic@digikod.net/
> 
> > BPF is capable
> > to be a superset of all existing LSMs whereas landlock and KRSI propsals today
> > are custom solutions to specific security concerns. BPF subsystem was extended
> > with custom things in the past. In networking we have lwt, skb, tc, xdp, sk
> > program types with a lot of overlapping functionality. We couldn't figure out
> > how to generalize them into single 'networking' program. Now we can and we
> > should. Accepting two partially overlapping bpf-based LSMs would be repeating
> > the same mistake again.
> 
> I'll let the LSM maintainers comment on whether BPF could be a superset
> of all LSM, but given the complexity of an access-control system, I have
> some doubts though. Anyway, we need to start somewhere and then iterate.
> This patch series is a first step.

I would like KRSI folks to speak up. So far I don't see any sharing happening
between landlock and KRSI. You're claiming this set is a first step. They're
claiming the same about their patches. I'd like to set a patchset that was
jointly developed.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-05 19:31       ` Alexei Starovoitov
@ 2019-11-05 19:55         ` Casey Schaufler
  2019-11-05 21:54           ` Alexei Starovoitov
  0 siblings, 1 reply; 26+ messages in thread
From: Casey Schaufler @ 2019-11-05 19:55 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Mickaël Salaün, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev, casey

On 11/5/2019 11:31 AM, Alexei Starovoitov wrote:
> On Tue, Nov 05, 2019 at 09:55:42AM -0800, Casey Schaufler wrote:
>> On 11/5/2019 9:18 AM, Alexei Starovoitov wrote:
>>> On Mon, Nov 04, 2019 at 06:21:43PM +0100, Mickaël Salaün wrote:
>>>> Add a first Landlock hook that can be used to enforce a security policy
>>>> or to audit some process activities.  For a sandboxing use-case, it is
>>>> needed to inform the kernel if a task can legitimately debug another.
>>>> ptrace(2) can also be used by an attacker to impersonate another task
>>>> and remain undetected while performing malicious activities.
>>>>
>>>> Using ptrace(2) and related features on a target process can lead to a
>>>> privilege escalation.  A sandboxed task must then be able to tell the
>>>> kernel if another task is more privileged, via ptrace_may_access().
>>>>
>>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>> ...
>>>> +static int check_ptrace(struct landlock_domain *domain,
>>>> +		struct task_struct *tracer, struct task_struct *tracee)
>>>> +{
>>>> +	struct landlock_hook_ctx_ptrace ctx_ptrace = {
>>>> +		.prog_ctx = {
>>>> +			.tracer = (uintptr_t)tracer,
>>>> +			.tracee = (uintptr_t)tracee,
>>>> +		},
>>>> +	};
>>> So you're passing two kernel pointers obfuscated as u64 into bpf program
>>> yet claiming that the end goal is to make landlock unprivileged?!
>>> The most basic security hole in the tool that is aiming to provide security.
>>>
>>> I think the only way bpf-based LSM can land is both landlock and KRSI
>>> developers work together on a design that solves all use cases. BPF is capable
>>> to be a superset of all existing LSMs
>> I can't agree with this. Nope. There are many security models
>> for which BPF introduces excessive complexity. You don't need
>> or want the generality of a general purpose programming language
>> to implement Smack or TOMOYO. Or a simple Bell & LaPadula for
>> that matter. SELinux? I can't imagine anyone trying to do that
>> in eBPF, although I'm willing to be surprised. Being able to
>> enforce a policy isn't the only criteria for an LSM. 
> what are the other criteria?

They include, but are not limited to, performance impact
and the ability to be analyzed. The interactions with other
subsystems meeting the requirements thereof is always a concern.


>
>> It's got
>> to perform well and integrate with the rest of the system. 
> what do you mean by that?

It has to be fast, or the networking people are
going to have fits. You can't require the addition
of a pointer into the skb because it'll get rejected
out of hand. You can't completely refactor the vfs locking
to accommodate you needs.

>
>> I see many issues with a BPF <-> vfs interface.
> There is no such interface today. What do you have in mind?

You can't implement SELinux or Smack using BPF without a way
to manipulate inode data.

>
>> the mechanisms needed for the concerns of the day. Ideally,
>> we should be able to drop mechanisms when we decide that they
>> no longer add value.
> Exactly. bpf-based lsm must not add to kernel abi.

Huh? I have no idea where that came from.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-05 19:55         ` Casey Schaufler
@ 2019-11-05 21:54           ` Alexei Starovoitov
  2019-11-05 22:32             ` Casey Schaufler
  0 siblings, 1 reply; 26+ messages in thread
From: Alexei Starovoitov @ 2019-11-05 21:54 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: Mickaël Salaün, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev

On Tue, Nov 05, 2019 at 11:55:17AM -0800, Casey Schaufler wrote:
> On 11/5/2019 11:31 AM, Alexei Starovoitov wrote:
> > On Tue, Nov 05, 2019 at 09:55:42AM -0800, Casey Schaufler wrote:
> >> On 11/5/2019 9:18 AM, Alexei Starovoitov wrote:
> >>> On Mon, Nov 04, 2019 at 06:21:43PM +0100, Mickaël Salaün wrote:
> >>>> Add a first Landlock hook that can be used to enforce a security policy
> >>>> or to audit some process activities.  For a sandboxing use-case, it is
> >>>> needed to inform the kernel if a task can legitimately debug another.
> >>>> ptrace(2) can also be used by an attacker to impersonate another task
> >>>> and remain undetected while performing malicious activities.
> >>>>
> >>>> Using ptrace(2) and related features on a target process can lead to a
> >>>> privilege escalation.  A sandboxed task must then be able to tell the
> >>>> kernel if another task is more privileged, via ptrace_may_access().
> >>>>
> >>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> >>> ...
> >>>> +static int check_ptrace(struct landlock_domain *domain,
> >>>> +		struct task_struct *tracer, struct task_struct *tracee)
> >>>> +{
> >>>> +	struct landlock_hook_ctx_ptrace ctx_ptrace = {
> >>>> +		.prog_ctx = {
> >>>> +			.tracer = (uintptr_t)tracer,
> >>>> +			.tracee = (uintptr_t)tracee,
> >>>> +		},
> >>>> +	};
> >>> So you're passing two kernel pointers obfuscated as u64 into bpf program
> >>> yet claiming that the end goal is to make landlock unprivileged?!
> >>> The most basic security hole in the tool that is aiming to provide security.
> >>>
> >>> I think the only way bpf-based LSM can land is both landlock and KRSI
> >>> developers work together on a design that solves all use cases. BPF is capable
> >>> to be a superset of all existing LSMs
> >> I can't agree with this. Nope. There are many security models
> >> for which BPF introduces excessive complexity. You don't need
> >> or want the generality of a general purpose programming language
> >> to implement Smack or TOMOYO. Or a simple Bell & LaPadula for
> >> that matter. SELinux? I can't imagine anyone trying to do that
> >> in eBPF, although I'm willing to be surprised. Being able to
> >> enforce a policy isn't the only criteria for an LSM. 
> > what are the other criteria?
> 
> They include, but are not limited to, performance impact
> and the ability to be analyzed. 

Right and BPF is the only thing that exists in the kernel where the verifier
knows precisely the number of instructions the critical path through the
program will take. Currently we don't quantify this cost for bpf helpers, but
it's easy to add. Can you do this for smack? Can you tell upfront the longest
execution time for all security rules?

> It has to be fast, or the networking people are
> going to have fits. You can't require the addition
> of a pointer into the skb because it'll get rejected
> out of hand. You can't completely refactor the vfs locking
> to accommodate you needs.

I'm not sure why you got such impression. I'm not proposing to refactor vfs or
add fields to skb. Once we have equivalent to smack policy implemented in
bpf-based lsm let's do performance benchmarking and compare actual numbers
instead of hypothesizing about them. Which policy do you think would be
the most representative of smack use case?

> 
> >
> >> I see many issues with a BPF <-> vfs interface.
> > There is no such interface today. What do you have in mind?
> 
> You can't implement SELinux or Smack using BPF without a way
> to manipulate inode data.

Are you talking about inode->i_security ? That's not manipulating inode data.
It's attaching extra metadata to inode object without changing inode itself.
BPF can do it already via hash maps. It's not as fast as direct pointer access,
but for many use cases it's good enough. If it turns out to be a performance
limiting factor we will accelerate it.

> >> the mechanisms needed for the concerns of the day. Ideally,
> >> we should be able to drop mechanisms when we decide that they
> >> no longer add value.
> > Exactly. bpf-based lsm must not add to kernel abi.
> 
> Huh? I have no idea where that came from.

It sounds to me that some folks in the community got wrong impression that
anything that BPF accesses is magically turning that thing into stable kernel
ABI. That is not true. BPF progs had access _all_ kernel data pointers and
structures for years without turning the whole kernel into stable ABI. I want
to make sure that this part is understood. This is also a requirement for
bpf-based LSM. It must not make LSM hooks into stable ABI.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-05 19:34       ` Alexei Starovoitov
@ 2019-11-05 22:18         ` Mickaël Salaün
  2019-11-06 10:06         ` KP Singh
  1 sibling, 0 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-05 22:18 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev


On 05/11/2019 20:34, Alexei Starovoitov wrote:
> On Tue, Nov 05, 2019 at 07:01:41PM +0100, Mickaël Salaün wrote:
>>
>> On 05/11/2019 18:18, Alexei Starovoitov wrote:
>>> On Mon, Nov 04, 2019 at 06:21:43PM +0100, Mickaël Salaün wrote:
>>>> Add a first Landlock hook that can be used to enforce a security policy
>>>> or to audit some process activities.  For a sandboxing use-case, it is
>>>> needed to inform the kernel if a task can legitimately debug another.
>>>> ptrace(2) can also be used by an attacker to impersonate another task
>>>> and remain undetected while performing malicious activities.
>>>>
>>>> Using ptrace(2) and related features on a target process can lead to a
>>>> privilege escalation.  A sandboxed task must then be able to tell the
>>>> kernel if another task is more privileged, via ptrace_may_access().
>>>>
>>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>> ...
>>>> +static int check_ptrace(struct landlock_domain *domain,
>>>> +		struct task_struct *tracer, struct task_struct *tracee)
>>>> +{
>>>> +	struct landlock_hook_ctx_ptrace ctx_ptrace = {
>>>> +		.prog_ctx = {
>>>> +			.tracer = (uintptr_t)tracer,
>>>> +			.tracee = (uintptr_t)tracee,
>>>> +		},
>>>> +	};
>>>
>>> So you're passing two kernel pointers obfuscated as u64 into bpf program
>>> yet claiming that the end goal is to make landlock unprivileged?!
>>> The most basic security hole in the tool that is aiming to provide security.
>>
>> How could you used these pointers without dedicated BPF helpers? This
>> context items are typed as PTR_TO_TASK and can't be used without a
>> dedicated helper able to deal with ARG_PTR_TO_TASK. Moreover, pointer
>> arithmetic is explicitly forbidden (and I added tests for that). Did I
>> miss something?
> 
> It's a pointer leak.

The lifetimes of the pointers are scoped by the two LSM hooks that
expose them. The LSM framework guarantee that they are safe to use in
this context.

> 
>>
>>>
>>> I think the only way bpf-based LSM can land is both landlock and KRSI
>>> developers work together on a design that solves all use cases.
>>
>> As I said in a previous cover letter [1], that would be great. I think
>> that the current Landlock bases (almost everything from this series
>> except the seccomp interface) should meet both needs, but I would like
>> to have the point of view of the KRSI developers.
>>
>> [1] https://lore.kernel.org/lkml/20191029171505.6650-1-mic@digikod.net/
>>
>>> BPF is capable
>>> to be a superset of all existing LSMs whereas landlock and KRSI propsals today
>>> are custom solutions to specific security concerns. BPF subsystem was extended
>>> with custom things in the past. In networking we have lwt, skb, tc, xdp, sk
>>> program types with a lot of overlapping functionality. We couldn't figure out
>>> how to generalize them into single 'networking' program. Now we can and we
>>> should. Accepting two partially overlapping bpf-based LSMs would be repeating
>>> the same mistake again.
>>
>> I'll let the LSM maintainers comment on whether BPF could be a superset
>> of all LSM, but given the complexity of an access-control system, I have
>> some doubts though. Anyway, we need to start somewhere and then iterate.
>> This patch series is a first step.
> 
> I would like KRSI folks to speak up. So far I don't see any sharing happening
> between landlock and KRSI. You're claiming this set is a first step. They're
> claiming the same about their patches. I'd like to set a patchset that was
> jointly developed.

With all due respect, Landlock got much more feedback than KRSI and I
think this thirteenth Landlock patch series is more mature than the
first KRSI RFC. I'm open to concrete suggestions and I'm willing to
collaborate with the KRSI folks if they want to. However, I'm OK if they
don't want to use Landlock as a common ground, and I don't think it
should be a blocker for any of the projects.

Perfect is the enemy of good. ;)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-05 21:54           ` Alexei Starovoitov
@ 2019-11-05 22:32             ` Casey Schaufler
  0 siblings, 0 replies; 26+ messages in thread
From: Casey Schaufler @ 2019-11-05 22:32 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Mickaël Salaün, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, KP Singh, Michael Kerrisk, Mickaël Salaün,
	Paul Moore, Sargun Dhillon, Serge E . Hallyn, Shuah Khan,
	Stephen Smalley, Tejun Heo, Tetsuo Handa, Tycho Andersen,
	Will Drewry, bpf, kernel-hardening, linux-api,
	linux-security-module, netdev, casey

On 11/5/2019 1:54 PM, Alexei Starovoitov wrote:
> On Tue, Nov 05, 2019 at 11:55:17AM -0800, Casey Schaufler wrote:
>> On 11/5/2019 11:31 AM, Alexei Starovoitov wrote:
>>> On Tue, Nov 05, 2019 at 09:55:42AM -0800, Casey Schaufler wrote:
>>>> On 11/5/2019 9:18 AM, Alexei Starovoitov wrote:
>>>>> On Mon, Nov 04, 2019 at 06:21:43PM +0100, Mickaël Salaün wrote:
>>>>>> Add a first Landlock hook that can be used to enforce a security policy
>>>>>> or to audit some process activities.  For a sandboxing use-case, it is
>>>>>> needed to inform the kernel if a task can legitimately debug another.
>>>>>> ptrace(2) can also be used by an attacker to impersonate another task
>>>>>> and remain undetected while performing malicious activities.
>>>>>>
>>>>>> Using ptrace(2) and related features on a target process can lead to a
>>>>>> privilege escalation.  A sandboxed task must then be able to tell the
>>>>>> kernel if another task is more privileged, via ptrace_may_access().
>>>>>>
>>>>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>>>> ...
>>>>>> +static int check_ptrace(struct landlock_domain *domain,
>>>>>> +		struct task_struct *tracer, struct task_struct *tracee)
>>>>>> +{
>>>>>> +	struct landlock_hook_ctx_ptrace ctx_ptrace = {
>>>>>> +		.prog_ctx = {
>>>>>> +			.tracer = (uintptr_t)tracer,
>>>>>> +			.tracee = (uintptr_t)tracee,
>>>>>> +		},
>>>>>> +	};
>>>>> So you're passing two kernel pointers obfuscated as u64 into bpf program
>>>>> yet claiming that the end goal is to make landlock unprivileged?!
>>>>> The most basic security hole in the tool that is aiming to provide security.
>>>>>
>>>>> I think the only way bpf-based LSM can land is both landlock and KRSI
>>>>> developers work together on a design that solves all use cases. BPF is capable
>>>>> to be a superset of all existing LSMs
>>>> I can't agree with this. Nope. There are many security models
>>>> for which BPF introduces excessive complexity. You don't need
>>>> or want the generality of a general purpose programming language
>>>> to implement Smack or TOMOYO. Or a simple Bell & LaPadula for
>>>> that matter. SELinux? I can't imagine anyone trying to do that
>>>> in eBPF, although I'm willing to be surprised. Being able to
>>>> enforce a policy isn't the only criteria for an LSM. 
>>> what are the other criteria?
>> They include, but are not limited to, performance impact
>> and the ability to be analyzed. 
> Right and BPF is the only thing that exists in the kernel where the verifier
> knows precisely the number of instructions the critical path through the
> program will take. Currently we don't quantify this cost for bpf helpers, but
> it's easy to add. Can you do this for smack? Can you tell upfront the longest
> execution time for all security rules?

There's much more to analyze than number of instructions.
There's also completion of policy enforcement. There are
lots of tools for measuring performance within the kernel.

>> It has to be fast, or the networking people are
>> going to have fits. You can't require the addition
>> of a pointer into the skb because it'll get rejected
>> out of hand. You can't completely refactor the vfs locking
>> to accommodate you needs.
> I'm not sure why you got such impression. I'm not proposing to refactor vfs or
> add fields to skb.

I'm not saying you did. Those are examples of things you would
have trouble with.

>  Once we have equivalent to smack policy implemented in
> bpf-based lsm let's do performance benchmarking and compare actual numbers
> instead of hypothesizing about them. Which policy do you think would be
> the most representative of smack use case?

The Tizen3 Three domain model will do just fine.
https://wiki.tizen.org/Security:SmackThreeDomainModel


>
>>>> I see many issues with a BPF <-> vfs interface.
>>> There is no such interface today. What do you have in mind?
>> You can't implement SELinux or Smack using BPF without a way
>> to manipulate inode data.
> Are you talking about inode->i_security ? That's not manipulating inode data.

Poppycock.

> It's attaching extra metadata to inode object without changing inode itself.

Where I come from, we call that inode object data.

> BPF can do it already via hash maps. It's not as fast as direct pointer access,

Then you're not listening. Performance MATTERS!

> but for many use cases it's good enough. If it turns out to be a performance
> limiting factor we will accelerate it.

How many times have I heard that bit of rubbish?
No. You can't start with a bad design and tweak it to acceptability later.


>>>> the mechanisms needed for the concerns of the day. Ideally,
>>>> we should be able to drop mechanisms when we decide that they
>>>> no longer add value.
>>> Exactly. bpf-based lsm must not add to kernel abi.
>> Huh? I have no idea where that came from.
> It sounds to me that some folks in the community got wrong impression that
> anything that BPF accesses is magically turning that thing into stable kernel
> ABI. That is not true. BPF progs had access _all_ kernel data pointers and
> structures for years without turning the whole kernel into stable ABI. I want
> to make sure that this part is understood. This is also a requirement for
> bpf-based LSM. It must not make LSM hooks into stable ABI.
>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-05 19:34       ` Alexei Starovoitov
  2019-11-05 22:18         ` Mickaël Salaün
@ 2019-11-06 10:06         ` KP Singh
  2019-11-06 16:55           ` Mickaël Salaün
  1 sibling, 1 reply; 26+ messages in thread
From: KP Singh @ 2019-11-06 10:06 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Mickaël Salaün, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Casey Schaufler, Daniel Borkmann,
	David Drysdale, Florent Revest, James Morris, Jann Horn,
	John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
	Mickaël Salaün, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
	Tetsuo Handa, Tycho Andersen, Will Drewry, bpf, kernel-hardening,
	linux-api, linux-security-module, netdev

On 05-Nov 11:34, Alexei Starovoitov wrote:
> On Tue, Nov 05, 2019 at 07:01:41PM +0100, Mickaël Salaün wrote:
> > 
> > On 05/11/2019 18:18, Alexei Starovoitov wrote:
> > > On Mon, Nov 04, 2019 at 06:21:43PM +0100, Mickaël Salaün wrote:
> > >> Add a first Landlock hook that can be used to enforce a security policy
> > >> or to audit some process activities.  For a sandboxing use-case, it is
> > >> needed to inform the kernel if a task can legitimately debug another.
> > >> ptrace(2) can also be used by an attacker to impersonate another task
> > >> and remain undetected while performing malicious activities.
> > >>
> > >> Using ptrace(2) and related features on a target process can lead to a
> > >> privilege escalation.  A sandboxed task must then be able to tell the
> > >> kernel if another task is more privileged, via ptrace_may_access().
> > >>
> > >> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > > ...
> > >> +static int check_ptrace(struct landlock_domain *domain,
> > >> +		struct task_struct *tracer, struct task_struct *tracee)
> > >> +{
> > >> +	struct landlock_hook_ctx_ptrace ctx_ptrace = {
> > >> +		.prog_ctx = {
> > >> +			.tracer = (uintptr_t)tracer,
> > >> +			.tracee = (uintptr_t)tracee,
> > >> +		},
> > >> +	};
> > > 
> > > So you're passing two kernel pointers obfuscated as u64 into bpf program
> > > yet claiming that the end goal is to make landlock unprivileged?!
> > > The most basic security hole in the tool that is aiming to provide security.
> > 
> > How could you used these pointers without dedicated BPF helpers? This
> > context items are typed as PTR_TO_TASK and can't be used without a
> > dedicated helper able to deal with ARG_PTR_TO_TASK. Moreover, pointer
> > arithmetic is explicitly forbidden (and I added tests for that). Did I
> > miss something?
> 
> It's a pointer leak.
> 
> > 
> > > 
> > > I think the only way bpf-based LSM can land is both landlock and KRSI
> > > developers work together on a design that solves all use cases.
> > 
> > As I said in a previous cover letter [1], that would be great. I think
> > that the current Landlock bases (almost everything from this series
> > except the seccomp interface) should meet both needs, but I would like
> > to have the point of view of the KRSI developers.
> > 
> > [1] https://lore.kernel.org/lkml/20191029171505.6650-1-mic@digikod.net/
> > 
> > > BPF is capable
> > > to be a superset of all existing LSMs whereas landlock and KRSI propsals today
> > > are custom solutions to specific security concerns. BPF subsystem was extended
> > > with custom things in the past. In networking we have lwt, skb, tc, xdp, sk
> > > program types with a lot of overlapping functionality. We couldn't figure out
> > > how to generalize them into single 'networking' program. Now we can and we
> > > should. Accepting two partially overlapping bpf-based LSMs would be repeating
> > > the same mistake again.
> > 
> > I'll let the LSM maintainers comment on whether BPF could be a superset
> > of all LSM, but given the complexity of an access-control system, I have
> > some doubts though. Anyway, we need to start somewhere and then iterate.
> > This patch series is a first step.
> 
> I would like KRSI folks to speak up. So far I don't see any sharing happening
> between landlock and KRSI. You're claiming this set is a first step. They're
> claiming the same about their patches. I'd like to set a patchset that was
> jointly developed.

We are willing to collaborate with the Landlock developers and come up
with a common approach that would work for Landlock and KRSI. I want
to mention that this collaboration and the current Landlock approach
of using an eBPF based LSM for unprivileged sandboxing only makes sense
if unprivileged usage of eBPF is going to be ever allowed.

Purely from a technical standpoint, both the current designs for
Landlock and KRSI target separate use cases and it would not be
possible to build "one on top of the other". We've tried to identify
the lowest denominator ("eBPF+LSM") requirements for both Landlock
(unprivileged sandboxing / Discretionary Access Control) and KRSI
(flexibility and unification of privileged MAC and Audit) and
prototyped an implementation based on the newly added / upcoming
features in BPF.

We've been successfully able to prototype the use cases for KRSI
(privileged MAC and Audit) using this "eBPF+LSM" and shared our
approach at the Linux Security Summit [1]:

* Use the new in-kernel BTF (CO-RE eBPF programs) [2] and the ability
  of the BPF verifier to use the BTF information for access validation
  to provide a more generic way to attach to the various LSM hooks.
  This potentially saves a lot of redundant work:

   - Creation of new program types.
   - Multiple types of contexts (or a single context with Unions).
   - Changes to the verifier and creation of new BPF argument types 
     (eg. PTR_TO_TASK)

* These new BPF features also alleviate the original concerns that we
  raised when initially proposing KRSI and designing for precise BPF
  helpers. We have some patches coming up which incorporate these new
  changes and will be sharing something on the mailing list after some
  cleanup.

We can use the common "eBPF+LSM" for both privileged MAC and Audit and
unprivileged sandboxing i.e. Discretionary Access Control.
Here's what it could look like:

* Common infrastructure allows attachment to all hooks which works well
  for privileged MAC and Audit. This could be extended to provide
  another attachment type for unprivileged DAC, which can restrict the
  hooks that can be attached to, and also the information that is
  exposed to the eBPF programs which is something that Landlock could
  build.

* This attachment could use the proposed landlock domains and attach to
  the task_struct providing the discretionary access control semantics.

[1] https://static.sched.com/hosted_files/lsseu2019/a2/Kernel%20Runtime%20Security%20Instrumentation.pdf
[2] http://vger.kernel.org/bpfconf2019_talks/bpf-core.pdf

- KP Singh

> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-05 18:01     ` Mickaël Salaün
  2019-11-05 19:34       ` Alexei Starovoitov
@ 2019-11-06 10:15       ` KP Singh
  2019-11-06 16:58         ` Mickaël Salaün
  1 sibling, 1 reply; 26+ messages in thread
From: KP Singh @ 2019-11-06 10:15 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Alexei Starovoitov, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Casey Schaufler, Daniel Borkmann,
	David Drysdale, Florent Revest, James Morris, Jann Horn,
	John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
	Mickaël Salaün, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
	Tetsuo Handa, Tycho Andersen, Will Drewry, bpf, kernel-hardening,
	linux-api, linux-security-module, netdev

On 05-Nov 19:01, Mickaël Salaün wrote:
> 
> On 05/11/2019 18:18, Alexei Starovoitov wrote:
> > On Mon, Nov 04, 2019 at 06:21:43PM +0100, Mickaël Salaün wrote:
> >> Add a first Landlock hook that can be used to enforce a security policy
> >> or to audit some process activities.  For a sandboxing use-case, it is
> >> needed to inform the kernel if a task can legitimately debug another.
> >> ptrace(2) can also be used by an attacker to impersonate another task
> >> and remain undetected while performing malicious activities.
> >>
> >> Using ptrace(2) and related features on a target process can lead to a
> >> privilege escalation.  A sandboxed task must then be able to tell the
> >> kernel if another task is more privileged, via ptrace_may_access().
> >>
> >> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ...
> >> +static int check_ptrace(struct landlock_domain *domain,
> >> +		struct task_struct *tracer, struct task_struct *tracee)
> >> +{
> >> +	struct landlock_hook_ctx_ptrace ctx_ptrace = {
> >> +		.prog_ctx = {
> >> +			.tracer = (uintptr_t)tracer,
> >> +			.tracee = (uintptr_t)tracee,
> >> +		},
> >> +	};
> > 
> > So you're passing two kernel pointers obfuscated as u64 into bpf program
> > yet claiming that the end goal is to make landlock unprivileged?!
> > The most basic security hole in the tool that is aiming to provide security.
> 
> How could you used these pointers without dedicated BPF helpers? This
> context items are typed as PTR_TO_TASK and can't be used without a
> dedicated helper able to deal with ARG_PTR_TO_TASK. Moreover, pointer
> arithmetic is explicitly forbidden (and I added tests for that). Did I
> miss something?
> 
> > 
> > I think the only way bpf-based LSM can land is both landlock and KRSI
> > developers work together on a design that solves all use cases.
> 
> As I said in a previous cover letter [1], that would be great. I think
> that the current Landlock bases (almost everything from this series
> except the seccomp interface) should meet both needs, but I would like
> to have the point of view of the KRSI developers.

As I mentioned we are willing to collaborate but the current landlock
patches does not meet the needs for KRSI:

* One program type per use-case (eg. LANDLOCK_PROG_PTRACE) as opposed to
  a single program type. This is something that KRSI proposed in it's
  initial design [1] and the new common "eBPF + LSM" based approach
  [2] would maintain as well.

* Landlock chooses to have multiple LSM hooks per landlock hook which is
  more restrictive. It's not easy to write precise MAC and Audit
  policies for a privileged LSM based on this and this ends up bloating
  the context that needs to be maintained and requires avoidable
  boilerplate work in the kernel.

[1] https://lore.kernel.org/patchwork/project/lkml/list/?series=410101
[2] https://lore.kernel.org/bpf/20191106100655.GA18815@chromium.org/T/#u

- KP Singh

> 
> [1] https://lore.kernel.org/lkml/20191029171505.6650-1-mic@digikod.net/
> 
> > BPF is capable
> > to be a superset of all existing LSMs whereas landlock and KRSI propsals today
> > are custom solutions to specific security concerns. BPF subsystem was extended
> > with custom things in the past. In networking we have lwt, skb, tc, xdp, sk
> > program types with a lot of overlapping functionality. We couldn't figure out
> > how to generalize them into single 'networking' program. Now we can and we
> > should. Accepting two partially overlapping bpf-based LSMs would be repeating
> > the same mistake again.
> 
> I'll let the LSM maintainers comment on whether BPF could be a superset
> of all LSM, but given the complexity of an access-control system, I have
> some doubts though. Anyway, we need to start somewhere and then iterate.
> This patch series is a first step.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-06 10:06         ` KP Singh
@ 2019-11-06 16:55           ` Mickaël Salaün
  2019-11-06 21:45             ` KP Singh
  0 siblings, 1 reply; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-06 16:55 UTC (permalink / raw)
  To: KP Singh, Alexei Starovoitov
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski,
	Casey Schaufler, Daniel Borkmann, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, Michael Kerrisk, Mickaël Salaün, Paul Moore,
	Sargun Dhillon, Serge E . Hallyn, Shuah Khan, Stephen Smalley,
	Tejun Heo, Tetsuo Handa, Tycho Andersen, Will Drewry, bpf,
	kernel-hardening, linux-api, linux-security-module, netdev,
	Casey Schaufler


On 06/11/2019 11:06, KP Singh wrote:
> On 05-Nov 11:34, Alexei Starovoitov wrote:
>> On Tue, Nov 05, 2019 at 07:01:41PM +0100, Mickaël Salaün wrote:
>>> On 05/11/2019 18:18, Alexei Starovoitov wrote:

[...]

>>>> I think the only way bpf-based LSM can land is both landlock and KRSI
>>>> developers work together on a design that solves all use cases.
>>>
>>> As I said in a previous cover letter [1], that would be great. I think
>>> that the current Landlock bases (almost everything from this series
>>> except the seccomp interface) should meet both needs, but I would like
>>> to have the point of view of the KRSI developers.
>>>
>>> [1] https://lore.kernel.org/lkml/20191029171505.6650-1-mic@digikod.net/
>>>
>>>> BPF is capable
>>>> to be a superset of all existing LSMs whereas landlock and KRSI propsals today
>>>> are custom solutions to specific security concerns. BPF subsystem was extended
>>>> with custom things in the past. In networking we have lwt, skb, tc, xdp, sk
>>>> program types with a lot of overlapping functionality. We couldn't figure out
>>>> how to generalize them into single 'networking' program. Now we can and we
>>>> should. Accepting two partially overlapping bpf-based LSMs would be repeating
>>>> the same mistake again.
>>>
>>> I'll let the LSM maintainers comment on whether BPF could be a superset
>>> of all LSM, but given the complexity of an access-control system, I have
>>> some doubts though. Anyway, we need to start somewhere and then iterate.
>>> This patch series is a first step.
>>
>> I would like KRSI folks to speak up. So far I don't see any sharing happening
>> between landlock and KRSI. You're claiming this set is a first step. They're
>> claiming the same about their patches. I'd like to set a patchset that was
>> jointly developed.
> 
> We are willing to collaborate with the Landlock developers and come up
> with a common approach that would work for Landlock and KRSI. I want
> to mention that this collaboration and the current Landlock approach
> of using an eBPF based LSM for unprivileged sandboxing only makes sense
> if unprivileged usage of eBPF is going to be ever allowed.

The ability to *potentially* do unprivileged sandboxing is definitely
not tied nor a blocker to the unprivileged usage of eBPF. As explained
in the documentation [1] (cf. Guiding principles / Unprivileged use),
Landlock is designed to be as safe as possible (from a security point of
view). The impact is more complex and important than just using
unprivileged eBPF, which may not be required. Unprivileged use of eBPF
would be nice, but I think the current direction is to extend the Linux
capabilities with one or multiple dedicated to eBPF [2] (e.g. CAP_BPF +
something else), which may be even better (and a huge difference with
CAP_SYS_ADMIN, a.k.a. privileged mode or root). Landlock is designed to
deal with unprivileged (i.e. non-root) use cases, but of course, if the
Landlock architecture may enable to do unprivileged stuff, it definitely
can do privileged stuff too. However, having an architecture designed
with safe unprivileged use in mind can't be achieve afterwards.

[1] https://lore.kernel.org/lkml/20191104172146.30797-8-mic@digikod.net/
[2] https://lore.kernel.org/bpf/20190827205213.456318-1-ast@kernel.org/


> 
> Purely from a technical standpoint, both the current designs for
> Landlock and KRSI target separate use cases and it would not be
> possible to build "one on top of the other". We've tried to identify
> the lowest denominator ("eBPF+LSM") requirements for both Landlock
> (unprivileged sandboxing / Discretionary Access Control) and KRSI
> (flexibility and unification of privileged MAC and Audit) and
> prototyped an implementation based on the newly added / upcoming
> features in BPF.

This is not as binary as that. Sandboxing can be seen as DAC but also as
MAC, depending on the subject which apply the security policy and the
subjects which are enforced by this policy. If the sandboxing is applied
system-wide, it is what we usually call MAC. DAC, in the Linux world,
enables any user to restrict access to their files to other users.

With Landlock it is not the same because a process can restrict itself
but also enforce these restrictions on all its future children (which
may be malicious, whatever their UID/GID). The threat and the definition
of the attacker are not the same in both cases. With the Linux DAC the
potentially malicious subjects are the other users, whereas with
Landlock the potentially malicious subjects are (for now) the current
process and all its children. Another way to explain it, and how
Landlock is designed, is that a specific enforcement (i.e. a set of BPF
programs) is tied to a domain, in which a set of subject are. From this
perspective, this approach (subjects/tasks in a domain) is orthogonal to
the DAC system (subjects/users). This design may apply to a system-wide
MAC system by putting all the system tasks in one domain, and managing
restrictions (by subject) with other means (e.g. task's UID,
command-line strings, environment variables). In short, Landlock (in
this patch series) is closer to a (potentially scoped) MAC system. But
thanks to eBPF, Landlock is firstly a programmatic access-control, which
means that the one who write the programs and tie them to a set of
tasks, can implement their own access-control system (e.g. RBAC,
time-based…), or something else (e.g. an audit system).

The audit part can simply be achieve with dedicated helpers and programs
that always allow accesses.

Landlock evolved over multiple iterations and is now designed to be very
flexible. The current way to enforce a security policy is to go through
the seccomp syscall (which makes sense for multiple reasons explained
and accepted before). But Landlock is designed to enable similar
enforcements (or audit) with other ways to define a domain (e.g. cgroups
[3], or system-wide securityfs as done in KRSI). Indeed, the only part
tied to this scoped enforcement is in the domain_syscall.c file. A new
file domain_fs.c could be added to implement a securityfs for a
system-wide enforcement (and have other features as KRSI does).

[3] https://lore.kernel.org/lkml/20160914072415.26021-17-mic@digikod.net/

One possible important difference between Landlock and KRSI right now is
the BPF program management. Both manage a list of programs per hook.
However KRSI needs to be able to replace a program in these lists. This
is not implemented in this Landlock patch series, first because it is
not the main use-case and it is safer to have an append-only way to add
restrictions (cf. seccomp-bpf model), and second because it is simpler
to deal with immutable lists. However, it is worth considering extending
the Landlock domain management with the ability to update the program
lists. One challenge may be to identify which program should be replaced
(which KRSI does with the program name). I think it would be wiser to
implement this in a second step though, maybe not for the syscall
interface (thanks to a new seccomp operation), but first with the
securityfs one.


> 
> We've been successfully able to prototype the use cases for KRSI
> (privileged MAC and Audit) using this "eBPF+LSM" and shared our
> approach at the Linux Security Summit [1]:
> 
> * Use the new in-kernel BTF (CO-RE eBPF programs) [2] and the ability
>   of the BPF verifier to use the BTF information for access validation
>   to provide a more generic way to attach to the various LSM hooks.
>   This potentially saves a lot of redundant work:
> 
>    - Creation of new program types.
>    - Multiple types of contexts (or a single context with Unions).
>    - Changes to the verifier and creation of new BPF argument types 
>      (eg. PTR_TO_TASK)

As I understood from the LSS talk, KRSI's approach is to use the same
hooks as LSM (cf. the securityfs). As said Alexei [4] "It must not make
LSM hooks into stable ABI".  Moveover, the LSM hooks may change
according to internal kernel evolution, and their semantic may not make
sense from a user space point of view. This is one reason for which
Lanlock abstract those hooks into something that is simpler and designed
to fit well with eBPF (program contexts and their attached types, as
explained in the documentation).

[4]
https://lore.kernel.org/lkml/20191105215453.szhdkrvuekwfz6le@ast-mbp.dhcp.thefacebook.com/

How does KRSI plan to deal with one LSM hook being split in two hooks in
a next version of the kernel (cf. [5])?

[5] https://lore.kernel.org/lkml/20190910115527.5235-6-kpsingh@chromium.org/


Another reason to have multiple different attach types/contexts (cf.
landlock_domain->programs[]) is to limit useless BPF program
interpretation (in addition to the non-system-wide scoped of programs).
 It also enables to handle and verify strict context use (which is also
explain in the Guiding principles). It would be a huge wast of time to
run every BPF programs for all LSM hooks. KRSI does the same but instead
of relying on the program type it rely on the list tied to the
securityfs file.

BTF is great, but as far as I know, it's goal is to easily deal with the
moving kernel ABI (e.g. task_struct layout, config/constant variables),
and it is definitely useful to programs using bpf_probe_read() and
similar accessors. However, I don't see how KRSI would avoid BPF types
thanks to BTF.

There is only one program type for Landlock (i.e.
BPF_PROG_TYPE_LANDLOCK_HOOK), and I don't see why adding new program
*attach* types (e.g. BPF_LANDLOCK_PTRACE) may be an issue. The kernel
will still need to be modified to implement new hooks and the new BPF
helpers anyway, BTF will not change that, except maybe if the internal
LSM API is exposed in a way or another to BPF (thanks to BTF), which
does not seem acceptable. Am I missing something?


The current KRSI approach is to allow a common set of helpers to be
called by all programs (because there is no way to differentiate them
with their type).
How KRSI would deal with kernel objects other than the current task
(e.g. a ptrace hook with a tracer and a tracee, a file open/read) with
the struct krsi_ctx unions [6]?

[6] https://lore.kernel.org/lkml/20190910115527.5235-7-kpsingh@chromium.org/


How does KRSI plan to deal with security blobs?


> 
> * These new BPF features also alleviate the original concerns that we
>   raised when initially proposing KRSI and designing for precise BPF
>   helpers. We have some patches coming up which incorporate these new
>   changes and will be sharing something on the mailing list after some
>   cleanup.
> 
> We can use the common "eBPF+LSM" for both privileged MAC and Audit and
> unprivileged sandboxing i.e. Discretionary Access Control.
> Here's what it could look like:
> 
> * Common infrastructure allows attachment to all hooks which works well
>   for privileged MAC and Audit. This could be extended to provide
>   another attachment type for unprivileged DAC, which can restrict the
>   hooks that can be attached to, and also the information that is
>   exposed to the eBPF programs which is something that Landlock could
>   build.

I agree that the "privileged-only" hooks should be a superset of the
"security-safe-and-potentially-unprivileged" hooks. :)
However, as said previously, I'm convinced it is a requirement to have
abstract hooks (and associated program attach types) as defined by Landlock.

I'm not sure what you mean by "the information that is exposed to the
eBPF program". Is it the current Landlock implementation of specific
contexts and attach types?

> 
> * This attachment could use the proposed landlock domains and attach to
>   the task_struct providing the discretionary access control semantics.

Not task_struct but creds, yes. This is a characteristic of sandboxing,
which may not be useful for the KRSI use case. It makes sense for KRSI
to attach program sets (or Landlock domains) to the whole system, then
using the creds does not make sense here. This difference is small and a
previous version of Landlock already validated this use case with
cgroups [3] (which is postponed to simplify the patch series).

[3] https://lore.kernel.org/lkml/20160914072415.26021-17-mic@digikod.net/


> 
> [1] https://static.sched.com/hosted_files/lsseu2019/a2/Kernel%20Runtime%20Security%20Instrumentation.pdf
> [2] http://vger.kernel.org/bpfconf2019_talks/bpf-core.pdf
> 
> - KP Singh

I think it should be OK to first land something close to this Landlock
patch series and then we could extend the domain management features and
add the securityfs support that KRSI needs. The main concern seems to be
about hook definitions.

Another approach would be to land Landlock and KRSI as distinct LSM
while trying as much as possible to mutualize code/helpers.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-06 10:15       ` KP Singh
@ 2019-11-06 16:58         ` Mickaël Salaün
  0 siblings, 0 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-06 16:58 UTC (permalink / raw)
  To: KP Singh
  Cc: Alexei Starovoitov, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Casey Schaufler, Daniel Borkmann,
	David Drysdale, Florent Revest, James Morris, Jann Horn,
	John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
	Mickaël Salaün, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
	Tetsuo Handa, Tycho Andersen, Will Drewry, bpf, kernel-hardening,
	linux-api, linux-security-module, netdev


On 06/11/2019 11:15, KP Singh wrote:
> On 05-Nov 19:01, Mickaël Salaün wrote:
>> On 05/11/2019 18:18, Alexei Starovoitov wrote:

[...]

>>>
>>> I think the only way bpf-based LSM can land is both landlock and KRSI
>>> developers work together on a design that solves all use cases.
>>
>> As I said in a previous cover letter [1], that would be great. I think
>> that the current Landlock bases (almost everything from this series
>> except the seccomp interface) should meet both needs, but I would like
>> to have the point of view of the KRSI developers.
> 
> As I mentioned we are willing to collaborate but the current landlock
> patches does not meet the needs for KRSI:
> 
> * One program type per use-case (eg. LANDLOCK_PROG_PTRACE) as opposed to
>   a single program type. This is something that KRSI proposed in it's
>   initial design [1] and the new common "eBPF + LSM" based approach
>   [2] would maintain as well.

As ask in my previous email [1], I don't see how KRSI would efficiently
deal with other LSM hooks with a unique program (attach) type.

[1]
https://lore.kernel.org/lkml/813cedde-8ed7-2d3b-883d-909efa978d41@digikod.net/

> 
> * Landlock chooses to have multiple LSM hooks per landlock hook which is
>   more restrictive. It's not easy to write precise MAC and Audit
>   policies for a privileged LSM based on this and this ends up bloating
>   the context that needs to be maintained and requires avoidable
>   boilerplate work in the kernel.

Why do you think it is more restrictive or it adds boilerplate work? How
does KRSI will deal with more complex hooks than execve-like with
multiple kernel objects?


> 
> [1] https://lore.kernel.org/patchwork/project/lkml/list/?series=410101
> [2] https://lore.kernel.org/bpf/20191106100655.GA18815@chromium.org/T/#u
> 
> - KP Singh

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-06 16:55           ` Mickaël Salaün
@ 2019-11-06 21:45             ` KP Singh
  2019-11-08 14:08               ` Mickaël Salaün
  0 siblings, 1 reply; 26+ messages in thread
From: KP Singh @ 2019-11-06 21:45 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Alexei Starovoitov, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Casey Schaufler, Daniel Borkmann,
	David Drysdale, Florent Revest, James Morris, Jann Horn,
	John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
	Mickaël Salaün, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
	Tetsuo Handa, Tycho Andersen, Will Drewry, bpf, kernel-hardening,
	linux-api, linux-security-module, netdev

On 06-Nov 17:55, Mickaël Salaün wrote:
> 
> On 06/11/2019 11:06, KP Singh wrote:
> > On 05-Nov 11:34, Alexei Starovoitov wrote:
> >> On Tue, Nov 05, 2019 at 07:01:41PM +0100, Mickaël Salaün wrote:
> >>> On 05/11/2019 18:18, Alexei Starovoitov wrote:
> 
> [...]
> 
> >>>> I think the only way bpf-based LSM can land is both landlock and KRSI
> >>>> developers work together on a design that solves all use cases.
> >>>
> >>> As I said in a previous cover letter [1], that would be great. I think
> >>> that the current Landlock bases (almost everything from this series
> >>> except the seccomp interface) should meet both needs, but I would like
> >>> to have the point of view of the KRSI developers.
> >>>
> >>> [1] https://lore.kernel.org/lkml/20191029171505.6650-1-mic@digikod.net/
> >>>
> >>>> BPF is capable
> >>>> to be a superset of all existing LSMs whereas landlock and KRSI propsals today
> >>>> are custom solutions to specific security concerns. BPF subsystem was extended
> >>>> with custom things in the past. In networking we have lwt, skb, tc, xdp, sk
> >>>> program types with a lot of overlapping functionality. We couldn't figure out
> >>>> how to generalize them into single 'networking' program. Now we can and we
> >>>> should. Accepting two partially overlapping bpf-based LSMs would be repeating
> >>>> the same mistake again.
> >>>
> >>> I'll let the LSM maintainers comment on whether BPF could be a superset
> >>> of all LSM, but given the complexity of an access-control system, I have
> >>> some doubts though. Anyway, we need to start somewhere and then iterate.
> >>> This patch series is a first step.
> >>
> >> I would like KRSI folks to speak up. So far I don't see any sharing happening
> >> between landlock and KRSI. You're claiming this set is a first step. They're
> >> claiming the same about their patches. I'd like to set a patchset that was
> >> jointly developed.
> > 
> > We are willing to collaborate with the Landlock developers and come up
> > with a common approach that would work for Landlock and KRSI. I want
> > to mention that this collaboration and the current Landlock approach
> > of using an eBPF based LSM for unprivileged sandboxing only makes sense
> > if unprivileged usage of eBPF is going to be ever allowed.
> 
> The ability to *potentially* do unprivileged sandboxing is definitely
> not tied nor a blocker to the unprivileged usage of eBPF. As explained
> in the documentation [1] (cf. Guiding principles / Unprivileged use),
> Landlock is designed to be as safe as possible (from a security point of
> view). The impact is more complex and important than just using
> unprivileged eBPF, which may not be required. Unprivileged use of eBPF
> would be nice, but I think the current direction is to extend the Linux
> capabilities with one or multiple dedicated to eBPF [2] (e.g. CAP_BPF +
> something else), which may be even better (and a huge difference with
> CAP_SYS_ADMIN, a.k.a. privileged mode or root). Landlock is designed to
> deal with unprivileged (i.e. non-root) use cases, but of course, if the
> Landlock architecture may enable to do unprivileged stuff, it definitely
> can do privileged stuff too. However, having an architecture designed
> with safe unprivileged use in mind can't be achieve afterwards.
> 
> [1] https://lore.kernel.org/lkml/20191104172146.30797-8-mic@digikod.net/
> [2] https://lore.kernel.org/bpf/20190827205213.456318-1-ast@kernel.org/
> 
> 
> > 
> > Purely from a technical standpoint, both the current designs for
> > Landlock and KRSI target separate use cases and it would not be
> > possible to build "one on top of the other". We've tried to identify
> > the lowest denominator ("eBPF+LSM") requirements for both Landlock
> > (unprivileged sandboxing / Discretionary Access Control) and KRSI
> > (flexibility and unification of privileged MAC and Audit) and
> > prototyped an implementation based on the newly added / upcoming
> > features in BPF.
> 
> This is not as binary as that. Sandboxing can be seen as DAC but also as
> MAC, depending on the subject which apply the security policy and the
> subjects which are enforced by this policy. If the sandboxing is applied
> system-wide, it is what we usually call MAC. DAC, in the Linux world,
> enables any user to restrict access to their files to other users.
> 
> With Landlock it is not the same because a process can restrict itself
> but also enforce these restrictions on all its future children (which
> may be malicious, whatever their UID/GID). The threat and the definition
> of the attacker are not the same in both cases. With the Linux DAC the
> potentially malicious subjects are the other users, whereas with
> Landlock the potentially malicious subjects are (for now) the current
> process and all its children. Another way to explain it, and how
> Landlock is designed, is that a specific enforcement (i.e. a set of BPF
> programs) is tied to a domain, in which a set of subject are. From this
> perspective, this approach (subjects/tasks in a domain) is orthogonal to
> the DAC system (subjects/users). This design may apply to a system-wide
> MAC system by putting all the system tasks in one domain, and managing
> restrictions (by subject) with other means (e.g. task's UID,
> command-line strings, environment variables). In short, Landlock (in
> this patch series) is closer to a (potentially scoped) MAC system. But
> thanks to eBPF, Landlock is firstly a programmatic access-control, which
> means that the one who write the programs and tie them to a set of
> tasks, can implement their own access-control system (e.g. RBAC,
> time-based…), or something else (e.g. an audit system).
> 
> The audit part can simply be achieve with dedicated helpers and programs
> that always allow accesses.
> 
> Landlock evolved over multiple iterations and is now designed to be very
> flexible. The current way to enforce a security policy is to go through
> the seccomp syscall (which makes sense for multiple reasons explained
> and accepted before). But Landlock is designed to enable similar
> enforcements (or audit) with other ways to define a domain (e.g. cgroups
> [3], or system-wide securityfs as done in KRSI). Indeed, the only part
> tied to this scoped enforcement is in the domain_syscall.c file. A new
> file domain_fs.c could be added to implement a securityfs for a
> system-wide enforcement (and have other features as KRSI does).
> 

Given the current way landlock exposes LSM hooks, I don't think it's
possible to build system-wide detections. But let’s try to come to a
consensus on the semantics of the how the LSM hooks are exposed to
BPF. At the moment I think we should:


* Bring the core interface exposed to eBPF closer to the LSM surface in
  a way that supports both use cases. One way Landlock can still provide
  a more abstract interface is by providing some BPF helper libraries
  that build on top of the core framework.

* Use a single BPF program type; this is necessary for a key requirement
  of KRSI, i.e. runtime instrumentation. The upcoming prototype should
  illustrate how this works for KRSI - note that it’s possible to vary
  the context types exposed by different hooks.


It would be nice to get the BPF maintainers’ opinion on these points.


> [3] https://lore.kernel.org/lkml/20160914072415.26021-17-mic@digikod.net/
> 
> One possible important difference between Landlock and KRSI right now is
> the BPF program management. Both manage a list of programs per hook.
> However KRSI needs to be able to replace a program in these lists. This
> is not implemented in this Landlock patch series, first because it is
> not the main use-case and it is safer to have an append-only way to add
> restrictions (cf. seccomp-bpf model), and second because it is simpler
> to deal with immutable lists. However, it is worth considering extending
> the Landlock domain management with the ability to update the program
> lists. One challenge may be to identify which program should be replaced
> (which KRSI does with the program name). I think it would be wiser to
> implement this in a second step though, maybe not for the syscall
> interface (thanks to a new seccomp operation), but first with the
> securityfs one.
> 
> 
> > 
> > We've been successfully able to prototype the use cases for KRSI
> > (privileged MAC and Audit) using this "eBPF+LSM" and shared our
> > approach at the Linux Security Summit [1]:
> > 
> > * Use the new in-kernel BTF (CO-RE eBPF programs) [2] and the ability
> >   of the BPF verifier to use the BTF information for access validation
> >   to provide a more generic way to attach to the various LSM hooks.
> >   This potentially saves a lot of redundant work:
> > 
> >    - Creation of new program types.
> >    - Multiple types of contexts (or a single context with Unions).
> >    - Changes to the verifier and creation of new BPF argument types 
> >      (eg. PTR_TO_TASK)
> 
> As I understood from the LSS talk, KRSI's approach is to use the same
> hooks as LSM (cf. the securityfs). As said Alexei [4] "It must not make
> LSM hooks into stable ABI".  Moveover, the LSM hooks may change
> according to internal kernel evolution, and their semantic may not make

I think you misunderstand Alexei here. I will let him elaborate.

> sense from a user space point of view. This is one reason for which
> Lanlock abstract those hooks into something that is simpler and designed
> to fit well with eBPF (program contexts and their attached types, as
> explained in the documentation).
> 
> [4]
> https://lore.kernel.org/lkml/20191105215453.szhdkrvuekwfz6le@ast-mbp.dhcp.thefacebook.com/
> 
> How does KRSI plan to deal with one LSM hook being split in two hooks in
> a next version of the kernel (cf. [5])?

How often has that happened in the past? And even if it does happen,
it can still be handled as a part of the base framework we are trying
to implement.

> 
> [5] https://lore.kernel.org/lkml/20190910115527.5235-6-kpsingh@chromium.org/
> 
> 
> Another reason to have multiple different attach types/contexts (cf.
> landlock_domain->programs[]) is to limit useless BPF program
> interpretation (in addition to the non-system-wide scoped of programs).
>  It also enables to handle and verify strict context use (which is also
> explain in the Guiding principles). It would be a huge wast of time to
> run every BPF programs for all LSM hooks. KRSI does the same but instead
> of relying on the program type it rely on the list tied to the
> securityfs file.
> 
> BTF is great, but as far as I know, it's goal is to easily deal with the
> moving kernel ABI (e.g. task_struct layout, config/constant variables),
> and it is definitely useful to programs using bpf_probe_read() and
> similar accessors. However, I don't see how KRSI would avoid BPF types
> thanks to BTF.
> 

This should become clearer once we post our updated patch-set. Do note
that I am currently traveling and will be away for the next couple of
weeks.

> There is only one program type for Landlock (i.e.
> BPF_PROG_TYPE_LANDLOCK_HOOK), and I don't see why adding new program
> *attach* types (e.g. BPF_LANDLOCK_PTRACE) may be an issue. The kernel
> will still need to be modified to implement new hooks and the new BPF
> helpers anyway, BTF will not change that, except maybe if the internal
> LSM API is exposed in a way or another to BPF (thanks to BTF), which
> does not seem acceptable. Am I missing something?
> 
> 
> The current KRSI approach is to allow a common set of helpers to be
> called by all programs (because there is no way to differentiate them
> with their type).
> How KRSI would deal with kernel objects other than the current task
> (e.g. a ptrace hook with a tracer and a tracee, a file open/read) with
> the struct krsi_ctx unions [6]?
> 
> [6] https://lore.kernel.org/lkml/20190910115527.5235-7-kpsingh@chromium.org/
> 

The best part of BTF is that it can provide a common way to pass
different contexts to the various attachments points and the verifier
can use the BTF information to validate accesses which essentially
allows us to change the helpers from:

       is_running_executable(magical_krsi_ctx)

          to

       is_running_executable(inode)


which can work on any inode (ARG_PTR_TO_BTF_ID = btf_id(struct inode))

This makes the helpers much more useful and generic. All this is
better explained in our upcoming patch-set.

> 
> How does KRSI plan to deal with security blobs?

The new prototype uses security blobs but does not expose them to
user-space. These blobs are then used in various helpers like
“is_running_executable” which uses blobs on the inode and the
task_struct. This should become clearer when the next patchset is
posted.

I don’t think it’s currently possible to allow the blobs to be set
using eBPF programs with the main reason being that the blob will only
be set after the program is loaded. The answer to
“is_running_executable” becomes dependent on whether the file was
executed before the blob setting eBPF program was loaded.

Blob management with eBPF is not possible unless we can load eBPF
programs that can set blobs at boot-time.
In short, the next KRSI version will not give eBPF
programs access to arbitrarily write security blobs.

> 
> 
> > 
> > * These new BPF features also alleviate the original concerns that we
> >   raised when initially proposing KRSI and designing for precise BPF
> >   helpers. We have some patches coming up which incorporate these new
> >   changes and will be sharing something on the mailing list after some
> >   cleanup.
> > 
> > We can use the common "eBPF+LSM" for both privileged MAC and Audit and
> > unprivileged sandboxing i.e. Discretionary Access Control.
> > Here's what it could look like:
> > 
> > * Common infrastructure allows attachment to all hooks which works well
> >   for privileged MAC and Audit. This could be extended to provide
> >   another attachment type for unprivileged DAC, which can restrict the
> >   hooks that can be attached to, and also the information that is
> >   exposed to the eBPF programs which is something that Landlock could
> >   build.
> 
> I agree that the "privileged-only" hooks should be a superset of the
> "security-safe-and-potentially-unprivileged" hooks. :)
> However, as said previously, I'm convinced it is a requirement to have
> abstract hooks (and associated program attach types) as defined by Landlock.

I would like to hear the BPF maintainers’ perspective on this. I am
not sure they agree with you here.

- KP Singh

> 
> I'm not sure what you mean by "the information that is exposed to the
> eBPF program". Is it the current Landlock implementation of specific
> contexts and attach types?
> 
> > 
> > * This attachment could use the proposed landlock domains and attach to
> >   the task_struct providing the discretionary access control semantics.
> 
> Not task_struct but creds, yes. This is a characteristic of sandboxing,
> which may not be useful for the KRSI use case. It makes sense for KRSI
> to attach program sets (or Landlock domains) to the whole system, then
> using the creds does not make sense here. This difference is small and a
> previous version of Landlock already validated this use case with
> cgroups [3] (which is postponed to simplify the patch series).
> 
> [3] https://lore.kernel.org/lkml/20160914072415.26021-17-mic@digikod.net/
> 
> 
> > 
> > [1] https://static.sched.com/hosted_files/lsseu2019/a2/Kernel%20Runtime%20Security%20Instrumentation.pdf
> > [2] http://vger.kernel.org/bpfconf2019_talks/bpf-core.pdf
> > 
> > - KP Singh
> 
> I think it should be OK to first land something close to this Landlock
> patch series and then we could extend the domain management features and
> add the securityfs support that KRSI needs. The main concern seems to be
> about hook definitions.
> 
> Another approach would be to land Landlock and KRSI as distinct LSM
> while trying as much as possible to mutualize code/helpers.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-06 21:45             ` KP Singh
@ 2019-11-08 14:08               ` Mickaël Salaün
  2019-11-08 14:34                 ` Daniel Borkmann
  2019-11-08 15:27                 ` KP Singh
  0 siblings, 2 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-08 14:08 UTC (permalink / raw)
  To: KP Singh
  Cc: Alexei Starovoitov, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Casey Schaufler, Daniel Borkmann,
	David Drysdale, Florent Revest, James Morris, Jann Horn,
	John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
	Mickaël Salaün, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
	Tetsuo Handa, Tycho Andersen, Will Drewry, bpf, kernel-hardening,
	linux-api, linux-security-module, netdev


On 06/11/2019 22:45, KP Singh wrote:
> On 06-Nov 17:55, Mickaël Salaün wrote:
>>
>> On 06/11/2019 11:06, KP Singh wrote:
>>> On 05-Nov 11:34, Alexei Starovoitov wrote:
>>>> On Tue, Nov 05, 2019 at 07:01:41PM +0100, Mickaël Salaün wrote:
>>>>> On 05/11/2019 18:18, Alexei Starovoitov wrote:
>>
>> [...]
>>
>>>>>> I think the only way bpf-based LSM can land is both landlock and KRSI
>>>>>> developers work together on a design that solves all use cases.
>>>>>
>>>>> As I said in a previous cover letter [1], that would be great. I think
>>>>> that the current Landlock bases (almost everything from this series
>>>>> except the seccomp interface) should meet both needs, but I would like
>>>>> to have the point of view of the KRSI developers.
>>>>>
>>>>> [1] https://lore.kernel.org/lkml/20191029171505.6650-1-mic@digikod.net/
>>>>>
>>>>>> BPF is capable
>>>>>> to be a superset of all existing LSMs whereas landlock and KRSI propsals today
>>>>>> are custom solutions to specific security concerns. BPF subsystem was extended
>>>>>> with custom things in the past. In networking we have lwt, skb, tc, xdp, sk
>>>>>> program types with a lot of overlapping functionality. We couldn't figure out
>>>>>> how to generalize them into single 'networking' program. Now we can and we
>>>>>> should. Accepting two partially overlapping bpf-based LSMs would be repeating
>>>>>> the same mistake again.
>>>>>
>>>>> I'll let the LSM maintainers comment on whether BPF could be a superset
>>>>> of all LSM, but given the complexity of an access-control system, I have
>>>>> some doubts though. Anyway, we need to start somewhere and then iterate.
>>>>> This patch series is a first step.
>>>>
>>>> I would like KRSI folks to speak up. So far I don't see any sharing happening
>>>> between landlock and KRSI. You're claiming this set is a first step. They're
>>>> claiming the same about their patches. I'd like to set a patchset that was
>>>> jointly developed.
>>>
>>> We are willing to collaborate with the Landlock developers and come up
>>> with a common approach that would work for Landlock and KRSI. I want
>>> to mention that this collaboration and the current Landlock approach
>>> of using an eBPF based LSM for unprivileged sandboxing only makes sense
>>> if unprivileged usage of eBPF is going to be ever allowed.
>>
>> The ability to *potentially* do unprivileged sandboxing is definitely
>> not tied nor a blocker to the unprivileged usage of eBPF. As explained
>> in the documentation [1] (cf. Guiding principles / Unprivileged use),
>> Landlock is designed to be as safe as possible (from a security point of
>> view). The impact is more complex and important than just using
>> unprivileged eBPF, which may not be required. Unprivileged use of eBPF
>> would be nice, but I think the current direction is to extend the Linux
>> capabilities with one or multiple dedicated to eBPF [2] (e.g. CAP_BPF +
>> something else), which may be even better (and a huge difference with
>> CAP_SYS_ADMIN, a.k.a. privileged mode or root). Landlock is designed to
>> deal with unprivileged (i.e. non-root) use cases, but of course, if the
>> Landlock architecture may enable to do unprivileged stuff, it definitely
>> can do privileged stuff too. However, having an architecture designed
>> with safe unprivileged use in mind can't be achieve afterwards.
>>
>> [1] https://lore.kernel.org/lkml/20191104172146.30797-8-mic@digikod.net/
>> [2] https://lore.kernel.org/bpf/20190827205213.456318-1-ast@kernel.org/
>>
>>
>>>
>>> Purely from a technical standpoint, both the current designs for
>>> Landlock and KRSI target separate use cases and it would not be
>>> possible to build "one on top of the other". We've tried to identify
>>> the lowest denominator ("eBPF+LSM") requirements for both Landlock
>>> (unprivileged sandboxing / Discretionary Access Control) and KRSI
>>> (flexibility and unification of privileged MAC and Audit) and
>>> prototyped an implementation based on the newly added / upcoming
>>> features in BPF.
>>
>> This is not as binary as that. Sandboxing can be seen as DAC but also as
>> MAC, depending on the subject which apply the security policy and the
>> subjects which are enforced by this policy. If the sandboxing is applied
>> system-wide, it is what we usually call MAC. DAC, in the Linux world,
>> enables any user to restrict access to their files to other users.
>>
>> With Landlock it is not the same because a process can restrict itself
>> but also enforce these restrictions on all its future children (which
>> may be malicious, whatever their UID/GID). The threat and the definition
>> of the attacker are not the same in both cases. With the Linux DAC the
>> potentially malicious subjects are the other users, whereas with
>> Landlock the potentially malicious subjects are (for now) the current
>> process and all its children. Another way to explain it, and how
>> Landlock is designed, is that a specific enforcement (i.e. a set of BPF
>> programs) is tied to a domain, in which a set of subject are. From this
>> perspective, this approach (subjects/tasks in a domain) is orthogonal to
>> the DAC system (subjects/users). This design may apply to a system-wide
>> MAC system by putting all the system tasks in one domain, and managing
>> restrictions (by subject) with other means (e.g. task's UID,
>> command-line strings, environment variables). In short, Landlock (in
>> this patch series) is closer to a (potentially scoped) MAC system. But
>> thanks to eBPF, Landlock is firstly a programmatic access-control, which
>> means that the one who write the programs and tie them to a set of
>> tasks, can implement their own access-control system (e.g. RBAC,
>> time-based…), or something else (e.g. an audit system).
>>
>> The audit part can simply be achieve with dedicated helpers and programs
>> that always allow accesses.
>>
>> Landlock evolved over multiple iterations and is now designed to be very
>> flexible. The current way to enforce a security policy is to go through
>> the seccomp syscall (which makes sense for multiple reasons explained
>> and accepted before). But Landlock is designed to enable similar
>> enforcements (or audit) with other ways to define a domain (e.g. cgroups
>> [3], or system-wide securityfs as done in KRSI). Indeed, the only part
>> tied to this scoped enforcement is in the domain_syscall.c file. A new
>> file domain_fs.c could be added to implement a securityfs for a
>> system-wide enforcement (and have other features as KRSI does).
>>
> 
> Given the current way landlock exposes LSM hooks, I don't think it's
> possible to build system-wide detections.

Why ?


> But let’s try to come to a
> consensus on the semantics of the how the LSM hooks are exposed to
> BPF. At the moment I think we should:
> 
> 
> * Bring the core interface exposed to eBPF closer to the LSM surface in
>   a way that supports both use cases. One way Landlock can still provide
>   a more abstract interface is by providing some BPF helper libraries
>   that build on top of the core framework.

I still don't get why you think it is the only way or the better. I gave
a lot of arguments and I explained why Landlock is designed the way it
is, especially in the documentation (Guiding principles). Is there
something similar for KRSI?


> 
> * Use a single BPF program type; this is necessary for a key requirement
>   of KRSI, i.e. runtime instrumentation. The upcoming prototype should
>   illustrate how this works for KRSI - note that it’s possible to vary
>   the context types exposed by different hooks.

Why a single BPF program type? Do you mean *attach* types? Landlock only
use one program type, but will use multiple attach types.

Why do you think it is necessary for KRSI or for runtime instrumentation?

If it is justified, it could be a dedicated program attach type (e.g.
BPF_LANDLOCK_INTROSPECTION).

What is the advantage to have the possibility to vary the context types
over dedicated *typed* contexts? I don't see any advantages, but at
least one main drawback: to require runtime checks (when helpers use
this generic context) instead of load time checks (thanks to static type
checking of the context).


> It would be nice to get the BPF maintainers’ opinion on these points.
> 
> 
>> [3] https://lore.kernel.org/lkml/20160914072415.26021-17-mic@digikod.net/
>>
>> One possible important difference between Landlock and KRSI right now is
>> the BPF program management. Both manage a list of programs per hook.
>> However KRSI needs to be able to replace a program in these lists. This
>> is not implemented in this Landlock patch series, first because it is
>> not the main use-case and it is safer to have an append-only way to add
>> restrictions (cf. seccomp-bpf model), and second because it is simpler
>> to deal with immutable lists. However, it is worth considering extending
>> the Landlock domain management with the ability to update the program
>> lists. One challenge may be to identify which program should be replaced
>> (which KRSI does with the program name). I think it would be wiser to
>> implement this in a second step though, maybe not for the syscall
>> interface (thanks to a new seccomp operation), but first with the
>> securityfs one.
>>
>>
>>>
>>> We've been successfully able to prototype the use cases for KRSI
>>> (privileged MAC and Audit) using this "eBPF+LSM" and shared our
>>> approach at the Linux Security Summit [1]:
>>>
>>> * Use the new in-kernel BTF (CO-RE eBPF programs) [2] and the ability
>>>   of the BPF verifier to use the BTF information for access validation
>>>   to provide a more generic way to attach to the various LSM hooks.
>>>   This potentially saves a lot of redundant work:
>>>
>>>    - Creation of new program types.
>>>    - Multiple types of contexts (or a single context with Unions).
>>>    - Changes to the verifier and creation of new BPF argument types 
>>>      (eg. PTR_TO_TASK)
>>
>> As I understood from the LSS talk, KRSI's approach is to use the same
>> hooks as LSM (cf. the securityfs). As said Alexei [4] "It must not make
>> LSM hooks into stable ABI".  Moveover, the LSM hooks may change
>> according to internal kernel evolution, and their semantic may not make
> 
> I think you misunderstand Alexei here. I will let him elaborate.
> 
>> sense from a user space point of view. This is one reason for which
>> Lanlock abstract those hooks into something that is simpler and designed
>> to fit well with eBPF (program contexts and their attached types, as
>> explained in the documentation).
>>
>> [4]
>> https://lore.kernel.org/lkml/20191105215453.szhdkrvuekwfz6le@ast-mbp.dhcp.thefacebook.com/
>>
>> How does KRSI plan to deal with one LSM hook being split in two hooks in
>> a next version of the kernel (cf. [5])?
> 
> How often has that happened in the past? And even if it does happen,
> it can still be handled as a part of the base framework we are trying
> to implement.

I guess the security maintainers should have an opinion on this.

I don't clearly see the properties of this base framework. Could this be
elaborated?


>> [5] https://lore.kernel.org/lkml/20190910115527.5235-6-kpsingh@chromium.org/
>>
>>
>> Another reason to have multiple different attach types/contexts (cf.
>> landlock_domain->programs[]) is to limit useless BPF program
>> interpretation (in addition to the non-system-wide scoped of programs).
>>  It also enables to handle and verify strict context use (which is also
>> explain in the Guiding principles). It would be a huge wast of time to
>> run every BPF programs for all LSM hooks. KRSI does the same but instead
>> of relying on the program type it rely on the list tied to the
>> securityfs file.
>>
>> BTF is great, but as far as I know, it's goal is to easily deal with the
>> moving kernel ABI (e.g. task_struct layout, config/constant variables),
>> and it is definitely useful to programs using bpf_probe_read() and
>> similar accessors. However, I don't see how KRSI would avoid BPF types
>> thanks to BTF.
>>
> 
> This should become clearer once we post our updated patch-set. Do note
> that I am currently traveling and will be away for the next couple of
> weeks.
> 
>> There is only one program type for Landlock (i.e.
>> BPF_PROG_TYPE_LANDLOCK_HOOK), and I don't see why adding new program
>> *attach* types (e.g. BPF_LANDLOCK_PTRACE) may be an issue. The kernel
>> will still need to be modified to implement new hooks and the new BPF
>> helpers anyway, BTF will not change that, except maybe if the internal
>> LSM API is exposed in a way or another to BPF (thanks to BTF), which
>> does not seem acceptable. Am I missing something?
>>
>>
>> The current KRSI approach is to allow a common set of helpers to be
>> called by all programs (because there is no way to differentiate them
>> with their type).
>> How KRSI would deal with kernel objects other than the current task
>> (e.g. a ptrace hook with a tracer and a tracee, a file open/read) with
>> the struct krsi_ctx unions [6]?
>>
>> [6] https://lore.kernel.org/lkml/20190910115527.5235-7-kpsingh@chromium.org/
>>
> 
> The best part of BTF is that it can provide a common way to pass
> different contexts to the various attachments points and the verifier
> can use the BTF information to validate accesses which essentially
> allows us to change the helpers from:
> 
>        is_running_executable(magical_krsi_ctx)
> 
>           to
> 
>        is_running_executable(inode)
> 
> 
> which can work on any inode (ARG_PTR_TO_BTF_ID = btf_id(struct inode))
> 
> This makes the helpers much more useful and generic. All this is
> better explained in our upcoming patch-set.

I get the usefulness of BTF for future helper evolution and for moving
kernel API, but again, I don't see advantages over static typing check
of (well abstract/generic) kernel object handles in the context.

For instance, how BTF would replace the current BPF_LANDLOCK_PTRACE
context and the associated helper?

Does this mean that the KRSI v1 design is outdated and superseded by the
(future) v2 because of the more important use of BTF?


>> How does KRSI plan to deal with security blobs?
> 
> The new prototype uses security blobs but does not expose them to
> user-space. These blobs are then used in various helpers like
> “is_running_executable” which uses blobs on the inode and the
> task_struct. This should become clearer when the next patchset is
> posted.
> 
> I don’t think it’s currently possible to allow the blobs to be set
> using eBPF programs with the main reason being that the blob will only
> be set after the program is loaded. The answer to
> “is_running_executable” becomes dependent on whether the file was
> executed before the blob setting eBPF program was loaded.
> 
> Blob management with eBPF is not possible unless we can load eBPF
> programs that can set blobs at boot-time.
> In short, the next KRSI version will not give eBPF
> programs access to arbitrarily write security blobs.

A previous version of Landlock enabled programs to tag inodes (cf.
FS_GET): https://lore.kernel.org/lkml/20180227004121.3633-8-mic@digikod.net/


>>>
>>> * These new BPF features also alleviate the original concerns that we
>>>   raised when initially proposing KRSI and designing for precise BPF
>>>   helpers. We have some patches coming up which incorporate these new
>>>   changes and will be sharing something on the mailing list after some
>>>   cleanup.
>>>
>>> We can use the common "eBPF+LSM" for both privileged MAC and Audit and
>>> unprivileged sandboxing i.e. Discretionary Access Control.
>>> Here's what it could look like:
>>>
>>> * Common infrastructure allows attachment to all hooks which works well
>>>   for privileged MAC and Audit. This could be extended to provide
>>>   another attachment type for unprivileged DAC, which can restrict the
>>>   hooks that can be attached to, and also the information that is
>>>   exposed to the eBPF programs which is something that Landlock could
>>>   build.
>>
>> I agree that the "privileged-only" hooks should be a superset of the
>> "security-safe-and-potentially-unprivileged" hooks. :)
>> However, as said previously, I'm convinced it is a requirement to have
>> abstract hooks (and associated program attach types) as defined by Landlock.
> 
> I would like to hear the BPF maintainers’ perspective on this. I am
> not sure they agree with you here.
> 
> - KP Singh
> 
>>
>> I'm not sure what you mean by "the information that is exposed to the
>> eBPF program". Is it the current Landlock implementation of specific
>> contexts and attach types?

…or does BTF could magically solve this?


>>
>>>
>>> * This attachment could use the proposed landlock domains and attach to
>>>   the task_struct providing the discretionary access control semantics.
>>
>> Not task_struct but creds, yes. This is a characteristic of sandboxing,
>> which may not be useful for the KRSI use case. It makes sense for KRSI
>> to attach program sets (or Landlock domains) to the whole system, then
>> using the creds does not make sense here. This difference is small and a
>> previous version of Landlock already validated this use case with
>> cgroups [3] (which is postponed to simplify the patch series).
>>
>> [3] https://lore.kernel.org/lkml/20160914072415.26021-17-mic@digikod.net/
>>
>>
>>>
>>> [1] https://static.sched.com/hosted_files/lsseu2019/a2/Kernel%20Runtime%20Security%20Instrumentation.pdf
>>> [2] http://vger.kernel.org/bpfconf2019_talks/bpf-core.pdf
>>>
>>> - KP Singh
>>
>> I think it should be OK to first land something close to this Landlock
>> patch series and then we could extend the domain management features and
>> add the securityfs support that KRSI needs. The main concern seems to be
>> about hook definitions.
>>
>> Another approach would be to land Landlock and KRSI as distinct LSM
>> while trying as much as possible to mutualize code/helpers.
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-08 14:08               ` Mickaël Salaün
@ 2019-11-08 14:34                 ` Daniel Borkmann
  2019-11-08 15:39                   ` Mickaël Salaün
  2019-11-08 15:27                 ` KP Singh
  1 sibling, 1 reply; 26+ messages in thread
From: Daniel Borkmann @ 2019-11-08 14:34 UTC (permalink / raw)
  To: Mickaël Salaün, KP Singh
  Cc: Alexei Starovoitov, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Casey Schaufler, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, Michael Kerrisk, Mickaël Salaün, Paul Moore,
	Sargun Dhillon, Serge E . Hallyn, Shuah Khan, Stephen Smalley,
	Tejun Heo, Tetsuo Handa, Tycho Andersen, Will Drewry, bpf,
	kernel-hardening, linux-api, linux-security-module, netdev

On 11/8/19 3:08 PM, Mickaël Salaün wrote:
> On 06/11/2019 22:45, KP Singh wrote:
>> On 06-Nov 17:55, Mickaël Salaün wrote:
>>> On 06/11/2019 11:06, KP Singh wrote:
>>>> On 05-Nov 11:34, Alexei Starovoitov wrote:
>>>>> On Tue, Nov 05, 2019 at 07:01:41PM +0100, Mickaël Salaün wrote:
>>>>>> On 05/11/2019 18:18, Alexei Starovoitov wrote:
[...]
>> * Use a single BPF program type; this is necessary for a key requirement
>>    of KRSI, i.e. runtime instrumentation. The upcoming prototype should
>>    illustrate how this works for KRSI - note that it’s possible to vary
>>    the context types exposed by different hooks.
> 
> Why a single BPF program type? Do you mean *attach* types? Landlock only
> use one program type, but will use multiple attach types.
> 
> Why do you think it is necessary for KRSI or for runtime instrumentation?
> 
> If it is justified, it could be a dedicated program attach type (e.g.
> BPF_LANDLOCK_INTROSPECTION).
> 
> What is the advantage to have the possibility to vary the context types
> over dedicated *typed* contexts? I don't see any advantages, but at
> least one main drawback: to require runtime checks (when helpers use
> this generic context) instead of load time checks (thanks to static type
> checking of the context).

Lets take security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
as one specific example here: the running kernel has its own internal
btf_vmlinux and therefore a complete description of itself. From verifier
side we can retrieve & introspect the security_sock_rcv_skb signatue and
thus know that the given BPF attachment point has struct sock and struct
sk_buff as input arguments which can then be accessed generically by the
prog in order to allow sk_filter_trim_cap() to pass or to drop the skb.
The same generic approach can be done for many of the other lsm hooks, so
single program type would be enough there and context is derived automatically,
no dedicated extra context per attach type would be needed and no runtime
checks as you mentioned above since its still all asserted at verification
time.

Thanks,
Daniel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-08 14:08               ` Mickaël Salaün
  2019-11-08 14:34                 ` Daniel Borkmann
@ 2019-11-08 15:27                 ` KP Singh
  1 sibling, 0 replies; 26+ messages in thread
From: KP Singh @ 2019-11-08 15:27 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Alexei Starovoitov, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Casey Schaufler, Daniel Borkmann,
	David Drysdale, Florent Revest, James Morris, Jann Horn,
	John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
	Mickaël Salaün, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
	Tetsuo Handa, Tycho Andersen, Will Drewry, bpf, kernel-hardening,
	linux-api, linux-security-module, netdev

On 08-Nov 15:08, Mickaël Salaün wrote:
> 
> On 06/11/2019 22:45, KP Singh wrote:
> > On 06-Nov 17:55, Mickaël Salaün wrote:
> >>
> >> On 06/11/2019 11:06, KP Singh wrote:
> >>> On 05-Nov 11:34, Alexei Starovoitov wrote:
> >>>> On Tue, Nov 05, 2019 at 07:01:41PM +0100, Mickaël Salaün wrote:
> >>>>> On 05/11/2019 18:18, Alexei Starovoitov wrote:
> >>
> >> [...]
> >>
> >>>>>> I think the only way bpf-based LSM can land is both landlock and KRSI
> >>>>>> developers work together on a design that solves all use cases.
> >>>>>
> >>>>> As I said in a previous cover letter [1], that would be great. I think
> >>>>> that the current Landlock bases (almost everything from this series
> >>>>> except the seccomp interface) should meet both needs, but I would like
> >>>>> to have the point of view of the KRSI developers.
> >>>>>
> >>>>> [1] https://lore.kernel.org/lkml/20191029171505.6650-1-mic@digikod.net/
> >>>>>
> >>>>>> BPF is capable
> >>>>>> to be a superset of all existing LSMs whereas landlock and KRSI propsals today
> >>>>>> are custom solutions to specific security concerns. BPF subsystem was extended
> >>>>>> with custom things in the past. In networking we have lwt, skb, tc, xdp, sk
> >>>>>> program types with a lot of overlapping functionality. We couldn't figure out
> >>>>>> how to generalize them into single 'networking' program. Now we can and we
> >>>>>> should. Accepting two partially overlapping bpf-based LSMs would be repeating
> >>>>>> the same mistake again.
> >>>>>
> >>>>> I'll let the LSM maintainers comment on whether BPF could be a superset
> >>>>> of all LSM, but given the complexity of an access-control system, I have
> >>>>> some doubts though. Anyway, we need to start somewhere and then iterate.
> >>>>> This patch series is a first step.
> >>>>
> >>>> I would like KRSI folks to speak up. So far I don't see any sharing happening
> >>>> between landlock and KRSI. You're claiming this set is a first step. They're
> >>>> claiming the same about their patches. I'd like to set a patchset that was
> >>>> jointly developed.
> >>>
> >>> We are willing to collaborate with the Landlock developers and come up
> >>> with a common approach that would work for Landlock and KRSI. I want
> >>> to mention that this collaboration and the current Landlock approach
> >>> of using an eBPF based LSM for unprivileged sandboxing only makes sense
> >>> if unprivileged usage of eBPF is going to be ever allowed.
> >>
> >> The ability to *potentially* do unprivileged sandboxing is definitely
> >> not tied nor a blocker to the unprivileged usage of eBPF. As explained
> >> in the documentation [1] (cf. Guiding principles / Unprivileged use),
> >> Landlock is designed to be as safe as possible (from a security point of
> >> view). The impact is more complex and important than just using
> >> unprivileged eBPF, which may not be required. Unprivileged use of eBPF
> >> would be nice, but I think the current direction is to extend the Linux
> >> capabilities with one or multiple dedicated to eBPF [2] (e.g. CAP_BPF +
> >> something else), which may be even better (and a huge difference with
> >> CAP_SYS_ADMIN, a.k.a. privileged mode or root). Landlock is designed to
> >> deal with unprivileged (i.e. non-root) use cases, but of course, if the
> >> Landlock architecture may enable to do unprivileged stuff, it definitely
> >> can do privileged stuff too. However, having an architecture designed
> >> with safe unprivileged use in mind can't be achieve afterwards.
> >>
> >> [1] https://lore.kernel.org/lkml/20191104172146.30797-8-mic@digikod.net/
> >> [2] https://lore.kernel.org/bpf/20190827205213.456318-1-ast@kernel.org/
> >>
> >>
> >>>
> >>> Purely from a technical standpoint, both the current designs for
> >>> Landlock and KRSI target separate use cases and it would not be
> >>> possible to build "one on top of the other". We've tried to identify
> >>> the lowest denominator ("eBPF+LSM") requirements for both Landlock
> >>> (unprivileged sandboxing / Discretionary Access Control) and KRSI
> >>> (flexibility and unification of privileged MAC and Audit) and
> >>> prototyped an implementation based on the newly added / upcoming
> >>> features in BPF.
> >>
> >> This is not as binary as that. Sandboxing can be seen as DAC but also as
> >> MAC, depending on the subject which apply the security policy and the
> >> subjects which are enforced by this policy. If the sandboxing is applied
> >> system-wide, it is what we usually call MAC. DAC, in the Linux world,
> >> enables any user to restrict access to their files to other users.
> >>
> >> With Landlock it is not the same because a process can restrict itself
> >> but also enforce these restrictions on all its future children (which
> >> may be malicious, whatever their UID/GID). The threat and the definition
> >> of the attacker are not the same in both cases. With the Linux DAC the
> >> potentially malicious subjects are the other users, whereas with
> >> Landlock the potentially malicious subjects are (for now) the current
> >> process and all its children. Another way to explain it, and how
> >> Landlock is designed, is that a specific enforcement (i.e. a set of BPF
> >> programs) is tied to a domain, in which a set of subject are. From this
> >> perspective, this approach (subjects/tasks in a domain) is orthogonal to
> >> the DAC system (subjects/users). This design may apply to a system-wide
> >> MAC system by putting all the system tasks in one domain, and managing
> >> restrictions (by subject) with other means (e.g. task's UID,
> >> command-line strings, environment variables). In short, Landlock (in
> >> this patch series) is closer to a (potentially scoped) MAC system. But
> >> thanks to eBPF, Landlock is firstly a programmatic access-control, which
> >> means that the one who write the programs and tie them to a set of
> >> tasks, can implement their own access-control system (e.g. RBAC,
> >> time-based…), or something else (e.g. an audit system).
> >>
> >> The audit part can simply be achieve with dedicated helpers and programs
> >> that always allow accesses.
> >>
> >> Landlock evolved over multiple iterations and is now designed to be very
> >> flexible. The current way to enforce a security policy is to go through
> >> the seccomp syscall (which makes sense for multiple reasons explained
> >> and accepted before). But Landlock is designed to enable similar
> >> enforcements (or audit) with other ways to define a domain (e.g. cgroups
> >> [3], or system-wide securityfs as done in KRSI). Indeed, the only part
> >> tied to this scoped enforcement is in the domain_syscall.c file. A new
> >> file domain_fs.c could be added to implement a securityfs for a
> >> system-wide enforcement (and have other features as KRSI does).
> >>
> > 
> > Given the current way landlock exposes LSM hooks, I don't think it's
> > possible to build system-wide detections.
> 
> Why ?

This and some of the other questions (about the core-framework and
guiding principles) are better discussed in the context of the
new-patchset. It might take us some-time as I am on vacation for the
next few weeks and some of the other members are traveling.

> 
> 
> > But let’s try to come to a
> > consensus on the semantics of the how the LSM hooks are exposed to
> > BPF. At the moment I think we should:
> > 
> > 
> > * Bring the core interface exposed to eBPF closer to the LSM surface in
> >   a way that supports both use cases. One way Landlock can still provide
> >   a more abstract interface is by providing some BPF helper libraries
> >   that build on top of the core framework.
> 
> I still don't get why you think it is the only way or the better. I gave
> a lot of arguments and I explained why Landlock is designed the way it
> is, especially in the documentation (Guiding principles). Is there
> something similar for KRSI?
> 
> 
> > 
> > * Use a single BPF program type; this is necessary for a key requirement
> >   of KRSI, i.e. runtime instrumentation. The upcoming prototype should
> >   illustrate how this works for KRSI - note that it’s possible to vary
> >   the context types exposed by different hooks.
> 
> Why a single BPF program type? Do you mean *attach* types? Landlock only
> use one program type, but will use multiple attach types.
> 
> Why do you think it is necessary for KRSI or for runtime instrumentation?
> 
> If it is justified, it could be a dedicated program attach type (e.g.
> BPF_LANDLOCK_INTROSPECTION).
> 
> What is the advantage to have the possibility to vary the context types
> over dedicated *typed* contexts? I don't see any advantages, but at
> least one main drawback: to require runtime checks (when helpers use
> this generic context) instead of load time checks (thanks to static type
> checking of the context).
> 
> 
> > It would be nice to get the BPF maintainers’ opinion on these points.
> > 
> > 
> >> [3] https://lore.kernel.org/lkml/20160914072415.26021-17-mic@digikod.net/
> >>
> >> One possible important difference between Landlock and KRSI right now is
> >> the BPF program management. Both manage a list of programs per hook.
> >> However KRSI needs to be able to replace a program in these lists. This
> >> is not implemented in this Landlock patch series, first because it is
> >> not the main use-case and it is safer to have an append-only way to add
> >> restrictions (cf. seccomp-bpf model), and second because it is simpler
> >> to deal with immutable lists. However, it is worth considering extending
> >> the Landlock domain management with the ability to update the program
> >> lists. One challenge may be to identify which program should be replaced
> >> (which KRSI does with the program name). I think it would be wiser to
> >> implement this in a second step though, maybe not for the syscall
> >> interface (thanks to a new seccomp operation), but first with the
> >> securityfs one.
> >>
> >>
> >>>
> >>> We've been successfully able to prototype the use cases for KRSI
> >>> (privileged MAC and Audit) using this "eBPF+LSM" and shared our
> >>> approach at the Linux Security Summit [1]:
> >>>
> >>> * Use the new in-kernel BTF (CO-RE eBPF programs) [2] and the ability
> >>>   of the BPF verifier to use the BTF information for access validation
> >>>   to provide a more generic way to attach to the various LSM hooks.
> >>>   This potentially saves a lot of redundant work:
> >>>
> >>>    - Creation of new program types.
> >>>    - Multiple types of contexts (or a single context with Unions).
> >>>    - Changes to the verifier and creation of new BPF argument types 
> >>>      (eg. PTR_TO_TASK)
> >>
> >> As I understood from the LSS talk, KRSI's approach is to use the same
> >> hooks as LSM (cf. the securityfs). As said Alexei [4] "It must not make
> >> LSM hooks into stable ABI".  Moveover, the LSM hooks may change
> >> according to internal kernel evolution, and their semantic may not make
> > 
> > I think you misunderstand Alexei here. I will let him elaborate.
> > 
> >> sense from a user space point of view. This is one reason for which
> >> Lanlock abstract those hooks into something that is simpler and designed
> >> to fit well with eBPF (program contexts and their attached types, as
> >> explained in the documentation).
> >>
> >> [4]
> >> https://lore.kernel.org/lkml/20191105215453.szhdkrvuekwfz6le@ast-mbp.dhcp.thefacebook.com/
> >>
> >> How does KRSI plan to deal with one LSM hook being split in two hooks in
> >> a next version of the kernel (cf. [5])?
> > 
> > How often has that happened in the past? And even if it does happen,
> > it can still be handled as a part of the base framework we are trying
> > to implement.
> 
> I guess the security maintainers should have an opinion on this.
> 
> I don't clearly see the properties of this base framework. Could this be
> elaborated?
> 
> 
> >> [5] https://lore.kernel.org/lkml/20190910115527.5235-6-kpsingh@chromium.org/
> >>
> >>
> >> Another reason to have multiple different attach types/contexts (cf.
> >> landlock_domain->programs[]) is to limit useless BPF program
> >> interpretation (in addition to the non-system-wide scoped of programs).
> >>  It also enables to handle and verify strict context use (which is also
> >> explain in the Guiding principles). It would be a huge wast of time to
> >> run every BPF programs for all LSM hooks. KRSI does the same but instead
> >> of relying on the program type it rely on the list tied to the
> >> securityfs file.
> >>
> >> BTF is great, but as far as I know, it's goal is to easily deal with the
> >> moving kernel ABI (e.g. task_struct layout, config/constant variables),
> >> and it is definitely useful to programs using bpf_probe_read() and
> >> similar accessors. However, I don't see how KRSI would avoid BPF types
> >> thanks to BTF.
> >>
> > 
> > This should become clearer once we post our updated patch-set. Do note
> > that I am currently traveling and will be away for the next couple of
> > weeks.
> > 
> >> There is only one program type for Landlock (i.e.
> >> BPF_PROG_TYPE_LANDLOCK_HOOK), and I don't see why adding new program
> >> *attach* types (e.g. BPF_LANDLOCK_PTRACE) may be an issue. The kernel
> >> will still need to be modified to implement new hooks and the new BPF
> >> helpers anyway, BTF will not change that, except maybe if the internal
> >> LSM API is exposed in a way or another to BPF (thanks to BTF), which
> >> does not seem acceptable. Am I missing something?
> >>
> >>
> >> The current KRSI approach is to allow a common set of helpers to be
> >> called by all programs (because there is no way to differentiate them
> >> with their type).
> >> How KRSI would deal with kernel objects other than the current task
> >> (e.g. a ptrace hook with a tracer and a tracee, a file open/read) with
> >> the struct krsi_ctx unions [6]?
> >>
> >> [6] https://lore.kernel.org/lkml/20190910115527.5235-7-kpsingh@chromium.org/
> >>
> > 
> > The best part of BTF is that it can provide a common way to pass
> > different contexts to the various attachments points and the verifier
> > can use the BTF information to validate accesses which essentially
> > allows us to change the helpers from:
> > 
> >        is_running_executable(magical_krsi_ctx)
> > 
> >           to
> > 
> >        is_running_executable(inode)
> > 
> > 
> > which can work on any inode (ARG_PTR_TO_BTF_ID = btf_id(struct inode))
> > 
> > This makes the helpers much more useful and generic. All this is
> > better explained in our upcoming patch-set.
> 
> I get the usefulness of BTF for future helper evolution and for moving
> kernel API, but again, I don't see advantages over static typing check
> of (well abstract/generic) kernel object handles in the context.
> 
> For instance, how BTF would replace the current BPF_LANDLOCK_PTRACE
> context and the associated helper?
> 
> Does this mean that the KRSI v1 design is outdated and superseded by the
> (future) v2 because of the more important use of BTF?

Yes, that's correct and based on the feedback we got on the RFC v1 
at the Linux Plumbers 2019. 

Also, the changes that allow BPF verifier to use the
BTF information to validate accesses and dynamically populate the
context are very recent and have influenced the design for the
next iteration.

- KP Singh

> 
> 
> >> How does KRSI plan to deal with security blobs?
> > 
> > The new prototype uses security blobs but does not expose them to
> > user-space. These blobs are then used in various helpers like
> > “is_running_executable” which uses blobs on the inode and the
> > task_struct. This should become clearer when the next patchset is
> > posted.
> > 
> > I don’t think it’s currently possible to allow the blobs to be set
> > using eBPF programs with the main reason being that the blob will only
> > be set after the program is loaded. The answer to
> > “is_running_executable” becomes dependent on whether the file was
> > executed before the blob setting eBPF program was loaded.
> > 
> > Blob management with eBPF is not possible unless we can load eBPF
> > programs that can set blobs at boot-time.
> > In short, the next KRSI version will not give eBPF
> > programs access to arbitrarily write security blobs.
> 
> A previous version of Landlock enabled programs to tag inodes (cf.
> FS_GET): https://lore.kernel.org/lkml/20180227004121.3633-8-mic@digikod.net/
> 
> 
> >>>
> >>> * These new BPF features also alleviate the original concerns that we
> >>>   raised when initially proposing KRSI and designing for precise BPF
> >>>   helpers. We have some patches coming up which incorporate these new
> >>>   changes and will be sharing something on the mailing list after some
> >>>   cleanup.
> >>>
> >>> We can use the common "eBPF+LSM" for both privileged MAC and Audit and
> >>> unprivileged sandboxing i.e. Discretionary Access Control.
> >>> Here's what it could look like:
> >>>
> >>> * Common infrastructure allows attachment to all hooks which works well
> >>>   for privileged MAC and Audit. This could be extended to provide
> >>>   another attachment type for unprivileged DAC, which can restrict the
> >>>   hooks that can be attached to, and also the information that is
> >>>   exposed to the eBPF programs which is something that Landlock could
> >>>   build.
> >>
> >> I agree that the "privileged-only" hooks should be a superset of the
> >> "security-safe-and-potentially-unprivileged" hooks. :)
> >> However, as said previously, I'm convinced it is a requirement to have
> >> abstract hooks (and associated program attach types) as defined by Landlock.
> > 
> > I would like to hear the BPF maintainers’ perspective on this. I am
> > not sure they agree with you here.
> > 
> > - KP Singh
> > 
> >>
> >> I'm not sure what you mean by "the information that is exposed to the
> >> eBPF program". Is it the current Landlock implementation of specific
> >> contexts and attach types?
> 
> …or does BTF could magically solve this?
> 
> 
> >>
> >>>
> >>> * This attachment could use the proposed landlock domains and attach to
> >>>   the task_struct providing the discretionary access control semantics.
> >>
> >> Not task_struct but creds, yes. This is a characteristic of sandboxing,
> >> which may not be useful for the KRSI use case. It makes sense for KRSI
> >> to attach program sets (or Landlock domains) to the whole system, then
> >> using the creds does not make sense here. This difference is small and a
> >> previous version of Landlock already validated this use case with
> >> cgroups [3] (which is postponed to simplify the patch series).
> >>
> >> [3] https://lore.kernel.org/lkml/20160914072415.26021-17-mic@digikod.net/
> >>
> >>
> >>>
> >>> [1] https://static.sched.com/hosted_files/lsseu2019/a2/Kernel%20Runtime%20Security%20Instrumentation.pdf
> >>> [2] http://vger.kernel.org/bpfconf2019_talks/bpf-core.pdf
> >>>
> >>> - KP Singh
> >>
> >> I think it should be OK to first land something close to this Landlock
> >> patch series and then we could extend the domain management features and
> >> add the securityfs support that KRSI needs. The main concern seems to be
> >> about hook definitions.
> >>
> >> Another approach would be to land Landlock and KRSI as distinct LSM
> >> while trying as much as possible to mutualize code/helpers.
> > 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks
  2019-11-08 14:34                 ` Daniel Borkmann
@ 2019-11-08 15:39                   ` Mickaël Salaün
  0 siblings, 0 replies; 26+ messages in thread
From: Mickaël Salaün @ 2019-11-08 15:39 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: KP Singh, Alexei Starovoitov, linux-kernel, Alexei Starovoitov,
	Andy Lutomirski, Casey Schaufler, David Drysdale, Florent Revest,
	James Morris, Jann Horn, John Johansen, Jonathan Corbet,
	Kees Cook, Michael Kerrisk, Mickaël Salaün, Paul Moore,
	Sargun Dhillon, Serge E . Hallyn, Shuah Khan, Stephen Smalley,
	Tejun Heo, Tetsuo Handa, Tycho Andersen, Will Drewry, bpf,
	kernel-hardening, linux-api, linux-security-module, netdev


On 08/11/2019 15:34, Daniel Borkmann wrote:
> On 11/8/19 3:08 PM, Mickaël Salaün wrote:
>> On 06/11/2019 22:45, KP Singh wrote:
>>> On 06-Nov 17:55, Mickaël Salaün wrote:
>>>> On 06/11/2019 11:06, KP Singh wrote:
>>>>> On 05-Nov 11:34, Alexei Starovoitov wrote:
>>>>>> On Tue, Nov 05, 2019 at 07:01:41PM +0100, Mickaël Salaün wrote:
>>>>>>> On 05/11/2019 18:18, Alexei Starovoitov wrote:
> [...]
>>> * Use a single BPF program type; this is necessary for a key requirement
>>>    of KRSI, i.e. runtime instrumentation. The upcoming prototype should
>>>    illustrate how this works for KRSI - note that it’s possible to vary
>>>    the context types exposed by different hooks.
>>
>> Why a single BPF program type? Do you mean *attach* types? Landlock only
>> use one program type, but will use multiple attach types.
>>
>> Why do you think it is necessary for KRSI or for runtime instrumentation?
>>
>> If it is justified, it could be a dedicated program attach type (e.g.
>> BPF_LANDLOCK_INTROSPECTION).
>>
>> What is the advantage to have the possibility to vary the context types
>> over dedicated *typed* contexts? I don't see any advantages, but at
>> least one main drawback: to require runtime checks (when helpers use
>> this generic context) instead of load time checks (thanks to static type
>> checking of the context).
> 
> Lets take security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
> as one specific example here: the running kernel has its own internal
> btf_vmlinux and therefore a complete description of itself. From verifier
> side we can retrieve & introspect the security_sock_rcv_skb signatue

OK, this is indeed the signature defined by the kernel API. What happen
if this API change (e.g. if struct sock is replaced with a struct
sock_meta)?


> and
> thus know that the given BPF attachment point has struct sock and struct
> sk_buff as input arguments

How does the verifier know a given BPF attachment point for a program
without relying on its type or attach type? How and where is registered
this mapping?

To say it another way, if there is no way to differentiate two program
targeting different hook, I don't understand how the verifier could
check if a given program can legitimately call a helper which could read
the tracer and tracee fields (legitimate for a ptrace hook), whereas
this program may be attached to a sock_rcv_skb hook (and there is no way
to know that).


> which can then be accessed generically by the
> prog in order to allow sk_filter_trim_cap() to pass or to drop the skb.
> The same generic approach can be done for many of the other lsm hooks, so
> single program type would be enough there and context is derived
> automatically,
> no dedicated extra context per attach type would be needed and no runtime
> checks as you mentioned above since its still all asserted at verification
> time.

I mentioned runtime check because I though a helper should handle the
case when it doesn't make sense for a program attached to a specific
point/hook (e.g. ptrace) to use an input argument (e.g. sk) defined for
another point/hook (e.g. sock_rcv_skb).


> 
> Thanks,
> Daniel
> 

Thanks for this explanation Daniel.

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2019-11-08 15:41 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04 17:21 [PATCH bpf-next v13 0/7] Landlock LSM Mickaël Salaün
2019-11-04 17:21 ` [PATCH bpf-next v13 1/7] bpf,landlock: Define an eBPF program type for Landlock hooks Mickaël Salaün
2019-11-04 17:21 ` [PATCH bpf-next v13 2/7] landlock: Add the management of domains Mickaël Salaün
2019-11-04 17:21 ` [PATCH bpf-next v13 3/7] landlock,seccomp: Apply Landlock programs to process hierarchy Mickaël Salaün
2019-11-04 17:21 ` [PATCH bpf-next v13 4/7] landlock: Add ptrace LSM hooks Mickaël Salaün
2019-11-05 17:18   ` Alexei Starovoitov
2019-11-05 17:55     ` Casey Schaufler
2019-11-05 19:31       ` Alexei Starovoitov
2019-11-05 19:55         ` Casey Schaufler
2019-11-05 21:54           ` Alexei Starovoitov
2019-11-05 22:32             ` Casey Schaufler
2019-11-05 18:01     ` Mickaël Salaün
2019-11-05 19:34       ` Alexei Starovoitov
2019-11-05 22:18         ` Mickaël Salaün
2019-11-06 10:06         ` KP Singh
2019-11-06 16:55           ` Mickaël Salaün
2019-11-06 21:45             ` KP Singh
2019-11-08 14:08               ` Mickaël Salaün
2019-11-08 14:34                 ` Daniel Borkmann
2019-11-08 15:39                   ` Mickaël Salaün
2019-11-08 15:27                 ` KP Singh
2019-11-06 10:15       ` KP Singh
2019-11-06 16:58         ` Mickaël Salaün
2019-11-04 17:21 ` [PATCH bpf-next v13 5/7] bpf,landlock: Add task_landlock_ptrace_ancestor() helper Mickaël Salaün
2019-11-04 17:21 ` [PATCH bpf-next v13 6/7] bpf,landlock: Add tests for the Landlock ptrace program type Mickaël Salaün
2019-11-04 17:21 ` [PATCH bpf-next v13 7/7] landlock: Add user and kernel documentation for Landlock Mickaël Salaün

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).