Re: [PATCH v21 07/12] landlock: Support filesystem access-control

2020-10-14 Thread James Morris
On Wed, 14 Oct 2020, Mickaël Salaün wrote:

> 
> On 14/10/2020 20:52, Mickaël Salaün wrote:
> > 
> > On 14/10/2020 20:07, James Morris wrote:
> >> On Thu, 8 Oct 2020, Mickaël Salaün wrote:
> >>
> >>> +config ARCH_EPHEMERAL_STATES
> >>> + def_bool n
> >>> + help
> >>> +   An arch should select this symbol if it does not keep an internal 
> >>> kernel
> >>> +   state for kernel objects such as inodes, but instead relies on 
> >>> something
> >>> +   else (e.g. the host kernel for an UML kernel).
> >>> +
> >>
> >> This is used to disable Landlock for UML, correct?
> > 
> > Yes
> > 
> >> I wonder if it could be 
> >> more specific: "ephemeral states" is a very broad term.
> >>
> >> How about something like ARCH_OWN_INODES ?
> > 
> > Sounds good. We may need add new ones (e.g. for network socket, UID,
> > etc.) in the future though.
> > 
> 
> Because UML is the exception here, it would be more convenient to keep
> the inverted semantic. What about ARCH_NO_OWN_INODES or
> ARCH_EPHEMERAL_INODES?

The latter seems good.

-- 
James Morris



Re: [PATCH v21 07/12] landlock: Support filesystem access-control

2020-10-14 Thread Mickaël Salaün


On 14/10/2020 20:52, Mickaël Salaün wrote:
> 
> On 14/10/2020 20:07, James Morris wrote:
>> On Thu, 8 Oct 2020, Mickaël Salaün wrote:
>>
>>> +config ARCH_EPHEMERAL_STATES
>>> +   def_bool n
>>> +   help
>>> + An arch should select this symbol if it does not keep an internal 
>>> kernel
>>> + state for kernel objects such as inodes, but instead relies on 
>>> something
>>> + else (e.g. the host kernel for an UML kernel).
>>> +
>>
>> This is used to disable Landlock for UML, correct?
> 
> Yes
> 
>> I wonder if it could be 
>> more specific: "ephemeral states" is a very broad term.
>>
>> How about something like ARCH_OWN_INODES ?
> 
> Sounds good. We may need add new ones (e.g. for network socket, UID,
> etc.) in the future though.
> 

Because UML is the exception here, it would be more convenient to keep
the inverted semantic. What about ARCH_NO_OWN_INODES or
ARCH_EPHEMERAL_INODES?


Re: [PATCH v21 07/12] landlock: Support filesystem access-control

2020-10-14 Thread Mickaël Salaün


On 14/10/2020 20:07, James Morris wrote:
> On Thu, 8 Oct 2020, Mickaël Salaün wrote:
> 
>> +config ARCH_EPHEMERAL_STATES
>> +def_bool n
>> +help
>> +  An arch should select this symbol if it does not keep an internal 
>> kernel
>> +  state for kernel objects such as inodes, but instead relies on 
>> something
>> +  else (e.g. the host kernel for an UML kernel).
>> +
> 
> This is used to disable Landlock for UML, correct?

Yes

> I wonder if it could be 
> more specific: "ephemeral states" is a very broad term.
> 
> How about something like ARCH_OWN_INODES ?

Sounds good. We may need add new ones (e.g. for network socket, UID,
etc.) in the future though.


Re: [PATCH v21 07/12] landlock: Support filesystem access-control

2020-10-14 Thread James Morris
On Thu, 8 Oct 2020, Mickaël Salaün wrote:

> +config ARCH_EPHEMERAL_STATES
> + def_bool n
> + help
> +   An arch should select this symbol if it does not keep an internal 
> kernel
> +   state for kernel objects such as inodes, but instead relies on 
> something
> +   else (e.g. the host kernel for an UML kernel).
> +

This is used to disable Landlock for UML, correct? I wonder if it could be 
more specific: "ephemeral states" is a very broad term.

How about something like ARCH_OWN_INODES ?


-- 
James Morris



[PATCH v21 07/12] landlock: Support filesystem access-control

2020-10-08 Thread Mickaël Salaün
From: Mickaël Salaün 

Thanks to the Landlock objects and ruleset, it is possible to identify
inodes according to a process's domain.  To enable an unprivileged
process to express a file hierarchy, it first needs to open a directory
(or a file) and pass this file descriptor to the kernel through
landlock_add_rule(2).  When checking if a file access request is
allowed, we walk from the requested dentry to the real root, following
the different mount layers.  The access to each "tagged" inodes are
collected according to their rule layer level, and ANDed to create
access to the requested file hierarchy.  This makes possible to identify
a lot of files without tagging every inodes nor modifying the
filesystem, while still following the view and understanding the user
has from the filesystem.

