Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Mickaël Salaün]

On 03/06/2018 11:28 PM, Mickaël Salaün wrote:
> 
> On 28/02/2018 01:09, Andy Lutomirski wrote:
>> On Wed, Feb 28, 2018 at 12:00 AM, Mickaël Salaün  wrote:
>>>
>>> On 28/02/2018 00:23, Andy Lutomirski wrote:
 On Tue, Feb 27, 2018 at 11:02 PM, Andy Lutomirski  wrote:
> On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:
>>
>
> I think you're wrong here.  Any sane container trying to use Landlock
> like this would also create a PID namespace.  Problem solved.  I still
> think you should drop this patch.
>>>
>>> Containers is one use case, another is build-in sandboxing (e.g. for web
>>> browser…) and another one is for sandbox managers (e.g. Firejail,
>>> Bubblewrap, Flatpack…). In some of these use cases, especially from a
>>> developer point of view, you may want/need to debug your applications
>>> (without requiring to be root). For nested Landlock access-controls
>>> (e.g. container + user session + web browser), it may not be allowed to
>>> create a PID namespace, but you still want to have a meaningful
>>> access-control.
>>>
>>
>> The consideration should be exactly the same as for normal seccomp.
>> If I'm in a container (using PID namespaces + seccomp) and a run a web
>> browser, I can debug the browser.
>>
>> If there's a real use case for adding this type of automatic ptrace
>> protection, then by all means, let's add it as a general seccomp
>> feature.
>>
> 
> Right, it makes sense to add this feature to seccomp filters as well.
> What do you think Kees?
> 

As a second though, it may be useful for seccomp but it should be
another patch series, independent from this one.

The idea to keep in mind is that this ptrace restriction is an automatic
way to define what is called a subject in common access control
vocabulary, like used by SELinux. A subject should not be able to
impersonate another one with less restrictions (to get more rights).
Because of the stackable restrictions of Landlock (same principle used
by seccomp), it is easy to identify which subject (i.e. group of
processes) is more restricted (or with different restrictions) than
another. This follow the same principle as Yama's ptrace_scope.

Another important argument for a different ptrace-protection
mechanism than seccomp is that Landlock programs may be applied (i.e.
define subject) otherwise than with a process hierarchy. Another way to
define a Landlock subject may be by using cgroups (which was previously
discussed). I'm also thinking about being able to create (real)
capabilities (not to be confused with POSIX capabilities), which may be
useful to implement some parts of Capsicum, by attaching Landlock
programs to a file descriptor (and not directly to a group of
processes). All this to highlight that the ptrace protection is specific
to Landlock and may not be directly shared with seccomp.

Even if Landlock follows the footprints of seccomp, they are different
beasts.



signature.asc
Description: OpenPGP digital signature

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Mickaël Salaün]

On 03/06/2018 11:28 PM, Mickaël Salaün wrote:
> 
> On 28/02/2018 01:09, Andy Lutomirski wrote:
>> On Wed, Feb 28, 2018 at 12:00 AM, Mickaël Salaün  wrote:
>>>
>>> On 28/02/2018 00:23, Andy Lutomirski wrote:
 On Tue, Feb 27, 2018 at 11:02 PM, Andy Lutomirski  wrote:
> On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:
>>
>
> I think you're wrong here.  Any sane container trying to use Landlock
> like this would also create a PID namespace.  Problem solved.  I still
> think you should drop this patch.
>>>
>>> Containers is one use case, another is build-in sandboxing (e.g. for web
>>> browser…) and another one is for sandbox managers (e.g. Firejail,
>>> Bubblewrap, Flatpack…). In some of these use cases, especially from a
>>> developer point of view, you may want/need to debug your applications
>>> (without requiring to be root). For nested Landlock access-controls
>>> (e.g. container + user session + web browser), it may not be allowed to
>>> create a PID namespace, but you still want to have a meaningful
>>> access-control.
>>>
>>
>> The consideration should be exactly the same as for normal seccomp.
>> If I'm in a container (using PID namespaces + seccomp) and a run a web
>> browser, I can debug the browser.
>>
>> If there's a real use case for adding this type of automatic ptrace
>> protection, then by all means, let's add it as a general seccomp
>> feature.
>>
> 
> Right, it makes sense to add this feature to seccomp filters as well.
> What do you think Kees?
> 

As a second though, it may be useful for seccomp but it should be
another patch series, independent from this one.

The idea to keep in mind is that this ptrace restriction is an automatic
way to define what is called a subject in common access control
vocabulary, like used by SELinux. A subject should not be able to
impersonate another one with less restrictions (to get more rights).
Because of the stackable restrictions of Landlock (same principle used
by seccomp), it is easy to identify which subject (i.e. group of
processes) is more restricted (or with different restrictions) than
another. This follow the same principle as Yama's ptrace_scope.

Another important argument for a different ptrace-protection
mechanism than seccomp is that Landlock programs may be applied (i.e.
define subject) otherwise than with a process hierarchy. Another way to
define a Landlock subject may be by using cgroups (which was previously
discussed). I'm also thinking about being able to create (real)
capabilities (not to be confused with POSIX capabilities), which may be
useful to implement some parts of Capsicum, by attaching Landlock
programs to a file descriptor (and not directly to a group of
processes). All this to highlight that the ptrace protection is specific
to Landlock and may not be directly shared with seccomp.

Even if Landlock follows the footprints of seccomp, they are different
beasts.



signature.asc
Description: OpenPGP digital signature

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Mickaël Salaün]

On 28/02/2018 01:09, Andy Lutomirski wrote:
> On Wed, Feb 28, 2018 at 12:00 AM, Mickaël Salaün  wrote:
>>
>> On 28/02/2018 00:23, Andy Lutomirski wrote:
>>> On Tue, Feb 27, 2018 at 11:02 PM, Andy Lutomirski  wrote:
 On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:
>

 I think you're wrong here.  Any sane container trying to use Landlock
 like this would also create a PID namespace.  Problem solved.  I still
 think you should drop this patch.
>>
>> Containers is one use case, another is build-in sandboxing (e.g. for web
>> browser…) and another one is for sandbox managers (e.g. Firejail,
>> Bubblewrap, Flatpack…). In some of these use cases, especially from a
>> developer point of view, you may want/need to debug your applications
>> (without requiring to be root). For nested Landlock access-controls
>> (e.g. container + user session + web browser), it may not be allowed to
>> create a PID namespace, but you still want to have a meaningful
>> access-control.
>>
> 
> The consideration should be exactly the same as for normal seccomp.
> If I'm in a container (using PID namespaces + seccomp) and a run a web
> browser, I can debug the browser.
> 
> If there's a real use case for adding this type of automatic ptrace
> protection, then by all means, let's add it as a general seccomp
> feature.
> 

Right, it makes sense to add this feature to seccomp filters as well.
What do you think Kees?



signature.asc
Description: OpenPGP digital signature

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Mickaël Salaün]

On 28/02/2018 01:09, Andy Lutomirski wrote:
> On Wed, Feb 28, 2018 at 12:00 AM, Mickaël Salaün  wrote:
>>
>> On 28/02/2018 00:23, Andy Lutomirski wrote:
>>> On Tue, Feb 27, 2018 at 11:02 PM, Andy Lutomirski  wrote:
 On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:
>

 I think you're wrong here.  Any sane container trying to use Landlock
 like this would also create a PID namespace.  Problem solved.  I still
 think you should drop this patch.
