From 037b0a47b0d7df09d720dda6703135117e7e0472 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 4 Jun 2020 11:46:36 +0200 Subject: userdb: replace recursion lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously we'd used the existance of a specific AF_UNIX socket in the abstract namespace as lock for disabling lookup recursions. (for breaking out of the loop: userdb synthesized from nss → nss synthesized from userdb → userdb synthesized from nss → …) I did it like that because it promised to work the same both in static and in dynmically linked environments and is accessible easily from any programming language. However, it has a weakness regarding reuse attacks: the socket is securely hashed (siphash) from the thread ID in combination with the AT_RANDOM secret. Thus it should not be guessable from an attacker in advance. That's only true if a thread takes the lock only once and keeps it forever. However, if a thread takes and releases it multiple times an attacker might monitor that and quickly take the lock after the first iteration for follow-up iterations. It's not a big issue given that userdb (as the primary user for this) never released the lock and we never made the concept a public interface, and it was only included in one release so far, but it's something that deserves fixing. (moreover it's a local DoS only, only permitting to disable native userdb lookups) With this rework the libnss_systemd.so.2 module will now export two additional symbols. These symbols are not used by glibc, but can be used by arbitrary programs: one can be used to disable nss-systemd, the other to check if it is currently disabled. The lock is per-thread. It's slightly less pretty, since it requires people to manually link against C code via dlopen()/dlsym(), but it should work safely without the aforementioned weakness. --- src/nss-systemd/nss-systemd.c | 69 ++++++++++++++++++++++++----------------- src/nss-systemd/nss-systemd.h | 13 ++++++++ src/nss-systemd/nss-systemd.sym | 4 +++ src/nss-systemd/userdb-glue.c | 52 +++++++++++-------------------- 4 files changed, 75 insertions(+), 63 deletions(-) create mode 100644 src/nss-systemd/nss-systemd.h (limited to 'src/nss-systemd') diff --git a/src/nss-systemd/nss-systemd.c b/src/nss-systemd/nss-systemd.c index e11f917c19..5dc5aacdff 100644 --- a/src/nss-systemd/nss-systemd.c +++ b/src/nss-systemd/nss-systemd.c @@ -8,6 +8,7 @@ #include "fd-util.h" #include "group-record-nss.h" #include "macro.h" +#include "nss-systemd.h" #include "nss-util.h" #include "pthread-util.h" #include "signal-util.h" @@ -299,7 +300,7 @@ enum nss_status _nss_systemd_setpwent(int stayopen) { PROTECT_ERRNO; BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); - if (userdb_nss_compat_is_enabled() <= 0) + if (_nss_systemd_is_blocked()) return NSS_STATUS_NOTFOUND; _cleanup_(pthread_mutex_unlock_assertp) pthread_mutex_t *_l = NULL; @@ -323,7 +324,7 @@ enum nss_status _nss_systemd_setgrent(int stayopen) { PROTECT_ERRNO; BLOCK_SIGNALS(NSS_SIGNALS_BLOCK); - if (userdb_nss_compat_is_enabled() <= 0) + if (_nss_systemd_is_blocked()) return NSS_STATUS_NOTFOUND; _cleanup_(pthread_mutex_unlock_assertp) pthread_mutex_t *_l = NULL; @@ -353,13 +354,7 @@ enum nss_status _nss_systemd_getpwent_r( assert(result); assert(errnop); - r = userdb_nss_compat_is_enabled(); - if (r < 0) { - UNPROTECT_ERRNO; - *errnop = -r; - return NSS_STATUS_UNAVAIL; - } - if (!r) + if (_nss_systemd_is_blocked()) return NSS_STATUS_NOTFOUND; _cleanup_(pthread_mutex_unlock_assertp) pthread_mutex_t *_l = NULL; @@ -406,14 +401,8 @@ enum nss_status _nss_systemd_getgrent_r( assert(result); assert(errnop); - r = userdb_nss_compat_is_enabled(); - if (r < 0) { - UNPROTECT_ERRNO; - *errnop = -r; - return NSS_STATUS_UNAVAIL; - } - if (!r) - return NSS_STATUS_UNAVAIL; + if (_nss_systemd_is_blocked()) + return NSS_STATUS_NOTFOUND; _cleanup_(pthread_mutex_unlock_assertp) pthread_mutex_t *_l = NULL; @@ -459,7 +448,7 @@ enum nss_status _nss_systemd_getgrent_r( } if (getgrent_data.by_membership) { - _cleanup_close_ int lock_fd = -1; + _cleanup_(_nss_systemd_unblockp) bool blocked = false; for (;;) { _cleanup_free_ char *user_name = NULL, *group_name = NULL; @@ -479,13 +468,15 @@ enum nss_status _nss_systemd_getgrent_r( continue; /* We are about to recursively call into NSS, let's make sure we disable recursion into our own code. */ - if (lock_fd < 0) { - lock_fd = userdb_nss_compat_disable(); - if (lock_fd < 0 && lock_fd != -EBUSY) { + if (!blocked) { + r = _nss_systemd_block(true); + if (r < 0) { UNPROTECT_ERRNO; - *errnop = -lock_fd; + *errnop = -r; return NSS_STATUS_UNAVAIL; } + + blocked = true; } r = nss_group_record_by_name(group_name, false, &gr); @@ -549,13 +540,7 @@ enum nss_status _nss_systemd_initgroups_dyn( if (STR_IN_SET(user_name, root_passwd.pw_name, nobody_passwd.pw_name)) return NSS_STATUS_NOTFOUND; - r = userdb_nss_compat_is_enabled(); - if (r < 0) { - UNPROTECT_ERRNO; - *errnop = -r; - return NSS_STATUS_UNAVAIL; - } - if (!r) + if (_nss_systemd_is_blocked()) return NSS_STATUS_NOTFOUND; r = membershipdb_by_user(user_name, nss_glue_userdb_flags(), &iterator); @@ -627,3 +612,29 @@ enum nss_status _nss_systemd_initgroups_dyn( return any ? NSS_STATUS_SUCCESS : NSS_STATUS_NOTFOUND; } + +static thread_local unsigned _blocked = 0; + +_public_ int _nss_systemd_block(bool b) { + + /* This blocks recursively: it's blocked for as many times this function is called with `true` until + * it is called an equal time with `false`. */ + + if (b) { + if (_blocked >= UINT_MAX) + return -EOVERFLOW; + + _blocked++; + } else { + if (_blocked <= 0) + return -EOVERFLOW; + + _blocked--; + } + + return b; /* Return what is passed in, i.e. the new state from the PoV of the caller */ +} + +_public_ bool _nss_systemd_is_blocked(void) { + return _blocked > 0; +} diff --git a/src/nss-systemd/nss-systemd.h b/src/nss-systemd/nss-systemd.h new file mode 100644 index 0000000000..ffa75c12c4 --- /dev/null +++ b/src/nss-systemd/nss-systemd.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +#pragma once + +#include + +int _nss_systemd_block(bool b); +bool _nss_systemd_is_blocked(void); + +/* For use with the _cleanup_() macro */ +static inline void _nss_systemd_unblockp(bool *b) { + if (*b) + assert_se(_nss_systemd_block(false) >= 0); +} diff --git a/src/nss-systemd/nss-systemd.sym b/src/nss-systemd/nss-systemd.sym index 77e1fbe93f..f86d7643d1 100644 --- a/src/nss-systemd/nss-systemd.sym +++ b/src/nss-systemd/nss-systemd.sym @@ -20,5 +20,9 @@ global: _nss_systemd_setgrent; _nss_systemd_getgrent_r; _nss_systemd_initgroups_dyn; + + /* These two are not used by glibc, but can be used by apps to explicitly disable nss-systemd for the calling thread. */ + _nss_systemd_block; + _nss_systemd_is_blocked; local: *; }; diff --git a/src/nss-systemd/userdb-glue.c b/src/nss-systemd/userdb-glue.c index da1248a132..8e5b3eba6c 100644 --- a/src/nss-systemd/userdb-glue.c +++ b/src/nss-systemd/userdb-glue.c @@ -3,6 +3,7 @@ #include "env-util.h" #include "fd-util.h" #include "group-record-nss.h" +#include "nss-systemd.h" #include "strv.h" #include "user-record.h" #include "userdb-glue.h" @@ -74,12 +75,7 @@ enum nss_status userdb_getpwnam( assert(pwd); assert(errnop); - r = userdb_nss_compat_is_enabled(); - if (r < 0) { - *errnop = -r; - return NSS_STATUS_UNAVAIL; - } - if (!r) + if (_nss_systemd_is_blocked()) return NSS_STATUS_NOTFOUND; r = userdb_by_name(name, nss_glue_userdb_flags(), &hr); @@ -112,12 +108,7 @@ enum nss_status userdb_getpwuid( assert(pwd); assert(errnop); - r = userdb_nss_compat_is_enabled(); - if (r < 0) { - *errnop = -r; - return NSS_STATUS_UNAVAIL; - } - if (!r) + if (_nss_systemd_is_blocked()) return NSS_STATUS_NOTFOUND; r = userdb_by_uid(uid, nss_glue_userdb_flags(), &hr); @@ -214,12 +205,7 @@ enum nss_status userdb_getgrnam( assert(gr); assert(errnop); - r = userdb_nss_compat_is_enabled(); - if (r < 0) { - *errnop = -r; - return NSS_STATUS_UNAVAIL; - } - if (!r) + if (_nss_systemd_is_blocked()) return NSS_STATUS_NOTFOUND; r = groupdb_by_name(name, nss_glue_userdb_flags(), &g); @@ -235,7 +221,7 @@ enum nss_status userdb_getgrnam( } if (!g) { - _cleanup_close_ int lock_fd = -1; + _cleanup_(_nss_systemd_unblockp) bool blocked = false; if (strv_isempty(members)) return NSS_STATUS_NOTFOUND; @@ -245,11 +231,13 @@ enum nss_status userdb_getgrnam( * acquire it, so that we can extend it (that's because glibc's group merging feature will * merge groups only if both GID and name match and thus we need to have both first). It * sucks behaving recursively likely this, but it's apparently what everybody does. We break - * the recursion for ourselves via the userdb_nss_compat_disable() lock. */ + * the recursion for ourselves via the _nss_systemd_block_nss() lock. */ + + r = _nss_systemd_block(true); + if (r < 0) + return r; - lock_fd = userdb_nss_compat_disable(); - if (lock_fd < 0 && lock_fd != -EBUSY) - return lock_fd; + blocked = true; r = nss_group_record_by_name(name, false, &g); if (r == -ESRCH) @@ -285,12 +273,7 @@ enum nss_status userdb_getgrgid( assert(gr); assert(errnop); - r = userdb_nss_compat_is_enabled(); - if (r < 0) { - *errnop = -r; - return NSS_STATUS_UNAVAIL; - } - if (!r) + if (_nss_systemd_is_blocked()) return NSS_STATUS_NOTFOUND; r = groupdb_by_gid(gid, nss_glue_userdb_flags(), &g); @@ -300,20 +283,21 @@ enum nss_status userdb_getgrgid( } if (!g) { - _cleanup_close_ int lock_fd = -1; + _cleanup_(_nss_systemd_unblockp) bool blocked = false; /* So, quite possibly we have to extend an existing group record with additional members. But * to do this we need to know the group name first. The group didn't exist via non-NSS * queries though, hence let's try to acquire it here recursively via NSS. */ - lock_fd = userdb_nss_compat_disable(); - if (lock_fd < 0 && lock_fd != -EBUSY) - return lock_fd; + r = _nss_systemd_block(true); + if (r < 0) + return r; + + blocked = true; r = nss_group_record_by_gid(gid, false, &g); if (r == -ESRCH) return NSS_STATUS_NOTFOUND; - if (r < 0) { *errnop = -r; return NSS_STATUS_UNAVAIL; -- cgit v1.2.1