Re: [PATCH net-next v7 08/10] bpf: Add a Landlock sandbox example

[Mickaël Salaün]

On 01/09/2017 12:25, Alban Crequy wrote:
> Hi Mickaël,
> 
> On 21 August 2017 at 02:09, Mickaël Salaün  wrote:
>> Add a basic sandbox tool to create a process isolated from some part of
>> the system. This sandbox create a read-only environment. It is only
>> allowed to write to a character device such as a TTY:
> ...
>> +   /*
>> +* This check allows the action on the file if it is a directory or a
>> +* pipe. Otherwise, a message is printed to the eBPF log.
>> +*/
>> +   if (S_ISCHR(ret) || S_ISFIFO(ret))
>> +   return 0;
> 
> 
> The comment says "directory", but the code checks for "character device".
> 
> Thanks!
> Alban
> 

Fixed, thanks!



signature.asc
Description: OpenPGP digital signature

Re: [PATCH net-next v7 08/10] bpf: Add a Landlock sandbox example

[Alban Crequy]

Hi Mickaël,

On 21 August 2017 at 02:09, Mickaël Salaün  wrote:
> Add a basic sandbox tool to create a process isolated from some part of
> the system. This sandbox create a read-only environment. It is only
> allowed to write to a character device such as a TTY:
...
> +   /*
> +* This check allows the action on the file if it is a directory or a
> +* pipe. Otherwise, a message is printed to the eBPF log.
> +*/
> +   if (S_ISCHR(ret) || S_ISFIFO(ret))
> +   return 0;


The comment says "directory", but the code checks for "character device".

Thanks!
Alban

Re: [PATCH net-next v7 08/10] bpf: Add a Landlock sandbox example

[Mickaël Salaün]

On 24/08/2017 04:59, Alexei Starovoitov wrote:
> On Mon, Aug 21, 2017 at 02:09:31AM +0200, Mickaël Salaün wrote:
>> Add a basic sandbox tool to create a process isolated from some part of
>> the system. This sandbox create a read-only environment. It is only
>> allowed to write to a character device such as a TTY:
>>
>>   # :> X
>>   # echo $?
>>   0
>>   # ./samples/bpf/landlock1 /bin/sh -i
>>   Launching a new sandboxed process.
>>   # :> Y
>>   cannot create Y: Operation not permitted
>>
>> Signed-off-by: Mickaël Salaün 
> 
> ...
> 
>> +SEC("landlock1")
>> +static int landlock_fs_prog1(struct landlock_context *ctx)
>> +{
>> +char fmt_error_mode[] = "landlock1: error: get_mode:%lld\n";
>> +char fmt_error_access[] = "landlock1: error: access denied\n";
>> +long long ret;
>> +
>> +/*
>> + * The argument ctx->arg2 contains bitflags of actions for which the
>> + * rule is run.  The flag LANDLOCK_ACTION_FS_WRITE means that a write
>> + * is requested by one of the userspace processes restricted by this
>> + * rule. The following test allows any actions which does not include a
>> + * write.
>> + */
>> +if (!(ctx->arg2 & LANDLOCK_ACTION_FS_WRITE))
>> +return 0;
>> +
>> +/*
>> + * The argument ctx->arg1 is a file handle for which the process want
>> + * to access. The function bpf_handle_fs_get_mode() return the mode of
>> + * a file (e.g. S_IFBLK, S_IFDIR, S_IFREG...). If there is an error,
>> + * for example if the argument is not a file handle, then an
>> + * -errno value is returned. Otherwise the caller get the file mode as
>> + *  with stat(2).
>> + */
>> +ret = bpf_handle_fs_get_mode((void *)ctx->arg1);
>> +if (ret < 0) {
>> +
>> +/*
>> + * The bpf_trace_printk() function enable to write in the
>> + * kernel eBPF debug log, accessible through
>> + * /sys/kernel/debug/tracing/trace_pipe . To be allowed to call
>> + * this function, a Landlock rule must have the
>> + * LANDLOCK_SUBTYPE_ABILITY_DEBUG ability, which is only
>> + * allowed for CAP_SYS_ADMIN.
>> + */
>> +bpf_trace_printk(fmt_error_mode, sizeof(fmt_error_mode), ret);
>> +return 1;
>> +}
>> +
>> +/*
>> + * This check allows the action on the file if it is a directory or a
>> + * pipe. Otherwise, a message is printed to the eBPF log.
>> + */
>> +if (S_ISCHR(ret) || S_ISFIFO(ret))
>> +return 0;
>> +bpf_trace_printk(fmt_error_access, sizeof(fmt_error_access));
>> +return 1;
>> +}
>> +
>> +/*
>> + * This subtype enable to set the ABI, which ensure that the eBPF context 
>> and
>> + * program behavior will be compatible with this Landlock rule.
>> + */
>> +SEC("subtype")
>> +static const union bpf_prog_subtype _subtype = {
>> +.landlock_rule = {
>> +.abi = 1,
>> +.event = LANDLOCK_SUBTYPE_EVENT_FS,
>> +.ability = LANDLOCK_SUBTYPE_ABILITY_DEBUG,
>> +}
>> +};
> 
> from rule writer perspective can you somehow merge subtype definition
> with the program? It seems they go hand in hand.
> Like section name of the program can be:
> SEC("landlock_rule1/event=fs/ability=debug")
> static int landlock_fs_prog1(struct landlock_context *ctx)...
> and the loader can parse this string and prepare appropriate
> data structures for the kernel.

