From d43e78b643535da398345d5ae680a96d7b65940e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Nov 2022 17:31:34 +0100 Subject: [PATCH 1/2] chase-symlinks: add new flag for prohibiting any following of symlinks This is useful when operating in the ESP, which is untrusted territory, and where under no circumstances we should be tricked by symlinks into doing anything we don't want to. --- src/basic/chase-symlinks.c | 18 ++++++++++++++++++ src/basic/chase-symlinks.h | 1 + src/test/test-fs-util.c | 9 +++++++++ 3 files changed, 28 insertions(+) diff --git a/src/basic/chase-symlinks.c b/src/basic/chase-symlinks.c index a0d29427d3..35fa97773b 100644 --- a/src/basic/chase-symlinks.c +++ b/src/basic/chase-symlinks.c @@ -57,6 +57,21 @@ static int log_autofs_mount_point(int fd, const char *path, ChaseSymlinksFlags f strna(n1), path); } +static int log_prohibited_symlink(int fd, ChaseSymlinksFlags flags) { + _cleanup_free_ char *n1 = NULL; + + assert(fd >= 0); + + if (!FLAGS_SET(flags, CHASE_WARN)) + return -EREMCHG; + + (void) fd_get_path(fd, &n1); + + return log_warning_errno(SYNTHETIC_ERRNO(EREMCHG), + "Detected symlink where not symlink is allowed at %s, refusing.", + strna(n1)); +} + int chase_symlinks_at( int dir_fd, const char *path, @@ -291,6 +306,9 @@ int chase_symlinks_at( if (S_ISLNK(st.st_mode) && !((flags & CHASE_NOFOLLOW) && isempty(todo))) { _cleanup_free_ char *destination = NULL; + if (flags & CHASE_PROHIBIT_SYMLINKS) + return log_prohibited_symlink(child, flags); + /* This is a symlink, in this case read the destination. But let's make sure we * don't follow symlinks without bounds. */ if (--max_follow <= 0) diff --git a/src/basic/chase-symlinks.h b/src/basic/chase-symlinks.h index 6ea0b17875..af0fcf155a 100644 --- a/src/basic/chase-symlinks.h +++ b/src/basic/chase-symlinks.h @@ -19,6 +19,7 @@ typedef enum ChaseSymlinksFlags { * Note: this may do an NSS lookup, hence this flag cannot be used in PID 1. */ CHASE_AT_RESOLVE_IN_ROOT = 1 << 8, /* Same as openat2()'s RESOLVE_IN_ROOT flag, symlinks are resolved * relative to the given directory fd instead of root. */ + CHASE_PROHIBIT_SYMLINKS = 1 << 9, /* Refuse all symlinks */ } ChaseSymlinksFlags; bool unsafe_transition(const struct stat *a, const struct stat *b); diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index d0259843b6..4bf0a5daf8 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -386,6 +386,15 @@ TEST(chase_symlinks) { assert_se(path_equal(path_startswith(result, p), "usr")); result = mfree(result); + /* Test CHASE_PROHIBIT_SYMLINKS */ + + assert_se(chase_symlinks("top/dot", temp, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, NULL, NULL) == -EREMCHG); + assert_se(chase_symlinks("top/dot", temp, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS|CHASE_WARN, NULL, NULL) == -EREMCHG); + assert_se(chase_symlinks("top/dotdot", temp, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, NULL, NULL) == -EREMCHG); + assert_se(chase_symlinks("top/dotdot", temp, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS|CHASE_WARN, NULL, NULL) == -EREMCHG); + assert_se(chase_symlinks("top/dot/dot", temp, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, NULL, NULL) == -EREMCHG); + assert_se(chase_symlinks("top/dot/dot", temp, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS|CHASE_WARN, NULL, NULL) == -EREMCHG); + cleanup: assert_se(rm_rf(temp, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0); } From b353d5eee9e8df0aa2f4cbb1bfb0a46a963ba78f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Nov 2022 17:36:29 +0100 Subject: [PATCH 2/2] bootctl,bootspec: make use of CHASE_PROHIBIT_SYMLINKS whenever we access the ESP/XBOOTLDR Let's make use of the new flag whenever we access the ESP or XBOOTLDR. The resources we make use of in these partitions can't possibly use symlinks (because UEFI knows no symlink concept), and they are untrusted territory, hence under no circumstances we should be tricked into following symlinks that shouldn't be there in the first place. Of course, you might argue thta ESP/XBOOTLDR are VFAT and thus don#t know symlinks. But the thing is, they don#t have to be. Firmware can support other file systems too, and people can use efifs to gain access to arbitrary Linux file systems from EFI. Hence, let's better be safe than sorry. --- src/boot/bootctl.c | 18 +++++++++--------- src/shared/bootspec.c | 8 ++++---- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 79b2676537..0f142bff87 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -484,7 +484,7 @@ static int enumerate_binaries( assert(previous); assert(is_first); - r = chase_symlinks_and_opendir(path, esp_path, CHASE_PREFIX_ROOT, &p, &d); + r = chase_symlinks_and_opendir(path, esp_path, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, &p, &d); if (r == -ENOENT) return 0; if (r < 0) @@ -913,10 +913,10 @@ static int copy_one_file(const char *esp_path, const char *name, bool force) { if (!p) return log_oom(); - r = chase_symlinks(p, root, CHASE_PREFIX_ROOT, &source_path, NULL); + r = chase_symlinks(p, root, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, &source_path, NULL); /* If we had a root directory to try, we didn't find it and we are in auto mode, retry on the host */ if (r == -ENOENT && root && arg_install_source == ARG_INSTALL_SOURCE_AUTO) - r = chase_symlinks(p, NULL, CHASE_PREFIX_ROOT, &source_path, NULL); + r = chase_symlinks(p, NULL, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, &source_path, NULL); if (r < 0) return log_error_errno(r, "Failed to resolve path %s%s%s: %m", @@ -928,7 +928,7 @@ static int copy_one_file(const char *esp_path, const char *name, bool force) { if (!q) return log_oom(); - r = chase_symlinks(q, esp_path, CHASE_PREFIX_ROOT | CHASE_NONEXISTENT, &dest_path, NULL); + r = chase_symlinks(q, esp_path, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS|CHASE_NONEXISTENT, &dest_path, NULL); if (r < 0) return log_error_errno(r, "Failed to resolve path %s under directory %s: %m", q, esp_path); @@ -945,7 +945,7 @@ static int copy_one_file(const char *esp_path, const char *name, bool force) { v = strjoina("/EFI/BOOT/BOOT", e); ascii_strupper(strrchr(v, '/') + 1); - r = chase_symlinks(v, esp_path, CHASE_PREFIX_ROOT | CHASE_NONEXISTENT, &default_dest_path, NULL); + r = chase_symlinks(v, esp_path, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS|CHASE_NONEXISTENT, &default_dest_path, NULL); if (r < 0) return log_error_errno(r, "Failed to resolve path %s under directory %s: %m", v, esp_path); @@ -963,10 +963,10 @@ static int install_binaries(const char *esp_path, const char *arch, bool force) _cleanup_free_ char *path = NULL; int r; - r = chase_symlinks_and_opendir(BOOTLIBDIR, root, CHASE_PREFIX_ROOT, &path, &d); + r = chase_symlinks_and_opendir(BOOTLIBDIR, root, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, &path, &d); /* If we had a root directory to try, we didn't find it and we are in auto mode, retry on the host */ if (r == -ENOENT && root && arg_install_source == ARG_INSTALL_SOURCE_AUTO) - r = chase_symlinks_and_opendir(BOOTLIBDIR, NULL, CHASE_PREFIX_ROOT, &path, &d); + r = chase_symlinks_and_opendir(BOOTLIBDIR, NULL, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, &path, &d); if (r < 0) return log_error_errno(r, "Failed to open boot loader directory %s%s: %m", strempty(root), BOOTLIBDIR); @@ -1136,7 +1136,7 @@ static int install_variables( return 0; } - r = chase_symlinks_and_access(path, esp_path, CHASE_PREFIX_ROOT, F_OK, NULL, NULL); + r = chase_symlinks_and_access(path, esp_path, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, F_OK, NULL, NULL); if (r == -ENOENT) return 0; if (r < 0) @@ -1167,7 +1167,7 @@ static int remove_boot_efi(const char *esp_path) { _cleanup_free_ char *p = NULL; int r, c = 0; - r = chase_symlinks_and_opendir("/EFI/BOOT", esp_path, CHASE_PREFIX_ROOT, &p, &d); + r = chase_symlinks_and_opendir("/EFI/BOOT", esp_path, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, &p, &d); if (r == -ENOENT) return 0; if (r < 0) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index c3015487c5..0afc41d200 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -508,7 +508,7 @@ static int boot_loader_read_conf_path(BootConfig *config, const char *root, cons assert(config); assert(path); - r = chase_symlinks_and_fopen_unlocked(path, root, CHASE_PREFIX_ROOT, "re", &full, &f); + r = chase_symlinks_and_fopen_unlocked(path, root, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, "re", &full, &f); if (r == -ENOENT) return 0; if (r < 0) @@ -593,7 +593,7 @@ static int boot_entries_find_type1( assert(root); assert(dir); - dir_fd = chase_symlinks_and_open(dir, root, CHASE_PREFIX_ROOT, O_DIRECTORY|O_CLOEXEC, &full); + dir_fd = chase_symlinks_and_open(dir, root, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, O_DIRECTORY|O_CLOEXEC, &full); if (dir_fd == -ENOENT) return 0; if (dir_fd < 0) @@ -853,7 +853,7 @@ static int boot_entries_find_unified( assert(config); assert(dir); - r = chase_symlinks_and_opendir(dir, root, CHASE_PREFIX_ROOT, &full, &d); + r = chase_symlinks_and_opendir(dir, root, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, &full, &d); if (r == -ENOENT) return 0; if (r < 0) @@ -1260,7 +1260,7 @@ static void boot_entry_file_list( assert(p); assert(ret_status); - int status = chase_symlinks_and_access(p, root, CHASE_PREFIX_ROOT, F_OK, NULL, NULL); + int status = chase_symlinks_and_access(p, root, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, F_OK, NULL, NULL); printf("%13s%s ", strempty(field), field ? ":" : " "); if (status < 0) {