Message-ID: <87y0tcu23d.fsf@gmail.com>
Date: Fri, 27 Jun 2025 18:38:42 -0600
From: Abhinav Saxena <xandfury@...il.com>
To: xandfury@...il.com
Cc: Shuah Khan <shuah@...nel.org>, Nathan Chancellor <nathan@...nel.org>,
 Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Bill Wendling
 <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>, Paul Moore
 <paul@...l-moore.com>, Stephen Smalley <stephen.smalley.work@...il.com>,
 Ondrej Mosnacek <omosnace@...hat.com>, linux-kernel@...r.kernel.org,
 linux-kselftest@...r.kernel.org, llvm@...ts.linux.dev,
 selinux@...r.kernel.org, kees@...nel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH 0/2] Possible TTY privilege escalation in TIOCSTI ioctl

Abhinav Saxena via B4 Relay <devnull+xandfury.gmail.com@...nel.org> writes:

> This patch series was initially sent to security@k.o; resending it in
> public. I might follow-up with a tests series which addresses similar
> issues with TIOCLINUX.
>
> `============='
>
> The TIOCSTI ioctl uses capable(CAP_SYS_ADMIN) for access control, which
> checks the current process’s credentials. However, it doesn’t validate
> against the file opener’s credentials stored in file->f_cred.
>
> This creates a potential security issue where an unprivileged process
> can open a TTY fd and pass it to a privileged process via SCM_RIGHTS.
> The privileged process may then inadvertently grant access based on its
> elevated privileges rather than the original opener’s credentials.
>
> Background
> `========'
>
> As noted in previous discussion, while CONFIG_LEGACY_TIOCSTI can restrict
> TIOCSTI usage, it is enabled by default in most distributions. Even when
> CONFIG_LEGACY_TIOCSTI=n, processes with CAP_SYS_ADMIN can still use TIOCSTI
> according to the Kconfig documentation.
>
> Additionally, CONFIG_LEGACY_TIOCSTI controls the default value for the
> dev.tty.legacy_tiocsti sysctl, which remains runtime-configurable. This
> means the described attack vector could work on systems even with
> CONFIG_LEGACY_TIOCSTI=n, particularly on Ubuntu 24.04 where it’s “restricted”
> but still functional.
>
> Solution Approach
> `==============='
>
> This series addresses the issue through SELinux LSM integration rather
> than modifying core TTY credential checking to avoid potential compatibility
> issues with existing userspace.
>
> The enhancement adds proper current task and file credential capability
> validation in SELinux’s selinux_file_ioctl() hook specifically for
> TIOCSTI operations.
>
> Testing
> `====='
>
> All patches have been validated using:
> - scripts/checkpatch.pl –strict (0 errors, 0 warnings)
> - Functional testing on kernel v6.16-rc2
> - File descriptor passing security test scenarios
> - SELinux policy enforcement testing
>
> The fd_passing_security test demonstrates the security concern.
> To verify, disable legacy TIOCSTI and run the test:
>
> $ echo “0” | sudo tee /proc/sys/dev/tty/legacy_tiocsti
> $ sudo ./tools/testing/selftests/tty/tty_tiocsti_test -t fd_passing_security
>
> Patch Overview
> `============'
>
> PATCH 1/2: selftests/tty: add TIOCSTI test suite
> Comprehensive test suite demonstrating the issue and fix validation
>
> PATCH 2/2: selinux: add capability checks for TIOCSTI ioctl
> Core security enhancement via SELinux LSM hook
>
> References
> `========'
>
> - tty_ioctl(4) - documents TIOCSTI ioctl and capability requirements
> - commit 83efeeeb3d04 (“tty: Allow TIOCSTI to be disabled”)
> - Documentation/security/credentials.rst
> - <https://github.com/KSPP/linux/issues/156>
> - <https://lore.kernel.org/linux-hardening/Y0m9l52AKmw6Yxi1@hostpad>/
> - drivers/tty/Kconfig
>
> Configuration References:
> [1] - <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/Kconfig#n149>
> [2] - <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/Kconfig#n162>
> [3] - <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/Kconfig#n188>
>
> To: Shuah Khan <shuah@...nel.org>
> To: Nathan Chancellor <nathan@...nel.org>
> To: Nick Desaulniers <nick.desaulniers+lkml@...il.com>
> To: Bill Wendling <morbo@...gle.com>
> To: Justin Stitt <justinstitt@...gle.com>
> To: Paul Moore <paul@...l-moore.com>
> To: Stephen Smalley <stephen.smalley.work@...il.com>
> To: Ondrej Mosnacek <omosnace@...hat.com>
> Cc: linux-kernel@...r.kernel.org
> Cc: linux-kselftest@...r.kernel.org
> Cc: llvm@...ts.linux.dev
> Cc: selinux@...r.kernel.org
>
> Signed-off-by: Abhinav Saxena <xandfury@...il.com>
> —
> Abhinav Saxena (2):
>       selftests/tty: add TIOCSTI test suite
>       selinux: add capability checks for TIOCSTI ioctl
>
>  security/selinux/hooks.c                       |   6 +
>  tools/testing/selftests/tty/Makefile           |   6 +-
>  tools/testing/selftests/tty/config             |   1 +
>  tools/testing/selftests/tty/tty_tiocsti_test.c | 421 +++++++++++++++++++++++++
>  4 files changed, 433 insertions(+), 1 deletion(-)
> —
> base-commit: 5adb635077d1b4bd65b183022775a59a378a9c00
> change-id: 20250618-toicsti-bug-7822b8e94a32
>
> Best regards,

