* [PATCH 0/2] Possible TTY privilege escalation in TIOCSTI ioctl @ 2025-06-23 1:41 Abhinav Saxena via B4 Relay 2025-06-23 1:41 ` [PATCH 1/2] selftests/tty: add TIOCSTI test suite Abhinav Saxena via B4 Relay ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: Abhinav Saxena via B4 Relay @ 2025-06-23 1:41 UTC (permalink / raw) To: Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Paul Moore, Stephen Smalley, Ondrej Mosnacek Cc: linux-kernel, linux-kselftest, llvm, selinux, Abhinav Saxena, kees, linux-hardening 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@kernel.org> To: Nathan Chancellor <nathan@kernel.org> To: Nick Desaulniers <nick.desaulniers+lkml@gmail.com> To: Bill Wendling <morbo@google.com> To: Justin Stitt <justinstitt@google.com> To: Paul Moore <paul@paul-moore.com> To: Stephen Smalley <stephen.smalley.work@gmail.com> To: Ondrej Mosnacek <omosnace@redhat.com> Cc: linux-kernel@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: llvm@lists.linux.dev Cc: selinux@vger.kernel.org Signed-off-by: Abhinav Saxena <xandfury@gmail.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, -- Abhinav Saxena <xandfury@gmail.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] selftests/tty: add TIOCSTI test suite 2025-06-23 1:41 [PATCH 0/2] Possible TTY privilege escalation in TIOCSTI ioctl Abhinav Saxena via B4 Relay @ 2025-06-23 1:41 ` Abhinav Saxena via B4 Relay 2025-06-23 12:42 ` Stephen Smalley 2025-06-23 1:41 ` [PATCH 2/2] selinux: add capability checks for TIOCSTI ioctl Abhinav Saxena via B4 Relay ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Abhinav Saxena via B4 Relay @ 2025-06-23 1:41 UTC (permalink / raw) To: Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Paul Moore, Stephen Smalley, Ondrej Mosnacek Cc: linux-kernel, linux-kselftest, llvm, selinux, Abhinav Saxena, kees, linux-hardening From: Abhinav Saxena <xandfury@gmail.com> TIOCSTI is a TTY ioctl command that allows inserting characters into the terminal input queue, making it appear as if the user typed those characters. Add a test suite with four tests to verify TIOCSTI behaviour in different scenarios when dev.tty.legacy_tiocsti is both enabled and disabled: - Test TIOCSTI functionality when legacy support is enabled - Test TIOCSTI rejection when legacy support is disabled - Test capability requirements for TIOCSTI usage - Test TIOCSTI security with file descriptor passing The tests validate proper enforcement of the legacy_tiocsti sysctl introduced in commit 83efeeeb3d04 ("tty: Allow TIOCSTI to be disabled"). See tty_ioctl(4) for details on TIOCSTI behavior and security requirements. Signed-off-by: Abhinav Saxena <xandfury@gmail.com> --- tools/testing/selftests/tty/Makefile | 6 +- tools/testing/selftests/tty/config | 1 + tools/testing/selftests/tty/tty_tiocsti_test.c | 421 +++++++++++++++++++++++++ 3 files changed, 427 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/tty/Makefile b/tools/testing/selftests/tty/Makefile index 50d7027b2ae3..7f6fbe5a0cd5 100644 --- a/tools/testing/selftests/tty/Makefile +++ b/tools/testing/selftests/tty/Makefile @@ -1,5 +1,9 @@ # SPDX-License-Identifier: GPL-2.0 CFLAGS = -O2 -Wall -TEST_GEN_PROGS := tty_tstamp_update +TEST_GEN_PROGS := tty_tstamp_update tty_tiocsti_test +LDLIBS += -lcap include ../lib.mk + +# Add libcap for TIOCSTI test +$(OUTPUT)/tty_tiocsti_test: LDLIBS += -lcap diff --git a/tools/testing/selftests/tty/config b/tools/testing/selftests/tty/config new file mode 100644 index 000000000000..c6373aba6636 --- /dev/null +++ b/tools/testing/selftests/tty/config @@ -0,0 +1 @@ +CONFIG_LEGACY_TIOCSTI=y diff --git a/tools/testing/selftests/tty/tty_tiocsti_test.c b/tools/testing/selftests/tty/tty_tiocsti_test.c new file mode 100644 index 000000000000..6a4b497078b0 --- /dev/null +++ b/tools/testing/selftests/tty/tty_tiocsti_test.c @@ -0,0 +1,421 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * TTY Tests - TIOCSTI + * + * Copyright © 2025 Abhinav Saxena <xandfury@gmail.com> + */ + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <fcntl.h> +#include <sys/ioctl.h> +#include <errno.h> +#include <stdbool.h> +#include <string.h> +#include <sys/socket.h> +#include <sys/wait.h> +#include <pwd.h> +#include <termios.h> +#include <grp.h> +#include <sys/capability.h> +#include <sys/prctl.h> + +#include "../kselftest_harness.h" + +/* Helper function to send FD via SCM_RIGHTS */ +static int send_fd_via_socket(int socket_fd, int fd_to_send) +{ + struct msghdr msg = { 0 }; + struct cmsghdr *cmsg; + char cmsg_buf[CMSG_SPACE(sizeof(int))]; + char dummy_data = 'F'; + struct iovec iov = { .iov_base = &dummy_data, .iov_len = 1 }; + + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + msg.msg_control = cmsg_buf; + msg.msg_controllen = sizeof(cmsg_buf); + + cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + + memcpy(CMSG_DATA(cmsg), &fd_to_send, sizeof(int)); + + return sendmsg(socket_fd, &msg, 0) < 0 ? -1 : 0; +} + +/* Helper function to receive FD via SCM_RIGHTS */ +static int recv_fd_via_socket(int socket_fd) +{ + struct msghdr msg = { 0 }; + struct cmsghdr *cmsg; + char cmsg_buf[CMSG_SPACE(sizeof(int))]; + char dummy_data; + struct iovec iov = { .iov_base = &dummy_data, .iov_len = 1 }; + int received_fd = -1; + + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + msg.msg_control = cmsg_buf; + msg.msg_controllen = sizeof(cmsg_buf); + + if (recvmsg(socket_fd, &msg, 0) < 0) + return -1; + + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) { + if (cmsg->cmsg_level == SOL_SOCKET && + cmsg->cmsg_type == SCM_RIGHTS) { + memcpy(&received_fd, CMSG_DATA(cmsg), sizeof(int)); + break; + } + } + + return received_fd; +} + +static inline bool has_cap_sys_admin(void) +{ + cap_t caps = cap_get_proc(); + + if (!caps) + return false; + + cap_flag_value_t cap_val; + bool has_cap = (cap_get_flag(caps, CAP_SYS_ADMIN, CAP_EFFECTIVE, + &cap_val) == 0) && + (cap_val == CAP_SET); + + cap_free(caps); + return has_cap; +} + +/* + * Simple privilege drop that just changes uid/gid in current process + * and also capabilities like CAP_SYS_ADMIN + */ +static inline bool drop_to_nobody(void) +{ + /* Drop supplementary groups */ + if (setgroups(0, NULL) != 0) { + printf("setgroups failed: %s", strerror(errno)); + return false; + } + + /* Change group to nobody */ + if (setgid(65534) != 0) { + printf("setgid failed: %s", strerror(errno)); + return false; + } + + /* Change user to nobody (this drops capabilities) */ + if (setuid(65534) != 0) { + printf("setuid failed: %s", strerror(errno)); + return false; + } + + /* Verify we no longer have CAP_SYS_ADMIN */ + if (has_cap_sys_admin()) { + printf("ERROR: Still have CAP_SYS_ADMIN after changing to nobody"); + return false; + } + + printf("Successfully changed to nobody (uid:%d gid:%d)\n", getuid(), + getgid()); + return true; +} + +static inline int get_legacy_tiocsti_setting(void) +{ + FILE *fp; + int value = -1; + + fp = fopen("/proc/sys/dev/tty/legacy_tiocsti", "r"); + if (!fp) { + if (errno == ENOENT) { + printf("legacy_tiocsti sysctl not available (kernel < 6.2)\n"); + } else { + printf("Cannot read legacy_tiocsti: %s\n", + strerror(errno)); + } + return -1; + } + + if (fscanf(fp, "%d", &value) == 1) { + printf("legacy_tiocsti setting=%d\n", value); + + if (value < 0 || value > 1) { + printf("legacy_tiocsti unexpected value %d\n", value); + value = -1; + } else { + printf("legacy_tiocsti=%d (%s mode)\n", value, + value == 0 ? "restricted" : "permissive"); + } + } else { + printf("Failed to parse legacy_tiocsti value"); + value = -1; + } + + fclose(fp); + return value; +} + +static inline int test_tiocsti_injection(int fd) +{ + int ret; + char test_char = 'X'; + + ret = ioctl(fd, TIOCSTI, &test_char); + if (ret == 0) { + /* Clear the injected character */ + printf("TIOCSTI injection succeeded\n"); + } else { + printf("TIOCSTI injection failed: %s (errno=%d)\n", + strerror(errno), errno); + } + return ret == 0 ? 0 : -1; +} + +FIXTURE(tty_tiocsti) +{ + int tty_fd; + char *tty_name; + bool has_tty; + bool initial_cap_sys_admin; + int legacy_tiocsti_setting; +}; + +FIXTURE_SETUP(tty_tiocsti) +{ + TH_LOG("Running as UID: %d with effective UID: %d", getuid(), + geteuid()); + + self->tty_fd = open("/dev/tty", O_RDWR); + self->has_tty = (self->tty_fd >= 0); + + if (self->tty_fd < 0) + TH_LOG("Cannot open /dev/tty: %s", strerror(errno)); + + self->tty_name = ttyname(STDIN_FILENO); + TH_LOG("Current TTY: %s", self->tty_name ? self->tty_name : "none"); + + self->initial_cap_sys_admin = has_cap_sys_admin(); + TH_LOG("Initial CAP_SYS_ADMIN: %s", + self->initial_cap_sys_admin ? "yes" : "no"); + + self->legacy_tiocsti_setting = get_legacy_tiocsti_setting(); +} + +FIXTURE_TEARDOWN(tty_tiocsti) +{ + if (self->has_tty && self->tty_fd >= 0) + close(self->tty_fd); +} + +/* Test case 1: legacy_tiocsti != 0 (permissive mode) */ +TEST_F(tty_tiocsti, permissive_mode) +{ + // clang-format off + if (self->legacy_tiocsti_setting < 0) + SKIP(return, + "legacy_tiocsti sysctl not available (kernel < 6.2)"); + + if (self->legacy_tiocsti_setting == 0) + SKIP(return, + "Test requires permissive mode (legacy_tiocsti=1)"); + // clang-format on + + ASSERT_TRUE(self->has_tty); + + if (self->initial_cap_sys_admin) { + ASSERT_TRUE(drop_to_nobody()); + ASSERT_FALSE(has_cap_sys_admin()); + } + + /* In permissive mode, TIOCSTI should work without CAP_SYS_ADMIN */ + EXPECT_EQ(test_tiocsti_injection(self->tty_fd), 0) + { + TH_LOG("TIOCSTI should succeed in permissive mode without CAP_SYS_ADMIN"); + } +} + +/* Test case 2: legacy_tiocsti == 0, without CAP_SYS_ADMIN (should fail) */ +TEST_F(tty_tiocsti, restricted_mode_nopriv) +{ + // clang-format off + if (self->legacy_tiocsti_setting < 0) + SKIP(return, + "legacy_tiocsti sysctl not available (kernel < 6.2)"); + + if (self->legacy_tiocsti_setting != 0) + SKIP(return, + "Test requires restricted mode (legacy_tiocsti=0)"); + // clang-format on + + ASSERT_TRUE(self->has_tty); + + if (self->initial_cap_sys_admin) { + ASSERT_TRUE(drop_to_nobody()); + ASSERT_FALSE(has_cap_sys_admin()); + } + /* In restricted mode, TIOCSTI should fail without CAP_SYS_ADMIN */ + EXPECT_EQ(test_tiocsti_injection(self->tty_fd), -1); + + /* + * it might fail with either EPERM or EIO + * EXPECT_TRUE(errno == EPERM || errno == EIO) + * { + * TH_LOG("Expected EPERM, got: %s", strerror(errno)); + * } + */ +} + +/* Test case 3: legacy_tiocsti == 0, with CAP_SYS_ADMIN (should succeed) */ +TEST_F(tty_tiocsti, restricted_mode_priv) +{ + // clang-format off + if (self->legacy_tiocsti_setting < 0) + SKIP(return, + "legacy_tiocsti sysctl not available (kernel < 6.2)"); + + if (self->legacy_tiocsti_setting != 0) + SKIP(return, + "Test requires restricted mode (legacy_tiocsti=0)"); + // clang-format on + + /* Must have CAP_SYS_ADMIN for this test */ + if (!self->initial_cap_sys_admin) + SKIP(return, "Test requires CAP_SYS_ADMIN"); + + ASSERT_TRUE(self->has_tty); + ASSERT_TRUE(has_cap_sys_admin()); + + /* In restricted mode, TIOCSTI should succeed with CAP_SYS_ADMIN */ + EXPECT_EQ(test_tiocsti_injection(self->tty_fd), 0) + { + TH_LOG("TIOCSTI should succeed in restricted mode with CAP_SYS_ADMIN"); + } +} + +/* Test TIOCSTI security with file descriptor passing */ +TEST_F(tty_tiocsti, fd_passing_security) +{ + // clang-format off + if (self->legacy_tiocsti_setting < 0) + SKIP(return, + "legacy_tiocsti sysctl not available (kernel < 6.2)"); + + if (self->legacy_tiocsti_setting != 0) + SKIP(return, + "Test requires restricted mode (legacy_tiocsti=0)"); + // clang-format on + + /* Must start with CAP_SYS_ADMIN */ + if (!self->initial_cap_sys_admin) + SKIP(return, "Test requires initial CAP_SYS_ADMIN"); + + int sockpair[2]; + pid_t child_pid; + + ASSERT_EQ(socketpair(AF_UNIX, SOCK_STREAM, 0, sockpair), 0); + + child_pid = fork(); + ASSERT_GE(child_pid, 0) + TH_LOG("Fork failed: %s", strerror(errno)); + + if (child_pid == 0) { + /* Child process - become unprivileged, open TTY, send FD to parent */ + close(sockpair[0]); + + TH_LOG("Child: Dropping privileges..."); + + /* Drop to nobody user (loses all capabilities) */ + drop_to_nobody(); + + /* Verify we no longer have CAP_SYS_ADMIN */ + if (has_cap_sys_admin()) { + TH_LOG("Child: Failed to drop CAP_SYS_ADMIN"); + _exit(1); + } + + TH_LOG("Child: Opening TTY as unprivileged user..."); + + int unprivileged_tty_fd = open("/dev/tty", O_RDWR); + + if (unprivileged_tty_fd < 0) { + TH_LOG("Child: Cannot open TTY: %s", strerror(errno)); + _exit(1); + } + + /* Test that we can't use TIOCSTI directly (should fail) */ + + char test_char = 'X'; + + if (ioctl(unprivileged_tty_fd, TIOCSTI, &test_char) == 0) { + TH_LOG("Child: ERROR - Direct TIOCSTI succeeded unexpectedly!"); + close(unprivileged_tty_fd); + _exit(1); + } + TH_LOG("Child: Good - Direct TIOCSTI failed as expected: %s", + strerror(errno)); + + /* Send the TTY FD to privileged parent via SCM_RIGHTS */ + TH_LOG("Child: Sending TTY FD to privileged parent..."); + if (send_fd_via_socket(sockpair[1], unprivileged_tty_fd) != 0) { + TH_LOG("Child: Failed to send FD"); + close(unprivileged_tty_fd); + _exit(1); + } + + close(unprivileged_tty_fd); + close(sockpair[1]); + _exit(0); /* Child success */ + + } else { + /* Parent process - keep CAP_SYS_ADMIN, receive FD, test TIOCSTI */ + close(sockpair[1]); + + TH_LOG("Parent: Waiting for TTY FD from unprivileged child..."); + + /* Verify we still have CAP_SYS_ADMIN */ + ASSERT_TRUE(has_cap_sys_admin()); + + /* Receive the TTY FD from unprivileged child */ + int received_fd = recv_fd_via_socket(sockpair[0]); + + ASSERT_GE(received_fd, 0) + TH_LOG("Parent: Received FD %d (opened by unprivileged process)", + received_fd); + + /* + * VULNERABILITY TEST: Try TIOCSTI with FD opened by unprivileged process + * This should FAIL even though parent has CAP_SYS_ADMIN + * because the FD was opened by unprivileged process + */ + char attack_char = 'V'; /* V for Vulnerability */ + int ret = ioctl(received_fd, TIOCSTI, &attack_char); + + TH_LOG("Parent: Testing TIOCSTI on FD from unprivileged process..."); + if (ret == 0) { + TH_LOG("*** VULNERABILITY DETECTED ***"); + TH_LOG("Privileged process can use TIOCSTI on unprivileged FD"); + } else { + TH_LOG("TIOCSTI failed on unprivileged FD: %s", + strerror(errno)); + EXPECT_EQ(errno, EPERM); + } + close(received_fd); + close(sockpair[0]); + + /* Wait for child */ + int status; + + ASSERT_EQ(waitpid(child_pid, &status, 0), child_pid); + EXPECT_EQ(WEXITSTATUS(status), 0); + ASSERT_NE(ret, 0); + } +} + +TEST_HARNESS_MAIN -- 2.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] selftests/tty: add TIOCSTI test suite 2025-06-23 1:41 ` [PATCH 1/2] selftests/tty: add TIOCSTI test suite Abhinav Saxena via B4 Relay @ 2025-06-23 12:42 ` Stephen Smalley 0 siblings, 0 replies; 11+ messages in thread From: Stephen Smalley @ 2025-06-23 12:42 UTC (permalink / raw) To: xandfury Cc: Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Paul Moore, Ondrej Mosnacek, linux-kernel, linux-kselftest, llvm, selinux, kees, linux-hardening On Sun, Jun 22, 2025 at 9:41 PM Abhinav Saxena via B4 Relay <devnull+xandfury.gmail.com@kernel.org> wrote: > > From: Abhinav Saxena <xandfury@gmail.com> > > TIOCSTI is a TTY ioctl command that allows inserting characters into > the terminal input queue, making it appear as if the user typed those > characters. > > Add a test suite with four tests to verify TIOCSTI behaviour in > different scenarios when dev.tty.legacy_tiocsti is both enabled and > disabled: > > - Test TIOCSTI functionality when legacy support is enabled > - Test TIOCSTI rejection when legacy support is disabled > - Test capability requirements for TIOCSTI usage > - Test TIOCSTI security with file descriptor passing > > The tests validate proper enforcement of the legacy_tiocsti sysctl > introduced in commit 83efeeeb3d04 ("tty: Allow TIOCSTI to be disabled"). > See tty_ioctl(4) for details on TIOCSTI behavior and security > requirements. SELinux has its own testsuite at [1] since not everyone enables SELinux, which is where any tests specific to SELinux functionality should be added. [1] https://github.com/selinuxproject/selinux-testsuite > Signed-off-by: Abhinav Saxena <xandfury@gmail.com> > --- > tools/testing/selftests/tty/Makefile | 6 +- > tools/testing/selftests/tty/config | 1 + > tools/testing/selftests/tty/tty_tiocsti_test.c | 421 +++++++++++++++++++++++++ > 3 files changed, 427 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/tty/Makefile b/tools/testing/selftests/tty/Makefile > index 50d7027b2ae3..7f6fbe5a0cd5 100644 > --- a/tools/testing/selftests/tty/Makefile > +++ b/tools/testing/selftests/tty/Makefile > @@ -1,5 +1,9 @@ > # SPDX-License-Identifier: GPL-2.0 > CFLAGS = -O2 -Wall > -TEST_GEN_PROGS := tty_tstamp_update > +TEST_GEN_PROGS := tty_tstamp_update tty_tiocsti_test > +LDLIBS += -lcap > > include ../lib.mk > + > +# Add libcap for TIOCSTI test > +$(OUTPUT)/tty_tiocsti_test: LDLIBS += -lcap > diff --git a/tools/testing/selftests/tty/config b/tools/testing/selftests/tty/config > new file mode 100644 > index 000000000000..c6373aba6636 > --- /dev/null > +++ b/tools/testing/selftests/tty/config > @@ -0,0 +1 @@ > +CONFIG_LEGACY_TIOCSTI=y > diff --git a/tools/testing/selftests/tty/tty_tiocsti_test.c b/tools/testing/selftests/tty/tty_tiocsti_test.c > new file mode 100644 > index 000000000000..6a4b497078b0 > --- /dev/null > +++ b/tools/testing/selftests/tty/tty_tiocsti_test.c > @@ -0,0 +1,421 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * TTY Tests - TIOCSTI > + * > + * Copyright © 2025 Abhinav Saxena <xandfury@gmail.com> > + */ > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <unistd.h> > +#include <fcntl.h> > +#include <sys/ioctl.h> > +#include <errno.h> > +#include <stdbool.h> > +#include <string.h> > +#include <sys/socket.h> > +#include <sys/wait.h> > +#include <pwd.h> > +#include <termios.h> > +#include <grp.h> > +#include <sys/capability.h> > +#include <sys/prctl.h> > + > +#include "../kselftest_harness.h" > + > +/* Helper function to send FD via SCM_RIGHTS */ > +static int send_fd_via_socket(int socket_fd, int fd_to_send) > +{ > + struct msghdr msg = { 0 }; > + struct cmsghdr *cmsg; > + char cmsg_buf[CMSG_SPACE(sizeof(int))]; > + char dummy_data = 'F'; > + struct iovec iov = { .iov_base = &dummy_data, .iov_len = 1 }; > + > + msg.msg_iov = &iov; > + msg.msg_iovlen = 1; > + msg.msg_control = cmsg_buf; > + msg.msg_controllen = sizeof(cmsg_buf); > + > + cmsg = CMSG_FIRSTHDR(&msg); > + cmsg->cmsg_level = SOL_SOCKET; > + cmsg->cmsg_type = SCM_RIGHTS; > + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); > + > + memcpy(CMSG_DATA(cmsg), &fd_to_send, sizeof(int)); > + > + return sendmsg(socket_fd, &msg, 0) < 0 ? -1 : 0; > +} > + > +/* Helper function to receive FD via SCM_RIGHTS */ > +static int recv_fd_via_socket(int socket_fd) > +{ > + struct msghdr msg = { 0 }; > + struct cmsghdr *cmsg; > + char cmsg_buf[CMSG_SPACE(sizeof(int))]; > + char dummy_data; > + struct iovec iov = { .iov_base = &dummy_data, .iov_len = 1 }; > + int received_fd = -1; > + > + msg.msg_iov = &iov; > + msg.msg_iovlen = 1; > + msg.msg_control = cmsg_buf; > + msg.msg_controllen = sizeof(cmsg_buf); > + > + if (recvmsg(socket_fd, &msg, 0) < 0) > + return -1; > + > + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) { > + if (cmsg->cmsg_level == SOL_SOCKET && > + cmsg->cmsg_type == SCM_RIGHTS) { > + memcpy(&received_fd, CMSG_DATA(cmsg), sizeof(int)); > + break; > + } > + } > + > + return received_fd; > +} > + > +static inline bool has_cap_sys_admin(void) > +{ > + cap_t caps = cap_get_proc(); > + > + if (!caps) > + return false; > + > + cap_flag_value_t cap_val; > + bool has_cap = (cap_get_flag(caps, CAP_SYS_ADMIN, CAP_EFFECTIVE, > + &cap_val) == 0) && > + (cap_val == CAP_SET); > + > + cap_free(caps); > + return has_cap; > +} > + > +/* > + * Simple privilege drop that just changes uid/gid in current process > + * and also capabilities like CAP_SYS_ADMIN > + */ > +static inline bool drop_to_nobody(void) > +{ > + /* Drop supplementary groups */ > + if (setgroups(0, NULL) != 0) { > + printf("setgroups failed: %s", strerror(errno)); > + return false; > + } > + > + /* Change group to nobody */ > + if (setgid(65534) != 0) { > + printf("setgid failed: %s", strerror(errno)); > + return false; > + } > + > + /* Change user to nobody (this drops capabilities) */ > + if (setuid(65534) != 0) { > + printf("setuid failed: %s", strerror(errno)); > + return false; > + } > + > + /* Verify we no longer have CAP_SYS_ADMIN */ > + if (has_cap_sys_admin()) { > + printf("ERROR: Still have CAP_SYS_ADMIN after changing to nobody"); > + return false; > + } > + > + printf("Successfully changed to nobody (uid:%d gid:%d)\n", getuid(), > + getgid()); > + return true; > +} > + > +static inline int get_legacy_tiocsti_setting(void) > +{ > + FILE *fp; > + int value = -1; > + > + fp = fopen("/proc/sys/dev/tty/legacy_tiocsti", "r"); > + if (!fp) { > + if (errno == ENOENT) { > + printf("legacy_tiocsti sysctl not available (kernel < 6.2)\n"); > + } else { > + printf("Cannot read legacy_tiocsti: %s\n", > + strerror(errno)); > + } > + return -1; > + } > + > + if (fscanf(fp, "%d", &value) == 1) { > + printf("legacy_tiocsti setting=%d\n", value); > + > + if (value < 0 || value > 1) { > + printf("legacy_tiocsti unexpected value %d\n", value); > + value = -1; > + } else { > + printf("legacy_tiocsti=%d (%s mode)\n", value, > + value == 0 ? "restricted" : "permissive"); > + } > + } else { > + printf("Failed to parse legacy_tiocsti value"); > + value = -1; > + } > + > + fclose(fp); > + return value; > +} > + > +static inline int test_tiocsti_injection(int fd) > +{ > + int ret; > + char test_char = 'X'; > + > + ret = ioctl(fd, TIOCSTI, &test_char); > + if (ret == 0) { > + /* Clear the injected character */ > + printf("TIOCSTI injection succeeded\n"); > + } else { > + printf("TIOCSTI injection failed: %s (errno=%d)\n", > + strerror(errno), errno); > + } > + return ret == 0 ? 0 : -1; > +} > + > +FIXTURE(tty_tiocsti) > +{ > + int tty_fd; > + char *tty_name; > + bool has_tty; > + bool initial_cap_sys_admin; > + int legacy_tiocsti_setting; > +}; > + > +FIXTURE_SETUP(tty_tiocsti) > +{ > + TH_LOG("Running as UID: %d with effective UID: %d", getuid(), > + geteuid()); > + > + self->tty_fd = open("/dev/tty", O_RDWR); > + self->has_tty = (self->tty_fd >= 0); > + > + if (self->tty_fd < 0) > + TH_LOG("Cannot open /dev/tty: %s", strerror(errno)); > + > + self->tty_name = ttyname(STDIN_FILENO); > + TH_LOG("Current TTY: %s", self->tty_name ? self->tty_name : "none"); > + > + self->initial_cap_sys_admin = has_cap_sys_admin(); > + TH_LOG("Initial CAP_SYS_ADMIN: %s", > + self->initial_cap_sys_admin ? "yes" : "no"); > + > + self->legacy_tiocsti_setting = get_legacy_tiocsti_setting(); > +} > + > +FIXTURE_TEARDOWN(tty_tiocsti) > +{ > + if (self->has_tty && self->tty_fd >= 0) > + close(self->tty_fd); > +} > + > +/* Test case 1: legacy_tiocsti != 0 (permissive mode) */ > +TEST_F(tty_tiocsti, permissive_mode) > +{ > + // clang-format off > + if (self->legacy_tiocsti_setting < 0) > + SKIP(return, > + "legacy_tiocsti sysctl not available (kernel < 6.2)"); > + > + if (self->legacy_tiocsti_setting == 0) > + SKIP(return, > + "Test requires permissive mode (legacy_tiocsti=1)"); > + // clang-format on > + > + ASSERT_TRUE(self->has_tty); > + > + if (self->initial_cap_sys_admin) { > + ASSERT_TRUE(drop_to_nobody()); > + ASSERT_FALSE(has_cap_sys_admin()); > + } > + > + /* In permissive mode, TIOCSTI should work without CAP_SYS_ADMIN */ > + EXPECT_EQ(test_tiocsti_injection(self->tty_fd), 0) > + { > + TH_LOG("TIOCSTI should succeed in permissive mode without CAP_SYS_ADMIN"); > + } > +} > + > +/* Test case 2: legacy_tiocsti == 0, without CAP_SYS_ADMIN (should fail) */ > +TEST_F(tty_tiocsti, restricted_mode_nopriv) > +{ > + // clang-format off > + if (self->legacy_tiocsti_setting < 0) > + SKIP(return, > + "legacy_tiocsti sysctl not available (kernel < 6.2)"); > + > + if (self->legacy_tiocsti_setting != 0) > + SKIP(return, > + "Test requires restricted mode (legacy_tiocsti=0)"); > + // clang-format on > + > + ASSERT_TRUE(self->has_tty); > + > + if (self->initial_cap_sys_admin) { > + ASSERT_TRUE(drop_to_nobody()); > + ASSERT_FALSE(has_cap_sys_admin()); > + } > + /* In restricted mode, TIOCSTI should fail without CAP_SYS_ADMIN */ > + EXPECT_EQ(test_tiocsti_injection(self->tty_fd), -1); > + > + /* > + * it might fail with either EPERM or EIO > + * EXPECT_TRUE(errno == EPERM || errno == EIO) > + * { > + * TH_LOG("Expected EPERM, got: %s", strerror(errno)); > + * } > + */ > +} > + > +/* Test case 3: legacy_tiocsti == 0, with CAP_SYS_ADMIN (should succeed) */ > +TEST_F(tty_tiocsti, restricted_mode_priv) > +{ > + // clang-format off > + if (self->legacy_tiocsti_setting < 0) > + SKIP(return, > + "legacy_tiocsti sysctl not available (kernel < 6.2)"); > + > + if (self->legacy_tiocsti_setting != 0) > + SKIP(return, > + "Test requires restricted mode (legacy_tiocsti=0)"); > + // clang-format on > + > + /* Must have CAP_SYS_ADMIN for this test */ > + if (!self->initial_cap_sys_admin) > + SKIP(return, "Test requires CAP_SYS_ADMIN"); > + > + ASSERT_TRUE(self->has_tty); > + ASSERT_TRUE(has_cap_sys_admin()); > + > + /* In restricted mode, TIOCSTI should succeed with CAP_SYS_ADMIN */ > + EXPECT_EQ(test_tiocsti_injection(self->tty_fd), 0) > + { > + TH_LOG("TIOCSTI should succeed in restricted mode with CAP_SYS_ADMIN"); > + } > +} > + > +/* Test TIOCSTI security with file descriptor passing */ > +TEST_F(tty_tiocsti, fd_passing_security) > +{ > + // clang-format off > + if (self->legacy_tiocsti_setting < 0) > + SKIP(return, > + "legacy_tiocsti sysctl not available (kernel < 6.2)"); > + > + if (self->legacy_tiocsti_setting != 0) > + SKIP(return, > + "Test requires restricted mode (legacy_tiocsti=0)"); > + // clang-format on > + > + /* Must start with CAP_SYS_ADMIN */ > + if (!self->initial_cap_sys_admin) > + SKIP(return, "Test requires initial CAP_SYS_ADMIN"); > + > + int sockpair[2]; > + pid_t child_pid; > + > + ASSERT_EQ(socketpair(AF_UNIX, SOCK_STREAM, 0, sockpair), 0); > + > + child_pid = fork(); > + ASSERT_GE(child_pid, 0) > + TH_LOG("Fork failed: %s", strerror(errno)); > + > + if (child_pid == 0) { > + /* Child process - become unprivileged, open TTY, send FD to parent */ > + close(sockpair[0]); > + > + TH_LOG("Child: Dropping privileges..."); > + > + /* Drop to nobody user (loses all capabilities) */ > + drop_to_nobody(); > + > + /* Verify we no longer have CAP_SYS_ADMIN */ > + if (has_cap_sys_admin()) { > + TH_LOG("Child: Failed to drop CAP_SYS_ADMIN"); > + _exit(1); > + } > + > + TH_LOG("Child: Opening TTY as unprivileged user..."); > + > + int unprivileged_tty_fd = open("/dev/tty", O_RDWR); > + > + if (unprivileged_tty_fd < 0) { > + TH_LOG("Child: Cannot open TTY: %s", strerror(errno)); > + _exit(1); > + } > + > + /* Test that we can't use TIOCSTI directly (should fail) */ > + > + char test_char = 'X'; > + > + if (ioctl(unprivileged_tty_fd, TIOCSTI, &test_char) == 0) { > + TH_LOG("Child: ERROR - Direct TIOCSTI succeeded unexpectedly!"); > + close(unprivileged_tty_fd); > + _exit(1); > + } > + TH_LOG("Child: Good - Direct TIOCSTI failed as expected: %s", > + strerror(errno)); > + > + /* Send the TTY FD to privileged parent via SCM_RIGHTS */ > + TH_LOG("Child: Sending TTY FD to privileged parent..."); > + if (send_fd_via_socket(sockpair[1], unprivileged_tty_fd) != 0) { > + TH_LOG("Child: Failed to send FD"); > + close(unprivileged_tty_fd); > + _exit(1); > + } > + > + close(unprivileged_tty_fd); > + close(sockpair[1]); > + _exit(0); /* Child success */ > + > + } else { > + /* Parent process - keep CAP_SYS_ADMIN, receive FD, test TIOCSTI */ > + close(sockpair[1]); > + > + TH_LOG("Parent: Waiting for TTY FD from unprivileged child..."); > + > + /* Verify we still have CAP_SYS_ADMIN */ > + ASSERT_TRUE(has_cap_sys_admin()); > + > + /* Receive the TTY FD from unprivileged child */ > + int received_fd = recv_fd_via_socket(sockpair[0]); > + > + ASSERT_GE(received_fd, 0) > + TH_LOG("Parent: Received FD %d (opened by unprivileged process)", > + received_fd); > + > + /* > + * VULNERABILITY TEST: Try TIOCSTI with FD opened by unprivileged process > + * This should FAIL even though parent has CAP_SYS_ADMIN > + * because the FD was opened by unprivileged process > + */ > + char attack_char = 'V'; /* V for Vulnerability */ > + int ret = ioctl(received_fd, TIOCSTI, &attack_char); > + > + TH_LOG("Parent: Testing TIOCSTI on FD from unprivileged process..."); > + if (ret == 0) { > + TH_LOG("*** VULNERABILITY DETECTED ***"); > + TH_LOG("Privileged process can use TIOCSTI on unprivileged FD"); > + } else { > + TH_LOG("TIOCSTI failed on unprivileged FD: %s", > + strerror(errno)); > + EXPECT_EQ(errno, EPERM); > + } > + close(received_fd); > + close(sockpair[0]); > + > + /* Wait for child */ > + int status; > + > + ASSERT_EQ(waitpid(child_pid, &status, 0), child_pid); > + EXPECT_EQ(WEXITSTATUS(status), 0); > + ASSERT_NE(ret, 0); > + } > +} > + > +TEST_HARNESS_MAIN > > -- > 2.43.0 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] selinux: add capability checks for TIOCSTI ioctl 2025-06-23 1:41 [PATCH 0/2] Possible TTY privilege escalation in TIOCSTI ioctl Abhinav Saxena via B4 Relay 2025-06-23 1:41 ` [PATCH 1/2] selftests/tty: add TIOCSTI test suite Abhinav Saxena via B4 Relay @ 2025-06-23 1:41 ` Abhinav Saxena via B4 Relay 2025-06-23 5:13 ` Greg KH 2025-06-23 12:38 ` Stephen Smalley 2025-06-23 12:35 ` [PATCH 0/2] Possible TTY privilege escalation in " Stephen Smalley 2025-06-28 0:38 ` Abhinav Saxena 3 siblings, 2 replies; 11+ messages in thread From: Abhinav Saxena via B4 Relay @ 2025-06-23 1:41 UTC (permalink / raw) To: Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Paul Moore, Stephen Smalley, Ondrej Mosnacek Cc: linux-kernel, linux-kselftest, llvm, selinux, Abhinav Saxena, kees, linux-hardening From: Abhinav Saxena <xandfury@gmail.com> The TIOCSTI ioctl currently only checks the current process's credentials, creating a TOCTOU vulnerability where an unprivileged process can open a TTY fd and pass it to a privileged process via SCM_RIGHTS. Fix by requiring BOTH the file opener (file->f_cred) AND the current process to have CAP_SYS_ADMIN. This prevents privilege escalation while ensuring legitimate use cases continue to work. Link: https://github.com/KSPP/linux/issues/156 Signed-off-by: Abhinav Saxena <xandfury@gmail.com> --- security/selinux/hooks.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 595ceb314aeb..a628551873ab 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3847,6 +3847,12 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd, CAP_OPT_NONE, true); break; + case TIOCSTI: + if (!file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN) || + !capable(CAP_SYS_ADMIN)) + error = -EPERM; + break; + case FIOCLEX: case FIONCLEX: if (!selinux_policycap_ioctl_skip_cloexec()) -- 2.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] selinux: add capability checks for TIOCSTI ioctl 2025-06-23 1:41 ` [PATCH 2/2] selinux: add capability checks for TIOCSTI ioctl Abhinav Saxena via B4 Relay @ 2025-06-23 5:13 ` Greg KH 2025-06-23 12:38 ` Stephen Smalley 1 sibling, 0 replies; 11+ messages in thread From: Greg KH @ 2025-06-23 5:13 UTC (permalink / raw) To: xandfury Cc: Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Paul Moore, Stephen Smalley, Ondrej Mosnacek, linux-kernel, linux-kselftest, llvm, selinux, kees, linux-hardening On Sun, Jun 22, 2025 at 07:41:08PM -0600, Abhinav Saxena via B4 Relay wrote: > From: Abhinav Saxena <xandfury@gmail.com> > > The TIOCSTI ioctl currently only checks the current process's > credentials, creating a TOCTOU vulnerability where an unprivileged > process can open a TTY fd and pass it to a privileged process via > SCM_RIGHTS. If a priviliged process has a fd, what is the problem with it using this ioctl in the firstplace? > > Fix by requiring BOTH the file opener (file->f_cred) AND the current > process to have CAP_SYS_ADMIN. This prevents privilege escalation > while ensuring legitimate use cases continue to work. > > Link: https://github.com/KSPP/linux/issues/156 > > Signed-off-by: Abhinav Saxena <xandfury@gmail.com> > --- > security/selinux/hooks.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 595ceb314aeb..a628551873ab 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -3847,6 +3847,12 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd, > CAP_OPT_NONE, true); > break; > > + case TIOCSTI: > + if (!file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN) || > + !capable(CAP_SYS_ADMIN)) > + error = -EPERM; > + break; Are you sure this type of policy should be in the selinux core code? Wouldn't you need a "rule" for selinux to follow (or not follow) for this type of thing and not just a blanket change to the logic? Also, have you looked at what userspace tools actually use this ioctl to see if this change would break anything? thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] selinux: add capability checks for TIOCSTI ioctl 2025-06-23 1:41 ` [PATCH 2/2] selinux: add capability checks for TIOCSTI ioctl Abhinav Saxena via B4 Relay 2025-06-23 5:13 ` Greg KH @ 2025-06-23 12:38 ` Stephen Smalley 2025-06-23 15:15 ` Paul Moore 1 sibling, 1 reply; 11+ messages in thread From: Stephen Smalley @ 2025-06-23 12:38 UTC (permalink / raw) To: xandfury Cc: Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Paul Moore, Ondrej Mosnacek, linux-kernel, linux-kselftest, llvm, selinux, kees, linux-hardening On Sun, Jun 22, 2025 at 9:41 PM Abhinav Saxena via B4 Relay <devnull+xandfury.gmail.com@kernel.org> wrote: > > From: Abhinav Saxena <xandfury@gmail.com> > > The TIOCSTI ioctl currently only checks the current process's > credentials, creating a TOCTOU vulnerability where an unprivileged > process can open a TTY fd and pass it to a privileged process via > SCM_RIGHTS. > > Fix by requiring BOTH the file opener (file->f_cred) AND the current > process to have CAP_SYS_ADMIN. This prevents privilege escalation > while ensuring legitimate use cases continue to work. > > Link: https://github.com/KSPP/linux/issues/156 > > Signed-off-by: Abhinav Saxena <xandfury@gmail.com> > --- > security/selinux/hooks.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 595ceb314aeb..a628551873ab 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -3847,6 +3847,12 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd, > CAP_OPT_NONE, true); > break; > > + case TIOCSTI: > + if (!file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN) || > + !capable(CAP_SYS_ADMIN)) > + error = -EPERM; > + break; > + So, aside from what I said previously, this also will break any existing policies currently controlling TIOCSTI via the selinux ioctl checking in the default case, so at the very least, this would need to be gated by a new SELinux policy capability for compatibility purposes. But I'm still unconvinced that this is the right approach. > case FIOCLEX: > case FIONCLEX: > if (!selinux_policycap_ioctl_skip_cloexec()) > > -- > 2.43.0 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] selinux: add capability checks for TIOCSTI ioctl 2025-06-23 12:38 ` Stephen Smalley @ 2025-06-23 15:15 ` Paul Moore 2025-06-24 20:58 ` Günther Noack 0 siblings, 1 reply; 11+ messages in thread From: Paul Moore @ 2025-06-23 15:15 UTC (permalink / raw) To: Stephen Smalley Cc: xandfury, Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Ondrej Mosnacek, linux-kernel, linux-kselftest, llvm, selinux, kees, linux-hardening On Mon, Jun 23, 2025 at 8:39 AM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > On Sun, Jun 22, 2025 at 9:41 PM Abhinav Saxena via B4 Relay > <devnull+xandfury.gmail.com@kernel.org> wrote: > > > > From: Abhinav Saxena <xandfury@gmail.com> > > > > The TIOCSTI ioctl currently only checks the current process's > > credentials, creating a TOCTOU vulnerability where an unprivileged > > process can open a TTY fd and pass it to a privileged process via > > SCM_RIGHTS. > > > > Fix by requiring BOTH the file opener (file->f_cred) AND the current > > process to have CAP_SYS_ADMIN. This prevents privilege escalation > > while ensuring legitimate use cases continue to work. > > > > Link: https://github.com/KSPP/linux/issues/156 > > > > Signed-off-by: Abhinav Saxena <xandfury@gmail.com> > > --- > > security/selinux/hooks.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index 595ceb314aeb..a628551873ab 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -3847,6 +3847,12 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd, > > CAP_OPT_NONE, true); > > break; > > > > + case TIOCSTI: > > + if (!file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN) || > > + !capable(CAP_SYS_ADMIN)) > > + error = -EPERM; > > + break; > > + > > So, aside from what I said previously, this also will break any > existing policies currently controlling TIOCSTI > via the selinux ioctl checking in the default case, so at the very > least, this would need to be gated by a new > SELinux policy capability for compatibility purposes. But I'm still > unconvinced that this is the right approach. I want to add my voice to the other comments that adding these capability checks to the SELinux code and not the main TIOCSTI kernel code is not an approach we want to support. Beyond that, as others have already pointed out, I think some additional inspection and testing is needed to ensure that the additional capability checks do not break existing, valid use cases. -- paul-moore.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] selinux: add capability checks for TIOCSTI ioctl 2025-06-23 15:15 ` Paul Moore @ 2025-06-24 20:58 ` Günther Noack 0 siblings, 0 replies; 11+ messages in thread From: Günther Noack @ 2025-06-24 20:58 UTC (permalink / raw) To: Paul Moore Cc: Stephen Smalley, xandfury, Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Ondrej Mosnacek, linux-kernel, linux-kselftest, llvm, selinux, kees, linux-hardening On Mon, Jun 23, 2025 at 11:15:39AM -0400, Paul Moore wrote: > On Mon, Jun 23, 2025 at 8:39 AM Stephen Smalley > <stephen.smalley.work@gmail.com> wrote: > > On Sun, Jun 22, 2025 at 9:41 PM Abhinav Saxena via B4 Relay > > <devnull+xandfury.gmail.com@kernel.org> wrote: > > > --- a/security/selinux/hooks.c > > > +++ b/security/selinux/hooks.c > > > @@ -3847,6 +3847,12 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd, > > > CAP_OPT_NONE, true); > > > break; > > > > > > + case TIOCSTI: > > > + if (!file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN) || > > > + !capable(CAP_SYS_ADMIN)) > > > + error = -EPERM; > > > + break; > > > + > > > > So, aside from what I said previously, this also will break any > > existing policies currently controlling TIOCSTI > > via the selinux ioctl checking in the default case, so at the very > > least, this would need to be gated by a new > > SELinux policy capability for compatibility purposes. But I'm still > > unconvinced that this is the right approach. > > I want to add my voice to the other comments that adding these > capability checks to the SELinux code and not the main TIOCSTI kernel > code is not an approach we want to support. Beyond that, as others > have already pointed out, I think some additional inspection and > testing is needed to ensure that the additional capability checks do > not break existing, valid use cases. +1 from me as well. If the perceived problem is in core TTY logic, but the proposed fix is in SELinux, it only addresses a fraction of the install base, as not all machines use SELinux. Also, it's not clear to me why the perceived problem of FD-passsing with SCM_RIGHTS is a problem at all. If a CAP_SYS_ADMIN process accepts FDs over SCM_RIGHTS, it is the responsibility of that process not to do unjustified privileged operations with these FDs, on behalf of other, less privileged, processes. In the more classic attack scenarios (as described in a series of CVEs [1]) the process who had the FD first is normally the more privileged one, for for those ones, the existing CAP_SYS_ADMIN check seems fine. —Günther [1] https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=TIOCSTI -- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] Possible TTY privilege escalation in TIOCSTI ioctl 2025-06-23 1:41 [PATCH 0/2] Possible TTY privilege escalation in TIOCSTI ioctl Abhinav Saxena via B4 Relay 2025-06-23 1:41 ` [PATCH 1/2] selftests/tty: add TIOCSTI test suite Abhinav Saxena via B4 Relay 2025-06-23 1:41 ` [PATCH 2/2] selinux: add capability checks for TIOCSTI ioctl Abhinav Saxena via B4 Relay @ 2025-06-23 12:35 ` Stephen Smalley 2025-06-28 0:38 ` Abhinav Saxena 3 siblings, 0 replies; 11+ messages in thread From: Stephen Smalley @ 2025-06-23 12:35 UTC (permalink / raw) To: xandfury Cc: Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Paul Moore, Ondrej Mosnacek, linux-kernel, linux-kselftest, llvm, selinux, kees, linux-hardening On Sun, Jun 22, 2025 at 9:41 PM Abhinav Saxena via B4 Relay <devnull+xandfury.gmail.com@kernel.org> wrote: > > 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. So I'm unconvinced that fixing bugs in core kernel permission checks by adding new checks to SELinux is the right way to go. Also, SELinux already provides a way to control ioctl, although it too uses the current cred, but the granularity of the process labels and the ability to distinguish individual ioctl cmds may mitigate this issue in practice. > > 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@kernel.org> > To: Nathan Chancellor <nathan@kernel.org> > To: Nick Desaulniers <nick.desaulniers+lkml@gmail.com> > To: Bill Wendling <morbo@google.com> > To: Justin Stitt <justinstitt@google.com> > To: Paul Moore <paul@paul-moore.com> > To: Stephen Smalley <stephen.smalley.work@gmail.com> > To: Ondrej Mosnacek <omosnace@redhat.com> > Cc: linux-kernel@vger.kernel.org > Cc: linux-kselftest@vger.kernel.org > Cc: llvm@lists.linux.dev > Cc: selinux@vger.kernel.org > > Signed-off-by: Abhinav Saxena <xandfury@gmail.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, > -- > Abhinav Saxena <xandfury@gmail.com> > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] Possible TTY privilege escalation in TIOCSTI ioctl 2025-06-23 1:41 [PATCH 0/2] Possible TTY privilege escalation in TIOCSTI ioctl Abhinav Saxena via B4 Relay ` (2 preceding siblings ...) 2025-06-23 12:35 ` [PATCH 0/2] Possible TTY privilege escalation in " Stephen Smalley @ 2025-06-28 0:38 ` Abhinav Saxena 2025-06-28 1:52 ` Theodore Ts'o 3 siblings, 1 reply; 11+ messages in thread From: Abhinav Saxena @ 2025-06-28 0:38 UTC (permalink / raw) To: xandfury Cc: Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Paul Moore, Stephen Smalley, Ondrej Mosnacek, linux-kernel, linux-kselftest, llvm, selinux, kees, linux-hardening [-- Attachment #1: Type: text/plain, Size: 6919 bytes --] Abhinav Saxena via B4 Relay <devnull+xandfury.gmail.com@kernel.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@kernel.org> > To: Nathan Chancellor <nathan@kernel.org> > To: Nick Desaulniers <nick.desaulniers+lkml@gmail.com> > To: Bill Wendling <morbo@google.com> > To: Justin Stitt <justinstitt@google.com> > To: Paul Moore <paul@paul-moore.com> > To: Stephen Smalley <stephen.smalley.work@gmail.com> > To: Ondrej Mosnacek <omosnace@redhat.com> > Cc: linux-kernel@vger.kernel.org > Cc: linux-kselftest@vger.kernel.org > Cc: llvm@lists.linux.dev > Cc: selinux@vger.kernel.org > > Signed-off-by: Abhinav Saxena <xandfury@gmail.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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] Possible TTY privilege escalation in TIOCSTI ioctl 2025-06-28 0:38 ` Abhinav Saxena @ 2025-06-28 1:52 ` Theodore Ts'o 0 siblings, 0 replies; 11+ messages in thread From: Theodore Ts'o @ 2025-06-28 1:52 UTC (permalink / raw) To: Abhinav Saxena Cc: Shuah Khan, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Paul Moore, Stephen Smalley, Ondrej Mosnacek, linux-kernel, linux-kselftest, llvm, selinux, kees, linux-hardening On Fri, Jun 27, 2025 at 06:38:42PM -0600, Abhinav Saxena wrote: > > > > 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. What is the threat scenario that you are concerned about? The concern with TIOSTI is that it is a privilege escalation mechanism. But you need to have root (well, CAP_SYS_ADMIN) to either enable the dev.tty.legacy_tiocsti sysctl, or to use TIOCSTI. So what's the privilege escalation that you're concerned about? I could imagine some fairly esoteric ways that this might be a problem, but if it's not a common case concern, maybe using some kind of LSM to more forcibly disable TIOCSTI is sufficient? Yes, we could imagine ways in which it could be permanently disabled (perhaps via a boot command line option) such that it can't be re-enabled without rebooting. But is the extra complexity worth it, especially when there is always the LSM solution for the super-paranoid sysadmins? - Ted ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-06-28 1:52 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-23 1:41 [PATCH 0/2] Possible TTY privilege escalation in TIOCSTI ioctl Abhinav Saxena via B4 Relay 2025-06-23 1:41 ` [PATCH 1/2] selftests/tty: add TIOCSTI test suite Abhinav Saxena via B4 Relay 2025-06-23 12:42 ` Stephen Smalley 2025-06-23 1:41 ` [PATCH 2/2] selinux: add capability checks for TIOCSTI ioctl Abhinav Saxena via B4 Relay 2025-06-23 5:13 ` Greg KH 2025-06-23 12:38 ` Stephen Smalley 2025-06-23 15:15 ` Paul Moore 2025-06-24 20:58 ` Günther Noack 2025-06-23 12:35 ` [PATCH 0/2] Possible TTY privilege escalation in " Stephen Smalley 2025-06-28 0:38 ` Abhinav Saxena 2025-06-28 1:52 ` Theodore Ts'o
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).