Re: [kernel-hardening] [PATCH net-next v6 06/11] seccomp,landlock: Handle Landlock events per process hierarchy

[Kees Cook]

On Fri, Mar 31, 2017 at 2:15 PM, Mickaël Salaün  wrote:
>
>
> On 29/03/2017 12:35, Djalal Harouni wrote:
>> On Wed, Mar 29, 2017 at 1:46 AM, Mickaël Salaün  wrote:
>
>>> @@ -25,6 +30,9 @@ struct seccomp_filter;
>>>  struct seccomp {
>>> int mode;
>>> struct seccomp_filter *filter;
>>> +#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_SECURITY_LANDLOCK)
>>> +   struct landlock_events *landlock_events;
>>> +#endif /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK */
>>>  };
>>
>> Sorry if this was discussed before, but since this is mean to be a
>> stackable LSM, I'm wondering if later you could move the events from
>> seccomp, and go with a security_task_alloc() model [1] ?
>>
>> Thanks!
>>
>> [1] 
>> http://kernsec.org/pipermail/linux-security-module-archive/2017-March/000184.html
>>
>
> Landlock use the seccomp syscall to attach a rule to a process and using
> struct seccomp to store this rule make sense. There is currently no way
> to store multiple task->security, which is needed for a stackable LSM
> like Landlock, but we could move the events there if needed in the future.

It does stand out to me that the only thing landlock is using seccomp
for is its syscall... :P

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] [PATCH net-next v6 06/11] seccomp,landlock: Handle Landlock events per process hierarchy

[Kees Cook]

On Fri, Mar 31, 2017 at 2:15 PM, Mickaël Salaün  wrote:
>
>
> On 29/03/2017 12:35, Djalal Harouni wrote:
>> On Wed, Mar 29, 2017 at 1:46 AM, Mickaël Salaün  wrote:
>
>>> @@ -25,6 +30,9 @@ struct seccomp_filter;
>>>  struct seccomp {
>>> int mode;
>>> struct seccomp_filter *filter;
>>> +#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_SECURITY_LANDLOCK)
>>> +   struct landlock_events *landlock_events;
>>> +#endif /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK */
>>>  };
>>
>> Sorry if this was discussed before, but since this is mean to be a
>> stackable LSM, I'm wondering if later you could move the events from
>> seccomp, and go with a security_task_alloc() model [1] ?
>>
>> Thanks!
>>
>> [1] 
>> http://kernsec.org/pipermail/linux-security-module-archive/2017-March/000184.html
>>
>
> Landlock use the seccomp syscall to attach a rule to a process and using
> struct seccomp to store this rule make sense. There is currently no way
> to store multiple task->security, which is needed for a stackable LSM
> like Landlock, but we could move the events there if needed in the future.

It does stand out to me that the only thing landlock is using seccomp
for is its syscall... :P

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] [PATCH net-next v6 06/11] seccomp,landlock: Handle Landlock events per process hierarchy

[Mickaël Salaün]

On 29/03/2017 12:35, Djalal Harouni wrote:
> On Wed, Mar 29, 2017 at 1:46 AM, Mickaël Salaün  wrote:

>> @@ -25,6 +30,9 @@ struct seccomp_filter;
>>  struct seccomp {
>> int mode;
>> struct seccomp_filter *filter;
>> +#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_SECURITY_LANDLOCK)
>> +   struct landlock_events *landlock_events;
>> +#endif /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK */
>>  };
> 
> Sorry if this was discussed before, but since this is mean to be a
> stackable LSM, I'm wondering if later you could move the events from
> seccomp, and go with a security_task_alloc() model [1] ?
> 
> Thanks!
> 
> [1] 
> http://kernsec.org/pipermail/linux-security-module-archive/2017-March/000184.html
> 

Landlock use the seccomp syscall to attach a rule to a process and using
struct seccomp to store this rule make sense. There is currently no way
to store multiple task->security, which is needed for a stackable LSM
like Landlock, but we could move the events there if needed in the future.

 Mickaël



signature.asc
Description: OpenPGP digital signature

Re: [kernel-hardening] [PATCH net-next v6 06/11] seccomp,landlock: Handle Landlock events per process hierarchy

[Mickaël Salaün]

On 29/03/2017 12:35, Djalal Harouni wrote:
> On Wed, Mar 29, 2017 at 1:46 AM, Mickaël Salaün  wrote:

