Re: [PATCH v15 0/9] open: introduce openat2(2) syscall

[Aleksa Sarai]

On 2019-11-12, Kees Cook  wrote:
> On Tue, Nov 12, 2019 at 12:24:04AM +1100, Aleksa Sarai wrote:
> > On 2019-11-05, Aleksa Sarai  wrote:
> > > This patchset is being developed here:
> > >   
> > > 
> > > Patch changelog:
> > >  v15:
> > >   * Fix code style for LOOKUP_IN_ROOT handling in path_init(). [Linus 
> > > Torvalds]
> > >   * Split out patches for each individual LOOKUP flag.
> > >   * Reword commit messages to give more background information about the
> > > series, as well as mention the semantics of each flag in more detail.
> > > [...]
> > 
> > Ping -- this patch hasn't been touched for a week. Thanks.
> 
> If I've been following correctly, everyone is happy with this series.
> (i.e. Linus's comment appear to have been addressed.)
> 
> Perhaps the next question is should this go via a pull request by you to
> Linus directly during the v5.5 merge window, via akpm, via akpm, via
> Christian, or some other path? Besides Linus, it's not been clear who
> should "claim" this series. :)

Given the namei changes, I wanted to avoid stepping on Al's toes. Though
he did review the series a few versions ago, the discussion didn't focus
on the openat2(2) semantics (which have also changed since then). I'm
not sure whether to interpret the silence to mean he's satisfied with
things as they are, or if he hasn't had more time to look at the series.

As for which tree it should be routed to, I don't mind -- Christian is
the most straight-forward choice (but if Al wants to route it, that's
fine with me too).

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH



signature.asc
Description: PGP signature

Re: [PATCH v15 0/9] open: introduce openat2(2) syscall

[Christian Brauner]

On Tue, Nov 12, 2019 at 03:01:26PM -0800, Kees Cook wrote:
> On Tue, Nov 12, 2019 at 12:24:04AM +1100, Aleksa Sarai wrote:
> > On 2019-11-05, Aleksa Sarai  wrote:
> > > This patchset is being developed here:
> > >   
> > > 
> > > Patch changelog:
> > >  v15:
> > >   * Fix code style for LOOKUP_IN_ROOT handling in path_init(). [Linus 
> > > Torvalds]
> > >   * Split out patches for each individual LOOKUP flag.
> > >   * Reword commit messages to give more background information about the
> > > series, as well as mention the semantics of each flag in more detail.
> > > [...]
> > 
> > Ping -- this patch hasn't been touched for a week. Thanks.
> 
> If I've been following correctly, everyone is happy with this series.
> (i.e. Linus's comment appear to have been addressed.)
> 
> Perhaps the next question is should this go via a pull request by you to
> Linus directly during the v5.5 merge window, via akpm, via akpm, via
> Christian, or some other path? Besides Linus, it's not been clear who
> should "claim" this series. :)

I like this series and the same with the copy_struct_from_user() part of
it I've taken I'm happy to stuff this into a dedicated branch, merge it
into my for-next and send it for v5.5.
Though I'd _much_ rather see Al pick this up or have him give his
blessing first.

Christian

Re: [PATCH v15 0/9] open: introduce openat2(2) syscall

[Kees Cook]

On Tue, Nov 12, 2019 at 12:24:04AM +1100, Aleksa Sarai wrote:
> On 2019-11-05, Aleksa Sarai  wrote:
> > This patchset is being developed here:
> >   
> > 
> > Patch changelog:
> >  v15:
> >   * Fix code style for LOOKUP_IN_ROOT handling in path_init(). [Linus 
> > Torvalds]
> >   * Split out patches for each individual LOOKUP flag.
> >   * Reword commit messages to give more background information about the
> > series, as well as mention the semantics of each flag in more detail.
> > [...]
> 
> Ping -- this patch hasn't been touched for a week. Thanks.

