Re: [PATCH v2 0/6] Landlock: ioctl support

["Günther Noack" <gnoack@google.com>]

Hello!

On Fri, Jun 23, 2023 at 04:43:23PM +0200, Günther Noack wrote:
> Proposed approach
> ~~~~~~~~~~~~~~~~~
> 
> Introduce the LANDLOCK_ACCESS_FS_IOCTL right, which restricts the use
> of ioctl(2) on file descriptors.
> 
> We attach the LANDLOCK_ACCESS_FS_IOCTL right to opened file
> descriptors, as we already do for LANDLOCK_ACCESS_FS_TRUNCATE.
> 
> I believe that this approach works for the majority of use cases, and
> offers a good trade-off between Landlock API and implementation
> complexity and flexibility when the feature is used.
> 
> Current limitations
> ~~~~~~~~~~~~~~~~~~~
> 
> With this patch set, ioctl(2) requests can *not* be filtered based on
> file type, device number (dev_t) or on the ioctl(2) request number.
> 
> On the initial RFC patch set [1], we have reached consensus to start
> with this simpler coarse-grained approach, and build additional ioctl
> restriction capabilities on top in subsequent steps.

We *could* use this opportunity to blanket disable the TIOCSTI ioctl, if a
Landlock policy gets enabled and ioctls are handled.

TIOCSTI is a TTY ioctl which is commonly used as a sandbox escape, if the
sandboxes system can get a hold on a TTY file descriptor from outside the
sandbox (like STDOUT, hah).

An excellent summary of these problems was already written by Kees Cook on the
Linux patch which removes TIOCSTI depending on a kernel config option:
https://lore.kernel.org/lkml/20221022182828.give.717-kees@kernel.org/

Unfortunately on the distributions I have tested so far (Debian and Arch Linux),
TIOCSTI is still enabled.

***Proposal***:

  I am in favor of *disabling* TIOCSTI in all Landlocked processes,
  if the Landlock policy handles the LANDLOCK_ACCESS_FS_IOCTL right.

If we want to do it in a backwards-compatible way, now would be the time to add
it to the patch set. :)

As far as I can tell, there are no good legitimate use cases for TIOCSTI, and it
would fix the aforementioned sandbox escaping trick for a Landlocked process.
With the patch set as it is now, the only way to prevent that sandbox escape is
unfortunately to either (1) close the TTY file descriptors when enabling
Landlock, or (2) rely on an outside process to pass something else than a TTY
for FDs 0, 1 and 2.

Does that sound reasonable?  If yes, I'd send an update to this patch set which
forbids TIOCSTI.

Thanks,
—Günther

-- 
Sent using Mutt 🐕 Woof Woof