diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index 35b762ca3f..e7a87c5839 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -161,7 +161,7 @@ L /tmp/foobar - - - - /dev/null Type The type consists of a single letter and optionally an exclamation mark (!) - and/or minus sign (-). + minus sign (-), and/or equals sign (=). The following line types are understood: @@ -482,6 +482,11 @@ r! /tmp/.X[0-9]*-lock # Modify sysfs but don't fail if we are in a container with a read-only /proc w- /proc/sys/vm/swappiness - - - - 10 + If the equals sign (=) is used, the file types of existing objects in the specified path + are checked, and removed if they do not match. This includes any implicitly created parent directories (which can + be either directories or directory symlinks). For example, if there is a FIFO in place of one of the parent path + components it will be replaced with a directory. + Note that for all line types that result in creation of any kind of file node (i.e. f/F, d/D/v/q/Q, diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index f2999ba077..5fe8fbab98 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -733,7 +733,7 @@ int inotify_add_watch_and_warn(int fd, const char *pathname, uint32_t mask) { return wd; } -static bool unsafe_transition(const struct stat *a, const struct stat *b) { +bool unsafe_transition(const struct stat *a, const struct stat *b) { /* Returns true if the transition from a to b is safe, i.e. that we never transition from unprivileged to * privileged files or directories. Why bother? So that unprivileged code can't symlink to privileged files * making us believe we read something safe even though it isn't safe in the specific context we open it in. */ diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index 7bac25704f..85bdea64df 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -94,6 +94,8 @@ enum { CHASE_WARN = 1 << 7, /* Emit an appropriate warning when an error is encountered */ }; +bool unsafe_transition(const struct stat *a, const struct stat *b); + /* How many iterations to execute before returning -ELOOP */ #define CHASE_SYMLINKS_MAX 32 diff --git a/src/basic/rm-rf.c b/src/basic/rm-rf.c index 2f2ebc39e9..900a7fb5ff 100644 --- a/src/basic/rm-rf.c +++ b/src/basic/rm-rf.c @@ -48,11 +48,7 @@ static int patch_dirfd_mode( return 0; } -static int unlinkat_harder( - int dfd, - const char *filename, - int unlink_flags, - RemoveFlags remove_flags) { +int unlinkat_harder(int dfd, const char *filename, int unlink_flags, RemoveFlags remove_flags) { mode_t old_mode; int r; @@ -77,13 +73,15 @@ static int unlinkat_harder( return r; } - /* If this worked, we won't reset the old mode, since we'll need it for other entries too, and we - * should destroy the whole thing */ + if (FLAGS_SET(remove_flags, REMOVE_CHMOD_RESTORE) && fchmod(dfd, old_mode) < 0) + return -errno; + + /* If this worked, we won't reset the old mode by default, since we'll need it for other entries too, + * and we should destroy the whole thing */ return 0; } -static int fstatat_harder( - int dfd, +int fstatat_harder(int dfd, const char *filename, struct stat *ret, int fstatat_flags, @@ -109,6 +107,9 @@ static int fstatat_harder( return r; } + if (FLAGS_SET(remove_flags, REMOVE_CHMOD_RESTORE) && fchmod(dfd, old_mode) < 0) + return -errno; + return 0; } diff --git a/src/basic/rm-rf.h b/src/basic/rm-rf.h index 87be9b3410..40f0894c96 100644 --- a/src/basic/rm-rf.h +++ b/src/basic/rm-rf.h @@ -12,9 +12,17 @@ typedef enum RemoveFlags { REMOVE_PHYSICAL = 1 << 2, /* If not set, only removes files on tmpfs, never physical file systems */ REMOVE_SUBVOLUME = 1 << 3, /* Drop btrfs subvolumes in the tree too */ REMOVE_MISSING_OK = 1 << 4, /* If the top-level directory is missing, ignore the ENOENT for it */ - REMOVE_CHMOD = 1 << 5, /* chmod() for write access if we cannot delete something */ + REMOVE_CHMOD = 1 << 5, /* chmod() for write access if we cannot delete or access something */ + REMOVE_CHMOD_RESTORE = 1 << 6, /* Restore the old mode before returning */ } RemoveFlags; +int unlinkat_harder(int dfd, const char *filename, int unlink_flags, RemoveFlags remove_flags); +int fstatat_harder(int dfd, + const char *filename, + struct stat *ret, + int fstatat_flags, + RemoveFlags remove_flags); + int rm_rf_children(int fd, RemoveFlags flags, struct stat *root_dev); int rm_rf(const char *path, RemoveFlags flags); diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 45cd549029..8f4ceee037 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -142,6 +142,8 @@ typedef struct Item { bool allow_failure:1; + bool try_replace:1; + OperationMask done; } Item; @@ -2002,6 +2004,180 @@ static int glob_item_recursively(Item *i, fdaction_t action) { return r; } +static int rm_if_wrong_type_safe( + mode_t mode, + int parent_fd, + const struct stat *parent_st /* Only used if follow is true. */, + const char *name, + int flags) { + _cleanup_free_ char *parent_name = NULL; + bool follow_links = !FLAGS_SET(flags, AT_SYMLINK_NOFOLLOW); + struct stat st; + int r; + + assert(name); + assert((mode & ~S_IFMT) == 0); + assert(!follow_links || parent_st); + assert((flags & ~AT_SYMLINK_NOFOLLOW) == 0); + + if (!filename_is_valid(name)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "\"%s\" is not a valid filename.", name); + + r = fstatat_harder(parent_fd, name, &st, flags, REMOVE_CHMOD | REMOVE_CHMOD_RESTORE); + if (r < 0) { + (void) fd_get_path(parent_fd, &parent_name); + return log_full_errno(r == -ENOENT? LOG_DEBUG : LOG_ERR, r, + "Failed to stat \"%s\" at \"%s\": %m", name, strna(parent_name)); + } + + /* Fail before removing anything if this is an unsafe transition. */ + if (follow_links && unsafe_transition(parent_st, &st)) { + (void) fd_get_path(parent_fd, &parent_name); + return log_error_errno(SYNTHETIC_ERRNO(ENOLINK), + "Unsafe transition from \"%s\" to \"%s\".", parent_name, name); + } + + if ((st.st_mode & S_IFMT) == mode) + return 0; + + (void) fd_get_path(parent_fd, &parent_name); + log_notice("Wrong file type 0x%x; rm -rf \"%s/%s\"", st.st_mode & S_IFMT, strna(parent_name), name); + + /* If the target of the symlink was the wrong type, the link needs to be removed instead of the + * target, so make sure it is identified as a link and not a directory. */ + if (follow_links) { + r = fstatat_harder(parent_fd, name, &st, AT_SYMLINK_NOFOLLOW, REMOVE_CHMOD | REMOVE_CHMOD_RESTORE); + if (r < 0) + return log_error_errno(r, "Failed to stat \"%s\" at \"%s\": %m", name, strna(parent_name)); + } + + /* Do not remove mount points. */ + r = fd_is_mount_point(parent_fd, name, follow_links ? AT_SYMLINK_FOLLOW : 0); + if (r < 0) + (void) log_warning_errno(r, "Failed to check if \"%s/%s\" is a mount point: %m; Continuing", + strna(parent_name), name); + else if (r > 0) + return log_error_errno(SYNTHETIC_ERRNO(EBUSY), + "Not removing \"%s/%s\" because it is a mount point.", strna(parent_name), name); + + if ((st.st_mode & S_IFMT) == S_IFDIR) { + _cleanup_close_ int child_fd = -1; + + child_fd = openat(parent_fd, name, O_NOCTTY | O_CLOEXEC | O_DIRECTORY); + if (child_fd < 0) + return log_error_errno(errno, "Failed to open \"%s\" at \"%s\": %m", name, strna(parent_name)); + + r = rm_rf_children(TAKE_FD(child_fd), REMOVE_ROOT|REMOVE_SUBVOLUME|REMOVE_PHYSICAL, &st); + if (r < 0) + return log_error_errno(r, "Failed to remove contents of \"%s\" at \"%s\": %m", name, strna(parent_name)); + + r = unlinkat_harder(parent_fd, name, AT_REMOVEDIR, REMOVE_CHMOD | REMOVE_CHMOD_RESTORE); + } else + r = unlinkat_harder(parent_fd, name, 0, REMOVE_CHMOD | REMOVE_CHMOD_RESTORE); + if (r < 0) + return log_error_errno(r, "Failed to remove \"%s\" at \"%s\": %m", name, strna(parent_name)); + + /* This is covered by the log_notice "Wrong file type..." It is logged earlier because it gives + * context to other error messages that might follow. */ + return -ENOENT; +} + +/* If child_mode is non-zero, rm_if_wrong_type_safe will be executed for the last path component. */ +static int mkdir_parents_rm_if_wrong_type(mode_t child_mode, const char *path) { + _cleanup_close_ int parent_fd = -1, next_fd = -1; + _cleanup_free_ char *parent_name = NULL; + struct stat parent_st; + const char *s, *e; + int r; + size_t path_len; + + assert(path); + assert((child_mode & ~S_IFMT) == 0); + + path_len = strlen(path); + + if (!is_path(path)) + /* rm_if_wrong_type_safe already logs errors. */ + return child_mode != 0 ? rm_if_wrong_type_safe(child_mode, AT_FDCWD, NULL, path, AT_SYMLINK_NOFOLLOW) : 0; + + if (child_mode != 0 && endswith(path, "/")) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Trailing path separators are only allowed if child_mode is not set; got \"%s\"", path); + + /* Get the parent_fd and stat. */ + parent_fd = AT_FDCWD; + if (path_is_absolute(path)) { + parent_fd = open("/", O_NOCTTY | O_CLOEXEC | O_DIRECTORY); + if (parent_fd < 0) + return log_error_errno(errno, "Failed to open root: %m"); + } + if (fstat(parent_fd, &parent_st) < 0) + return log_error_errno(errno, "Failed to stat root: %m"); + + /* Check every parent directory in the path, except the last component */ + e = path; + for (;;) { + char t[path_len + 1]; + + /* Find the start of the next path component. */ + s = e + strspn(e, "/"); + /* Find the end of the next path component. */ + e = s + strcspn(s, "/"); + + /* Copy the path component to t so it can be a null terminated string. */ + *((char*) mempcpy(t, s, e - s)) = 0; + + /* Is this the last component? If so, then check the type */ + if (*e == 0) + return child_mode != 0 ? rm_if_wrong_type_safe(child_mode, parent_fd, &parent_st, t, AT_SYMLINK_NOFOLLOW) : 0; + else { + r = rm_if_wrong_type_safe(S_IFDIR, parent_fd, &parent_st, t, 0); + /* Remove dangling symlinks. */ + if (r == -ENOENT) + r = rm_if_wrong_type_safe(S_IFDIR, parent_fd, &parent_st, t, AT_SYMLINK_NOFOLLOW); + } + + if (r == -ENOENT) { + RUN_WITH_UMASK(0000) + r = mkdirat_label(parent_fd, t, 0755); + if (r < 0) { + (void) fd_get_path(parent_fd, &parent_name); + return log_error_errno(r, "Failed to mkdir \"%s\" at \"%s\": %m", t, parent_name); + } + } else if (r < 0) + /* rm_if_wrong_type_safe already logs errors. */ + return r; + + next_fd = openat(parent_fd, t, O_NOCTTY | O_CLOEXEC | O_DIRECTORY); + if (next_fd < 0) { + r = -errno; + (void) fd_get_path(parent_fd, &parent_name); + return log_error_errno(r, "Failed to open \"%s\" at \"%s\": %m", t, parent_name); + } + if (fstat(next_fd, &parent_st) < 0) { + r = -errno; + (void) fd_get_path(parent_fd, &parent_name); + return log_error_errno(r, "Failed to stat \"%s\" at \"%s\": %m", t, parent_name); + } + + safe_close(parent_fd); + parent_fd = TAKE_FD(next_fd); + } +} + +static int mkdir_parents_item(Item *i, mode_t child_mode) { + int r; + if (i->try_replace) { + r = mkdir_parents_rm_if_wrong_type(child_mode, i->path); + if (r < 0 && r != -ENOENT) + return r; + } else + RUN_WITH_UMASK(0000) + (void) mkdir_parents_label(i->path, 0755); + + return 0; +} + static int create_item(Item *i) { CreationMode creation; int r = 0; @@ -2020,8 +2196,9 @@ static int create_item(Item *i) { case TRUNCATE_FILE: case CREATE_FILE: - RUN_WITH_UMASK(0000) - (void) mkdir_parents_label(i->path, 0755); + r = mkdir_parents_item(i, S_IFREG); + if (r < 0) + return r; if ((i->type == CREATE_FILE && i->append_or_force) || i->type == TRUNCATE_FILE) r = truncate_file(i, i->path); @@ -2033,8 +2210,9 @@ static int create_item(Item *i) { break; case COPY_FILES: - RUN_WITH_UMASK(0000) - (void) mkdir_parents_label(i->path, 0755); + r = mkdir_parents_item(i, 0); + if (r < 0) + return r; r = copy_files(i); if (r < 0) @@ -2050,8 +2228,9 @@ static int create_item(Item *i) { case CREATE_DIRECTORY: case TRUNCATE_DIRECTORY: - RUN_WITH_UMASK(0000) - (void) mkdir_parents_label(i->path, 0755); + r = mkdir_parents_item(i, S_IFDIR); + if (r < 0) + return r; r = create_directory(i, i->path); if (r < 0) @@ -2061,8 +2240,9 @@ static int create_item(Item *i) { case CREATE_SUBVOLUME: case CREATE_SUBVOLUME_INHERIT_QUOTA: case CREATE_SUBVOLUME_NEW_QUOTA: - RUN_WITH_UMASK(0000) - (void) mkdir_parents_label(i->path, 0755); + r = mkdir_parents_item(i, S_IFDIR); + if (r < 0) + return r; r = create_subvolume(i, i->path); if (r < 0) @@ -2076,8 +2256,9 @@ static int create_item(Item *i) { break; case CREATE_FIFO: - RUN_WITH_UMASK(0000) - (void) mkdir_parents_label(i->path, 0755); + r = mkdir_parents_item(i, S_IFIFO); + if (r < 0) + return r; r = create_fifo(i, i->path); if (r < 0) @@ -2085,8 +2266,9 @@ static int create_item(Item *i) { break; case CREATE_SYMLINK: { - RUN_WITH_UMASK(0000) - (void) mkdir_parents_label(i->path, 0755); + r = mkdir_parents_item(i, S_IFLNK); + if (r < 0) + return r; mac_selinux_create_file_prepare(i->path, S_IFLNK); r = symlink(i->argument, i->path); @@ -2142,8 +2324,9 @@ static int create_item(Item *i) { return 0; } - RUN_WITH_UMASK(0000) - (void) mkdir_parents_label(i->path, 0755); + r = mkdir_parents_item(i, i->type == CREATE_BLOCK_DEVICE ? S_IFBLK : S_IFCHR); + if (r < 0) + return r; r = create_device(i, i->type == CREATE_BLOCK_DEVICE ? S_IFBLK : S_IFCHR); if (r < 0) @@ -2659,7 +2842,7 @@ static int parse_line( ItemArray *existing; OrderedHashmap *h; int r, pos; - bool append_or_force = false, boot = false, allow_failure = false; + bool append_or_force = false, boot = false, allow_failure = false, try_replace = false; assert(fname); assert(line >= 1); @@ -2704,6 +2887,8 @@ static int parse_line( append_or_force = true; else if (action[pos] == '-' && !allow_failure) allow_failure = true; + else if (action[pos] == '=' && !try_replace) + try_replace = true; else { *invalid_config = true; return log_syntax(NULL, LOG_ERR, fname, line, SYNTHETIC_ERRNO(EBADMSG), "Unknown modifiers in command '%s'", action); @@ -2718,6 +2903,7 @@ static int parse_line( i.type = action[0]; i.append_or_force = append_or_force; i.allow_failure = allow_failure; + i.try_replace = try_replace; r = specifier_printf(path, PATH_MAX-1, specifier_table, NULL, &i.path); if (r == -ENXIO) diff --git a/test/test-systemd-tmpfiles.py b/test/test-systemd-tmpfiles.py index 255922d05b..3376029463 100755 --- a/test/test-systemd-tmpfiles.py +++ b/test/test-systemd-tmpfiles.py @@ -48,6 +48,7 @@ def test_invalids(*, user): test_line('f++ /too/many/plusses', user=user) test_line('f+!+ /too/many/plusses', user=user) test_line('f!+! /too/many/bangs', user=user) + test_line('f== /too/many/equals', user=user) test_line('w /unresolved/argument - - - - "%Y"', user=user) test_line('w /unresolved/argument/sandwich - - - - "%v%Y%v"', user=user) test_line('w /unresolved/filename/%Y - - - - "whatever"', user=user) @@ -73,9 +74,11 @@ def test_uninitialized_t(): test_line('w /foo - - - - "specifier for --user %t"', user=True, returncode=0, extra={'env':{}}) -def test_content(line, expected, *, user, extra={}): +def test_content(line, expected, *, user, extra={}, subpath='/arg', path_cb=None): d = tempfile.TemporaryDirectory(prefix='test-systemd-tmpfiles.') - arg = d.name + '/arg' + if path_cb is not None: + path_cb(d.name, subpath) + arg = d.name + subpath spec = line.format(arg) test_line(spec, user=user, returncode=0, extra=extra) content = open(arg).read() @@ -134,6 +137,57 @@ def test_valid_specifiers(*, user): test_content('f {} - - - - %%', '%', user=user) +def mkfifo(parent, subpath): + os.makedirs(parent, mode=0o755, exist_ok=True) + first_component = subpath.split('/')[1] + path = parent + '/' + first_component + print('path: {}'.format(path)) + os.mkfifo(path) + +def mkdir(parent, subpath): + first_component = subpath.split('/')[1] + path = parent + '/' + first_component + os.makedirs(path, mode=0o755, exist_ok=True) + os.symlink(path, path + '/self', target_is_directory=True) + +def symlink(parent, subpath): + link_path = parent + '/link-target' + os.makedirs(parent, mode=0o755, exist_ok=True) + with open(link_path, 'wb') as f: + f.write(b'target') + first_component = subpath.split('/')[1] + path = parent + '/' + first_component + os.symlink(link_path, path, target_is_directory=True) + +def file(parent, subpath): + content = 'file-' + subpath.split('/')[1] + path = parent + subpath + os.makedirs(os.path.dirname(path), mode=0o755, exist_ok=True) + with open(path, 'wb') as f: + f.write(content.encode()) + +def valid_symlink(parent, subpath): + target = 'link-target' + link_path = parent + target + os.makedirs(link_path, mode=0o755, exist_ok=True) + first_component = subpath.split('/')[1] + path = parent + '/' + first_component + os.symlink(target, path, target_is_directory=True) + +def test_hard_cleanup(*, user): + type_cbs = [None, file, mkdir, symlink] + if 'mkfifo' in dir(os): + type_cbs.append(mkfifo) + + for type_cb in type_cbs: + for subpath in ['/shallow', '/deep/1/2']: + label = '{}-{}'.format('None' if type_cb is None else type_cb.__name__, subpath.split('/')[1]) + test_content('f= {} - - - - ' + label, label, user=user, subpath=subpath, path_cb=type_cb) + + # Test the case that a valid symlink is in the path. + label = 'valid_symlink-deep' + test_content('f= {} - - - - ' + label, label, user=user, subpath='/deep/1/2', path_cb=valid_symlink) + if __name__ == '__main__': test_invalids(user=False) test_invalids(user=True) @@ -141,3 +195,6 @@ if __name__ == '__main__': test_valid_specifiers(user=False) test_valid_specifiers(user=True) + + test_hard_cleanup(user=False) + test_hard_cleanup(user=True)