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

Message ID 20221022182949.2684794-2-keescook@chromium.org
State New
Headers
Series tty: Allow TIOCSTI to be disabled |

Commit Message

Kees Cook Oct. 22, 2022, 6:29 p.m. UTC
  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(+)
  

Comments

Geert Uytterhoeven Nov. 15, 2022, 1:17 p.m. UTC | #1
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
  
Samuel Thibault Dec. 27, 2022, 11:40 p.m. UTC | #2
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 Thibault Dec. 27, 2022, 11:41 p.m. UTC | #3
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.
  
Kees Cook Dec. 28, 2022, 3:32 a.m. UTC | #4
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
  
Samuel Thibault Dec. 28, 2022, 8:57 p.m. UTC | #5
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
  
Samuel Thibault June 25, 2023, 3:56 p.m. UTC | #6
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
  
Samuel Thibault June 27, 2023, 9:50 p.m. UTC | #7
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
  
Paul Moore June 28, 2023, 12:21 a.m. UTC | #8
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.
  
Kees Cook June 28, 2023, 2:48 a.m. UTC | #9
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
  
Samuel Thibault June 28, 2023, 6:07 a.m. UTC | #10
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
  
Kees Cook June 28, 2023, 4:32 p.m. UTC | #11
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
  
David Laight June 29, 2023, 1:23 p.m. UTC | #12
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)
  
Samuel Thibault June 29, 2023, 1:32 p.m. UTC | #13
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
  
Samuel Thibault July 2, 2023, midnight UTC | #14
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
  
Kees Cook July 3, 2023, 7:41 p.m. UTC | #15
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
  

Patch

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,