diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 9e2a28cc1e..7f7beb830b 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -68,6 +68,7 @@ jobs: include: - test: disable-systemd - test: check + - test: check-no-openat2 - test: enable-shared - test: embedded-blake3 - test: system-blake3 @@ -129,6 +130,20 @@ jobs: echo run tests as rootless in a user namespace unshare -r make check ASAN_OPTIONS=detect_leaks=false || (cat test-suite.log; exit 1) ;; + check-no-openat2) + # Run the test suite with openat2(2) forced to fail with + # ENOSYS, so that the safe_openat() fallback used on kernels + # older than 5.6 is exercised. GitHub runners always have a + # much newer kernel, so this path is otherwise never tested. + sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 + ./configure --enable-embedded-blake3 --disable-dl + make + # Only the root run: without CAP_SYS_ADMIN the helper has to + # set NO_NEW_PRIVS to install its seccomp filter, and that is + # inherited by the test containers. + echo run tests as root, openat2 blocked + sudo make check-no-openat2 ASAN_OPTIONS=detect_leaks=false || (cat test-suite.log; exit 1) + ;; podman) sudo mkdir -p /var/lib/containers /var/tmp sudo docker build -t crun-podman tests/podman diff --git a/Makefile.am b/Makefile.am index bc2e40d9b2..8e2cef8f80 100644 --- a/Makefile.am +++ b/Makefile.am @@ -204,7 +204,7 @@ noinst_PROGRAMS = crun endif if BUILD_TESTS -check_PROGRAMS = tests/init $(UNIT_TESTS) tests/tests_libcrun_fuzzer +check_PROGRAMS = tests/init $(UNIT_TESTS) tests/tests_libcrun_fuzzer tests/no_openat2 TESTS_LDADD = libcrun_testing.la $(FOUND_LIBS) $(JSON_C_LIBS) @@ -213,6 +213,10 @@ tests_init_LDFLAGS = -static-libgcc -all-static tests_init_CFLAGS = -g -O2 tests_init_SOURCES = tests/init.c +tests_no_openat2_LDADD = +tests_no_openat2_CFLAGS = -g -O2 +tests_no_openat2_SOURCES = tests/no_openat2.c + tests_tests_libcrun_utils_CFLAGS = -I $(abs_top_builddir)/libocispec/src -I $(abs_top_srcdir)/libocispec/src $(JSON_C_CFLAGS) -I $(abs_top_builddir)/src -I $(abs_top_srcdir)/src tests_tests_libcrun_utils_SOURCES = tests/tests_libcrun_utils.c tests_tests_libcrun_utils_LDADD = $(TESTS_LDADD) @@ -327,6 +331,14 @@ PYTHON_TESTS = tests/test_capabilities.py \ if BUILD_TESTS TESTS = $(PYTHON_TESTS) $(UNIT_TESTS) + +# Run the whole test suite with openat2(2) forced to fail with ENOSYS. This +# exercises the safe_openat() fallback used on kernels older than 5.6 and +# wherever a seccomp filter blocks openat2, which is otherwise never reached +# on a modern kernel. Run it as root: without CAP_SYS_ADMIN the helper must +# set NO_NEW_PRIVS, which the test containers inherit. +check-no-openat2: tests/no_openat2$(EXEEXT) + $(MAKE) $(AM_MAKEFLAGS) check TESTS_ENVIRONMENT='$(abs_top_builddir)/tests/no_openat2$(EXEEXT)' endif .version: @@ -526,4 +538,4 @@ clean-local: coverage-clean # Coverage targets must not run in parallel due to race conditions in .gcda file writes .NOTPARALLEL: coverage-reset coverage-check coverage-html coverage-xml coverage-summary coverage-multi-env -.PHONY: coverity sync generate-rust-bindings generate-signals.c generate-mount_flags.c clang-format shellcheck coverage-clean coverage-reset coverage-check coverage-html coverage-xml coverage-summary coverage-multi-env +.PHONY: check-no-openat2 coverity sync generate-rust-bindings generate-signals.c generate-mount_flags.c clang-format shellcheck coverage-clean coverage-reset coverage-check coverage-html coverage-xml coverage-summary coverage-multi-env diff --git a/src/libcrun/chroot_realpath.c b/src/libcrun/chroot_realpath.c index deefb576f8..5a94bff3c5 100644 --- a/src/libcrun/chroot_realpath.c +++ b/src/libcrun/chroot_realpath.c @@ -157,9 +157,13 @@ char *chroot_realpath(const char *chroot, const char *path, char resolved_path[] new_path = got_path_root; *new_path++ = '/'; } - else - /* Otherwise back up over this component. */ + else { + /* Otherwise back up over this component, keeping the + separator, so that the expanded symlink is not + concatenated to the parent directory name. */ while (*(--new_path) != '/'); + new_path++; + } /* Safe sex check. */ if (strlen(path) + n >= PATH_MAX - 2) { __set_errno(ENAMETOOLONG); diff --git a/src/libcrun/utils.c b/src/libcrun/utils.c index 07dd1f6f62..fabb85ce6d 100644 --- a/src/libcrun/utils.c +++ b/src/libcrun/utils.c @@ -308,6 +308,11 @@ check_fd_under_path (const char *rootfs, size_t rootfslen, int fd, const char *f char link[PATH_MAX]; int ret; + /* Every path is under "/", there is nothing to verify. The check below + would reject any path since it expects a '/' right after the rootfs. */ + if (rootfslen == 0 || (rootfslen == 1 && rootfs[0] == '/')) + return 0; + get_proc_self_fd_path (fdpath, fd); ret = TEMP_FAILURE_RETRY (readlink (fdpath, link, sizeof (link))); if (UNLIKELY (ret < 0)) @@ -332,39 +337,86 @@ close_and_replace (int *oldfd, int newfd) /* Defined in chroot_realpath.c */ char *chroot_realpath (const char *chroot, const char *path, char resolved_path[]); +/* DIRFD must be a file descriptor for ROOTFS itself: PATH is resolved + against ROOTFS and the result is then opened relatively to DIRFD. */ static int safe_openat_fallback (int dirfd, const char *rootfs, const char *path, int flags, int mode, libcrun_error_t *err) { + cleanup_free char *parent_path = NULL; + const char *last_component = NULL; + const char *orig_path = path; const char *path_in_chroot; cleanup_close int fd = -1; + char resolved[PATH_MAX]; char buffer[PATH_MAX]; - size_t rootfs_len = strlen (rootfs); + size_t rootfs_len = is_empty_string (rootfs) ? 0 : strlen (rootfs); int ret; + /* chroot_realpath resolves the last component as well, so when O_NOFOLLOW + is requested resolve only the parent directory and let openat(2) deal + with the last component, otherwise a symlink would be followed even + though the caller asked not to. */ + if (flags & O_NOFOLLOW) + { + char *sep; + + parent_path = xstrdup (path); + sep = strrchr (parent_path, '/'); + if (sep == NULL) + { + /* No parent directory, the entire path is the last component. */ + last_component = path; + parent_path[0] = '\0'; + } + else if (sep[1] != '\0') + { + *sep = '\0'; + last_component = path + (sep - parent_path) + 1; + } + /* A trailing '/' forces the symlink to be resolved anyway, so in that + case keep resolving the entire path. */ + + if (last_component) + path = parent_path; + } + path_in_chroot = chroot_realpath (rootfs, path, buffer); if (path_in_chroot == NULL) - return crun_make_error (err, errno, "cannot resolve `%s` under rootfs", path); + return crun_make_error (err, errno, "cannot resolve `%s` under rootfs", orig_path); - path_in_chroot += rootfs_len; + /* When rootfs is "/" or not set, chroot_realpath returns the path + unchanged, so drop the prefix only when it is really there. */ + if (rootfs_len > 0 && strncmp (path_in_chroot, rootfs, rootfs_len) == 0) + path_in_chroot += rootfs_len; path_in_chroot = consume_slashes (path_in_chroot); + if (last_component) + { + ret = snprintf (resolved, sizeof (resolved), "%s%s%s", path_in_chroot, + path_in_chroot[0] == '\0' ? "" : "/", last_component); + if (UNLIKELY (ret >= (int) sizeof (resolved))) + return crun_make_error (err, ENAMETOOLONG, "resolve `%s` under rootfs", orig_path); + + path_in_chroot = resolved; + } + /* If the path is empty we are at the root, dup the dirfd itself. */ if (path_in_chroot[0] == '\0') { ret = dup (dirfd); if (UNLIKELY (ret < 0)) - return crun_make_error (err, errno, "dup `%s`", rootfs); + return crun_make_error (err, errno, "dup `%s`", rootfs_len ? rootfs : "/"); return ret; } ret = openat (dirfd, path_in_chroot, flags, mode); if (UNLIKELY (ret < 0)) - return crun_make_error (err, errno, "open `%s`", path); + return crun_make_error (err, errno, "open `%s`", orig_path); fd = ret; - ret = check_fd_under_path (rootfs, rootfs_len, fd, path, err); + ret = check_fd_under_path (rootfs, rootfs_len, fd, orig_path, err); if (UNLIKELY (ret < 0)) return ret; @@ -384,6 +436,10 @@ safe_openat (int dirfd, const char *rootfs, const char *path, int flags, int mod { cleanup_close int fd = -1; + /* A container without a rootfs of its own uses the host root. */ + if (is_empty_string (rootfs)) + rootfs = "/"; + fd = open (rootfs, flags, mode); if (UNLIKELY (fd < 0)) return crun_make_error (err, errno, "open `%s`", rootfs); diff --git a/tests/no_openat2.c b/tests/no_openat2.c new file mode 100644 index 0000000000..32543c84c2 --- /dev/null +++ b/tests/no_openat2.c @@ -0,0 +1,104 @@ +/* + * crun - OCI runtime written in C + * + * Copyright (C) 2026 crun Authors + * crun is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * crun is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with crun. If not, see . + */ + +/* Install a seccomp filter that makes openat2(2) fail with ENOSYS, then exec + the given command. The filter is inherited across exec() and by every + child process, so running the test suite under this helper: + + ./tests/no_openat2 make check + + exercises the safe_openat() fallback path used on kernels older than 5.6, + as well as on any kernel where a seccomp filter blocks openat2. + + Note that without CAP_SYS_ADMIN the filter can only be installed after + NO_NEW_PRIVS is set, and NO_NEW_PRIVS is inherited by the containers + created by the test suite; tests looking at noNewPrivileges fail in that + case. For this reason the test suite is run under this helper only as + root. */ + +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifndef __NR_openat2 +# define __NR_openat2 437 +#endif + +#ifndef SECCOMP_SET_MODE_FILTER +# define SECCOMP_SET_MODE_FILTER 1 +#endif + +static int +install_filter (void) +{ + struct sock_filter filter[] = { + /* Load the syscall number. */ + BPF_STMT (BPF_LD | BPF_W | BPF_ABS, offsetof (struct seccomp_data, nr)), + /* If it is not openat2, allow it. */ + BPF_JUMP (BPF_JMP | BPF_JEQ | BPF_K, __NR_openat2, 0, 1), + BPF_STMT (BPF_RET | BPF_K, SECCOMP_RET_ERRNO | (ENOSYS & SECCOMP_RET_DATA)), + BPF_STMT (BPF_RET | BPF_K, SECCOMP_RET_ALLOW), + }; + struct sock_fprog prog = { + .len = (unsigned short) (sizeof (filter) / sizeof (filter[0])), + .filter = filter, + }; + + if (syscall (__NR_seccomp, SECCOMP_SET_MODE_FILTER, 0, &prog) == 0) + return 0; + + /* Without CAP_SYS_ADMIN the filter can only be installed after + NO_NEW_PRIVS is set. */ + if (errno != EACCES) + return -1; + + if (prctl (PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) < 0) + return -1; + + return syscall (__NR_seccomp, SECCOMP_SET_MODE_FILTER, 0, &prog); +} + +int +main (int argc, char **argv) +{ + if (argc < 2) + { + fprintf (stderr, "usage: %s COMMAND [ARGS...]\n", argv[0]); + return 2; + } + + if (install_filter () < 0) + { + fprintf (stderr, "%s: cannot install seccomp filter: %s\n", argv[0], strerror (errno)); + return 2; + } + + execvp (argv[1], argv + 1); + + fprintf (stderr, "%s: exec `%s`: %s\n", argv[0], argv[1], strerror (errno)); + return 127; +} diff --git a/tests/tests_libcrun_chroot_realpath.c b/tests/tests_libcrun_chroot_realpath.c index e06dd2d2ed..ff8f899a97 100644 --- a/tests/tests_libcrun_chroot_realpath.c +++ b/tests/tests_libcrun_chroot_realpath.c @@ -20,6 +20,10 @@ #include #include #include +#include +#include +#include +#include typedef int (*test) (); @@ -213,6 +217,131 @@ test_deep_path () return 0; } +/* Create a temporary tree used by the symlink tests: + + $root/file a regular file + $root/link -> file (relative, top level) + $root/dir/file a regular file + $root/dir/link -> file (relative, nested) + $root/dir/up -> ../file (relative, with "..") + $root/dir/abs -> /file (absolute, i.e. $root/file) + + Return 0 on success and 77 to skip the test when the tree cannot be + created. */ +static int +make_symlink_tree (char *root, size_t root_size) +{ + char path[PATH_MAX]; + char *tmpdir; + int fd; + + if (snprintf (root, root_size, "%s/crun-chroot-realpath-XXXXXX", + getenv ("TMPDIR") ? getenv ("TMPDIR") : "/tmp") + >= (int) root_size) + return 77; + + tmpdir = mkdtemp (root); + if (tmpdir == NULL) + return 77; + + snprintf (path, sizeof (path), "%s/file", root); + fd = creat (path, 0600); + if (fd < 0) + return 77; + close (fd); + + snprintf (path, sizeof (path), "%s/link", root); + if (symlink ("file", path) < 0) + return 77; + + snprintf (path, sizeof (path), "%s/dir", root); + if (mkdir (path, 0700) < 0) + return 77; + + snprintf (path, sizeof (path), "%s/dir/file", root); + fd = creat (path, 0600); + if (fd < 0) + return 77; + close (fd); + + snprintf (path, sizeof (path), "%s/dir/link", root); + if (symlink ("file", path) < 0) + return 77; + + snprintf (path, sizeof (path), "%s/dir/up", root); + if (symlink ("../file", path) < 0) + return 77; + + snprintf (path, sizeof (path), "%s/dir/abs", root); + if (symlink ("/file", path) < 0) + return 77; + + return 0; +} + +static void +cleanup_symlink_tree (const char *root) +{ + char path[PATH_MAX]; + const char *files[] = { "dir/abs", "dir/up", "dir/link", "dir/file", "dir", "link", "file", NULL }; + size_t i; + + for (i = 0; files[i]; i++) + { + snprintf (path, sizeof (path), "%s/%s", root, files[i]); + if (remove (path) < 0) + continue; + } + rmdir (root); +} + +/* Expanding a symlink must not drop the separator between the parent + directory and the symlink target. */ +static int +test_symlinks () +{ + struct + { + const char *path; + const char *expected; + } cases[] = { + { "/link", "/file" }, + { "/dir/link", "/dir/file" }, + { "/dir/up", "/file" }, + { "/dir/abs", "/file" }, + { NULL, NULL }, + }; + char root[PATH_MAX]; + char resolved[PATH_MAX]; + char expected[PATH_MAX]; + int ret = 0; + size_t i; + + ret = make_symlink_tree (root, sizeof (root)); + if (ret != 0) + return ret; + + for (i = 0; cases[i].path; i++) + { + char *result = chroot_realpath (root, cases[i].path, resolved); + if (result == NULL) + { + ret = -1; + break; + } + + snprintf (expected, sizeof (expected), "%s%s", root, cases[i].expected); + if (strcmp (resolved, expected) != 0) + { + ret = -1; + break; + } + } + + cleanup_symlink_tree (root); + return ret; +} + static void run_and_print_test_result (const char *name, int id, test t) { @@ -235,7 +364,7 @@ int main () { int id = 1; - printf ("1..10\n"); + printf ("1..11\n"); RUN_TEST (test_null_chroot); RUN_TEST (test_empty_chroot); RUN_TEST (test_root_chroot); @@ -246,5 +375,6 @@ main () RUN_TEST (test_simple_path); RUN_TEST (test_trailing_slash); RUN_TEST (test_deep_path); + RUN_TEST (test_symlinks); return 0; } diff --git a/tests/tests_libcrun_utils.c b/tests/tests_libcrun_utils.c index d0b692b0a3..fdc71255e2 100644 --- a/tests/tests_libcrun_utils.c +++ b/tests/tests_libcrun_utils.c @@ -16,6 +16,8 @@ * along with crun. If not, see . */ +#define _GNU_SOURCE + #include #include #include @@ -762,6 +764,53 @@ test_format_default_id_mapping () return 0; } +/* safe_openat must work when the rootfs is "/", both through openat2 and + through the fallback used when openat2 is not available. */ +static int +test_safe_openat_root () +{ + libcrun_error_t err = NULL; + int rootfd, fd; + + rootfd = open ("/", O_PATH | O_CLOEXEC); + if (rootfd < 0) + return -1; + + fd = safe_openat (rootfd, "/", "proc/self/status", O_RDONLY | O_CLOEXEC, 0, &err); + close (rootfd); + if (fd < 0) + { + crun_error_release (&err); + return -1; + } + close (fd); + + return 0; +} + +/* A NULL rootfs is used for containers without a rootfs of their own. */ +static int +test_safe_openat_null_rootfs () +{ + libcrun_error_t err = NULL; + int rootfd, fd; + + rootfd = open ("/", O_PATH | O_CLOEXEC); + if (rootfd < 0) + return -1; + + fd = safe_openat (rootfd, NULL, "proc/self/status", O_RDONLY | O_CLOEXEC, 0, &err); + close (rootfd); + if (fd < 0) + { + crun_error_release (&err); + return -1; + } + close (fd); + + return 0; +} + static void run_and_print_test_result (const char *name, int id, test t) { @@ -785,9 +834,9 @@ main () { int id = 1; #ifdef HAVE_SYSTEMD - printf ("1..18\n"); + printf ("1..20\n"); #else - printf ("1..15\n"); + printf ("1..17\n"); #endif RUN_TEST (test_crun_path_exists); RUN_TEST (test_write_read_file); @@ -804,6 +853,8 @@ main () RUN_TEST (test_crun_ensure_directory); RUN_TEST (test_channel_fd_pair_no_busy_loop_on_blocked_output); RUN_TEST (test_format_default_id_mapping); + RUN_TEST (test_safe_openat_root); + RUN_TEST (test_safe_openat_null_rootfs); #ifdef HAVE_SYSTEMD RUN_TEST (test_parse_sd_array); RUN_TEST (test_get_scope_path);