From 14f3480af1f5b5884827977fedecf0d7bb61ebab Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 24 May 2018 14:17:07 +0200 Subject: [PATCH 01/26] tmpfiles: stat file in item_do() rather than in its callers This a slight simplification since all callers of item_do() (glob_item_recursively() and item_do() itself) stat the file descriptor only for passing it to item_do(). --- src/tmpfiles/tmpfiles.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 52f4d90fba..31d4c923a2 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1295,18 +1295,23 @@ static int write_one_file(Item *i, const char *path) { typedef int (*action_t)(Item *, const char *); typedef int (*fdaction_t)(Item *, int fd, const struct stat *st); -static int item_do(Item *i, int fd, const struct stat *st, fdaction_t action) { +static int item_do(Item *i, int fd, fdaction_t action) { + struct stat st; int r = 0, q; assert(i); assert(fd >= 0); - assert(st); + + if (fstat(fd, &st) < 0) { + r = -errno; + goto finish; + } /* This returns the first error we run into, but nevertheless * tries to go on */ - r = action(i, fd, st); + r = action(i, fd, &st); - if (S_ISDIR(st->st_mode)) { + if (S_ISDIR(st.st_mode)) { char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)]; _cleanup_closedir_ DIR *d = NULL; struct dirent *de; @@ -1322,16 +1327,15 @@ static int item_do(Item *i, int fd, const struct stat *st, fdaction_t action) { } FOREACH_DIRENT_ALL(de, d, q = -errno; goto finish) { - struct stat de_st; int de_fd; if (dot_or_dot_dot(de->d_name)) continue; de_fd = openat(fd, de->d_name, O_NOFOLLOW|O_CLOEXEC|O_PATH); - if (de_fd >= 0 && fstat(de_fd, &de_st) >= 0) + if (de_fd >= 0) /* pass ownership of dirent fd over */ - q = item_do(i, de_fd, &de_st, action); + q = item_do(i, de_fd, action); else q = -errno; @@ -1377,7 +1381,6 @@ static int glob_item_recursively(Item *i, fdaction_t action) { STRV_FOREACH(fn, g.gl_pathv) { _cleanup_close_ int fd = -1; - struct stat st; /* Make sure we won't trigger/follow file object (such as * device nodes, automounts, ...) pointed out by 'fn' with @@ -1390,12 +1393,7 @@ static int glob_item_recursively(Item *i, fdaction_t action) { continue; } - if (fstat(fd, &st) < 0) { - r = r ?: -errno; - continue; - } - - k = item_do(i, fd, &st, action); + k = item_do(i, fd, action); if (k < 0 && r == 0) r = k; From b206ac8e54a3656b681b2f6031c7cd0feb7f5e26 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 13 Apr 2018 11:39:39 +0200 Subject: [PATCH 02/26] tmpfiles: make the stat struct parameter in fd_set_*() optional So every callers are not forced to stat() the passed file descriptor before calling those functions. --- src/tmpfiles/tmpfiles.c | 53 ++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 31d4c923a2..8b6342b2e4 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -772,6 +772,7 @@ static bool hardlink_vulnerable(const struct stat *st) { static int fd_set_perms(Item *i, int fd, const struct stat *st) { _cleanup_free_ char *path = NULL; + struct stat stbuf; int r; assert(i); @@ -784,6 +785,12 @@ static int fd_set_perms(Item *i, int fd, const struct stat *st) { if (!i->mode_set && !i->uid_set && !i->gid_set) goto shortcut; + if (!st) { + if (fstat(fd, &stbuf) < 0) + return log_error_errno(errno, "fstat(%s) failed: %m", path); + st = &stbuf; + } + if (hardlink_vulnerable(st)) { log_error("Refusing to set permissions on hardlinked file %s while the fs.protected_hardlinks sysctl is turned off.", path); return -EPERM; @@ -837,7 +844,7 @@ shortcut: static int path_set_perms(Item *i, const char *path) { _cleanup_close_ int fd = -1; - struct stat st; + struct stat stbuf, *st = NULL; assert(i); assert(path); @@ -857,13 +864,20 @@ static int path_set_perms(Item *i, const char *path) { return r; } - if (fstat(fd, &st) < 0) - return log_error_errno(errno, "Failed to fstat() file %s: %m", path); + if (i->type == EMPTY_DIRECTORY) { + /* FIXME: introduce fd_is_dir() helper ? */ + if (fstat(fd, &stbuf) < 0) + return log_error_errno(errno, "Failed to fstat() file %s: %m", path); - if (i->type == EMPTY_DIRECTORY && !S_ISDIR(st.st_mode)) - return log_error_errno(EEXIST, "'%s' already exists and is not a directory. ", path); + if (!S_ISDIR(stbuf.st_mode)) { + log_error("'%s' already exists and is not a directory. ", path); + return -EEXIST; + } - return fd_set_perms(i, fd, &st); + st = &stbuf; + } + + return fd_set_perms(i, fd, st); } static int parse_xattrs_from_arg(Item *i) { @@ -1011,15 +1025,21 @@ static int fd_set_acls(Item *item, int fd, const struct stat *st) { #if HAVE_ACL char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)]; _cleanup_free_ char *path = NULL; + struct stat stbuf; assert(item); assert(fd); - assert(st); r = fd_get_path(fd, &path); if (r < 0) return r; + if (!st) { + if (fstat(fd, &stbuf) < 0) + return log_error_errno(errno, "fstat(%s) failed: %m", path); + st = &stbuf; + } + if (hardlink_vulnerable(st)) { log_error("Refusing to set ACLs on hardlinked file %s while the fs.protected_hardlinks sysctl is turned off.", path); return -EPERM; @@ -1055,7 +1075,6 @@ static int path_set_acls(Item *item, const char *path) { int r = 0; #if HAVE_ACL _cleanup_close_ int fd = -1; - struct stat st; assert(item); assert(path); @@ -1064,10 +1083,7 @@ static int path_set_acls(Item *item, const char *path) { if (fd < 0) return log_error_errno(errno, "Adjusting ACL of %s failed: %m", path); - if (fstat(fd, &st) < 0) - return log_error_errno(errno, "Failed to fstat() file %s: %m", path); - - r = fd_set_acls(item, fd, &st); + r = fd_set_acls(item, fd, NULL); #endif return r; } @@ -1174,6 +1190,7 @@ static int parse_attribute_from_arg(Item *item) { static int fd_set_attribute(Item *item, int fd, const struct stat *st) { _cleanup_close_ int procfs_fd = -1; _cleanup_free_ char *path = NULL; + struct stat stbuf; unsigned f; int r; @@ -1184,6 +1201,12 @@ static int fd_set_attribute(Item *item, int fd, const struct stat *st) { if (r < 0) return r; + if (!st) { + if (fstat(fd, &stbuf) < 0) + return log_error_errno(errno, "fstat(%s) failed: %m", path); + st = &stbuf; + } + /* Issuing the file attribute ioctls on device nodes is not * safe, as that will be delivered to the drivers, not the * file system containing the device node. */ @@ -1214,7 +1237,6 @@ static int fd_set_attribute(Item *item, int fd, const struct stat *st) { static int path_set_attribute(Item *item, const char *path) { _cleanup_close_ int fd = -1; - struct stat st; if (!item->attribute_set || item->attribute_mask == 0) return 0; @@ -1223,10 +1245,7 @@ static int path_set_attribute(Item *item, const char *path) { if (fd < 0) return log_error_errno(errno, "Cannot open '%s': %m", path); - if (fstat(fd, &st) < 0) - return log_error_errno(errno, "Cannot stat '%s': %m", path); - - return fd_set_attribute(item, fd, &st); + return fd_set_attribute(item, fd, NULL); } static int write_one_file(Item *i, const char *path) { From 31c84ff11ea22b0bc1b2459db09e4d2aa66bdf2a Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 13 Apr 2018 09:19:24 +0200 Subject: [PATCH 03/26] tmpfiles: introduce create_file() which deals with 'f'/'F' exclusively write_one_file() only deals with the 'w' command and 'f'/'F' are now handled by a new function create_file(). This is primarly done because 'w' is allowed to operate on any kind of files, not just regular ones. --- src/tmpfiles/tmpfiles.c | 42 ++++++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 8b6342b2e4..62a3683396 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1249,15 +1249,47 @@ static int path_set_attribute(Item *item, const char *path) { } static int write_one_file(Item *i, const char *path) { + _cleanup_close_ int fd = -1; + int r; + + assert(i); + assert(path); + assert(i->argument); + assert(i->type == WRITE_FILE); + + /* Follows symlinks */ + fd = open(path, O_NONBLOCK|O_CLOEXEC|O_WRONLY|O_NOCTTY, i->mode); + if (fd < 0) { + if (errno == ENOENT) { + log_debug_errno(errno, "Not writing missing file \"%s\": %m", path); + return 0; + } + return log_error_errno(errno, "Failed to open file \"%s\": %m", path); + } + + /* 'w' is allowed to write into any kind of files. */ + log_debug("Writing to \"%s\".", path); + + r = loop_write(fd, i->argument, strlen(i->argument), false); + if (r < 0) + return log_error_errno(r, "Failed to write file \"%s\": %m", path); + + return path_set_perms(i, path); +} + +static int create_file(Item *i, const char *path) { _cleanup_close_ int fd = -1; int flags, r = 0; struct stat st; assert(i); assert(path); + assert(IN_SET(i->type, CREATE_FILE, TRUNCATE_FILE)); - flags = i->type == CREATE_FILE ? O_CREAT|O_EXCL|O_NOFOLLOW : - i->type == TRUNCATE_FILE ? O_CREAT|O_TRUNC|O_NOFOLLOW : 0; + /* FIXME: O_TRUNC is unspecified if file is neither a regular file nor a + * fifo nor a terminal device. Therefore we should fail if file is + * anything but a regular file with 'F'. */ + flags = O_CREAT|O_NOFOLLOW|(i->type == CREATE_FILE ? O_EXCL : O_TRUNC); RUN_WITH_UMASK(0000) { mac_selinux_create_file_prepare(path, S_IFREG); @@ -1266,10 +1298,6 @@ static int write_one_file(Item *i, const char *path) { } if (fd < 0) { - if (i->type == WRITE_FILE && errno == ENOENT) { - log_debug_errno(errno, "Not writing missing file \"%s\": %m", path); - return 0; - } if (i->type == CREATE_FILE && errno == EEXIST) { log_debug_errno(errno, "Not writing to pre-existing file \"%s\": %m", path); goto done; @@ -1462,7 +1490,7 @@ static int create_item(Item *i) { RUN_WITH_UMASK(0000) (void) mkdir_parents_label(i->path, 0755); - r = write_one_file(i, i->path); + r = create_file(i, i->path); if (r < 0) return r; break; From 5ec9d065128cfc6ffde28abd3938f5009e36e41a Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 13 Apr 2018 17:31:22 +0200 Subject: [PATCH 04/26] tmpfiles: introduce truncate_file() which deals with 'F' exclusively TRUNCATE_FILE is now handled by a new dedicated function truncate_file(). Indeed we have to take special care when truncating existing file since the behavior is only specified for regular files. Well that's not entirely true for fifo and terminal devices since O_TRUNC is ignored in this case but even in for these types of file, truncating is probably not the right thing to do. It is worth noting that both truncate_file() and create_file() have been modified so they use fstat(2) instead of stat(2) since both functions are not supposed to follow symlinks. --- src/tmpfiles/tmpfiles.c | 184 ++++++++++++++++++++++++++++++++-------- 1 file changed, 147 insertions(+), 37 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 62a3683396..dd19cde13a 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -842,6 +842,28 @@ shortcut: return label_fix(path, 0); } +static int path_open_parent_safe(const char *path) { + _cleanup_free_ char *dn = NULL; + int fd; + + if (path_equal(path, "/") || !path_is_normalized(path)) { + log_error("Failed to open parent of '%s': invalid path.", path); + return -EINVAL; + } + + dn = dirname_malloc(path); + if (!dn) + return log_oom(); + + fd = chase_symlinks(dn, NULL, CHASE_OPEN|CHASE_SAFE, NULL); + if (fd == -EPERM) + return log_error_errno(fd, "Unsafe symlinks encountered in %s, refusing.", path); + if (fd < 0) + return log_error_errno(fd, "Failed to validate path %s: %m", path); + + return fd; +} + static int path_set_perms(Item *i, const char *path) { _cleanup_close_ int fd = -1; struct stat stbuf, *st = NULL; @@ -1278,65 +1300,145 @@ static int write_one_file(Item *i, const char *path) { } static int create_file(Item *i, const char *path) { - _cleanup_close_ int fd = -1; - int flags, r = 0; - struct stat st; + _cleanup_close_ int fd = -1, dir_fd = -1; + struct stat stbuf, *st = NULL; + int r = 0; + char *bn; assert(i); assert(path); - assert(IN_SET(i->type, CREATE_FILE, TRUNCATE_FILE)); + assert(i->type == CREATE_FILE); - /* FIXME: O_TRUNC is unspecified if file is neither a regular file nor a - * fifo nor a terminal device. Therefore we should fail if file is - * anything but a regular file with 'F'. */ - flags = O_CREAT|O_NOFOLLOW|(i->type == CREATE_FILE ? O_EXCL : O_TRUNC); + /* 'f' operates on regular files exclusively. */ + + /* Validate the path and keep the fd on the directory for opening the + * file so we're sure that it can't be changed behind our back. */ + dir_fd = path_open_parent_safe(path); + if (dir_fd < 0) + return dir_fd; + + bn = basename(path); RUN_WITH_UMASK(0000) { mac_selinux_create_file_prepare(path, S_IFREG); - fd = open(path, flags|O_NONBLOCK|O_CLOEXEC|O_WRONLY|O_NOCTTY, i->mode); + fd = openat(dir_fd, bn, O_CREAT|O_EXCL|O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC|O_WRONLY|O_NOCTTY, i->mode); mac_selinux_create_file_clear(); } if (fd < 0) { - if (i->type == CREATE_FILE && errno == EEXIST) { - log_debug_errno(errno, "Not writing to pre-existing file \"%s\": %m", path); - goto done; + /* Even on a read-only filesystem, open(2) returns EEXIST if the + * file already exists. It returns EROFS only if it needs to + * create the file. */ + if (errno != EEXIST) + return log_error_errno(errno, "Failed to create file %s: %m", path); + + /* Re-open the file. At that point it must exist since open(2) + * failed with EEXIST. We still need to check if the perms/mode + * need to be changed. For read-only filesystems, we let + * fd_set_perms() report the error if the perms need to be + * modified. */ + fd = openat(dir_fd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH, i->mode); + if (fd < 0) + return log_error_errno(errno, "Failed to re-open file %s: %m", path); + + if (fstat(fd, &stbuf) < 0) + return log_error_errno(errno, "stat(%s) failed: %m", path); + + if (!S_ISREG(stbuf.st_mode)) { + log_error("%s exists and is not a regular file.", path); + return -EEXIST; } - r = -errno; - if (!i->argument && errno == EROFS && stat(path, &st) == 0 && - (i->type == CREATE_FILE || st.st_size == 0)) - goto check_mode; + st = &stbuf; + } else { - return log_error_errno(r, "Failed to create file %s: %m", path); - } - - if (i->argument) { - log_debug("%s to \"%s\".", i->type == CREATE_FILE ? "Appending" : "Writing", path); - - r = loop_write(fd, i->argument, strlen(i->argument), false); - if (r < 0) - return log_error_errno(r, "Failed to write file \"%s\": %m", path); - } else log_debug("\"%s\" has been created.", path); - fd = safe_close(fd); + if (i->argument) { + log_debug("Writing to \"%s\".", path); - done: - if (stat(path, &st) < 0) + r = loop_write(fd, i->argument, strlen(i->argument), false); + if (r < 0) + return log_error_errno(r, "Failed to write file \"%s\": %m", path); + } + } + + return fd_set_perms(i, fd, st); +} + +static int truncate_file(Item *i, const char *path) { + _cleanup_close_ int fd = -1; + struct stat stbuf, *st = NULL; + bool erofs = false; + int r = 0; + + assert(i); + assert(path); + assert(i->type == TRUNCATE_FILE); + + /* We want to operate on regular file exclusively especially since + * O_TRUNC is unspecified if the file is neither a regular file nor a + * fifo nor a terminal device. Therefore we first open the file and make + * sure it's a regular one before truncating it. */ + + RUN_WITH_UMASK(0000) { + mac_selinux_create_file_prepare(path, S_IFREG); + fd = open(path, O_CREAT|O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC|O_WRONLY|O_NOCTTY, i->mode); + mac_selinux_create_file_clear(); + } + + if (fd < 0) { + if (errno != EROFS) + return log_error_errno(errno, "Failed to open/create file %s: %m", path); + + /* On a read-only filesystem, we don't want to fail if the + * target is already empty and the perms are set. So we still + * proceed with the sanity checks and let the remaining + * operations fail with EROFS if they try to modify the target + * file. */ + + fd = open(path, O_NOFOLLOW|O_CLOEXEC|O_PATH, i->mode); + if (fd < 0) { + if (errno == ENOENT) { + log_error("Cannot create file %s on a read-only file system.", path); + return -EROFS; + } + + return log_error_errno(errno, "Failed to re-open file %s: %m", path); + } + + erofs = true; + } + + if (fstat(fd, &stbuf) < 0) return log_error_errno(errno, "stat(%s) failed: %m", path); - check_mode: - if (!S_ISREG(st.st_mode)) { - log_error("%s is not a file.", path); + if (!S_ISREG(stbuf.st_mode)) { + log_error("%s exists and is not a regular file.", path); return -EEXIST; } - r = path_set_perms(i, path); - if (r < 0) - return r; + if (stbuf.st_size > 0) { + if (ftruncate(fd, 0) < 0) { + r = erofs ? -EROFS : -errno; + return log_error_errno(r, "Failed to truncate file %s: %m", path); + } + } else + st = &stbuf; - return 0; + log_debug("\"%s\" has been created.", path); + + if (i->argument) { + log_debug("Writing to \"%s\".", path); + + r = loop_write(fd, i->argument, strlen(i->argument), false); + if (r < 0) { + r = erofs ? -EROFS : r; + return log_error_errno(r, "Failed to write file %s: %m", path); + } + } + + return fd_set_perms(i, fd, st); } typedef int (*action_t)(Item *, const char *); @@ -1486,7 +1588,6 @@ static int create_item(Item *i) { return 0; case CREATE_FILE: - case TRUNCATE_FILE: RUN_WITH_UMASK(0000) (void) mkdir_parents_label(i->path, 0755); @@ -1495,6 +1596,15 @@ static int create_item(Item *i) { return r; break; + case TRUNCATE_FILE: + RUN_WITH_UMASK(0000) + (void) mkdir_parents_label(i->path, 0755); + + r = truncate_file(i, i->path); + if (r < 0) + return r; + break; + case COPY_FILES: RUN_WITH_UMASK(0000) (void) mkdir_parents_label(i->path, 0755); From 14ab804e14edfa224430ed2c8182864034c9075d Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 13 Apr 2018 17:50:49 +0200 Subject: [PATCH 05/26] tmpfiles: make truncate_file() safe --- src/tmpfiles/tmpfiles.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index dd19cde13a..28e6550243 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1367,10 +1367,11 @@ static int create_file(Item *i, const char *path) { } static int truncate_file(Item *i, const char *path) { - _cleanup_close_ int fd = -1; + _cleanup_close_ int fd = -1, dir_fd = -1; struct stat stbuf, *st = NULL; bool erofs = false; int r = 0; + char *bn; assert(i); assert(path); @@ -1381,9 +1382,17 @@ static int truncate_file(Item *i, const char *path) { * fifo nor a terminal device. Therefore we first open the file and make * sure it's a regular one before truncating it. */ + /* Validate the path and keep the fd on the directory for opening the + * file so we're sure that it can't be changed behind our back. */ + dir_fd = path_open_parent_safe(path); + if (dir_fd < 0) + return dir_fd; + + bn = basename(path); + RUN_WITH_UMASK(0000) { mac_selinux_create_file_prepare(path, S_IFREG); - fd = open(path, O_CREAT|O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC|O_WRONLY|O_NOCTTY, i->mode); + fd = openat(dir_fd, bn, O_CREAT|O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC|O_WRONLY|O_NOCTTY, i->mode); mac_selinux_create_file_clear(); } @@ -1397,7 +1406,7 @@ static int truncate_file(Item *i, const char *path) { * operations fail with EROFS if they try to modify the target * file. */ - fd = open(path, O_NOFOLLOW|O_CLOEXEC|O_PATH, i->mode); + fd = openat(dir_fd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH, i->mode); if (fd < 0) { if (errno == ENOENT) { log_error("Cannot create file %s on a read-only file system.", path); From 551470ecf2bb56d28e30440ca93e07ee5427e3b8 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 13 Apr 2018 17:54:09 +0200 Subject: [PATCH 06/26] tmpfiles: make write_one_file() safe --- src/tmpfiles/tmpfiles.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 28e6550243..36784a3ad9 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1271,7 +1271,8 @@ static int path_set_attribute(Item *item, const char *path) { } static int write_one_file(Item *i, const char *path) { - _cleanup_close_ int fd = -1; + _cleanup_close_ int fd = -1, dir_fd = -1; + char *bn; int r; assert(i); @@ -1279,8 +1280,16 @@ static int write_one_file(Item *i, const char *path) { assert(i->argument); assert(i->type == WRITE_FILE); + /* Validate the path and keep the fd on the directory for opening the + * file so we're sure that it can't be changed behind our back. */ + dir_fd = path_open_parent_safe(path); + if (dir_fd < 0) + return dir_fd; + + bn = basename(path); + /* Follows symlinks */ - fd = open(path, O_NONBLOCK|O_CLOEXEC|O_WRONLY|O_NOCTTY, i->mode); + fd = openat(dir_fd, bn, O_NONBLOCK|O_CLOEXEC|O_WRONLY|O_NOCTTY, i->mode); if (fd < 0) { if (errno == ENOENT) { log_debug_errno(errno, "Not writing missing file \"%s\": %m", path); @@ -1296,7 +1305,7 @@ static int write_one_file(Item *i, const char *path) { if (r < 0) return log_error_errno(r, "Failed to write file \"%s\": %m", path); - return path_set_perms(i, path); + return fd_set_perms(i, fd, NULL); } static int create_file(Item *i, const char *path) { From b1f7b17f9a6374ac621c80da5e0678c0e867fe18 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 12 Apr 2018 12:19:22 +0200 Subject: [PATCH 07/26] tmpfiles: introduce copy_files() routine No functional changes. --- src/tmpfiles/tmpfiles.c | 65 +++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 36784a3ad9..06ae67b403 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1459,6 +1459,41 @@ static int truncate_file(Item *i, const char *path) { return fd_set_perms(i, fd, st); } +static int copy_files(Item *i) { + struct stat st; + int r; + + log_debug("Copying tree \"%s\" to \"%s\".", i->argument, i->path); + + r = copy_tree(i->argument, i->path, + i->uid_set ? i->uid : UID_INVALID, + i->gid_set ? i->gid : GID_INVALID, + COPY_REFLINK); + + if (r == -EROFS && stat(i->path, &st) == 0) + r = -EEXIST; + + if (r < 0) { + struct stat a, b; + + if (r != -EEXIST) + return log_error_errno(r, "Failed to copy files to %s: %m", i->path); + + if (stat(i->argument, &a) < 0) + return log_error_errno(errno, "stat(%s) failed: %m", i->argument); + + if (stat(i->path, &b) < 0) + return log_error_errno(errno, "stat(%s) failed: %m", i->path); + + if ((a.st_mode ^ b.st_mode) & S_IFMT) { + log_debug("Can't copy to %s, file exists already and is of different type", i->path); + return 0; + } + } + + return path_set_perms(i, i->path); +} + typedef int (*action_t)(Item *, const char *); typedef int (*fdaction_t)(Item *, int fd, const struct stat *st); @@ -1627,37 +1662,9 @@ static int create_item(Item *i) { RUN_WITH_UMASK(0000) (void) mkdir_parents_label(i->path, 0755); - log_debug("Copying tree \"%s\" to \"%s\".", i->argument, i->path); - r = copy_tree(i->argument, i->path, - i->uid_set ? i->uid : UID_INVALID, - i->gid_set ? i->gid : GID_INVALID, - COPY_REFLINK); - - if (r == -EROFS && stat(i->path, &st) == 0) - r = -EEXIST; - - if (r < 0) { - struct stat a, b; - - if (r != -EEXIST) - return log_error_errno(r, "Failed to copy files to %s: %m", i->path); - - if (stat(i->argument, &a) < 0) - return log_error_errno(errno, "stat(%s) failed: %m", i->argument); - - if (stat(i->path, &b) < 0) - return log_error_errno(errno, "stat(%s) failed: %m", i->path); - - if ((a.st_mode ^ b.st_mode) & S_IFMT) { - log_debug("Can't copy to %s, file exists already and is of different type", i->path); - return 0; - } - } - - r = path_set_perms(i, i->path); + r = copy_files(i); if (r < 0) return r; - break; case WRITE_FILE: From 16ba55adb1d60577b321c781d81a410da3d28dcd Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 12 Apr 2018 12:55:22 +0200 Subject: [PATCH 08/26] tmpfiles: make copy_files() safe --- src/tmpfiles/tmpfiles.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 06ae67b403..a0e20c0696 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1460,29 +1460,42 @@ static int truncate_file(Item *i, const char *path) { } static int copy_files(Item *i) { - struct stat st; + _cleanup_close_ int dfd = -1, fd = -1; + char *bn; int r; log_debug("Copying tree \"%s\" to \"%s\".", i->argument, i->path); - r = copy_tree(i->argument, i->path, - i->uid_set ? i->uid : UID_INVALID, - i->gid_set ? i->gid : GID_INVALID, - COPY_REFLINK); + bn = basename(i->path); - if (r == -EROFS && stat(i->path, &st) == 0) - r = -EEXIST; + /* Validate the path and use the returned directory fd for copying the + * target so we're sure that the path can't be changed behind our + * back. */ + dfd = path_open_parent_safe(i->path); + if (dfd < 0) + return dfd; + r = copy_tree_at(AT_FDCWD, i->argument, + dfd, bn, + i->uid_set ? i->uid : UID_INVALID, + i->gid_set ? i->gid : GID_INVALID, + COPY_REFLINK); if (r < 0) { struct stat a, b; + /* If the target already exists on read-only filesystems, trying + * to create the target will not fail with EEXIST but with + * EROFS. */ + if (r == -EROFS && faccessat(dfd, bn, F_OK, AT_SYMLINK_NOFOLLOW) == 0) + r = -EEXIST; + if (r != -EEXIST) return log_error_errno(r, "Failed to copy files to %s: %m", i->path); if (stat(i->argument, &a) < 0) return log_error_errno(errno, "stat(%s) failed: %m", i->argument); - if (stat(i->path, &b) < 0) + if (fstatat(dfd, bn, &b, AT_SYMLINK_NOFOLLOW) < 0) return log_error_errno(errno, "stat(%s) failed: %m", i->path); if ((a.st_mode ^ b.st_mode) & S_IFMT) { @@ -1491,7 +1504,11 @@ static int copy_files(Item *i) { } } - return path_set_perms(i, i->path); + fd = openat(dfd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH); + if (fd < 0) + return log_error_errno(errno, "Failed to openat(%s): %m", i->path); + + return fd_set_perms(i, fd, NULL); } typedef int (*action_t)(Item *, const char *); From 074bd73fd36a6379d10d41c307223706b0ef300d Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 12 Apr 2018 18:10:57 +0200 Subject: [PATCH 09/26] tmpfiles: introduce create_device() --- src/tmpfiles/tmpfiles.c | 144 +++++++++++++++++++++------------------- 1 file changed, 75 insertions(+), 69 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index a0e20c0696..aaa783afea 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1511,6 +1511,79 @@ static int copy_files(Item *i) { return fd_set_perms(i, fd, NULL); } +typedef enum { + CREATION_NORMAL, + CREATION_EXISTING, + CREATION_FORCE, + _CREATION_MODE_MAX, + _CREATION_MODE_INVALID = -1 +} CreationMode; + +static const char *creation_mode_verb_table[_CREATION_MODE_MAX] = { + [CREATION_NORMAL] = "Created", + [CREATION_EXISTING] = "Found existing", + [CREATION_FORCE] = "Created replacement", +}; + +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(creation_mode_verb, CreationMode); + +static int create_device(Item *i, mode_t file_type) { + CreationMode creation; + struct stat st; + int r; + + assert(i); + assert(IN_SET(file_type, S_IFBLK, S_IFCHR)); + + RUN_WITH_UMASK(0000) { + mac_selinux_create_file_prepare(i->path, file_type); + r = mknod(i->path, i->mode | file_type, i->major_minor); + mac_selinux_create_file_clear(); + } + + if (r < 0) { + if (errno == EPERM) { + log_debug("We lack permissions, possibly because of cgroup configuration; " + "skipping creation of device node %s.", i->path); + return 0; + } + + if (errno != EEXIST) + return log_error_errno(errno, "Failed to create device node %s: %m", i->path); + + if (lstat(i->path, &st) < 0) + return log_error_errno(errno, "stat(%s) failed: %m", i->path); + + if ((st.st_mode & S_IFMT) != file_type) { + + if (i->force) { + + RUN_WITH_UMASK(0000) { + mac_selinux_create_file_prepare(i->path, file_type); + r = mknod_atomic(i->path, i->mode | file_type, i->major_minor); + mac_selinux_create_file_clear(); + } + + if (r < 0) + return log_error_errno(r, "Failed to create device node \"%s\": %m", i->path); + creation = CREATION_FORCE; + } else { + log_debug("%s is not a device node.", i->path); + return 0; + } + } else + creation = CREATION_EXISTING; + } else + creation = CREATION_NORMAL; + + log_debug("%s %s device node \"%s\" %u:%u.", + creation_mode_verb_to_string(creation), + i->type == CREATE_BLOCK_DEVICE ? "block" : "char", + i->path, major(i->mode), minor(i->mode)); + + return path_set_perms(i, i->path); +} + typedef int (*action_t)(Item *, const char *); typedef int (*fdaction_t)(Item *, int fd, const struct stat *st); @@ -1623,22 +1696,6 @@ static int glob_item_recursively(Item *i, fdaction_t action) { return r; } -typedef enum { - CREATION_NORMAL, - CREATION_EXISTING, - CREATION_FORCE, - _CREATION_MODE_MAX, - _CREATION_MODE_INVALID = -1 -} CreationMode; - -static const char *creation_mode_verb_table[_CREATION_MODE_MAX] = { - [CREATION_NORMAL] = "Created", - [CREATION_EXISTING] = "Found existing", - [CREATION_FORCE] = "Created replacement", -}; - -DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(creation_mode_verb, CreationMode); - static int create_item(Item *i) { struct stat st; int r = 0; @@ -1870,9 +1927,7 @@ static int create_item(Item *i) { } case CREATE_BLOCK_DEVICE: - case CREATE_CHAR_DEVICE: { - mode_t file_type; - + case CREATE_CHAR_DEVICE: if (have_effective_cap(CAP_MKNOD) == 0) { /* In a container we lack CAP_MKNOD. We shouldn't attempt to create the device node in @@ -1886,60 +1941,11 @@ static int create_item(Item *i) { RUN_WITH_UMASK(0000) (void) mkdir_parents_label(i->path, 0755); - file_type = i->type == CREATE_BLOCK_DEVICE ? S_IFBLK : S_IFCHR; - - RUN_WITH_UMASK(0000) { - mac_selinux_create_file_prepare(i->path, file_type); - r = mknod(i->path, i->mode | file_type, i->major_minor); - mac_selinux_create_file_clear(); - } - - if (r < 0) { - if (errno == EPERM) { - log_debug("We lack permissions, possibly because of cgroup configuration; " - "skipping creation of device node %s.", i->path); - return 0; - } - - if (errno != EEXIST) - return log_error_errno(errno, "Failed to create device node %s: %m", i->path); - - if (lstat(i->path, &st) < 0) - return log_error_errno(errno, "stat(%s) failed: %m", i->path); - - if ((st.st_mode & S_IFMT) != file_type) { - - if (i->force) { - - RUN_WITH_UMASK(0000) { - mac_selinux_create_file_prepare(i->path, file_type); - r = mknod_atomic(i->path, i->mode | file_type, i->major_minor); - mac_selinux_create_file_clear(); - } - - if (r < 0) - return log_error_errno(r, "Failed to create device node \"%s\": %m", i->path); - creation = CREATION_FORCE; - } else { - log_debug("%s is not a device node.", i->path); - return 0; - } - } else - creation = CREATION_EXISTING; - } else - creation = CREATION_NORMAL; - - log_debug("%s %s device node \"%s\" %u:%u.", - creation_mode_verb_to_string(creation), - i->type == CREATE_BLOCK_DEVICE ? "block" : "char", - i->path, major(i->mode), minor(i->mode)); - - r = path_set_perms(i, i->path); + r = create_device(i, i->type == CREATE_BLOCK_DEVICE ? S_IFBLK : S_IFCHR); if (r < 0) return r; break; - } case ADJUST_MODE: case RELABEL_PATH: From c7700a7748c2b96a048532530f1e793b654f507f Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 12 Apr 2018 18:33:54 +0200 Subject: [PATCH 10/26] tmpfiles: make create_device() safe --- src/tmpfiles/tmpfiles.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index aaa783afea..945158a16b 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1528,20 +1528,32 @@ static const char *creation_mode_verb_table[_CREATION_MODE_MAX] = { DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(creation_mode_verb, CreationMode); static int create_device(Item *i, mode_t file_type) { + _cleanup_close_ int dfd = -1, fd = -1; CreationMode creation; - struct stat st; + char *bn; int r; assert(i); assert(IN_SET(file_type, S_IFBLK, S_IFCHR)); + bn = basename(i->path); + + /* Validate the path and use the returned directory fd for copying the + * target so we're sure that the path can't be changed behind our + * back. */ + dfd = path_open_parent_safe(i->path); + if (dfd < 0) + return dfd; + RUN_WITH_UMASK(0000) { mac_selinux_create_file_prepare(i->path, file_type); - r = mknod(i->path, i->mode | file_type, i->major_minor); + r = mknodat(dfd, bn, i->mode | file_type, i->major_minor); mac_selinux_create_file_clear(); } if (r < 0) { + struct stat st; + if (errno == EPERM) { log_debug("We lack permissions, possibly because of cgroup configuration; " "skipping creation of device node %s.", i->path); @@ -1551,7 +1563,7 @@ static int create_device(Item *i, mode_t file_type) { if (errno != EEXIST) return log_error_errno(errno, "Failed to create device node %s: %m", i->path); - if (lstat(i->path, &st) < 0) + if (fstatat(dfd, bn, &st, 0) < 0) return log_error_errno(errno, "stat(%s) failed: %m", i->path); if ((st.st_mode & S_IFMT) != file_type) { @@ -1560,6 +1572,7 @@ static int create_device(Item *i, mode_t file_type) { RUN_WITH_UMASK(0000) { mac_selinux_create_file_prepare(i->path, file_type); + /* FIXME: need to introduce mknodat_atomic() */ r = mknod_atomic(i->path, i->mode | file_type, i->major_minor); mac_selinux_create_file_clear(); } @@ -1581,7 +1594,11 @@ static int create_device(Item *i, mode_t file_type) { i->type == CREATE_BLOCK_DEVICE ? "block" : "char", i->path, major(i->mode), minor(i->mode)); - return path_set_perms(i, i->path); + fd = openat(dfd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH); + if (fd < 0) + return log_error_errno(errno, "Failed to openat(%s): %m", i->path); + + return fd_set_perms(i, fd, NULL); } typedef int (*action_t)(Item *, const char *); From 4ad36844103474d8035743d55c334b88f32b808c Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 26 Apr 2018 14:54:20 +0200 Subject: [PATCH 11/26] tmpfiles: introduce create_directory_or_subvolume() No functional changes. --- src/tmpfiles/tmpfiles.c | 160 +++++++++++++++++++++------------------- 1 file changed, 86 insertions(+), 74 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 945158a16b..85a2ea8caa 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1527,6 +1527,86 @@ static const char *creation_mode_verb_table[_CREATION_MODE_MAX] = { DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(creation_mode_verb, CreationMode); +static int create_directory_or_subvolume(Item *i, const char *path) { + CreationMode creation; + int r, q = 0; + + assert(i); + assert(IN_SET(i->type, + CREATE_DIRECTORY, TRUNCATE_DIRECTORY, + CREATE_SUBVOLUME, CREATE_SUBVOLUME_NEW_QUOTA, CREATE_SUBVOLUME_INHERIT_QUOTA)); + + if (IN_SET(i->type, CREATE_SUBVOLUME, CREATE_SUBVOLUME_INHERIT_QUOTA, CREATE_SUBVOLUME_NEW_QUOTA)) { + + if (btrfs_is_subvol(empty_to_root(arg_root)) <= 0) + + /* Don't create a subvolume unless the root directory is + * one, too. We do this under the assumption that if the + * root directory is just a plain directory (i.e. very + * light-weight), we shouldn't try to split it up into + * subvolumes (i.e. more heavy-weight). Thus, chroot() + * environments and suchlike will get a full brtfs + * subvolume set up below their tree only if they + * specifically set up a btrfs subvolume for the root + * dir too. */ + + r = -ENOTTY; + else { + RUN_WITH_UMASK((~i->mode) & 0777) + r = btrfs_subvol_make(i->path); + } + } else + r = 0; + + if (IN_SET(i->type, CREATE_DIRECTORY, TRUNCATE_DIRECTORY) || r == -ENOTTY) + RUN_WITH_UMASK(0000) + r = mkdir_label(i->path, i->mode); + + if (r < 0) { + int k; + + if (!IN_SET(r, -EEXIST, -EROFS)) + return log_error_errno(r, "Failed to create directory or subvolume \"%s\": %m", i->path); + + k = is_dir(i->path, false); + if (k == -ENOENT && r == -EROFS) + return log_error_errno(r, "%s does not exist and cannot be created as the file system is read-only.", i->path); + if (k < 0) + return log_error_errno(k, "Failed to check if %s exists: %m", i->path); + if (!k) { + log_warning("\"%s\" already exists and is not a directory.", i->path); + return 0; + } + + creation = CREATION_EXISTING; + } else + creation = CREATION_NORMAL; + + log_debug("%s directory \"%s\".", creation_mode_verb_to_string(creation), i->path); + + if (IN_SET(i->type, CREATE_SUBVOLUME_NEW_QUOTA, CREATE_SUBVOLUME_INHERIT_QUOTA)) { + r = btrfs_subvol_auto_qgroup(i->path, 0, i->type == CREATE_SUBVOLUME_NEW_QUOTA); + if (r == -ENOTTY) + log_debug_errno(r, "Couldn't adjust quota for subvolume \"%s\" (unsupported fs or dir not a subvolume): %m", i->path); + else if (r == -EROFS) + log_debug_errno(r, "Couldn't adjust quota for subvolume \"%s\" (fs is read-only).", i->path); + else if (r == -ENOPROTOOPT) + log_debug_errno(r, "Couldn't adjust quota for subvolume \"%s\" (quota support is disabled).", i->path); + else if (r < 0) + q = log_error_errno(r, "Failed to adjust quota for subvolume \"%s\": %m", i->path); + else if (r > 0) + log_debug("Adjusted quota for subvolume \"%s\".", i->path); + else if (r == 0) + log_debug("Quota for subvolume \"%s\" already in place, no change made.", i->path); + } + + r = path_set_perms(i, i->path); + if (q < 0) + return q; + + return r; +} + static int create_device(Item *i, mode_t file_type) { _cleanup_close_ int dfd = -1, fd = -1; CreationMode creation; @@ -1716,7 +1796,6 @@ static int glob_item_recursively(Item *i, fdaction_t action) { static int create_item(Item *i) { struct stat st; int r = 0; - int q = 0; CreationMode creation; assert(i); @@ -1773,82 +1852,15 @@ static int create_item(Item *i) { RUN_WITH_UMASK(0000) (void) mkdir_parents_label(i->path, 0755); - if (IN_SET(i->type, CREATE_SUBVOLUME, CREATE_SUBVOLUME_INHERIT_QUOTA, CREATE_SUBVOLUME_NEW_QUOTA)) { - - if (btrfs_is_subvol(empty_to_root(arg_root)) <= 0) - - /* Don't create a subvolume unless the - * root directory is one, too. We do - * this under the assumption that if - * the root directory is just a plain - * directory (i.e. very light-weight), - * we shouldn't try to split it up - * into subvolumes (i.e. more - * heavy-weight). Thus, chroot() - * environments and suchlike will get - * a full brtfs subvolume set up below - * their tree only if they - * specifically set up a btrfs - * subvolume for the root dir too. */ - - r = -ENOTTY; - else { - RUN_WITH_UMASK((~i->mode) & 0777) - r = btrfs_subvol_make(i->path); - } - } else - r = 0; - - if (IN_SET(i->type, CREATE_DIRECTORY, TRUNCATE_DIRECTORY) || r == -ENOTTY) - RUN_WITH_UMASK(0000) - r = mkdir_label(i->path, i->mode); - - if (r < 0) { - int k; - - if (!IN_SET(r, -EEXIST, -EROFS)) - return log_error_errno(r, "Failed to create directory or subvolume \"%s\": %m", i->path); - - k = is_dir(i->path, false); - if (k == -ENOENT && r == -EROFS) - return log_error_errno(r, "%s does not exist and cannot be created as the file system is read-only.", i->path); - if (k < 0) - return log_error_errno(k, "Failed to check if %s exists: %m", i->path); - if (!k) { - log_warning("\"%s\" already exists and is not a directory.", i->path); - return 0; - } - - creation = CREATION_EXISTING; - } else - creation = CREATION_NORMAL; - - log_debug("%s directory \"%s\".", creation_mode_verb_to_string(creation), i->path); - - if (IN_SET(i->type, CREATE_SUBVOLUME_NEW_QUOTA, CREATE_SUBVOLUME_INHERIT_QUOTA)) { - r = btrfs_subvol_auto_qgroup(i->path, 0, i->type == CREATE_SUBVOLUME_NEW_QUOTA); - if (r == -ENOTTY) - log_debug_errno(r, "Couldn't adjust quota for subvolume \"%s\" (unsupported fs or dir not a subvolume): %m", i->path); - else if (r == -EROFS) - log_debug_errno(r, "Couldn't adjust quota for subvolume \"%s\" (fs is read-only).", i->path); - else if (r == -ENOPROTOOPT) - log_debug_errno(r, "Couldn't adjust quota for subvolume \"%s\" (quota support is disabled).", i->path); - else if (r < 0) - q = log_error_errno(r, "Failed to adjust quota for subvolume \"%s\": %m", i->path); - else if (r > 0) - log_debug("Adjusted quota for subvolume \"%s\".", i->path); - else if (r == 0) - log_debug("Quota for subvolume \"%s\" already in place, no change made.", i->path); - } - - _fallthrough_; - case EMPTY_DIRECTORY: - r = glob_item(i, path_set_perms); - if (q < 0) - return q; + r = create_directory_or_subvolume(i, i->path); if (r < 0) return r; + break; + case EMPTY_DIRECTORY: + r = glob_item(i, path_set_perms); + if (r < 0) + return r; break; case CREATE_FIFO: From 2c3d5adde0289fcbf9a99308c392bd8a3d2d91b4 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Sat, 30 Jun 2018 12:19:55 +0200 Subject: [PATCH 12/26] smack: introduce mac_smack_fix_at() --- src/basic/smack-util.c | 91 ++++++++++++++++++++++++++++-------------- src/basic/smack-util.h | 1 + 2 files changed, 62 insertions(+), 30 deletions(-) diff --git a/src/basic/smack-util.c b/src/basic/smack-util.c index 9d31b7717f..5d7be1f1d5 100644 --- a/src/basic/smack-util.c +++ b/src/basic/smack-util.c @@ -122,43 +122,20 @@ int mac_smack_apply_pid(pid_t pid, const char *label) { return r; } -int mac_smack_fix(const char *path, LabelFixFlags flags) { +static int smack_fix_fd(int fd , const char *abspath, LabelFixFlags flags) { char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)]; - _cleanup_close_ int fd = -1; const char *label; struct stat st; int r; - assert(path); + /* The caller should have done the sanity checks. */ + assert(abspath); + assert(path_is_absolute(abspath)); - if (!mac_smack_use()) + /* Path must be in /dev. */ + if (!path_startswith(abspath, "/dev")) return 0; - /* Path must be in /dev. Note that this check is pretty sloppy, as we might be called with non-normalized paths - * and hence not detect all cases of /dev. */ - - if (path_is_absolute(path)) { - if (!path_startswith(path, "/dev")) - return 0; - } else { - _cleanup_free_ char *cwd = NULL; - - r = safe_getcwd(&cwd); - if (r < 0) - return r; - - if (!path_startswith(cwd, "/dev")) - return 0; - } - - fd = open(path, O_NOFOLLOW|O_CLOEXEC|O_PATH); - if (fd < 0) { - if ((flags & LABEL_IGNORE_ENOENT) && errno == ENOENT) - return 0; - - return -errno; - } - if (fstat(fd, &st) < 0) return -errno; @@ -196,12 +173,62 @@ int mac_smack_fix(const char *path, LabelFixFlags flags) { streq(old_label, label)) return 0; - return log_debug_errno(r, "Unable to fix SMACK label of %s: %m", path); + return log_debug_errno(r, "Unable to fix SMACK label of %s: %m", abspath); } return 0; } +int mac_smack_fix_at(int dirfd, const char *path, LabelFixFlags flags) { + _cleanup_free_ char *p = NULL; + _cleanup_close_ int fd = -1; + int r; + + assert(path); + + if (!mac_smack_use()) + return 0; + + fd = openat(dirfd, path, O_NOFOLLOW|O_CLOEXEC|O_PATH); + if (fd < 0) { + if ((flags & LABEL_IGNORE_ENOENT) && errno == ENOENT) + return 0; + + return -errno; + } + + r = fd_get_path(fd, &p); + if (r < 0) + return r; + + return smack_fix_fd(fd, p, flags); +} + +int mac_smack_fix(const char *path, LabelFixFlags flags) { + _cleanup_free_ char *abspath = NULL; + _cleanup_close_ int fd = -1; + int r; + + assert(path); + + if (!mac_smack_use()) + return 0; + + r = path_make_absolute_cwd(path, &abspath); + if (r < 0) + return r; + + fd = open(abspath, O_NOFOLLOW|O_CLOEXEC|O_PATH); + if (fd < 0) { + if ((flags & LABEL_IGNORE_ENOENT) && errno == ENOENT) + return 0; + + return -errno; + } + + return smack_fix_fd(fd, abspath, flags); +} + int mac_smack_copy(const char *dest, const char *src) { int r = 0; _cleanup_free_ char *label = NULL; @@ -249,6 +276,10 @@ int mac_smack_fix(const char *path, LabelFixFlags flags) { return 0; } +int mac_smack_fix_at(int dirfd, const char *path, LabelFixFlags flags) { + return 0; +} + int mac_smack_copy(const char *dest, const char *src) { return 0; } diff --git a/src/basic/smack-util.h b/src/basic/smack-util.h index fd59787ecb..395ec07b57 100644 --- a/src/basic/smack-util.h +++ b/src/basic/smack-util.h @@ -30,6 +30,7 @@ typedef enum SmackAttr { bool mac_smack_use(void); int mac_smack_fix(const char *path, LabelFixFlags flags); +int mac_smack_fix_at(int dirfd, const char *path, LabelFixFlags flags); const char* smack_attr_to_string(SmackAttr i) _const_; SmackAttr smack_attr_from_string(const char *s) _pure_; From 7e531a5265687aef5177b070c36ca4ceab42e768 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 2 Jul 2018 10:22:56 +0200 Subject: [PATCH 13/26] selinux: introduce mac_selinux_create_file_prepare_at() --- src/basic/selinux-util.c | 93 +++++++++++++++++++++++++++++----------- src/basic/selinux-util.h | 1 + 2 files changed, 68 insertions(+), 26 deletions(-) diff --git a/src/basic/selinux-util.c b/src/basic/selinux-util.c index e15bd7e1fa..238b1d95e2 100644 --- a/src/basic/selinux-util.c +++ b/src/basic/selinux-util.c @@ -316,47 +316,88 @@ char* mac_selinux_free(char *label) { return NULL; } -int mac_selinux_create_file_prepare(const char *path, mode_t mode) { +#if HAVE_SELINUX +static int selinux_create_file_prepare_abspath(const char *abspath, mode_t mode) { + _cleanup_freecon_ char *filecon = NULL; + _cleanup_free_ char *path = NULL; + int r; + + assert(abspath); + assert(path_is_absolute(abspath)); + + r = selabel_lookup_raw(label_hnd, &filecon, abspath, mode); + if (r < 0) { + /* No context specified by the policy? Proceed without setting it. */ + if (errno == ENOENT) + return 0; + + log_enforcing_errno(errno, "Failed to determine SELinux security context for %s: %m", abspath); + } else { + if (setfscreatecon_raw(filecon) >= 0) + return 0; /* Success! */ + + log_enforcing_errno(errno, "Failed to set SELinux security context %s for %s: %m", filecon, abspath); + } + + if (security_getenforce() > 0) + return -errno; + + return 0; +} +#endif + +int mac_selinux_create_file_prepare_at(int dirfd, const char *path, mode_t mode) { + int r = 0; #if HAVE_SELINUX - _cleanup_freecon_ char *filecon = NULL; - int r; + _cleanup_free_ char *abspath = NULL; + _cleanup_close_ int fd = -1; assert(path); if (!label_hnd) return 0; - if (path_is_absolute(path)) - r = selabel_lookup_raw(label_hnd, &filecon, path, mode); - else { - _cleanup_free_ char *newpath = NULL; + if (!path_is_absolute(path)) { + _cleanup_free_ char *p = NULL; - r = path_make_absolute_cwd(path, &newpath); + if (dirfd == AT_FDCWD) + r = safe_getcwd(&p); + else + r = fd_get_path(dirfd, &p); if (r < 0) return r; - r = selabel_lookup_raw(label_hnd, &filecon, newpath, mode); + abspath = path_join(NULL, p, path); + if (!abspath) + return -ENOMEM; + + path = abspath; } - if (r < 0) { - /* No context specified by the policy? Proceed without setting it. */ - if (errno == ENOENT) - return 0; - - log_enforcing_errno(errno, "Failed to determine SELinux security context for %s: %m", path); - } else { - if (setfscreatecon_raw(filecon) >= 0) - return 0; /* Success! */ - - log_enforcing_errno(errno, "Failed to set SELinux security context %s for %s: %m", filecon, path); - } - - if (security_getenforce() > 0) - return -errno; - + r = selinux_create_file_prepare_abspath(path, mode); #endif - return 0; + return r; +} + +int mac_selinux_create_file_prepare(const char *path, mode_t mode) { + int r = 0; + +#if HAVE_SELINUX + _cleanup_free_ char *abspath = NULL; + + assert(path); + + if (!label_hnd) + return 0; + + r = path_make_absolute_cwd(path, &abspath); + if (r < 0) + return r; + + r = selinux_create_file_prepare_abspath(abspath, mode); +#endif + return r; } void mac_selinux_create_file_clear(void) { diff --git a/src/basic/selinux-util.h b/src/basic/selinux-util.h index 08314057fb..bd5207c318 100644 --- a/src/basic/selinux-util.h +++ b/src/basic/selinux-util.h @@ -23,6 +23,7 @@ int mac_selinux_get_child_mls_label(int socket_fd, const char *exe, const char * char* mac_selinux_free(char *label); int mac_selinux_create_file_prepare(const char *path, mode_t mode); +int mac_selinux_create_file_prepare_at(int dirfd, const char *path, mode_t mode); void mac_selinux_create_file_clear(void); int mac_selinux_create_socket_prepare(const char *label); From 1e9126316fa10f8f1e2d40b90c77fb63fe10f781 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 27 Apr 2018 12:55:56 +0200 Subject: [PATCH 14/26] basic/label: introduce mkdirat_label()/mkdirat_errno_wrapper() helpers --- src/basic/label.h | 1 + src/basic/mkdir-label.c | 17 +++++++++++++++++ src/basic/mkdir.c | 6 ++++++ src/basic/mkdir.h | 1 + 4 files changed, 25 insertions(+) diff --git a/src/basic/label.h b/src/basic/label.h index 08fd109bcf..594fd65974 100644 --- a/src/basic/label.h +++ b/src/basic/label.h @@ -12,6 +12,7 @@ typedef enum LabelFixFlags { int label_fix(const char *path, LabelFixFlags flags); int mkdir_label(const char *path, mode_t mode); +int mkdirat_label(int dirfd, const char *path, mode_t mode); int symlink_label(const char *old_path, const char *new_path); int btrfs_subvol_make_label(const char *path); diff --git a/src/basic/mkdir-label.c b/src/basic/mkdir-label.c index 1d51e92e9a..0eba7fc514 100644 --- a/src/basic/mkdir-label.c +++ b/src/basic/mkdir-label.c @@ -28,6 +28,23 @@ int mkdir_label(const char *path, mode_t mode) { return mac_smack_fix(path, 0); } +int mkdirat_label(int dirfd, const char *path, mode_t mode) { + int r; + + assert(path); + + r = mac_selinux_create_file_prepare_at(dirfd, path, S_IFDIR); + if (r < 0) + return r; + + r = mkdirat_errno_wrapper(dirfd, path, mode); + mac_selinux_create_file_clear(); + if (r < 0) + return r; + + return mac_smack_fix_at(dirfd, path, 0); +} + int mkdir_safe_label(const char *path, mode_t mode, uid_t uid, gid_t gid, MkdirFlags flags) { return mkdir_safe_internal(path, mode, uid, gid, flags, mkdir_label); } diff --git a/src/basic/mkdir.c b/src/basic/mkdir.c index 6ab1b4422b..4bb65d5838 100644 --- a/src/basic/mkdir.c +++ b/src/basic/mkdir.c @@ -80,6 +80,12 @@ int mkdir_errno_wrapper(const char *pathname, mode_t mode) { return 0; } +int mkdirat_errno_wrapper(int dirfd, const char *pathname, mode_t mode) { + if (mkdirat(dirfd, pathname, mode) < 0) + return -errno; + return 0; +} + int mkdir_safe(const char *path, mode_t mode, uid_t uid, gid_t gid, MkdirFlags flags) { return mkdir_safe_internal(path, mode, uid, gid, flags, mkdir_errno_wrapper); } diff --git a/src/basic/mkdir.h b/src/basic/mkdir.h index bdca15460e..eb54853ea7 100644 --- a/src/basic/mkdir.h +++ b/src/basic/mkdir.h @@ -9,6 +9,7 @@ typedef enum MkdirFlags { } MkdirFlags; int mkdir_errno_wrapper(const char *pathname, mode_t mode); +int mkdirat_errno_wrapper(int dirfd, const char *pathname, mode_t mode); int mkdir_safe(const char *path, mode_t mode, uid_t uid, gid_t gid, MkdirFlags flags); int mkdir_parents(const char *path, mode_t mode); int mkdir_p(const char *path, mode_t mode); From 62f9666ae03d3a2f0205ffa54eb4267faca43b74 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 27 Apr 2018 17:04:47 +0200 Subject: [PATCH 15/26] btrfs-util: introduce btrfs_subvol_make_fd() --- src/basic/btrfs-util.c | 26 +++++++++++++++++++------- src/basic/btrfs-util.h | 1 + 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/basic/btrfs-util.c b/src/basic/btrfs-util.c index efac0b9420..abd34824e7 100644 --- a/src/basic/btrfs-util.c +++ b/src/basic/btrfs-util.c @@ -116,8 +116,25 @@ int btrfs_is_subvol(const char *path) { return btrfs_is_subvol_fd(fd); } -int btrfs_subvol_make(const char *path) { +int btrfs_subvol_make_fd(int fd, const char *subvolume) { struct btrfs_ioctl_vol_args args = {}; + int r; + + assert(subvolume); + + r = validate_subvolume_name(subvolume); + if (r < 0) + return r; + + strncpy(args.name, subvolume, sizeof(args.name)-1); + + if (ioctl(fd, BTRFS_IOC_SUBVOL_CREATE, &args) < 0) + return -errno; + + return 0; +} + +int btrfs_subvol_make(const char *path) { _cleanup_close_ int fd = -1; const char *subvolume; int r; @@ -132,12 +149,7 @@ int btrfs_subvol_make(const char *path) { if (fd < 0) return fd; - strncpy(args.name, subvolume, sizeof(args.name)-1); - - if (ioctl(fd, BTRFS_IOC_SUBVOL_CREATE, &args) < 0) - return -errno; - - return 0; + return btrfs_subvol_make_fd(fd, subvolume); } int btrfs_subvol_set_read_only_fd(int fd, bool b) { diff --git a/src/basic/btrfs-util.h b/src/basic/btrfs-util.h index a594387b5a..b0cf6739f7 100644 --- a/src/basic/btrfs-util.h +++ b/src/basic/btrfs-util.h @@ -65,6 +65,7 @@ int btrfs_resize_loopback_fd(int fd, uint64_t size, bool grow_only); int btrfs_resize_loopback(const char *path, uint64_t size, bool grow_only); int btrfs_subvol_make(const char *path); +int btrfs_subvol_make_fd(int fd, const char *subvolume); int btrfs_subvol_snapshot_fd(int old_fd, const char *new_path, BtrfsSnapshotFlags flags); int btrfs_subvol_snapshot(const char *old_path, const char *new_path, BtrfsSnapshotFlags flags); From a12e4ade1b7634f7286f53d9ce2b3a12ab17826e Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 20 Jul 2018 09:05:38 +0200 Subject: [PATCH 16/26] basic/stat-util: introduce is_dir_fd() --- src/basic/stat-util.c | 11 +++++++++++ src/basic/stat-util.h | 1 + 2 files changed, 12 insertions(+) diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c index 07154e25bb..762777e94f 100644 --- a/src/basic/stat-util.c +++ b/src/basic/stat-util.c @@ -45,6 +45,17 @@ int is_dir(const char* path, bool follow) { return !!S_ISDIR(st.st_mode); } +int is_dir_fd(int fd) { + struct stat st; + int r; + + r = fstat(fd, &st); + if (r < 0) + return -errno; + + return !!S_ISDIR(st.st_mode); +} + int is_device_node(const char *path) { struct stat info; diff --git a/src/basic/stat-util.h b/src/basic/stat-util.h index f8014ed30b..1a725f1da0 100644 --- a/src/basic/stat-util.h +++ b/src/basic/stat-util.h @@ -12,6 +12,7 @@ int is_symlink(const char *path); int is_dir(const char *path, bool follow); +int is_dir_fd(int fd); int is_device_node(const char *path); int dir_is_empty(const char *path); From 4c39d899ff00e90b7290e4985696f321d7f2726f Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 27 Apr 2018 11:51:14 +0200 Subject: [PATCH 17/26] tmpfiles: introduce create_directory() and create_subvolume() and make them safe --- src/tmpfiles/tmpfiles.c | 87 ++++++++++++++++++++++++++++++----------- 1 file changed, 65 insertions(+), 22 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 85a2ea8caa..9e007d54e6 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1527,17 +1527,18 @@ static const char *creation_mode_verb_table[_CREATION_MODE_MAX] = { DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(creation_mode_verb, CreationMode); -static int create_directory_or_subvolume(Item *i, const char *path) { +static int create_directory_or_subvolume(const char *path, mode_t mode, bool subvol) { + _cleanup_close_ int pfd = -1; CreationMode creation; - int r, q = 0; + int r; - assert(i); - assert(IN_SET(i->type, - CREATE_DIRECTORY, TRUNCATE_DIRECTORY, - CREATE_SUBVOLUME, CREATE_SUBVOLUME_NEW_QUOTA, CREATE_SUBVOLUME_INHERIT_QUOTA)); + assert(path); - if (IN_SET(i->type, CREATE_SUBVOLUME, CREATE_SUBVOLUME_INHERIT_QUOTA, CREATE_SUBVOLUME_NEW_QUOTA)) { + pfd = path_open_parent_safe(path); + if (pfd < 0) + return pfd; + if (subvol) { if (btrfs_is_subvol(empty_to_root(arg_root)) <= 0) /* Don't create a subvolume unless the root directory is @@ -1550,42 +1551,76 @@ static int create_directory_or_subvolume(Item *i, const char *path) { * specifically set up a btrfs subvolume for the root * dir too. */ - r = -ENOTTY; + subvol = false; else { - RUN_WITH_UMASK((~i->mode) & 0777) - r = btrfs_subvol_make(i->path); + RUN_WITH_UMASK((~mode) & 0777) + r = btrfs_subvol_make_fd(pfd, basename(path)); } } else r = 0; - if (IN_SET(i->type, CREATE_DIRECTORY, TRUNCATE_DIRECTORY) || r == -ENOTTY) + if (!subvol || r == -ENOTTY) RUN_WITH_UMASK(0000) - r = mkdir_label(i->path, i->mode); + r = mkdirat_label(pfd, basename(path), mode); if (r < 0) { int k; if (!IN_SET(r, -EEXIST, -EROFS)) - return log_error_errno(r, "Failed to create directory or subvolume \"%s\": %m", i->path); + return log_error_errno(r, "Failed to create directory or subvolume \"%s\": %m", path); - k = is_dir(i->path, false); + k = is_dir_fd(pfd); if (k == -ENOENT && r == -EROFS) - return log_error_errno(r, "%s does not exist and cannot be created as the file system is read-only.", i->path); + return log_error_errno(r, "%s does not exist and cannot be created as the file system is read-only.", path); if (k < 0) - return log_error_errno(k, "Failed to check if %s exists: %m", i->path); + return log_error_errno(k, "Failed to check if %s exists: %m", path); if (!k) { - log_warning("\"%s\" already exists and is not a directory.", i->path); - return 0; + log_warning("\"%s\" already exists and is not a directory.", path); + return -EEXIST; } creation = CREATION_EXISTING; } else creation = CREATION_NORMAL; - log_debug("%s directory \"%s\".", creation_mode_verb_to_string(creation), i->path); + log_debug("%s directory \"%s\".", creation_mode_verb_to_string(creation), path); + + r = openat(pfd, basename(path), O_NOCTTY|O_CLOEXEC|O_DIRECTORY); + if (r < 0) + return -errno; + return r; +} + +static int create_directory(Item *i, const char *path) { + _cleanup_close_ int fd = -1; + + assert(i); + assert(IN_SET(i->type, CREATE_DIRECTORY, TRUNCATE_DIRECTORY)); + + fd = create_directory_or_subvolume(path, i->mode, false); + if (fd == -EEXIST) + return 0; + if (fd < 0) + return fd; + + return fd_set_perms(i, fd, NULL); +} + +static int create_subvolume(Item *i, const char *path) { + _cleanup_close_ int fd = -1; + int r, q = 0; + + assert(i); + assert(IN_SET(i->type, CREATE_SUBVOLUME, CREATE_SUBVOLUME_NEW_QUOTA, CREATE_SUBVOLUME_INHERIT_QUOTA)); + + fd = create_directory_or_subvolume(path, i->mode, true); + if (fd == -EEXIST) + return 0; + if (fd < 0) + return fd; if (IN_SET(i->type, CREATE_SUBVOLUME_NEW_QUOTA, CREATE_SUBVOLUME_INHERIT_QUOTA)) { - r = btrfs_subvol_auto_qgroup(i->path, 0, i->type == CREATE_SUBVOLUME_NEW_QUOTA); + r = btrfs_subvol_auto_qgroup_fd(fd, 0, i->type == CREATE_SUBVOLUME_NEW_QUOTA); if (r == -ENOTTY) log_debug_errno(r, "Couldn't adjust quota for subvolume \"%s\" (unsupported fs or dir not a subvolume): %m", i->path); else if (r == -EROFS) @@ -1600,7 +1635,7 @@ static int create_directory_or_subvolume(Item *i, const char *path) { log_debug("Quota for subvolume \"%s\" already in place, no change made.", i->path); } - r = path_set_perms(i, i->path); + r = fd_set_perms(i, fd, NULL); if (q < 0) return q; @@ -1846,13 +1881,21 @@ static int create_item(Item *i) { case CREATE_DIRECTORY: case TRUNCATE_DIRECTORY: + RUN_WITH_UMASK(0000) + (void) mkdir_parents_label(i->path, 0755); + + r = create_directory(i, i->path); + if (r < 0) + return r; + break; + 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 = create_directory_or_subvolume(i, i->path); + r = create_subvolume(i, i->path); if (r < 0) return r; break; From 5494602195f5300db8b2eac7c2b77b922d0ceddf Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 26 Apr 2018 15:45:17 +0200 Subject: [PATCH 18/26] tmpfiles: introduce empty_directory() --- src/tmpfiles/tmpfiles.c | 56 ++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 9e007d54e6..7b8c4bb53f 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -866,40 +866,15 @@ static int path_open_parent_safe(const char *path) { static int path_set_perms(Item *i, const char *path) { _cleanup_close_ int fd = -1; - struct stat stbuf, *st = NULL; assert(i); assert(path); fd = open(path, O_NOFOLLOW|O_CLOEXEC|O_PATH); - if (fd < 0) { - int level = LOG_ERR, r = -errno; + if (fd < 0) + return log_error_errno(errno, "Failed to open \"%s\" to adjust permissions: %m", path); - /* Option "e" operates only on existing objects. Do not - * print errors about non-existent files or directories */ - if (i->type == EMPTY_DIRECTORY && errno == ENOENT) { - level = LOG_DEBUG; - r = 0; - } - - log_full_errno(level, errno, "Adjusting owner and mode for %s failed: %m", path); - return r; - } - - if (i->type == EMPTY_DIRECTORY) { - /* FIXME: introduce fd_is_dir() helper ? */ - if (fstat(fd, &stbuf) < 0) - return log_error_errno(errno, "Failed to fstat() file %s: %m", path); - - if (!S_ISDIR(stbuf.st_mode)) { - log_error("'%s' already exists and is not a directory. ", path); - return -EEXIST; - } - - st = &stbuf; - } - - return fd_set_perms(i, fd, st); + return fd_set_perms(i, fd, NULL); } static int parse_xattrs_from_arg(Item *i) { @@ -1642,6 +1617,29 @@ static int create_subvolume(Item *i, const char *path) { return r; } +static int empty_directory(Item *i, const char *path) { + int r; + + assert(i); + assert(i->type == EMPTY_DIRECTORY); + + r = is_dir(path, false); + if (r == -ENOENT) { + /* Option "e" operates only on existing objects. Do not + * print errors about non-existent files or directories */ + log_debug("Skipping missing directory: %s", path); + return 0; + } + if (r < 0) + return log_error_errno(r, "is_dir() failed on path %s: %m", path); + if (r == 0) { + log_error("'%s' already exists and is not a directory.", path); + return -EEXIST; + } + + return path_set_perms(i, path); +} + static int create_device(Item *i, mode_t file_type) { _cleanup_close_ int dfd = -1, fd = -1; CreationMode creation; @@ -1901,7 +1899,7 @@ static int create_item(Item *i) { break; case EMPTY_DIRECTORY: - r = glob_item(i, path_set_perms); + r = glob_item(i, empty_directory); if (r < 0) return r; break; From a2fc2f8dd30c17ad1e23a31fc6ff2aeba4c6fa27 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 27 Apr 2018 18:11:26 +0200 Subject: [PATCH 19/26] tmpfiles: introduce create_fifo() --- src/tmpfiles/tmpfiles.c | 85 ++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 7b8c4bb53f..a934cbff91 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1714,6 +1714,49 @@ static int create_device(Item *i, mode_t file_type) { return fd_set_perms(i, fd, NULL); } +static int create_fifo(Item *i, const char *path) { + CreationMode creation; + struct stat st; + int r; + + RUN_WITH_UMASK(0000) { + mac_selinux_create_file_prepare(path, S_IFIFO); + r = mkfifo(path, i->mode); + mac_selinux_create_file_clear(); + } + + if (r < 0) { + if (errno != EEXIST) + return log_error_errno(errno, "Failed to create fifo %s: %m", path); + + if (lstat(path, &st) < 0) + return log_error_errno(errno, "stat(%s) failed: %m", path); + + if (!S_ISFIFO(st.st_mode)) { + + if (i->force) { + RUN_WITH_UMASK(0000) { + mac_selinux_create_file_prepare(path, S_IFIFO); + r = mkfifo_atomic(path, i->mode); + mac_selinux_create_file_clear(); + } + + if (r < 0) + return log_error_errno(r, "Failed to create fifo %s: %m", path); + creation = CREATION_FORCE; + } else { + log_warning("\"%s\" already exists and is not a fifo.", path); + return 0; + } + } else + creation = CREATION_EXISTING; + } else + creation = CREATION_NORMAL; + log_debug("%s fifo \"%s\".", creation_mode_verb_to_string(creation), path); + + return path_set_perms(i, path); +} + typedef int (*action_t)(Item *, const char *); typedef int (*fdaction_t)(Item *, int fd, const struct stat *st); @@ -1827,9 +1870,8 @@ static int glob_item_recursively(Item *i, fdaction_t action) { } static int create_item(Item *i) { - struct stat st; - int r = 0; CreationMode creation; + int r = 0; assert(i); @@ -1905,47 +1947,12 @@ static int create_item(Item *i) { break; case CREATE_FIFO: - RUN_WITH_UMASK(0000) { + RUN_WITH_UMASK(0000) (void) mkdir_parents_label(i->path, 0755); - mac_selinux_create_file_prepare(i->path, S_IFIFO); - r = mkfifo(i->path, i->mode); - mac_selinux_create_file_clear(); - } - - if (r < 0) { - if (errno != EEXIST) - return log_error_errno(errno, "Failed to create fifo %s: %m", i->path); - - if (lstat(i->path, &st) < 0) - return log_error_errno(errno, "stat(%s) failed: %m", i->path); - - if (!S_ISFIFO(st.st_mode)) { - - if (i->force) { - RUN_WITH_UMASK(0000) { - mac_selinux_create_file_prepare(i->path, S_IFIFO); - r = mkfifo_atomic(i->path, i->mode); - mac_selinux_create_file_clear(); - } - - if (r < 0) - return log_error_errno(r, "Failed to create fifo %s: %m", i->path); - creation = CREATION_FORCE; - } else { - log_warning("\"%s\" already exists and is not a fifo.", i->path); - return 0; - } - } else - creation = CREATION_EXISTING; - } else - creation = CREATION_NORMAL; - log_debug("%s fifo \"%s\".", creation_mode_verb_to_string(creation), i->path); - - r = path_set_perms(i, i->path); + r = create_fifo(i, i->path); if (r < 0) return r; - break; case CREATE_SYMLINK: { From 43231f00c2b9508c37d4cbbb2e49e9b99b096792 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 30 Jul 2018 15:10:49 +0200 Subject: [PATCH 20/26] fileio: make tempfn_random_child() accept empty string as path In this case it simply returns the random generated filename with anything prefixed. --- src/basic/fileio.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 20d3f567c9..ea607f8cfb 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1319,8 +1319,7 @@ int tempfn_random_child(const char *p, const char *extra, char **ret) { r = tmp_dir(&p); if (r < 0) return r; - } else if (isempty(p)) - return -EINVAL; + } extra = strempty(extra); @@ -1328,7 +1327,10 @@ int tempfn_random_child(const char *p, const char *extra, char **ret) { if (!t) return -ENOMEM; - x = stpcpy(stpcpy(stpcpy(t, p), "/.#"), extra); + if (isempty(p)) + x = stpcpy(stpcpy(t, ".#"), extra); + else + x = stpcpy(stpcpy(stpcpy(t, p), "/.#"), extra); u = random_u64(); for (i = 0; i < 16; i++) { From 4fe3828c587e4324184fbf153606277058dbe33a Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 27 Apr 2018 18:20:38 +0200 Subject: [PATCH 21/26] fs-util: introduce mkfifoat_atomic() helper --- src/basic/fs-util.c | 25 +++++++++++++++++++++++++ src/basic/fs-util.h | 1 + 2 files changed, 26 insertions(+) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index aca9921de7..1fbd40ade7 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -435,6 +435,31 @@ int mkfifo_atomic(const char *path, mode_t mode) { return 0; } +int mkfifoat_atomic(int dirfd, const char *path, mode_t mode) { + _cleanup_free_ char *t = NULL; + int r; + + assert(path); + + if (path_is_absolute(path)) + return mkfifo_atomic(path, mode); + + /* We're only interested in the (random) filename. */ + r = tempfn_random_child("", NULL, &t); + if (r < 0) + return r; + + if (mkfifoat(dirfd, t, mode) < 0) + return -errno; + + if (renameat(dirfd, t, dirfd, path) < 0) { + unlink_noerrno(t); + return -errno; + } + + return 0; +} + int get_files_in_directory(const char *path, char ***list) { _cleanup_closedir_ DIR *d = NULL; struct dirent *de; diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index 754163defd..b1a366ae49 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -42,6 +42,7 @@ int symlink_idempotent(const char *from, const char *to); int symlink_atomic(const char *from, const char *to); int mknod_atomic(const char *path, mode_t mode, dev_t dev); int mkfifo_atomic(const char *path, mode_t mode); +int mkfifoat_atomic(int dir_fd, const char *path, mode_t mode); int get_files_in_directory(const char *path, char ***list); From 7ea5a87f92bbc7e30cf198bfbad2472a1ecdbf78 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 27 Apr 2018 18:17:32 +0200 Subject: [PATCH 22/26] tmpfiles: make create_fifo() safe --- src/tmpfiles/tmpfiles.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index a934cbff91..71f8dec49b 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1715,13 +1715,21 @@ static int create_device(Item *i, mode_t file_type) { } static int create_fifo(Item *i, const char *path) { + _cleanup_close_ int pfd = -1, fd = -1; CreationMode creation; struct stat st; + char *bn; int r; + pfd = path_open_parent_safe(path); + if (pfd < 0) + return pfd; + + bn = basename(path); + RUN_WITH_UMASK(0000) { mac_selinux_create_file_prepare(path, S_IFIFO); - r = mkfifo(path, i->mode); + r = mkfifoat(pfd, bn, i->mode); mac_selinux_create_file_clear(); } @@ -1729,7 +1737,7 @@ static int create_fifo(Item *i, const char *path) { if (errno != EEXIST) return log_error_errno(errno, "Failed to create fifo %s: %m", path); - if (lstat(path, &st) < 0) + if (fstatat(pfd, bn, &st, AT_SYMLINK_NOFOLLOW) < 0) return log_error_errno(errno, "stat(%s) failed: %m", path); if (!S_ISFIFO(st.st_mode)) { @@ -1737,7 +1745,7 @@ static int create_fifo(Item *i, const char *path) { if (i->force) { RUN_WITH_UMASK(0000) { mac_selinux_create_file_prepare(path, S_IFIFO); - r = mkfifo_atomic(path, i->mode); + r = mkfifoat_atomic(pfd, bn, i->mode); mac_selinux_create_file_clear(); } @@ -1752,9 +1760,14 @@ static int create_fifo(Item *i, const char *path) { creation = CREATION_EXISTING; } else creation = CREATION_NORMAL; + log_debug("%s fifo \"%s\".", creation_mode_verb_to_string(creation), path); - return path_set_perms(i, path); + fd = openat(pfd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH); + if (fd < 0) + return log_error_errno(fd, "Failed to openat(%s): %m", path); + + return fd_set_perms(i, fd, NULL); } typedef int (*action_t)(Item *, const char *); From 1f56e4ce773f195bbdf2dfc639d967309321441c Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 26 Apr 2018 22:46:55 +0200 Subject: [PATCH 23/26] fs-util: add new CHASE_NOFOLLOW flag to chase_symlinks() This flag mimics what "O_NOFOLLOW|O_PATH" does for open(2) that is chase_symlinks() will not resolve the final pathname component if it's a symlink and instead will return a file descriptor referring to the symlink itself. Note: if CHASE_SAFE is also passed, no safety checking is performed on the transition done if the symlink would have been followed. --- src/basic/fs-util.c | 4 ++-- src/basic/fs-util.h | 1 + src/test/test-fs-util.c | 25 +++++++++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 1fbd40ade7..09fcc32e0e 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -695,7 +695,7 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, if (!original_root && !ret && (flags & (CHASE_NONEXISTENT|CHASE_NO_AUTOFS|CHASE_SAFE|CHASE_OPEN|CHASE_STEP)) == CHASE_OPEN) { /* Shortcut the CHASE_OPEN case if the caller isn't interested in the actual path and has no root set * and doesn't care about any of the other special features we provide either. */ - r = open(path, O_PATH|O_CLOEXEC); + r = open(path, O_PATH|O_CLOEXEC|((flags & CHASE_NOFOLLOW) ? O_NOFOLLOW : 0)); if (r < 0) return -errno; @@ -850,7 +850,7 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, fd_is_fs_type(child, AUTOFS_SUPER_MAGIC) > 0) return -EREMOTE; - if (S_ISLNK(st.st_mode)) { + if (S_ISLNK(st.st_mode) && !((flags & CHASE_NOFOLLOW) && isempty(todo))) { char *joined; _cleanup_free_ char *destination = NULL; diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index b1a366ae49..4b65625861 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -73,6 +73,7 @@ enum { CHASE_OPEN = 1 << 4, /* If set, return an O_PATH object to the final component */ CHASE_TRAIL_SLASH = 1 << 5, /* If set, any trailing slash will be preserved */ CHASE_STEP = 1 << 6, /* If set, just execute a single step of the normalization */ + CHASE_NOFOLLOW = 1 << 7, /* Only valid with CHASE_OPEN: when the path's right-most component refers to symlink return O_PATH fd of the symlink, rather than following it. */ }; /* How many iterations to execute before returning -ELOOP */ diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index fc650b513e..d188c24f7b 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -22,6 +22,7 @@ static void test_chase_symlinks(void) { _cleanup_free_ char *result = NULL; char temp[] = "/tmp/test-chase.XXXXXX"; const char *top, *p, *pslash, *q, *qslash; + struct stat st; int r, pfd; assert_se(mkdtemp(temp)); @@ -266,6 +267,30 @@ static void test_chase_symlinks(void) { assert_se(sd_id128_equal(a, b)); } + /* Test CHASE_NOFOLLOW */ + + p = strjoina(temp, "/target"); + q = strjoina(temp, "/symlink"); + assert_se(symlink(p, q) >= 0); + pfd = chase_symlinks(q, NULL, CHASE_OPEN|CHASE_NOFOLLOW, &result); + assert_se(pfd > 0); + assert_se(path_equal(result, q)); + assert_se(fstat(pfd, &st) >= 0); + assert_se(S_ISLNK(st.st_mode)); + result = mfree(result); + + /* s1 -> s2 -> nonexistent */ + q = strjoina(temp, "/s1"); + assert_se(symlink("s2", q) >= 0); + p = strjoina(temp, "/s2"); + assert_se(symlink("nonexistent", p) >= 0); + pfd = chase_symlinks(q, NULL, CHASE_OPEN|CHASE_NOFOLLOW, &result); + assert_se(pfd > 0); + assert_se(path_equal(result, q)); + assert_se(fstat(pfd, &st) >= 0); + assert_se(S_ISLNK(st.st_mode)); + result = mfree(result); + /* Test CHASE_ONE */ p = strjoina(temp, "/start"); From addc3e302dad239fb11cf280b0ce4761fe07e015 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 20 Mar 2018 08:58:48 +0100 Subject: [PATCH 24/26] tmpfiles: don't follow unsafe transitions in path_set_*() Since all path_set_*() helpers don't follow symlinks, it's possible to use chase_symlinks(CHASE_NOFOLLOW) flag to both open the files specified by the passed paths and check their validity (unlike their counterpart fd_set_*() helpers). --- src/tmpfiles/tmpfiles.c | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 71f8dec49b..cfd9044c5b 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -864,15 +864,38 @@ static int path_open_parent_safe(const char *path) { return fd; } +static int path_open_safe(const char *path) { + int fd; + + /* path_open_safe() returns a file descriptor opened with O_PATH after + * verifying that the path doesn't contain unsafe transitions, except + * for its final component as the function does not follow symlink. */ + + assert(path); + + if (!path_is_normalized(path)) { + log_error("Failed to open invalid path '%s'.", path); + return -EINVAL; + } + + fd = chase_symlinks(path, NULL, CHASE_OPEN|CHASE_SAFE|CHASE_NOFOLLOW, NULL); + if (fd == -EPERM) + return log_error_errno(fd, "Unsafe symlinks encountered in %s, refusing.", path); + if (fd < 0) + return log_error_errno(fd, "Failed to validate path %s: %m", path); + + return fd; +} + static int path_set_perms(Item *i, const char *path) { _cleanup_close_ int fd = -1; assert(i); assert(path); - fd = open(path, O_NOFOLLOW|O_CLOEXEC|O_PATH); + fd = path_open_safe(path); if (fd < 0) - return log_error_errno(errno, "Failed to open \"%s\" to adjust permissions: %m", path); + return fd; return fd_set_perms(i, fd, NULL); } @@ -945,9 +968,9 @@ static int path_set_xattrs(Item *i, const char *path) { assert(i); assert(path); - fd = open(path, O_CLOEXEC|O_NOFOLLOW|O_PATH); + fd = path_open_safe(path); if (fd < 0) - return log_error_errno(errno, "Cannot open '%s': %m", path); + return fd; return fd_set_xattrs(i, fd, NULL); } @@ -1076,9 +1099,9 @@ static int path_set_acls(Item *item, const char *path) { assert(item); assert(path); - fd = open(path, O_NOFOLLOW|O_CLOEXEC|O_PATH); + fd = path_open_safe(path); if (fd < 0) - return log_error_errno(errno, "Adjusting ACL of %s failed: %m", path); + return fd; r = fd_set_acls(item, fd, NULL); #endif @@ -1238,9 +1261,9 @@ static int path_set_attribute(Item *item, const char *path) { if (!item->attribute_set || item->attribute_mask == 0) return 0; - fd = open(path, O_CLOEXEC|O_NOFOLLOW|O_PATH); + fd = path_open_safe(path); if (fd < 0) - return log_error_errno(errno, "Cannot open '%s': %m", path); + return fd; return fd_set_attribute(item, fd, NULL); } From 9f36a8fb381afd99c1a8c46f71521cbb99472b6c Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 13 Apr 2018 15:32:25 +0200 Subject: [PATCH 25/26] tmpfiles: add more tests --- test/TEST-22-TMPFILES/test-02.sh | 95 +++++++++++++ test/TEST-22-TMPFILES/test-03.sh | 236 +++++++++++++++++++++++++++++++ test/TEST-22-TMPFILES/test-04.sh | 44 ++++++ test/TEST-22-TMPFILES/test.sh | 2 + 4 files changed, 377 insertions(+) create mode 100755 test/TEST-22-TMPFILES/test-02.sh create mode 100755 test/TEST-22-TMPFILES/test-03.sh create mode 100755 test/TEST-22-TMPFILES/test-04.sh diff --git a/test/TEST-22-TMPFILES/test-02.sh b/test/TEST-22-TMPFILES/test-02.sh new file mode 100755 index 0000000000..fe8b903298 --- /dev/null +++ b/test/TEST-22-TMPFILES/test-02.sh @@ -0,0 +1,95 @@ +#! /bin/bash +# +# Basic tests for types creating directories +# + +set -e +set -x + +rm -fr /tmp/{d,D,e} +mkdir /tmp/{d,D,e} + +# +# 'd' +# +mkdir /tmp/d/2 +chmod 777 /tmp/d/2 + +systemd-tmpfiles --create - </tmp/F/truncated +echo "This should be truncated" >/tmp/F/truncated-with-content + +systemd-tmpfiles --create - </tmp/F/rw-fs/foo +! systemd-tmpfiles --create - </tmp/F/rw-fs/foo +! systemd-tmpfiles --create - < fails. +! systemd-tmpfiles --create - < Date: Mon, 16 Jul 2018 17:57:41 +0200 Subject: [PATCH 26/26] path-util: make use of path_join() in path_make_absolute_cwd() --- src/basic/path-util.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index d214c72916..b62786f27e 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -110,10 +110,7 @@ int path_make_absolute_cwd(const char *p, char **ret) { if (r < 0) return r; - if (endswith(cwd, "/")) - c = strjoin(cwd, p); - else - c = strjoin(cwd, "/", p); + c = path_join(NULL, cwd, p); } if (!c) return -ENOMEM;