Right, I'll try that.



signature.asc
Description: OpenPGP digital signature

Re: [PATCH net-next v7 08/10] bpf: Add a Landlock sandbox example

[Alexei Starovoitov]

On Mon, Aug 21, 2017 at 02:09:31AM +0200, Mickaël Salaün wrote:
> Add a basic sandbox tool to create a process isolated from some part of
> the system. This sandbox create a read-only environment. It is only
> allowed to write to a character device such as a TTY:
> 
>   # :> X
>   # echo $?
>   0
>   # ./samples/bpf/landlock1 /bin/sh -i
>   Launching a new sandboxed process.
>   # :> Y
>   cannot create Y: Operation not permitted
> 
> Signed-off-by: Mickaël Salaün 

...

> +SEC("landlock1")
> +static int landlock_fs_prog1(struct landlock_context *ctx)
> +{
> + char fmt_error_mode[] = "landlock1: error: get_mode:%lld\n";
> + char fmt_error_access[] = "landlock1: error: access denied\n";
> + long long ret;
> +
> + /*
> +  * The argument ctx->arg2 contains bitflags of actions for which the
> +  * rule is run.  The flag LANDLOCK_ACTION_FS_WRITE means that a write
> +  * is requested by one of the userspace processes restricted by this
> +  * rule. The following test allows any actions which does not include a
> +  * write.
> +  */
> + if (!(ctx->arg2 & LANDLOCK_ACTION_FS_WRITE))
> + return 0;
> +
> + /*
> +  * The argument ctx->arg1 is a file handle for which the process want
> +  * to access. The function bpf_handle_fs_get_mode() return the mode of
> +  * a file (e.g. S_IFBLK, S_IFDIR, S_IFREG...). If there is an error,
> +  * for example if the argument is not a file handle, then an
> +  * -errno value is returned. Otherwise the caller get the file mode as
> +  *  with stat(2).
> +  */
> + ret = bpf_handle_fs_get_mode((void *)ctx->arg1);
> + if (ret < 0) {
> +
> + /*
> +  * The bpf_trace_printk() function enable to write in the
> +  * kernel eBPF debug log, accessible through
> +  * /sys/kernel/debug/tracing/trace_pipe . To be allowed to call
> +  * this function, a Landlock rule must have the
> +  * LANDLOCK_SUBTYPE_ABILITY_DEBUG ability, which is only
> +  * allowed for CAP_SYS_ADMIN.
> +  */
> + bpf_trace_printk(fmt_error_mode, sizeof(fmt_error_mode), ret);
> + return 1;
> + }
> +
> + /*
> +  * This check allows the action on the file if it is a directory or a
> +  * pipe. Otherwise, a message is printed to the eBPF log.
> +  */
> + if (S_ISCHR(ret) || S_ISFIFO(ret))
> + return 0;
> + bpf_trace_printk(fmt_error_access, sizeof(fmt_error_access));
> + return 1;
> +}
> +
> +/*
> + * This subtype enable to set the ABI, which ensure that the eBPF context and
> + * program behavior will be compatible with this Landlock rule.
> + */
> +SEC("subtype")
> +static const union bpf_prog_subtype _subtype = {
> + .landlock_rule = {
> + .abi = 1,
> + .event = LANDLOCK_SUBTYPE_EVENT_FS,
> + .ability = LANDLOCK_SUBTYPE_ABILITY_DEBUG,
> + }
> +};

from rule writer perspective can you somehow merge subtype definition
with the program? It seems they go hand in hand.
Like section name of the program can be:
SEC("landlock_rule1/event=fs/ability=debug")
static int landlock_fs_prog1(struct landlock_context *ctx)...
and the loader can parse this string and prepare appropriate
data structures for the kernel.

[PATCH net-next v7 08/10] bpf: Add a Landlock sandbox example

[Mickaël Salaün]

Add a basic sandbox tool to create a process isolated from some part of
the system. This sandbox create a read-only environment. It is only
allowed to write to a character device such as a TTY:

  # :> X
  # echo $?
  0
  # ./samples/bpf/landlock1 /bin/sh -i
  Launching a new sandboxed process.
  # :> Y
  cannot create Y: Operation not permitted

Signed-off-by: Mickaël Salaün 
Cc: Alexei Starovoitov 
Cc: Andy Lutomirski 
Cc: Daniel Borkmann 
Cc: David S. Miller 
Cc: James Morris 
Cc: Kees Cook 
Cc: Serge E. Hallyn 
---