>>
>> Containers is one use case, another is build-in sandboxing (e.g. for web
>> browser…) and another one is for sandbox managers (e.g. Firejail,
>> Bubblewrap, Flatpack…). In some of these use cases, especially from a
>> developer point of view, you may want/need to debug your applications
>> (without requiring to be root). For nested Landlock access-controls
>> (e.g. container + user session + web browser), it may not be allowed to
>> create a PID namespace, but you still want to have a meaningful
>> access-control.
>>
> 
> The consideration should be exactly the same as for normal seccomp.
> If I'm in a container (using PID namespaces + seccomp) and a run a web
> browser, I can debug the browser.
> 
> If there's a real use case for adding this type of automatic ptrace
> protection, then by all means, let's add it as a general seccomp
> feature.
> 

Right, it makes sense to add this feature to seccomp filters as well.
What do you think Kees?



signature.asc
Description: OpenPGP digital signature

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Andy Lutomirski]

On Wed, Feb 28, 2018 at 12:00 AM, Mickaël Salaün  wrote:
>
> On 28/02/2018 00:23, Andy Lutomirski wrote:
>> On Tue, Feb 27, 2018 at 11:02 PM, Andy Lutomirski  wrote:
>>> On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:

>>>
>>> I think you're wrong here.  Any sane container trying to use Landlock
>>> like this would also create a PID namespace.  Problem solved.  I still
>>> think you should drop this patch.
>
> Containers is one use case, another is build-in sandboxing (e.g. for web
> browser…) and another one is for sandbox managers (e.g. Firejail,
> Bubblewrap, Flatpack…). In some of these use cases, especially from a
> developer point of view, you may want/need to debug your applications
> (without requiring to be root). For nested Landlock access-controls
> (e.g. container + user session + web browser), it may not be allowed to
> create a PID namespace, but you still want to have a meaningful
> access-control.
>

The consideration should be exactly the same as for normal seccomp.
If I'm in a container (using PID namespaces + seccomp) and a run a web
browser, I can debug the browser.

If there's a real use case for adding this type of automatic ptrace
protection, then by all means, let's add it as a general seccomp
feature.

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Andy Lutomirski]

On Wed, Feb 28, 2018 at 12:00 AM, Mickaël Salaün  wrote:
>
> On 28/02/2018 00:23, Andy Lutomirski wrote:
>> On Tue, Feb 27, 2018 at 11:02 PM, Andy Lutomirski  wrote:
>>> On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:

>>>
>>> I think you're wrong here.  Any sane container trying to use Landlock
>>> like this would also create a PID namespace.  Problem solved.  I still
>>> think you should drop this patch.
>
> Containers is one use case, another is build-in sandboxing (e.g. for web
> browser…) and another one is for sandbox managers (e.g. Firejail,
> Bubblewrap, Flatpack…). In some of these use cases, especially from a
> developer point of view, you may want/need to debug your applications
> (without requiring to be root). For nested Landlock access-controls
> (e.g. container + user session + web browser), it may not be allowed to
> create a PID namespace, but you still want to have a meaningful
> access-control.
>

The consideration should be exactly the same as for normal seccomp.
If I'm in a container (using PID namespaces + seccomp) and a run a web
browser, I can debug the browser.

If there's a real use case for adding this type of automatic ptrace
protection, then by all means, let's add it as a general seccomp
feature.

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Mickaël Salaün]

On 28/02/2018 00:23, Andy Lutomirski wrote:
> On Tue, Feb 27, 2018 at 11:02 PM, Andy Lutomirski  wrote:
>> On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:
>>>
>>> On 27/02/2018 06:01, Andy Lutomirski wrote:


> On Feb 26, 2018, at 8:17 PM, Andy Lutomirski  wrote:
>
>> On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  
>> wrote:
>> A landlocked process has less privileges than a non-landlocked process
>> and must then be subject to additional restrictions when manipulating
>> processes. To be allowed to use ptrace(2) and related syscalls on a
>> target process, a landlocked process must have a subset of the target
>> process' rules.
>>
>> 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:
>> * factor out ptrace check
>> * constify pointers
>> * cleanup headers
>> * use the new security_add_hooks()
>> ---
>> security/landlock/Makefile   |   2 +-
>> security/landlock/hooks_ptrace.c | 124 
>> +++
>> security/landlock/hooks_ptrace.h |  11 
>> security/landlock/init.c |   2 +
>> 4 files changed, 138 insertions(+), 1 deletion(-)
>> 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 d0f532a93b4e..605504d852d3 100644
>> --- a/security/landlock/Makefile
>> +++ b/security/landlock/Makefile
>> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>> landlock-y := init.o chain.o task.o \
>>tag.o tag_fs.o \
>>enforce.o enforce_seccomp.o \
>> -   hooks.o hooks_cred.o hooks_fs.o
>> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
>> diff --git a/security/landlock/hooks_ptrace.c 
>> b/security/landlock/hooks_ptrace.c
>> new file mode 100644
>> index ..f1b977b9c808
>> --- /dev/null
>> +++ b/security/landlock/hooks_ptrace.c
>> @@ -0,0 +1,124 @@
>> +/*
>> + * Landlock LSM - ptrace hooks
>> + *
>> + * Copyright © 2017 Mickaël Salaün 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2, as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include  /* ARRAY_SIZE */
>> +#include 
>> +#include  /* struct task_struct */
>> +#include 
>> +
>> +#include "common.h" /* struct landlock_prog_set */
>> +#include "hooks.h" /* landlocked() */
>> +#include "hooks_ptrace.h"
>> +
>> +static bool progs_are_subset(const struct landlock_prog_set *parent,
>> +   const struct landlock_prog_set *child)
>> +{
>> +   size_t i;
>> +
>> +   if (!parent || !child)
>> +   return false;
>> +   if (parent == child)
>> +   return true;
>> +
>> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {
>
> ARRAY_SIZE(child->programs) seems misleading.  Is there no define
> NUM_LANDLOCK_PROG_TYPES or similar?
>
>> +   struct landlock_prog_list *walker;
>> +   bool found_parent = false;
>> +
>> +   if (!parent->programs[i])
>> +   continue;
>> +   for (walker = child->programs[i]; walker;
>> +   walker = walker->prev) {
>> +   if (walker == parent->programs[i]) {
>> +   found_parent = true;
>> +   break;
>> +   }
>> +   }
>> +   if (!found_parent)
>> +   return false;
>> +   }
>> +   return true;
>> +}
>
> If you used seccomp, you'd get this type of check for free, and it
> would be a lot easier to comprehend.  AFAICT the only extra leniency
> you're granting is that you're agnostic to the order in which the
> rules associated with different program types were applied, which
> could easily be added to seccomp.

 On second thought, this is all way too complicated.  I think the correct 
 logic is either "if you are filtered by Landlock, you cannot ptrace 
 anything" or to delete this patch entirely.
>>>

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Mickaël Salaün]

On 28/02/2018 00:23, Andy Lutomirski wrote:
> On Tue, Feb 27, 2018 at 11:02 PM, Andy Lutomirski  wrote:
>> On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:
>>>
>>> On 27/02/2018 06:01, Andy Lutomirski wrote:


