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

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

[Kees Cook]

Hi,

v3: make sysctl a bool (Jiri)
v2: https://lore.kernel.org/lkml/20221015064222.gonna.435-kees@kernel.org
v1: https://lore.kernel.org/lkml/20221015041352.never.966-kees@kernel.org/

This is so very long over-due. We just need to do this and put an end
to this legacy operation.

Repeating the commit log from patch 2:

    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.

    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@mail.gmail.com/

Thanks,

-Kees


Kees Cook (2):
  tty: Move sysctl setup into "core" tty logic
  tty: Allow TIOCSTI to be disabled

 drivers/tty/Kconfig     | 19 +++++++++++++++++
 drivers/tty/tty.h       |  2 +-
 drivers/tty/tty_io.c    | 45 +++++++++++++++++++++++++++++++++++++++--
 drivers/tty/tty_ldisc.c | 38 +---------------------------------
 4 files changed, 64 insertions(+), 40 deletions(-)

-- 
2.34.1

[PATCH v3 1/2] tty: Move sysctl setup into "core" tty logic

[Kees Cook]

In preparation for adding another sysctl to the tty subsystem, move the
tty setup code into the "core" tty code, which contains tty_init() itself.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/tty/tty.h       |  2 +-
 drivers/tty/tty_io.c    | 34 ++++++++++++++++++++++++++++++++--
 drivers/tty/tty_ldisc.c | 38 +-------------------------------------
 3 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index 1c08c9b67b16..f45cd683c02e 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -93,7 +93,7 @@ void tty_ldisc_release(struct tty_struct *tty);
 int __must_check tty_ldisc_init(struct tty_struct *tty);
 void tty_ldisc_deinit(struct tty_struct *tty);
 
-void tty_sysctl_init(void);
+extern int tty_ldisc_autoload;
 
 /* tty_audit.c */
 #ifdef CONFIG_AUDIT
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index de06c3c2ff70..fe77a3d41326 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3572,13 +3572,44 @@ void console_sysfs_notify(void)
 		sysfs_notify(&consdev->kobj, NULL, "active");
 }
 
+static struct ctl_table tty_table[] = {
+	{
+		.procname	= "ldisc_autoload",
+		.data		= &tty_ldisc_autoload,
+		.maxlen		= sizeof(tty_ldisc_autoload),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_ONE,
+	},
+	{ }
+};
+
+static struct ctl_table tty_dir_table[] = {
+	{
+		.procname	= "tty",
+		.mode		= 0555,
+		.child		= tty_table,
+	},
+	{ }
+};
+
+static struct ctl_table tty_root_table[] = {
+	{
+		.procname	= "dev",
+		.mode		= 0555,
+		.child		= tty_dir_table,
+	},
+	{ }
+};
+
 /*
  * Ok, now we can initialize the rest of the tty devices and can count
  * on memory allocations, interrupts etc..
  */
 int __init tty_init(void)
 {
-	tty_sysctl_init();
+	register_sysctl_table(tty_root_table);
 	cdev_init(&tty_cdev, &tty_fops);
 	if (cdev_add(&tty_cdev, MKDEV(TTYAUX_MAJOR, 0), 1) ||
 	    register_chrdev_region(MKDEV(TTYAUX_MAJOR, 0), 1, "/dev/tty") < 0)
@@ -3600,4 +3631,3 @@ int __init tty_init(void)
 #endif
 	return 0;
 }
-
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 776d8a62f77c..e758f44729e7 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -117,7 +117,7 @@ static void put_ldops(struct tty_ldisc_ops *ldops)
 	raw_spin_unlock_irqrestore(&tty_ldiscs_lock, flags);
 }
 
