From 2aea5883f1d016ec7304acdb59516c30cae92452 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Apr 2020 22:27:09 +0200 Subject: [PATCH 1/5] userdbctl: drop redundant user name validity check The userdb_by_name() invocation immediately following does the same check anyway, no need to do this twice. (Also, make sure we exit the function early on failure) --- src/userdb/userdbctl.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/userdb/userdbctl.c b/src/userdb/userdbctl.c index b3ddd9d141..793ef6b07c 100644 --- a/src/userdb/userdbctl.c +++ b/src/userdb/userdbctl.c @@ -541,16 +541,15 @@ static int ssh_authorized_keys(int argc, char *argv[], void *userdata) { _cleanup_(user_record_unrefp) UserRecord *ur = NULL; int r; - if (!valid_user_group_name(argv[1])) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid user name '%s'.", argv[1]); - r = userdb_by_name(argv[1], arg_userdb_flags, &ur); if (r == -ESRCH) - log_error_errno(r, "User %s does not exist.", argv[1]); + return log_error_errno(r, "User %s does not exist.", argv[1]); else if (r == -EHOSTDOWN) - log_error_errno(r, "Selected user database service is not available for this request."); + return log_error_errno(r, "Selected user database service is not available for this request."); + else if (r == -EINVAL) + return log_error_errno(r, "Failed to find user %s: %m (Invalid user name?)", argv[1]); else if (r < 0) - log_error_errno(r, "Failed to find user %s: %m", argv[1]); + return log_error_errno(r, "Failed to find user %s: %m", argv[1]); if (strv_isempty(ur->ssh_authorized_keys)) log_debug("User record for %s has no public SSH keys.", argv[1]); From 7a8867abfab10e5bbca10590ec2aa40c5b27d8fb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 4 Apr 2020 12:23:02 +0200 Subject: [PATCH 2/5] user-util: rework how we validate user names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reworks the user validation infrastructure. There are now two modes. In regular mode we are strict and test against a strict set of valid chars. And in "relaxed" mode we just filter out some really obvious, dangerous stuff. i.e. strict is whitelisting what is OK, but "relaxed" is blacklisting what is really not OK. The idea is that we use strict mode whenver we allocate a new user (i.e. in sysusers.d or homed), while "relaxed" mode is when we process users registered elsewhere, (i.e. userdb, logind, …) The requirements on user name validity vary wildly. SSSD thinks its fine to embedd "@" for example, while the suggested NAME_REGEX field on Debian does not even allow uppercase chars… This effectively liberaralizes a lot what we expect from usernames. The code that warns about questionnable user names is now optional and only used at places such as unit file parsing, so that it doesn't show up on every userdb query, but only when processing configuration files that know better. Fixes: #15149 #15090 --- src/basic/user-util.c | 182 ++++++++++++++---------- src/basic/user-util.h | 21 +-- src/core/dbus-execute.c | 6 +- src/core/dbus-manager.c | 2 +- src/core/dbus-socket.c | 4 +- src/core/dbus-util.c | 7 +- src/core/dbus-util.h | 2 +- src/core/dynamic-user.c | 2 +- src/core/load-fragment.c | 4 +- src/core/unit.c | 2 +- src/home/home-util.c | 2 +- src/home/homectl.c | 6 +- src/home/homed-manager-bus.c | 6 +- src/home/pam_systemd_home.c | 2 +- src/nss-systemd/nss-systemd.c | 6 +- src/shared/group-record.c | 10 +- src/shared/json.c | 2 +- src/shared/json.h | 1 + src/shared/user-record.c | 8 +- src/shared/userdb.c | 8 +- src/systemd/sd-messages.h | 3 + src/sysusers/sysusers.c | 6 +- src/test/test-user-util.c | 255 ++++++++++++++++++---------------- 23 files changed, 302 insertions(+), 245 deletions(-) diff --git a/src/basic/user-util.c b/src/basic/user-util.c index 1510fc96ef..2e3580017d 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -10,6 +10,8 @@ #include #include +#include "sd-messages.h" + #include "alloc-util.h" #include "errno-util.h" #include "fd-util.h" @@ -18,6 +20,7 @@ #include "macro.h" #include "parse-util.h" #include "path-util.h" +#include "path-util.h" #include "random-util.h" #include "string-util.h" #include "strv.h" @@ -698,92 +701,125 @@ int take_etc_passwd_lock(const char *root) { return fd; } -bool valid_user_group_name_full(const char *u, bool strict) { +bool valid_user_group_name(const char *u, ValidUserFlags flags) { const char *i; - long sz; - bool warned = false; - /* 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: + /* Checks if the specified name is a valid user/group name. There are two flavours of this call: + * strict mode is the default which is POSIX plus some extra rules; and relaxed mode where we accept + * pretty much everything except the really worst offending names. * - * - We require that names fit into the appropriate utmp field - * - We don't allow empty user names - * - No dots 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) - * - We don't allow a digit as the first character - * - * Note that other systems are even more restrictive, and don't permit underscores or uppercase characters. - */ + * Whenever we synthesize users ourselves we should use the strict mode. But when we process users + * created by other stuff, let's be more liberal. */ - if (isempty(u)) + if (isempty(u)) /* An empty user name is never valid */ return false; - if (!(u[0] >= 'a' && u[0] <= 'z') && - !(u[0] >= 'A' && u[0] <= 'Z') && - !(u[0] >= '0' && u[0] <= '9' && !strict) && - u[0] != '_') - return false; + if (parse_uid(u, NULL) >= 0) /* Something that parses as numeric UID string is valid exactly when the + * flag for it is set */ + return FLAGS_SET(flags, VALID_USER_ALLOW_NUMERIC); - bool only_digits_seen = u[0] >= '0' && u[0] <= '9'; + if (FLAGS_SET(flags, VALID_USER_RELAX)) { - if (only_digits_seen) { - log_warning("User or group name \"%s\" starts with a digit, accepting for compatibility.", u); - warned = true; + /* In relaxed mode we just check very superficially. Apparently SSSD and other stuff is + * extremely liberal (way too liberal if you ask me, even inserting "@" in user names, which + * is bound to cause problems for example when used with an MTA), hence only filter the most + * obvious cases, or where things would result in an invalid entry if such a user name would + * show up in /etc/passwd (or equivalent getent output). + * + * Note that we stepped far out of POSIX territory here. It's not our fault though, but + * SSSD's, Samba's and everybody else who ignored POSIX on this. (I mean, I am happy to step + * outside of POSIX' bounds any day, but I must say in this case I probably wouldn't + * have...) */ + + if (startswith(u, " ") || endswith(u, " ")) /* At least expect whitespace padding is removed + * at front and back (accept in the middle, since + * that's apparently a thing on Windows). Note + * that this also blocks usernames consisting of + * whitespace only. */ + return false; + + if (!utf8_is_valid(u)) /* We want to synthesize JSON from this, hence insist on UTF-8 */ + return false; + + if (string_has_cc(u, NULL)) /* CC characters are just dangerous (and \n in particular is the + * record separator in /etc/passwd), so we can't allow that. */ + return false; + + if (strpbrk(u, ":/")) /* Colons are the field separator in /etc/passwd, we can't allow + * that. Slashes are special to file systems paths and user names + * typically show up in the file system as home directories, hence + * don't allow slashes. */ + return false; + + if (in_charset(u, "0123456789")) /* Don't allow fully numeric strings, they might be confused + * with with UIDs (note that this test is more broad than + * the parse_uid() test above, as it will cover more than + * the 32bit range, and it will detect 65535 (which is in + * invalid UID, even though in the unsigned 32 bit range) */ + return false; + + if (u[0] == '-' && in_charset(u + 1, "0123456789")) /* Don't allow negative fully numeric + * strings either. After all some people + * write 65535 as -1 (even though that's + * not even true on 32bit uid_t + * anyway) */ + return false; + + if (dot_or_dot_dot(u)) /* User names typically become home directory names, and these two are + * special in that context, don't allow that. */ + return false; + + /* Compare with strict result and warn if result doesn't match */ + if (FLAGS_SET(flags, VALID_USER_WARN) && !valid_user_group_name(u, 0)) + log_struct(LOG_NOTICE, + "MESSAGE=Accepting user/group name '%s', which does not match strict user/group name rules.", u, + "USER_GROUP_NAME=%s", u, + "MESSAGE_ID=" SD_MESSAGE_UNSAFE_USER_NAME_STR); + + /* Note that we make no restrictions on the length in relaxed mode! */ + } else { + long sz; + size_t l; + + /* 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 empty user names (see above) + * - We require that names fit into the appropriate utmp field + * - We don't allow any dots (this conflicts with chown syntax which permits dots as user/group name separator) + * - We don't allow dashes or digit as the first character + * + * Note that other systems are even more restrictive, and don't permit underscores or uppercase characters. + */ + + if (!(u[0] >= 'a' && u[0] <= 'z') && + !(u[0] >= 'A' && u[0] <= 'Z') && + 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, '_', '-')) + return false; + + l = i - u; + + sz = sysconf(_SC_LOGIN_NAME_MAX); + assert_se(sz > 0); + + if (l > (size_t) sz) + return false; + if (l > FILENAME_MAX) + return false; + if (l > UT_NAMESIZE - 1) + return false; } - for (i = u+1; *i; i++) { - if (((*i >= 'a' && *i <= 'z') || - (*i >= 'A' && *i <= 'Z') || - (*i >= '0' && *i <= '9') || - IN_SET(*i, '_', '-'))) { - if (!(*i >= '0' && *i <= '9')) - only_digits_seen = false; - continue; - } - - if (*i == '.' && !strict) { - if (!warned) { - log_warning("Bad user or group name \"%s\", accepting for compatibility.", u); - warned = true; - } - - continue; - } - - return false; - } - - if (only_digits_seen) - return false; - - sz = sysconf(_SC_LOGIN_NAME_MAX); - assert_se(sz > 0); - - if ((size_t) (i-u) > (size_t) sz) - return false; - - if ((size_t) (i-u) > UT_NAMESIZE - 1) - return false; - return true; } -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. */ - - if (isempty(u)) - return false; - - if (parse_uid(u, NULL) >= 0) - return true; - - return valid_user_group_name_full(u, strict); -} - bool valid_gecos(const char *d) { if (!d) diff --git a/src/basic/user-util.h b/src/basic/user-util.h index 6796ac42c1..1f267d21a3 100644 --- a/src/basic/user-util.h +++ b/src/basic/user-util.h @@ -97,20 +97,13 @@ static inline bool userns_supported(void) { return access("/proc/self/uid_map", F_OK) >= 0; } -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); -} +typedef enum ValidUserFlags { + VALID_USER_RELAX = 1 << 0, + VALID_USER_WARN = 1 << 1, + VALID_USER_ALLOW_NUMERIC = 1 << 2, +} ValidUserFlags; + +bool valid_user_group_name(const char *u, ValidUserFlags flags); bool valid_gecos(const char *d); bool valid_home(const char *p); diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 5696a60ba8..93857436b4 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1204,10 +1204,10 @@ int bus_exec_context_set_transient_property( flags |= UNIT_PRIVATE; if (streq(name, "User")) - return bus_set_transient_user_compat(u, name, &c->user, message, flags, error); + return bus_set_transient_user_relaxed(u, name, &c->user, message, flags, error); if (streq(name, "Group")) - return bus_set_transient_user_compat(u, name, &c->group, message, flags, error); + return bus_set_transient_user_relaxed(u, name, &c->group, message, flags, error); if (streq(name, "TTYPath")) return bus_set_transient_path(u, name, &c->tty_path, message, flags, error); @@ -1392,7 +1392,7 @@ int bus_exec_context_set_transient_property( return r; STRV_FOREACH(p, l) - if (!isempty(*p) && !valid_user_group_name_or_id_compat(*p)) + if (!isempty(*p) && !valid_user_group_name(*p, VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX|VALID_USER_WARN)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid supplementary group names"); diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 60f55aef5f..ef4bb316cc 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1643,7 +1643,7 @@ static int method_lookup_dynamic_user_by_name(sd_bus_message *message, void *use if (!MANAGER_IS_SYSTEM(m)) return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Dynamic users are only supported in the system instance."); - if (!valid_user_group_name(name)) + if (!valid_user_group_name(name, VALID_USER_RELAX)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "User name invalid: %s", name); r = dynamic_user_lookup_name(m, name, &uid); diff --git a/src/core/dbus-socket.c b/src/core/dbus-socket.c index c8253c7940..ad7b41a95b 100644 --- a/src/core/dbus-socket.c +++ b/src/core/dbus-socket.c @@ -278,10 +278,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_compat(u, name, &s->user, message, flags, error); + return bus_set_transient_user_relaxed(u, name, &s->user, message, flags, error); if (streq(name, "SocketGroup")) - return bus_set_transient_user_compat(u, name, &s->group, message, flags, error); + return bus_set_transient_user_relaxed(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 7862beaacb..951450e53d 100644 --- a/src/core/dbus-util.c +++ b/src/core/dbus-util.c @@ -30,7 +30,12 @@ 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_compat, valid_user_group_name_or_id_compat); + +static inline bool valid_user_group_name_or_id_relaxed(const char *u) { + return valid_user_group_name(u, VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX); +} + +BUS_DEFINE_SET_TRANSIENT_STRING_WITH_CHECK(user_relaxed, valid_user_group_name_or_id_relaxed); 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 ec8c245fff..654ceb5279 100644 --- a/src/core/dbus-util.h +++ b/src/core/dbus-util.h @@ -236,7 +236,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_compat(Unit *u, const char *name, char **p, sd_bus_message *message, UnitWriteFlags flags, sd_bus_error *error); +int bus_set_transient_user_relaxed(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/dynamic-user.c b/src/core/dynamic-user.c index f1819b36bc..58b37925bf 100644 --- a/src/core/dynamic-user.c +++ b/src/core/dynamic-user.c @@ -116,7 +116,7 @@ static int dynamic_user_acquire(Manager *m, const char *name, DynamicUser** ret) return 0; } - if (!valid_user_group_name_or_id(name)) + if (!valid_user_group_name(name, VALID_USER_ALLOW_NUMERIC)) return -EINVAL; if (socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, storage_socket) < 0) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 646364eb89..f3fe73535e 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2068,7 +2068,7 @@ int config_parse_user_group_compat( return -ENOEXEC; } - if (!valid_user_group_name_or_id_compat(k)) { + if (!valid_user_group_name(k, VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX|VALID_USER_WARN)) { log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid user/group name or numeric ID: %s", k); return -ENOEXEC; } @@ -2122,7 +2122,7 @@ int config_parse_user_group_strv_compat( return -ENOEXEC; } - if (!valid_user_group_name_or_id_compat(k)) { + if (!valid_user_group_name(k, VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX|VALID_USER_WARN)) { log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid user/group name or numeric ID: %s", k); return -ENOEXEC; } diff --git a/src/core/unit.c b/src/core/unit.c index 96e1a6c320..95d574d8d4 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4287,7 +4287,7 @@ static int user_from_unit_name(Unit *u, char **ret) { if (r < 0) return r; - if (valid_user_group_name(n)) { + if (valid_user_group_name(n, 0)) { *ret = TAKE_PTR(n); return 0; } diff --git a/src/home/home-util.c b/src/home/home-util.c index a53b0d3391..69ab645484 100644 --- a/src/home/home-util.c +++ b/src/home/home-util.c @@ -17,7 +17,7 @@ bool suitable_user_name(const char *name) { * restrictive, so that we can change the rules server-side without having to update things * client-side too. */ - if (!valid_user_group_name(name)) + if (!valid_user_group_name(name, 0)) return false; /* We generally rely on NSS to tell us which users not to care for, but let's filter out some diff --git a/src/home/homectl.c b/src/home/homectl.c index 1ccc053d3f..66d4bb6bd6 100644 --- a/src/home/homectl.c +++ b/src/home/homectl.c @@ -540,7 +540,7 @@ static int inspect_home(int argc, char *argv[], void *userdata) { r = parse_uid(*i, &uid); if (r < 0) { - if (!valid_user_group_name(*i)) { + if (!valid_user_group_name(*i, 0)) { log_error("Invalid user name '%s'.", *i); if (ret == 0) ret = -EINVAL; @@ -1395,7 +1395,7 @@ static int create_home(int argc, char *argv[], void *userdata) { if (argc >= 2) { /* If a username was specified, use it */ - if (valid_user_group_name(argv[1])) + if (valid_user_group_name(argv[1], 0)) r = json_variant_set_field_string(&arg_identity_extra, "userName", argv[1]); else { _cleanup_free_ char *un = NULL, *rr = NULL; @@ -3357,7 +3357,7 @@ static int parse_argv(int argc, char *argv[]) { if (r == 0) break; - if (!valid_user_group_name(word)) + if (!valid_user_group_name(word, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid group name %s.", word); mo = json_variant_ref(json_variant_by_key(arg_identity_extra, "memberOf")); diff --git a/src/home/homed-manager-bus.c b/src/home/homed-manager-bus.c index fce8545274..34a7b49452 100644 --- a/src/home/homed-manager-bus.c +++ b/src/home/homed-manager-bus.c @@ -81,7 +81,7 @@ static int method_get_home_by_name( r = sd_bus_message_read(message, "s", &user_name); if (r < 0) return r; - if (!valid_user_group_name(user_name)) + if (!valid_user_group_name(user_name, 0)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "User name %s is not valid", user_name); h = hashmap_get(m->homes_by_name, user_name); @@ -212,7 +212,7 @@ static int method_get_user_record_by_name( r = sd_bus_message_read(message, "s", &user_name); if (r < 0) return r; - if (!valid_user_group_name(user_name)) + if (!valid_user_group_name(user_name, 0)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "User name %s is not valid", user_name); h = hashmap_get(m->homes_by_name, user_name); @@ -287,7 +287,7 @@ static int generic_home_method( if (r < 0) return r; - if (!valid_user_group_name(user_name)) + if (!valid_user_group_name(user_name, 0)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "User name %s is not valid", user_name); h = hashmap_get(m->homes_by_name, user_name); diff --git a/src/home/pam_systemd_home.c b/src/home/pam_systemd_home.c index 440ed85e2c..afc72412d4 100644 --- a/src/home/pam_systemd_home.c +++ b/src/home/pam_systemd_home.c @@ -88,7 +88,7 @@ static int acquire_user_record( /* Let's bypass all IPC complexity for the two user names we know for sure we don't manage, and for * user names we don't consider valid. */ - if (STR_IN_SET(username, "root", NOBODY_USER_NAME) || !valid_user_group_name(username)) + if (STR_IN_SET(username, "root", NOBODY_USER_NAME) || !valid_user_group_name(username, 0)) return PAM_USER_UNKNOWN; /* Let's check if a previous run determined that this user is not managed by homed. If so, let's exit early */ diff --git a/src/nss-systemd/nss-systemd.c b/src/nss-systemd/nss-systemd.c index 581b7959bd..c1e780edcb 100644 --- a/src/nss-systemd/nss-systemd.c +++ b/src/nss-systemd/nss-systemd.c @@ -96,7 +96,7 @@ enum nss_status _nss_systemd_getpwnam_r( /* If the username is not valid, then we don't know it. Ideally libc would filter these for us * anyway. We don't generate EINVAL here, because it isn't really out business to complain about * invalid user names. */ - if (!valid_user_group_name(name)) + if (!valid_user_group_name(name, VALID_USER_RELAX)) return NSS_STATUS_NOTFOUND; /* Synthesize entries for the root and nobody users, in case they are missing in /etc/passwd */ @@ -193,7 +193,7 @@ enum nss_status _nss_systemd_getgrnam_r( assert(gr); assert(errnop); - if (!valid_user_group_name(name)) + if (!valid_user_group_name(name, VALID_USER_RELAX)) return NSS_STATUS_NOTFOUND; /* Synthesize records for root and nobody, in case they are missing from /etc/group */ @@ -536,7 +536,7 @@ enum nss_status _nss_systemd_initgroups_dyn( assert(groupsp); assert(errnop); - if (!valid_user_group_name(user_name)) + if (!valid_user_group_name(user_name, VALID_USER_RELAX)) return NSS_STATUS_NOTFOUND; /* Don't allow extending these two special users, the same as we won't resolve them via getpwnam() */ diff --git a/src/shared/group-record.c b/src/shared/group-record.c index d5ec32f07d..3c9520693d 100644 --- a/src/shared/group-record.c +++ b/src/shared/group-record.c @@ -86,8 +86,8 @@ static int dispatch_per_machine(const char *name, JsonVariant *variant, JsonDisp { "matchMachineId", _JSON_VARIANT_TYPE_INVALID, NULL, 0, 0 }, { "matchHostname", _JSON_VARIANT_TYPE_INVALID, NULL, 0, 0 }, { "gid", JSON_VARIANT_UNSIGNED, json_dispatch_uid_gid, offsetof(GroupRecord, gid), 0 }, - { "members", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(GroupRecord, members), 0 }, - { "administrators", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(GroupRecord, administrators), 0 }, + { "members", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(GroupRecord, members), JSON_RELAX}, + { "administrators", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(GroupRecord, administrators), JSON_RELAX}, {}, }; @@ -190,14 +190,14 @@ int group_record_load( UserRecordLoadFlags load_flags) { static const JsonDispatch group_dispatch_table[] = { - { "groupName", JSON_VARIANT_STRING, json_dispatch_user_group_name, offsetof(GroupRecord, group_name), 0 }, + { "groupName", JSON_VARIANT_STRING, json_dispatch_user_group_name, offsetof(GroupRecord, group_name), JSON_RELAX}, { "realm", JSON_VARIANT_STRING, json_dispatch_realm, offsetof(GroupRecord, realm), 0 }, { "disposition", JSON_VARIANT_STRING, json_dispatch_user_disposition, offsetof(GroupRecord, disposition), 0 }, { "service", JSON_VARIANT_STRING, json_dispatch_string, offsetof(GroupRecord, service), JSON_SAFE }, { "lastChangeUSec", JSON_VARIANT_UNSIGNED, json_dispatch_uint64, offsetof(GroupRecord, last_change_usec), 0 }, { "gid", JSON_VARIANT_UNSIGNED, json_dispatch_uid_gid, offsetof(GroupRecord, gid), 0 }, - { "members", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(GroupRecord, members), 0 }, - { "administrators", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(GroupRecord, administrators), 0 }, + { "members", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(GroupRecord, members), JSON_RELAX}, + { "administrators", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(GroupRecord, administrators), JSON_RELAX}, { "privileged", JSON_VARIANT_OBJECT, dispatch_privileged, 0, 0 }, diff --git a/src/shared/json.c b/src/shared/json.c index e28a5df19e..7a79acdee8 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -4107,7 +4107,7 @@ int json_dispatch_user_group_name(const char *name, JsonVariant *variant, JsonDi return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a string.", strna(name)); n = json_variant_string(variant); - if (!valid_user_group_name_compat(n)) + if (!valid_user_group_name(n, FLAGS_SET(flags, JSON_RELAX) ? VALID_USER_RELAX : 0)) return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is not a valid user/group name.", strna(name)); r = free_and_strdup(s, n); diff --git a/src/shared/json.h b/src/shared/json.h index b9b300a1d2..a4e5b6f507 100644 --- a/src/shared/json.h +++ b/src/shared/json.h @@ -255,6 +255,7 @@ typedef enum JsonDispatchFlags { JSON_MANDATORY = 1 << 1, /* Should existence of this property be mandatory? */ JSON_LOG = 1 << 2, /* Should the parser log about errors? */ JSON_SAFE = 1 << 3, /* Don't accept "unsafe" strings in json_dispatch_string() + json_dispatch_string() */ + JSON_RELAX = 1 << 4, /* Use relaxed user name checking in json_dispatch_user_group_name */ /* The following two may be passed into log_json() in addition to the three above */ JSON_DEBUG = 1 << 4, /* Indicates that this log message is a debug message */ diff --git a/src/shared/user-record.c b/src/shared/user-record.c index ff2cc41143..080c2a140d 100644 --- a/src/shared/user-record.c +++ b/src/shared/user-record.c @@ -600,7 +600,7 @@ int json_dispatch_user_group_list(const char *name, JsonVariant *variant, JsonDi if (!json_variant_is_string(e)) return json_log(e, flags, SYNTHETIC_ERRNO(EINVAL), "JSON array element is not a string."); - if (!valid_user_group_name_compat(json_variant_string(e))) + if (!valid_user_group_name(json_variant_string(e), FLAGS_SET(flags, JSON_RELAX) ? VALID_USER_RELAX : 0)) return json_log(e, flags, SYNTHETIC_ERRNO(EINVAL), "JSON array element is not a valid user/group name: %s", json_variant_string(e)); r = strv_extend(&l, json_variant_string(e)); @@ -938,7 +938,7 @@ static int dispatch_per_machine(const char *name, JsonVariant *variant, JsonDisp { "imagePath", JSON_VARIANT_STRING, json_dispatch_path, offsetof(UserRecord, image_path), 0 }, { "uid", JSON_VARIANT_UNSIGNED, json_dispatch_uid_gid, offsetof(UserRecord, uid), 0 }, { "gid", JSON_VARIANT_UNSIGNED, json_dispatch_uid_gid, offsetof(UserRecord, gid), 0 }, - { "memberOf", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(UserRecord, member_of), 0 }, + { "memberOf", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(UserRecord, member_of), JSON_RELAX}, { "fileSystemType", JSON_VARIANT_STRING, json_dispatch_string, offsetof(UserRecord, file_system_type), JSON_SAFE }, { "partitionUuid", JSON_VARIANT_STRING, json_dispatch_id128, offsetof(UserRecord, partition_uuid), 0 }, { "luksUuid", JSON_VARIANT_STRING, json_dispatch_id128, offsetof(UserRecord, luks_uuid), 0 }, @@ -1231,7 +1231,7 @@ int user_group_record_mangle( int user_record_load(UserRecord *h, JsonVariant *v, UserRecordLoadFlags load_flags) { static const JsonDispatch user_dispatch_table[] = { - { "userName", JSON_VARIANT_STRING, json_dispatch_user_group_name, offsetof(UserRecord, user_name), 0 }, + { "userName", JSON_VARIANT_STRING, json_dispatch_user_group_name, offsetof(UserRecord, user_name), JSON_RELAX}, { "realm", JSON_VARIANT_STRING, json_dispatch_realm, offsetof(UserRecord, realm), 0 }, { "realName", JSON_VARIANT_STRING, json_dispatch_gecos, offsetof(UserRecord, real_name), 0 }, { "emailAddress", JSON_VARIANT_STRING, json_dispatch_string, offsetof(UserRecord, email_address), JSON_SAFE }, @@ -1270,7 +1270,7 @@ int user_record_load(UserRecord *h, JsonVariant *v, UserRecordLoadFlags load_fla { "homeDirectory", JSON_VARIANT_STRING, json_dispatch_home_directory, offsetof(UserRecord, home_directory), 0 }, { "uid", JSON_VARIANT_UNSIGNED, json_dispatch_uid_gid, offsetof(UserRecord, uid), 0 }, { "gid", JSON_VARIANT_UNSIGNED, json_dispatch_uid_gid, offsetof(UserRecord, gid), 0 }, - { "memberOf", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(UserRecord, member_of), 0 }, + { "memberOf", JSON_VARIANT_ARRAY, json_dispatch_user_group_list, offsetof(UserRecord, member_of), JSON_RELAX}, { "fileSystemType", JSON_VARIANT_STRING, json_dispatch_string, offsetof(UserRecord, file_system_type), JSON_SAFE }, { "partitionUuid", JSON_VARIANT_STRING, json_dispatch_id128, offsetof(UserRecord, partition_uuid), 0 }, { "luksUuid", JSON_VARIANT_STRING, json_dispatch_id128, offsetof(UserRecord, luks_uuid), 0 }, diff --git a/src/shared/userdb.c b/src/shared/userdb.c index 92f8796768..0769a792c2 100644 --- a/src/shared/userdb.c +++ b/src/shared/userdb.c @@ -587,7 +587,7 @@ int userdb_by_name(const char *name, UserDBFlags flags, UserRecord **ret) { _cleanup_(json_variant_unrefp) JsonVariant *query = NULL; int r; - if (!valid_user_group_name_compat(name)) + if (!valid_user_group_name(name, VALID_USER_RELAX)) return -EINVAL; r = json_build(&query, JSON_BUILD_OBJECT( @@ -795,7 +795,7 @@ int groupdb_by_name(const char *name, UserDBFlags flags, GroupRecord **ret) { _cleanup_(json_variant_unrefp) JsonVariant *query = NULL; int r; - if (!valid_user_group_name_compat(name)) + if (!valid_user_group_name(name, VALID_USER_RELAX)) return -EINVAL; r = json_build(&query, JSON_BUILD_OBJECT( @@ -982,7 +982,7 @@ int membershipdb_by_user(const char *name, UserDBFlags flags, UserDBIterator **r assert(ret); - if (!valid_user_group_name_compat(name)) + if (!valid_user_group_name(name, VALID_USER_RELAX)) return -EINVAL; r = json_build(&query, JSON_BUILD_OBJECT( @@ -1025,7 +1025,7 @@ int membershipdb_by_group(const char *name, UserDBFlags flags, UserDBIterator ** assert(ret); - if (!valid_user_group_name_compat(name)) + if (!valid_user_group_name(name, VALID_USER_RELAX)) return -EINVAL; r = json_build(&query, JSON_BUILD_OBJECT( diff --git a/src/systemd/sd-messages.h b/src/systemd/sd-messages.h index e8d26ab71b..162b650e64 100644 --- a/src/systemd/sd-messages.h +++ b/src/systemd/sd-messages.h @@ -158,6 +158,9 @@ _SD_BEGIN_DECLARATIONS; #define SD_MESSAGE_DNSSEC_DOWNGRADE SD_ID128_MAKE(36,db,2d,fa,5a,90,45,e1,bd,4a,f5,f9,3e,1c,f0,57) #define SD_MESSAGE_DNSSEC_DOWNGRADE_STR SD_ID128_MAKE_STR(36,db,2d,fa,5a,90,45,e1,bd,4a,f5,f9,3e,1c,f0,57) +#define SD_MESSAGE_UNSAFE_USER_NAME SD_ID128_MAKE(b6,1f,da,c6,12,e9,4b,91,82,28,5b,99,88,43,06,1f) +#define SD_MESSAGE_UNSAFE_USER_NAME_STR SD_ID128_MAKE_STR(b6,1f,da,c6,12,e9,4b,91,82,28,5b,99,88,43,06,1f) + _SD_END_DECLARATIONS; #endif diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index f7cc7e0900..c3e5457fc1 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -1445,7 +1445,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (r < 0) log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", fname, line, name); - if (!valid_user_group_name(resolved_name)) + if (!valid_user_group_name(resolved_name, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "[%s:%u] '%s' is not a valid user or group name.", fname, line, resolved_name); @@ -1548,7 +1548,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { "[%s:%u] Lines of type 'm' require a group name in the third field.", fname, line); - if (!valid_user_group_name(resolved_id)) + if (!valid_user_group_name(resolved_id, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "[%s:%u] '%s' is not a valid user or group name.", fname, line, resolved_id); @@ -1589,7 +1589,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (split_pair(resolved_id, ":", &uid, &gid) == 0) { r = parse_gid(gid, &i->gid); if (r < 0) { - if (valid_user_group_name(gid)) + if (valid_user_group_name(gid, 0)) i->group_name = TAKE_PTR(gid); else return log_error_errno(r, "Failed to parse GID: '%s': %m", id); diff --git a/src/test/test-user-util.c b/src/test/test-user-util.c index 11c01e5189..a0e1495186 100644 --- a/src/test/test-user-util.c +++ b/src/test/test-user-util.c @@ -63,144 +63,163 @@ 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) { +static void test_valid_user_group_name_relaxed(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(NULL, VALID_USER_RELAX)); + assert_se(!valid_user_group_name("", VALID_USER_RELAX)); + assert_se(!valid_user_group_name("1", VALID_USER_RELAX)); + assert_se(!valid_user_group_name("65535", VALID_USER_RELAX)); + assert_se(!valid_user_group_name("-1", VALID_USER_RELAX)); + assert_se(!valid_user_group_name("foo\nbar", VALID_USER_RELAX)); + assert_se(!valid_user_group_name("0123456789012345678901234567890123456789", VALID_USER_RELAX)); + assert_se(!valid_user_group_name("aaa:bbb", VALID_USER_RELAX|VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name(".aaa:bbb", VALID_USER_RELAX|VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name(".", VALID_USER_RELAX)); + assert_se(!valid_user_group_name("..", VALID_USER_RELAX)); - 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("root", VALID_USER_RELAX)); + assert_se(valid_user_group_name("lennart", VALID_USER_RELAX)); + assert_se(valid_user_group_name("LENNART", VALID_USER_RELAX)); + assert_se(valid_user_group_name("_kkk", VALID_USER_RELAX)); + assert_se(valid_user_group_name("kkk-", VALID_USER_RELAX)); + assert_se(valid_user_group_name("kk-k", VALID_USER_RELAX)); + assert_se(valid_user_group_name("eff.eff", VALID_USER_RELAX)); + assert_se(valid_user_group_name("eff.", VALID_USER_RELAX)); + assert_se(valid_user_group_name("-kkk", VALID_USER_RELAX)); + assert_se(valid_user_group_name("rööt", VALID_USER_RELAX)); + assert_se(valid_user_group_name(".eff", VALID_USER_RELAX)); + assert_se(valid_user_group_name(".1", VALID_USER_RELAX)); + assert_se(valid_user_group_name(".65535", VALID_USER_RELAX)); + assert_se(valid_user_group_name(".-1", VALID_USER_RELAX)); + assert_se(valid_user_group_name(".-kkk", VALID_USER_RELAX)); + assert_se(valid_user_group_name(".rööt", VALID_USER_RELAX)); + assert_se(valid_user_group_name("...", VALID_USER_RELAX)); - assert_se(valid_user_group_name_compat("some5")); - assert_se(valid_user_group_name_compat("5some")); - assert_se(valid_user_group_name_compat("INNER5NUMBER")); + assert_se(valid_user_group_name("some5", VALID_USER_RELAX)); + assert_se(valid_user_group_name("5some", VALID_USER_RELAX)); + assert_se(valid_user_group_name("INNER5NUMBER", VALID_USER_RELAX)); + + assert_se(valid_user_group_name("piff.paff@ad.domain.example", VALID_USER_RELAX)); + assert_se(valid_user_group_name("Dāvis", VALID_USER_RELAX)); } static void test_valid_user_group_name(void) { log_info("/* %s */", __func__); - assert_se(!valid_user_group_name(NULL)); - 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(".")); - 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(NULL, 0)); + assert_se(!valid_user_group_name("", 0)); + assert_se(!valid_user_group_name("1", 0)); + assert_se(!valid_user_group_name("65535", 0)); + assert_se(!valid_user_group_name("-1", 0)); + assert_se(!valid_user_group_name("-kkk", 0)); + assert_se(!valid_user_group_name("rööt", 0)); + assert_se(!valid_user_group_name(".", 0)); + assert_se(!valid_user_group_name(".eff", 0)); + assert_se(!valid_user_group_name("foo\nbar", 0)); + assert_se(!valid_user_group_name("0123456789012345678901234567890123456789", 0)); + assert_se(!valid_user_group_name("aaa:bbb", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name(".", 0)); + assert_se(!valid_user_group_name("..", 0)); + assert_se(!valid_user_group_name("...", 0)); + assert_se(!valid_user_group_name(".1", 0)); + assert_se(!valid_user_group_name(".65535", 0)); + assert_se(!valid_user_group_name(".-1", 0)); + assert_se(!valid_user_group_name(".-kkk", 0)); + assert_se(!valid_user_group_name(".rööt", 0)); + assert_se(!valid_user_group_name(".aaa:bbb", VALID_USER_ALLOW_NUMERIC)); - assert_se(valid_user_group_name("root")); - assert_se(valid_user_group_name("lennart")); - assert_se(valid_user_group_name("LENNART")); - 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("root", 0)); + assert_se(valid_user_group_name("lennart", 0)); + assert_se(valid_user_group_name("LENNART", 0)); + assert_se(valid_user_group_name("_kkk", 0)); + assert_se(valid_user_group_name("kkk-", 0)); + assert_se(valid_user_group_name("kk-k", 0)); + assert_se(!valid_user_group_name("eff.eff", 0)); + assert_se(!valid_user_group_name("eff.", 0)); - assert_se(valid_user_group_name("some5")); - assert_se(!valid_user_group_name("5some")); - assert_se(valid_user_group_name("INNER5NUMBER")); + assert_se(valid_user_group_name("some5", 0)); + assert_se(!valid_user_group_name("5some", 0)); + assert_se(valid_user_group_name("INNER5NUMBER", 0)); + + assert_se(!valid_user_group_name("piff.paff@ad.domain.example", 0)); + assert_se(!valid_user_group_name("Dāvis", 0)); } -static void test_valid_user_group_name_or_id_compat(void) { +static void test_valid_user_group_name_or_numeric_relaxed(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(NULL, VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(!valid_user_group_name("", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("0", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("1", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("65534", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(!valid_user_group_name("65535", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("65536", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(!valid_user_group_name("-1", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(!valid_user_group_name("foo\nbar", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(!valid_user_group_name("0123456789012345678901234567890123456789", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(!valid_user_group_name("aaa:bbb", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(!valid_user_group_name(".", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(!valid_user_group_name("..", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); - 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("root", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("lennart", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("LENNART", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("_kkk", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("kkk-", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("kk-k", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("-kkk", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("rööt", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name(".eff", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("eff.eff", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("eff.", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("...", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); - 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")); + assert_se(valid_user_group_name("some5", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("5some", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("INNER5NUMBER", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + + assert_se(valid_user_group_name("piff.paff@ad.domain.example", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); + assert_se(valid_user_group_name("Dāvis", VALID_USER_ALLOW_NUMERIC|VALID_USER_RELAX)); } -static void test_valid_user_group_name_or_id(void) { +static void test_valid_user_group_name_or_numeric(void) { log_info("/* %s */", __func__); - assert_se(!valid_user_group_name_or_id(NULL)); - assert_se(!valid_user_group_name_or_id("")); - assert_se(valid_user_group_name_or_id("0")); - assert_se(valid_user_group_name_or_id("1")); - assert_se(valid_user_group_name_or_id("65534")); - assert_se(!valid_user_group_name_or_id("65535")); - assert_se(valid_user_group_name_or_id("65536")); - assert_se(!valid_user_group_name_or_id("-1")); - 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")); + assert_se(!valid_user_group_name(NULL, VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("", VALID_USER_ALLOW_NUMERIC)); + assert_se(valid_user_group_name("0", VALID_USER_ALLOW_NUMERIC)); + assert_se(valid_user_group_name("1", VALID_USER_ALLOW_NUMERIC)); + assert_se(valid_user_group_name("65534", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("65535", VALID_USER_ALLOW_NUMERIC)); + assert_se(valid_user_group_name("65536", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("-1", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("-kkk", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("rööt", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name(".", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("..", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("...", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name(".eff", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("eff.eff", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("eff.", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("foo\nbar", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("0123456789012345678901234567890123456789", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("aaa:bbb", VALID_USER_ALLOW_NUMERIC)); - assert_se(valid_user_group_name_or_id("root")); - assert_se(valid_user_group_name_or_id("lennart")); - assert_se(valid_user_group_name_or_id("LENNART")); - assert_se(valid_user_group_name_or_id("_kkk")); - assert_se(valid_user_group_name_or_id("kkk-")); - assert_se(valid_user_group_name_or_id("kk-k")); + assert_se(valid_user_group_name("root", VALID_USER_ALLOW_NUMERIC)); + assert_se(valid_user_group_name("lennart", VALID_USER_ALLOW_NUMERIC)); + assert_se(valid_user_group_name("LENNART", VALID_USER_ALLOW_NUMERIC)); + assert_se(valid_user_group_name("_kkk", VALID_USER_ALLOW_NUMERIC)); + assert_se(valid_user_group_name("kkk-", VALID_USER_ALLOW_NUMERIC)); + assert_se(valid_user_group_name("kk-k", VALID_USER_ALLOW_NUMERIC)); - assert_se(valid_user_group_name_or_id("some5")); - assert_se(!valid_user_group_name_or_id("5some")); - assert_se(valid_user_group_name_or_id("INNER5NUMBER")); + assert_se(valid_user_group_name("some5", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("5some", VALID_USER_ALLOW_NUMERIC)); + assert_se(valid_user_group_name("INNER5NUMBER", VALID_USER_ALLOW_NUMERIC)); + + assert_se(!valid_user_group_name("piff.paff@ad.domain.example", VALID_USER_ALLOW_NUMERIC)); + assert_se(!valid_user_group_name("Dāvis", VALID_USER_ALLOW_NUMERIC)); } static void test_valid_gecos(void) { @@ -355,10 +374,10 @@ int main(int argc, char *argv[]) { test_parse_uid(); test_uid_ptr(); - test_valid_user_group_name_compat(); + test_valid_user_group_name_relaxed(); test_valid_user_group_name(); - test_valid_user_group_name_or_id_compat(); - test_valid_user_group_name_or_id(); + test_valid_user_group_name_or_numeric_relaxed(); + test_valid_user_group_name_or_numeric(); test_valid_gecos(); test_valid_home(); From cafed7b32cdac13024c4093b7942a49ee8602dcf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Apr 2020 10:38:39 +0200 Subject: [PATCH 3/5] docs: add a longer document explaining our rules on user/group names --- docs/USER_NAMES.md | 169 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+) create mode 100644 docs/USER_NAMES.md diff --git a/docs/USER_NAMES.md b/docs/USER_NAMES.md new file mode 100644 index 0000000000..ccbb0a360d --- /dev/null +++ b/docs/USER_NAMES.md @@ -0,0 +1,169 @@ +-- +title: User/Group Name Syntax +category: Concepts +layout: default +--- + +# User/Group Name Syntax + +The precise set of allowed user and group names on Linux systems is weakly +defined. Depending on the distribution a different set of requirements and +restrictions on the syntax of user/group names are enforced — on some +distributions the accepted syntax is even configurable by the administrator. In +the interest of interoperability systemd enforces different rules when +processing users/group defined by other subsystems and when defining users/groups +itself, following the principle of "Be conservative in what you send, be +liberal in what you accept". Also in the interest of interoperability systemd +will enforce the same rules everywhere and not make them configurable or +distribution dependent. The precise rules are described below. + +Generally, the same rules apply for user as for group names. + +## Other Systems + +* On POSIX the set of [valid user + names](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_437) + is defined as [lower and upper case ASCII letters, digits, period, + underscore, and + hyphen](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_282), + with the restriction that hyphen is now allowed as first character of the + user name. Interestingly no size limit is declared, i.e. in neither + direction, meaning that strictly speaking according to POSIX both the empty + string is a valid user name as well as a string of gigabytes in length. + +* Debian/Ubuntu based systems enforce the regular expression + `^[a-z][-a-z0-9]*$`, i.e. only lower case ASCII letters, digits and + hyphens. As first character only lowercase ASCII letters are allowed. This + regular expression is configurable by the administrator at runtime + though. This rule enforces a minimum length of one character but no maximum + length. + +* Upstream shadow-utils enforces the regular expression + `^[a-z_][a-z0-9_-]*[$]$`, i.e. is similar to the Debian/Ubuntu rule, but + allows underscores and hyphens, but the latter not as first character. Also, + an optional trailing dollar character is permitted. + +* Fedora/Red Hat based systems enforce the regular expression of + `^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,30}[a-zA-Z0-9_.$-]?$`, i.e. a size limit of + 32 characters, with upper and lower case letters, digits, underscores, + hyphens and periods. No hyphen as first character though, and the last + character may be a dollar character. On top of that, `.` and `..` are not + allowed as user/group names. + +* sssd is known to generate user names with embedded `@` and white-space + characters, as well as non-ASCII (i.e. UTF-8) user/group names. + +* winbindd is known to generate user/group names with embedded `\` and + white-space characters, as well as non-ASCII (i.e. UTF-8) user/group names. + +Other operating systems enforce different rules; in this documentation we'll +focus on Linux systems only however, hence those are out of scope. That said, +software like Samba is frequently deployed on Linux for providing compatibility +with Windows systems; on such systems it might be wise to stick to user/group +names also valid according to Windows rules. + +## Rules systemd enforces + +Distilled from the above, below are the rules systemd enforces on user/group +names. An additional, common rule between both modes listed below is that empty +strings are not valid user/group names. + +Philosophically, the strict mode described below enforces a white-list of what's +allowed and prohibits everything else, while the relaxed mode described below +implements a blacklist of what's not allowed and permits everything else. + +### Strict mode + +Strict user/group name syntax is enforced whenever a systemd component is used +to register a user or group in the system, for example a system user/group +using +[`systemd-sysusers.service`](https://www.freedesktop.org/software/systemd/man/systemd-sysusers.html) +or a regular user with +[`systemd-homed.service`](https://www.freedesktop.org/software/systemd/man/systemd-homed.html). + +In strict mode, only uppercase and lowercase characters are allowed, as well as +digits, underscores and hyphens. The first character may not be a digit or +hyphen. A size limit is enforced: the minimum of `sysconf(_SC_LOGIN_NAME_MAX)` +(typically 256 on Linux; rationale: this is how POSIX suggests to detect the +limit), `UT_NAMESIZE-1` (typically 31 on Linux; rationale: names longer than +this cannot correctly appear in `utmp`/`wtmp` and create ambiguity with login +accounting) and `FILENAME_MAX` (4096 on Linux; rationale: user names typically +appear in directory names, i.e. the home directory), thus MIN(256, 31, 4096) = +31. + +Note that these rules are both more strict and more relaxed than all of the +rules enforced by other systems listed above. A user/group name conforming to +systemd's strict rules will not necessarily pass a test by the rules enforced +by these other subsystems. + +Written as regular expression the above is: `^[a-zA-Z_][a-zA-Z0-9_-]{0,30}$` + +### Relaxed mode + +Relaxed user/group name syntax is enforced whenever a systemd component accepts +and makes use of user/group names registered by other (non-systemd) +components of the system, for example in +[`systemd-logind.service`](https://www.freedesktop.org/software/systemd/man/systemd-logind.html). + +Relaxed syntax is also enforced by the `User=` setting in service unit files, +i.e. for system services used for running services. Since these users may be +registered by a variety of tools relaxed mode is used, but since the primary +purpose of these users is to run a system service and thus a job for systemd a +warning is shown if the specified user name does not qualify by the strict +rules above. + +* No embedded NUL bytes (rationale: handling in C must be possible and + straight-forward) + +* No names consisting fully of digits (rationale: avoid confusion with numeric + UID/GID specifications) + +* Similar, no names consisting of an initial hyphen and otherwise entirely made + up of digits (rationale: avoid confusion with negative, numeric UID/GID + specifications, e.g. `-1`) + +* No strings that do not qualify as valid UTF-8 (rationale: we want to be able + to embed these strings in JSON, with permits only valid UTF-8 in its strings; + user names using other character sets, such as JIS/Shift-JIS will cause + validation errors) + +* No control characters (i.e. characters in ASCII range 1…31; rationale: they + tend to have special meaning when output on a terminal in other contexts, + moreover the newline character — as a specific control character — is used as + record separator in `/etc/passwd`, and hence it's crucial to avoid + ambiguities here) + +* No colon characters (rationale: it is used as field separator in `/etc/passwd`) + +* The two strings `.` and `..` are not permitted, as these have special meaning + in file system paths, and user names are frequently included in file system + paths, in particular for the purpose of home directories. + +* Similar, no slashes, as these have special meaning in file system paths + +* No leading or trailing white-space is permitted; and hence no user/group names + consisting of white-space only either (rationale: this typically indicates + parsing errors, and creates confusion since not visible on screen) + +Note that these relaxed rules are implied by the strict rules above, i.e. all +user/group names accepted by the strict rules are also accepted by the relaxed +rules, but not vice versa. + +Note that this relaxed mode does not refuse a couple of very questionable +syntaxes. For example it permits a leading or embedded period. A leading period +is problematic because the matching home directory would typically be hidden +from the user's/administrator's view. An embedded period is problematic since +it creates ambiguity in traditional `chown` syntax (which is still accepted +today) that uses it to separate user and group names in the command's +parameter: without consulting the user/group databases it is not possible to +determine if a `chown` invocation would change just the owning user or both the +owning user and group. It also allows embeddeding `@` (which is confusing to +MTAs). + +## Common Core + +Combining all rules listed above, user/group names that shall be considered +valid in all systemd contexts and on all Linux systems should match the +following regular expression (at least according to our understanding): + +`^[a-z][a-z0-9-]{0,30}$` From 887a8fa341d9b24a7c9cd3f1fce328f8e43a1b4f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Apr 2020 11:04:59 +0200 Subject: [PATCH 4/5] docs: hook up the new USER_NAMES document everywhere (Also correct the set of names we accept in User=, which was forgotten to be updated in ae480f0b09aec815b64579bb1828ea935d8ee236. --- docs/USER_RECORD.md | 3 ++- man/homectl.xml | 5 ++++- man/systemd.exec.xml | 15 +++++++++------ man/sysusers.d.xml | 3 +++ 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/docs/USER_RECORD.md b/docs/USER_RECORD.md index 3bc0879a48..b9c21a1fea 100644 --- a/docs/USER_RECORD.md +++ b/docs/USER_RECORD.md @@ -205,7 +205,8 @@ object. The following fields are currently defined: UNIX user name. This field is the only mandatory field, all others are optional. Corresponds with the `pw_name` field of of `struct passwd` and the `sp_namp` field of `struct spwd` (i.e. the shadow user record stored in -`/etc/shadow`). +`/etc/shadow`). See [User/Group Name Syntax](https://systemd.io/USER_NAMES) for +the (relaxed) rules the various systemd components enforce on user/group names. `realm` → The "realm" a user is defined in. This concept allows distinguishing users with the same name that originate in different organizations or diff --git a/man/homectl.xml b/man/homectl.xml index ae502c8ebb..2d99af93b1 100644 --- a/man/homectl.xml +++ b/man/homectl.xml @@ -677,7 +677,10 @@ Create a new home directory/user account of the specified name. Use the various user record property options (as documented above) to control various aspects of the home directory - and its user accounts. + and its user accounts. + + The specified user name should follow the strict syntax described on User/Group Name Syntax. diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index 79a2c744c6..fd1755e422 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -217,12 +217,15 @@ is set, the default group of the user is used. This setting does not affect commands whose command line is prefixed with +. - Note that restrictions on the user/group name syntax are enforced: the specified name must consist only - of the characters a-z, A-Z, 0-9, _ and -, except for the first character - which must be one of a-z, A-Z or _ (i.e. numbers and - are not permitted - as first character). The user/group name must have at least one character, and at most 31. These restrictions - are enforced in order to avoid ambiguities and to ensure user/group names and unit files remain portable among - Linux systems. + Note that this enforces only weak restrictions on the user/group name syntax, but will generate + warnings in many cases where user/group names do not adhere to the following rules: the specified + name should consist only of the characters a-z, A-Z, 0-9, _ and + -, except for the first character which must be one of a-z, A-Z and + _ (i.e. digits and - are not permitted as first character). The + user/group name must have at least one character, and at most 31. These restrictions are made in + order to avoid ambiguities and to ensure user/group names and unit files remain portable among Linux + systems. For further details on the names accepted and the names warned about see User/Group Name Syntax. When used in conjunction with DynamicUser= the user/group name specified is dynamically allocated at the time the service is started, and released at the time the service is diff --git a/man/sysusers.d.xml b/man/sysusers.d.xml index c632e8713e..f0e0f2bedc 100644 --- a/man/sysusers.d.xml +++ b/man/sysusers.d.xml @@ -154,6 +154,9 @@ r - 500-900 A-Z or _ (i.e. numbers and - are not permitted as first character). The user/group name must have at least one character, and at most 31. + For further details about the syntax of user/group names, see User/Group Name Syntax. + It is strongly recommended to pick user and group names that are unlikely to clash with normal users created by the administrator. A good scheme to guarantee this is by prefixing all system and group names with the underscore, and avoiding too generic names. From ad313ec33bb367624c25c9264994d6e43b8a7e2e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Apr 2020 11:15:49 +0200 Subject: [PATCH 5/5] catalog: add entry for SD_MESSAGE_UNSAFE_USER_NAME --- catalog/systemd.catalog.in | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/catalog/systemd.catalog.in b/catalog/systemd.catalog.in index db275d7c50..3342d59422 100644 --- a/catalog/systemd.catalog.in +++ b/catalog/systemd.catalog.in @@ -412,3 +412,26 @@ as the best process to terminate and has been forcibly terminated by the kernel. Note that the memory pressure might or might not have been caused by @UNIT@. + +-- b61fdac612e94b9182285b998843061f +Subject: Accepting user/group name @USER_GROUP_NAME@, which does not match strict user/group name rules. +Defined-By: systemd +Support: %SUPPORT_URL% + +The user/group name @USER_GROUP_NAME@ has been specified, which is accepted +according the relaxed user/group name rules, but does not qualify under the +strict rules. + +The strict user/group name rules written as regular expression are: + +^[a-zA-Z_][a-zA-Z0-9_-]{0,30}$ + +The relaxed user/group name rules accept all names, except for the empty +string; names containing NUL bytes, control characters, colon or slash +characters; names not valid UTF-8; names with leading or trailing whitespace; +the strings "." or ".."; fully numeric strings, or strings beginning in a +hyphen and otherwise fully numeric. + +For further details on strict and relaxed user/group name rules, see: + +https://systemd.io/USER_NAMES