From 3841fee822184d8df8d00d47b9143cdfcad49087 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Jan 2020 16:24:33 +0100 Subject: [PATCH 1/5] path-util: introduce path_strv_contains() helper it's like strv_contains() but uses path_equal() rather than streq() to compare strings. --- src/basic/path-util.c | 10 ++++++++++ src/basic/path-util.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index f1be8d0988..49a211a527 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -1115,3 +1115,13 @@ bool empty_or_root(const char *root) { return root[strspn(root, "/")] == 0; } + +bool path_strv_contains(char **l, const char *path) { + char **i; + + STRV_FOREACH(i, l) + if (path_equal(*i, path)) + return true; + + return false; +} diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 111d85d445..3e8c12481b 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -181,3 +181,5 @@ bool empty_or_root(const char *root); static inline const char *empty_to_root(const char *path) { return isempty(path) ? "/" : path; } + +bool path_strv_contains(char **l, const char *path); From 3593fa60f2a1549a09335e88ca7553f548f07ad8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Jan 2020 12:23:57 +0100 Subject: [PATCH 2/5] path-util: express PATH_IN_SET() through path_strv_contains() --- src/basic/path-util.h | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 3e8c12481b..f49a876f3d 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -71,17 +71,7 @@ static inline bool path_equal_ptr(const char *a, const char *b) { } /* Note: the search terminates on the first NULL item. */ -#define PATH_IN_SET(p, ...) \ - ({ \ - char **_s; \ - bool _found = false; \ - STRV_FOREACH(_s, STRV_MAKE(__VA_ARGS__)) \ - if (path_equal(p, *_s)) { \ - _found = true; \ - break; \ - } \ - _found; \ - }) +#define PATH_IN_SET(p, ...) path_strv_contains(STRV_MAKE(__VA_ARGS__), p) char* path_startswith_strv(const char *p, char **set); #define PATH_STARTSWITH_SET(p, ...) path_startswith_strv(p, STRV_MAKE(__VA_ARGS__)) From 9a2ec8f7a6d55edc029eb83d23bb5cc786b24edb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Jan 2020 16:42:25 +0100 Subject: [PATCH 3/5] install: use path_strv_contains() where appropriate --- src/shared/install.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/shared/install.c b/src/shared/install.c index 0e673b3358..b98c8e01cf 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -105,7 +105,6 @@ DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(unit_file_type, UnitFileType); static int in_search_path(const LookupPaths *p, const char *path) { _cleanup_free_ char *parent = NULL; - char **i; assert(path); @@ -113,11 +112,7 @@ static int in_search_path(const LookupPaths *p, const char *path) { if (!parent) return -ENOMEM; - STRV_FOREACH(i, p->search_path) - if (path_equal(parent, *i)) - return true; - - return false; + return path_strv_contains(p->search_path, parent); } static const char* skip_root(const LookupPaths *p, const char *path) { From c8c535d589cce28f586e03a291be93cb2e48cb97 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Jan 2020 16:25:11 +0100 Subject: [PATCH 4/5] namespace: tweak checks whether we can mount image read-only So far we set up a loopback file read-only iff ProtectSystem= and ProtectHome= both where set to values that mark these dirs read-only. Let's extend that and also be happy if /home and the root dir are marked read-only by some other means. Fixes: #14442 --- src/core/namespace.c | 59 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index fee4c98096..fd38174ac0 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -1190,6 +1190,57 @@ static void normalize_mounts(const char *root_directory, MountEntry *mounts, siz drop_nop(mounts, n_mounts); } +static bool root_read_only( + char **read_only_paths, + ProtectSystem protect_system) { + + /* Determine whether the root directory is going to be read-only given the configured settings. */ + + if (protect_system == PROTECT_SYSTEM_STRICT) + return true; + + if (path_strv_contains(read_only_paths, "/")) + return true; + + return false; +} + +static bool home_read_only( + char** read_only_paths, + char** inaccessible_paths, + char** empty_directories, + const BindMount *bind_mounts, + size_t n_bind_mounts, + const TemporaryFileSystem *temporary_filesystems, + size_t n_temporary_filesystems, + ProtectHome protect_home) { + + size_t i; + + /* Determine whether the /home directory is going to be read-only given the configured settings. Yes, + * this is a bit sloppy, since we don't bother checking for cases where / is affected by multiple + * settings. */ + + if (protect_home != PROTECT_HOME_NO) + return true; + + if (path_strv_contains(read_only_paths, "/home") || + path_strv_contains(inaccessible_paths, "/home") || + path_strv_contains(empty_directories, "/home")) + return true; + + for (i = 0; i < n_temporary_filesystems; i++) + if (path_equal(temporary_filesystems[i].path, "/home")) + return true; + + /* If /home is overmounted with some dir from the host it's not writable. */ + for (i = 0; i < n_bind_mounts; i++) + if (path_equal(bind_mounts[i].destination, "/home")) + return true; + + return false; +} + int setup_namespace( const char* root_directory, const char* root_image, @@ -1228,8 +1279,12 @@ int setup_namespace( if (root_image) { dissect_image_flags |= DISSECT_IMAGE_REQUIRE_ROOT; - if (protect_system == PROTECT_SYSTEM_STRICT && - protect_home != PROTECT_HOME_NO && + /* Make the whole image read-only if we can determine that we only access it in a read-only fashion. */ + if (root_read_only(read_only_paths, + protect_system) && + home_read_only(read_only_paths, inaccessible_paths, empty_directories, + bind_mounts, n_bind_mounts, temporary_filesystems, n_temporary_filesystems, + protect_home) && strv_isempty(read_write_paths)) dissect_image_flags |= DISSECT_IMAGE_READ_ONLY; From b0a94268f8761cfa0dfa532fb8b18620b381f440 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Jan 2020 16:56:05 +0100 Subject: [PATCH 5/5] core: when we cannot open an image file for write, try read-only Closes: #14442 --- src/core/namespace.c | 2 +- src/shared/loop-util.c | 25 +++++++++++++++++++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index fd38174ac0..a6f803671e 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -1289,7 +1289,7 @@ int setup_namespace( dissect_image_flags |= DISSECT_IMAGE_READ_ONLY; r = loop_device_make_by_path(root_image, - dissect_image_flags & DISSECT_IMAGE_READ_ONLY ? O_RDONLY : O_RDWR, + FLAGS_SET(dissect_image_flags, DISSECT_IMAGE_READ_ONLY) ? O_RDONLY : -1 /* < 0 means writable if possible, read-only as fallback */, LO_FLAGS_PARTSCAN, &loop_device); if (r < 0) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index d92ef96162..7aee239e33 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -14,6 +14,7 @@ #include #include "alloc-util.h" +#include "errno-util.h" #include "fd-util.h" #include "fileio.h" #include "loop-util.h" @@ -157,14 +158,30 @@ int loop_device_make( int loop_device_make_by_path(const char *path, int open_flags, uint32_t loop_flags, LoopDevice **ret) { _cleanup_close_ int fd = -1; + int r; assert(path); assert(ret); - assert(IN_SET(open_flags, O_RDWR, O_RDONLY)); + assert(open_flags < 0 || IN_SET(open_flags, O_RDWR, O_RDONLY)); - fd = open(path, O_CLOEXEC|O_NONBLOCK|O_NOCTTY|open_flags); - if (fd < 0) - return -errno; + /* Passing < 0 as open_flags here means we'll try to open the device writable if we can, retrying + * read-only if we cannot. */ + + fd = open(path, O_CLOEXEC|O_NONBLOCK|O_NOCTTY|(open_flags >= 0 ? open_flags : O_RDWR)); + if (fd < 0) { + r = -errno; + + /* Retry read-only? */ + if (open_flags >= 0 || !(ERRNO_IS_PRIVILEGE(r) || r == -EROFS)) + return r; + + fd = open(path, O_CLOEXEC|O_NONBLOCK|O_NOCTTY|O_RDONLY); + if (fd < 0) + return r; /* Propagate original error */ + + open_flags = O_RDONLY; + } else if (open_flags < 0) + open_flags = O_RDWR; return loop_device_make(fd, open_flags, 0, 0, loop_flags, ret); }