> On Feb 26, 2018, at 8:17 PM, Andy Lutomirski  wrote:
>
>> On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  
>> wrote:
>> A landlocked process has less privileges than a non-landlocked process
>> and must then be subject to additional restrictions when manipulating
>> processes. To be allowed to use ptrace(2) and related syscalls on a
>> target process, a landlocked process must have a subset of the target
>> process' rules.
>>
>> 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:
>> * factor out ptrace check
>> * constify pointers
>> * cleanup headers
>> * use the new security_add_hooks()
>> ---
>> security/landlock/Makefile   |   2 +-
>> security/landlock/hooks_ptrace.c | 124 
>> +++
>> security/landlock/hooks_ptrace.h |  11 
>> security/landlock/init.c |   2 +
>> 4 files changed, 138 insertions(+), 1 deletion(-)
>> 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 d0f532a93b4e..605504d852d3 100644
>> --- a/security/landlock/Makefile
>> +++ b/security/landlock/Makefile
>> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>> landlock-y := init.o chain.o task.o \
>>tag.o tag_fs.o \
>>enforce.o enforce_seccomp.o \
>> -   hooks.o hooks_cred.o hooks_fs.o
>> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
>> diff --git a/security/landlock/hooks_ptrace.c 
>> b/security/landlock/hooks_ptrace.c
>> new file mode 100644
>> index ..f1b977b9c808
>> --- /dev/null
>> +++ b/security/landlock/hooks_ptrace.c
>> @@ -0,0 +1,124 @@
>> +/*
>> + * Landlock LSM - ptrace hooks
>> + *
>> + * Copyright © 2017 Mickaël Salaün 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2, as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include  /* ARRAY_SIZE */
>> +#include 
>> +#include  /* struct task_struct */
>> +#include 
>> +
>> +#include "common.h" /* struct landlock_prog_set */
>> +#include "hooks.h" /* landlocked() */
>> +#include "hooks_ptrace.h"
>> +
>> +static bool progs_are_subset(const struct landlock_prog_set *parent,
>> +   const struct landlock_prog_set *child)
>> +{
>> +   size_t i;
>> +
>> +   if (!parent || !child)
>> +   return false;
>> +   if (parent == child)
>> +   return true;
>> +
>> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {
>
> ARRAY_SIZE(child->programs) seems misleading.  Is there no define
> NUM_LANDLOCK_PROG_TYPES or similar?
>
>> +   struct landlock_prog_list *walker;
>> +   bool found_parent = false;
>> +
>> +   if (!parent->programs[i])
>> +   continue;
>> +   for (walker = child->programs[i]; walker;
>> +   walker = walker->prev) {
>> +   if (walker == parent->programs[i]) {
>> +   found_parent = true;
>> +   break;
>> +   }
>> +   }
>> +   if (!found_parent)
>> +   return false;
>> +   }
>> +   return true;
>> +}
>
> If you used seccomp, you'd get this type of check for free, and it
> would be a lot easier to comprehend.  AFAICT the only extra leniency
> you're granting is that you're agnostic to the order in which the
> rules associated with different program types were applied, which
> could easily be added to seccomp.

 On second thought, this is all way too complicated.  I think the correct 
 logic is either "if you are filtered by Landlock, you cannot ptrace 
 anything" or to delete this patch entirely.
>>>
>>> This does not fit a lot of use cases like running a container
>>> constrained with some Landlock programs. We should not deny users the
>>> ability to debug their stuff.
>>>
>>> This patch add the minimal protection which are needed to have
>>> meaningful 

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Andy Lutomirski]

On Tue, Feb 27, 2018 at 11:02 PM, Andy Lutomirski  wrote:
> On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:
>>
>> On 27/02/2018 06:01, Andy Lutomirski wrote:
>>>
>>>
 On Feb 26, 2018, at 8:17 PM, Andy Lutomirski  wrote:

> On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
> A landlocked process has less privileges than a non-landlocked process
> and must then be subject to additional restrictions when manipulating
> processes. To be allowed to use ptrace(2) and related syscalls on a
> target process, a landlocked process must have a subset of the target
> process' rules.
>
> 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:
> * factor out ptrace check
> * constify pointers
> * cleanup headers
> * use the new security_add_hooks()
> ---
> security/landlock/Makefile   |   2 +-
> security/landlock/hooks_ptrace.c | 124 
> +++
> security/landlock/hooks_ptrace.h |  11 
> security/landlock/init.c |   2 +
> 4 files changed, 138 insertions(+), 1 deletion(-)
> 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 d0f532a93b4e..605504d852d3 100644
> --- a/security/landlock/Makefile
> +++ b/security/landlock/Makefile
> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
> landlock-y := init.o chain.o task.o \
>tag.o tag_fs.o \
>enforce.o enforce_seccomp.o \
> -   hooks.o hooks_cred.o hooks_fs.o
> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
> diff --git a/security/landlock/hooks_ptrace.c 
> b/security/landlock/hooks_ptrace.c
> new file mode 100644
> index ..f1b977b9c808
> --- /dev/null
> +++ b/security/landlock/hooks_ptrace.c
> @@ -0,0 +1,124 @@
> +/*
> + * Landlock LSM - ptrace hooks
> + *
> + * Copyright © 2017 Mickaël Salaün 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include  /* ARRAY_SIZE */
> +#include 
> +#include  /* struct task_struct */
> +#include 
> +
> +#include "common.h" /* struct landlock_prog_set */
> +#include "hooks.h" /* landlocked() */
> +#include "hooks_ptrace.h"
> +
> +static bool progs_are_subset(const struct landlock_prog_set *parent,
> +   const struct landlock_prog_set *child)
> +{
> +   size_t i;
> +
> +   if (!parent || !child)
> +   return false;
> +   if (parent == child)
> +   return true;
> +
> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {

 ARRAY_SIZE(child->programs) seems misleading.  Is there no define
 NUM_LANDLOCK_PROG_TYPES or similar?

> +   struct landlock_prog_list *walker;
> +   bool found_parent = false;
> +
> +   if (!parent->programs[i])
> +   continue;
> +   for (walker = child->programs[i]; walker;
> +   walker = walker->prev) {
> +   if (walker == parent->programs[i]) {
> +   found_parent = true;
> +   break;
> +   }
> +   }
> +   if (!found_parent)
> +   return false;
> +   }
> +   return true;
> +}

 If you used seccomp, you'd get this type of check for free, and it
 would be a lot easier to comprehend.  AFAICT the only extra leniency
 you're granting is that you're agnostic to the order in which the
 rules associated with different program types were applied, which
 could easily be added to seccomp.
>>>
>>> On second thought, this is all way too complicated.  I think the correct 
>>> logic is either "if you are filtered by Landlock, you cannot ptrace 
>>> anything" or to delete this patch entirely.
>>
>> This does not fit a lot of use cases like running a container
>> constrained with some Landlock programs. We should not deny users the
>> ability to debug their stuff.
>>
>> 

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Andy Lutomirski]

On Tue, Feb 27, 2018 at 11:02 PM, Andy Lutomirski  wrote:
> On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:
>>
>> On 27/02/2018 06:01, Andy Lutomirski wrote:
>>>
>>>
 On Feb 26, 2018, at 8:17 PM, Andy Lutomirski  wrote:

