From d2a236929b1f23918f7e70af9b9ac1fef14edcf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 10:02:14 +0200 Subject: [PATCH 1/4] core: remove one {} --- src/core/dbus-execute.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index c816569f2b..a92897081e 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1368,10 +1368,10 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - STRV_FOREACH(p, l) { + STRV_FOREACH(p, l) if (!isempty(*p) && !valid_user_group_name_or_id(*p)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid supplementary group names"); - } + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Invalid supplementary group names"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { if (strv_isempty(l)) { From 1a29610f5fa1bcb2eeb37d2c6b79d8d1a6dbb865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 09:58:27 +0200 Subject: [PATCH 2/4] shared/user-util: add compat forms of user name checking functions New functions are called valid_user_group_name_compat() and valid_user_group_name_or_id_compat() and accept dots in the user or group name. No functional change except the tests. --- src/basic/user-util.c | 27 +++++++------ src/basic/user-util.h | 16 +++++++- src/test/test-user-util.c | 85 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 113 insertions(+), 15 deletions(-) diff --git a/src/basic/user-util.c b/src/basic/user-util.c index d127b0c107..b1ab84c5f0 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -620,16 +620,19 @@ int take_etc_passwd_lock(const char *root) { return fd; } -bool valid_user_group_name(const char *u) { +bool valid_user_group_name_full(const char *u, bool strict) { const char *i; long sz; /* Checks if the specified name is a valid user/group name. Also see POSIX IEEE Std 1003.1-2008, 2016 Edition, * 3.437. We are a bit stricter here however. Specifically we deviate from POSIX rules: * - * - We don't allow any dots (this would break chown syntax which permits dots as user/group name separator) * - We require that names fit into the appropriate utmp field * - We don't allow empty user names + * - No dots or digits in the first character + * + * If strict==true, additionally: + * - We don't allow any dots (this conflicts with chown syntax which permits dots as user/group name separator) * * Note that other systems are even more restrictive, and don't permit underscores or uppercase characters. */ @@ -642,13 +645,13 @@ bool valid_user_group_name(const char *u) { u[0] != '_') return false; - for (i = u+1; *i; i++) { - if (!(*i >= 'a' && *i <= 'z') && - !(*i >= 'A' && *i <= 'Z') && - !(*i >= '0' && *i <= '9') && - !IN_SET(*i, '_', '-')) + for (i = u+1; *i; i++) + if (!((*i >= 'a' && *i <= 'z') || + (*i >= 'A' && *i <= 'Z') || + (*i >= '0' && *i <= '9') || + IN_SET(*i, '_', '-') || + (!strict && *i == '.'))) return false; - } sz = sysconf(_SC_LOGIN_NAME_MAX); assert_se(sz > 0); @@ -662,15 +665,15 @@ bool valid_user_group_name(const char *u) { return true; } -bool valid_user_group_name_or_id(const char *u) { +bool valid_user_group_name_or_id_full(const char *u, bool strict) { - /* Similar as above, but is also fine with numeric UID/GID specifications, as long as they are in the right - * range, and not the invalid user ids. */ + /* Similar as above, but is also fine with numeric UID/GID specifications, as long as they are in the + * right range, and not the invalid user ids. */ if (isempty(u)) return false; - if (valid_user_group_name(u)) + if (valid_user_group_name_full(u, strict)) return true; return parse_uid(u, NULL) >= 0; diff --git a/src/basic/user-util.h b/src/basic/user-util.h index 52f3df792d..cfa515f5e8 100644 --- a/src/basic/user-util.h +++ b/src/basic/user-util.h @@ -85,8 +85,20 @@ static inline bool userns_supported(void) { return access("/proc/self/uid_map", F_OK) >= 0; } -bool valid_user_group_name(const char *u); -bool valid_user_group_name_or_id(const char *u); +bool valid_user_group_name_full(const char *u, bool strict); +bool valid_user_group_name_or_id_full(const char *u, bool strict); +static inline bool valid_user_group_name(const char *u) { + return valid_user_group_name_full(u, true); +} +static inline bool valid_user_group_name_or_id(const char *u) { + return valid_user_group_name_or_id_full(u, true); +} +static inline bool valid_user_group_name_compat(const char *u) { + return valid_user_group_name_full(u, false); +} +static inline bool valid_user_group_name_or_id_compat(const char *u) { + return valid_user_group_name_or_id_full(u, false); +} bool valid_gecos(const char *d); bool valid_home(const char *p); diff --git a/src/test/test-user-util.c b/src/test/test-user-util.c index e6d7262e78..9475b99c28 100644 --- a/src/test/test-user-util.c +++ b/src/test/test-user-util.c @@ -61,6 +61,43 @@ static void test_uid_ptr(void) { assert_se(PTR_TO_UID(UID_TO_PTR(1000)) == 1000); } +static void test_valid_user_group_name_compat(void) { + log_info("/* %s */", __func__); + + assert_se(!valid_user_group_name_compat(NULL)); + assert_se(!valid_user_group_name_compat("")); + assert_se(!valid_user_group_name_compat("1")); + assert_se(!valid_user_group_name_compat("65535")); + assert_se(!valid_user_group_name_compat("-1")); + assert_se(!valid_user_group_name_compat("-kkk")); + assert_se(!valid_user_group_name_compat("rööt")); + assert_se(!valid_user_group_name_compat(".")); + assert_se(!valid_user_group_name_compat(".eff")); + assert_se(!valid_user_group_name_compat("foo\nbar")); + assert_se(!valid_user_group_name_compat("0123456789012345678901234567890123456789")); + assert_se(!valid_user_group_name_or_id_compat("aaa:bbb")); + assert_se(!valid_user_group_name_compat(".")); + assert_se(!valid_user_group_name_compat(".1")); + assert_se(!valid_user_group_name_compat(".65535")); + assert_se(!valid_user_group_name_compat(".-1")); + assert_se(!valid_user_group_name_compat(".-kkk")); + assert_se(!valid_user_group_name_compat(".rööt")); + assert_se(!valid_user_group_name_or_id_compat(".aaa:bbb")); + + assert_se(valid_user_group_name_compat("root")); + assert_se(valid_user_group_name_compat("lennart")); + assert_se(valid_user_group_name_compat("LENNART")); + assert_se(valid_user_group_name_compat("_kkk")); + assert_se(valid_user_group_name_compat("kkk-")); + assert_se(valid_user_group_name_compat("kk-k")); + assert_se(valid_user_group_name_compat("eff.eff")); + assert_se(valid_user_group_name_compat("eff.")); + + assert_se(valid_user_group_name_compat("some5")); + assert_se(!valid_user_group_name_compat("5some")); + assert_se(valid_user_group_name_compat("INNER5NUMBER")); +} + static void test_valid_user_group_name(void) { log_info("/* %s */", __func__); @@ -72,10 +109,17 @@ static void test_valid_user_group_name(void) { assert_se(!valid_user_group_name("-kkk")); assert_se(!valid_user_group_name("rööt")); assert_se(!valid_user_group_name(".")); - assert_se(!valid_user_group_name("eff.eff")); + assert_se(!valid_user_group_name(".eff")); assert_se(!valid_user_group_name("foo\nbar")); assert_se(!valid_user_group_name("0123456789012345678901234567890123456789")); assert_se(!valid_user_group_name_or_id("aaa:bbb")); + assert_se(!valid_user_group_name(".")); + assert_se(!valid_user_group_name(".1")); + assert_se(!valid_user_group_name(".65535")); + assert_se(!valid_user_group_name(".-1")); + assert_se(!valid_user_group_name(".-kkk")); + assert_se(!valid_user_group_name(".rööt")); + assert_se(!valid_user_group_name_or_id(".aaa:bbb")); assert_se(valid_user_group_name("root")); assert_se(valid_user_group_name("lennart")); @@ -83,12 +127,47 @@ static void test_valid_user_group_name(void) { assert_se(valid_user_group_name("_kkk")); assert_se(valid_user_group_name("kkk-")); assert_se(valid_user_group_name("kk-k")); + assert_se(!valid_user_group_name("eff.eff")); + assert_se(!valid_user_group_name("eff.")); assert_se(valid_user_group_name("some5")); assert_se(!valid_user_group_name("5some")); assert_se(valid_user_group_name("INNER5NUMBER")); } +static void test_valid_user_group_name_or_id_compat(void) { + log_info("/* %s */", __func__); + + assert_se(!valid_user_group_name_or_id_compat(NULL)); + assert_se(!valid_user_group_name_or_id_compat("")); + assert_se(valid_user_group_name_or_id_compat("0")); + assert_se(valid_user_group_name_or_id_compat("1")); + assert_se(valid_user_group_name_or_id_compat("65534")); + assert_se(!valid_user_group_name_or_id_compat("65535")); + assert_se(valid_user_group_name_or_id_compat("65536")); + assert_se(!valid_user_group_name_or_id_compat("-1")); + assert_se(!valid_user_group_name_or_id_compat("-kkk")); + assert_se(!valid_user_group_name_or_id_compat("rööt")); + assert_se(!valid_user_group_name_or_id_compat(".")); + assert_se(!valid_user_group_name_or_id_compat(".eff")); + assert_se(valid_user_group_name_or_id_compat("eff.eff")); + assert_se(valid_user_group_name_or_id_compat("eff.")); + assert_se(!valid_user_group_name_or_id_compat("foo\nbar")); + assert_se(!valid_user_group_name_or_id_compat("0123456789012345678901234567890123456789")); + assert_se(!valid_user_group_name_or_id_compat("aaa:bbb")); + + assert_se(valid_user_group_name_or_id_compat("root")); + assert_se(valid_user_group_name_or_id_compat("lennart")); + assert_se(valid_user_group_name_or_id_compat("LENNART")); + assert_se(valid_user_group_name_or_id_compat("_kkk")); + assert_se(valid_user_group_name_or_id_compat("kkk-")); + assert_se(valid_user_group_name_or_id_compat("kk-k")); + + assert_se(valid_user_group_name_or_id_compat("some5")); + assert_se(!valid_user_group_name_or_id_compat("5some")); + assert_se(valid_user_group_name_or_id_compat("INNER5NUMBER")); +} + static void test_valid_user_group_name_or_id(void) { log_info("/* %s */", __func__); @@ -103,7 +182,9 @@ static void test_valid_user_group_name_or_id(void) { assert_se(!valid_user_group_name_or_id("-kkk")); assert_se(!valid_user_group_name_or_id("rööt")); assert_se(!valid_user_group_name_or_id(".")); + assert_se(!valid_user_group_name_or_id(".eff")); assert_se(!valid_user_group_name_or_id("eff.eff")); + assert_se(!valid_user_group_name_or_id("eff.")); assert_se(!valid_user_group_name_or_id("foo\nbar")); assert_se(!valid_user_group_name_or_id("0123456789012345678901234567890123456789")); assert_se(!valid_user_group_name_or_id("aaa:bbb")); @@ -230,7 +311,9 @@ int main(int argc, char *argv[]) { test_parse_uid(); test_uid_ptr(); + test_valid_user_group_name_compat(); test_valid_user_group_name(); + test_valid_user_group_name_or_id_compat(); test_valid_user_group_name_or_id(); test_valid_gecos(); test_valid_home(); From ae480f0b09aec815b64579bb1828ea935d8ee236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 09:58:27 +0200 Subject: [PATCH 3/4] shared/user-util: allow usernames with dots in specific fields People do have usernames with dots, and it makes them very unhappy that systemd doesn't like their that. It seems that there is no actual problem with allowing dots in the username. In particular chown declares ":" as the official separator, and internally in systemd we never rely on "." as the seperator between user and group (nor do we call chown directly). Using dots in the name is probably not a very good idea, but we don't need to care. Debian tools (adduser) do not allow users with dots to be created. This patch allows *existing* names with dots to be used in User, Group, SupplementaryGroups, SocketUser, SocketGroup fields, both in unit files and on the command line. DynamicUsers and sysusers still follow the strict policy. user@.service and tmpfiles already allowed arbitrary user names, and this remains unchanged. Fixes #12754. --- src/core/dbus-execute.c | 6 +++--- src/core/dbus-socket.c | 4 ++-- src/core/dbus-util.c | 2 +- src/core/dbus-util.h | 2 +- src/core/load-fragment-gperf.gperf.m4 | 10 +++++----- src/core/load-fragment.c | 8 ++++---- src/core/load-fragment.h | 4 ++-- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index a92897081e..68726c2cc0 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1187,10 +1187,10 @@ int bus_exec_context_set_transient_property( flags |= UNIT_PRIVATE; if (streq(name, "User")) - return bus_set_transient_user(u, name, &c->user, message, flags, error); + return bus_set_transient_user_compat(u, name, &c->user, message, flags, error); if (streq(name, "Group")) - return bus_set_transient_user(u, name, &c->group, message, flags, error); + return bus_set_transient_user_compat(u, name, &c->group, message, flags, error); if (streq(name, "TTYPath")) return bus_set_transient_path(u, name, &c->tty_path, message, flags, error); @@ -1369,7 +1369,7 @@ int bus_exec_context_set_transient_property( return r; STRV_FOREACH(p, l) - if (!isempty(*p) && !valid_user_group_name_or_id(*p)) + if (!isempty(*p) && !valid_user_group_name_or_id_compat(*p)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid supplementary group names"); diff --git a/src/core/dbus-socket.c b/src/core/dbus-socket.c index e895c94e12..25d3d71391 100644 --- a/src/core/dbus-socket.c +++ b/src/core/dbus-socket.c @@ -277,10 +277,10 @@ static int bus_socket_set_transient_property( return bus_set_transient_fdname(u, name, &s->fdname, message, flags, error); if (streq(name, "SocketUser")) - return bus_set_transient_user(u, name, &s->user, message, flags, error); + return bus_set_transient_user_compat(u, name, &s->user, message, flags, error); if (streq(name, "SocketGroup")) - return bus_set_transient_user(u, name, &s->group, message, flags, error); + return bus_set_transient_user_compat(u, name, &s->group, message, flags, error); if (streq(name, "BindIPv6Only")) return bus_set_transient_bind_ipv6_only(u, name, &s->bind_ipv6_only, message, flags, error); diff --git a/src/core/dbus-util.c b/src/core/dbus-util.c index f4fbb72cb9..7862beaacb 100644 --- a/src/core/dbus-util.c +++ b/src/core/dbus-util.c @@ -30,7 +30,7 @@ int bus_property_get_triggered_unit( BUS_DEFINE_SET_TRANSIENT(mode_t, "u", uint32_t, mode_t, "%040o"); BUS_DEFINE_SET_TRANSIENT(unsigned, "u", uint32_t, unsigned, "%" PRIu32); -BUS_DEFINE_SET_TRANSIENT_STRING_WITH_CHECK(user, valid_user_group_name_or_id); +BUS_DEFINE_SET_TRANSIENT_STRING_WITH_CHECK(user_compat, valid_user_group_name_or_id_compat); BUS_DEFINE_SET_TRANSIENT_STRING_WITH_CHECK(path, path_is_absolute); int bus_set_transient_string( diff --git a/src/core/dbus-util.h b/src/core/dbus-util.h index 12b055e4ac..a3316c6701 100644 --- a/src/core/dbus-util.h +++ b/src/core/dbus-util.h @@ -235,7 +235,7 @@ int bus_property_get_triggered_unit(sd_bus *bus, const char *path, const char *i int bus_set_transient_mode_t(Unit *u, const char *name, mode_t *p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); int bus_set_transient_unsigned(Unit *u, const char *name, unsigned *p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); -int bus_set_transient_user(Unit *u, const char *name, char **p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); +int bus_set_transient_user_compat(Unit *u, const char *name, char **p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); int bus_set_transient_path(Unit *u, const char *name, char **p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); int bus_set_transient_string(Unit *u, const char *name, char **p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); int bus_set_transient_bool(Unit *u, const char *name, bool *p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index 10e4801cfe..58a28e3241 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -25,9 +25,9 @@ m4_define(`EXEC_CONTEXT_CONFIG_ITEMS', `$1.WorkingDirectory, config_parse_working_directory, 0, offsetof($1, exec_context) $1.RootDirectory, config_parse_unit_path_printf, true, offsetof($1, exec_context.root_directory) $1.RootImage, config_parse_unit_path_printf, true, offsetof($1, exec_context.root_image) -$1.User, config_parse_user_group, 0, offsetof($1, exec_context.user) -$1.Group, config_parse_user_group, 0, offsetof($1, exec_context.group) -$1.SupplementaryGroups, config_parse_user_group_strv, 0, offsetof($1, exec_context.supplementary_groups) +$1.User, config_parse_user_group_compat, 0, offsetof($1, exec_context.user) +$1.Group, config_parse_user_group_compat, 0, offsetof($1, exec_context.group) +$1.SupplementaryGroups, config_parse_user_group_strv_compat, 0, offsetof($1, exec_context.supplementary_groups) $1.Nice, config_parse_exec_nice, 0, offsetof($1, exec_context) $1.OOMScoreAdjust, config_parse_exec_oom_score_adjust, 0, offsetof($1, exec_context) $1.IOSchedulingClass, config_parse_exec_io_class, 0, offsetof($1, exec_context) @@ -365,8 +365,8 @@ Socket.ExecStartPost, config_parse_exec, SOCKET_EXEC Socket.ExecStopPre, config_parse_exec, SOCKET_EXEC_STOP_PRE, offsetof(Socket, exec_command) Socket.ExecStopPost, config_parse_exec, SOCKET_EXEC_STOP_POST, offsetof(Socket, exec_command) Socket.TimeoutSec, config_parse_sec_fix_0, 0, offsetof(Socket, timeout_usec) -Socket.SocketUser, config_parse_user_group, 0, offsetof(Socket, user) -Socket.SocketGroup, config_parse_user_group, 0, offsetof(Socket, group) +Socket.SocketUser, config_parse_user_group_compat, 0, offsetof(Socket, user) +Socket.SocketGroup, config_parse_user_group_compat, 0, offsetof(Socket, group) Socket.SocketMode, config_parse_mode, 0, offsetof(Socket, socket_mode) Socket.DirectoryMode, config_parse_mode, 0, offsetof(Socket, directory_mode) Socket.Accept, config_parse_bool, 0, offsetof(Socket, accept) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 20079e1fb1..a4564f8a9d 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2004,7 +2004,7 @@ int config_parse_sec_fix_0( return 0; } -int config_parse_user_group( +int config_parse_user_group_compat( const char *unit, const char *filename, unsigned line, @@ -2037,7 +2037,7 @@ int config_parse_user_group( return -ENOEXEC; } - if (!valid_user_group_name_or_id(k)) { + if (!valid_user_group_name_or_id_compat(k)) { log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid user/group name or numeric ID: %s", k); return -ENOEXEC; } @@ -2045,7 +2045,7 @@ int config_parse_user_group( return free_and_replace(*user, k); } -int config_parse_user_group_strv( +int config_parse_user_group_strv_compat( const char *unit, const char *filename, unsigned line, @@ -2091,7 +2091,7 @@ int config_parse_user_group_strv( return -ENOEXEC; } - if (!valid_user_group_name_or_id(k)) { + if (!valid_user_group_name_or_id_compat(k)) { log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid user/group name or numeric ID: %s", k); return -ENOEXEC; } diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index 326e80893b..664643f08e 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -99,8 +99,8 @@ CONFIG_PARSER_PROTOTYPE(config_parse_exec_utmp_mode); CONFIG_PARSER_PROTOTYPE(config_parse_working_directory); CONFIG_PARSER_PROTOTYPE(config_parse_fdname); CONFIG_PARSER_PROTOTYPE(config_parse_sec_fix_0); -CONFIG_PARSER_PROTOTYPE(config_parse_user_group); -CONFIG_PARSER_PROTOTYPE(config_parse_user_group_strv); +CONFIG_PARSER_PROTOTYPE(config_parse_user_group_compat); +CONFIG_PARSER_PROTOTYPE(config_parse_user_group_strv_compat); CONFIG_PARSER_PROTOTYPE(config_parse_restrict_namespaces); CONFIG_PARSER_PROTOTYPE(config_parse_bind_paths); CONFIG_PARSER_PROTOTYPE(config_parse_exec_keyring_mode); From 88e2ed0b5bf6f08f5a2d4d64b1fefdc7192b9aac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 28 Aug 2019 12:05:52 +0200 Subject: [PATCH 4/4] shared/user-util: emit a warning on names with dots --- src/basic/user-util.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/basic/user-util.c b/src/basic/user-util.c index b1ab84c5f0..3b253bc264 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -645,13 +645,26 @@ bool valid_user_group_name_full(const char *u, bool strict) { u[0] != '_') return false; - for (i = u+1; *i; i++) - if (!((*i >= 'a' && *i <= 'z') || - (*i >= 'A' && *i <= 'Z') || - (*i >= '0' && *i <= '9') || - IN_SET(*i, '_', '-') || - (!strict && *i == '.'))) - return false; + bool warned = false; + + for (i = u+1; *i; i++) { + if (((*i >= 'a' && *i <= 'z') || + (*i >= 'A' && *i <= 'Z') || + (*i >= '0' && *i <= '9') || + IN_SET(*i, '_', '-'))) + continue; + + if (*i == '.' && !strict) { + if (!warned) { + log_warning("Bad user or group name \"%s\", accepting for compatibility.", u); + warned = true; + } + + continue; + } + + return false; + } sz = sysconf(_SC_LOGIN_NAME_MAX); assert_se(sz > 0);