Re: [PATCH v3 2/2] tty: Allow TIOCSTI to be disabled

[Samuel Thibault]

Hello,

Kees Cook, le sam. 22 oct. 2022 11:29:49 -0700, a ecrit:
> TIOCSTI continues its long history of being used in privilege escalation
> attacks[1]. Prior attempts to provide a mechanism to disable this have
> devolved into discussions around creating full-blown LSMs to provide
> arbitrary ioctl filtering, which is hugely over-engineered -- only
> TIOCSTI is being used this way. 3 years ago OpenBSD entirely removed
> TIOCSTI[2], Android has had it filtered for longer[3], and the tools that
> had historically used TIOCSTI either do not need it, are not commonly
> built with it, or have had its use removed.

No. The Brltty screen reader entirely relies on TIOCSTI to be able to
support input from various Braille devices. Please make sure to keep
TIOCSTI enabled by default, otherwise some people would just completely
lose their usual way of simply typing on Linux.

Samuel

> Provide a simple CONFIG and global sysctl to disable this for the system
> builders who have wanted this functionality for literally decades now,
> much like the ldisc_autoload CONFIG and sysctl.
> 
> [1] https://lore.kernel.org/linux-hardening/Y0m9l52AKmw6Yxi1@hostpad
> [2] https://undeadly.org/cgi?action=article;sid=20170701132619
> [3] https://lore.kernel.org/lkml/CAFJ0LnFGRuEEn1tCLhoki8ZyWrKfktbF+rwwN7WzyC_kBFoQVA@xxxxxxxxxxxxxx/
> 
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Jiri Slaby <jirislaby@xxxxxxxxxx>
> Cc: Simon Brand <simon.brand@xxxxxxxxxxxxxxxx>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
>  drivers/tty/Kconfig  | 19 +++++++++++++++++++
>  drivers/tty/tty_io.c | 11 +++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
> index cc30ff93e2e4..d35fc068da74 100644
> --- a/drivers/tty/Kconfig
> +++ b/drivers/tty/Kconfig
> @@ -149,6 +149,25 @@ config LEGACY_PTY_COUNT
>  	  When not in use, each legacy PTY occupies 12 bytes on 32-bit
>  	  architectures and 24 bytes on 64-bit architectures.
>  
> +config LEGACY_TIOCSTI
> +	bool "Allow legacy TIOCSTI usage"
> +	default y
> +	help
> +	  Historically the kernel has allowed TIOCSTI, which will push
> +	  characters into a controlling TTY. This continues to be used
> +	  as a malicious privilege escalation mechanism, and provides no
> +	  meaningful real-world utility any more.

Yes it does.

> +       Its use is considered
> +	  a dangerous legacy operation, and can be disabled on most
> +	  systems.
> +
> +	  Say 'Y here only if you have confirmed that your system's
> +	  userspace depends on this functionality to continue operating
> +	  normally.
> +
> +	  This functionality can be changed at runtime with the
> +	  dev.tty.legacy_tiocsti sysctl. This configuration option sets
> +	  the default value of the sysctl.
> +
>  config LDISC_AUTOLOAD
>  	bool "Automatically load TTY Line Disciplines"
>  	default y
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index fe77a3d41326..a6a16cf986b7 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -2268,11 +2268,15 @@ static int tty_fasync(int fd, struct file *filp, int on)
>   *  * Called functions take tty_ldiscs_lock
>   *  * current->signal->tty check is safe without locks
>   */
> +static bool tty_legacy_tiocsti __read_mostly = IS_ENABLED(CONFIG_LEGACY_TIOCSTI);
>  static int tiocsti(struct tty_struct *tty, char __user *p)
>  {
>  	char ch, mbz = 0;
>  	struct tty_ldisc *ld;
>  
> +	if (!tty_legacy_tiocsti)
> +		return -EIO;
> +
>  	if ((current->signal->tty != tty) && !capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  	if (get_user(ch, p))
> @@ -3573,6 +3577,13 @@ void console_sysfs_notify(void)
>  }
>  
>  static struct ctl_table tty_table[] = {
> +	{
> +		.procname	= "legacy_tiocsti",
> +		.data		= &tty_legacy_tiocsti,
> +		.maxlen		= sizeof(tty_legacy_tiocsti),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dobool,
> +	},
>  	{
>  		.procname	= "ldisc_autoload",
>  		.data		= &tty_ldisc_autoload,
> -- 
> 2.34.1
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.