> On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
> A landlocked process has less privileges than a non-landlocked process
> and must then be subject to additional restrictions when manipulating
> processes. To be allowed to use ptrace(2) and related syscalls on a
> target process, a landlocked process must have a subset of the target
> process' rules.
>
> 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:
> * factor out ptrace check
> * constify pointers
> * cleanup headers
> * use the new security_add_hooks()
> ---
> security/landlock/Makefile   |   2 +-
> security/landlock/hooks_ptrace.c | 124 
> +++
> security/landlock/hooks_ptrace.h |  11 
> security/landlock/init.c |   2 +
> 4 files changed, 138 insertions(+), 1 deletion(-)
> 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 d0f532a93b4e..605504d852d3 100644
> --- a/security/landlock/Makefile
> +++ b/security/landlock/Makefile
> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
> landlock-y := init.o chain.o task.o \
>tag.o tag_fs.o \
>enforce.o enforce_seccomp.o \
> -   hooks.o hooks_cred.o hooks_fs.o
> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
> diff --git a/security/landlock/hooks_ptrace.c 
> b/security/landlock/hooks_ptrace.c
> new file mode 100644
> index ..f1b977b9c808
> --- /dev/null
> +++ b/security/landlock/hooks_ptrace.c
> @@ -0,0 +1,124 @@
> +/*
> + * Landlock LSM - ptrace hooks
> + *
> + * Copyright © 2017 Mickaël Salaün 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include  /* ARRAY_SIZE */
> +#include 
> +#include  /* struct task_struct */
> +#include 
> +
> +#include "common.h" /* struct landlock_prog_set */
> +#include "hooks.h" /* landlocked() */
> +#include "hooks_ptrace.h"
> +
> +static bool progs_are_subset(const struct landlock_prog_set *parent,
> +   const struct landlock_prog_set *child)
> +{
> +   size_t i;
> +
> +   if (!parent || !child)
> +   return false;
> +   if (parent == child)
> +   return true;
> +
> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {

 ARRAY_SIZE(child->programs) seems misleading.  Is there no define
 NUM_LANDLOCK_PROG_TYPES or similar?

> +   struct landlock_prog_list *walker;
> +   bool found_parent = false;
> +
> +   if (!parent->programs[i])
> +   continue;
> +   for (walker = child->programs[i]; walker;
> +   walker = walker->prev) {
> +   if (walker == parent->programs[i]) {
> +   found_parent = true;
> +   break;
> +   }
> +   }
> +   if (!found_parent)
> +   return false;
> +   }
> +   return true;
> +}

 If you used seccomp, you'd get this type of check for free, and it
 would be a lot easier to comprehend.  AFAICT the only extra leniency
 you're granting is that you're agnostic to the order in which the
 rules associated with different program types were applied, which
 could easily be added to seccomp.
>>>
>>> On second thought, this is all way too complicated.  I think the correct 
>>> logic is either "if you are filtered by Landlock, you cannot ptrace 
>>> anything" or to delete this patch entirely.
>>
>> This does not fit a lot of use cases like running a container
>> constrained with some Landlock programs. We should not deny users the
>> ability to debug their stuff.
>>
>> This patch add the minimal protection which are needed to have
>> meaningful Landlock security policy. Without it, they may be easily
>> bypassable, hence useless.
>>
>
> I think you're wrong here.  Any sane container trying to use Landlock
> like this would 

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Andy Lutomirski]

On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:
>
> On 27/02/2018 06:01, Andy Lutomirski wrote:
>>
>>
>>> On Feb 26, 2018, at 8:17 PM, Andy Lutomirski  wrote:
>>>
 On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
 A landlocked process has less privileges than a non-landlocked process
 and must then be subject to additional restrictions when manipulating
 processes. To be allowed to use ptrace(2) and related syscalls on a
 target process, a landlocked process must have a subset of the target
 process' rules.

 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:
 * factor out ptrace check
 * constify pointers
 * cleanup headers
 * use the new security_add_hooks()
 ---
 security/landlock/Makefile   |   2 +-
 security/landlock/hooks_ptrace.c | 124 
 +++
 security/landlock/hooks_ptrace.h |  11 
 security/landlock/init.c |   2 +
 4 files changed, 138 insertions(+), 1 deletion(-)
 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 d0f532a93b4e..605504d852d3 100644
 --- a/security/landlock/Makefile
 +++ b/security/landlock/Makefile
 @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
 landlock-y := init.o chain.o task.o \
tag.o tag_fs.o \
enforce.o enforce_seccomp.o \
 -   hooks.o hooks_cred.o hooks_fs.o
 +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
 diff --git a/security/landlock/hooks_ptrace.c 
 b/security/landlock/hooks_ptrace.c
 new file mode 100644
 index ..f1b977b9c808
 --- /dev/null
 +++ b/security/landlock/hooks_ptrace.c
 @@ -0,0 +1,124 @@
 +/*
 + * Landlock LSM - ptrace hooks
 + *
 + * Copyright © 2017 Mickaël Salaün 
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2, as
 + * published by the Free Software Foundation.
 + */
 +
 +#include 
 +#include 
 +#include  /* ARRAY_SIZE */
 +#include 
 +#include  /* struct task_struct */
 +#include 
 +
 +#include "common.h" /* struct landlock_prog_set */
 +#include "hooks.h" /* landlocked() */
 +#include "hooks_ptrace.h"
 +
 +static bool progs_are_subset(const struct landlock_prog_set *parent,
 +   const struct landlock_prog_set *child)
 +{
 +   size_t i;
 +
 +   if (!parent || !child)
 +   return false;
 +   if (parent == child)
 +   return true;
 +
 +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {
>>>
>>> ARRAY_SIZE(child->programs) seems misleading.  Is there no define
>>> NUM_LANDLOCK_PROG_TYPES or similar?
>>>
 +   struct landlock_prog_list *walker;
 +   bool found_parent = false;
 +
 +   if (!parent->programs[i])
 +   continue;
 +   for (walker = child->programs[i]; walker;
 +   walker = walker->prev) {
 +   if (walker == parent->programs[i]) {
 +   found_parent = true;
 +   break;
 +   }
 +   }
 +   if (!found_parent)
 +   return false;
 +   }
 +   return true;
 +}
>>>
>>> If you used seccomp, you'd get this type of check for free, and it
>>> would be a lot easier to comprehend.  AFAICT the only extra leniency
>>> you're granting is that you're agnostic to the order in which the
>>> rules associated with different program types were applied, which
>>> could easily be added to seccomp.
>>
>> On second thought, this is all way too complicated.  I think the correct 
>> logic is either "if you are filtered by Landlock, you cannot ptrace 
>> anything" or to delete this patch entirely.
>
> This does not fit a lot of use cases like running a container
> constrained with some Landlock programs. We should not deny users the
> ability to debug their stuff.
>
> This patch add the minimal protection which are needed to have
> meaningful Landlock security policy. Without it, they may be easily
> bypassable, hence useless.
>

I think you're wrong here.  Any 

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Andy Lutomirski]

On Tue, Feb 27, 2018 at 10:14 PM, Mickaël Salaün  wrote:
>
> On 27/02/2018 06:01, Andy Lutomirski wrote:
>>
>>
>>> On Feb 26, 2018, at 8:17 PM, Andy Lutomirski  wrote:
>>>
 On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
 A landlocked process has less privileges than a non-landlocked process
 and must then be subject to additional restrictions when manipulating
 processes. To be allowed to use ptrace(2) and related syscalls on a
 target process, a landlocked process must have a subset of the target
 process' rules.

 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:
 * factor out ptrace check
 * constify pointers
 * cleanup headers
 * use the new security_add_hooks()
 ---
 security/landlock/Makefile   |   2 +-
 security/landlock/hooks_ptrace.c | 124 
 +++
 security/landlock/hooks_ptrace.h |  11 
 security/landlock/init.c |   2 +
 4 files changed, 138 insertions(+), 1 deletion(-)
 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 d0f532a93b4e..605504d852d3 100644
 --- a/security/landlock/Makefile
 +++ b/security/landlock/Makefile
 @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
 landlock-y := init.o chain.o task.o \