-static int tty_ldisc_autoload = IS_BUILTIN(CONFIG_LDISC_AUTOLOAD);
+int tty_ldisc_autoload = IS_BUILTIN(CONFIG_LDISC_AUTOLOAD);
 
 /**
  * tty_ldisc_get	-	take a reference to an ldisc
@@ -817,39 +817,3 @@ void tty_ldisc_deinit(struct tty_struct *tty)
 		tty_ldisc_put(tty->ldisc);
 	tty->ldisc = NULL;
 }
-
-static struct ctl_table tty_table[] = {
-	{
-		.procname	= "ldisc_autoload",
-		.data		= &tty_ldisc_autoload,
-		.maxlen		= sizeof(tty_ldisc_autoload),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
-		.extra1		= SYSCTL_ZERO,
-		.extra2		= SYSCTL_ONE,
-	},
-	{ }
-};
-
-static struct ctl_table tty_dir_table[] = {
-	{
-		.procname	= "tty",
-		.mode		= 0555,
-		.child		= tty_table,
-	},
-	{ }
-};
-
-static struct ctl_table tty_root_table[] = {
-	{
-		.procname	= "dev",
-		.mode		= 0555,
-		.child		= tty_dir_table,
-	},
-	{ }
-};
-
-void tty_sysctl_init(void)
-{
-	register_sysctl_table(tty_root_table);
-}
-- 
2.34.1

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

[Kees Cook]

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.

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@mail.gmail.com/

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Simon Brand <simon.brand@postadigitale.de>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 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. 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

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

[Geert Uytterhoeven]

Hi Kees,

On Sat, Oct 22, 2022 at 9:14 PM Kees Cook <keescook@chromium.org> wrote:
> 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.
>
> 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@mail.gmail.com/
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Simon Brand <simon.brand@postadigitale.de>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Thanks for your patch, which is now commit 83efeeeb3d04b22a ("tty:
Allow TIOCSTI to be disabled") in tty/tty-next.

> --- 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

Obviously this should either default to n, ...

> +       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. 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.

... or the help text should be made less scary.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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@mail.gmail.com/
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Simon Brand <simon.brand@postadigitale.de>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  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.

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

[Samuel Thibault]

Samuel Thibault, le mer. 28 déc. 2022 00:40:00 +0100, a ecrit:
> 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.

(it only needs support for it on the linux console itself, nowhere else)

> 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@mail.gmail.com/
> > 
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Jiri Slaby <jirislaby@kernel.org>
> > Cc: Simon Brand <simon.brand@postadigitale.de>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  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.

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

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

[Kees Cook]

On December 27, 2022 3:40:00 PM PST, Samuel Thibault <samuel.thibault@aquilenet.fr> wrote:
>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.

Yup, it remains default enabled:

> [...]
>> +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.

Can you send a patch to adjust this language?

Also, what does FreeBSD use for screen readers?

-Kees


-- 
Kees Cook

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

[Samuel Thibault]

Hello,

Kees Cook, le mar. 27 déc. 2022 19:32:55 -0800, a ecrit:
> On December 27, 2022 3:40:00 PM PST, Samuel Thibault <samuel.thibault@aquilenet.fr> wrote:
> >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.
> 
> Yup, it remains default enabled:

Yes, but thining of it, very soon people in various security-sensitive
distributions will disable it, as they should indeed. And people who
need to use their Braille device on such distributions will get stuck.

Can we perhaps just introduce a CAP_TIOCSTI that the brltty daemon would
be able to use? We could even make it only allow TIOCSTI on the linux
console (tty->ops == con_ops).

> Also, what does FreeBSD use for screen readers?

FreeBSD provides poor support for that, people have to use a patched
screen tool to somehow access the console (but only after login).

Samuel

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

[Samuel Thibault]

Hello,

Samuel Thibault, le mer. 28 déc. 2022 21:57:26 +0100, a ecrit:
> Kees Cook, le mar. 27 déc. 2022 19:32:55 -0800, a ecrit:
> > On December 27, 2022 3:40:00 PM PST, Samuel Thibault <samuel.thibault@aquilenet.fr> wrote:
> > >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.
> > 
> > Yup, it remains default enabled:
> 
> Yes, but thining of it, very soon people in various security-sensitive
> distributions will disable it, as they should indeed. And people who
> need to use their Braille device on such distributions will get stuck.

And as expected, it did get disabled in Debian for instance, very much
to the dismay of blind users, whose keyboard suddenly stopped working at
all after rebooting with a Linux 6.3 kernel!...

> Can we perhaps just introduce a CAP_TIOCSTI that the brltty daemon would
> be able to use? We could even make it only allow TIOCSTI on the linux
> console (tty->ops == con_ops).

*Please* comment on this so we can progress. ATM people are
advising each other to set dev.tty.legacy_tiocsti=1, which is just
counter-productive in terms of security...

Really, this a serious regression for the people affected by this.

Samuel

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

[Samuel Thibault]

Samuel Thibault, le dim. 25 juin 2023 17:56:25 +0200, a ecrit:
> Samuel Thibault, le mer. 28 déc. 2022 21:57:26 +0100, a ecrit:
> > Can we perhaps just introduce a CAP_TIOCSTI that the brltty daemon would
> > be able to use? We could even make it only allow TIOCSTI on the linux
> > console (tty->ops == con_ops).
> 
> *Please* comment on this so we can progress. ATM people are
> advising each other to set dev.tty.legacy_tiocsti=1, which is just
> counter-productive in terms of security...

People are even discussing adding that configuration to the brltty
package, which e.g. on ubuntu is installed by default, and thus
defeating completely the security measure by default...

Please do contribute to the discussion so we can fix this.

Samuel

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

[Paul Moore]

On Tue, Jun 27, 2023 at 5:50 PM Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
> Samuel Thibault, le dim. 25 juin 2023 17:56:25 +0200, a ecrit:
> > Samuel Thibault, le mer. 28 déc. 2022 21:57:26 +0100, a ecrit:
> > > Can we perhaps just introduce a CAP_TIOCSTI that the brltty daemon would
> > > be able to use? We could even make it only allow TIOCSTI on the linux
> > > console (tty->ops == con_ops).
> >
> > *Please* comment on this so we can progress. ATM people are
> > advising each other to set dev.tty.legacy_tiocsti=1, which is just
> > counter-productive in terms of security...
>
> People are even discussing adding that configuration to the brltty
> package, which e.g. on ubuntu is installed by default, and thus
> defeating completely the security measure by default...
>
> Please do contribute to the discussion so we can fix this.

Hi Samuel,

I'm sorry to hear that this is impacting Braille terminals, but I do
believe there are solutions in place which would allow affected users
to re-enable TIOCSTI system-wide via the sysctl and then selectively
allow access to the terminal applications.  However, I do believe they
would all require some additional work on the distro/user's part if
the user didn't want to continue to allow system-wide access to
TIOCSTI.

The first thing that comes to mind is an Android-esque filtering that
Kees already mentioned in the commit itself.  I'm not an Android
expert, but based on the linked "ioctl_macros" file in the Android
source, it looks like Android is leveraging the SELinux ioctl extended
permission functionality to limit access to TIOCSTI.  I'm not sure
what experience you have with SELinux, but if you have some
understanding of SELinux policy the documentation below might help you
get started playing with this:

* https://github.com/SELinuxProject/selinux-notebook/blob/main/src/xperm_rules.md

Another option to restrict TIOCSTI once it has been re-enabled
system-wide would be to leverage seccomp to block `ioctl(..., TIOCSTI,
...)`.  Sadly, I don't think one would be able to use systemd's
existing seccomp filtering as it doesn't support syscall parameters,
but I imagine with some work one could add some ioctl smarts to the
systemd seccomp code and/or use an existing seccomp sandboxing tool to
effectively remove TIOCSTI.  Using libseccomp, a simple filter would
look something like this (untested pseudocode, you've been warned):

  ctx = seccomp_init(SCMP_ACT_ALLOW);
  seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EIO), SCMP_SYS(ioctl), 1,
SCMP_A1(TIOCSTI));

I'm sure there are some other good ideas that aren't coming to mind
right now, but I tend to think that the solutions to this are going to
be up in userspace.

-- 
paul-moore.com

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

[Kees Cook]

On Sun, Jun 25, 2023 at 05:56:25PM +0200, Samuel Thibault wrote:
> > Can we perhaps just introduce a CAP_TIOCSTI that the brltty daemon would
> > be able to use? We could even make it only allow TIOCSTI on the linux
> > console (tty->ops == con_ops).

Does brltty run with CAP_SYS_ADMIN? That seems a sensible exception to
be made.

> *Please* comment on this so we can progress. ATM people are
> advising each other to set dev.tty.legacy_tiocsti=1, which is just
> counter-productive in terms of security...

So is there really no solution for brltty and TIOCSTI being disabled?
What is FreeBSD doing? I imagine it's the same situation there too,
though maybe there is just no support?

https://www.mail-archive.com/brltty@brltty.app/msg02892.html

> Really, this a serious regression for the people affected by this.

Can you send a patch adding a CAP_SYS_ADMIN exception?

-Kees

-- 
Kees Cook

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

[Samuel Thibault]

Kees Cook, le mar. 27 juin 2023 19:48:45 -0700, a ecrit:
> On Sun, Jun 25, 2023 at 05:56:25PM +0200, Samuel Thibault wrote:
> > > Can we perhaps just introduce a CAP_TIOCSTI that the brltty daemon would
> > > be able to use? We could even make it only allow TIOCSTI on the linux
> > > console (tty->ops == con_ops).
> 
> Does brltty run with CAP_SYS_ADMIN?

ATM most often, yes, though we are trying to reduce the CAP_* privileges
to what it actually needs.

> > *Please* comment on this so we can progress. ATM people are
> > advising each other to set dev.tty.legacy_tiocsti=1, which is just
> > counter-productive in terms of security...
> 
> So is there really no solution for brltty and TIOCSTI being disabled?

No, there is no way to simulate characters on the Linux console. The
alternative would be to use uinput, but that simulates keycodes, not
characters, thus requiring backtranslating first, which is very fragile.

> What is FreeBSD doing? I imagine it's the same situation there too,
> though maybe there is just no support?

There is just no support in the kernel, only a patch against "screen".

> > Really, this a serious regression for the people affected by this.
> 
> Can you send a patch adding a CAP_SYS_ADMIN exception?

Sure!

Samuel

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

[Kees Cook]

On Wed, Jun 28, 2023 at 08:07:16AM +0200, Samuel Thibault wrote:
> Kees Cook, le mar. 27 juin 2023 19:48:45 -0700, a ecrit:
> > On Sun, Jun 25, 2023 at 05:56:25PM +0200, Samuel Thibault wrote:
> > > > Can we perhaps just introduce a CAP_TIOCSTI that the brltty daemon would
> > > > be able to use? We could even make it only allow TIOCSTI on the linux
> > > > console (tty->ops == con_ops).
> > 
> > Does brltty run with CAP_SYS_ADMIN?
> 
> ATM most often, yes, though we are trying to reduce the CAP_* privileges
> to what it actually needs.
> 
> > > *Please* comment on this so we can progress. ATM people are
> > > advising each other to set dev.tty.legacy_tiocsti=1, which is just
> > > counter-productive in terms of security...
> > 
> > So is there really no solution for brltty and TIOCSTI being disabled?
> 
> No, there is no way to simulate characters on the Linux console. The
> alternative would be to use uinput, but that simulates keycodes, not
> characters, thus requiring backtranslating first, which is very fragile.
> 
> > What is FreeBSD doing? I imagine it's the same situation there too,
> > though maybe there is just no support?
> 
> There is just no support in the kernel, only a patch against "screen".
> 
> > > Really, this a serious regression for the people affected by this.
> > 
> > Can you send a patch adding a CAP_SYS_ADMIN exception?
> 
> Sure!

Thanks! (And be sure to use file->f_cred for the check[1], not "current",
that way brltty can open the tty and drop caps and still do the ioctl.)

-Kees

https://docs.kernel.org/security/credentials.html?highlight=confused+deputy#open-file-credentials

-- 
Kees Cook

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

[Samuel Thibault]

Kees Cook, le mer. 28 juin 2023 09:32:20 -0700, a ecrit:
> On Wed, Jun 28, 2023 at 08:07:16AM +0200, Samuel Thibault wrote:
> > Kees Cook, le mar. 27 juin 2023 19:48:45 -0700, a ecrit:
> > > > Really, this a serious regression for the people affected by this.
> > > 
> > > Can you send a patch adding a CAP_SYS_ADMIN exception?
> > 
> > Sure!
> 
> Thanks! (And be sure to use file->f_cred for the check[1], not "current",
> that way brltty can open the tty and drop caps and still do the ioctl.)

Actually brltty re-opens the various tty[1-6] consoles when the users
switches, so I kept just testing capable(CAP_SYS_ADMIN).

Samuel

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

[Kees Cook]

On Sun, Jul 02, 2023 at 02:00:23AM +0200, Samuel Thibault wrote:
> Kees Cook, le mer. 28 juin 2023 09:32:20 -0700, a ecrit:
> > On Wed, Jun 28, 2023 at 08:07:16AM +0200, Samuel Thibault wrote:
> > > Kees Cook, le mar. 27 juin 2023 19:48:45 -0700, a ecrit:
> > > > > Really, this a serious regression for the people affected by this.
> > > > 
> > > > Can you send a patch adding a CAP_SYS_ADMIN exception?
> > > 
> > > Sure!
> > 
> > Thanks! (And be sure to use file->f_cred for the check[1], not "current",
> > that way brltty can open the tty and drop caps and still do the ioctl.)
> 
> Actually brltty re-opens the various tty[1-6] consoles when the users
> switches, so I kept just testing capable(CAP_SYS_ADMIN).

Well that's frustrating. :P

-- 
Kees Cook

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

[David Laight]

From: Samuel Thibault
> Sent: 28 June 2023 07:07
...
> > So is there really no solution for brltty and TIOCSTI being disabled?
> 
> No, there is no way to simulate characters on the Linux console. The
> alternative would be to use uinput, but that simulates keycodes, not
> characters, thus requiring backtranslating first, which is very fragile.

It could probably be rewritten to use a pseudo-tty pair.
It might even be possible to emulate (the functionality of) TIOCSTI
in the relay process that handles the pseudo-tty.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

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

[Samuel Thibault]

David Laight, le jeu. 29 juin 2023 13:23:54 +0000, a ecrit:
> From: Samuel Thibault
> > Sent: 28 June 2023 07:07
> ...
> > > So is there really no solution for brltty and TIOCSTI being disabled?
> > 
> > No, there is no way to simulate characters on the Linux console. The
> > alternative would be to use uinput, but that simulates keycodes, not
> > characters, thus requiring backtranslating first, which is very fragile.
> 
> It could probably be rewritten to use a pseudo-tty pair.

That's what yasr does for instance, but that does not make the login
prompt accessible, which is a must (just like sighted users don't log in
blindly...)

Samuel

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

[Kees Cook]

On Sat, Oct 22, 2022 at 11:29:47AM -0700, Kees Cook wrote:
> This is so very long over-due. We just need to do this and put an end
> to this legacy operation.

thread ping. Should I carry this in my hardening tree instead?

-- 
Kees Cook

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

[Greg Kroah-Hartman]

On Tue, Nov 01, 2022 at 07:15:43PM -0700, Kees Cook wrote:
> On Sat, Oct 22, 2022 at 11:29:47AM -0700, Kees Cook wrote:
> > This is so very long over-due. We just need to do this and put an end
> > to this legacy operation.
> 
> thread ping. Should I carry this in my hardening tree instead?

I can take it in my tree, I have a bunch of tty/serial patches to go
through this week, give me a few more days please.

thanks,

greg k-h

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

[Kees Cook]

On Wed, Nov 02, 2022 at 03:29:57AM +0100, Greg Kroah-Hartman wrote:
> On Tue, Nov 01, 2022 at 07:15:43PM -0700, Kees Cook wrote:
> > On Sat, Oct 22, 2022 at 11:29:47AM -0700, Kees Cook wrote:
> > > This is so very long over-due. We just need to do this and put an end
> > > to this legacy operation.
> > 
> > thread ping. Should I carry this in my hardening tree instead?
> 
> I can take it in my tree, I have a bunch of tty/serial patches to go
> through this week, give me a few more days please.

Sounds good; thanks for the update!

-- 
Kees Cook