>> @@ -25,6 +30,9 @@ struct seccomp_filter;
>>  struct seccomp {
>> int mode;
>> struct seccomp_filter *filter;
>> +#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_SECURITY_LANDLOCK)
>> +   struct landlock_events *landlock_events;
>> +#endif /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK */
>>  };
> 
> Sorry if this was discussed before, but since this is mean to be a
> stackable LSM, I'm wondering if later you could move the events from
> seccomp, and go with a security_task_alloc() model [1] ?
> 
> Thanks!
> 
> [1] 
> http://kernsec.org/pipermail/linux-security-module-archive/2017-March/000184.html
> 

Landlock use the seccomp syscall to attach a rule to a process and using
struct seccomp to store this rule make sense. There is currently no way
to store multiple task->security, which is needed for a stackable LSM
like Landlock, but we could move the events there if needed in the future.

 Mickaël



signature.asc
Description: OpenPGP digital signature

Re: [kernel-hardening] [PATCH net-next v6 06/11] seccomp,landlock: Handle Landlock events per process hierarchy

[Djalal Harouni]

On Wed, Mar 29, 2017 at 1:46 AM, Mickaël Salaün  wrote:
> The seccomp(2) syscall can be used by a task to apply a Landlock rule to
> itself. As a seccomp filter, a Landlock rule is enforced for the current
> task and all its future children. A rule is immutable and a task can
> only add new restricting rules to itself, forming a chain of rules.
>
> A Landlock rule is tied to a Landlock event. If the use of a kernel
> object is allowed by the other Linux security mechanisms (e.g. DAC,
> capabilities, other LSM), then a Landlock event related to this kind of
> object is triggered. The chain of rules for this event is then
> evaluated. Each rule return a 32-bit value which can deny the use of a
> kernel object with a non-zero value. If every rules of the chain return
> zero, then the use of the object is allowed.
>
> 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
>
> Signed-off-by: Mickaël Salaün 
> Cc: Alexei Starovoitov 
> Cc: Andrew Morton 
> Cc: Andy Lutomirski 
> Cc: James Morris 
> Cc: Kees Cook 
> Cc: Serge E. Hallyn 
> Cc: Will Drewry 
> Link: 
> https://lkml.kernel.org/r/c10a503d-5e35-7785-2f3d-25ed8dd63...@digikod.net
> ---
>  include/linux/landlock.h  |  36 +++
>  include/linux/seccomp.h   |   8 ++
>  include/uapi/linux/seccomp.h  |   1 +
>  kernel/fork.c |  14 ++-
>  kernel/seccomp.c  |   8 ++
>  security/landlock/Makefile|   2 +-
>  security/landlock/hooks.c |  37 +++
>  security/landlock/hooks.h |   5 +
>  security/landlock/init.c  |   3 +-
>  security/landlock/providers.c | 232 
> ++
>  10 files changed, 342 insertions(+), 4 deletions(-)
>  create mode 100644 security/landlock/providers.c
>
> diff --git a/include/linux/landlock.h b/include/linux/landlock.h
> index 53013dc374fe..c40ee78e86e0 100644
> --- a/include/linux/landlock.h
> +++ b/include/linux/landlock.h
> @@ -12,6 +12,9 @@
>  #define _LINUX_LANDLOCK_H
>  #ifdef CONFIG_SECURITY_LANDLOCK
>
> +#include  /* _LANDLOCK_SUBTYPE_EVENT_LAST */
> +#include  /* atomic_t */
> +
>  /*
>   * This is not intended for the UAPI headers. Each userland software should 
> use
>   * a static minimal version for the required features as explained in the
> @@ -19,5 +22,38 @@
>   */
>  #define LANDLOCK_VERSION 1
>
> +struct landlock_rule {
> +   atomic_t usage;
> +   struct landlock_rule *prev;
> +   struct bpf_prog *prog;
> +};
> +
> +/**
> + * struct landlock_events - Landlock event rules enforced on a thread
> + *
> + * This is used for low performance impact when forking a process. Instead of
> + * copying the full array and incrementing the usage of each entries, only
> + * create a pointer to  landlock_events and increments its usage. When
> + * appending a new rule, if  landlock_events is shared with other 
> tasks,
> + * then duplicate it and append the rule to this new  landlock_events.
> + *
> + * @usage: reference count to manage the object lifetime. When a thread need 
> to
> + * add Landlock rules and if @usage is greater than 1, then the 
> thread
> + * must duplicate  landlock_events to not change the 
> children's
> + * rules as well.
> + * @rules: array of non-NULL  landlock_rule pointers
> + */
> +struct 

