| Message ID | 20221015064517.1554119-2-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tty: Allow TIOCSTI to be disabled | expand |
Hi, On 15. 10. 22, 8:45, Kees Cook 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> ... > --- a/drivers/tty/tty_io.c > +++ b/drivers/tty/tty_io.c > @@ -2275,11 +2275,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 int tty_legacy_tiocsti __read_mostly = IS_BUILTIN(CONFIG_LEGACY_TIOCSTI); This can be bool, right? And IS_ENABLED() sounds more appropriate here. > 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)) > @@ -3582,6 +3586,15 @@ 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_dointvec, > + .extra1 = SYSCTL_ZERO, > + .extra2 = SYSCTL_ONE, Then this becomes just proc_dobool without extras. Or we can leave it as int, allow 0, 1, and 2. 2 would log_limited the caller's comm before EIO. Just thinking loudly. Maybe the EIO is enough for users to notice. Likely… thanks,
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 b397b223eada..fa36dac7559e 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -2275,11 +2275,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 int tty_legacy_tiocsti __read_mostly = IS_BUILTIN(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)) @@ -3582,6 +3586,15 @@ 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_dointvec, + .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_ONE, + }, { .procname = "ldisc_autoload", .data = &tty_ldisc_autoload,
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 | 13 +++++++++++++ 2 files changed, 32 insertions(+)