tag.o tag_fs.o \
enforce.o enforce_seccomp.o \
 -   hooks.o hooks_cred.o hooks_fs.o
 +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
 diff --git a/security/landlock/hooks_ptrace.c 
 b/security/landlock/hooks_ptrace.c
 new file mode 100644
 index ..f1b977b9c808
 --- /dev/null
 +++ b/security/landlock/hooks_ptrace.c
 @@ -0,0 +1,124 @@
 +/*
 + * Landlock LSM - ptrace hooks
 + *
 + * Copyright © 2017 Mickaël Salaün 
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2, as
 + * published by the Free Software Foundation.
 + */
 +
 +#include 
 +#include 
 +#include  /* ARRAY_SIZE */
 +#include 
 +#include  /* struct task_struct */
 +#include 
 +
 +#include "common.h" /* struct landlock_prog_set */
 +#include "hooks.h" /* landlocked() */
 +#include "hooks_ptrace.h"
 +
 +static bool progs_are_subset(const struct landlock_prog_set *parent,
 +   const struct landlock_prog_set *child)
 +{
 +   size_t i;
 +
 +   if (!parent || !child)
 +   return false;
 +   if (parent == child)
 +   return true;
 +
 +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {
>>>
>>> ARRAY_SIZE(child->programs) seems misleading.  Is there no define
>>> NUM_LANDLOCK_PROG_TYPES or similar?
>>>
 +   struct landlock_prog_list *walker;
 +   bool found_parent = false;
 +
 +   if (!parent->programs[i])
 +   continue;
 +   for (walker = child->programs[i]; walker;
 +   walker = walker->prev) {
 +   if (walker == parent->programs[i]) {
 +   found_parent = true;
 +   break;
 +   }
 +   }
 +   if (!found_parent)
 +   return false;
 +   }
 +   return true;
 +}
>>>
>>> If you used seccomp, you'd get this type of check for free, and it
>>> would be a lot easier to comprehend.  AFAICT the only extra leniency
>>> you're granting is that you're agnostic to the order in which the
>>> rules associated with different program types were applied, which
>>> could easily be added to seccomp.
>>
>> On second thought, this is all way too complicated.  I think the correct 
>> logic is either "if you are filtered by Landlock, you cannot ptrace 
>> anything" or to delete this patch entirely.
>
> This does not fit a lot of use cases like running a container
> constrained with some Landlock programs. We should not deny users the
> ability to debug their stuff.
>
> This patch add the minimal protection which are needed to have
> meaningful Landlock security policy. Without it, they may be easily
> bypassable, hence useless.
>

I think you're wrong here.  Any sane container trying to use Landlock
like this would also create a PID namespace.  Problem solved.  I still
think you should drop this patch.

>
>> If something like Tycho's notifiers goes in, then it's not obvious that, 
>> just because you 

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Mickaël Salaün]

