From 1429dfe5f8d0e4d2dcc73d1702510697880a46de Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Dec 2016 17:59:21 +0100 Subject: util-lib: add a comment explaining the user name rules we enforce --- src/basic/user-util.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/basic/user-util.c b/src/basic/user-util.c index 938533d2e7..c619dad527 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -46,6 +46,8 @@ bool uid_is_valid(uid_t uid) { + /* Also see POSIX IEEE Std 1003.1-2008, 2016 Edition, 3.436. */ + /* Some libc APIs use UID_INVALID as special placeholder */ if (uid == (uid_t) UINT32_C(0xFFFFFFFF)) return false; @@ -519,7 +521,15 @@ bool valid_user_group_name(const char *u) { const char *i; long sz; - /* Checks if the specified name is a valid user/group name. */ + /* 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 + * + * Note that other systems are even more restrictive, and don't permit underscores or uppercase characters. + */ if (isempty(u)) return false; -- cgit v1.2.3-54-g00ecf From d6c575e303520ffdcee1590a4181410024d5f917 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Dec 2016 17:59:38 +0100 Subject: nss: when we encounter an invalid user/group name or UID/GID, don't return EINVAL It's not our business to validate invalid user/group names or UID/GID. Ideally, libc would filter these out, but they don't, hence we have to filter, but let's not propagate this as error, but simply as "not found" to the caller. User name rules are pretty vaguely defined, and the rules defined by POSIX clash with reality quite heavily (for example, utmp doesn't offer enough room for user name length, and /usr/bin/chown permits separating user/group names by a single dot, even though POSIX allows dots being used in user/group names themselves.) We enforce stricter rules than POSIX for good reason, and hence in doing so we should not categorically return EINVAL on stuff we don't consider valid, but other components might. Fixes: #4983 --- src/nss-mymachines/nss-mymachines.c | 12 ++++-------- src/nss-systemd/nss-systemd.c | 26 ++++++++++---------------- 2 files changed, 14 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/nss-mymachines/nss-mymachines.c b/src/nss-mymachines/nss-mymachines.c index 895f61c462..fac37faea5 100644 --- a/src/nss-mymachines/nss-mymachines.c +++ b/src/nss-mymachines/nss-mymachines.c @@ -512,10 +512,8 @@ enum nss_status _nss_mymachines_getpwuid_r( BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); - if (!uid_is_valid(uid)) { - r = -EINVAL; - goto fail; - } + if (!uid_is_valid(uid)) + goto not_found; /* We consider all uids < 65536 host uids */ if (uid < HOST_UID_LIMIT) @@ -686,10 +684,8 @@ enum nss_status _nss_mymachines_getgrgid_r( BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); - if (!gid_is_valid(gid)) { - r = -EINVAL; - goto fail; - } + if (!gid_is_valid(gid)) + goto not_found; /* We consider all gids < 65536 host gids */ if (gid < HOST_GID_LIMIT) diff --git a/src/nss-systemd/nss-systemd.c b/src/nss-systemd/nss-systemd.c index c80972742b..fd5064c937 100644 --- a/src/nss-systemd/nss-systemd.c +++ b/src/nss-systemd/nss-systemd.c @@ -123,10 +123,10 @@ enum nss_status _nss_systemd_getpwnam_r( assert(name); assert(pwd); - if (!valid_user_group_name(name)) { - r = -EINVAL; - goto fail; - } + /* 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)) + goto not_found; /* Synthesize entries for the root and nobody users, in case they are missing in /etc/passwd */ if (streq(name, root_passwd.pw_name)) { @@ -227,10 +227,8 @@ enum nss_status _nss_systemd_getpwuid_r( BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); - if (!uid_is_valid(uid)) { - r = -EINVAL; - goto fail; - } + if (!uid_is_valid(uid)) + goto not_found; /* Synthesize data for the root user and for nobody in case they are missing from /etc/passwd */ if (uid == root_passwd.pw_uid) { @@ -329,10 +327,8 @@ enum nss_status _nss_systemd_getgrnam_r( assert(name); assert(gr); - if (!valid_user_group_name(name)) { - r = -EINVAL; - goto fail; - } + if (!valid_user_group_name(name)) + goto not_found; /* Synthesize records for root and nobody, in case they are missing form /etc/group */ if (streq(name, root_group.gr_name)) { @@ -430,10 +426,8 @@ enum nss_status _nss_systemd_getgrgid_r( BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); - if (!gid_is_valid(gid)) { - r = -EINVAL; - goto fail; - } + if (!gid_is_valid(gid)) + goto not_found; /* Synthesize records for root and nobody, in case they are missing from /etc/group */ if (gid == root_group.gr_gid) { -- cgit v1.2.3-54-g00ecf