Re: [PATCH net-next v6 00/11] Landlock LSM: Toward unprivileged sandboxing

2017-04-18 Thread Mickaël Salaün

On 19/04/2017 01:26, Kees Cook wrote:
> On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün  wrote:
>> This sixth series add some changes to the previous one [1], including a 
>> simpler
>> rule inheritance hierarchy (similar to seccomp-bpf), a ptrace scope 
>> protection,
>> some file renaming (better feature identification per file), a future-proof
>> eBPF subtype and miscellaneous cosmetic fixes.
> 
> Sorry for the delay in review! I finally had a chunk of time I could
> devote to this. I really like how it's heading. I still wonder about
> its overlap with seccomp (it's really only using the syscall now...),
> but that's just a detail. Getting the abstraction away from direct LSM
> hooks looks good.
> 
>> There is as yet no way to allow a process to access only a subset of the
>> filesystem where the subset is specified via a path or a file descriptor.  
>> This
>> feature is intentionally left out so as to minimize the amount of code of 
>> this
>> patch series but will come in a following series.  However, it is possible to
>> check the file type, as done in the following example.
> 
> I understand why you've taken a progressive approach here, but I think
> there are two fundamental areas where people will use Landlock: path
> evaluation and network address evaluation. I think it's worth
> expanding this series to include those two confinement examples, since
> that can help people understand what the "general" case will look
> like.

I agree that it would be more useful to add a path/FS evaluation,
however we agreed at LPC that it would be another patch series:
https://lkml.kernel.org/r/5828776a.1010...@digikod.net
It brings more complexity and a new kind of BPF map, which should be
reviewed in a separate series.

> 
> As I mentioned in one of the patch review emails, I think there needs
> to be a clearer explanation of how usage counting works vs the
> "events" (which I think of as "rule tables" not events -- maybe it
> needs a new name?) and "rule" lists. I think I understand it, but I
> spent a lot of time trying to get there. More comments would help.

I though about different names and the previous one was PDP (for Policy
Decision Point) which is used in the literature, but "event" seems
easier to understand and close enough with "hook". Rule tables doesn't
seems to express what is the meaning to register an event/hook.

I'll add more comments to explain how it works.

> 
> Finally, another thing I'm curious about is how to deal with the
> thread-sync issue. Seccomp uses its TSYNC thing, and I'd expect we'd
> want something similar for landlock... but that looks really hairy as
> far as locking goes. Perhaps it's already solved by using the same
> locking seccomp uses, in which case I'm less inclined to kick landlock
> out of seccomp.c. :)

I didn't work on it but it should be really similar to seccomp-bpf.

> 
> Looks like it's coming along nicely! Thanks for continuing to work on this!
> 
> -Kees
> 



signature.asc
Description: OpenPGP digital signature


Re: [PATCH net-next v6 00/11] Landlock LSM: Toward unprivileged sandboxing

2017-04-18 Thread Mickaël Salaün

On 19/04/2017 01:26, Kees Cook wrote:
> On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün  wrote:
>> This sixth series add some changes to the previous one [1], including a 
>> simpler
>> rule inheritance hierarchy (similar to seccomp-bpf), a ptrace scope 
>> protection,
>> some file renaming (better feature identification per file), a future-proof
>> eBPF subtype and miscellaneous cosmetic fixes.
> 
> Sorry for the delay in review! I finally had a chunk of time I could
> devote to this. I really like how it's heading. I still wonder about
> its overlap with seccomp (it's really only using the syscall now...),
> but that's just a detail. Getting the abstraction away from direct LSM
> hooks looks good.
> 
>> There is as yet no way to allow a process to access only a subset of the
>> filesystem where the subset is specified via a path or a file descriptor.  
>> This
>> feature is intentionally left out so as to minimize the amount of code of 
>> this
>> patch series but will come in a following series.  However, it is possible to
>> check the file type, as done in the following example.
> 
> I understand why you've taken a progressive approach here, but I think
> there are two fundamental areas where people will use Landlock: path
> evaluation and network address evaluation. I think it's worth
> expanding this series to include those two confinement examples, since
> that can help people understand what the "general" case will look
> like.

I agree that it would be more useful to add a path/FS evaluation,
however we agreed at LPC that it would be another patch series:
https://lkml.kernel.org/r/5828776a.1010...@digikod.net
It brings more complexity and a new kind of BPF map, which should be
reviewed in a separate series.

> 
> As I mentioned in one of the patch review emails, I think there needs
> to be a clearer explanation of how usage counting works vs the
> "events" (which I think of as "rule tables" not events -- maybe it
> needs a new name?) and "rule" lists. I think I understand it, but I
> spent a lot of time trying to get there. More comments would help.

I though about different names and the previous one was PDP (for Policy
Decision Point) which is used in the literature, but "event" seems
easier to understand and close enough with "hook". Rule tables doesn't
seems to express what is the meaning to register an event/hook.

I'll add more comments to explain how it works.

> 
> Finally, another thing I'm curious about is how to deal with the
> thread-sync issue. Seccomp uses its TSYNC thing, and I'd expect we'd
> want something similar for landlock... but that looks really hairy as
> far as locking goes. Perhaps it's already solved by using the same
> locking seccomp uses, in which case I'm less inclined to kick landlock
> out of seccomp.c. :)

