Justin Stitt <[email protected]> writes: Hi,
> Hi, > > On Wed, Jul 30, 2025 at 06:14:43PM -0600, Abhinav Saxena wrote: > > <snip> > >> — >> tools/testing/selftests/tty/Makefile | 6 +- >> tools/testing/selftests/tty/config | 1 + >> tools/testing/selftests/tty/tty_tiocsti_test.c | 650 >> +++++++++++++++++++++++++ >> 3 files changed, 656 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 > > Is it necessary to append -lcap to LDLIBS twice? Once globally and once > for that TU? > So I built the tests in two ways: 1. Building all targets with `make -C tools/testing/selftests/tty/` 2. Building a specific target which is tty_tiocsti_test in this case. I do this with `make -C tools/testing/selftests/tty/ tty_tiocsti_test` I may be completely wrong here, but I think I need the global for (1) and TU specific append for (2). There may better ways to do this, however. Open to ideas :) >> 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..1eafef6e36fa >> — /dev/null >> +++ b/tools/testing/selftests/tty/tty_tiocsti_test.c >> @@ -0,0 +1,650 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * TTY Tests - TIOCSTI >> + * >> + * Copyright © 2025 Abhinav Saxena <[email protected]> >> + */ >> + >> +#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 <pty.h> >> +#include <utmp.h> >> + >> +#include “../kselftest_harness.h” >> + >> +enum test_type { >> + TEST_PTY_TIOCSTI_BASIC, >> + TEST_PTY_TIOCSTI_FD_PASSING, >> + /* other tests cases such as serial may be added. */ >> +}; >> + >> +/* >> + * Test Strategy: >> + * - Basic tests: Use PTY with/without TIOCSCTTY (controlling terminal for >> + * current process) >> + * - FD passing tests: Child creates PTY, parent receives FD (demonstrates >> + * security issue) >> + * >> + * SECURITY VULNERABILITY DEMONSTRATION: >> + * FD passing tests show that TIOCSTI uses CURRENT process credentials, not >> + * opener credentials. This means privileged processes can be given FDs from >> + * unprivileged processes and successfully perform TIOCSTI operations that >> the >> + * unprivileged process couldn’t do directly. >> + * >> + * Attack scenario: >> + * 1. Unprivileged process opens TTY (direct TIOCSTI fails due to lack of >> + * privileges) >> + * 2. Unprivileged process passes FD to privileged process via SCM_RIGHTS >> + * 3. Privileged process can use TIOCSTI on the FD (succeeds due to its >> + * privileges) >> + * 4. Result: Effective privilege escalation via file descriptor passing >> + * >> + * This matches the kernel logic in tiocsti(): >> + * 1. if (!tty_legacy_tiocsti && !capable(CAP_SYS_ADMIN)) return -EIO; >> + * 2. if ((current->signal->tty != tty) && !capable(CAP_SYS_ADMIN)) >> + * return -EPERM; >> + * Note: Both checks use capable() on CURRENT process, not FD opener! >> + * >> + * If the file credentials were also checked along with the capable() checks >> + * then the results for FD pass tests would be consistent with the basic >> tests. >> + */ >> + >> +FIXTURE(tiocsti) >> +{ >> + int pty_master_fd; /* PTY - for basic tests */ >> + int pty_slave_fd; >> + bool has_pty; >> + bool initial_cap_sys_admin; >> + int original_legacy_tiocsti_setting; >> + bool can_modify_sysctl; >> +}; >> + >> +FIXTURE_VARIANT(tiocsti) >> +{ >> + const enum test_type test_type; >> + const bool controlling_tty; /* true=current->signal->tty `= tty */ >> + const int legacy_tiocsti; /* 0=restricted, 1=permissive */ >> + const bool requires_cap; /* true=with CAP_SYS_ADMIN, false=without */ >> + const int expected_success; /* 0=success, -EIO/-EPERM=specific error */ >> +}; >> + >> +/* >> + * Tests Controlling Terminal Variants (current->signal->tty =' tty) >> + * >> + * TIOCSTI Test Matrix: >> + * >> + * | legacy_tiocsti | CAP_SYS_ADMIN | Expected Result | Error | >> + * |—————-|—————|—————–|——-| >> + * | 1 (permissive) | true | SUCCESS | - | >> + * | 1 (permissive) | false | SUCCESS | - | >> + * | 0 (restricted) | true | SUCCESS | - | >> + * | 0 (restricted) | false | FAILURE | -EIO | >> + */ >> + >> +/* clang-format off */ >> +FIXTURE_VARIANT_ADD(tiocsti, basic_pty_permissive_withcap) { >> + .test_type = TEST_PTY_TIOCSTI_BASIC, >> + .controlling_tty = true, >> + .legacy_tiocsti = 1, >> + .requires_cap = true, >> + .expected_success = 0, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, basic_pty_permissive_nocap) { >> + .test_type = TEST_PTY_TIOCSTI_BASIC, >> + .controlling_tty = true, >> + .legacy_tiocsti = 1, >> + .requires_cap = false, >> + .expected_success = 0, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, basic_pty_restricted_withcap) { >> + .test_type = TEST_PTY_TIOCSTI_BASIC, >> + .controlling_tty = true, >> + .legacy_tiocsti = 0, >> + .requires_cap = true, >> + .expected_success = 0, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, basic_pty_restricted_nocap) { >> + .test_type = TEST_PTY_TIOCSTI_BASIC, >> + .controlling_tty = true, >> + .legacy_tiocsti = 0, >> + .requires_cap = false, >> + .expected_success = -EIO, /* FAILURE: legacy restriction */ >> +}; /* clang-format on */ >> + >> +/* >> + * Note for FD Passing Test Variants >> + * Since we’re testing the scenario where an unprivileged process pass an FD >> + * to a privileged one, .requires_cap here means the caps of the child >> process. >> + * Not the parent; parent would always be privileged. >> + */ >> + >> +/* clang-format off */ >> +FIXTURE_VARIANT_ADD(tiocsti, fdpass_pty_permissive_withcap) { >> + .test_type = TEST_PTY_TIOCSTI_FD_PASSING, >> + .controlling_tty = true, >> + .legacy_tiocsti = 1, >> + .requires_cap = true, >> + .expected_success = 0, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, fdpass_pty_permissive_nocap) { >> + .test_type = TEST_PTY_TIOCSTI_FD_PASSING, >> + .controlling_tty = true, >> + .legacy_tiocsti = 1, >> + .requires_cap = false, >> + .expected_success = 0, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, fdpass_pty_restricted_withcap) { >> + .test_type = TEST_PTY_TIOCSTI_FD_PASSING, >> + .controlling_tty = true, >> + .legacy_tiocsti = 0, >> + .requires_cap = true, >> + .expected_success = 0, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, fdpass_pty_restricted_nocap) { >> + .test_type = TEST_PTY_TIOCSTI_FD_PASSING, >> + .controlling_tty = true, >> + .legacy_tiocsti = 0, >> + .requires_cap = false, >> + .expected_success = -EIO, >> +}; /* clang-format on */ >> + >> +/* >> + * Non-Controlling Terminal Variants (current->signal->tty != tty) >> + * >> + * TIOCSTI Test Matrix: >> + * >> + * | legacy_tiocsti | CAP_SYS_ADMIN | Expected Result | Error | >> + * |—————-|—————|—————–|——-| >> + * | 1 (permissive) | true | SUCCESS | - | >> + * | 1 (permissive) | false | FAILURE | -EPERM| >> + * | 0 (restricted) | true | SUCCESS | - | >> + * | 0 (restricted) | false | FAILURE | -EIO | >> + */ >> + >> +/* clang-format off */ >> +FIXTURE_VARIANT_ADD(tiocsti, basic_nopty_permissive_withcap) { >> + .test_type = TEST_PTY_TIOCSTI_BASIC, >> + .controlling_tty = false, >> + .legacy_tiocsti = 1, >> + .requires_cap = true, >> + .expected_success = 0, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, basic_nopty_permissive_nocap) { >> + .test_type = TEST_PTY_TIOCSTI_BASIC, >> + .controlling_tty = false, >> + .legacy_tiocsti = 1, >> + .requires_cap = false, >> + .expected_success = -EPERM, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, basic_nopty_restricted_withcap) { >> + .test_type = TEST_PTY_TIOCSTI_BASIC, >> + .controlling_tty = false, >> + .legacy_tiocsti = 0, >> + .requires_cap = true, >> + .expected_success = 0, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, basic_nopty_restricted_nocap) { >> + .test_type = TEST_PTY_TIOCSTI_BASIC, >> + .controlling_tty = false, >> + .legacy_tiocsti = 0, >> + .requires_cap = false, >> + .expected_success = -EIO, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, fdpass_nopty_permissive_withcap) { >> + .test_type = TEST_PTY_TIOCSTI_FD_PASSING, >> + .controlling_tty = false, >> + .legacy_tiocsti = 1, >> + .requires_cap = true, >> + .expected_success = 0, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, fdpass_nopty_permissive_nocap) { >> + .test_type = TEST_PTY_TIOCSTI_FD_PASSING, >> + .controlling_tty = false, >> + .legacy_tiocsti = 1, >> + .requires_cap = false, >> + .expected_success = -EPERM, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, fdpass_nopty_restricted_withcap) { >> + .test_type = TEST_PTY_TIOCSTI_FD_PASSING, >> + .controlling_tty = false, >> + .legacy_tiocsti = 0, >> + .requires_cap = true, >> + .expected_success = 0, >> +}; >> + >> +FIXTURE_VARIANT_ADD(tiocsti, fdpass_nopty_restricted_nocap) { >> + .test_type = TEST_PTY_TIOCSTI_FD_PASSING, >> + .controlling_tty = false, >> + .legacy_tiocsti = 0, >> + .requires_cap = false, >> + .expected_success = -EIO, >> +}; /* clang-format on */ >> + >> +/* 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; >> +} >> + >> +/* >> + * Drop to nobody user (uid/gid 65534) to lose all capabilities >> + */ >> +static inline bool drop_to_nobody(struct __test_metadata *_metadata) >> +{ > > Maybe we can retrieve the uid/gid from getpwnam(3) with: > const struct passwd *pw = getpwnam(“nobody”); > > … then use pw->pw_{uid,gid}. I suggest this because there might be > portability issues with the hardcoded 65534 – not 100% sure though. > Thanks! Yep, I guess it is better to get rid of `nobody` logic completely for better portability. I replaced it with `drop_all_privs` in v4 [1]. >> + ASSERT_EQ(setgroups(0, NULL), 0); >> + ASSERT_EQ(setgid(65534), 0); >> + ASSERT_EQ(setuid(65534), 0); >> + >> + ASSERT_FALSE(has_cap_sys_admin()); >> + return true; >> +} >> + >> +static inline int get_legacy_tiocsti_setting(struct __test_metadata >> *_metadata) >> +{ >> + FILE *fp; >> + int value = -1; >> + >> + fp = fopen(“/proc/sys/dev/tty/legacy_tiocsti”, “r”); >> + if (!fp) { >> + /* legacy_tiocsti sysctl not available (kernel < 6.2) */ >> + return -1; >> + } >> + >> + if (fscanf(fp, “%d”, &value) == 1) { >> + if (value < 0 || value > 1) >> + value = -1; /* Invalid value */ >> + } else { >> + value = -1; /* Failed to parse */ >> + } >> + >> + fclose(fp); >> + return value; >> +} >> + > > <snip> > >> — >> base-commit: 283564a43383d6f26a55546fe9ae345b5fa95e66 >> change-id: 20250618-toicsti-bug-7822b8e94a32 >> >> Best regards, >> – >> Abhinav Saxena <[email protected]> >> >> > > Justin Thanks for the feedback! • Abhinav [1] - Link to v4: <https://lore.kernel.org/all/[email protected]/>