Add a new ARCH_EPHEMERAL_STATES for UML because it currently does not
keep the same struct inodes for the same inodes whereas these inodes are
in use.

This commit adds a minimal set of supported filesystem access-control
which doesn't enable to restrict all file-related actions.  This is the
result of multiple discussions to minimize the code of Landlock to ease
review.  Thanks to the Landlock design, extending this access-control
without breaking user space will not be a problem.  Moreover, seccomp
filters can be used to restrict the use of syscall families which may
not be currently handled by Landlock.

Cc: Al Viro 
Cc: Anton Ivanov 
Cc: James Morris 
Cc: Jann Horn 
Cc: Jeff Dike 
Cc: Kees Cook 
Cc: Richard Weinberger 
Cc: Serge E. Hallyn 
Signed-off-by: Mickaël Salaün 
---

Changes since v19:
* Fix spelling (spotted by Randy Dunlap).

Changes since v18:
* Remove useless include.
* Fix spelling.

Changes since v17:
* Replace landlock_release_inodes() with security_sb_delete() (requested
  by James Morris).
* Replace struct super_block->s_landlock_inode_refs with the LSM
  infrastructure management of the superblock (requested by James
  Morris).
* Fix mknod restriction with a zero mode (spotted by Vincent Dagonneau).
* Minimize executed code in path_mknod and file_open hooks when the
  current tasks is not sandboxed.
* Remove useless checks on the file pointer and inode in
  hook_file_open() .
* Constify domain pointers.
* Rename inode_landlock() to landlock_inode().
* Import include/uapi/linux/landlock.h and _LANDLOCK_ACCESS_FS_* from
  the ruleset and domain management patch.
* Explain the rational of this minimal set of access-control.
  https://lore.kernel.org/lkml/f646e1c7-33cf-333f-070c-0a40ad046...@digikod.net/

Changes since v16:
* Add ARCH_EPHEMERAL_STATES and enable it for UML.

Changes since v15:
* Replace layer_levels and layer_depth with a bitfield of layers: this
  enables to properly manage superset and subset of access rights,
  whatever their order in the stack of layers.
  Cf. 
https://lore.kernel.org/lkml/e07fe473-1801-01cc-12ae-b3167f952...@digikod.net/
* Allow to open pipes and similar special files through /proc/self/fd/.
* Properly handle internal filesystems such as nsfs: always allow these
  kind of roots because disconnected path cannot be evaluated.
* Remove the LANDLOCK_ACCESS_FS_LINK_TO and
  LANDLOCK_ACCESS_FS_RENAME_{TO,FROM}, but use the
  LANDLOCK_ACCESS_FS_REMOVE_{FILE,DIR} and LANDLOCK_ACCESS_FS_MAKE_*
  instead.  Indeed, it is not possible for now (and not really useful)
  to express the semantic of a source and a destination.
* Check access rights to remove a directory or a file with rename(2).
* Forbid reparenting when linking or renaming.  This is needed to easily
  protect against possible privilege escalation by changing the place of
  a file or directory in relation to an enforced access policy (from the
  set of layers).  This will be relaxed in the future.
* Update hooks to take into account replacement of the object's self and
  beneath access bitfields with one.  Simplify the code.
* Check file related access rights.
* Check d_is_negative() instead of !d_backing_inode() in
  check_access_path_continue(), and continue the path walk while there
  is no mapped inode e.g., with rename(2).
* Check private inode in check_access_path().
* Optimize get_file_access() when dealing with a directory.
* Add missing atomic.h .

Changes since v14:
* Simplify the object, rule and ruleset management at the expense of a
  less aggressive memory freeing (contributed by Jann Horn, with
  additional modifications):
  - Rewrite release_inode() to use inode->sb->s_landlock_inode_refs.
  - Remove useless checks in landlock_release_inodes(), clean object
pointer according to the new struct landlock_object and wait for all
iput() to complete.
  - Rewrite get_inode_object() according to the new struct
landlock_object.  If there is a race-condition when cleaning up an
object, we retry until the concurrent thread finished the object
cleaning.
  Cf. 
https://lore.kernel.org/lkml/cag48ez21ben0wl1bbmtiiu8j9jp5iewthowz4turuj+ki0y...@mail.gmail.com/
* Fix nested domains by implementing