Re: [PATCH] tty: Require CAP_SYS_ADMIN for all usages of TIOCL_SELMOUSEREPORT

["Günther Noack" <gnoack3000@gmail.com>]

On Sun, Feb 23, 2025 at 09:54:50PM +0100, Günther Noack wrote:
> This requirement was overeagerly loosened in commit 2f83e38a095f
> ("tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN"), but as
> it turns out,
> 
>   (1) the logic I implemented there was inconsistent (apologies!),
> 
>   (2) TIOCL_SELMOUSEREPORT might actually be a small security risk
>       after all, and
> 
>   (3) TIOCL_SELMOUSEREPORT is only meant to be used by the mouse
>       daemon (GPM or Consolation), which runs as CAP_SYS_ADMIN
>       already.


Greg and Jared: Friendly ping on this patch.

Could you please have a look if you can find the time?



To maybe explain in an overview again why this is safe:

The TIOCLINUX ioctl has various subcommands (uapi/linux/tiocl.h),
and one of these has in turn more subcommands.  The structure is:

* TIOCLINUX, with "subcodes":
  * TIOCL_SETSEL, with "selection modes":
    * TIOCL_SELCHAR
    * TIOCL_SELWORD
    * TIOCL_SELLINE
    * TIOCL_SELPOINTER
    * TIOCL_SELCLEAR
    * TIOCL_SELMOUSEREPORT
  * TIOCL_PASTESEL
  * TIOCL_SELLOADLUT
  * ...

While securing TIOCLINUX, we restricted access to various subcommands
with CAP_SYS_ADMIN, but permitted different subcommands.

This table gives an overview of which TIOCL_SETSEL subcommands
required CAP_SYS_ADMIN at which point in time:

                          point in time
  TIOCL_SETSEL sel_mode | 0 | 1 | 2 | 3
  ----------------------|---|---|---|---
  TIOCL_SELCHAR         |   | x | x | x
  TIOCL_SELWORD         |   | x | x | x 
  TIOCL_SELLINE         |   | x | x | x 
  TIOCL_SELPOINTER      |   | x |   |
  TIOCL_SELCLEAR        |   | x |   | 
  TIOCL_SELMOUSEREPORT  |   | x | ? | x  <-- This is the change

  "x" means "requires CAP_SYS_ADMIN"
  "?" means "inconsistently requires CAP_SYS_ADMIN"

The points in time are:

 (0) before we required CAP_SYS_ADMIN on TIOCLINUX subcommands
 (1) after commit 8d1b43f6a6df ("tty: Restrict access to TIOCLINUX' copy-and-paste subcommands")
 (2) after commit 2f83e38a095f ("tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN
")
 (3) after this patch ("tty: Require CAP_SYS_ADMIN for all usages of TIOCL_SELMOUSEREPORT")

This patch **reverts the behaviour for TIOCL_SELMOUSEREPORT back to
what it was in phase (1)** after commit 8d1b43f6a6df ("tty: Restrict
access to TIOCLINUX' copy-and-paste subcommands").  We have double
checked this in Emacs and GPM's source code earlier in this mail
thread [1] and have confidence that this is better, because:

 (a) TIOCL_SELMOUSEREPORT can maybe be abused after all,
 (b) it is not required for Emacs as we thought in patch (2)
 (c) the behavior I implemented in patch (2) was accidentally
     inconsistent

Again, apologies for the pointless back-and-forth on this fix, but it
will be better after this iteration.  I hope that this summary helps
in the review.  Please let me know if you have further questions.

Thanks,
–Günther

[1] https://lore.kernel.org/all/491f3df9de6593df8e70dbe77614b026@finder.org/