Change up user classification logic again

relying on login.defs is fragile, and the
user heuristics are fragile.

This commit requires an explicit uid minimum
get configured, and heuristics now only get
applied to the specific problematic range
they were added to address.

https://bugs.freedesktop.org/show_bug.cgi?id=71801

2 files changed, 32 insertions, 105 deletions

diff --git a/configure.ac b/configure.ac
index cb1fcda..39c5b92 100644
--- a/configure.ac
+++ b/configure.ac
@@ -55,11 +55,17 @@ AS_IF([test x$enable_admin_group = xauto], [
 AC_DEFINE_UNQUOTED([ADMIN_GROUP], ["$enable_admin_group"], [Define to the group for administrator users])
 
 AC_ARG_ENABLE(user-heuristics,
-        [AS_HELP_STRING([--enable-user-heuristics],[Enable heuristics for guessing system vs. human users])],
+        [AS_HELP_STRING([--enable-user-heuristics],[Enable heuristics for guessing system vs. human users in the range 500-minimum-uid])],
         [if test "$enableval" = yes; then
            AC_DEFINE([ENABLE_USER_HEURISTICS], , [System vs. human user heuristics enabled])
         fi])
 
+AC_ARG_WITH(minimum-uid,
+        [AS_HELP_STRING([--with-minimum-uid],[Set minimum uid for human users])],
+        ,with_minimum_uid=1000)
+
+AC_DEFINE_UNQUOTED([MINIMUM_UID], $with_minimum_uid, [Define to the minumum UID of human users])
+
 dnl ---------------------------------------------------------------------------
 dnl - coverage
 dnl ---------------------------------------------------------------------------
diff --git a/src/user-classify.c b/src/user-classify.c
index b68c9ae..69e6809 100644
--- a/src/user-classify.c
+++ b/src/user-classify.c
@@ -26,7 +26,6 @@
 
 #include <string.h>
 
-#ifdef ENABLE_USER_HEURISTICS
 static const char *default_excludes[] = {
         "bin",
         "root",
@@ -57,16 +56,10 @@ static const char *default_excludes[] = {
         "gnome-initial-setup"
 };
 
-#define PATH_NOLOGIN "/sbin/nologin"
-#define PATH_FALSE "/bin/false"
-
 static gboolean
-user_classify_is_excluded_by_heuristics (const gchar *username,
-                                         const gchar *shell,
-                                         const gchar *password_hash)
+user_classify_is_blacklisted (const char *username)
 {
         static GHashTable *exclusions;
-        gboolean ret = FALSE;
 
         if (exclusions == NULL) {
                 guint i;
@@ -82,6 +75,20 @@ user_classify_is_excluded_by_heuristics (const gchar *username,
                 return TRUE;
         }
 
+        return FALSE;
+}
+
+#define PATH_NOLOGIN "/sbin/nologin"
+#define PATH_FALSE "/bin/false"
+
+#ifdef ENABLE_USER_HEURISTICS
+static gboolean
+user_classify_is_excluded_by_heuristics (const gchar *username,
+                                         const gchar *shell,
+                                         const gchar *password_hash)
+{
+        gboolean ret = FALSE;
+
         if (shell != NULL) {
                 char *basename, *nologin_basename, *false_basename;
 
@@ -139,99 +146,6 @@ user_classify_is_excluded_by_heuristics (const gchar *username,
 
         return ret;
 }
-
-#else /* ENABLE_USER_HEURISTICS */
-
-static gboolean
-user_classify_parse_login_defs_field (const gchar *contents,
-                                      const gchar *key,
-                                      uid_t       *result)
-{
-        gsize key_len;
-        gint64 value;
-        gchar *end;
-
-        key_len = strlen (key);
-
-        for (;;) {
-                /* Our key has to be at the start of the line, followed by whitespace */
-                if (strncmp (contents, key, key_len) == 0 && g_ascii_isspace (contents[key_len])) {
-                        /* Found it.  Move contents past the key itself and break out. */
-                        contents += key_len;
-                        break;
-                }
-
-                /* Didn't find it.  Find the end of the line. */
-                contents = strchr (contents, '\n');
-
-                /* EOF? */
-                if (!contents) {
-                        /* We didn't find the field... */
-                        return FALSE;
-                }
-
-                /* Start at the beginning of the next line on next iteration. */
-                contents++;
-        }
-
-        /* 'contents' now points at the whitespace character just after
-         * the field name.  strtoll can deal with that.
-         */
-        value = g_ascii_strtoll (contents, &end, 10);
-
-        if (*end && !g_ascii_isspace (*end)) {
-                g_warning ("Trailing junk after '%s' field in login.defs", key);
-                return FALSE;
-        }
-
-        if (value <= 0 || value >= G_MAXINT32) {
-                g_warning ("Value for '%s' field out of range", key);
-                return FALSE;
-        }
-
-        *result = value;
-
-        return TRUE;
-}
-
-static void
-user_classify_read_login_defs (uid_t *min_uid,
-                               uid_t *max_uid)
-{
-        GError *error = NULL;
-        char *contents;
-
-        if (!g_file_get_contents ("/etc/login.defs", &contents, NULL, &error)) {
-                g_warning ("Could not open /etc/login.defs: %s.  Falling back to default human uid range of %d to %d",
-                           error->message, (int) *min_uid, (int) *max_uid);
-                g_error_free (error);
-                return;
-        }
-
-        if (!user_classify_parse_login_defs_field (contents, "UID_MIN", min_uid)) {
-                g_warning ("Could not find UID_MIN value in login.defs.  Using default of %d", (int) *min_uid);
-        }
-
-        if (!user_classify_parse_login_defs_field (contents, "UID_MAX", max_uid)) {
-                g_warning ("Could not find UID_MIN value in login.defs.  Using default of %d", (int) *max_uid);
-        }
-
-        g_free (contents);
-}
-
-static gboolean
-user_classify_is_in_human_range (uid_t uid)
-{
-        static uid_t min_uid = 1000, max_uid = 60000;
-        static gboolean initialised;
-
-        if (!initialised) {
-                user_classify_read_login_defs (&min_uid, &max_uid);
-                initialised = TRUE;
-        }
-
-        return min_uid <= uid && uid <= max_uid;
-}
 #endif /* ENABLE_USER_HEURISTICS */
 
 gboolean
@@ -240,9 +154,16 @@ user_classify_is_human (uid_t        uid,
                         const gchar *shell,
                         const gchar *password_hash)
 {
+        if (user_classify_is_blacklisted (username))
+                return FALSE;
+
 #ifdef ENABLE_USER_HEURISTICS
-        return !user_classify_is_excluded_by_heuristics (username, shell, password_hash);
-#else
-        return user_classify_is_in_human_range (uid);
+        /* only do heuristics on the range 500-1000 to catch one off migration problems in Fedora */
+        if (uid >= 500 && uid < MINIMUM_UID) {
+                if (!user_classify_is_excluded_by_heuristics (username, shell, password_hash))
+                        return TRUE;
+        }
 #endif
+
+        return uid >= MINIMUM_UID;
 }