If I've been following correctly, everyone is happy with this series.
(i.e. Linus's comment appear to have been addressed.)

Perhaps the next question is should this go via a pull request by you to
Linus directly during the v5.5 merge window, via akpm, via akpm, via
Christian, or some other path? Besides Linus, it's not been clear who
should "claim" this series. :)

-- 
Kees Cook

Re: [PATCH v15 0/9] open: introduce openat2(2) syscall

[Aleksa Sarai]

On 2019-11-05, Aleksa Sarai  wrote:
> This patchset is being developed here:
>   
> 
> Patch changelog:
>  v15:
>   * Fix code style for LOOKUP_IN_ROOT handling in path_init(). [Linus 
> Torvalds]
>   * Split out patches for each individual LOOKUP flag.
>   * Reword commit messages to give more background information about the
> series, as well as mention the semantics of each flag in more detail.
>  v14: 
>   
>  v13: 
>  v12: 
>  v11: 
>   
>  v10: 
>  v09: 
>  v08: 
>  v07: 
>  v06: 
>  v05: 
>  v04: 
>  v03: 
>  v02: 
>  v01: 
> 
> For a very long time, extending openat(2) with new features has been
> incredibly frustrating. This stems from the fact that openat(2) is
> possibly the most famous counter-example to the mantra "don't silently
> accept garbage from userspace" -- it doesn't check whether unknown flags
> are present[1].
> 
> This means that (generally) the addition of new flags to openat(2) has
> been fraught with backwards-compatibility issues (O_TMPFILE has to be
> defined as __O_TMPFILE|O_DIRECTORY|[O_RDWR or O_WRONLY] to ensure old
> kernels gave errors, since it's insecure to silently ignore the
> flag[2]). All new security-related flags therefore have a tough road to
> being added to openat(2).
> 
> Furthermore, the need for some sort of control over VFS's path resolution (to
> avoid malicious paths resulting in inadvertent breakouts) has been a very
> long-standing desire of many userspace applications. This patchset is a 
> revival
> of Al Viro's old AT_NO_JUMPS[3] patchset (which was a variant of David
> Drysdale's O_BENEATH patchset[4] which was a spin-off of the Capsicum
> project[5]) with a few additions and changes made based on the previous
> discussion within [6] as well as others I felt were useful.
> 
> In line with the conclusions of the original discussion of AT_NO_JUMPS, the
> flag has been split up into separate flags. However, instead of being an
> openat(2) flag it is provided through a new syscall openat2(2) which provides
> several other improvements to the openat(2) interface (see the patch
> description for more details). The following new LOOKUP_* flags are added:
> 
>   * LOOKUP_NO_XDEV blocks all mountpoint crossings (upwards, downwards,
> or through absolute links). Absolute pathnames alone in openat(2) do not
> trigger this. Magic-link traversal which implies a vfsmount jump is also
> blocked (though magic-link jumps on the same vfsmount are permitted).
> 
>   * LOOKUP_NO_MAGICLINKS blocks resolution through /proc/$pid/fd-style
> links. This is done by blocking the usage of nd_jump_link() during
> resolution in a filesystem. The term "magic-links" is used to match
> with the only reference to these links in Documentation/, but I'm
> happy to change the name.
> 
> It should be noted that this is different to the scope of
> ~LOOKUP_FOLLOW in that it applies to all path components. However,
> you can do openat2(NO_FOLLOW|NO_MAGICLINKS) on a magic-link and it
> will *not* fail (assuming that no parent component was a
> magic-link), and you will have an fd for the magic-link.
> 
> In order to correctly detect magic-links, the introduction of a new
> LOOKUP_MAGICLINK_JUMPED state flag was required.
> 
>   * LOOKUP_BENEATH disallows escapes to outside the starting dirfd's
> tree, using techniques such as ".." or absolute links. Absolute
> paths in openat(2) are also disallowed. Conceptually this flag is to
> ensure you "stay below" a certain point in the filesystem tree --
> but this requires some additional to protect against various races
> that would allow escape using "..".
> 
> Currently LOOKUP_BENEATH implies LOOKUP_NO_MAGICLINKS, because it
> can trivially beam you around the filesystem 