Hi everyone,

Thanks for all the feedback.

SUMMARY OF FEEDBACK RECEIVED
`==========================='

Re: SELinux, I agree. As I mention in the cover letter, using LSM was just a
suggested fix (not a solution). In retrospect, I should’ve added RFC to
the patch series.

I spoke with Kees this past week about the possibility of completely
disabling TIOCSTI while maintaining backwards compatibility. He confirmed
the initial focus was just on adding tests and improving test coverage.

OUTSTANDING QUESTIONS/COMMENTS
`============================'

Based on the discussion, I have three specific questions:

1) Test coverage: Are the current selftests sufficient, or should I add
   more comprehensive TIOCSTI-specific tests? Or maybe KUnit tests?

2) SELinux patch: I can remove the SELinux-specific patch from the
   series.

3) Complete disable option: Is there broader consensus on supporting
   complete TIOCSTI disabling while maintaining backwards compatibility?

DESIGN OPTIONS FOR EXTENDED CONTROL
`=================================='

For question 3, I’ve analyzed three approaches for extending the current
boolean tty_legacy_tiocsti sysctl:

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 Tri-state  Signed    Dual Bool 
 (0,1,2)    (0,1,-1)  Controls  
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
———————|———–|———-|———–|
Backwards Compat     | No        | No       | Yes       |
Clear Semantics      | Yes       | Partial  | Yes       |
Extensibility        | Yes       | Partial  | Yes       |
Single Control       | Yes       | Yes      | No        |
Kernel Precedent     | Common    | Rare     | Common    |

**Option 1 (Tri-state):** 0=CAP_SYS_ADMIN required, 1=legacy, 2=disabled
**Option 2 (Signed):** -1=disabled, 0=restricted, 1=legacy
**Option 3 (Dual bool):** Keep existing bool + add disable bool

The dual boolean approach preserves compatibility but requires managing
two separate controls.

NEXT STEPS
`========'

Based on community input, I’ll:
• Focus on test improvements first
• Drop SELinux-specific changes
• Implement extended control only if there’s consensus on the approach

Feedback welcome on any of these points.

Thanks,
Abhinav

Powered by blists - more mailing lists