From 21c7bc1dad196e2b2f830d848a0a3413d649e0b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 Dec 2021 18:33:32 +0100 Subject: [PATCH 1/6] gitignore: remove patterns for build files in source dir Those made sense when autotoolz were used. With meson, everything should land in the build dir, and this is only likely to obscure issues with custom build rules. C.f. https://github.com/systemd/systemd/pull/20429#issuecomment-988132332 --- .gitignore | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.gitignore b/.gitignore index 3e65a61ea3..b3aba0921b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,11 +1,6 @@ -*.a *.cache -*.gch -*.log -*.o *.plist *.py[co] -*.stamp *.swp *.trs *~ From 3f316701d7488de018499d72cdf2a9139fbd4ca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 8 Dec 2021 10:19:53 +0100 Subject: [PATCH 2/6] sysusers: use uppercase UID/GID consistently in messages --- src/sysusers/sysusers.c | 52 ++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 0011ff4770..2189ab7379 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -60,8 +60,8 @@ typedef struct Item { bool gid_set:1; - /* When set the group with the specified gid must exist - * and the check if a uid clashes with the gid is skipped. + /* When set the group with the specified GID must exist + * and the check if a UID clashes with the GID is skipped. */ bool id_set_strict:1; @@ -969,13 +969,13 @@ static int read_id_from_file(Item *i, uid_t *_uid, gid_t *_gid) { assert(i); - /* First, try to get the gid directly */ + /* First, try to get the GID directly */ if (_gid && i->gid_path && root_stat(i->gid_path, &st) >= 0) { gid = st.st_gid; found_gid = true; } - /* Then, try to get the uid directly */ + /* Then, try to get the UID directly */ if ((_uid || (_gid && !found_gid)) && i->uid_path && root_stat(i->uid_path, &st) >= 0) { @@ -983,14 +983,14 @@ static int read_id_from_file(Item *i, uid_t *_uid, gid_t *_gid) { uid = st.st_uid; found_uid = true; - /* If we need the gid, but had no success yet, also derive it from the uid path */ + /* If we need the gid, but had no success yet, also derive it from the UID path */ if (_gid && !found_gid) { gid = st.st_gid; found_gid = true; } } - /* If that didn't work yet, then let's reuse the gid as uid */ + /* If that didn't work yet, then let's reuse the GID as UID */ if (_uid && !found_uid && i->gid_path) { if (found_gid) { @@ -1055,11 +1055,11 @@ static int add_user(Item *i) { return log_error_errno(errno, "Failed to check if user %s already exists: %m", i->name); } - /* Try to use the suggested numeric uid */ + /* Try to use the suggested numeric UID */ if (i->uid_set) { r = uid_is_ok(i->uid, i->name, !i->id_set_strict); if (r < 0) - return log_error_errno(r, "Failed to verify uid " UID_FMT ": %m", i->uid); + return log_error_errno(r, "Failed to verify UID " UID_FMT ": %m", i->uid); if (r == 0) { log_debug("Suggested user ID " UID_FMT " for %s already used.", i->uid, i->name); i->uid_set = false; @@ -1077,7 +1077,7 @@ static int add_user(Item *i) { else { r = uid_is_ok(c, i->name, true); if (r < 0) - return log_error_errno(r, "Failed to verify uid " UID_FMT ": %m", i->uid); + return log_error_errno(r, "Failed to verify UID " UID_FMT ": %m", i->uid); else if (r > 0) { i->uid = c; i->uid_set = true; @@ -1091,7 +1091,7 @@ static int add_user(Item *i) { if (!i->uid_set && i->gid_set) { r = uid_is_ok((uid_t) i->gid, i->name, true); if (r < 0) - return log_error_errno(r, "Failed to verify uid " UID_FMT ": %m", i->uid); + return log_error_errno(r, "Failed to verify UID " UID_FMT ": %m", i->uid); if (r > 0) { i->uid = (uid_t) i->gid; i->uid_set = true; @@ -1109,7 +1109,7 @@ static int add_user(Item *i) { r = uid_is_ok(search_uid, i->name, true); if (r < 0) - return log_error_errno(r, "Failed to verify uid " UID_FMT ": %m", i->uid); + return log_error_errno(r, "Failed to verify UID " UID_FMT ": %m", i->uid); else if (r > 0) break; } @@ -1120,16 +1120,16 @@ static int add_user(Item *i) { r = ordered_hashmap_ensure_put(&todo_uids, NULL, UID_TO_PTR(i->uid), i); if (r == -EEXIST) - return log_error_errno(r, "Requested user %s with uid " UID_FMT " and gid" GID_FMT " to be created is duplicated " + return log_error_errno(r, "Requested user %s with UID " UID_FMT " and gid" GID_FMT " to be created is duplicated " "or conflicts with another user.", i->name, i->uid, i->gid); if (r == -ENOMEM) return log_oom(); if (r < 0) - return log_error_errno(r, "Failed to store user %s with uid " UID_FMT " and gid " GID_FMT " to be created: %m", + return log_error_errno(r, "Failed to store user %s with UID " UID_FMT " and GID " GID_FMT " to be created: %m", i->name, i->uid, i->gid); i->todo_user = true; - log_info("Creating user %s (%s) with uid " UID_FMT " and gid " GID_FMT ".", + log_info("Creating user %s (%s) with UID " UID_FMT " and GID " GID_FMT ".", i->name, strna(i->description), i->uid, i->gid); return 0; @@ -1214,15 +1214,15 @@ static int add_group(Item *i) { return 0; } - /* Try to use the suggested numeric gid */ + /* Try to use the suggested numeric GID */ if (i->gid_set) { r = gid_is_ok(i->gid); if (r < 0) - return log_error_errno(r, "Failed to verify gid " GID_FMT ": %m", i->gid); + return log_error_errno(r, "Failed to verify GID " GID_FMT ": %m", i->gid); if (i->id_set_strict) { - /* If we require the gid to already exist we can return here: - * r > 0: means the gid does not exist -> fail - * r == 0: means the gid exists -> nothing more to do. + /* If we require the GID to already exist we can return here: + * r > 0: means the GID does not exist -> fail + * r == 0: means the GID exists -> nothing more to do. */ if (r > 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), @@ -1241,7 +1241,7 @@ static int add_group(Item *i) { if (!i->gid_set && i->uid_set) { r = gid_is_ok((gid_t) i->uid); if (r < 0) - return log_error_errno(r, "Failed to verify gid " GID_FMT ": %m", i->gid); + return log_error_errno(r, "Failed to verify GID " GID_FMT ": %m", i->gid); if (r > 0) { i->gid = (gid_t) i->uid; i->gid_set = true; @@ -1259,7 +1259,7 @@ static int add_group(Item *i) { else { r = gid_is_ok(c); if (r < 0) - return log_error_errno(r, "Failed to verify gid " GID_FMT ": %m", i->gid); + return log_error_errno(r, "Failed to verify GID " GID_FMT ": %m", i->gid); else if (r > 0) { i->gid = c; i->gid_set = true; @@ -1281,7 +1281,7 @@ static int add_group(Item *i) { r = gid_is_ok(search_uid); if (r < 0) - return log_error_errno(r, "Failed to verify gid " GID_FMT ": %m", i->gid); + return log_error_errno(r, "Failed to verify GID " GID_FMT ": %m", i->gid); else if (r > 0) break; } @@ -1292,14 +1292,14 @@ static int add_group(Item *i) { r = ordered_hashmap_ensure_put(&todo_gids, NULL, GID_TO_PTR(i->gid), i); if (r == -EEXIST) - return log_error_errno(r, "Requested group %s with gid "GID_FMT " to be created is duplicated or conflicts with another user.", i->name, i->gid); + return log_error_errno(r, "Requested group %s with GID "GID_FMT " to be created is duplicated or conflicts with another user.", i->name, i->gid); if (r == -ENOMEM) return log_oom(); if (r < 0) - return log_error_errno(r, "Failed to store group %s with gid " GID_FMT " to be created: %m", i->name, i->gid); + return log_error_errno(r, "Failed to store group %s with GID " GID_FMT " to be created: %m", i->name, i->gid); i->todo_group = true; - log_info("Creating group %s with gid " GID_FMT ".", i->name, i->gid); + log_info("Creating group %s with GID " GID_FMT ".", i->name, i->gid); return 0; } @@ -2061,7 +2061,7 @@ static int run(int argc, char *argv[]) { login_defs_need_warning = true; /* We pick a range that very conservative: we look at compiled-in maximum and the value in - * /etc/login.defs. That way the uids/gids which we allocate will be interpreted correctly, + * /etc/login.defs. That way the UIDs/GIDs which we allocate will be interpreted correctly, * even if /etc/login.defs is removed later. (The bottom bound doesn't matter much, since * it's only used during allocation, so we use the configured value directly). */ uid_t begin = login_defs.system_alloc_uid_min, From 98167876a0d63f157d5fb6e90e8fd6fbdb278846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 8 Dec 2021 10:44:00 +0100 Subject: [PATCH 3/6] sysusers: use ordered_hashmap_isempty() --- src/sysusers/sysusers.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 2189ab7379..4299a75614 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -387,7 +387,7 @@ static int write_temporary_passwd(const char *passwd_path, FILE **tmpfile, char Item *i; int r; - if (ordered_hashmap_size(todo_uids) == 0) + if (ordered_hashmap_isempty(todo_uids)) return 0; r = fopen_temporary_label("/etc/passwd", passwd_path, &passwd, &passwd_tmp); @@ -506,7 +506,7 @@ static int write_temporary_shadow(const char *shadow_path, FILE **tmpfile, char Item *i; int r; - if (ordered_hashmap_size(todo_uids) == 0) + if (ordered_hashmap_isempty(todo_uids)) return 0; r = fopen_temporary_label("/etc/shadow", shadow_path, &shadow, &shadow_tmp); @@ -639,7 +639,7 @@ static int write_temporary_group(const char *group_path, FILE **tmpfile, char ** Item *i; int r; - if (ordered_hashmap_size(todo_gids) == 0 && ordered_hashmap_size(members) == 0) + if (ordered_hashmap_isempty(todo_gids) && ordered_hashmap_isempty(members)) return 0; r = fopen_temporary_label("/etc/group", group_path, &group, &group_tmp); @@ -740,7 +740,7 @@ static int write_temporary_gshadow(const char * gshadow_path, FILE **tmpfile, ch Item *i; int r; - if (ordered_hashmap_size(todo_gids) == 0 && ordered_hashmap_size(members) == 0) + if (ordered_hashmap_isempty(todo_gids) && ordered_hashmap_isempty(members)) return 0; r = fopen_temporary_label("/etc/gshadow", gshadow_path, &gshadow, &gshadow_tmp); From 64fe10957b9fdce55d59c2af101e004c93997419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 8 Dec 2021 10:33:53 +0100 Subject: [PATCH 4/6] sysusers: add --dry-run --- man/systemd-sysusers.xml | 6 ++++++ src/sysusers/sysusers.c | 38 ++++++++++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/man/systemd-sysusers.xml b/man/systemd-sysusers.xml index cee8e6ef54..3ad8135dcd 100644 --- a/man/systemd-sysusers.xml +++ b/man/systemd-sysusers.xml @@ -115,6 +115,12 @@ + + + Process the configuration and figure out what entries would be created, but don't + actually write anything. + + Treat each positional argument as a separate configuration diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 4299a75614..a2ffeb96c6 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -75,6 +75,7 @@ static char *arg_root = NULL; static char *arg_image = NULL; static bool arg_cat_config = false; static const char *arg_replace = NULL; +static bool arg_dry_run = false; static bool arg_inline = false; static PagerFlags arg_pager_flags = 0; @@ -390,6 +391,11 @@ static int write_temporary_passwd(const char *passwd_path, FILE **tmpfile, char if (ordered_hashmap_isempty(todo_uids)) return 0; + if (arg_dry_run) { + log_info("Would write /etc/passwd…"); + return 0; + } + r = fopen_temporary_label("/etc/passwd", passwd_path, &passwd, &passwd_tmp); if (r < 0) return log_debug_errno(r, "Failed to open temporary copy of %s: %m", passwd_path); @@ -509,6 +515,11 @@ static int write_temporary_shadow(const char *shadow_path, FILE **tmpfile, char if (ordered_hashmap_isempty(todo_uids)) return 0; + if (arg_dry_run) { + log_info("Would write /etc/shadow…"); + return 0; + } + r = fopen_temporary_label("/etc/shadow", shadow_path, &shadow, &shadow_tmp); if (r < 0) return log_debug_errno(r, "Failed to open temporary copy of %s: %m", shadow_path); @@ -642,6 +653,11 @@ static int write_temporary_group(const char *group_path, FILE **tmpfile, char ** if (ordered_hashmap_isempty(todo_gids) && ordered_hashmap_isempty(members)) return 0; + if (arg_dry_run) { + log_info("Would write /etc/group…"); + return 0; + } + r = fopen_temporary_label("/etc/group", group_path, &group, &group_tmp); if (r < 0) return log_debug_errno(r, "Failed to open temporary copy of %s: %m", group_path); @@ -743,6 +759,11 @@ static int write_temporary_gshadow(const char * gshadow_path, FILE **tmpfile, ch if (ordered_hashmap_isempty(todo_gids) && ordered_hashmap_isempty(members)) return 0; + if (arg_dry_run) { + log_info("Would write /etc/gshadow…"); + return 0; + } + r = fopen_temporary_label("/etc/gshadow", gshadow_path, &gshadow, &gshadow_tmp); if (r < 0) return log_debug_errno(r, "Failed to open temporary copy of %s: %m", gshadow_path); @@ -1825,6 +1846,7 @@ static int help(void) { " --root=PATH Operate on an alternate filesystem root\n" " --image=PATH Operate on disk image as filesystem root\n" " --replace=PATH Treat arguments as replacement for PATH\n" + " --dry-run Just print what would be done\n" " --inline Treat arguments as configuration lines\n" " --no-pager Do not pipe output into a pager\n" "\nSee the %s for details.\n", @@ -1842,6 +1864,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_ROOT, ARG_IMAGE, ARG_REPLACE, + ARG_DRY_RUN, ARG_INLINE, ARG_NO_PAGER, }; @@ -1851,8 +1874,9 @@ static int parse_argv(int argc, char *argv[]) { { "version", no_argument, NULL, ARG_VERSION }, { "cat-config", no_argument, NULL, ARG_CAT_CONFIG }, { "root", required_argument, NULL, ARG_ROOT }, - { "image", required_argument, NULL, ARG_IMAGE }, + { "image", required_argument, NULL, ARG_IMAGE }, { "replace", required_argument, NULL, ARG_REPLACE }, + { "dry-run", no_argument, NULL, ARG_DRY_RUN }, { "inline", no_argument, NULL, ARG_INLINE }, { "no-pager", no_argument, NULL, ARG_NO_PAGER }, {} @@ -1903,6 +1927,10 @@ static int parse_argv(int argc, char *argv[]) { arg_replace = optarg; break; + case ARG_DRY_RUN: + arg_dry_run = true; + break; + case ARG_INLINE: arg_inline = true; break; @@ -2077,9 +2105,11 @@ static int run(int argc, char *argv[]) { if (r < 0) return r; - lock = take_etc_passwd_lock(arg_root); - if (lock < 0) - return log_error_errno(lock, "Failed to take /etc/passwd lock: %m"); + if (!arg_dry_run) { + lock = take_etc_passwd_lock(arg_root); + if (lock < 0) + return log_error_errno(lock, "Failed to take /etc/passwd lock: %m"); + } r = load_user_database(); if (r < 0) From 9a5af4b7ac16368ae14f26735657e93cce46f065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 8 Dec 2021 11:21:17 +0100 Subject: [PATCH 5/6] sysusers: improve message about conflicting entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without any markup, the sentence could be quite confusing: g user 55 g user 56 → "Two or more conflicting lines for user configured" It also wasn't clear which line is ignored. Inspired by https://github.com/systemd/systemd/pull/21156. --- src/sysusers/sysusers.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index a2ffeb96c6..855096020d 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -38,12 +38,27 @@ #include "util.h" typedef enum ItemType { - ADD_USER = 'u', - ADD_GROUP = 'g', + ADD_USER = 'u', + ADD_GROUP = 'g', ADD_MEMBER = 'm', - ADD_RANGE = 'r', + ADD_RANGE = 'r', } ItemType; +static inline const char* item_type_to_string(ItemType t) { + switch (t) { + case ADD_USER: + return "user"; + case ADD_GROUP: + return "group"; + case ADD_MEMBER: + return "member"; + case ADD_RANGE: + return "range"; + default: + assert_not_reached(); + } +} + typedef struct Item { ItemType type; @@ -1150,7 +1165,7 @@ static int add_user(Item *i) { i->name, i->uid, i->gid); i->todo_user = true; - log_info("Creating user %s (%s) with UID " UID_FMT " and GID " GID_FMT ".", + log_info("Creating user '%s' (%s) with UID " UID_FMT " and GID " GID_FMT ".", i->name, strna(i->description), i->uid, i->gid); return 0; @@ -1320,7 +1335,7 @@ static int add_group(Item *i) { return log_error_errno(r, "Failed to store group %s with GID " GID_FMT " to be created: %m", i->name, i->gid); i->todo_group = true; - log_info("Creating group %s with GID " GID_FMT ".", i->name, i->gid); + log_info("Creating group '%s' with GID " GID_FMT ".", i->name, i->gid); return 0; } @@ -1484,7 +1499,6 @@ static bool item_equal(Item *a, Item *b) { } static int parse_line(const char *fname, unsigned line, const char *buffer) { - _cleanup_free_ char *action = NULL, *name = NULL, *resolved_name = NULL, *id = NULL, *resolved_id = NULL, @@ -1749,7 +1763,9 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (existing) { /* Two identical items are fine */ if (!item_equal(existing, i)) - log_warning("Two or more conflicting lines for %s configured, ignoring.", i->name); + log_warning("%s:%u: conflict with earlier configuration for %s '%s', ignoring line.", + fname, line, + item_type_to_string(i->type), i->name); return 0; } From 5374bc7cb4383abb1b3ae84a041eca93b259e4ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 8 Dec 2021 11:46:09 +0100 Subject: [PATCH 6/6] sysusers: raise level of message about UID/GID being already used It can be quite important that a different value was used, so let's log this by default. --- src/sysusers/sysusers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 855096020d..c3a9e54f3f 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -1097,7 +1097,7 @@ static int add_user(Item *i) { if (r < 0) return log_error_errno(r, "Failed to verify UID " UID_FMT ": %m", i->uid); if (r == 0) { - log_debug("Suggested user ID " UID_FMT " for %s already used.", i->uid, i->name); + log_info("Suggested user ID " UID_FMT " for %s already used.", i->uid, i->name); i->uid_set = false; } } @@ -1268,7 +1268,7 @@ static int add_group(Item *i) { return 0; } if (r == 0) { - log_debug("Suggested group ID " GID_FMT " for %s already used.", i->gid, i->name); + log_info("Suggested group ID " GID_FMT " for %s already used.", i->gid, i->name); i->gid_set = false; } }