userdb: replace recursion lock

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.

4 files changed, 75 insertions, 63 deletions

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 <stdbool.h>
+
+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;