On 27/02/2018 05:17, Andy Lutomirski wrote:
> On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
>> A landlocked process has less privileges than a non-landlocked process
>> and must then be subject to additional restrictions when manipulating
>> processes. To be allowed to use ptrace(2) and related syscalls on a
>> target process, a landlocked process must have a subset of the target
>> process' rules.
>>
>> 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:
>> * factor out ptrace check
>> * constify pointers
>> * cleanup headers
>> * use the new security_add_hooks()
>> ---
>>  security/landlock/Makefile   |   2 +-
>>  security/landlock/hooks_ptrace.c | 124 
>> +++
>>  security/landlock/hooks_ptrace.h |  11 
>>  security/landlock/init.c |   2 +
>>  4 files changed, 138 insertions(+), 1 deletion(-)
>>  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 d0f532a93b4e..605504d852d3 100644
>> --- a/security/landlock/Makefile
>> +++ b/security/landlock/Makefile
>> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>>  landlock-y := init.o chain.o task.o \
>> tag.o tag_fs.o \
>> enforce.o enforce_seccomp.o \
>> -   hooks.o hooks_cred.o hooks_fs.o
>> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
>> diff --git a/security/landlock/hooks_ptrace.c 
>> b/security/landlock/hooks_ptrace.c
>> new file mode 100644
>> index ..f1b977b9c808
>> --- /dev/null
>> +++ b/security/landlock/hooks_ptrace.c
>> @@ -0,0 +1,124 @@
>> +/*
>> + * Landlock LSM - ptrace hooks
>> + *
>> + * Copyright © 2017 Mickaël Salaün 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2, as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include  /* ARRAY_SIZE */
>> +#include 
>> +#include  /* struct task_struct */
>> +#include 
>> +
>> +#include "common.h" /* struct landlock_prog_set */
>> +#include "hooks.h" /* landlocked() */
>> +#include "hooks_ptrace.h"
>> +
>> +static bool progs_are_subset(const struct landlock_prog_set *parent,
>> +   const struct landlock_prog_set *child)
>> +{
>> +   size_t i;
>> +
>> +   if (!parent || !child)
>> +   return false;
>> +   if (parent == child)
>> +   return true;
>> +
>> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {
> 
> ARRAY_SIZE(child->programs) seems misleading.  Is there no define
> NUM_LANDLOCK_PROG_TYPES or similar?

Yes, there is _LANDLOCK_HOOK_LAST, but this code seems more readable
exactly because it does not require the developer (or the code checking
tools) to know about this static value.



signature.asc
Description: OpenPGP digital signature

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Mickaël Salaün]

On 27/02/2018 05:17, Andy Lutomirski wrote:
> On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
>> A landlocked process has less privileges than a non-landlocked process
>> and must then be subject to additional restrictions when manipulating
>> processes. To be allowed to use ptrace(2) and related syscalls on a
>> target process, a landlocked process must have a subset of the target
>> process' rules.
>>
>> 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:
>> * factor out ptrace check
>> * constify pointers
>> * cleanup headers
>> * use the new security_add_hooks()
>> ---
>>  security/landlock/Makefile   |   2 +-
>>  security/landlock/hooks_ptrace.c | 124 
>> +++
>>  security/landlock/hooks_ptrace.h |  11 
>>  security/landlock/init.c |   2 +
>>  4 files changed, 138 insertions(+), 1 deletion(-)
>>  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 d0f532a93b4e..605504d852d3 100644
>> --- a/security/landlock/Makefile
>> +++ b/security/landlock/Makefile
>> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>>  landlock-y := init.o chain.o task.o \
>> tag.o tag_fs.o \
>> enforce.o enforce_seccomp.o \
>> -   hooks.o hooks_cred.o hooks_fs.o
>> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
>> diff --git a/security/landlock/hooks_ptrace.c 
>> b/security/landlock/hooks_ptrace.c
>> new file mode 100644
>> index ..f1b977b9c808
>> --- /dev/null
>> +++ b/security/landlock/hooks_ptrace.c
>> @@ -0,0 +1,124 @@
>> +/*
>> + * Landlock LSM - ptrace hooks
>> + *
>> + * Copyright © 2017 Mickaël Salaün 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2, as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include  /* ARRAY_SIZE */
>> +#include 
>> +#include  /* struct task_struct */
>> +#include 
>> +
>> +#include "common.h" /* struct landlock_prog_set */
>> +#include "hooks.h" /* landlocked() */
>> +#include "hooks_ptrace.h"
>> +
>> +static bool progs_are_subset(const struct landlock_prog_set *parent,
>> +   const struct landlock_prog_set *child)
>> +{
>> +   size_t i;
>> +
>> +   if (!parent || !child)
>> +   return false;
>> +   if (parent == child)
>> +   return true;
>> +
>> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {
> 
> ARRAY_SIZE(child->programs) seems misleading.  Is there no define
> NUM_LANDLOCK_PROG_TYPES or similar?

Yes, there is _LANDLOCK_HOOK_LAST, but this code seems more readable
exactly because it does not require the developer (or the code checking
tools) to know about this static value.



signature.asc
Description: OpenPGP digital signature

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Mickaël Salaün]

On 27/02/2018 06:01, Andy Lutomirski wrote:
> 
> 
>> On Feb 26, 2018, at 8:17 PM, Andy Lutomirski  wrote:
>>
>>> On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
>>> A landlocked process has less privileges than a non-landlocked process
>>> and must then be subject to additional restrictions when manipulating
>>> processes. To be allowed to use ptrace(2) and related syscalls on a
>>> target process, a landlocked process must have a subset of the target
>>> process' rules.
>>>
>>> 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:
>>> * factor out ptrace check
>>> * constify pointers
>>> * cleanup headers
>>> * use the new security_add_hooks()
>>> ---
>>> security/landlock/Makefile   |   2 +-
>>> security/landlock/hooks_ptrace.c | 124 
>>> +++
>>> security/landlock/hooks_ptrace.h |  11 
>>> security/landlock/init.c |   2 +
>>> 4 files changed, 138 insertions(+), 1 deletion(-)
>>> 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 d0f532a93b4e..605504d852d3 100644
>>> --- a/security/landlock/Makefile
>>> +++ b/security/landlock/Makefile
>>> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>>> landlock-y := init.o chain.o task.o \
>>>tag.o tag_fs.o \
>>>enforce.o enforce_seccomp.o \
>>> -   hooks.o hooks_cred.o hooks_fs.o
>>> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
>>> diff --git a/security/landlock/hooks_ptrace.c 
>>> b/security/landlock/hooks_ptrace.c
>>> new file mode 100644
>>> index ..f1b977b9c808
>>> --- /dev/null
>>> +++ b/security/landlock/hooks_ptrace.c
>>> @@ -0,0 +1,124 @@
>>> +/*
>>> + * Landlock LSM - ptrace hooks
>>> + *
>>> + * Copyright © 2017 Mickaël Salaün 
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2, as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include  /* ARRAY_SIZE */
>>> +#include 
>>> +#include  /* struct task_struct */
>>> +#include 
>>> +
>>> +#include "common.h" /* struct landlock_prog_set */
>>> +#include "hooks.h" /* landlocked() */
>>> +#include "hooks_ptrace.h"
>>> +
>>> +static bool progs_are_subset(const struct landlock_prog_set *parent,
>>> +   const struct landlock_prog_set *child)
>>> +{
>>> +   size_t i;
>>> +
>>> +   if (!parent || !child)
>>> +   return false;
>>> +   if (parent == child)
>>> +   return true;
>>> +
>>> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {
>>
>> ARRAY_SIZE(child->programs) seems misleading.  Is there no define
>> NUM_LANDLOCK_PROG_TYPES or similar?
>>
>>> +   struct landlock_prog_list *walker;
>>> +   bool found_parent = false;
>>> +
>>> +   if (!parent->programs[i])
>>> +   continue;
>>> +   for (walker = child->programs[i]; walker;
>>> +   walker = walker->prev) {
>>> +   if (walker == parent->programs[i]) {
>>> +   found_parent = true;
>>> +   break;
>>> +   }
>>> +   }
>>> +   if (!found_parent)
>>> +   return false;
>>> +   }
>>> +   return true;
>>> +}
>>
>> If you used seccomp, you'd get this type of check for free, and it
>> would be a lot easier to comprehend.  AFAICT the only extra leniency
>> you're granting is that you're agnostic to the order in which the
>> rules associated with different program types were applied, which
>> could easily be added to seccomp.
> 
> On second thought, this is all way too complicated.  I think the correct 
> logic is either "if you are filtered by Landlock, you cannot ptrace anything" 
> or to delete this patch entirely.

This does not fit a lot of use cases like running a container
constrained with some Landlock programs. We should not deny users the
ability to debug their stuff.

This patch add the minimal protection which are needed to have
meaningful Landlock security policy. Without it, they may be easily
bypassable, hence useless.


> If something like Tycho's notifiers goes in, then it's not obvious that, just 
> because you have the same set of filters, you have the same privilege.  
> Similarly, if a feature that lets a filter query its cgroup goes in (and you 
> 

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Mickaël Salaün]

On 27/02/2018 06:01, Andy Lutomirski wrote:
> 
> 
>> On Feb 26, 2018, at 8:17 PM, Andy Lutomirski  wrote:
>>
>>> On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
>>> A landlocked process has less privileges than a non-landlocked process
>>> and must then be subject to additional restrictions when manipulating
>>> processes. To be allowed to use ptrace(2) and related syscalls on a
>>> target process, a landlocked process must have a subset of the target
>>> process' rules.
>>>
>>> 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:
>>> * factor out ptrace check
>>> * constify pointers
>>> * cleanup headers
>>> * use the new security_add_hooks()
>>> ---
>>> security/landlock/Makefile   |   2 +-
>>> security/landlock/hooks_ptrace.c | 124 
>>> +++
>>> security/landlock/hooks_ptrace.h |  11 
>>> security/landlock/init.c |   2 +
>>> 4 files changed, 138 insertions(+), 1 deletion(-)
>>> 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 d0f532a93b4e..605504d852d3 100644
>>> --- a/security/landlock/Makefile
>>> +++ b/security/landlock/Makefile
>>> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>>> landlock-y := init.o chain.o task.o \
>>>tag.o tag_fs.o \
>>>enforce.o enforce_seccomp.o \
>>> -   hooks.o hooks_cred.o hooks_fs.o
>>> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
>>> diff --git a/security/landlock/hooks_ptrace.c 
>>> b/security/landlock/hooks_ptrace.c
>>> new file mode 100644
>>> index ..f1b977b9c808
>>> --- /dev/null
>>> +++ b/security/landlock/hooks_ptrace.c
>>> @@ -0,0 +1,124 @@
>>> +/*
>>> + * Landlock LSM - ptrace hooks
>>> + *
>>> + * Copyright © 2017 Mickaël Salaün 
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2, as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include  /* ARRAY_SIZE */
>>> +#include 
>>> +#include  /* struct task_struct */
>>> +#include 
>>> +
>>> +#include "common.h" /* struct landlock_prog_set */
>>> +#include "hooks.h" /* landlocked() */
>>> +#include "hooks_ptrace.h"
>>> +
>>> +static bool progs_are_subset(const struct landlock_prog_set *parent,
>>> +   const struct landlock_prog_set *child)
>>> +{
>>> +   size_t i;
>>> +
>>> +   if (!parent || !child)
>>> +   return false;
>>> +   if (parent == child)
>>> +   return true;
>>> +
>>> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {
>>
>> ARRAY_SIZE(child->programs) seems misleading.  Is there no define
>> NUM_LANDLOCK_PROG_TYPES or similar?
>>
>>> +   struct landlock_prog_list *walker;
>>> +   bool found_parent = false;
>>> +
>>> +   if (!parent->programs[i])
>>> +   continue;
>>> +   for (walker = child->programs[i]; walker;
>>> +   walker = walker->prev) {
>>> +   if (walker == parent->programs[i]) {
>>> +   found_parent = true;
>>> +   break;
>>> +   }
>>> +   }
>>> +   if (!found_parent)
>>> +   return false;
>>> +   }
>>> +   return true;
>>> +}
>>
>> If you used seccomp, you'd get this type of check for free, and it
>> would be a lot easier to comprehend.  AFAICT the only extra leniency
>> you're granting is that you're agnostic to the order in which the
>> rules associated with different program types were applied, which
>> could easily be added to seccomp.
> 
> On second thought, this is all way too complicated.  I think the correct 
> logic is either "if you are filtered by Landlock, you cannot ptrace anything" 
> or to delete this patch entirely.

This does not fit a lot of use cases like running a container
constrained with some Landlock programs. We should not deny users the
ability to debug their stuff.

This patch add the minimal protection which are needed to have
meaningful Landlock security policy. Without it, they may be easily
bypassable, hence useless.


> If something like Tycho's notifiers goes in, then it's not obvious that, just 
> because you have the same set of filters, you have the same privilege.  
> Similarly, if a feature that lets a filter query its cgroup goes in (and you 
> proposed this once!) then the logic you implemented here is wrong.

I don't get your point. Please take a look at the tests (patch 10).

> 
> Or you could just say that it's the responsibility of a Landlock user to 
> properly 

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Andy Lutomirski]

> On Feb 26, 2018, at 8:17 PM, Andy Lutomirski  wrote:
> 
>> On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
>> A landlocked process has less privileges than a non-landlocked process
>> and must then be subject to additional restrictions when manipulating
>> processes. To be allowed to use ptrace(2) and related syscalls on a
>> target process, a landlocked process must have a subset of the target
>> process' rules.
>> 
>> 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:
>> * factor out ptrace check
>> * constify pointers
>> * cleanup headers
>> * use the new security_add_hooks()
>> ---
>> security/landlock/Makefile   |   2 +-
>> security/landlock/hooks_ptrace.c | 124 
>> +++
>> security/landlock/hooks_ptrace.h |  11 
>> security/landlock/init.c |   2 +
>> 4 files changed, 138 insertions(+), 1 deletion(-)
>> 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 d0f532a93b4e..605504d852d3 100644
>> --- a/security/landlock/Makefile
>> +++ b/security/landlock/Makefile
>> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>> landlock-y := init.o chain.o task.o \
>>tag.o tag_fs.o \
>>enforce.o enforce_seccomp.o \
>> -   hooks.o hooks_cred.o hooks_fs.o
>> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
>> diff --git a/security/landlock/hooks_ptrace.c 
>> b/security/landlock/hooks_ptrace.c
>> new file mode 100644
>> index ..f1b977b9c808
>> --- /dev/null
>> +++ b/security/landlock/hooks_ptrace.c
>> @@ -0,0 +1,124 @@
>> +/*
>> + * Landlock LSM - ptrace hooks
>> + *
>> + * Copyright © 2017 Mickaël Salaün 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2, as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include  /* ARRAY_SIZE */
>> +#include 
>> +#include  /* struct task_struct */
>> +#include 
>> +
>> +#include "common.h" /* struct landlock_prog_set */
>> +#include "hooks.h" /* landlocked() */
>> +#include "hooks_ptrace.h"
>> +
>> +static bool progs_are_subset(const struct landlock_prog_set *parent,
>> +   const struct landlock_prog_set *child)
>> +{
>> +   size_t i;
>> +
>> +   if (!parent || !child)
>> +   return false;
>> +   if (parent == child)
>> +   return true;
>> +
>> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {
> 
> ARRAY_SIZE(child->programs) seems misleading.  Is there no define
> NUM_LANDLOCK_PROG_TYPES or similar?
> 
>> +   struct landlock_prog_list *walker;
>> +   bool found_parent = false;
>> +
>> +   if (!parent->programs[i])
>> +   continue;
>> +   for (walker = child->programs[i]; walker;
>> +   walker = walker->prev) {
>> +   if (walker == parent->programs[i]) {
>> +   found_parent = true;
>> +   break;
>> +   }
>> +   }
>> +   if (!found_parent)
>> +   return false;
>> +   }
>> +   return true;
>> +}
> 
> If you used seccomp, you'd get this type of check for free, and it
> would be a lot easier to comprehend.  AFAICT the only extra leniency
> you're granting is that you're agnostic to the order in which the
> rules associated with different program types were applied, which
> could easily be added to seccomp.

On second thought, this is all way too complicated.  I think the correct logic 
is either "if you are filtered by Landlock, you cannot ptrace anything" or to 
delete this patch entirely. If something like Tycho's notifiers goes in, then 
it's not obvious that, just because you have the same set of filters, you have 
the same privilege.  Similarly, if a feature that lets a filter query its 
cgroup goes in (and you proposed this once!) then the logic you implemented 
here is wrong.

Or you could just say that it's the responsibility of a Landlock user to 
properly filter ptrace() just like it's the responsibility of seccomp users to 
filter ptrace if needed.

I take this as further evidence that Landlock makes much more sense as part of 
seccomp than as a totally separate thing.  We've very carefully reviewed these 
things for seccomp.  Please don't make us do it again from scratch.

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Andy Lutomirski]

