| 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: <20240826.queS8Fah5foo@digikod.net>
Date: Mon, 26 Aug 2024 14:40:07 +0200
From: Mickaël Salaün <mic@...ikod.net>
To: Tahera Fahimi <fahimitahera@...il.com>
Cc: outreachy@...ts.linux.dev, gnoack@...gle.com, paul@...l-moore.com,
jmorris@...ei.org, serge@...lyn.com, linux-security-module@...r.kernel.org,
linux-kernel@...r.kernel.org, bjorn3_gh@...tonmail.com, jannh@...gle.com,
netdev@...r.kernel.org
Subject: Re: [PATCH v3 3/6] selftest/Landlock: Signal restriction tests
On Thu, Aug 15, 2024 at 12:29:22PM -0600, Tahera Fahimi wrote:
> This patch expands Landlock ABI version 6 by providing tests for
> signal scoping mechanism. Base on kill(2), if the signal is 0,
> no signal will be sent, but the permission of a process to send
> a signal will be checked. Likewise, this test consider one signal
> for each signal category.
>
> Signed-off-by: Tahera Fahimi <fahimitahera@...il.com>
> ---
> Chnages in versions:
> V2:
> * Moving tests from ptrace_test.c to scoped_signal_test.c
> * Remove debugging statements.
> * Covering all basic restriction scenarios by sending 0 as signal
> V1:
> * Expanding Landlock ABI version 6 by providing basic tests for
> four signals to test signal scoping mechanism.
> ---
> .../selftests/landlock/scoped_signal_test.c | 302 ++++++++++++++++++
> 1 file changed, 302 insertions(+)
> create mode 100644 tools/testing/selftests/landlock/scoped_signal_test.c
>
> diff --git a/tools/testing/selftests/landlock/scoped_signal_test.c b/tools/testing/selftests/landlock/scoped_signal_test.c
> new file mode 100644
> index 000000000000..92958c6266ca
> --- /dev/null
> +++ b/tools/testing/selftests/landlock/scoped_signal_test.c
> @@ -0,0 +1,302 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Landlock tests - Signal Scoping
> + *
> + * Copyright © 2017-2020 Mickaël Salaün <mic@...ikod.net>
> + * Copyright © 2019-2020 ANSSI
> + */
> +
> +#define _GNU_SOURCE
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <linux/landlock.h>
> +#include <signal.h>
> +#include <sys/prctl.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <unistd.h>
> +
> +#include "common.h"
> +
> +static sig_atomic_t signaled;
static volatile sig_atomic_t signaled;
> +
> +static void create_signal_domain(struct __test_metadata *const _metadata)
> +{
> + int ruleset_fd;
> + const struct landlock_ruleset_attr ruleset_attr = {
> + .scoped = LANDLOCK_SCOPED_SIGNAL,
> + };
> +
> + ruleset_fd =
> + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> + EXPECT_LE(0, ruleset_fd)
> + {
> + TH_LOG("Failed to create a ruleset: %s", strerror(errno));
> + }
> + enforce_ruleset(_metadata, ruleset_fd);
> + EXPECT_EQ(0, close(ruleset_fd));
> +}
> +
> +static void scope_signal_handler(int sig, siginfo_t *info, void *ucontext)
> +{
> + if (sig == SIGHUP || sig == SIGURG || sig == SIGTSTP ||
> + sig == SIGTRAP || sig == SIGUSR1) {
> + signaled = 1;
> + }
> +}
> +
> +/* clang-format off */
> +FIXTURE(signal_scoping) {};
> +/* clang-format on */
> +
> +FIXTURE_VARIANT(signal_scoping)
> +{
> + const int sig;
> + const bool domain_both;
> + const bool domain_parent;
> + const bool domain_child;
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, allow_without_domain) {
> + /* clang-format on */
> + .sig = 0,
> + .domain_both = false,
> + .domain_parent = false,
> + .domain_child = false,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, deny_with_child_domain) {
> + /* clang-format on */
> + .sig = 0,
> + .domain_both = false,
> + .domain_parent = false,
> + .domain_child = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, allow_with_parent_domain) {
> + /* clang-format on */
> + .sig = 0,
> + .domain_both = false,
> + .domain_parent = true,
> + .domain_child = false,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, deny_with_sibling_domain) {
> + /* clang-format on */
> + .sig = 0,
> + .domain_both = false,
> + .domain_parent = true,
> + .domain_child = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, allow_sibling_domain) {
> + /* clang-format on */
> + .sig = 0,
> + .domain_both = true,
> + .domain_parent = false,
> + .domain_child = false,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, deny_with_nested_domain) {
> + /* clang-format on */
> + .sig = 0,
> + .domain_both = true,
> + .domain_parent = false,
> + .domain_child = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, allow_with_nested_and_parent_domain) {
> + /* clang-format on */
> + .sig = 0,
> + .domain_both = true,
> + .domain_parent = true,
> + .domain_child = false,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, deny_with_forked_domain) {
> + /* clang-format on */
> + .sig = 0,
> + .domain_both = true,
> + .domain_parent = true,
> + .domain_child = true,
> +};
> +
> +/* Default Action: Terminate*/
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, deny_with_forked_domain_SIGHUP) {
> + /* clang-format on */
> + .sig = SIGHUP,
> + .domain_both = true,
> + .domain_parent = true,
> + .domain_child = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, allow_with_forked_domain_SIGHUP) {
> + /* clang-format on */
> + .sig = SIGHUP,
> + .domain_both = false,
> + .domain_parent = true,
> + .domain_child = false,
> +};
> +
> +/* Default Action: Ignore*/
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, deny_with_forked_domain_SIGURG) {
> + /* clang-format on */
> + .sig = SIGURG,
> + .domain_both = true,
> + .domain_parent = true,
> + .domain_child = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, allow_with_forked_domain_SIGURG) {
> + /* clang-format on */
> + .sig = SIGURG,
> + .domain_both = false,
> + .domain_parent = true,
> + .domain_child = false,
> +};
> +
> +/* Default Action: Stop*/
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, deny_with_forked_domain_SIGTSTP) {
> + /* clang-format on */
> + .sig = SIGTSTP,
> + .domain_both = true,
> + .domain_parent = true,
> + .domain_child = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, allow_with_forked_domain_SIGTSTP) {
> + /* clang-format on */
> + .sig = SIGTSTP,
> + .domain_both = false,
> + .domain_parent = true,
> + .domain_child = false,
> +};
> +
> +/* Default Action: Coredump*/
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, deny_with_forked_domain_SIGTRAP) {
> + /* clang-format on */
> + .sig = SIGTRAP,
> + .domain_both = true,
> + .domain_parent = true,
> + .domain_child = true,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, allow_with_forked_domain_SIGTRAP) {
> + /* clang-format on */
> + .sig = SIGTRAP,
> + .domain_both = false,
> + .domain_parent = true,
> + .domain_child = false,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(signal_scoping, deny_with_forked_domain_SIGUSR1) {
> + /* clang-format on */
> + .sig = SIGUSR1,
> + .domain_both = true,
> + .domain_parent = true,
> + .domain_child = true,
> +};
> +
> +FIXTURE_SETUP(signal_scoping)
> +{
> +}
> +
> +FIXTURE_TEARDOWN(signal_scoping)
> +{
> +}
> +
> +TEST_F(signal_scoping, test_signal)
Sometime, this test hang. I suspect the following issue:
> +{
> + pid_t child;
> + pid_t parent = getpid();
> + int status;
> + bool can_signal;
> + int pipe_parent[2];
> + struct sigaction action = {
> + .sa_sigaction = scope_signal_handler,
> + .sa_flags = SA_SIGINFO,
> +
> + };
> +
> + can_signal = !variant->domain_child;
> +
> + if (variant->sig > 0)
> + ASSERT_LE(0, sigaction(variant->sig, &action, NULL));
> +
> + if (variant->domain_both) {
> + create_signal_domain(_metadata);
> + if (!__test_passed(_metadata))
> + /* Aborts before forking. */
> + return;
> + }
> + ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> +
> + child = fork();
> + ASSERT_LE(0, child);
> + if (child == 0) {
> + char buf_child;
> + int err;
> +
> + ASSERT_EQ(0, close(pipe_parent[1]));
> + if (variant->domain_child)
> + create_signal_domain(_metadata);
> +
> + /* Waits for the parent to be in a domain, if any. */
> + ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
There is a race condition here when the parent process didn't yet called
pause().
> +
> + err = kill(parent, variant->sig);
> + if (can_signal) {
> + ASSERT_EQ(0, err);
> + } else {
> + ASSERT_EQ(-1, err);
> + ASSERT_EQ(EPERM, errno);
> + }
> + /* no matter of the domain, a process should be able to send
> + * a signal to itself.
> + */
> + ASSERT_EQ(0, raise(variant->sig));
> + if (variant->sig > 0)
> + ASSERT_EQ(1, signaled);
> + _exit(_metadata->exit_code);
> + return;
> + }
> + ASSERT_EQ(0, close(pipe_parent[0]));
> + if (variant->domain_parent)
> + create_signal_domain(_metadata);
> +
/* The process should not have already been signaled. */
EXPECT_EQ(0, signaled);
> + /* Signals that the parent is in a domain, if any. */
> + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> +
> + if (can_signal && variant->sig > 0) {
> + ASSERT_EQ(-1, pause());
> + ASSERT_EQ(EINTR, errno);
This can hang indefinitely if the child process sent the signal after
reading from the pipe and before the call to pause().
This should be a better alternative to the use of pause():
/* Avoids race condition with the child's signal. */
while (!signaled && !usleep(1));
ASSERT_EQ(1, signaled);
BTW, we cannot reliably check for errno because usleep() may still
return 0, but that's OK.
> + ASSERT_EQ(1, signaled);
> + } else {
> + ASSERT_EQ(0, signaled);
> + }
> +
> + ASSERT_EQ(child, waitpid(child, &status, 0));
> +
> + if (WIFSIGNALED(status) || !WIFEXITED(status) ||
> + WEXITSTATUS(status) != EXIT_SUCCESS)
> + _metadata->exit_code = KSFT_FAIL;
> +}
> +
> +TEST_HARNESS_MAIN
> --
> 2.34.1
>
>
Powered by blists - more mailing lists