From 98dcb8f4c79e0e1685098c3b618719c176b113d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 25 Sep 2020 13:56:13 +0200 Subject: [PATCH 01/12] Move {uid,gid}_is_*() from basic to shared Those are functions that express policy, and nothing in basic/ uses (or should use) them. --- src/basic/user-util.h | 26 +------------------------- src/core/dynamic-user.c | 1 + src/coredump/coredump.c | 1 + src/journal/journald-server.c | 1 + src/shared/condition.c | 1 + src/shared/user-record.h | 24 ++++++++++++++++++++++++ src/test/test-condition.c | 1 + 7 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/basic/user-util.h b/src/basic/user-util.h index 7c142dd1e6..13e2c99e6c 100644 --- a/src/basic/user-util.h +++ b/src/basic/user-util.h @@ -3,7 +3,7 @@ #include #if ENABLE_GSHADOW -#include +# include #endif #include #include @@ -61,30 +61,6 @@ int take_etc_passwd_lock(const char *root); #define ETC_PASSWD_LOCK_PATH "/etc/.pwd.lock" -static inline bool uid_is_system(uid_t uid) { - return uid <= SYSTEM_UID_MAX; -} - -static inline bool gid_is_system(gid_t gid) { - return gid <= SYSTEM_GID_MAX; -} - -static inline bool uid_is_dynamic(uid_t uid) { - return DYNAMIC_UID_MIN <= uid && uid <= DYNAMIC_UID_MAX; -} - -static inline bool gid_is_dynamic(gid_t gid) { - return uid_is_dynamic((uid_t) gid); -} - -static inline bool uid_is_container(uid_t uid) { - return CONTAINER_UID_BASE_MIN <= uid && uid <= CONTAINER_UID_BASE_MAX; -} - -static inline bool gid_is_container(gid_t gid) { - return uid_is_container((uid_t) gid); -} - /* The following macros add 1 when converting things, since UID 0 is a valid UID, while the pointer * NULL is special */ #define PTR_TO_UID(p) ((uid_t) (((uintptr_t) (p))-1)) diff --git a/src/core/dynamic-user.c b/src/core/dynamic-user.c index be386df12b..8388d53dd1 100644 --- a/src/core/dynamic-user.c +++ b/src/core/dynamic-user.c @@ -19,6 +19,7 @@ #include "stdio-util.h" #include "string-util.h" #include "strv.h" +#include "user-record.h" #include "user-util.h" /* Takes a value generated randomly or by hashing and turns it into a UID in the right range */ diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 88739ed5bc..662413bd08 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -46,6 +46,7 @@ #include "string-util.h" #include "strv.h" #include "tmpfile-util.h" +#include "user-record.h" #include "user-util.h" /* The maximum size up to which we process coredumps */ diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index 34ba24daf6..ab6aadcb78 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -55,6 +55,7 @@ #include "string-table.h" #include "string-util.h" #include "syslog-util.h" +#include "user-record.h" #include "user-util.h" #define USER_JOURNALS_MAX 1024 diff --git a/src/shared/condition.c b/src/shared/condition.c index 1f6105622a..af5d48f55b 100644 --- a/src/shared/condition.c +++ b/src/shared/condition.c @@ -43,6 +43,7 @@ #include "string-table.h" #include "string-util.h" #include "tomoyo-util.h" +#include "user-record.h" #include "user-util.h" #include "util.h" #include "virt.h" diff --git a/src/shared/user-record.h b/src/shared/user-record.h index 357c246ea5..b6fe0d846b 100644 --- a/src/shared/user-record.h +++ b/src/shared/user-record.h @@ -17,6 +17,30 @@ /* The default disk size to use when nothing else is specified, relative to free disk space */ #define USER_DISK_SIZE_DEFAULT_PERCENT 85 +static inline bool uid_is_system(uid_t uid) { + return uid <= SYSTEM_UID_MAX; +} + +static inline bool gid_is_system(gid_t gid) { + return gid <= SYSTEM_GID_MAX; +} + +static inline bool uid_is_dynamic(uid_t uid) { + return DYNAMIC_UID_MIN <= uid && uid <= DYNAMIC_UID_MAX; +} + +static inline bool gid_is_dynamic(gid_t gid) { + return uid_is_dynamic((uid_t) gid); +} + +static inline bool uid_is_container(uid_t uid) { + return CONTAINER_UID_BASE_MIN <= uid && uid <= CONTAINER_UID_BASE_MAX; +} + +static inline bool gid_is_container(gid_t gid) { + return uid_is_container((uid_t) gid); +} + typedef enum UserDisposition { USER_INTRINSIC, /* root and nobody */ USER_SYSTEM, /* statically allocated users for system services */ diff --git a/src/test/test-condition.c b/src/test/test-condition.c index d209c1304c..b42de61200 100644 --- a/src/test/test-condition.c +++ b/src/test/test-condition.c @@ -31,6 +31,7 @@ #include "strv.h" #include "tests.h" #include "tomoyo-util.h" +#include "user-record.h" #include "user-util.h" #include "virt.h" From 28add648a8307d088d2997f7b722770af6a875c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 25 Sep 2020 16:09:00 +0200 Subject: [PATCH 02/12] coredump: use uid_is_system() when appropriate --- src/coredump/coredump.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 662413bd08..229e2d8f87 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -683,7 +683,7 @@ static int change_uid_gid(const Context *context) { if (r < 0) return r; - if (uid <= SYSTEM_UID_MAX) { + if (uid_is_system(uid)) { const char *user = "systemd-coredump"; r = get_user_creds(&user, &uid, &gid, NULL, NULL, 0); From 53393c894dd4ab944d88acd4e7070714342d1597 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 25 Sep 2020 16:31:42 +0200 Subject: [PATCH 03/12] Look at /etc/login.defs for the system_max_[ug]id values It makes little sense to make the boundary between systemd and user guids configurable. Nevertheless, a completely fixed compile-time define is not enough in two scenarios: - the systemd_uid_max boundary has moved over time. The default used to be 500 for a long time. Systems which are upgraded over time might have users in the wrong range, but changing existing systems is complicated and expensive (offline disks, backups, remote systems, read-only media, etc.) - systems are used in a heterogenous enviornment, where some vendors pick one value and others another. So let's make this boundary overridable using /etc/login.defs. Fixes #3855, #10184. --- docs/UIDS-GIDS.md | 9 ++-- meson.build | 2 + meson_options.txt | 8 ++-- src/shared/user-record.c | 96 +++++++++++++++++++++++++++++++++++++ src/shared/user-record.h | 16 ++++--- src/test/meson.build | 4 ++ src/test/test-user-record.c | 54 +++++++++++++++++++++ 7 files changed, 175 insertions(+), 14 deletions(-) create mode 100644 src/test/test-user-record.c diff --git a/docs/UIDS-GIDS.md b/docs/UIDS-GIDS.md index 67e6d083ff..e289a9b68e 100644 --- a/docs/UIDS-GIDS.md +++ b/docs/UIDS-GIDS.md @@ -171,10 +171,11 @@ pick — given that 64K UIDs are assigned to each container according to this allocation logic, the maximum UID used for this range is hence 1878982656+65535=1879048191.) -Note that systemd does not make any of these values runtime-configurable. All -these boundaries are chosen during build time. That said, the system UID/GID -boundary is traditionally configured in /etc/login.defs, though systemd won't -look there during runtime. +Systemd has compile-time default for these boundaries. Using those defaults is +recommended. It will nevertheless query `/etc/login.defs` at runtime, when +compiled with `-Dcompat-mutable-uid-boundaries=true` and that file is present. +Support for this is considered only a compatibility feature and should not be +used except when upgrading systems which were creating with different defaults. ## Considerations for container managers diff --git a/meson.build b/meson.build index 36314f7157..8083cf655b 100644 --- a/meson.build +++ b/meson.build @@ -1443,6 +1443,7 @@ foreach term : ['analyze', 'idn', 'ima', 'initrd', + 'compat-mutable-uid-boundaries', 'ldconfig', 'localed', 'logind', @@ -3627,6 +3628,7 @@ foreach tuple : [ ['libcurl'], ['idn'], ['initrd'], + ['compat-mutable-uid-boundaries'], ['libidn2'], ['libidn'], ['libiptc'], diff --git a/meson_options.txt b/meson_options.txt index fc9e4379f3..a33b6e27ae 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -28,9 +28,9 @@ option('static-libsystemd', type : 'combo', description : '''install a static library for libsystemd''') option('static-libudev', type : 'combo', choices : ['false', 'true', 'pic', 'no-pic'], - description : '''install a static library for libudev''') + description : 'install a static library for libudev') option('standalone-binaries', type : 'boolean', value : 'false', - description : '''also build standalone versions of supported binaries''') + description : 'also build standalone versions of supported binaries') option('sysvinit-path', type : 'string', value : '/etc/init.d', description : 'the directory where the SysV init scripts are located') @@ -40,8 +40,10 @@ option('telinit-path', type : 'string', value : '/lib/sysvinit/telinit', description : 'path to telinit') option('rc-local', type : 'string', value : '/etc/rc.local') -option('initrd', type: 'boolean', +option('initrd', type : 'boolean', description : 'install services for use when running systemd in initrd') +option('compat-mutable-uid-boundaries', type : 'boolean', value : 'false', + description : 'look at uid boundaries in /etc/login.defs for compatibility') option('quotaon-path', type : 'string', description : 'path to quotaon') option('quotacheck-path', type : 'string', description : 'path to quotacheck') diff --git a/src/shared/user-record.c b/src/shared/user-record.c index 4149205b8a..f60db5da00 100644 --- a/src/shared/user-record.c +++ b/src/shared/user-record.c @@ -5,6 +5,8 @@ #include "cgroup-util.h" #include "dns-domain.h" #include "env-util.h" +#include "fd-util.h" +#include "fileio.h" #include "fs-util.h" #include "hexdecoct.h" #include "hostname-util.h" @@ -21,6 +23,100 @@ #define DEFAULT_RATELIMIT_BURST 30 #define DEFAULT_RATELIMIT_INTERVAL_USEC (1*USEC_PER_MINUTE) +#if ENABLE_COMPAT_MUTABLE_UID_BOUNDARIES +static int parse_alloc_uid(const char *path, const char *name, const char *t, uid_t *ret_uid) { + uid_t uid; + int r; + + r = parse_uid(t, &uid); + if (r < 0) + return log_debug_errno(r, "%s: failed to parse %s %s, ignoring: %m", path, name, t); + if (uid == 0) + uid = 1; + + *ret_uid = uid; + return 0; +} + +static int read_login_defs(UGIDAllocationRange *ret_defs, const char *path) { + _cleanup_fclose_ FILE *f = NULL; + UGIDAllocationRange defs = { + .system_uid_max = SYSTEM_UID_MAX, + .system_gid_max = SYSTEM_GID_MAX, + }; + int r; + + if (!path) + path = "/etc/login.defs"; + + r = fopen_unlocked(path, "re", &f); + if (r == -ENOENT) + goto assign; + if (r < 0) + return log_debug_errno(r, "Failed to open %s: %m", path); + + for (;;) { + _cleanup_free_ char *line = NULL; + char *t; + + r = read_line(f, LINE_MAX, &line); + if (r < 0) + return log_debug_errno(r, "Failed to read %s: %m", path); + if (r == 0) + break; + + if ((t = first_word(line, "SYS_UID_MAX"))) + (void) parse_alloc_uid(path, "SYS_UID_MAX", t, &defs.system_uid_max); + else if ((t = first_word(line, "SYS_GID_MAX"))) + (void) parse_alloc_uid(path, "SYS_GID_MAX", t, &defs.system_gid_max); + } + + assign: + *ret_defs = defs; + return 0; +} +#endif + +const UGIDAllocationRange *acquire_ugid_allocation_range(void) { +#if ENABLE_COMPAT_MUTABLE_UID_BOUNDARIES + static thread_local UGIDAllocationRange defs = { +#else + static const UGIDAllocationRange defs = { +#endif + .system_uid_max = SYSTEM_UID_MAX, + .system_gid_max = SYSTEM_GID_MAX, + }; + +#if ENABLE_COMPAT_MUTABLE_UID_BOUNDARIES + /* This function will ignore failure to read the file, so it should only be called from places where + * we don't crucially depend on the answer. In other words, it's appropriate for journald, but + * probably not for sysusers. */ + + static thread_local bool initialized = false; + + if (!initialized) { + (void) read_login_defs(&defs, NULL); + initialized = true; + } +#endif + + return &defs; +} + +bool uid_is_system(uid_t uid) { + const UGIDAllocationRange *defs; + assert_se(defs = acquire_ugid_allocation_range()); + + return uid <= defs->system_uid_max; +} + +bool gid_is_system(gid_t gid) { + const UGIDAllocationRange *defs; + assert_se(defs = acquire_ugid_allocation_range()); + + return gid <= defs->system_gid_max; +} + UserRecord* user_record_new(void) { UserRecord *h; diff --git a/src/shared/user-record.h b/src/shared/user-record.h index b6fe0d846b..52348227a5 100644 --- a/src/shared/user-record.h +++ b/src/shared/user-record.h @@ -17,13 +17,8 @@ /* The default disk size to use when nothing else is specified, relative to free disk space */ #define USER_DISK_SIZE_DEFAULT_PERCENT 85 -static inline bool uid_is_system(uid_t uid) { - return uid <= SYSTEM_UID_MAX; -} - -static inline bool gid_is_system(gid_t gid) { - return gid <= SYSTEM_GID_MAX; -} +bool uid_is_system(uid_t uid); +bool gid_is_system(gid_t gid); static inline bool uid_is_dynamic(uid_t uid) { return DYNAMIC_UID_MIN <= uid && uid <= DYNAMIC_UID_MAX; @@ -41,6 +36,13 @@ static inline bool gid_is_container(gid_t gid) { return uid_is_container((uid_t) gid); } +typedef struct UGIDAllocationRange { + uid_t system_uid_max; + gid_t system_gid_max; +} UGIDAllocationRange; + +const UGIDAllocationRange *acquire_ugid_allocation_range(void); + typedef enum UserDisposition { USER_INTRINSIC, /* root and nobody */ USER_SYSTEM, /* statically allocated users for system services */ diff --git a/src/test/meson.build b/src/test/meson.build index 34d64a0f42..9bb3499963 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -339,6 +339,10 @@ tests += [ [], []], + [['src/test/test-user-record.c'], + [], + []], + [['src/test/test-user-util.c'], [], []], diff --git a/src/test/test-user-record.c b/src/test/test-user-record.c new file mode 100644 index 0000000000..e54e384b09 --- /dev/null +++ b/src/test/test-user-record.c @@ -0,0 +1,54 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include +#include + +#include "format-util.h" +#include "tests.h" +#include "user-record.h" + +static void test_acquire_ugid_allocation_range(void) { + log_info("/* %s */", __func__); + + const UGIDAllocationRange *defs; + assert_se(defs = acquire_ugid_allocation_range()); + + log_info("system_uid_max="UID_FMT, defs->system_uid_max); + log_info("system_gid_max="GID_FMT, defs->system_gid_max); +} + +static void test_uid_is_system(void) { + log_info("/* %s */", __func__); + + uid_t uid = 0; + log_info("uid_is_system("UID_FMT") = %s", uid, yes_no(uid_is_system(uid))); + + uid = 999; + log_info("uid_is_system("UID_FMT") = %s", uid, yes_no(uid_is_system(uid))); + + uid = getuid(); + log_info("uid_is_system("UID_FMT") = %s", uid, yes_no(uid_is_system(uid))); +} + +static void test_gid_is_system(void) { + log_info("/* %s */", __func__); + + gid_t gid = 0; + log_info("gid_is_system("GID_FMT") = %s", gid, yes_no(gid_is_system(gid))); + + gid = 999; + log_info("gid_is_system("GID_FMT") = %s", gid, yes_no(gid_is_system(gid))); + + gid = getgid(); + log_info("gid_is_system("GID_FMT") = %s", gid, yes_no(gid_is_system(gid))); +} + +int main(int argc, char *argv[]) { + test_setup_logging(LOG_DEBUG); + + test_acquire_ugid_allocation_range(); + test_uid_is_system(); + test_gid_is_system(); + + return 0; +} From fc1a5d1a70aaaa5874ad589957f9e69ce75b3acd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 25 Sep 2020 16:50:45 +0200 Subject: [PATCH 04/12] Also parse the minimum uid/gid values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't (and shouldn't I think) look at them when determining the type of the user, but they should be used during user/group allocation. (For example, an admin may specify SYS_UID_MIN==200 to allow statically numbered users that are shared with other systems in the range 1–199.) --- meson.build | 60 +++++++++++++++++-------------------- meson_options.txt | 4 +++ src/core/systemd.pc.in | 4 +-- src/shared/user-record.c | 21 ++++++++++++- src/shared/user-record.h | 2 ++ src/test/test-user-record.c | 2 ++ 6 files changed, 58 insertions(+), 35 deletions(-) diff --git a/meson.build b/meson.build index 8083cf655b..0a065fa3e5 100644 --- a/meson.build +++ b/meson.build @@ -694,35 +694,31 @@ if time_epoch == -1 endif conf.set('TIME_EPOCH', time_epoch) -system_uid_max = get_option('system-uid-max') -if system_uid_max == -1 - system_uid_max = run_command( - awk, - '/^\s*SYS_UID_MAX\s+/ { uid=$2 } END { print uid }', - '/etc/login.defs').stdout().strip() - if system_uid_max == '' - system_uid_max = 999 - else - system_uid_max = system_uid_max.to_int() +foreach tuple : [['system-alloc-uid-min', 'SYS_UID_MIN', 1], # Also see login.defs(5). + ['system-uid-max', 'SYS_UID_MAX', 999], + ['system-alloc-gid-min', 'SYS_GID_MIN', 1], + ['system-gid-max', 'SYS_GID_MAX', 999]] + v = get_option(tuple[0]) + if v == -1 + v = run_command( + awk, + '/^\s*@0@\s+/ { uid=$2 } END { print uid }'.format(tuple[1]), + '/etc/login.defs').stdout().strip() + if v == '' + v = tuple[2] + else + v = v.to_int() + endif endif + conf.set(tuple[0].underscorify().to_upper(), v) + substs.set(tuple[0].underscorify().to_upper(), v) +endforeach +if conf.get('SYSTEM_ALLOC_UID_MIN') >= conf.get('SYSTEM_UID_MAX') + error('Invalid uid allocation range') endif -conf.set('SYSTEM_UID_MAX', system_uid_max) -substs.set('systemuidmax', system_uid_max) - -system_gid_max = get_option('system-gid-max') -if system_gid_max == -1 - system_gid_max = run_command( - awk, - '/^\s*SYS_GID_MAX\s+/ { gid=$2 } END { print gid }', - '/etc/login.defs').stdout().strip() - if system_gid_max == '' - system_gid_max = 999 - else - system_gid_max = system_gid_max.to_int() - endif +if conf.get('SYSTEM_ALLOC_GID_MIN') >= conf.get('SYSTEM_GID_MAX') + error('Invalid gid allocation range') endif -conf.set('SYSTEM_GID_MAX', system_gid_max) -substs.set('systemgidmax', system_gid_max) dynamic_uid_min = get_option('dynamic-uid-min') dynamic_uid_max = get_option('dynamic-uid-max') @@ -3539,12 +3535,12 @@ status = [ get_option('debug-tty')), 'TTY GID: @0@'.format(tty_gid), 'users GID: @0@'.format(substs.get('USERS_GID')), - 'maximum system UID: @0@'.format(system_uid_max), - 'maximum system GID: @0@'.format(system_gid_max), - 'minimum dynamic UID: @0@'.format(dynamic_uid_min), - 'maximum dynamic UID: @0@'.format(dynamic_uid_max), - 'minimum container UID base: @0@'.format(container_uid_base_min), - 'maximum container UID base: @0@'.format(container_uid_base_max), + 'system UIDs: <=@0@ (alloc >=@1@)'.format(conf.get('SYSTEM_UID_MAX'), + conf.get('SYSTEM_ALLOC_UID_MIN')), + 'system GIDs: <=@0@ (alloc >=@1@)'.format(conf.get('SYSTEM_GID_MAX'), + conf.get('SYSTEM_ALLOC_GID_MIN')), + 'dynamic UIDs: @0@–@1@'.format(dynamic_uid_min, dynamic_uid_max), + 'container UID bases: @0@–@1@'.format(container_uid_base_min, container_uid_base_max), '/dev/kvm access mode: @0@'.format(get_option('dev-kvm-mode')), 'render group access mode: @0@'.format(get_option('group-render-mode')), 'certificate root directory: @0@'.format(get_option('certificate-root')), diff --git a/meson_options.txt b/meson_options.txt index a33b6e27ae..d5ce647ae6 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -194,6 +194,10 @@ option('status-unit-format-default', type : 'combo', description : 'use unit name or description in messages by default') option('time-epoch', type : 'integer', value : '-1', description : 'time epoch for time clients') +option('system-alloc-uid-min', type : 'integer', value : '-1', + description : 'minimum system UID used when allocating') +option('system-alloc-gid-min', type : 'integer', value : '-1', + description : 'minimum system GID used when allocating') option('system-uid-max', type : 'integer', value : '-1', description : 'maximum system UID') option('system-gid-max', type : 'integer', value : '-1', diff --git a/src/core/systemd.pc.in b/src/core/systemd.pc.in index 3af9f99830..c0554649de 100644 --- a/src/core/systemd.pc.in +++ b/src/core/systemd.pc.in @@ -80,9 +80,9 @@ modulesloaddir=${modules_load_dir} catalog_dir=/usr/lib/systemd/catalog catalogdir=${catalog_dir} -system_uid_max=@systemuidmax@ +system_uid_max=@SYSTEM_UID_MAX@ systemuidmax=${system_uid_max} -system_gid_max=@systemgidmax@ +system_gid_max=@SYSTEM_GID_MAX@ systemgidmax=${system_gid_max} dynamic_uid_min=@dynamicuidmin@ diff --git a/src/shared/user-record.c b/src/shared/user-record.c index f60db5da00..3ba78d455f 100644 --- a/src/shared/user-record.c +++ b/src/shared/user-record.c @@ -41,7 +41,9 @@ static int parse_alloc_uid(const char *path, const char *name, const char *t, ui static int read_login_defs(UGIDAllocationRange *ret_defs, const char *path) { _cleanup_fclose_ FILE *f = NULL; UGIDAllocationRange defs = { + .system_alloc_uid_min = SYSTEM_ALLOC_UID_MIN, .system_uid_max = SYSTEM_UID_MAX, + .system_alloc_gid_min = SYSTEM_ALLOC_GID_MIN, .system_gid_max = SYSTEM_GID_MAX, }; int r; @@ -65,13 +67,28 @@ static int read_login_defs(UGIDAllocationRange *ret_defs, const char *path) { if (r == 0) break; - if ((t = first_word(line, "SYS_UID_MAX"))) + if ((t = first_word(line, "SYS_UID_MIN"))) + (void) parse_alloc_uid(path, "SYS_UID_MIN", t, &defs.system_alloc_uid_min); + else if ((t = first_word(line, "SYS_UID_MAX"))) (void) parse_alloc_uid(path, "SYS_UID_MAX", t, &defs.system_uid_max); + else if ((t = first_word(line, "SYS_GID_MIN"))) + (void) parse_alloc_uid(path, "SYS_GID_MIN", t, &defs.system_alloc_gid_min); else if ((t = first_word(line, "SYS_GID_MAX"))) (void) parse_alloc_uid(path, "SYS_GID_MAX", t, &defs.system_gid_max); } assign: + if (defs.system_alloc_uid_min > defs.system_uid_max) { + log_debug("%s: SYS_UID_MIN > SYS_UID_MAX, resetting.", path); + defs.system_alloc_uid_min = MIN(defs.system_uid_max - 1, (uid_t) SYSTEM_ALLOC_UID_MIN); + /* Look at sys_uid_max to make sure sys_uid_min..sys_uid_max remains a valid range. */ + } + if (defs.system_alloc_gid_min > defs.system_gid_max) { + log_debug("%s: SYS_GID_MIN > SYS_GID_MAX, resetting.", path); + defs.system_alloc_gid_min = MIN(defs.system_gid_max - 1, (gid_t) SYSTEM_ALLOC_GID_MIN); + /* Look at sys_gid_max to make sure sys_gid_min..sys_gid_max remains a valid range. */ + } + *ret_defs = defs; return 0; } @@ -83,7 +100,9 @@ const UGIDAllocationRange *acquire_ugid_allocation_range(void) { #else static const UGIDAllocationRange defs = { #endif + .system_alloc_uid_min = SYSTEM_ALLOC_UID_MIN, .system_uid_max = SYSTEM_UID_MAX, + .system_alloc_gid_min = SYSTEM_ALLOC_GID_MIN, .system_gid_max = SYSTEM_GID_MAX, }; diff --git a/src/shared/user-record.h b/src/shared/user-record.h index 52348227a5..1f87eff6d5 100644 --- a/src/shared/user-record.h +++ b/src/shared/user-record.h @@ -37,7 +37,9 @@ static inline bool gid_is_container(gid_t gid) { } typedef struct UGIDAllocationRange { + uid_t system_alloc_uid_min; uid_t system_uid_max; + gid_t system_alloc_gid_min; gid_t system_gid_max; } UGIDAllocationRange; diff --git a/src/test/test-user-record.c b/src/test/test-user-record.c index e54e384b09..fcab61d694 100644 --- a/src/test/test-user-record.c +++ b/src/test/test-user-record.c @@ -13,7 +13,9 @@ static void test_acquire_ugid_allocation_range(void) { const UGIDAllocationRange *defs; assert_se(defs = acquire_ugid_allocation_range()); + log_info("system_alloc_uid_min="UID_FMT, defs->system_alloc_uid_min); log_info("system_uid_max="UID_FMT, defs->system_uid_max); + log_info("system_alloc_gid_min="GID_FMT, defs->system_alloc_gid_min); log_info("system_gid_max="GID_FMT, defs->system_gid_max); } From 196b596867b09838c3391566ada0539d972e2d2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 25 Sep 2020 17:51:41 +0200 Subject: [PATCH 05/12] shared/uid-range: reduce scope of iterator variables --- src/shared/uid-range.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/shared/uid-range.c b/src/shared/uid-range.c index 7cb7d8a477..201d27ab63 100644 --- a/src/shared/uid-range.c +++ b/src/shared/uid-range.c @@ -18,13 +18,11 @@ static bool uid_range_intersect(UidRange *range, uid_t start, uid_t nr) { } static void uid_range_coalesce(UidRange **p, unsigned *n) { - unsigned i, j; - assert(p); assert(n); - for (i = 0; i < *n; i++) { - for (j = i + 1; j < *n; j++) { + for (unsigned i = 0; i < *n; i++) { + for (unsigned j = i + 1; j < *n; j++) { UidRange *x = (*p)+i, *y = (*p)+j; if (uid_range_intersect(x, y->start, y->nr)) { @@ -59,7 +57,6 @@ static int uid_range_compare(const UidRange *a, const UidRange *b) { int uid_range_add(UidRange **p, unsigned *n, uid_t start, uid_t nr) { bool found = false; UidRange *x; - unsigned i; assert(p); assert(n); @@ -67,7 +64,7 @@ int uid_range_add(UidRange **p, unsigned *n, uid_t start, uid_t nr) { if (nr <= 0) return 0; - for (i = 0; i < *n; i++) { + for (unsigned i = 0; i < *n; i++) { x = (*p) + i; if (uid_range_intersect(x, start, nr)) { found = true; @@ -143,14 +140,13 @@ int uid_range_add_str(UidRange **p, unsigned *n, const char *s) { int uid_range_next_lower(const UidRange *p, unsigned n, uid_t *uid) { uid_t closest = UID_INVALID, candidate; - unsigned i; assert(p); assert(uid); candidate = *uid - 1; - for (i = 0; i < n; i++) { + for (unsigned i = 0; i < n; i++) { uid_t begin, end; begin = p[i].start; @@ -173,12 +169,10 @@ int uid_range_next_lower(const UidRange *p, unsigned n, uid_t *uid) { } bool uid_range_contains(const UidRange *p, unsigned n, uid_t uid) { - unsigned i; - assert(p); assert(uid); - for (i = 0; i < n; i++) + for (unsigned i = 0; i < n; i++) if (uid >= p[i].start && uid < p[i].start + p[i].nr) return true; From d338bfff4a083b64f405249c49b87f2b74a7d9b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Oct 2020 13:52:16 +0200 Subject: [PATCH 06/12] basic/fileio: add chase_symlinks_and_fopen_unlocked() --- src/basic/fileio.c | 36 ++++++++++++++++++++++++++++++++++++ src/basic/fileio.h | 8 ++++++++ src/basic/fs-util.h | 2 +- 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index c3d55d209a..c5a093a857 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -940,6 +940,42 @@ int search_and_fopen_nulstr(const char *path, const char *mode, const char *root return search_and_fopen_internal(path, mode, root, s, _f); } +int chase_symlinks_and_fopen_unlocked( + const char *path, + const char *root, + unsigned chase_flags, + const char *open_flags, + FILE **ret_file, + char **ret_path) { + + _cleanup_close_ int fd = -1; + _cleanup_free_ char *final_path = NULL; + int mode_flags, r; + FILE *f; + + assert(path); + assert(open_flags); + assert(ret_file); + + mode_flags = mode_to_flags(open_flags); + if (mode_flags < 0) + return mode_flags; + + fd = chase_symlinks_and_open(path, root, chase_flags, mode_flags, ret_path ? &final_path : NULL); + if (fd < 0) + return fd; + + r = fdopen_unlocked(fd, open_flags, &f); + if (r < 0) + return r; + TAKE_FD(fd); + + *ret_file = f; + if (ret_path) + *ret_path = TAKE_PTR(final_path); + return 0; +} + int fflush_and_check(FILE *f) { assert(f); diff --git a/src/basic/fileio.h b/src/basic/fileio.h index 7d58fa7cfc..9cba5a90e3 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -81,6 +81,14 @@ int xfopenat(int dir_fd, const char *path, const char *mode, int flags, FILE **r int search_and_fopen(const char *path, const char *mode, const char *root, const char **search, FILE **_f); int search_and_fopen_nulstr(const char *path, const char *mode, const char *root, const char *search, FILE **_f); +int chase_symlinks_and_fopen_unlocked( + const char *path, + const char *root, + unsigned chase_flags, + const char *open_flags, + FILE **ret_file, + char **ret_path); + int fflush_and_check(FILE *f); int fflush_sync_and_check(FILE *f); diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index eb6e1eee4f..2a785f690d 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -79,7 +79,7 @@ enum { CHASE_PREFIX_ROOT = 1 << 0, /* The specified path will be prefixed by the specified root before beginning the iteration */ CHASE_NONEXISTENT = 1 << 1, /* It's OK if the path doesn't actually exist. */ CHASE_NO_AUTOFS = 1 << 2, /* Return -EREMOTE if autofs mount point found */ - CHASE_SAFE = 1 << 3, /* Return EPERM if we ever traverse from unprivileged to privileged files or directories */ + CHASE_SAFE = 1 << 3, /* Return -EPERM if we ever traverse from unprivileged to privileged files or directories */ CHASE_TRAIL_SLASH = 1 << 4, /* Any trailing slash will be preserved */ CHASE_STEP = 1 << 5, /* Just execute a single step of the normalization */ CHASE_NOFOLLOW = 1 << 6, /* Do not follow the path's right-most component. With ret_fd, when the path's From bd7e6aa73ac213a7a23ad1837a4e7e0af21c9294 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 26 Sep 2020 11:58:24 +0200 Subject: [PATCH 07/12] test/TEST-21-SYSUSERS: turn into a unit test All this test does is manipulate text files in a subdir specified with --testroot. It can be a normal unittest without the overhead of creating a machine image. As a bonus, also test the .standalone version. --- meson.build | 25 +++- sysusers.d/meson.build | 2 - test/TEST-21-SYSUSERS/Makefile | 1 - test/TEST-21-SYSUSERS/test.sh | 128 ------------------ test/meson.build | 15 ++ test/test-sysusers.sh.in | 111 +++++++++++++++ .../inline.expected-group | 0 .../inline.expected-passwd | 0 .../test-1.expected-group | 0 .../test-1.expected-passwd | 0 .../test-1.input | 0 .../test-10.expected-group | 0 .../test-10.expected-passwd | 0 .../test-10.input | 0 .../test-11.expected-group | 0 .../test-11.expected-passwd | 0 .../test-11.initial-group | 0 .../test-11.initial-passwd | 0 .../test-11.input | 0 .../test-12.expected-group | 0 .../test-12.expected-passwd | 0 .../test-12.initial-group | 0 .../test-12.initial-passwd | 0 .../test-12.input | 0 .../test-13.expected-group | 0 .../test-13.expected-passwd | 0 .../test-13.input | 0 .../test-14.expected-group | 0 .../test-14.expected-passwd | 0 .../test-14.initial-group | 0 .../test-14.input | 0 .../test-2.expected-group | 0 .../test-2.expected-passwd | 0 .../test-2.input | 0 .../test-3.expected-group | 0 .../test-3.expected-passwd | 0 .../test-3.input | 0 .../test-4.expected-group | 0 .../test-4.expected-passwd | 0 .../test-4.input | 0 .../test-5.expected-group | 0 .../test-5.expected-passwd | 0 .../test-5.input | 0 .../test-6.expected-group | 0 .../test-6.expected-passwd | 0 .../test-6.input | 0 .../test-7.expected-group | 0 .../test-7.expected-passwd | 0 .../test-7.input | 0 .../test-8.expected-group | 0 .../test-8.expected-passwd | 0 .../test-8.input | 0 .../test-9.expected-group | 0 .../test-9.expected-passwd | 0 .../test-9.input | 0 .../unhappy-1.expected-err | 0 .../unhappy-1.input | 0 .../unhappy-2.expected-err | 0 .../unhappy-2.input | 0 .../unhappy-3.expected-err | 0 .../unhappy-3.input | 0 61 files changed, 148 insertions(+), 134 deletions(-) delete mode 120000 test/TEST-21-SYSUSERS/Makefile delete mode 100755 test/TEST-21-SYSUSERS/test.sh create mode 100755 test/test-sysusers.sh.in rename test/{TEST-21-SYSUSERS => test-sysusers}/inline.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/inline.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-1.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-1.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-1.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-10.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-10.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-10.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-11.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-11.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-11.initial-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-11.initial-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-11.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-12.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-12.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-12.initial-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-12.initial-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-12.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-13.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-13.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-13.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-14.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-14.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-14.initial-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-14.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-2.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-2.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-2.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-3.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-3.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-3.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-4.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-4.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-4.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-5.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-5.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-5.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-6.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-6.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-6.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-7.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-7.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-7.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-8.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-8.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-8.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-9.expected-group (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-9.expected-passwd (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/test-9.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/unhappy-1.expected-err (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/unhappy-1.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/unhappy-2.expected-err (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/unhappy-2.input (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/unhappy-3.expected-err (100%) rename test/{TEST-21-SYSUSERS => test-sysusers}/unhappy-3.input (100%) diff --git a/meson.build b/meson.build index 0a065fa3e5..3bec86db81 100644 --- a/meson.build +++ b/meson.build @@ -299,6 +299,7 @@ substs.set('CERTIFICATEROOT', get_option('certif substs.set('RANDOM_SEED', join_paths(randomseeddir, 'random-seed')) substs.set('SYSTEM_SYSVINIT_PATH', sysvinit_path) substs.set('SYSTEM_SYSVRCND_PATH', sysvrcnd_path) +substs.set('SYSTEMD_TEST_DATA', join_paths(testsdir, 'testdata')) substs.set('RC_LOCAL_PATH', get_option('rc-local')) substs.set('MEMORY_ACCOUNTING_DEFAULT', memory_accounting_default ? 'yes' : 'no') substs.set('STATUS_UNIT_FORMAT_DEFAULT', status_unit_format_default) @@ -1468,6 +1469,8 @@ foreach term : ['analyze', conf.set10(name, have) endforeach +enable_sysusers = conf.get('ENABLE_SYSUSERS') == 1 + foreach tuple : [['nss-mymachines', 'machined'], ['nss-resolve', 'resolve']] want = get_option(tuple[0]) @@ -2962,8 +2965,8 @@ public_programs += executable( install_rpath : rootlibexecdir, install : true) -if conf.get('ENABLE_SYSUSERS') == 1 - public_programs += executable( +if enable_sysusers + exe = executable( 'systemd-sysusers', 'src/sysusers/sysusers.c', include_directories : includes, @@ -2971,9 +2974,17 @@ if conf.get('ENABLE_SYSUSERS') == 1 install_rpath : rootlibexecdir, install : true, install_dir : rootbindir) + public_programs += exe + + if want_tests != 'false' + test('test-sysusers', + test_sysusers_sh, + # https://github.com/mesonbuild/meson/issues/2681 + args : exe.full_path()) + endif if have_standalone_binaries - public_programs += executable( + exe = executable( 'systemd-sysusers.standalone', 'src/sysusers/sysusers.c', include_directories : includes, @@ -2984,6 +2995,14 @@ if conf.get('ENABLE_SYSUSERS') == 1 libjournal_client], install : true, install_dir : rootbindir) + public_programs += exe + + if want_tests != 'false' + test('test-sysusers.standalone', + test_sysusers_sh, + # https://github.com/mesonbuild/meson/issues/2681 + args : exe.full_path()) + endif endif endif diff --git a/sysusers.d/meson.build b/sysusers.d/meson.build index eb99957e2f..146f922bed 100644 --- a/sysusers.d/meson.build +++ b/sysusers.d/meson.build @@ -2,8 +2,6 @@ in_files = ['basic.conf'] -enable_sysusers = conf.get('ENABLE_SYSUSERS') == 1 - foreach file : in_files gen = configure_file( input : file + '.in', diff --git a/test/TEST-21-SYSUSERS/Makefile b/test/TEST-21-SYSUSERS/Makefile deleted file mode 120000 index e9f93b1104..0000000000 --- a/test/TEST-21-SYSUSERS/Makefile +++ /dev/null @@ -1 +0,0 @@ -../TEST-01-BASIC/Makefile \ No newline at end of file diff --git a/test/TEST-21-SYSUSERS/test.sh b/test/TEST-21-SYSUSERS/test.sh deleted file mode 100755 index 3b37f7eaa6..0000000000 --- a/test/TEST-21-SYSUSERS/test.sh +++ /dev/null @@ -1,128 +0,0 @@ -#!/usr/bin/env bash -set -e -TEST_DESCRIPTION="Sysuser-related tests" -IMAGE_NAME="sysusers" -. $TEST_BASE_DIR/test-functions - -test_setup() { - mkdir -p $TESTDIR/etc/sysusers.d $TESTDIR/usr/lib/sysusers.d $TESTDIR/tmp -} - -prepare_testdir() { - rm -f $TESTDIR/etc/*{passwd,group,shadow} - for i in $1.initial-{passwd,group,shadow}; do - test -f $i && cp $i $TESTDIR/etc/${i#*.initial-} - done - return 0 -} - -preprocess() { - in="$1" - - # see meson.build how to extract this. gcc -E was used before to - # get this value from config.h, however the autopkgtest fails with - # it - [[ -e /etc/login.defs ]] && login_defs_file="/etc/login.defs" || login_defs_file="/usr/etc/login.defs" - SYSTEM_UID_MAX=$(awk 'BEGIN { uid=999 } /^\s*SYS_UID_MAX\s+/ { uid=$2 } END { print uid }' $login_defs_file) - SYSTEM_GID_MAX=$(awk 'BEGIN { gid=999 } /^\s*SYS_GID_MAX\s+/ { gid=$2 } END { print gid }' $login_defs_file) - - # we can't rely on config.h to get the nologin path, as autopkgtest - # uses pre-compiled binaries, so extract it from the systemd-sysusers - # binary which we are about to execute - NOLOGIN=$(strings $(type -p systemd-sysusers) | grep nologin) - - sed -e "s/SYSTEM_UID_MAX/${SYSTEM_UID_MAX}/g" \ - -e "s/SYSTEM_GID_MAX/${SYSTEM_GID_MAX}/g" \ - -e "s#NOLOGIN#${NOLOGIN}#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" - prepare_testdir ${f%.input} - cp $f $TESTDIR/usr/lib/sysusers.d/test.conf - systemd-sysusers --root=$TESTDIR - - compare $f "" - done - - for f in test-*.input; do - echo "*** Running $f on stdin" - prepare_testdir ${f%.input} - 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" - prepare_testdir ${f%.input} - 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" - prepare_testdir - # 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" - prepare_testdir - # 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" - prepare_testdir ${f%.input} - cp $f $TESTDIR/usr/lib/sysusers.d/test.conf - systemd-sysusers --root=$TESTDIR 2>&1 | tail -n1 > $TESTDIR/tmp/err - if ! diff -u $TESTDIR/tmp/err ${f%.*}.expected-err; then - echo "**** Unexpected error output for $f" - cat $TESTDIR/tmp/err - exit 1 - fi - done -} - -do_test "$@" diff --git a/test/meson.build b/test/meson.build index 5d9fb5ab50..5656abdf72 100644 --- a/test/meson.build +++ b/test/meson.build @@ -58,6 +58,21 @@ test_network_generator_conversion_sh = find_program('test-network-generator-conv ############################################################ +test_sysusers_dir = join_paths(meson.current_source_dir(), 'test-sysusers') + +test_sysusers_sh = configure_file( + input : 'test-sysusers.sh.in', + output : 'test-sysusers.sh', + configuration : substs) +if install_tests and conf.get('ENABLE_SYSUSERS') == 1 + install_data(test_sysusers_sh, + install_dir : testsdir) + install_subdir('test-sysusers', + install_dir : testdata_dir) +endif + +############################################################ + rule_syntax_check_py = find_program('rule-syntax-check.py') if want_tests != 'false' test('rule-syntax-check', diff --git a/test/test-sysusers.sh.in b/test/test-sysusers.sh.in new file mode 100755 index 0000000000..ac07c2145b --- /dev/null +++ b/test/test-sysusers.sh.in @@ -0,0 +1,111 @@ +#!/usr/bin/env bash +set -e + +SYSUSERS="${1:-systemd-sysusers}" + +[ -e "$(dirname $0)/../systemd-runtest.env" ] && . "$(dirname $0)/../systemd-runtest.env" +SYSTEMD_TEST_DATA=${SYSTEMD_TEST_DATA:-@SYSTEMD_TEST_DATA@} +SOURCE=$SYSTEMD_TEST_DATA/test-sysusers + +TESTDIR=$(mktemp --tmpdir --directory "test-sysusers.XXXXXXXXXX") +trap "rm -rf '$TESTDIR'" EXIT INT QUIT PIPE + +prepare_testdir() { + mkdir -p $TESTDIR/etc/sysusers.d/ + mkdir -p $TESTDIR/usr/lib/sysusers.d/ + rm -f $TESTDIR/etc/*{passwd,group,shadow} + for i in $1.initial-{passwd,group,shadow}; do + test -f $i && cp $i $TESTDIR/etc/${i#*.initial-} + done + return 0 +} + +preprocess() { + sed -e "s/SYSTEM_UID_MAX/@SYSTEM_UID_MAX@/g" \ + -e "s/SYSTEM_GID_MAX/@SYSTEM_GID_MAX@/g" \ + -e "s#NOLOGIN#@NOLOGIN@#g" "$1" +} + +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 +} + +rm -f $TESTDIR/etc/sysusers.d/* $TESTDIR/usr/lib/sysusers.d/* + +# happy tests +for f in $SOURCE/test-*.input; do + echo "*** Running $f" + prepare_testdir ${f%.input} + cp $f $TESTDIR/usr/lib/sysusers.d/test.conf + $SYSUSERS --root=$TESTDIR + + compare $f "" +done + +for f in $SOURCE/test-*.input; do + echo "*** Running $f on stdin" + prepare_testdir ${f%.input} + touch $TESTDIR/etc/sysusers.d/test.conf + cat $f | $SYSUSERS --root=$TESTDIR - + + compare $f "on stdin" +done + +for f in $SOURCE/test-*.input; do + echo "*** Running $f on stdin with --replace" + prepare_testdir ${f%.input} + touch $TESTDIR/etc/sysusers.d/test.conf + # this overrides test.conf which is masked on disk + cat $f | $SYSUSERS --root=$TESTDIR --replace=/etc/sysusers.d/test.conf - + # this should be ignored + cat $SOURCE/test-1.input | $SYSUSERS --root=$TESTDIR --replace=/usr/lib/sysusers.d/test.conf - + + compare $f "on stdin with --replace" +done + +# test --inline +echo "*** Testing --inline" +prepare_testdir +# copy a random file to make sure it is ignored +cp $f $TESTDIR/etc/sysusers.d/confuse.conf +$SYSUSERS --root=$TESTDIR --inline \ + "u u1 222 - - /bin/zsh" \ + "g g1 111" + +compare $SOURCE/inline "(--inline)" + +# test --replace +echo "*** Testing --inline with --replace" +prepare_testdir +# copy a random file to make sure it is ignored +cp $f $TESTDIR/etc/sysusers.d/confuse.conf +$SYSUSERS --root=$TESTDIR \ + --inline \ + --replace=/etc/sysusers.d/confuse.conf \ + "u u1 222 - - /bin/zsh" \ + "g g1 111" + +compare $SOURCE/inline "(--inline --replace=…)" + +rm -f $TESTDIR/etc/sysusers.d/* $TESTDIR/usr/lib/sysusers.d/* + +# tests for error conditions +for f in $SOURCE/unhappy-*.input; do + echo "*** Running test $f" + prepare_testdir ${f%.input} + cp $f $TESTDIR/usr/lib/sysusers.d/test.conf + $SYSUSERS --root=$TESTDIR 2>&1 | tail -n1 > $TESTDIR/err + if ! diff -u $TESTDIR/err ${f%.*}.expected-err; then + echo "**** Unexpected error output for $f" + cat $TESTDIR/err + exit 1 + fi +done diff --git a/test/TEST-21-SYSUSERS/inline.expected-group b/test/test-sysusers/inline.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/inline.expected-group rename to test/test-sysusers/inline.expected-group diff --git a/test/TEST-21-SYSUSERS/inline.expected-passwd b/test/test-sysusers/inline.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/inline.expected-passwd rename to test/test-sysusers/inline.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-1.expected-group b/test/test-sysusers/test-1.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-1.expected-group rename to test/test-sysusers/test-1.expected-group diff --git a/test/TEST-21-SYSUSERS/test-1.expected-passwd b/test/test-sysusers/test-1.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-1.expected-passwd rename to test/test-sysusers/test-1.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-1.input b/test/test-sysusers/test-1.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-1.input rename to test/test-sysusers/test-1.input diff --git a/test/TEST-21-SYSUSERS/test-10.expected-group b/test/test-sysusers/test-10.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-10.expected-group rename to test/test-sysusers/test-10.expected-group diff --git a/test/TEST-21-SYSUSERS/test-10.expected-passwd b/test/test-sysusers/test-10.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-10.expected-passwd rename to test/test-sysusers/test-10.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-10.input b/test/test-sysusers/test-10.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-10.input rename to test/test-sysusers/test-10.input diff --git a/test/TEST-21-SYSUSERS/test-11.expected-group b/test/test-sysusers/test-11.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-11.expected-group rename to test/test-sysusers/test-11.expected-group diff --git a/test/TEST-21-SYSUSERS/test-11.expected-passwd b/test/test-sysusers/test-11.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-11.expected-passwd rename to test/test-sysusers/test-11.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-11.initial-group b/test/test-sysusers/test-11.initial-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-11.initial-group rename to test/test-sysusers/test-11.initial-group diff --git a/test/TEST-21-SYSUSERS/test-11.initial-passwd b/test/test-sysusers/test-11.initial-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-11.initial-passwd rename to test/test-sysusers/test-11.initial-passwd diff --git a/test/TEST-21-SYSUSERS/test-11.input b/test/test-sysusers/test-11.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-11.input rename to test/test-sysusers/test-11.input diff --git a/test/TEST-21-SYSUSERS/test-12.expected-group b/test/test-sysusers/test-12.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-12.expected-group rename to test/test-sysusers/test-12.expected-group diff --git a/test/TEST-21-SYSUSERS/test-12.expected-passwd b/test/test-sysusers/test-12.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-12.expected-passwd rename to test/test-sysusers/test-12.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-12.initial-group b/test/test-sysusers/test-12.initial-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-12.initial-group rename to test/test-sysusers/test-12.initial-group diff --git a/test/TEST-21-SYSUSERS/test-12.initial-passwd b/test/test-sysusers/test-12.initial-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-12.initial-passwd rename to test/test-sysusers/test-12.initial-passwd diff --git a/test/TEST-21-SYSUSERS/test-12.input b/test/test-sysusers/test-12.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-12.input rename to test/test-sysusers/test-12.input diff --git a/test/TEST-21-SYSUSERS/test-13.expected-group b/test/test-sysusers/test-13.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-13.expected-group rename to test/test-sysusers/test-13.expected-group diff --git a/test/TEST-21-SYSUSERS/test-13.expected-passwd b/test/test-sysusers/test-13.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-13.expected-passwd rename to test/test-sysusers/test-13.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-13.input b/test/test-sysusers/test-13.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-13.input rename to test/test-sysusers/test-13.input diff --git a/test/TEST-21-SYSUSERS/test-14.expected-group b/test/test-sysusers/test-14.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-14.expected-group rename to test/test-sysusers/test-14.expected-group diff --git a/test/TEST-21-SYSUSERS/test-14.expected-passwd b/test/test-sysusers/test-14.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-14.expected-passwd rename to test/test-sysusers/test-14.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-14.initial-group b/test/test-sysusers/test-14.initial-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-14.initial-group rename to test/test-sysusers/test-14.initial-group diff --git a/test/TEST-21-SYSUSERS/test-14.input b/test/test-sysusers/test-14.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-14.input rename to test/test-sysusers/test-14.input diff --git a/test/TEST-21-SYSUSERS/test-2.expected-group b/test/test-sysusers/test-2.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-2.expected-group rename to test/test-sysusers/test-2.expected-group diff --git a/test/TEST-21-SYSUSERS/test-2.expected-passwd b/test/test-sysusers/test-2.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-2.expected-passwd rename to test/test-sysusers/test-2.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-2.input b/test/test-sysusers/test-2.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-2.input rename to test/test-sysusers/test-2.input diff --git a/test/TEST-21-SYSUSERS/test-3.expected-group b/test/test-sysusers/test-3.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-3.expected-group rename to test/test-sysusers/test-3.expected-group diff --git a/test/TEST-21-SYSUSERS/test-3.expected-passwd b/test/test-sysusers/test-3.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-3.expected-passwd rename to test/test-sysusers/test-3.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-3.input b/test/test-sysusers/test-3.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-3.input rename to test/test-sysusers/test-3.input diff --git a/test/TEST-21-SYSUSERS/test-4.expected-group b/test/test-sysusers/test-4.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-4.expected-group rename to test/test-sysusers/test-4.expected-group diff --git a/test/TEST-21-SYSUSERS/test-4.expected-passwd b/test/test-sysusers/test-4.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-4.expected-passwd rename to test/test-sysusers/test-4.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-4.input b/test/test-sysusers/test-4.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-4.input rename to test/test-sysusers/test-4.input diff --git a/test/TEST-21-SYSUSERS/test-5.expected-group b/test/test-sysusers/test-5.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-5.expected-group rename to test/test-sysusers/test-5.expected-group diff --git a/test/TEST-21-SYSUSERS/test-5.expected-passwd b/test/test-sysusers/test-5.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-5.expected-passwd rename to test/test-sysusers/test-5.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-5.input b/test/test-sysusers/test-5.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-5.input rename to test/test-sysusers/test-5.input diff --git a/test/TEST-21-SYSUSERS/test-6.expected-group b/test/test-sysusers/test-6.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-6.expected-group rename to test/test-sysusers/test-6.expected-group diff --git a/test/TEST-21-SYSUSERS/test-6.expected-passwd b/test/test-sysusers/test-6.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-6.expected-passwd rename to test/test-sysusers/test-6.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-6.input b/test/test-sysusers/test-6.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-6.input rename to test/test-sysusers/test-6.input diff --git a/test/TEST-21-SYSUSERS/test-7.expected-group b/test/test-sysusers/test-7.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-7.expected-group rename to test/test-sysusers/test-7.expected-group diff --git a/test/TEST-21-SYSUSERS/test-7.expected-passwd b/test/test-sysusers/test-7.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-7.expected-passwd rename to test/test-sysusers/test-7.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-7.input b/test/test-sysusers/test-7.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-7.input rename to test/test-sysusers/test-7.input diff --git a/test/TEST-21-SYSUSERS/test-8.expected-group b/test/test-sysusers/test-8.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-8.expected-group rename to test/test-sysusers/test-8.expected-group diff --git a/test/TEST-21-SYSUSERS/test-8.expected-passwd b/test/test-sysusers/test-8.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-8.expected-passwd rename to test/test-sysusers/test-8.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-8.input b/test/test-sysusers/test-8.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-8.input rename to test/test-sysusers/test-8.input diff --git a/test/TEST-21-SYSUSERS/test-9.expected-group b/test/test-sysusers/test-9.expected-group similarity index 100% rename from test/TEST-21-SYSUSERS/test-9.expected-group rename to test/test-sysusers/test-9.expected-group diff --git a/test/TEST-21-SYSUSERS/test-9.expected-passwd b/test/test-sysusers/test-9.expected-passwd similarity index 100% rename from test/TEST-21-SYSUSERS/test-9.expected-passwd rename to test/test-sysusers/test-9.expected-passwd diff --git a/test/TEST-21-SYSUSERS/test-9.input b/test/test-sysusers/test-9.input similarity index 100% rename from test/TEST-21-SYSUSERS/test-9.input rename to test/test-sysusers/test-9.input diff --git a/test/TEST-21-SYSUSERS/unhappy-1.expected-err b/test/test-sysusers/unhappy-1.expected-err similarity index 100% rename from test/TEST-21-SYSUSERS/unhappy-1.expected-err rename to test/test-sysusers/unhappy-1.expected-err diff --git a/test/TEST-21-SYSUSERS/unhappy-1.input b/test/test-sysusers/unhappy-1.input similarity index 100% rename from test/TEST-21-SYSUSERS/unhappy-1.input rename to test/test-sysusers/unhappy-1.input diff --git a/test/TEST-21-SYSUSERS/unhappy-2.expected-err b/test/test-sysusers/unhappy-2.expected-err similarity index 100% rename from test/TEST-21-SYSUSERS/unhappy-2.expected-err rename to test/test-sysusers/unhappy-2.expected-err diff --git a/test/TEST-21-SYSUSERS/unhappy-2.input b/test/test-sysusers/unhappy-2.input similarity index 100% rename from test/TEST-21-SYSUSERS/unhappy-2.input rename to test/test-sysusers/unhappy-2.input diff --git a/test/TEST-21-SYSUSERS/unhappy-3.expected-err b/test/test-sysusers/unhappy-3.expected-err similarity index 100% rename from test/TEST-21-SYSUSERS/unhappy-3.expected-err rename to test/test-sysusers/unhappy-3.expected-err diff --git a/test/TEST-21-SYSUSERS/unhappy-3.input b/test/test-sysusers/unhappy-3.input similarity index 100% rename from test/TEST-21-SYSUSERS/unhappy-3.input rename to test/test-sysusers/unhappy-3.input From 69a7c5fb1fc5aba77269701ceb417e441ae15f6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 27 Sep 2020 11:30:17 +0200 Subject: [PATCH 08/12] test-sysusers: sort examples This shouldn't affect the outcome, but makes outputs easier to compare. --- test/test-sysusers.sh.in | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test-sysusers.sh.in b/test/test-sysusers.sh.in index ac07c2145b..b84d6f2d80 100755 --- a/test/test-sysusers.sh.in +++ b/test/test-sysusers.sh.in @@ -41,7 +41,7 @@ compare() { rm -f $TESTDIR/etc/sysusers.d/* $TESTDIR/usr/lib/sysusers.d/* # happy tests -for f in $SOURCE/test-*.input; do +for f in $(ls -1 $SOURCE/test-*.input | sort -V); do echo "*** Running $f" prepare_testdir ${f%.input} cp $f $TESTDIR/usr/lib/sysusers.d/test.conf @@ -50,7 +50,7 @@ for f in $SOURCE/test-*.input; do compare $f "" done -for f in $SOURCE/test-*.input; do +for f in $(ls -1 $SOURCE/test-*.input | sort -V); do echo "*** Running $f on stdin" prepare_testdir ${f%.input} touch $TESTDIR/etc/sysusers.d/test.conf @@ -59,7 +59,7 @@ for f in $SOURCE/test-*.input; do compare $f "on stdin" done -for f in $SOURCE/test-*.input; do +for f in $(ls -1 $SOURCE/test-*.input | sort -V); do echo "*** Running $f on stdin with --replace" prepare_testdir ${f%.input} touch $TESTDIR/etc/sysusers.d/test.conf @@ -98,7 +98,7 @@ compare $SOURCE/inline "(--inline --replace=…)" rm -f $TESTDIR/etc/sysusers.d/* $TESTDIR/usr/lib/sysusers.d/* # tests for error conditions -for f in $SOURCE/unhappy-*.input; do +for f in $(ls -1 $SOURCE/unhappy-*.input | sort -V); do echo "*** Running test $f" prepare_testdir ${f%.input} cp $f $TESTDIR/usr/lib/sysusers.d/test.conf From 044df624aaa7293f82d2da48eb553cdf0ac780d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Oct 2020 14:16:46 +0200 Subject: [PATCH 09/12] test-sysusers: fix how paths are calculated We were looking at ${f%.*}, i.e. the $f with any suffix starting with a dot removed. This worked fine for paths like /some/path/test-11.input. It also worked for paths like /some/path/inline (there were no dots, so we got $f back unscathed). But in the ubuntu CI the package is built in a temporary directory like /tmp/autopkgtest-lxc.nnnfqb26/downtmp/build.UfW/ (yes, it has a dot, even two.). That still worked for the first case, but in the second case we truncated things after the first dot, and we would try to get /tmp/autopkgtest-lxc.nnnfqb26/downtmp/build and try to load /tmp/autopkgtest-lxc.nnnfqb26/downtmp/build.expected-password, which obviously didn't work as expected. To avoid this issue, do the suffix removal only when we know that there really is a suffix. A second minor issue was that we would try to copy $1.expected-*, and sometimes $1 would be given, and sometimes not. Effectively we were relying on there not being any files matching .expected-*. There weren't any such files, but let's avoid this ugliness and always pass $1. --- test/test-sysusers.sh.in | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/test-sysusers.sh.in b/test/test-sysusers.sh.in index b84d6f2d80..39d238c1f7 100755 --- a/test/test-sysusers.sh.in +++ b/test/test-sysusers.sh.in @@ -27,12 +27,12 @@ preprocess() { } compare() { - if ! diff -u $TESTDIR/etc/passwd <(preprocess ${1%.*}.expected-passwd); then - echo "**** Unexpected output for $f" + if ! diff -u $TESTDIR/etc/passwd <(preprocess $1.expected-passwd); then + echo "**** Unexpected output for $f $2" exit 1 fi - if ! diff -u $TESTDIR/etc/group <(preprocess ${1%.*}.expected-group); then + if ! diff -u $TESTDIR/etc/group <(preprocess $1.expected-group); then echo "**** Unexpected output for $f $2" exit 1 fi @@ -47,7 +47,7 @@ for f in $(ls -1 $SOURCE/test-*.input | sort -V); do cp $f $TESTDIR/usr/lib/sysusers.d/test.conf $SYSUSERS --root=$TESTDIR - compare $f "" + compare ${f%.*} "" done for f in $(ls -1 $SOURCE/test-*.input | sort -V); do @@ -56,7 +56,7 @@ for f in $(ls -1 $SOURCE/test-*.input | sort -V); do touch $TESTDIR/etc/sysusers.d/test.conf cat $f | $SYSUSERS --root=$TESTDIR - - compare $f "on stdin" + compare ${f%.*} "on stdin" done for f in $(ls -1 $SOURCE/test-*.input | sort -V); do @@ -68,12 +68,12 @@ for f in $(ls -1 $SOURCE/test-*.input | sort -V); do # this should be ignored cat $SOURCE/test-1.input | $SYSUSERS --root=$TESTDIR --replace=/usr/lib/sysusers.d/test.conf - - compare $f "on stdin with --replace" + compare ${f%.*} "on stdin with --replace" done # test --inline echo "*** Testing --inline" -prepare_testdir +prepare_testdir $SOURCE/inline # copy a random file to make sure it is ignored cp $f $TESTDIR/etc/sysusers.d/confuse.conf $SYSUSERS --root=$TESTDIR --inline \ @@ -84,7 +84,7 @@ compare $SOURCE/inline "(--inline)" # test --replace echo "*** Testing --inline with --replace" -prepare_testdir +prepare_testdir $SOURCE/inline # copy a random file to make sure it is ignored cp $f $TESTDIR/etc/sysusers.d/confuse.conf $SYSUSERS --root=$TESTDIR \ From aa25270cb22f5f7ca2b18c288d4e15bbc6eb239e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 25 Sep 2020 17:16:06 +0200 Subject: [PATCH 10/12] sysusers: look at login.defs when setting the default range to allocate users Also, even if login.defs are not present, don't start allocating at 1, but at SYSTEM_UID_MIN. Fixes #9769. The test is adjusted. Actually, it was busted before, because sysusers would never use SYSTEM_GID_MIN, so if SYSTEM_GID_MIN was different than SYSTEM_UID_MIN, the tests would fail. On all "normal" systems the two are equal, so we didn't notice. Since sysusers now always uses the minimum of the two, we only need to substitute one value. --- meson.build | 1 + src/shared/user-record.c | 13 +++-- src/shared/user-record.h | 1 + src/sysusers/sysusers.c | 22 ++++++-- src/test/test-user-record.c | 48 ++++++++++++++++++ test/test-sysusers.sh.in | 59 ++++++++++++++++++++-- test/test-sysusers/test-10.expected-group | 2 +- test/test-sysusers/test-10.expected-passwd | 2 +- test/test-sysusers/test-13.expected-group | 2 +- test/test-sysusers/test-13.expected-passwd | 2 +- test/test-sysusers/test-14.expected-passwd | 2 +- test/test-sysusers/test-2.expected-group | 2 +- test/test-sysusers/test-2.expected-passwd | 2 +- test/test-sysusers/test-2.input | 2 +- test/test-sysusers/test-6.expected-group | 2 +- test/test-sysusers/test-6.expected-passwd | 2 +- test/test-sysusers/test-8.expected-passwd | 2 +- test/test-sysusers/test-9.expected-passwd | 2 +- 18 files changed, 143 insertions(+), 25 deletions(-) diff --git a/meson.build b/meson.build index 3bec86db81..23cf3e528a 100644 --- a/meson.build +++ b/meson.build @@ -1467,6 +1467,7 @@ foreach term : ['analyze', have = get_option(term) name = 'ENABLE_' + term.underscorify().to_upper() conf.set10(name, have) + substs.set10(name, have) endforeach enable_sysusers = conf.get('ENABLE_SYSUSERS') == 1 diff --git a/src/shared/user-record.c b/src/shared/user-record.c index 3ba78d455f..7e7b28eb55 100644 --- a/src/shared/user-record.c +++ b/src/shared/user-record.c @@ -37,21 +37,24 @@ static int parse_alloc_uid(const char *path, const char *name, const char *t, ui *ret_uid = uid; return 0; } +#endif -static int read_login_defs(UGIDAllocationRange *ret_defs, const char *path) { - _cleanup_fclose_ FILE *f = NULL; +int read_login_defs(UGIDAllocationRange *ret_defs, const char *path, const char *root) { UGIDAllocationRange defs = { .system_alloc_uid_min = SYSTEM_ALLOC_UID_MIN, .system_uid_max = SYSTEM_UID_MAX, .system_alloc_gid_min = SYSTEM_ALLOC_GID_MIN, .system_gid_max = SYSTEM_GID_MAX, }; + +#if ENABLE_COMPAT_MUTABLE_UID_BOUNDARIES + _cleanup_fclose_ FILE *f = NULL; int r; if (!path) path = "/etc/login.defs"; - r = fopen_unlocked(path, "re", &f); + r = chase_symlinks_and_fopen_unlocked(path, root, CHASE_PREFIX_ROOT, "re", &f, NULL); if (r == -ENOENT) goto assign; if (r < 0) @@ -88,11 +91,11 @@ static int read_login_defs(UGIDAllocationRange *ret_defs, const char *path) { defs.system_alloc_gid_min = MIN(defs.system_gid_max - 1, (gid_t) SYSTEM_ALLOC_GID_MIN); /* Look at sys_gid_max to make sure sys_gid_min..sys_gid_max remains a valid range. */ } +#endif *ret_defs = defs; return 0; } -#endif const UGIDAllocationRange *acquire_ugid_allocation_range(void) { #if ENABLE_COMPAT_MUTABLE_UID_BOUNDARIES @@ -114,7 +117,7 @@ const UGIDAllocationRange *acquire_ugid_allocation_range(void) { static thread_local bool initialized = false; if (!initialized) { - (void) read_login_defs(&defs, NULL); + (void) read_login_defs(&defs, NULL, NULL); initialized = true; } #endif diff --git a/src/shared/user-record.h b/src/shared/user-record.h index 1f87eff6d5..2e74b910c2 100644 --- a/src/shared/user-record.h +++ b/src/shared/user-record.h @@ -43,6 +43,7 @@ typedef struct UGIDAllocationRange { gid_t system_gid_max; } UGIDAllocationRange; +int read_login_defs(UGIDAllocationRange *ret_defs, const char *path, const char *root); const UGIDAllocationRange *acquire_ugid_allocation_range(void); typedef enum UserDisposition { diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 7349e9fcb9..987950d602 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -26,6 +26,7 @@ #include "strv.h" #include "tmpfile-util-label.h" #include "uid-range.h" +#include "user-record.h" #include "user-util.h" #include "utf8.h" #include "util.h" @@ -1949,10 +1950,25 @@ static int run(int argc, char *argv[]) { return log_error_errno(errno, "Failed to set SYSTEMD_NSS_BYPASS_SYNTHETIC environment variable: %m"); if (!uid_range) { - /* Default to default range of 1..SYSTEM_UID_MAX */ - r = uid_range_add(&uid_range, &n_uid_range, 1, SYSTEM_UID_MAX); + /* Default to default range of SYSTEMD_UID_MIN..SYSTEM_UID_MAX. */ + UGIDAllocationRange defs; + + r = read_login_defs(&defs, NULL, arg_root); if (r < 0) - return log_oom(); + return log_error_errno(r, "Failed to read %s%s: %m", + strempty(arg_root), "/etc/login.defs"); + + /* 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, + * 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 = defs.system_alloc_uid_min, + end = MIN3((uid_t) SYSTEM_UID_MAX, defs.system_uid_max, defs.system_gid_max); + if (begin < end) { + r = uid_range_add(&uid_range, &n_uid_range, begin, end - begin + 1); + if (r < 0) + return log_oom(); + } } r = add_implicit(); diff --git a/src/test/test-user-record.c b/src/test/test-user-record.c index fcab61d694..d623706648 100644 --- a/src/test/test-user-record.c +++ b/src/test/test-user-record.c @@ -3,10 +3,55 @@ #include #include +#include "fd-util.h" +#include "fileio.h" #include "format-util.h" +#include "fs-util.h" +#include "tmpfile-util.h" #include "tests.h" #include "user-record.h" +static void test_read_login_defs(const char *path) { + log_info("/* %s(\"%s\") */", __func__, path ?: ""); + + _cleanup_(unlink_tempfilep) char name[] = "/tmp/test-user-record.XXXXXX"; + _cleanup_fclose_ FILE *f = NULL; + if (!path) { + assert_se(fmkostemp_safe(name, "r+", &f) == 0); + fprintf(f, + "SYS_UID_MIN "UID_FMT"\n" + "SYS_UID_MAX "UID_FMT"\n" + "SYS_GID_MIN "GID_FMT"\n" + "SYS_GID_MAX "GID_FMT"\n", + SYSTEM_ALLOC_UID_MIN + 5, + SYSTEM_UID_MAX + 5, + SYSTEM_ALLOC_GID_MIN + 5, + SYSTEM_GID_MAX + 5); + assert_se(fflush_and_check(f) >= 0); + } + + UGIDAllocationRange defs; + assert_se(read_login_defs(&defs, path ?: name, NULL) >= 0); + + log_info("system_alloc_uid_min="UID_FMT, defs.system_alloc_uid_min); + log_info("system_uid_max="UID_FMT, defs.system_uid_max); + log_info("system_alloc_gid_min="GID_FMT, defs.system_alloc_gid_min); + log_info("system_gid_max="GID_FMT, defs.system_gid_max); + + if (!path) { + uid_t offset = ENABLE_COMPAT_MUTABLE_UID_BOUNDARIES ? 5 : 0; + assert_se(defs.system_alloc_uid_min == SYSTEM_ALLOC_UID_MIN + offset); + assert_se(defs.system_uid_max == SYSTEM_UID_MAX + offset); + assert_se(defs.system_alloc_gid_min == SYSTEM_ALLOC_GID_MIN + offset); + assert_se(defs.system_gid_max == SYSTEM_GID_MAX + offset); + } else if (streq(path, "/dev/null")) { + assert_se(defs.system_alloc_uid_min == SYSTEM_ALLOC_UID_MIN); + assert_se(defs.system_uid_max == SYSTEM_UID_MAX); + assert_se(defs.system_alloc_gid_min == SYSTEM_ALLOC_GID_MIN); + assert_se(defs.system_gid_max == SYSTEM_GID_MAX); + } +} + static void test_acquire_ugid_allocation_range(void) { log_info("/* %s */", __func__); @@ -48,6 +93,9 @@ static void test_gid_is_system(void) { int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); + test_read_login_defs("/dev/null"); + test_read_login_defs("/etc/login.defs"); + test_read_login_defs(NULL); test_acquire_ugid_allocation_range(); test_uid_is_system(); test_gid_is_system(); diff --git a/test/test-sysusers.sh.in b/test/test-sysusers.sh.in index 39d238c1f7..6e133cc841 100755 --- a/test/test-sysusers.sh.in +++ b/test/test-sysusers.sh.in @@ -20,19 +20,22 @@ prepare_testdir() { return 0 } +[ @SYSTEM_UID_MAX@ -lt @SYSTEM_GID_MAX@ ] && system_guid_max=@SYSTEM_UID_MAX@ || system_guid_max=@SYSTEM_GID_MAX@ + preprocess() { - sed -e "s/SYSTEM_UID_MAX/@SYSTEM_UID_MAX@/g" \ - -e "s/SYSTEM_GID_MAX/@SYSTEM_GID_MAX@/g" \ - -e "s#NOLOGIN#@NOLOGIN@#g" "$1" + m=${2:-$system_guid_max} + + sed -e "s/SYSTEM_UGID_MAX/$m/g; + s#NOLOGIN#@NOLOGIN@#g" "$1" } compare() { - if ! diff -u $TESTDIR/etc/passwd <(preprocess $1.expected-passwd); then + if ! diff -u $TESTDIR/etc/passwd <(preprocess $1.expected-passwd $3); then echo "**** Unexpected output for $f $2" exit 1 fi - if ! diff -u $TESTDIR/etc/group <(preprocess $1.expected-group); then + if ! diff -u $TESTDIR/etc/group <(preprocess $1.expected-group $3); then echo "**** Unexpected output for $f $2" exit 1 fi @@ -97,6 +100,52 @@ compare $SOURCE/inline "(--inline --replace=…)" rm -f $TESTDIR/etc/sysusers.d/* $TESTDIR/usr/lib/sysusers.d/* +cat >$TESTDIR/etc/login.defs < Date: Thu, 1 Oct 2020 14:55:22 +0200 Subject: [PATCH 11/12] tests: when creating temp dirs, include test name in path This makes it easier to figure out which directory we want to look at when tests fail, and also which test left behind a directory when it shouldn't. --- src/partition/test-repart.sh | 2 +- test/hwdb-test.sh | 2 +- test/test-network-generator-conversion.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/partition/test-repart.sh b/src/partition/test-repart.sh index 5765978290..9af3049b6b 100755 --- a/src/partition/test-repart.sh +++ b/src/partition/test-repart.sh @@ -6,7 +6,7 @@ set -ex repart=$1 test -x $repart -D=$(mktemp --directory) +D=$(mktemp --tmpdir --directory "test-repart.XXXXXXXXXX") trap "rm -rf '$D'" EXIT INT QUIT PIPE mkdir -p $D/definitions diff --git a/test/hwdb-test.sh b/test/hwdb-test.sh index d12f82fa2d..8b909f7d80 100755 --- a/test/hwdb-test.sh +++ b/test/hwdb-test.sh @@ -18,7 +18,7 @@ if [ ! -x "$SYSTEMD_HWDB" ]; then exit 1 fi -D=$(mktemp --directory) +D=$(mktemp --tmpdir --directory "hwdb-test.XXXXXXXXXX") trap "rm -rf '$D'" EXIT INT QUIT PIPE mkdir -p "$D/etc/udev" ln -s "$ROOTDIR/hwdb.d" "$D/etc/udev/hwdb.d" diff --git a/test/test-network-generator-conversion.sh b/test/test-network-generator-conversion.sh index d0d0834518..50df69f1b0 100755 --- a/test/test-network-generator-conversion.sh +++ b/test/test-network-generator-conversion.sh @@ -17,7 +17,7 @@ for f in "$src"/test-*.input; do echo "*** Running $f" ( - out=$(mktemp --directory) + out=$(mktemp --tmpdir --directory "test-network-generator-conversion.XXXXXXXXXX") trap "rm -rf '$out'" EXIT INT QUIT PIPE $generator --root "$out" -- $(cat $f) From 4b6f9b202e0d2edaa32a2520534be33dcba3be39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Oct 2020 18:18:26 +0200 Subject: [PATCH 12/12] sysusers: emit warnings about login.defs overrides on first user or group creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *** Running /home/zbyszek/src/systemd-work/test/test-sysusers/test-14.input (with login.defs symlinked) login.defs specifies UID allocation range 401–555 that is different than the built-in defaults (201–998) login.defs specifies GID allocation range 405–666 that is different than the built-in defaults (201–990) --- src/sysusers/sysusers.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 987950d602..37ac46b441 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -84,6 +84,9 @@ static uid_t search_uid = UID_INVALID; static UidRange *uid_range = NULL; static unsigned n_uid_range = 0; +static UGIDAllocationRange login_defs = {}; +static bool login_defs_need_warning = false; + STATIC_DESTRUCTOR_REGISTER(groups, ordered_hashmap_freep); STATIC_DESTRUCTOR_REGISTER(users, ordered_hashmap_freep); STATIC_DESTRUCTOR_REGISTER(members, ordered_hashmap_freep); @@ -105,6 +108,26 @@ static int errno_is_not_exists(int code) { return IN_SET(code, 0, ENOENT, ESRCH, EBADF, EPERM); } +static void maybe_emit_login_defs_warning(void) { + if (!login_defs_need_warning) + return; + + if (login_defs.system_alloc_uid_min != SYSTEM_ALLOC_UID_MIN || + login_defs.system_uid_max != SYSTEM_UID_MAX) + log_warning("login.defs specifies UID allocation range "UID_FMT"–"UID_FMT + " that is different than the built-in defaults ("UID_FMT"–"UID_FMT")", + login_defs.system_alloc_uid_min, login_defs.system_uid_max, + SYSTEM_ALLOC_UID_MIN, SYSTEM_UID_MAX); + if (login_defs.system_alloc_gid_min != SYSTEM_ALLOC_GID_MIN || + login_defs.system_gid_max != SYSTEM_GID_MAX) + log_warning("login.defs specifies GID allocation range "GID_FMT"–"GID_FMT + " that is different than the built-in defaults ("GID_FMT"–"GID_FMT")", + login_defs.system_alloc_gid_min, login_defs.system_gid_max, + SYSTEM_ALLOC_GID_MIN, SYSTEM_GID_MAX); + + login_defs_need_warning = false; +} + static int load_user_database(void) { _cleanup_fclose_ FILE *f = NULL; const char *passwd_path; @@ -1002,6 +1025,8 @@ static int add_user(Item *i) { /* And if that didn't work either, let's try to find a free one */ if (!i->uid_set) { + maybe_emit_login_defs_warning(); + for (;;) { r = uid_range_next_lower(uid_range, n_uid_range, &search_uid); if (r < 0) @@ -1168,6 +1193,8 @@ static int add_group(Item *i) { /* And if that didn't work either, let's try to find a free one */ if (!i->gid_set) { + maybe_emit_login_defs_warning(); + for (;;) { /* We look for new GIDs in the UID pool! */ r = uid_range_next_lower(uid_range, n_uid_range, &search_uid); @@ -1951,19 +1978,19 @@ static int run(int argc, char *argv[]) { if (!uid_range) { /* Default to default range of SYSTEMD_UID_MIN..SYSTEM_UID_MAX. */ - UGIDAllocationRange defs; - - r = read_login_defs(&defs, NULL, arg_root); + r = read_login_defs(&login_defs, NULL, arg_root); if (r < 0) return log_error_errno(r, "Failed to read %s%s: %m", strempty(arg_root), "/etc/login.defs"); + 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, * 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 = defs.system_alloc_uid_min, - end = MIN3((uid_t) SYSTEM_UID_MAX, defs.system_uid_max, defs.system_gid_max); + uid_t begin = login_defs.system_alloc_uid_min, + end = MIN3((uid_t) SYSTEM_UID_MAX, login_defs.system_uid_max, login_defs.system_gid_max); if (begin < end) { r = uid_range_add(&uid_range, &n_uid_range, begin, end - begin + 1); if (r < 0)