rpms
/
systemd
2
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Don't require /proc to be mounted for systemd-sysusers to work

This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2020-03-03 13:51:43 +01:00
parent ab2423caa9
commit 111b3c5a31
4 changed files with 315 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
0001-sysusers-many-different-errnos-to-express-one-condit.patch Normal file
View File

@ -0,0 +1,53 @@
From e3ba241cd4003ee6eb6704e8c53240687534d6ce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 3 Mar 2020 10:18:32 +0100
Subject: [PATCH] sysusers: many different errnos to express one condition
See https://bugzilla.redhat.com/show_bug.cgi?id=1807768. It turns
out that sysusers cannot query if the group exists:
Failed to check if group dnsmasq already exists: No such process
...
Failed to check if group systemd-timesync already exists: No such process
When the same command is executed later, the issue does not occur. Not sure why
the behaviour in the initial transaction is different. But let's accept all
errors that the man pages list.
---
src/sysusers/sysusers.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c
index 2771fd959f..1b1f19e817 100644
--- a/src/sysusers/sysusers.c
+++ b/src/sysusers/sysusers.c
@@ -94,6 +94,12 @@ STATIC_DESTRUCTOR_REGISTER(database_groups, set_free_freep);
STATIC_DESTRUCTOR_REGISTER(uid_range, freep);
STATIC_DESTRUCTOR_REGISTER(arg_root, freep);
+static int errno_is_not_exists(int code) {
+ /* See getpwnam(3) and getgrnam(3): those codes and others can be returned if the user or group are
+ * not found. */
+ return IN_SET(code, 0, ENOENT, ESRCH, EBADF, EPERM);
+}
+
static int load_user_database(void) {
_cleanup_fclose_ FILE *f = NULL;
const char *passwd_path;
@@ -971,7 +977,7 @@ static int add_user(Item *i) {
return 0;
}
- if (!IN_SET(errno, 0, ENOENT))
+ if (!errno_is_not_exists(errno))
return log_error_errno(errno, "Failed to check if user %s already exists: %m", i->name);
}
@@ -1108,7 +1114,7 @@ static int get_gid_by_name(const char *name, gid_t *gid) {
*gid = g->gr_gid;
return 0;
}
- if (!IN_SET(errno, 0, ENOENT))
+ if (!errno_is_not_exists(errno))
return log_error_errno(errno, "Failed to check if group %s already exists: %m", name);
}

144
0002-basic-fs-util-add-a-version-of-chmod_and_chown-that-.patch Normal file
View File