[PATCH v15 0/9] open: introduce openat2(2) syscall

[Aleksa Sarai]

This patchset is being developed here:
  

Patch changelog:
 v15:
  * Fix code style for LOOKUP_IN_ROOT handling in path_init(). [Linus Torvalds]
  * Split out patches for each individual LOOKUP flag.
  * Reword commit messages to give more background information about the
series, as well as mention the semantics of each flag in more detail.
 v14: 
  
 v13: 
 v12: 
 v11: 
  
 v10: 
 v09: 
 v08: 
 v07: 
 v06: 
 v05: 
 v04: 
 v03: 
 v02: 
 v01: 

For a very long time, extending openat(2) with new features has been
incredibly frustrating. This stems from the fact that openat(2) is
possibly the most famous counter-example to the mantra "don't silently
accept garbage from userspace" -- it doesn't check whether unknown flags
are present[1].

This means that (generally) the addition of new flags to openat(2) has
been fraught with backwards-compatibility issues (O_TMPFILE has to be
defined as __O_TMPFILE|O_DIRECTORY|[O_RDWR or O_WRONLY] to ensure old
kernels gave errors, since it's insecure to silently ignore the
flag[2]). All new security-related flags therefore have a tough road to
being added to openat(2).

Furthermore, the need for some sort of control over VFS's path resolution (to
avoid malicious paths resulting in inadvertent breakouts) has been a very
long-standing desire of many userspace applications. This patchset is a revival
of Al Viro's old AT_NO_JUMPS[3] patchset (which was a variant of David
Drysdale's O_BENEATH patchset[4] which was a spin-off of the Capsicum
project[5]) with a few additions and changes made based on the previous
discussion within [6] as well as others I felt were useful.

In line with the conclusions of the original discussion of AT_NO_JUMPS, the
flag has been split up into separate flags. However, instead of being an
openat(2) flag it is provided through a new syscall openat2(2) which provides
several other improvements to the openat(2) interface (see the patch
description for more details). The following new LOOKUP_* flags are added:

  * LOOKUP_NO_XDEV blocks all mountpoint crossings (upwards, downwards,
or through absolute links). Absolute pathnames alone in openat(2) do not
trigger this. Magic-link traversal which implies a vfsmount jump is also
blocked (though magic-link jumps on the same vfsmount are permitted).

  * LOOKUP_NO_MAGICLINKS blocks resolution through /proc/$pid/fd-style
links. This is done by blocking the usage of nd_jump_link() during
resolution in a filesystem. The term "magic-links" is used to match
with the only reference to these links in Documentation/, but I'm
happy to change the name.

It should be noted that this is different to the scope of
~LOOKUP_FOLLOW in that it applies to all path components. However,
you can do openat2(NO_FOLLOW|NO_MAGICLINKS) on a magic-link and it
will *not* fail (assuming that no parent component was a
magic-link), and you will have an fd for the magic-link.

In order to correctly detect magic-links, the introduction of a new
LOOKUP_MAGICLINK_JUMPED state flag was required.

  * LOOKUP_BENEATH disallows escapes to outside the starting dirfd's
tree, using techniques such as ".." or absolute links. Absolute
paths in openat(2) are also disallowed. Conceptually this flag is to
ensure you "stay below" a certain point in the filesystem tree --
but this requires some additional to protect against various races
that would allow escape using "..".

Currently LOOKUP_BENEATH implies LOOKUP_NO_MAGICLINKS, because it
can trivially beam you around the filesystem (breaking the
protection). In future, there might be similar safety checks done as
in LOOKUP_IN_ROOT, but that requires more discussion.

In addition, two new flags are added that expand on the above