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] 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; +}