@ -0,0 +1,144 @@
From 6cb356ca9fe063846cfb883ef484f7e7e411096c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 3 Mar 2020 11:51:50 +0100
Subject: [PATCH 2/3] basic/fs-util: add a version of chmod_and_chown that
doesn not use /proc
---
src/basic/fs-util.c | 46 +++++++++++++++++++++++++++++++++++++++++
src/basic/fs-util.h | 1 +
src/test/test-fs-util.c | 45 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 92 insertions(+)
diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index f8095e85d8..558cafbcaf 100644
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -272,6 +272,52 @@ int fchmod_and_chown(int fd, mode_t mode, uid_t uid, gid_t gid) {
return do_chown || do_chmod;
}
+int chmod_and_chown_unsafe(const char *path, mode_t mode, uid_t uid, gid_t gid) {
+ bool do_chown, do_chmod;
+ struct stat st;
+
+ assert(path);
+
+ /* Change ownership and access mode of the specified path, see description of fchmod_and_chown().
+ * Should only be used on trusted paths. */
+
+ if (lstat(path, &st) < 0)
+ return -errno;
+
+ do_chown =
+ (uid != UID_INVALID && st.st_uid != uid) ||
+ (gid != GID_INVALID && st.st_gid != gid);
+
+ do_chmod =
+ !S_ISLNK(st.st_mode) && /* chmod is not defined on symlinks */
+ ((mode != MODE_INVALID && ((st.st_mode ^ mode) & 07777) != 0) ||
+ do_chown); /* If we change ownership, make sure we reset the mode afterwards, since chown()
+ * modifies the access mode too */
+
+ if (mode == MODE_INVALID)
+ mode = st.st_mode; /* If we only shall do a chown(), save original mode, since chown() might break it. */
+ else if ((mode & S_IFMT) != 0 && ((mode ^ st.st_mode) & S_IFMT) != 0)
+ return -EINVAL; /* insist on the right file type if it was specified */
+
+ if (do_chown && do_chmod) {
+ mode_t minimal = st.st_mode & mode; /* the subset of the old and the new mask */
+
+ if (((minimal ^ st.st_mode) & 07777) != 0)
+ if (chmod(path, minimal & 07777) < 0)
+ return -errno;
+ }
+
+ if (do_chown)
+ if (lchown(path, uid, gid) < 0)
+ return -errno;
+
+ if (do_chmod)
+ if (chmod(path, mode & 07777) < 0)
+ return -errno;
+
+ return do_chown || do_chmod;
+}
+
int fchmod_umask(int fd, mode_t m) {
mode_t u;
int r;
diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h
index 78d68be9fd..6b9ade2ec1 100644
--- a/src/basic/fs-util.h
+++ b/src/basic/fs-util.h
@@ -34,6 +34,7 @@ int readlink_and_make_absolute(const char *p, char **r);
int chmod_and_chown(const char *path, mode_t mode, uid_t uid, gid_t gid);
int fchmod_and_chown(int fd, mode_t mode, uid_t uid, gid_t gid);
+int chmod_and_chown_unsafe(const char *path, mode_t mode, uid_t uid, gid_t gid);
int fchmod_umask(int fd, mode_t mode);
int fchmod_opath(int fd, mode_t m);
diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c
index d0c6fb82bf..d97ccfda3b 100644
--- a/src/test/test-fs-util.c
+++ b/src/test/test-fs-util.c
@@ -802,6 +802,50 @@ static void test_chmod_and_chown(void) {
assert_se(S_ISLNK(st.st_mode));
}
+static void test_chmod_and_chown_unsafe(void) {
+ _cleanup_(rm_rf_physical_and_freep) char *d = NULL;
+ _unused_ _cleanup_umask_ mode_t u = umask(0000);
+ struct stat st;
+ const char *p;
+
+ if (geteuid() != 0)
+ return;
+
+ log_info("/* %s */", __func__);
+
+ assert_se(mkdtemp_malloc(NULL, &d) >= 0);
+
+ p = strjoina(d, "/reg");
+ assert_se(mknod(p, S_IFREG | 0123, 0) >= 0);
+
+ assert_se(chmod_and_chown_unsafe(p, S_IFREG | 0321, 1, 2) >= 0);
+ assert_se(chmod_and_chown_unsafe(p, S_IFDIR | 0555, 3, 4) == -EINVAL);
+
+ assert_se(lstat(p, &st) >= 0);
+ assert_se(S_ISREG(st.st_mode));
+ assert_se((st.st_mode & 07777) == 0321);
+
+ p = strjoina(d, "/dir");
+ assert_se(mkdir(p, 0123) >= 0);
+
+ assert_se(chmod_and_chown_unsafe(p, S_IFDIR | 0321, 1, 2) >= 0);
+ assert_se(chmod_and_chown_unsafe(p, S_IFREG | 0555, 3, 4) == -EINVAL);
+
+ assert_se(lstat(p, &st) >= 0);
+ assert_se(S_ISDIR(st.st_mode));
+ assert_se((st.st_mode & 07777) == 0321);
+
+ p = strjoina(d, "/lnk");
+ assert_se(symlink("idontexist", p) >= 0);
+
+ assert_se(chmod_and_chown_unsafe(p, S_IFLNK | 0321, 1, 2) >= 0);
+ assert_se(chmod_and_chown_unsafe(p, S_IFREG | 0555, 3, 4) == -EINVAL);
+ assert_se(chmod_and_chown_unsafe(p, S_IFDIR | 0555, 3, 4) == -EINVAL);
+
+ assert_se(lstat(p, &st) >= 0);
+ assert_se(S_ISLNK(st.st_mode));
+}
+
int main(int argc, char *argv[]) {
test_setup_logging(LOG_INFO);
@@ -819,6 +863,7 @@ int main(int argc, char *argv[]) {
test_fsync_directory_of_file();
test_rename_noreplace();
test_chmod_and_chown();
+ test_chmod_and_chown_unsafe();
return 0;
}

113
0003-sysusers-do-not-require-proc-to-be-mounted.patch Normal file
View File

@ -0,0 +1,113 @@
From 1fb5a5edc7c175ea0cd85a1e3a5af8d54084a891 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 3 Mar 2020 11:58:07 +0100
Subject: [PATCH 3/3] sysusers: do not require /proc to be mounted
We're operating on known paths in root-owned directories here, so the detour
through toctou-safe methods that require /proc to be mounted is not necessary.
Should fix https://bugzilla.redhat.com/show_bug.cgi?id=1807768.
---
src/sysusers/sysusers.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c
index 1b1f19e817..f7cc7e0900 100644
--- a/src/sysusers/sysusers.c
+++ b/src/sysusers/sysusers.c
@@ -199,7 +199,7 @@ static int load_group_database(void) {
static int make_backup(const char *target, const char *x) {
_cleanup_close_ int src = -1;
_cleanup_fclose_ FILE *dst = NULL;
- _cleanup_free_ char *temp = NULL;
+ _cleanup_free_ char *dst_tmp = NULL;
char *backup;
struct timespec ts[2];
struct stat st;
@@ -216,7 +216,7 @@ static int make_backup(const char *target, const char *x) {
if (fstat(src, &st) < 0)
return -errno;
- r = fopen_temporary_label(target, x, &dst, &temp);
+ r = fopen_temporary_label(target, x, &dst, &dst_tmp);
if (r < 0)
return r;
@@ -230,7 +230,7 @@ static int make_backup(const char *target, const char *x) {
backup = strjoina(x, "-");
/* Copy over the access mask */
- r = fchmod_and_chown(fileno(dst), st.st_mode & 07777, st.st_uid, st.st_gid);
+ r = chmod_and_chown_unsafe(dst_tmp, st.st_mode & 07777, st.st_uid, st.st_gid);
if (r < 0)
log_warning_errno(r, "Failed to change access mode or ownership of %s: %m", backup);
@@ -243,7 +243,7 @@ static int make_backup(const char *target, const char *x) {
if (r < 0)
goto fail;
- if (rename(temp, backup) < 0) {
+ if (rename(dst_tmp, backup) < 0) {
r = -errno;
goto fail;
}
@@ -251,7 +251,7 @@ static int make_backup(const char *target, const char *x) {
return 0;
fail:
- (void) unlink(temp);
+ (void) unlink(dst_tmp);
return r;
}
@@ -345,13 +345,13 @@ static int putsgent_with_members(const struct sgrp *sg, FILE *gshadow) {
}
#endif
-static int sync_rights(FILE *from, FILE *to) {
+static int sync_rights(FILE *from, const char *to) {
struct stat st;
if (fstat(fileno(from), &st) < 0)
return -errno;
- return fchmod_and_chown(fileno(to), st.st_mode & 07777, st.st_uid, st.st_gid);
+ return chmod_and_chown_unsafe(to, st.st_mode & 07777, st.st_uid, st.st_gid);
}
static int rename_and_apply_smack(const char *temp_path, const char *dest_path) {
@@ -389,7 +389,7 @@ static int write_temporary_passwd(const char *passwd_path, FILE **tmpfile, char
original = fopen(passwd_path, "re");
if (original) {
- r = sync_rights(original, passwd);
+ r = sync_rights(original, passwd_tmp);
if (r < 0)
return r;
@@ -491,7 +491,7 @@ static int write_temporary_shadow(const char *shadow_path, FILE **tmpfile, char
original = fopen(shadow_path, "re");
if (original) {
- r = sync_rights(original, shadow);
+ r = sync_rights(original, shadow_tmp);
if (r < 0)
return r;
@@ -588,7 +588,7 @@ static int write_temporary_group(const char *group_path, FILE **tmpfile, char **
original = fopen(group_path, "re");
if (original) {
- r = sync_rights(original, group);
+ r = sync_rights(original, group_tmp);
if (r < 0)
return r;
@@ -687,7 +687,7 @@ static int write_temporary_gshadow(const char * gshadow_path, FILE **tmpfile, ch
if (original) {
struct sgrp *sg;
- r = sync_rights(original, gshadow);
+ r = sync_rights(original, gshadow_tmp);
if (r < 0)
return r;

5
systemd.spec
View File

@ -66,6 +66,10 @@ GIT_DIR=../../src/systemd/.git git diffab -M v233..master@{2017-06-15} -- hwdb/[
# https://bugzilla.redhat.com/show_bug.cgi?id=1738828
Patch0001: use-bfq-scheduler.patch
Patch0002: 0001-sysusers-many-different-errnos-to-express-one-condit.patch
Patch0003: 0002-basic-fs-util-add-a-version-of-chmod_and_chown-that-.patch
Patch0004: 0003-sysusers-do-not-require-proc-to-be-mounted.patch
Patch0998: 0998-resolved-create-etc-resolv.conf-symlink-at-runtime.patch
# https://bugzilla.redhat.com/show_bug.cgi?id=1803293
@ -772,6 +776,7 @@ fi
group lookup failure in nss_systemd (#1809147)
- Drop autogenerated BOOT_IMAGE= parameter from stored kernel command lines
(#1716164)
- Don't require /proc to be mounted for systemd-sysusers to work (#1807768)
* Fri Feb 21 2020 Filipe Brandenburger <filbranden@gmail.com> - 245~rc1-4
- Update daemon-reexec fallback to check whether the system is booted with