> On Feb 26, 2018, at 8:17 PM, Andy Lutomirski  wrote:
> 
>> On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
>> A landlocked process has less privileges than a non-landlocked process
>> and must then be subject to additional restrictions when manipulating
>> processes. To be allowed to use ptrace(2) and related syscalls on a
>> target process, a landlocked process must have a subset of the target
>> process' rules.
>> 
>> 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:
>> * factor out ptrace check
>> * constify pointers
>> * cleanup headers
>> * use the new security_add_hooks()
>> ---
>> security/landlock/Makefile   |   2 +-
>> security/landlock/hooks_ptrace.c | 124 
>> +++
>> security/landlock/hooks_ptrace.h |  11 
>> security/landlock/init.c |   2 +
>> 4 files changed, 138 insertions(+), 1 deletion(-)
>> 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 d0f532a93b4e..605504d852d3 100644
>> --- a/security/landlock/Makefile
>> +++ b/security/landlock/Makefile
>> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>> landlock-y := init.o chain.o task.o \
>>tag.o tag_fs.o \
>>enforce.o enforce_seccomp.o \
>> -   hooks.o hooks_cred.o hooks_fs.o
>> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
>> diff --git a/security/landlock/hooks_ptrace.c 
>> b/security/landlock/hooks_ptrace.c
>> new file mode 100644
>> index ..f1b977b9c808
>> --- /dev/null
>> +++ b/security/landlock/hooks_ptrace.c
>> @@ -0,0 +1,124 @@
>> +/*
>> + * Landlock LSM - ptrace hooks
>> + *
>> + * Copyright © 2017 Mickaël Salaün 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2, as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include  /* ARRAY_SIZE */
>> +#include 
>> +#include  /* struct task_struct */
>> +#include 
>> +
>> +#include "common.h" /* struct landlock_prog_set */
>> +#include "hooks.h" /* landlocked() */
>> +#include "hooks_ptrace.h"
>> +
>> +static bool progs_are_subset(const struct landlock_prog_set *parent,
>> +   const struct landlock_prog_set *child)
>> +{
>> +   size_t i;
>> +
>> +   if (!parent || !child)
>> +   return false;
>> +   if (parent == child)
>> +   return true;
>> +
>> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {
> 
> ARRAY_SIZE(child->programs) seems misleading.  Is there no define
> NUM_LANDLOCK_PROG_TYPES or similar?
> 
>> +   struct landlock_prog_list *walker;
>> +   bool found_parent = false;
>> +
>> +   if (!parent->programs[i])
>> +   continue;
>> +   for (walker = child->programs[i]; walker;
>> +   walker = walker->prev) {
>> +   if (walker == parent->programs[i]) {
>> +   found_parent = true;
>> +   break;
>> +   }
>> +   }
>> +   if (!found_parent)
>> +   return false;
>> +   }
>> +   return true;
>> +}
> 
> If you used seccomp, you'd get this type of check for free, and it
> would be a lot easier to comprehend.  AFAICT the only extra leniency
> you're granting is that you're agnostic to the order in which the
> rules associated with different program types were applied, which
> could easily be added to seccomp.