Re: [kernel-hardening] [PATCH net-next v6 06/11] seccomp,landlock: Handle Landlock events per process hierarchy

[Djalal Harouni]

On Wed, Mar 29, 2017 at 1:46 AM, Mickaël Salaün  wrote:
> The seccomp(2) syscall can be used by a task to apply a Landlock rule to
> itself. As a seccomp filter, a Landlock rule is enforced for the current
> task and all its future children. A rule is immutable and a task can
> only add new restricting rules to itself, forming a chain of rules.
>
> A Landlock rule is tied to a Landlock event. If the use of a kernel
> object is allowed by the other Linux security mechanisms (e.g. DAC,
> capabilities, other LSM), then a Landlock event related to this kind of
> object is triggered. The chain of rules for this event is then
> evaluated. Each rule return a 32-bit value which can deny the use of a
> kernel object with a non-zero value. If every rules of the chain return
> zero, then the use of the object is allowed.
>
> 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
>
> Signed-off-by: Mickaël Salaün 
> Cc: Alexei Starovoitov 
> Cc: Andrew Morton 
> Cc: Andy Lutomirski 
> Cc: James Morris 
> Cc: Kees Cook 
> Cc: Serge E. Hallyn 
> Cc: Will Drewry 
> Link: 
> https://lkml.kernel.org/r/c10a503d-5e35-7785-2f3d-25ed8dd63...@digikod.net
> ---
>  include/linux/landlock.h  |  36 +++
>  include/linux/seccomp.h   |   8 ++
>  include/uapi/linux/seccomp.h  |   1 +
>  kernel/fork.c |  14 ++-
>  kernel/seccomp.c  |   8 ++
>  security/landlock/Makefile|   2 +-
>  security/landlock/hooks.c |  37 +++
>  security/landlock/hooks.h |   5 +
>  security/landlock/init.c  |   3 +-
>  security/landlock/providers.c | 232 
> ++
>  10 files changed, 342 insertions(+), 4 deletions(-)
>  create mode 100644 security/landlock/providers.c
>
> diff --git a/include/linux/landlock.h b/include/linux/landlock.h
> index 53013dc374fe..c40ee78e86e0 100644
> --- a/include/linux/landlock.h
> +++ b/include/linux/landlock.h
> @@ -12,6 +12,9 @@
>  #define _LINUX_LANDLOCK_H
>  #ifdef CONFIG_SECURITY_LANDLOCK
>
> +#include  /* _LANDLOCK_SUBTYPE_EVENT_LAST */
> +#include  /* atomic_t */
> +
>  /*
>   * This is not intended for the UAPI headers. Each userland software should 
> use
>   * a static minimal version for the required features as explained in the
> @@ -19,5 +22,38 @@
>   */
>  #define LANDLOCK_VERSION 1
>
> +struct landlock_rule {
> +   atomic_t usage;
> +   struct landlock_rule *prev;
> +   struct bpf_prog *prog;
> +};
> +
> +/**
> + * struct landlock_events - Landlock event rules enforced on a thread
> + *
> + * This is used for low performance impact when forking a process. Instead of
> + * copying the full array and incrementing the usage of each entries, only
> + * create a pointer to  landlock_events and increments its usage. When
> + * appending a new rule, if  landlock_events is shared with other 
> tasks,
> + * then duplicate it and append the rule to this new  landlock_events.
> + *
> + * @usage: reference count to manage the object lifetime. When a thread need 
> to
> + * add Landlock rules and if @usage is greater than 1, then the 
> thread
> + * must duplicate  landlock_events to not change the 
> children's
> + * rules as well.
> + * @rules: array of non-NULL  landlock_rule pointers
> + */
> +struct landlock_events {
> +   atomic_t usage;
> +   struct landlock_rule *rules[_LANDLOCK_SUBTYPE_EVENT_LAST];
> +};
> +
> +void put_landlock_events(struct landlock_events *events);
> +
>