From d1e4b8fd963ba43190ec824a803819612ff80441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 29 Jan 2018 14:23:31 +0100 Subject: [PATCH 1/8] sysusers: emit a bit more info at debug level when locking fails This is the first error message when running unprivileged, and the message is unspecific, so let's at least add some logging at debug level to make this less confusing. --- src/basic/user-util.c | 8 ++++---- src/basic/user-util.h | 2 ++ src/sysusers/sysusers.c | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/basic/user-util.c b/src/basic/user-util.c index 17a9b5a8f1..011b29ad02 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -553,18 +553,18 @@ int take_etc_passwd_lock(const char *root) { * awfully racy, and thus we just won't do them. */ if (root) - path = prefix_roota(root, "/etc/.pwd.lock"); + path = prefix_roota(root, ETC_PASSWD_LOCK_PATH); else - path = "/etc/.pwd.lock"; + path = ETC_PASSWD_LOCK_PATH; fd = open(path, O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW, 0600); if (fd < 0) - return -errno; + return log_debug_errno(errno, "Cannot open %s: %m", path); r = fcntl(fd, F_SETLKW, &flock); if (r < 0) { safe_close(fd); - return -errno; + return log_debug_errno(errno, "Locking %s failed: %m", path); } return fd; diff --git a/src/basic/user-util.h b/src/basic/user-util.h index 5f0391f2b8..290d17a0c6 100644 --- a/src/basic/user-util.h +++ b/src/basic/user-util.h @@ -63,6 +63,8 @@ int take_etc_passwd_lock(const char *root); #define UID_NOBODY ((uid_t) 65534U) #define GID_NOBODY ((gid_t) 65534U) +#define ETC_PASSWD_LOCK_PATH "/etc/.pwd.lock" + static inline bool uid_is_dynamic(uid_t uid) { return DYNAMIC_UID_MIN <= uid && uid <= DYNAMIC_UID_MAX; } diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 510d5fa59e..288f6a1665 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -1841,7 +1841,7 @@ int main(int argc, char *argv[]) { lock = take_etc_passwd_lock(arg_root); if (lock < 0) { - log_error_errno(lock, "Failed to take lock: %m"); + log_error_errno(lock, "Failed to take /etc/passwd lock: %m"); goto finish; } From 1b600bd522d2c01c493729cdda4bcc2e01203e98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 29 Jan 2018 14:47:01 +0100 Subject: [PATCH 2/8] sysusers: take configuration as positional arguments If the configuration is included in a script, this is more convient. I thought it would be possible to use this for rpm scriptlets with '%pre -p systemd-sysuser "..."', but apparently there is no way to pass arguments to the executable ($1 is used for the package installation count). But this functionality seems generally useful, e.g. for testing and one-off scripts, so let's keep it. There's a slight change in behaviour when files are given on the command line: if we cannot parse them, error out instead of ignoring the failure. When trying to parse all configuration files, we don't want to fail even if some config files are broken, but when parsing a list of items specified explicitly, we should. v2: - rename --direct to --inline --- man/systemd-sysusers.xml | 6 ++++++ src/sysusers/sysusers.c | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/man/systemd-sysusers.xml b/man/systemd-sysusers.xml index 73ba4e4a84..7816356889 100644 --- a/man/systemd-sysusers.xml +++ b/man/systemd-sysusers.xml @@ -94,6 +94,12 @@ paths. + + + Treat each positional argument as a separate configuration + line instead of a file name. + + diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 288f6a1665..340e0db798 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -74,6 +74,7 @@ typedef struct Item { } Item; static char *arg_root = NULL; +static bool arg_inline = false; static const char conf_file_dirs[] = CONF_PATHS_NULSTR("sysusers.d"); @@ -1718,6 +1719,7 @@ static void help(void) { " -h --help Show this help\n" " --version Show package version\n" " --root=PATH Operate on an alternate filesystem root\n" + " --inline Treat arguments as configuration lines\n" , program_invocation_short_name); } @@ -1726,12 +1728,14 @@ static int parse_argv(int argc, char *argv[]) { enum { ARG_VERSION = 0x100, ARG_ROOT, + ARG_INLINE, }; static const struct option options[] = { { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, ARG_VERSION }, { "root", required_argument, NULL, ARG_ROOT }, + { "inline", no_argument, NULL, ARG_INLINE }, {} }; @@ -1757,6 +1761,10 @@ static int parse_argv(int argc, char *argv[]) { return r; break; + case ARG_INLINE: + arg_inline = true; + break; + case '?': return -EINVAL; @@ -1795,9 +1803,13 @@ int main(int argc, char *argv[]) { int j; for (j = optind; j < argc; j++) { - k = read_config_file(argv[j], false); - if (k < 0 && r == 0) - r = k; + if (arg_inline) + /* Use (argument):n, where n==1 for the first positional arg */ + r = parse_line("(argument)", j - optind + 1, argv[j]); + else + r = read_config_file(argv[j], false); + if (r < 0) + goto finish; } } else { _cleanup_strv_free_ char **files = NULL; From 7b1aaf6633cad80c1e59eeedaf60595a3ec1efc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 30 Jan 2018 14:28:10 +0100 Subject: [PATCH 3/8] sysusers: allow the shell to be specified This is necessary for some system users where the "login shell" is set to a specific binary. --- man/sysusers.d.xml | 89 +++++++++++++++++++++++------------------ src/basic/user-util.c | 2 + src/basic/user-util.h | 9 +++++ src/sysusers/sysusers.c | 75 +++++++++++++++++++++++----------- 4 files changed, 111 insertions(+), 64 deletions(-) diff --git a/man/sysusers.d.xml b/man/sysusers.d.xml index c0d8a1682a..47f018f402 100644 --- a/man/sysusers.d.xml +++ b/man/sysusers.d.xml @@ -57,11 +57,14 @@ Description - systemd-sysusers uses the files from sysusers.d directory to create - system users and groups at package installation or boot time. This tool may be used to allocate system users and - groups only, it is not useful for creating non-system (i.e. regular, "human") users and groups, as it accesses - /etc/passwd and /etc/group directly, bypassing any more complex user - databases, for example any database involving NIS or LDAP. + systemd-sysusers uses the files from + sysusers.d directory to create system users and groups and + to add users to groups, at package installation or boot time. This tool may be + used to allocate system users and groups only, it is not useful for creating + non-system (i.e. regular, "human") users and groups, as it accesses + /etc/passwd and /etc/group directly, + bypassing any more complex user databases, for example any database involving NIS + or LDAP. @@ -100,15 +103,16 @@ Configuration File Format - The file format is one line per user or group containing - name, ID, GECOS field description and home directory: + The file format is one line per user or group containing name, ID, GECOS + field description, home directory, and login shell: - #Type Name ID GECOS Home directory -u httpd 440 "HTTP User" -u authd /usr/bin/authd "Authorization user" -g input - - -m authd input -u root 0 "Superuser" /root + #Type Name ID GECOS Home directory Shell +u httpd 404 "HTTP User" +u authd /usr/bin/authd "Authorization user" +u postgres - "Postgresql Database" /var/lib/pgsql /usr/libexec/postgresdb +g input - - +m authd input +u root 0 "Superuser" /root /bin/zsh Empty lines and lines beginning with the # character are ignored, and may be used for commenting. @@ -122,14 +126,10 @@ u root 0 "Superuser" /root u - Create a system user and group of the - specified name should they not exist yet. The user's primary - group will be set to the group bearing the same name. The - user's shell will be set to - /sbin/nologin, the home directory to - the specified home directory, or / if - none is given. The account will be created disabled, so that - logins are not allowed. + Create a system user and group of the specified name should + they not exist yet. The user's primary group will be set to the group + bearing the same name. The account will be created disabled, so that logins + are not allowed. @@ -187,7 +187,8 @@ u root 0 "Superuser" /root numeric 32-bit UID or GID of the user/group. Do not use IDs 65535 or 4294967295, as they have special placeholder meanings. Specify - for automatic UID/GID allocation - for the user or group. Alternatively, specify an absolute path + for the user or group (this is strongly recommended unless it is strictly + necessary to use a specific UID or GID). Alternatively, specify an absolute path in the file system. In this case, the UID/GID is read from the path's owner/group. This is useful to create users whose UID/GID match the owners of pre-existing files (such as SUID or SGID @@ -209,37 +210,45 @@ u root 0 "Superuser" /root GECOS - A short, descriptive string for users to be created, - enclosed in quotation marks. Note that this field may not - contain colons. + A short, descriptive string for users to be created, enclosed in + quotation marks. Note that this field may not contain colons. - Only applies to lines of type u and - should otherwise be left unset, or be set to - -. + Only applies to lines of type u and should otherwise + be left unset (or -). Home Directory - The home directory for a new system user. If omitted, - defaults to the root directory. It is recommended to not - unnecessarily specify home directories for system users, unless - software strictly requires one to be set. + The home directory for a new system user. If omitted, defaults to the + root directory. - Only applies to lines of type u and - should otherwise be left unset, or be set to - -. + Only applies to lines of type u and should otherwise + be left unset (or -). It is recommended to omit this, unless + software strictly requires a home directory to be set. + + + + Shell + + The login shell of the user. If not specified, this will be set to + /sbin/nologin, except if the UID of the user is 0, in + which case /bin/sh will be used. + + Only applies to lines of type u and should otherwise + be left unset (or -). It is recommended to omit this, unless + a shell different /sbin/nologin must be used. Idempotence - Note that systemd-sysusers will do - nothing if the specified users or groups already exist, so - normally, there is no reason to override - sysusers.d vendor configuration, except to - block certain users or groups from being created. + Note that systemd-sysusers will do nothing if the + specified users or groups already exist or the users are members of specified + groups, so normally there is no reason to override + sysusers.d vendor configuration, except to block certain + users or groups from being created. diff --git a/src/basic/user-util.c b/src/basic/user-util.c index 011b29ad02..db18ee31c0 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -645,6 +645,8 @@ bool valid_gecos(const char *d) { } bool valid_home(const char *p) { + /* Note that this function is also called by valid_shell(), any + * changes must account for that. */ if (isempty(p)) return false; diff --git a/src/basic/user-util.h b/src/basic/user-util.h index 290d17a0c6..e1259a1582 100644 --- a/src/basic/user-util.h +++ b/src/basic/user-util.h @@ -98,6 +98,15 @@ bool valid_user_group_name_or_id(const char *u); bool valid_gecos(const char *d); bool valid_home(const char *p); +static inline bool valid_shell(const char *p) { + /* We have the same requirements, so just piggy-back on the home check. + * + * Let's ignore /etc/shells because this is only applicable to real and + * not system users. It is also incompatible with the idea of empty /etc. + */ + return valid_home(p); +} + int maybe_setgroups(size_t size, const gid_t *list); bool synthesize_nobody(void); diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 340e0db798..e2a3b9968c 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -59,6 +59,7 @@ typedef struct Item { char *gid_path; char *description; char *home; + char *shell; gid_t gid; uid_t uid; @@ -384,6 +385,10 @@ static int rename_and_apply_smack(const char *temp_path, const char *dest_path) return r; } +static const char* default_shell(uid_t uid) { + return uid == 0 ? "/bin/sh" : "/sbin/nologin"; +} + static int write_temporary_passwd(const char *passwd_path, FILE **tmpfile, char **tmpfile_path) { _cleanup_fclose_ FILE *original = NULL, *passwd = NULL; _cleanup_(unlink_and_freep) char *passwd_tmp = NULL; @@ -451,7 +456,7 @@ static int write_temporary_passwd(const char *passwd_path, FILE **tmpfile, char /* Initialize the shell to nologin, with one exception: * for root we patch in something special */ - .pw_shell = i->uid == 0 ? (char*) "/bin/sh" : (char*) "/sbin/nologin", + .pw_shell = i->shell ?: (char*) default_shell(i->uid), }; errno = 0; @@ -1225,6 +1230,7 @@ static void item_free(Item *i) { free(i->gid_path); free(i->description); free(i->home); + free(i->shell); free(i); } @@ -1332,6 +1338,9 @@ static bool item_equal(Item *a, Item *b) { if (!streq_ptr(a->home, b->home)) return false; + if (!streq_ptr(a->shell, b->shell)) + return false; + return true; } @@ -1345,7 +1354,12 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { {} }; - _cleanup_free_ char *action = NULL, *name = NULL, *id = NULL, *resolved_name = NULL, *resolved_id = NULL, *description = NULL, *home = NULL; + _cleanup_free_ char *action = NULL, + *name = NULL, *resolved_name = NULL, + *id = NULL, *resolved_id = NULL, + *description = NULL, + *home = NULL, + *shell, *resolved_shell = NULL; _cleanup_(item_freep) Item *i = NULL; Item *existing; OrderedHashmap *h; @@ -1358,7 +1372,8 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { /* Parse columns */ p = buffer; - r = extract_many_words(&p, NULL, EXTRACT_QUOTES, &action, &name, &id, &description, &home, NULL); + r = extract_many_words(&p, NULL, EXTRACT_QUOTES, + &action, &name, &id, &description, &home, &shell, NULL); if (r < 0) { log_error("[%s:%u] Syntax error.", fname, line); return r; @@ -1434,6 +1449,24 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { } } + /* Verify shell */ + if (isempty(shell) || streq(shell, "-")) + shell = mfree(shell); + + if (shell) { + r = specifier_printf(shell, specifier_table, NULL, &resolved_shell); + if (r < 0) { + log_error("[%s:%u] Failed to replace specifiers: %s", fname, line, shell); + return r; + } + + if (!valid_shell(resolved_shell)) { + log_error("[%s:%u] '%s' is not a valid login shell field.", fname, line, resolved_shell); + return -EINVAL; + } + } + + switch (action[0]) { case ADD_RANGE: @@ -1447,13 +1480,10 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { return -EINVAL; } - if (description) { - log_error("[%s:%u] Lines of type 'r' don't take a GECOS field.", fname, line); - return -EINVAL; - } - - if (home) { - log_error("[%s:%u] Lines of type 'r' don't take a home directory field.", fname, line); + if (description || home || shell) { + log_error("[%s:%u] Lines of type '%c' don't take a %s field.", + fname, line, action[0], + description ? "GECOS" : home ? "home directory" : "login shell"); return -EINVAL; } @@ -1484,13 +1514,10 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { return -EINVAL; } - if (description) { - log_error("[%s:%u] Lines of type 'm' don't take a GECOS field.", fname, line); - return -EINVAL; - } - - if (home) { - log_error("[%s:%u] Lines of type 'm' don't take a home directory field.", fname, line); + if (description || home || shell) { + log_error("[%s:%u] Lines of type '%c' don't take a %s field.", + fname, line, action[0], + description ? "GECOS" : home ? "home directory" : "login shell"); return -EINVAL; } @@ -1574,6 +1601,9 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { i->home = home; home = NULL; + i->shell = resolved_shell; + resolved_shell = NULL; + h = users; break; @@ -1583,13 +1613,10 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { return -EINVAL; } - if (description) { - log_error("[%s:%u] Lines of type 'g' don't take a GECOS field.", fname, line); - return -EINVAL; - } - - if (home) { - log_error("[%s:%u] Lines of type 'g' don't take a home directory field.", fname, line); + if (description || home || shell) { + log_error("[%s:%u] Lines of type '%c' don't take a %s field.", + fname, line, action[0], + description ? "GECOS" : home ? "home directory" : "login shell"); return -EINVAL; } From 6e888894fc043a023731129ae4eb8ae5633e9f97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Feb 2018 12:50:18 +0100 Subject: [PATCH 4/8] basic/strv: add function to insert items at position --- src/basic/strv.c | 9 ++++++--- src/basic/strv.h | 7 ++++++- src/test/test-strv.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/basic/strv.c b/src/basic/strv.c index 1103245a36..68e2e874b4 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -446,7 +446,7 @@ int strv_push_pair(char ***l, char *a, char *b) { return 0; } -int strv_push_prepend(char ***l, char *value) { +int strv_insert(char ***l, unsigned position, char *value) { char **c; unsigned n, m, i; @@ -454,6 +454,7 @@ int strv_push_prepend(char ***l, char *value) { return 0; n = strv_length(*l); + position = MIN(position, n); /* increase and check for overflow */ m = n + 2; @@ -464,10 +465,12 @@ int strv_push_prepend(char ***l, char *value) { if (!c) return -ENOMEM; - for (i = 0; i < n; i++) + for (i = 0; i < position; i++) + c[i] = (*l)[i]; + c[position] = value; + for (i = position; i < n; i++) c[i+1] = (*l)[i]; - c[0] = value; c[n+1] = NULL; free(*l); diff --git a/src/basic/strv.h b/src/basic/strv.h index 75369d29cb..44fe1f279c 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -54,7 +54,12 @@ int strv_extendf(char ***l, const char *format, ...) _printf_(2,0); int strv_extend_front(char ***l, const char *value); int strv_push(char ***l, char *value); int strv_push_pair(char ***l, char *a, char *b); -int strv_push_prepend(char ***l, char *value); +int strv_insert(char ***l, unsigned position, char *value); + +static inline int strv_push_prepend(char ***l, char *value) { + return strv_insert(l, 0, value); +} + int strv_consume(char ***l, char *value); int strv_consume_pair(char ***l, char *a, char *b); int strv_consume_prepend(char ***l, char *value); diff --git a/src/test/test-strv.c b/src/test/test-strv.c index f78b1bcd46..76cd551eeb 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -447,6 +447,36 @@ static void test_strv_from_stdarg_alloca(void) { test_strv_from_stdarg_alloca_one(STRV_MAKE_EMPTY, NULL); } +static void test_strv_insert(void) { + _cleanup_strv_free_ char **a = NULL; + + assert_se(strv_insert(&a, 0, strdup("first")) == 0); + assert_se(streq(a[0], "first")); + assert_se(!a[1]); + + assert_se(strv_insert(&a, 0, NULL) == 0); + assert_se(streq(a[0], "first")); + assert_se(!a[1]); + + assert_se(strv_insert(&a, 1, strdup("two")) == 0); + assert_se(streq(a[0], "first")); + assert_se(streq(a[1], "two")); + assert_se(!a[2]); + + assert_se(strv_insert(&a, 4, strdup("tri")) == 0); + assert_se(streq(a[0], "first")); + assert_se(streq(a[1], "two")); + assert_se(streq(a[2], "tri")); + assert_se(!a[3]); + + assert_se(strv_insert(&a, 1, strdup("duo")) == 0); + assert_se(streq(a[0], "first")); + assert_se(streq(a[1], "duo")); + assert_se(streq(a[2], "two")); + assert_se(streq(a[3], "tri")); + assert_se(!a[4]); +} + static void test_strv_push_prepend(void) { _cleanup_strv_free_ char **a = NULL; @@ -723,6 +753,7 @@ int main(int argc, char *argv[]) { test_strv_extend(); test_strv_extendf(); test_strv_from_stdarg_alloca(); + test_strv_insert(); test_strv_push_prepend(); test_strv_push(); test_strv_equal(); From d16a1c1bb69e5f36e0eb41ed86670224d2e5ecda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Jan 2018 15:37:02 +0100 Subject: [PATCH 5/8] sysusers: allow admin/runtime overrides to command-line config When used in a package installation script, we want to invoke systemd-sysusers before that package is installed (so it can contain files owned by the newly created user), so the configuration to use is specified on the command line. This should be a copy of the configuration that will be installed as /usr/lib/sysusers.d/package.conf. We still want to obey any overrides in /etc/sysusers.d or /run/sysusers.d in the usual fashion. Otherwise, we'd get a different result when systemd-sysusers is run with a copy of the new config on the command line and when systemd-sysusers is run at boot after package instalation. In the second case any files in /etc or /run have higher priority, so the same should happen when the configuration is given on the command line. More generally, we want the behaviour in this special case to be as close to the case where the file is finally on disk as possible, so we have to read all configuration files, since they all might contain overrides and additional configuration that matters. Even files that have lower priority might specify additional groups for the user we are creating. Thus, we need to read all configuration, but insert our new configuration somewhere with the right priority. If --target=/path/to/file.conf is given on the command line, we gather the list of files, and pretend that the command-line config is read from /path/to/file.conf (doesn't matter if the file on disk actually exists or not). All package scripts should use this option to obtain consistent and idempotent behaviour. The corner case when --target= is specified and there are no positional arguments is disallowed. v1: - version with --config-name= v2: - disallow --config-name= and no positional args v3: - remove --config-name= v4: - add --target= and rework the code completely v5: - fix argcounting bug and add example in man page v6: - rename --target to --replace --- man/systemd-sysusers.xml | 55 +++++++++++++++--- src/basic/conf-files.c | 64 +++++++++++++++++++++ src/basic/conf-files.h | 1 + src/sysusers/sysusers.c | 118 +++++++++++++++++++++++++++++---------- 4 files changed, 200 insertions(+), 38 deletions(-) diff --git a/man/systemd-sysusers.xml b/man/systemd-sysusers.xml index 7816356889..a55d9f6a75 100644 --- a/man/systemd-sysusers.xml +++ b/man/systemd-sysusers.xml @@ -69,15 +69,18 @@ sysusers.d5. - If invoked with no arguments, it applies all directives from - all files found. If one or more filenames are passed on the - command line, only the directives in these files are applied. If - only the basename of a file is specified, all directories as - specified in - sysusers.d5 - are searched for a matching file. If the string - - is specified instead of a filename, entries from the - standard input of the process are read. + If invoked with no arguments, it applies all directives from all files + found in the directories specified by + sysusers.d5. + When invoked with positional arguments, if option + is specified, arguments + specified on the command line are used instead of the configuration file + PATH. Otherwise, just the configuration specified by + the command line arguments is executed. The string - may be + specified instead of a filename to instruct systemd-sysusers + to read the configuration from standard input. If only the basename of a file is + specified, all configuration directories are searched for a matching file and + the file found that has the highest priority is executed. @@ -94,6 +97,40 @@ paths. + + + When this option is given, one ore more positional arguments + must be specified. All configuration files found in the directories listed in + sysusers.d5 + will be read, and the configuration given on the command line will be + handled instead of and with the same priority as the configuration file + PATH. + + This option is intended to be used when package installation scripts + are running and files belonging to that package are not yet available on + disk, so their contents must be given on the command line, but the admin + configuration might already exist and should be given higher priority. + + + + RPM installation script for radvd + + echo 'u radvd - "radvd daemon"' | \ + systemd-sysusers --replace=/usr/lib/sysusers.d/radvd.conf - + + This will create the radvd user as if + /usr/lib/sysusers.d/radvd.conf was already on disk. + An admin might override the configuration specified on the command line by + placing /etc/sysusers.d/radvd.conf or even + /etc/sysusers.d/00-overrides.conf. + + Note that this is the expanded from, and when used in a package, this + would be written using a macro with "radvd" and a file containing the + configuration line as arguments. + + + + Treat each positional argument as a separate configuration diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index c0ac202f57..08ede2c766 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -154,6 +154,70 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const return 0; } +int conf_files_insert(char ***strv, const char *root, const char *dirs, const char *path) { + /* Insert a path into strv, at the place honouring the usual sorting rules: + * - we first compare by the basename + * - and then we compare by dirname, allowing just one file with the given + * basename. + * This means that we will + * - add a new entry if basename(path) was not on the list, + * - do nothing if an entry with higher priority was already present, + * - do nothing if our new entry matches the existing entry, + * - replace the existing entry if our new entry has higher priority. + */ + char *t; + unsigned i; + int r; + + for (i = 0; i < strv_length(*strv); i++) { + int c; + + c = base_cmp(*strv + i, &path); + if (c == 0) { + const char *dir; + + /* Oh, we found our spot and it already contains something. */ + NULSTR_FOREACH(dir, dirs) { + char *p1, *p2; + + p1 = path_startswith((*strv)[i], root); + if (p1) + /* Skip "/" in dir, because p1 is without "/" too */ + p1 = path_startswith(p1, dir + 1); + if (p1) + /* Existing entry with higher priority + * or same priority, no need to do anything. */ + return 0; + + p2 = path_startswith(path, dir); + if (p2) { + /* Our new entry has higher priority */ + t = path_join(root, path, NULL); + if (!t) + return log_oom(); + + return free_and_replace((*strv)[i], t); + } + } + + } else if (c > 0) + /* Following files have lower priority, let's go insert our + * new entry. */ + break; + + /* … we are not there yet, let's continue */ + } + + t = path_join(root, path, NULL); + if (!t) + return log_oom(); + + r = strv_insert(strv, i, t); + if (r < 0) + free(t); + return r; +} + int conf_files_list_strv(char ***strv, const char *suffix, const char *root, unsigned flags, const char* const* dirs) { _cleanup_strv_free_ char **copy = NULL; diff --git a/src/basic/conf-files.h b/src/basic/conf-files.h index 75dfd05e7c..ddee727826 100644 --- a/src/basic/conf-files.h +++ b/src/basic/conf-files.h @@ -28,3 +28,4 @@ enum { int conf_files_list(char ***ret, const char *suffix, const char *root, unsigned flags, const char *dir, ...); int conf_files_list_strv(char ***ret, const char *suffix, const char *root, unsigned flags, const char* const* dirs); int conf_files_list_nulstr(char ***ret, const char *suffix, const char *root, unsigned flags, const char *dirs); +int conf_files_insert(char ***strv, const char *root, const char *dirs, const char *path); diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index e2a3b9968c..af21e3b854 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -75,6 +75,7 @@ typedef struct Item { } Item; static char *arg_root = NULL; +static const char *arg_replace = NULL; static bool arg_inline = false; static const char conf_file_dirs[] = CONF_PATHS_NULSTR("sysusers.d"); @@ -1746,6 +1747,7 @@ static void help(void) { " -h --help Show this help\n" " --version Show package version\n" " --root=PATH Operate on an alternate filesystem root\n" + " --replace=PATH Treat arguments as replacement for PATH\n" " --inline Treat arguments as configuration lines\n" , program_invocation_short_name); } @@ -1755,6 +1757,7 @@ static int parse_argv(int argc, char *argv[]) { enum { ARG_VERSION = 0x100, ARG_ROOT, + ARG_REPLACE, ARG_INLINE, }; @@ -1762,6 +1765,7 @@ static int parse_argv(int argc, char *argv[]) { { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, ARG_VERSION }, { "root", required_argument, NULL, ARG_ROOT }, + { "replace", required_argument, NULL, ARG_REPLACE }, { "inline", no_argument, NULL, ARG_INLINE }, {} }; @@ -1788,6 +1792,16 @@ static int parse_argv(int argc, char *argv[]) { return r; break; + case ARG_REPLACE: + if (!path_is_absolute(optarg) || + !endswith(optarg, ".conf")) { + log_error("The argument to --replace= must an absolute path to a config file"); + return -EINVAL; + } + + arg_replace = optarg; + break; + case ARG_INLINE: arg_inline = true; break; @@ -1799,14 +1813,76 @@ static int parse_argv(int argc, char *argv[]) { assert_not_reached("Unhandled option"); } + if (arg_replace && optind >= argc) { + log_error("When --replace= is given, some configuration items must be specified"); + return -EINVAL; + } + return 1; } +static int parse_arguments(char **args) { + char **arg; + unsigned pos = 1; + int r; + + STRV_FOREACH(arg, args) { + if (arg_inline) + /* Use (argument):n, where n==1 for the first positional arg */ + r = parse_line("(argument)", pos, *arg); + else + r = read_config_file(*arg, false); + if (r < 0) + return r; + + pos++; + } + + return 0; +} + +static int read_config_files(const char* dirs, char **args) { + _cleanup_strv_free_ char **files = NULL; + _cleanup_free_ char *p = NULL; + char **f; + int r; + + r = conf_files_list_nulstr(&files, ".conf", arg_root, 0, dirs); + if (r < 0) + return log_error_errno(r, "Failed to enumerate sysusers.d files: %m"); + + if (arg_replace) { + r = conf_files_insert(&files, arg_root, dirs, arg_replace); + if (r < 0) + return log_error_errno(r, "Failed to extend sysusers.d file list: %m"); + + p = path_join(arg_root, arg_replace, NULL); + if (!p) + return log_oom(); + } + + STRV_FOREACH(f, files) + if (p && path_equal(*f, p)) { + log_debug("Parsing arguments at position \"%s\"…", *f); + + r = parse_arguments(args); + if (r < 0) + return r; + } else { + log_debug("Reading config file \"%s\"…", *f); + + /* Just warn, ignore result otherwise */ + (void) read_config_file(*f, true); + } + + return 0; +} + int main(int argc, char *argv[]) { _cleanup_close_ int lock = -1; Iterator iterator; - int r, k; + int r; Item *i; char *n; @@ -1826,34 +1902,18 @@ int main(int argc, char *argv[]) { goto finish; } - if (optind < argc) { - int j; - - for (j = optind; j < argc; j++) { - if (arg_inline) - /* Use (argument):n, where n==1 for the first positional arg */ - r = parse_line("(argument)", j - optind + 1, argv[j]); - else - r = read_config_file(argv[j], false); - if (r < 0) - goto finish; - } - } else { - _cleanup_strv_free_ char **files = NULL; - char **f; - - r = conf_files_list_nulstr(&files, ".conf", arg_root, 0, conf_file_dirs); - if (r < 0) { - log_error_errno(r, "Failed to enumerate sysusers.d files: %m"); - goto finish; - } - - STRV_FOREACH(f, files) { - k = read_config_file(*f, true); - if (k < 0 && r == 0) - r = k; - } - } + /* If command line arguments are specified along with --replace, read all + * configuration files and insert the positional arguments at the specified + * place. Otherwise, if command line arguments are specified, execute just + * them, and finally, without --replace= or any positional arguments, just + * read configuration and execute it. + */ + if (arg_replace || optind >= argc) + r = read_config_files(conf_file_dirs, argv + optind); + else + r = parse_arguments(argv + optind); + if (r < 0) + goto finish; /* Let's tell nss-systemd not to synthesize the "root" and "nobody" entries for it, so that our detection * whether the names or UID/GID area already used otherwise doesn't get confused. After all, even though From 4e9fe38dc0d6f71f87bce5d4e1b384e0f178192b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 2 Feb 2018 08:53:47 +0100 Subject: [PATCH 6/8] test/TEST-21-SYSUSERS: add tests for new functionality --- test/TEST-21-SYSUSERS/inline.expected-group | 2 + test/TEST-21-SYSUSERS/inline.expected-passwd | 1 + test/TEST-21-SYSUSERS/test-2.expected-group | 3 + test/TEST-21-SYSUSERS/test-2.expected-passwd | 3 + test/TEST-21-SYSUSERS/test-2.input | 10 ++- test/TEST-21-SYSUSERS/test.sh | 76 +++++++++++++++++--- 6 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 test/TEST-21-SYSUSERS/inline.expected-group create mode 100644 test/TEST-21-SYSUSERS/inline.expected-passwd diff --git a/test/TEST-21-SYSUSERS/inline.expected-group b/test/TEST-21-SYSUSERS/inline.expected-group new file mode 100644 index 0000000000..cc9093f807 --- /dev/null +++ b/test/TEST-21-SYSUSERS/inline.expected-group @@ -0,0 +1,2 @@ +g1:x:111: +u1:x:222: diff --git a/test/TEST-21-SYSUSERS/inline.expected-passwd b/test/TEST-21-SYSUSERS/inline.expected-passwd new file mode 100644 index 0000000000..f50f25c7d7 --- /dev/null +++ b/test/TEST-21-SYSUSERS/inline.expected-passwd @@ -0,0 +1 @@ +u1:x:222:222::/:/bin/zsh diff --git a/test/TEST-21-SYSUSERS/test-2.expected-group b/test/TEST-21-SYSUSERS/test-2.expected-group index 4d90426075..8fcc03f4e9 100644 --- a/test/TEST-21-SYSUSERS/test-2.expected-group +++ b/test/TEST-21-SYSUSERS/test-2.expected-group @@ -1 +1,4 @@ u1:x:SYSTEM_UID_MAX: +u2:x:777: +u3:x:778: +u4:x:779: diff --git a/test/TEST-21-SYSUSERS/test-2.expected-passwd b/test/TEST-21-SYSUSERS/test-2.expected-passwd index f438ed137c..9eeee5d387 100644 --- a/test/TEST-21-SYSUSERS/test-2.expected-passwd +++ b/test/TEST-21-SYSUSERS/test-2.expected-passwd @@ -1 +1,4 @@ u1:x:SYSTEM_UID_MAX:SYSTEM_UID_MAX:some gecos:/random/dir:/sbin/nologin +u2:x:777:777:some gecos:/random/dir:/bin/zsh +u3:x:778:778::/random/dir2:/bin/bash +u4:x:779:779::/:/bin/csh diff --git a/test/TEST-21-SYSUSERS/test-2.input b/test/TEST-21-SYSUSERS/test-2.input index bc3e182227..cedea9e401 100644 --- a/test/TEST-21-SYSUSERS/test-2.input +++ b/test/TEST-21-SYSUSERS/test-2.input @@ -1,4 +1,8 @@ -# Trivial smoke test that generate the ID dynamically based on SYSTEM_UID_MAX +# Test generation of ID dynamically based on SYSTEM_UID_MAX and +# replacement of all fields up to the login shell. # -#Type Name ID GECOS HOMEDIR -u u1 - "some gecos" /random/dir +#Type Name ID GECOS homedir shell +u u1 - "some gecos" /random/dir - +u u2 777 "some gecos" /random/dir /bin/zsh +u u3 778 - /random/dir2 /bin/bash +u u4 779 - - /bin/csh diff --git a/test/TEST-21-SYSUSERS/test.sh b/test/TEST-21-SYSUSERS/test.sh index f69d27748d..bebbab9d23 100755 --- a/test/TEST-21-SYSUSERS/test.sh +++ b/test/TEST-21-SYSUSERS/test.sh @@ -7,7 +7,7 @@ TEST_DESCRIPTION="Sysuser-related tests" . $TEST_BASE_DIR/test-functions test_setup() { - mkdir -p $TESTDIR/etc $TESTDIR/usr/lib/sysusers.d $TESTDIR/tmp + mkdir -p $TESTDIR/etc/sysusers.d $TESTDIR/usr/lib/sysusers.d $TESTDIR/tmp } preprocess() { @@ -20,31 +20,85 @@ preprocess() { sed "s/SYSTEM_UID_MAX/${SYSTEM_UID_MAX}/g" "$in" } +compare() { + if ! diff -u $TESTDIR/etc/passwd <(preprocess ${1%.*}.expected-passwd); then + echo "**** Unexpected output for $f" + exit 1 + fi + + if ! diff -u $TESTDIR/etc/group <(preprocess ${1%.*}.expected-group); then + echo "**** Unexpected output for $f $2" + exit 1 + fi +} + test_run() { # ensure our build of systemd-sysusers is run PATH=${BUILD_DIR}:$PATH + rm -f $TESTDIR/etc/sysusers.d/* $TESTDIR/usr/lib/sysusers.d/* + # happy tests for f in test-*.input; do echo "*** Running $f" - rm -f $TESTDIR/etc/* + rm -f $TESTDIR/etc/*{passwd,group,shadow} cp $f $TESTDIR/usr/lib/sysusers.d/test.conf systemd-sysusers --root=$TESTDIR - if ! diff -u $TESTDIR/etc/passwd <(preprocess ${f%.*}.expected-passwd); then - echo "**** Unexpected output for $f" - exit 1 - fi - if ! diff -u $TESTDIR/etc/group <(preprocess ${f%.*}.expected-group); then - echo "**** Unexpected output for $f" - exit 1 - fi + compare $f "" done + for f in test-*.input; do + echo "*** Running $f on stdin" + rm -f $TESTDIR/etc/*{passwd,group,shadow} + touch $TESTDIR/etc/sysusers.d/test.conf + cat $f | systemd-sysusers --root=$TESTDIR - + + compare $f "on stdin" + done + + for f in test-*.input; do + echo "*** Running $f on stdin with --replace" + rm -f $TESTDIR/etc/*{passwd,group,shadow} + touch $TESTDIR/etc/sysusers.d/test.conf + # this overrides test.conf which is masked on disk + cat $f | systemd-sysusers --root=$TESTDIR --replace=/etc/sysusers.d/test.conf - + # this should be ignored + cat test-1.input | systemd-sysusers --root=$TESTDIR --replace=/usr/lib/sysusers.d/test.conf - + + compare $f "on stdin with --replace" + done + + # test --inline + echo "*** Testing --inline" + rm -f $TESTDIR/etc/*{passwd,group,shadow} + # copy a random file to make sure it is ignored + cp $f $TESTDIR/etc/sysusers.d/confuse.conf + systemd-sysusers --root=$TESTDIR --inline \ + "u u1 222 - - /bin/zsh" \ + "g g1 111" + + compare inline "(--inline)" + + # test --replace + echo "*** Testing --inline with --replace" + rm -f $TESTDIR/etc/*{passwd,group,shadow} + # copy a random file to make sure it is ignored + cp $f $TESTDIR/etc/sysusers.d/confuse.conf + systemd-sysusers --root=$TESTDIR \ + --inline \ + --replace=/etc/sysusers.d/confuse.conf \ + "u u1 222 - - /bin/zsh" \ + "g g1 111" + + compare inline "(--inline --replace=…)" + + rm -f $TESTDIR/etc/sysusers.d/* $TESTDIR/usr/lib/sysusers.d/* + # tests for error conditions for f in unhappy-*.input; do echo "*** Running test $f" - rm -f $TESTDIR/etc/* + rm -f $TESTDIR/etc/*{passwd,group,shadow} cp $f $TESTDIR/usr/lib/sysusers.d/test.conf systemd-sysusers --root=$TESTDIR 2> /dev/null journalctl -t systemd-sysusers -o cat | tail -n1 > $TESTDIR/tmp/err From 07a7d4a0040d221ff09e527e91c112b4ffab1dba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 31 Jan 2018 16:07:59 +0100 Subject: [PATCH 7/8] rpm macros: add %sysusers_create_package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is close to %sysusers_create_inline and %sysusers_create that we had already, but expects a file name and uses --replace= to implement proper priority. This is used like: %sysusers_create_package %{name} %SOURCE1 where %SOURCE1 is a file with called %{name}.conf that will be installed into /usr/lib/sysusers.d/. The tough part is that the file needs to be available before %prep, i.e. outside of the source tarball. This is because the spec file is parsed (and any macros expanded), before the sources are unpackaged. v2: - disallow the case case when --config-name= is given but there are no positional args. Most likely this would be a user error, so at least for now forbid it. v3: - replace --config-name= with --target= - drop quotes around %1 and %2 — if necessary, the caller should add those. v4: - replace --target with --replace - add a big comment --- src/core/macros.systemd.in | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/core/macros.systemd.in b/src/core/macros.systemd.in index 940f1e895a..cee9b89870 100644 --- a/src/core/macros.systemd.in +++ b/src/core/macros.systemd.in @@ -100,6 +100,7 @@ journalctl --update-catalog >/dev/null 2>&1 || : \ systemd-tmpfiles --create %{?*} >/dev/null 2>&1 || : \ %{nil} +# Deprecated. Use %sysusers_create_package instead %sysusers_create() \ systemd-sysusers %{?*} >/dev/null 2>&1 || : \ %{nil} @@ -108,6 +109,24 @@ systemd-sysusers %{?*} >/dev/null 2>&1 || : \ echo %{?*} | systemd-sysusers - >/dev/null 2>&1 || : \ %{nil} +# This should be used by package installation scripts which +# require users or groups to be present before the files installed +# by the package are present on disk (for example because some files +# are owned by those users or groups). +# +# Example: +# Source1: %{name}.conf +# ... +# %install +# install -Dt %{buildroot}%{sysusersdir} %SOURCE1 +# %pre +# %sysusers_create_package %{name} %SOURCE1 +# %files +# %{sysusersdir}/%{name}.conf +%sysusers_create_package() \ +echo "%(cat %2)" | systemd-sysusers --replace=%_sysusersdir/%1.conf - >/dev/null 2>&1 || : \ +%{nil} + %sysctl_apply() \ @rootlibexecdir@/systemd-sysctl %{?*} >/dev/null 2>&1 || : \ %{nil} From fb959f14d7470ef6b3a7e166ce76ddf62a0af802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 2 Feb 2018 10:35:21 +0100 Subject: [PATCH 8/8] sysusers: use the usual comment style --- src/sysusers/sysusers.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index af21e3b854..cf16d2b55b 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -65,9 +65,12 @@ typedef struct Item { uid_t uid; bool gid_set:1; - // id_set_strict means that the group with the specified gid must - // exist and that the check if a uid clashes with a 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; + bool uid_set:1; bool todo_user:1;