From f41ff634cbc300de8ffb881385da2e10f5c0807c Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Dec 01 2023 17:10:38 +0000 Subject: Backport PRs #30170 and #30266 to fix BPF denials (RHBZ #2250930) --- diff --git a/0001-Make-sure-we-close-bpf-outer-map-fd-in-systemd-execu.patch b/0001-Make-sure-we-close-bpf-outer-map-fd-in-systemd-execu.patch new file mode 100644 index 0000000..5388c6a --- /dev/null +++ b/0001-Make-sure-we-close-bpf-outer-map-fd-in-systemd-execu.patch @@ -0,0 +1,31 @@ +From ef90e8f9db911626c8f5c18c49cf6fe445afdefb Mon Sep 17 00:00:00 2001 +From: Daan De Meyer +Date: Thu, 30 Nov 2023 11:01:14 +0100 +Subject: [PATCH] Make sure we close bpf outer map fd in systemd-executor + +Not doing so leaks it into the child service and causes selinux +denials. +--- + src/core/execute-serialize.c | 6 ++++++ + 1 file changed, 6 insertions(+) + +diff --git a/src/core/execute-serialize.c b/src/core/execute-serialize.c +index 56c4f4da8a..6c19cd42a2 100644 +--- a/src/core/execute-serialize.c ++++ b/src/core/execute-serialize.c +@@ -1625,6 +1625,12 @@ static int exec_parameters_deserialize(ExecParameters *p, FILE *f, FDSet *fds) { + if (fd < 0) + continue; + ++ /* This is special and relies on close-on-exec semantics, make sure it's ++ * there */ ++ r = fd_cloexec(fd, true); ++ if (r < 0) ++ return r; ++ + p->bpf_outer_map_fd = fd; + } else if ((val = startswith(l, "exec-parameters-notify-socket="))) { + r = free_and_strdup(&p->notify_socket, val); +-- +2.43.0 + diff --git a/0001-core-pass-bpf_outer_map_fd-to-sd-executor-only-if-Re.patch b/0001-core-pass-bpf_outer_map_fd-to-sd-executor-only-if-Re.patch new file mode 100644 index 0000000..cf947f2 --- /dev/null +++ b/0001-core-pass-bpf_outer_map_fd-to-sd-executor-only-if-Re.patch @@ -0,0 +1,47 @@ +From 60ef4baeedc34b5c7ab0e2f211684f9b96d63f82 Mon Sep 17 00:00:00 2001 +From: Luca Boccassi +Date: Thu, 23 Nov 2023 19:08:22 +0000 +Subject: [PATCH 1/3] core: pass bpf_outer_map_fd to sd-executor only if + RestrictFileSystems was set + +It causes SELinux denials to be raised, so restrict it only where needed + +Follow-up for beb4ae87558cae +--- + src/core/execute-serialize.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +diff --git a/src/core/execute-serialize.c b/src/core/execute-serialize.c +index 342883994a..60c121a0d1 100644 +--- a/src/core/execute-serialize.c ++++ b/src/core/execute-serialize.c +@@ -1244,7 +1244,7 @@ static bool exec_parameters_is_idle_pipe_set(const ExecParameters *p) { + p->idle_pipe[3] >= 0; + } + +-static int exec_parameters_serialize(const ExecParameters *p, FILE *f, FDSet *fds) { ++static int exec_parameters_serialize(const ExecParameters *p, const ExecContext *c, FILE *f, FDSet *fds) { + int r; + + assert(f); +@@ -1375,7 +1375,7 @@ static int exec_parameters_serialize(const ExecParameters *p, FILE *f, FDSet *fd + return r; + } + +- if (p->bpf_outer_map_fd >= 0) { ++ if (c && exec_context_restrict_filesystems_set(c) && p->bpf_outer_map_fd >= 0) { + r = serialize_fd(f, fds, "exec-parameters-bpf-outer-map-fd", p->bpf_outer_map_fd); + if (r < 0) + return r; +@@ -3860,7 +3860,7 @@ int exec_serialize_invocation( + if (r < 0) + return log_debug_errno(r, "Failed to serialize command: %m"); + +- r = exec_parameters_serialize(p, f, fds); ++ r = exec_parameters_serialize(p, ctx, f, fds); + if (r < 0) + return log_debug_errno(r, "Failed to serialize parameters: %m"); + +-- +2.43.0 + diff --git a/0002-core-remove-redundant-check-when-serializing-FDs.patch b/0002-core-remove-redundant-check-when-serializing-FDs.patch new file mode 100644 index 0000000..e09a2e0 --- /dev/null +++ b/0002-core-remove-redundant-check-when-serializing-FDs.patch @@ -0,0 +1,92 @@ +From 2d042c75ffb71f59ebd4689c3972786c29b4bf51 Mon Sep 17 00:00:00 2001 +From: Luca Boccassi +Date: Thu, 23 Nov 2023 19:14:45 +0000 +Subject: [PATCH 2/3] core: remove redundant check when serializing FDs + +The helpers already skip if the FD is < 0 +--- + src/core/execute-serialize.c | 50 ++++++++++++++---------------------- + 1 file changed, 19 insertions(+), 31 deletions(-) + +diff --git a/src/core/execute-serialize.c b/src/core/execute-serialize.c +index 60c121a0d1..56c4f4da8a 100644 +--- a/src/core/execute-serialize.c ++++ b/src/core/execute-serialize.c +@@ -1274,11 +1274,9 @@ static int exec_parameters_serialize(const ExecParameters *p, const ExecContext + return r; + } + +- if (p->n_socket_fds + p->n_storage_fds > 0) { +- r = serialize_fd_many(f, fds, "exec-parameters-fds", p->fds, p->n_socket_fds + p->n_storage_fds); +- if (r < 0) +- return r; +- } ++ r = serialize_fd_many(f, fds, "exec-parameters-fds", p->fds, p->n_socket_fds + p->n_storage_fds); ++ if (r < 0) ++ return r; + } + + r = serialize_strv(f, "exec-parameters-fd-names", p->fd_names); +@@ -1351,31 +1349,23 @@ static int exec_parameters_serialize(const ExecParameters *p, const ExecContext + return r; + } + +- if (p->stdin_fd >= 0) { +- r = serialize_fd(f, fds, "exec-parameters-stdin-fd", p->stdin_fd); +- if (r < 0) +- return r; +- } ++ r = serialize_fd(f, fds, "exec-parameters-stdin-fd", p->stdin_fd); ++ if (r < 0) ++ return r; + +- if (p->stdout_fd >= 0) { +- r = serialize_fd(f, fds, "exec-parameters-stdout-fd", p->stdout_fd); +- if (r < 0) +- return r; +- } ++ r = serialize_fd(f, fds, "exec-parameters-stdout-fd", p->stdout_fd); ++ if (r < 0) ++ return r; + +- if (p->stderr_fd >= 0) { +- r = serialize_fd(f, fds, "exec-parameters-stderr-fd", p->stderr_fd); +- if (r < 0) +- return r; +- } ++ r = serialize_fd(f, fds, "exec-parameters-stderr-fd", p->stderr_fd); ++ if (r < 0) ++ return r; + +- if (p->exec_fd >= 0) { +- r = serialize_fd(f, fds, "exec-parameters-exec-fd", p->exec_fd); +- if (r < 0) +- return r; +- } ++ r = serialize_fd(f, fds, "exec-parameters-exec-fd", p->exec_fd); ++ if (r < 0) ++ return r; + +- if (c && exec_context_restrict_filesystems_set(c) && p->bpf_outer_map_fd >= 0) { ++ if (c && exec_context_restrict_filesystems_set(c)) { + r = serialize_fd(f, fds, "exec-parameters-bpf-outer-map-fd", p->bpf_outer_map_fd); + if (r < 0) + return r; +@@ -1401,11 +1391,9 @@ static int exec_parameters_serialize(const ExecParameters *p, const ExecContext + if (r < 0) + return r; + +- if (p->user_lookup_fd >= 0) { +- r = serialize_fd(f, fds, "exec-parameters-user-lookup-fd", p->user_lookup_fd); +- if (r < 0) +- return r; +- } ++ r = serialize_fd(f, fds, "exec-parameters-user-lookup-fd", p->user_lookup_fd); ++ if (r < 0) ++ return r; + + r = serialize_strv(f, "exec-parameters-files-env", p->files_env); + if (r < 0) +-- +2.43.0 + diff --git a/0003-test-add-a-couple-of-tests-for-RestrictFileSystems.patch b/0003-test-add-a-couple-of-tests-for-RestrictFileSystems.patch new file mode 100644 index 0000000..4034b22 --- /dev/null +++ b/0003-test-add-a-couple-of-tests-for-RestrictFileSystems.patch @@ -0,0 +1,89 @@ +From 4a43c2b3a1066247f26d8a6e52ebfc40852a5f7e Mon Sep 17 00:00:00 2001 +From: Frantisek Sumsal +Date: Fri, 24 Nov 2023 16:00:15 +0100 +Subject: [PATCH 3/3] test: add a couple of tests for RestrictFileSystems= + +--- + test/units/testsuite-07.exec-context.sh | 31 +++++++++++++++++++++++++ + test/units/util.sh | 19 +++++++++++++++ + 2 files changed, 50 insertions(+) + +diff --git a/test/units/testsuite-07.exec-context.sh b/test/units/testsuite-07.exec-context.sh +index b4118d2fe8..10b425359d 100755 +--- a/test/units/testsuite-07.exec-context.sh ++++ b/test/units/testsuite-07.exec-context.sh +@@ -4,6 +4,9 @@ + set -eux + set -o pipefail + ++# shellcheck source=test/units/util.sh ++. "$(dirname "$0")"/util.sh ++ + # Make sure the unit's exec context matches its configuration + # See: https://github.com/systemd/systemd/pull/29552 + +@@ -284,6 +287,34 @@ systemd-run --wait --pipe "${ARGUMENTS[@]}" \ + ulimit -R || exit 0; + : RTTIME; [[ $(ulimit -SR) -eq 666666 ]]; [[ $(ulimit -HR) -eq 666666 ]];' + ++# RestrictFileSystems= ++# ++# Note: running instrumented binaries requires at least /proc to be accessible, so let's ++# skip the test when we're running under sanitizers ++if [[ ! -v ASAN_OPTIONS ]] && systemctl --version | grep "+BPF_FRAMEWORK" && kernel_supports_lsm bpf; then ++ ROOTFS="$(df --output=fstype /usr/bin | sed --quiet 2p)" ++ systemd-run --wait --pipe -p RestrictFileSystems="" ls / ++ systemd-run --wait --pipe -p RestrictFileSystems="$ROOTFS foo bar" ls / ++ (! systemd-run --wait --pipe -p RestrictFileSystems="$ROOTFS" ls /proc) ++ (! systemd-run --wait --pipe -p RestrictFileSystems="foo" ls /) ++ systemd-run --wait --pipe -p RestrictFileSystems="$ROOTFS foo bar baz proc" ls /proc ++ systemd-run --wait --pipe -p RestrictFileSystems="$ROOTFS @foo @basic-api" ls /proc ++ systemd-run --wait --pipe -p RestrictFileSystems="$ROOTFS @foo @basic-api" ls /sys/fs/cgroup ++ ++ systemd-run --wait --pipe -p RestrictFileSystems="~" ls / ++ systemd-run --wait --pipe -p RestrictFileSystems="~proc" ls / ++ systemd-run --wait --pipe -p RestrictFileSystems="~@basic-api" ls / ++ (! systemd-run --wait --pipe -p RestrictFileSystems="~$ROOTFS" ls /) ++ (! systemd-run --wait --pipe -p RestrictFileSystems="~proc" ls /proc) ++ (! systemd-run --wait --pipe -p RestrictFileSystems="~@basic-api" ls /proc) ++ (! systemd-run --wait --pipe -p RestrictFileSystems="~proc foo @bar @basic-api" ls /proc) ++ (! systemd-run --wait --pipe -p RestrictFileSystems="~proc foo @bar @basic-api" ls /sys) ++ systemd-run --wait --pipe -p RestrictFileSystems="~proc devtmpfs sysfs" ls / ++ (! systemd-run --wait --pipe -p RestrictFileSystems="~proc devtmpfs sysfs" ls /proc) ++ (! systemd-run --wait --pipe -p RestrictFileSystems="~proc devtmpfs sysfs" ls /dev) ++ (! systemd-run --wait --pipe -p RestrictFileSystems="~proc devtmpfs sysfs" ls /sys) ++fi ++ + # Ensure that clean-up codepaths work correctly if activation ultimately fails + touch /run/not-a-directory + mkdir /tmp/root +diff --git a/test/units/util.sh b/test/units/util.sh +index fdfb91f8c6..b5ed73237c 100755 +--- a/test/units/util.sh ++++ b/test/units/util.sh +@@ -197,3 +197,22 @@ openssl_supports_kdf() { + # but let's do that when/if the need arises + openssl kdf -keylen 16 -kdfopt digest:SHA2-256 -kdfopt key:foo -out /dev/null "$kdf" + } ++ ++kernel_supports_lsm() { ++ local lsm="${1:?}" ++ local items item ++ ++ if [[ ! -e /sys/kernel/security/lsm ]]; then ++ echo "/sys/kernel/security/lsm doesn't exist, assuming $lsm is not supported" ++ return 1 ++ fi ++ ++ mapfile -t -d, items