On second thought, this is all way too complicated.  I think the correct logic 
is either "if you are filtered by Landlock, you cannot ptrace anything" or to 
delete this patch entirely. If something like Tycho's notifiers goes in, then 
it's not obvious that, just because you have the same set of filters, you have 
the same privilege.  Similarly, if a feature that lets a filter query its 
cgroup goes in (and you proposed this once!) then the logic you implemented 
here is wrong.

Or you could just say that it's the responsibility of a Landlock user to 
properly filter ptrace() just like it's the responsibility of seccomp users to 
filter ptrace if needed.

I take this as further evidence that Landlock makes much more sense as part of 
seccomp than as a totally separate thing.  We've very carefully reviewed these 
things for seccomp.  Please don't make us do it again from scratch.

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Andy Lutomirski]

On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
> A landlocked process has less privileges than a non-landlocked process
> and must then be subject to additional restrictions when manipulating
> processes. To be allowed to use ptrace(2) and related syscalls on a
> target process, a landlocked process must have a subset of the target
> process' rules.
>
> 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:
> * factor out ptrace check
> * constify pointers
> * cleanup headers
> * use the new security_add_hooks()
> ---
>  security/landlock/Makefile   |   2 +-
>  security/landlock/hooks_ptrace.c | 124 
> +++
>  security/landlock/hooks_ptrace.h |  11 
>  security/landlock/init.c |   2 +
>  4 files changed, 138 insertions(+), 1 deletion(-)
>  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 d0f532a93b4e..605504d852d3 100644
> --- a/security/landlock/Makefile
> +++ b/security/landlock/Makefile
> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>  landlock-y := init.o chain.o task.o \
> tag.o tag_fs.o \
> enforce.o enforce_seccomp.o \
> -   hooks.o hooks_cred.o hooks_fs.o
> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
> diff --git a/security/landlock/hooks_ptrace.c 
> b/security/landlock/hooks_ptrace.c
> new file mode 100644
> index ..f1b977b9c808
> --- /dev/null
> +++ b/security/landlock/hooks_ptrace.c
> @@ -0,0 +1,124 @@
> +/*
> + * Landlock LSM - ptrace hooks
> + *
> + * Copyright © 2017 Mickaël Salaün 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include  /* ARRAY_SIZE */
> +#include 
> +#include  /* struct task_struct */
> +#include 
> +
> +#include "common.h" /* struct landlock_prog_set */
> +#include "hooks.h" /* landlocked() */
> +#include "hooks_ptrace.h"
> +
> +static bool progs_are_subset(const struct landlock_prog_set *parent,
> +   const struct landlock_prog_set *child)
> +{
> +   size_t i;
> +
> +   if (!parent || !child)
> +   return false;
> +   if (parent == child)
> +   return true;
> +
> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {

ARRAY_SIZE(child->programs) seems misleading.  Is there no define
NUM_LANDLOCK_PROG_TYPES or similar?

> +   struct landlock_prog_list *walker;
> +   bool found_parent = false;
> +
> +   if (!parent->programs[i])
> +   continue;
> +   for (walker = child->programs[i]; walker;
> +   walker = walker->prev) {
> +   if (walker == parent->programs[i]) {
> +   found_parent = true;
> +   break;
> +   }
> +   }
> +   if (!found_parent)
> +   return false;
> +   }
> +   return true;
> +}

If you used seccomp, you'd get this type of check for free, and it
would be a lot easier to comprehend.  AFAICT the only extra leniency
you're granting is that you're agnostic to the order in which the
rules associated with different program types were applied, which
could easily be added to seccomp.

Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions

[Andy Lutomirski]

On Tue, Feb 27, 2018 at 12:41 AM, Mickaël Salaün  wrote:
> A landlocked process has less privileges than a non-landlocked process
> and must then be subject to additional restrictions when manipulating
> processes. To be allowed to use ptrace(2) and related syscalls on a
> target process, a landlocked process must have a subset of the target
> process' rules.
>
> 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:
> * factor out ptrace check
> * constify pointers
> * cleanup headers
> * use the new security_add_hooks()
> ---
>  security/landlock/Makefile   |   2 +-
>  security/landlock/hooks_ptrace.c | 124 
> +++
>  security/landlock/hooks_ptrace.h |  11 
>  security/landlock/init.c |   2 +
>  4 files changed, 138 insertions(+), 1 deletion(-)
>  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 d0f532a93b4e..605504d852d3 100644
> --- a/security/landlock/Makefile
> +++ b/security/landlock/Makefile
> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>  landlock-y := init.o chain.o task.o \
> tag.o tag_fs.o \
> enforce.o enforce_seccomp.o \
> -   hooks.o hooks_cred.o hooks_fs.o
> +   hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o
> diff --git a/security/landlock/hooks_ptrace.c 
> b/security/landlock/hooks_ptrace.c
> new file mode 100644
> index ..f1b977b9c808
> --- /dev/null
> +++ b/security/landlock/hooks_ptrace.c
> @@ -0,0 +1,124 @@
> +/*
> + * Landlock LSM - ptrace hooks
> + *
> + * Copyright © 2017 Mickaël Salaün 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include  /* ARRAY_SIZE */
> +#include 
> +#include  /* struct task_struct */
> +#include 
> +
> +#include "common.h" /* struct landlock_prog_set */
> +#include "hooks.h" /* landlocked() */
> +#include "hooks_ptrace.h"
> +
> +static bool progs_are_subset(const struct landlock_prog_set *parent,
> +   const struct landlock_prog_set *child)
> +{
> +   size_t i;
> +
> +   if (!parent || !child)
> +   return false;
> +   if (parent == child)
> +   return true;
> +
> +   for (i = 0; i < ARRAY_SIZE(child->programs); i++) {

ARRAY_SIZE(child->programs) seems misleading.  Is there no define
NUM_LANDLOCK_PROG_TYPES or similar?

> +   struct landlock_prog_list *walker;
> +   bool found_parent = false;
> +
> +   if (!parent->programs[i])
> +   continue;
> +   for (walker = child->programs[i]; walker;
> +   walker = walker->prev) {
> +   if (walker == parent->programs[i]) {
> +   found_parent = true;
> +   break;
> +   }
> +   }
> +   if (!found_parent)
> +   return false;
> +   }
> +   return true;
> +}

If you used seccomp, you'd get this type of check for free, and it
would be a lot easier to comprehend.  AFAICT the only extra leniency
you're granting is that you're agnostic to the order in which the
rules associated with different program types were applied, which
could easily be added to seccomp.