| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87ms7cjqz5.fsf@gmail.com>
Date: Tue, 02 Sep 2025 18:44:55 -0600
From: Abhinav Saxena <xandfury@...il.com>
To: Justin Stitt <justinstitt@...gle.com>
Cc: Shuah Khan <shuah@...nel.org>, Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Bill Wendling
<morbo@...gle.com>, Paul Moore <paul@...l-moore.com>, Kees Cook
<kees@...nel.org>, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH v3] selftests/tty: add TIOCSTI test suite
Justin Stitt <justinstitt@...gle.com> 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 <xandfury@...il.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 <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 <xandfury@...il.com>
>>
>>
>
> Justin
Thanks for the feedback!
• Abhinav
[1] - Link to v4:
<https://lore.kernel.org/all/20250902-toicsti-bug-v4-1-e5c960e0b3d6@gmail.com/>
Powered by blists - more mailing lists