I didn't work on it but it should be really similar to seccomp-bpf.

> 
> Looks like it's coming along nicely! Thanks for continuing to work on this!
> 
> -Kees
> 



signature.asc
Description: OpenPGP digital signature


Re: [PATCH net-next v6 00/11] Landlock LSM: Toward unprivileged sandboxing

2017-04-18 Thread Kees Cook
On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün  wrote:
> This sixth series add some changes to the previous one [1], including a 
> simpler
> rule inheritance hierarchy (similar to seccomp-bpf), a ptrace scope 
> protection,
> some file renaming (better feature identification per file), a future-proof
> eBPF subtype and miscellaneous cosmetic fixes.

Sorry for the delay in review! I finally had a chunk of time I could
devote to this. I really like how it's heading. I still wonder about
its overlap with seccomp (it's really only using the syscall now...),
but that's just a detail. Getting the abstraction away from direct LSM
hooks looks good.

> There is as yet no way to allow a process to access only a subset of the
> filesystem where the subset is specified via a path or a file descriptor.  
> This
> feature is intentionally left out so as to minimize the amount of code of this
> patch series but will come in a following series.  However, it is possible to
> check the file type, as done in the following example.

I understand why you've taken a progressive approach here, but I think
there are two fundamental areas where people will use Landlock: path
evaluation and network address evaluation. I think it's worth
expanding this series to include those two confinement examples, since
that can help people understand what the "general" case will look
like.

As I mentioned in one of the patch review emails, I think there needs
to be a clearer explanation of how usage counting works vs the
"events" (which I think of as "rule tables" not events -- maybe it
needs a new name?) and "rule" lists. I think I understand it, but I
spent a lot of time trying to get there. More comments would help.

Finally, another thing I'm curious about is how to deal with the
thread-sync issue. Seccomp uses its TSYNC thing, and I'd expect we'd
want something similar for landlock... but that looks really hairy as
far as locking goes. Perhaps it's already solved by using the same
locking seccomp uses, in which case I'm less inclined to kick landlock
out of seccomp.c. :)

Looks like it's coming along nicely! Thanks for continuing to work on this!

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH net-next v6 00/11] Landlock LSM: Toward unprivileged sandboxing

2017-04-18 Thread Kees Cook
On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün  wrote:
> This sixth series add some changes to the previous one [1], including a 
> simpler
> rule inheritance hierarchy (similar to seccomp-bpf), a ptrace scope 
> protection,
> some file renaming (better feature identification per file), a future-proof
> eBPF subtype and miscellaneous cosmetic fixes.

Sorry for the delay in review! I finally had a chunk of time I could
devote to this. I really like how it's heading. I still wonder about
its overlap with seccomp (it's really only using the syscall now...),
but that's just a detail. Getting the abstraction away from direct LSM
hooks looks good.

> There is as yet no way to allow a process to access only a subset of the
> filesystem where the subset is specified via a path or a file descriptor.  
> This
> feature is intentionally left out so as to minimize the amount of code of this
> patch series but will come in a following series.  However, it is possible to
> check the file type, as done in the following example.

I understand why you've taken a progressive approach here, but I think
there are two fundamental areas where people will use Landlock: path
evaluation and network address evaluation. I think it's worth
expanding this series to include those two confinement examples, since
that can help people understand what the "general" case will look
like.

As I mentioned in one of the patch review emails, I think there needs
to be a clearer explanation of how usage counting works vs the
"events" (which I think of as "rule tables" not events -- maybe it
needs a new name?) and "rule" lists. I think I understand it, but I
spent a lot of time trying to get there. More comments would help.

Finally, another thing I'm curious about is how to deal with the
thread-sync issue. Seccomp uses its TSYNC thing, and I'd expect we'd
want something similar for landlock... but that looks really hairy as
far as locking goes. Perhaps it's already solved by using the same
locking seccomp uses, in which case I'm less inclined to kick landlock
out of seccomp.c. :)