Changes since v6:
* check return value of load_and_attach()
* allow to write on pipes
* rename BPF_PROG_TYPE_LANDLOCK to BPF_PROG_TYPE_LANDLOCK_RULE
* rename Landlock version to ABI to better reflect its purpose
* use const variable (suggested by Kees Cook)
* remove useless definitions (suggested by Kees Cook)
* add detailed explanations (suggested by Kees Cook)

Changes since v5:
* cosmetic fixes
* rebase

Changes since v4:
* write Landlock rule in C and compiled it with LLVM
* remove cgroup handling
* remove path handling: only handle a read-only environment
* remove errno return codes

Changes since v3:
* remove seccomp and origin field: completely free from seccomp programs
* handle more FS-related hooks
* handle inode hooks and directory traversal
* add faked but consistent view thanks to ENOENT
* add /lib64 in the example
* fix spelling
* rename some types and definitions (e.g. SECCOMP_ADD_LANDLOCK_RULE)

Changes since v2:
* use BPF_PROG_ATTACH for cgroup handling
---
 samples/bpf/Makefile |   4 ++
 samples/bpf/bpf_load.c   |  28 ++--
 samples/bpf/landlock1_kern.c | 100 +++
 samples/bpf/landlock1_user.c | 100 +++
 4 files changed, 229 insertions(+), 3 deletions(-)
 create mode 100644 samples/bpf/landlock1_kern.c
 create mode 100644 samples/bpf/landlock1_user.c

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index f1010fe759fe..08d5d728e3e0 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -40,6 +40,7 @@ hostprogs-y += load_sock_ops
 hostprogs-y += xdp_redirect
 hostprogs-y += xdp_redirect_map
 hostprogs-y += syscall_tp
+hostprogs-y += landlock1
 
 # Libbpf dependencies
 LIBBPF := ../../tools/lib/bpf/bpf.o
@@ -84,6 +85,7 @@ per_socket_stats_example-objs := $(LIBBPF) 
cookie_uid_helper_example.o
 xdp_redirect-objs := bpf_load.o $(LIBBPF) xdp_redirect_user.o
 xdp_redirect_map-objs := bpf_load.o $(LIBBPF) xdp_redirect_map_user.o
 syscall_tp-objs := bpf_load.o $(LIBBPF) syscall_tp_user.o
+landlock1-objs := bpf_load.o $(LIBBPF) landlock1_user.o
 
 # Tell kbuild to always build the programs
 always := $(hostprogs-y)
@@ -128,6 +130,7 @@ always += tcp_clamp_kern.o
 always += xdp_redirect_kern.o
 always += xdp_redirect_map_kern.o
 always += syscall_tp_kern.o
+always += landlock1_kern.o
 
 HOSTCFLAGS += -I$(objtree)/usr/include
 HOSTCFLAGS += -I$(srctree)/tools/lib/
@@ -167,6 +170,7 @@ HOSTLOADLIBES_test_map_in_map += -lelf
 HOSTLOADLIBES_xdp_redirect += -lelf
 HOSTLOADLIBES_xdp_redirect_map += -lelf
 HOSTLOADLIBES_syscall_tp += -lelf
+HOSTLOADLIBES_landlock1 += -lelf
 
 # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on 
cmdline:
 #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc 
CLANG=~/git/llvm/build/bin/clang
diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
index 01a506f768da..30fcddda8b81 100644
--- a/samples/bpf/bpf_load.c
+++ b/samples/bpf/bpf_load.c
@@ -31,6 +31,8 @@
 
 static char license[128];
 static int kern_version;
+static union bpf_prog_subtype subtype = {};
+static bool has_subtype;
 static bool processed_sec[128];
 char bpf_log_buf[BPF_LOG_BUF_SIZE];
 int map_fd[MAX_MAPS];
@@ -66,6 +68,7 @@ static int load_and_attach(const char *event, struct bpf_insn 
*prog, int size)
bool is_cgroup_sk = strncmp(event, "cgroup/sock", 11) == 0;
bool is_sockops = strncmp(event, "sockops", 7) == 0;
bool is_sk_skb = strncmp(event, "sk_skb", 6) == 0;
+   bool is_landlock = strncmp(event, "landlock", 8) == 0;
size_t insns_cnt = size / sizeof(struct bpf_insn);
enum bpf_prog_type prog_type;
char buf[256];
@@ -96,6 +99,13 @@ static int load_and_attach(const char *event, struct 
bpf_insn *prog, int size)
prog_type = BPF_PROG_TYPE_SOCK_OPS;
} else if (is_sk_skb) {
prog_type = BPF_PROG_TYPE_SK_SKB;
+   } else if (is_landlock) {
+   prog_type = BPF_PROG_TYPE_LANDLOCK_RULE;
+   if (!has_subtype) {
+   printf("No subtype\n");
+   return -1;
+   }
+   st = 
} else {
printf("Unknown event '%s'\n", event);
return -1;
@@ -110,7 +120,8 @@ static int load_and_attach(const