Looks like it's coming along nicely! Thanks for continuing to work on this!

-Kees

-- 
Kees Cook
Pixel Security


[PATCH net-next v6 00/11] Landlock LSM: Toward unprivileged sandboxing

2017-03-28 Thread Mickaël Salaün
Hi,

This sixth series add some changes to the previous one [1], including a simpler
rule inheritance hierarchy (similar to seccomp-bpf), a ptrace scope protection,
some file renaming (better feature identification per file), a future-proof
eBPF subtype and miscellaneous cosmetic fixes.

This is the first step of the roadmap discussed at LPC [2] (with the
inheritance feature included).  While the intended final goal is to allow
unprivileged users to use Landlock, this series allows only a process with
global CAP_SYS_ADMIN to load and enforce a rule.  This may help to get feedback
and avoid unexpected behaviors.  The documentation patch contains some kernel
documentation and explanations on how to use Landlock.  The compiled
documentation can be found here:
https://landlock-lsm.github.io/linux-doc/landlock-v6/security/landlock/index.html

This series can be applied on top of net-next: cc628c9680c2 ("Merge tag
'mlx5e-failsafe' of
git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux").  This can be
tested with CONFIG_SECCOMP_FILTER and CONFIG_SECURITY_LANDLOCK.  I would really
appreciate constructive comments on the usability, architecture, code, userland
API or use cases.


# Landlock LSM

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

The approach taken is to add the minimum amount of code while still allowing
the user-space application to create quite complex access rules.  A dedicated
security policy language such as the one used by SELinux, AppArmor and other
major LSMs involves a lot of code and is usually permitted to only a trusted
user (i.e. root).  On the contrary, eBPF programs already exist and are
designed to be safely loaded by unprivileged user-space.

This design does not seem too intrusive but is flexible enough to allow a
powerful sandbox mechanism accessible by any process on Linux. The use of
seccomp and Landlock is more suitable with the help of a user-space library
(e.g.  libseccomp) that could help to specify a high-level language to express
a security policy instead of raw eBPF programs. Moreover, thanks to the LLVM
front-end, it is quite easy to write an eBPF program with a subset of the C
language.


# Landlock events and rule enforcement

Unlike syscalls, LSM hooks are security checkpoints and are not architecture
dependent. They are designed to match a security need associated with a
security policy (e.g. access to a file).  The approach taken for Landlock is to
abstract these hooks with Landlock events such as a generic filesystem event
(LANDLOCK_SUBTYPE_EVENT_FS).  Further explanations can be found in the
documentation.

This series uses seccomp(2) only as an entry point to apply a rule to the
calling process and its future children.  It is planed to restore the ability
to use cgroup as an alternative way to enforce a Landlock rule.

There is as yet no way to allow a process to access only a subset of the
filesystem where the subset is specified via a path or a file descriptor.  This
feature is intentionally left out so as to minimize the amount of code of this
patch series but will come in a following series.  However, it is possible to
check the file type, as done in the following example.


# Sandbox example with a read-only filesystem

This example is provided in the samples/bpf directory.  It creates a read-only
environment for all kind of file access except for character devices 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


# Warning on read-only filesystems

Other than owing a mount namespace and remounting every accessible mounts
points as read-only, which may not be possible for an unprivileged security
sandbox, there is no way of preventing a process to change the access time of a
file, including anonymous inodes.  This provides a trivial way to leak
information from a sandboxed environment.  A new LSM hook has been proposed to
allow an LSM to enforce a real read-only filesystem view, but it did not get
strong support so far [5].


# Frequently asked questions

## Why is seccomp-bpf not enough?

A seccomp filter can access only raw syscall arguments (i.e. the register
values) which means that it is not possible to filter according to the value
pointed to by an argument, such as a file pathname. As an embryonic Landlock
version demonstrated, filtering at the syscall level is complicated (e.g. need
to take care of race conditions). This is mainly because the access control
checkpoints of the kernel are not at this high-level but more underneath, at
the LSM-hook level. The LSM hooks are designed to handle this kind of checks.
Landlock abstracts 

[PATCH net-next v6 00/11] Landlock LSM: Toward unprivileged sandboxing

2017-03-28 Thread Mickaël Salaün
Hi,

This sixth series add some changes to the previous one [1], including a simpler
rule inheritance hierarchy (similar to seccomp-bpf), a ptrace scope protection,
some file renaming (better feature identification per file), a future-proof
eBPF subtype and miscellaneous cosmetic fixes.

This is the first step of the roadmap discussed at LPC [2] (with the
inheritance feature included).  While the intended final goal is to allow
unprivileged users to use Landlock, this series allows only a process with
global CAP_SYS_ADMIN to load and enforce a rule.  This may help to get feedback
and avoid unexpected behaviors.  The documentation patch contains some kernel
documentation and explanations on how to use Landlock.  The compiled
documentation can be found here:
https://landlock-lsm.github.io/linux-doc/landlock-v6/security/landlock/index.html

This series can be applied on top of net-next: cc628c9680c2 ("Merge tag
'mlx5e-failsafe' of
git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux").  This can be
tested with CONFIG_SECCOMP_FILTER and CONFIG_SECURITY_LANDLOCK.  I would really
appreciate constructive comments on the usability, architecture, code, userland
API or use cases.


# Landlock LSM

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

The approach taken is to add the minimum amount of code while still allowing
the user-space application to create quite complex access rules.  A dedicated
security policy language such as the one used by SELinux, AppArmor and other
major LSMs involves a lot of code and is usually permitted to only a trusted
user (i.e. root).  On the contrary, eBPF programs already exist and are
designed to be safely loaded by unprivileged user-space.

This design does not seem too intrusive but is flexible enough to allow a
powerful sandbox mechanism accessible by any process on Linux. The use of
seccomp and Landlock is more suitable with the help of a user-space library
(e.g.  libseccomp) that could help to specify a high-level language to express
a security policy instead of raw eBPF programs. Moreover, thanks to the LLVM
front-end, it is quite easy to write an eBPF program with a subset of the C
language.


# Landlock events and rule enforcement

Unlike syscalls, LSM hooks are security checkpoints and are not architecture
dependent. They are designed to match a security need associated with a
security policy (e.g. access to a file).  The approach taken for Landlock is to
abstract these hooks with Landlock events such as a generic filesystem event
(LANDLOCK_SUBTYPE_EVENT_FS).  Further explanations can be found in the
documentation.

This series uses seccomp(2) only as an entry point to apply a rule to the
calling process and its future children.  It is planed to restore the ability
to use cgroup as an alternative way to enforce a Landlock rule.

There is as yet no way to allow a process to access only a subset of the
filesystem where the subset is specified via a path or a file descriptor.  This
feature is intentionally left out so as to minimize the amount of code of this
patch series but will come in a following series.  However, it is possible to
check the file type, as done in the following example.


# Sandbox example with a read-only filesystem

This example is provided in the samples/bpf directory.  It creates a read-only
environment for all kind of file access except for character devices 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


# Warning on read-only filesystems

Other than owing a mount namespace and remounting every accessible mounts
points as read-only, which may not be possible for an unprivileged security
sandbox, there is no way of preventing a process to change the access time of a
file, including anonymous inodes.  This provides a trivial way to leak
information from a sandboxed environment.  A new LSM hook has been proposed to
allow an LSM to enforce a real read-only filesystem view, but it did not get
strong support so far [5].


# Frequently asked questions

## Why is seccomp-bpf not enough?

A seccomp filter can access only raw syscall arguments (i.e. the register
values) which means that it is not possible to filter according to the value
pointed to by an argument, such as a file pathname. As an embryonic Landlock
version demonstrated, filtering at the syscall level is complicated (e.g. need
to take care of race conditions). This is mainly because the access control
checkpoints of the kernel are not at this high-level but more underneath, at
the LSM-hook level. The LSM hooks are designed to handle this kind of checks.
Landlock abstracts