Attachment 130488 Details for Bug 100422 – daemon: make sure explicitly requested users aren't lost on reloads

[patch] daemon: make sure explicitly requested users aren't lost on reloads

daemon-make-sure-explicitly-requested-users-arent-.patch (text/plain), 19.67 KB, created by Ray Strode [halfline] on 2017-03-27 17:57:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ray Strode [halfline]

Created: 2017-03-27 17:57:59 UTC

Size: 19.67 KB

patch

obsolete

>From 177509e9460b149ecbf85e75c930be2ea00b7d05 Mon Sep 17 00:00:00 2001
>From: Ray Strode <rstrode@redhat.com>
>Date: Thu, 23 Mar 2017 16:59:11 -0400
>Subject: [PATCH] daemon: make sure explicitly requested users aren't lost on
> reloads
>
>Right now, a user proxy can suddenly become defunct if the
>/etc/passwd file is updated or some other reason leads to a reload.
>
>This commit makes sure that the objects associated with proxies
>stick around across reloads.
>
>https://bugs.freedesktop.org/show_bug.cgi?id=100422
>---
> src/daemon.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 67 insertions(+), 7 deletions(-)
>
>diff --git a/src/daemon.c b/src/daemon.c
>index 05b810c..312394a 100644
>--- a/src/daemon.c
>+++ b/src/daemon.c
>@@ -35,77 +35,78 @@
> #include <unistd.h>
> #include <errno.h>
> #include <sys/types.h>
> 
> #include <glib.h>
> #include <glib/gi18n.h>
> #include <glib-object.h>
> #include <glib/gstdio.h>
> #include <gio/gio.h>
> #include <polkit/polkit.h>
> 
> #include "user-classify.h"
> #include "wtmp-helper.h"
> #include "daemon.h"
> #include "util.h"
> 
> #define PATH_PASSWD "/etc/passwd"
> #define PATH_SHADOW "/etc/shadow"
> #define PATH_GROUP "/etc/group"
> #define PATH_GDM_CUSTOM "/etc/gdm/custom.conf"
> 
> enum {
>         PROP_0,
>         PROP_DAEMON_VERSION
> };
> 
> struct DaemonPrivate {
>         GDBusConnection *bus_connection;
> 
>         GHashTable *users;
>+        GList *explicitly_requested_users;
> 
>         User *autologin;
> 
>         GFileMonitor *passwd_monitor;
>         GFileMonitor *shadow_monitor;
>         GFileMonitor *group_monitor;
>         GFileMonitor *gdm_monitor;
>         GFileMonitor *wtmp_monitor;
> 
>         guint reload_id;
>         guint autologin_id;
> 
>         PolkitAuthority *authority;
>         GHashTable *extension_ifaces;
> };
> 
>-typedef struct passwd * (* EntryGeneratorFunc) (GHashTable *, gpointer *, struct spwd **shadow_entry);
>+typedef struct passwd * (* EntryGeneratorFunc) (Daemon *, GHashTable *, gpointer *, struct spwd **shadow_entry);
> 
> static void daemon_accounts_accounts_iface_init (AccountsAccountsIface *iface);
> 
> G_DEFINE_TYPE_WITH_CODE (Daemon, daemon, ACCOUNTS_TYPE_ACCOUNTS_SKELETON, G_IMPLEMENT_INTERFACE (ACCOUNTS_TYPE_ACCOUNTS, daemon_accounts_accounts_iface_init));
> 
> #define DAEMON_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), TYPE_DAEMON, DaemonPrivate))
> 
> static const GDBusErrorEntry accounts_error_entries[] =
> { 
>         { ERROR_FAILED, "org.freedesktop.Accounts.Error.Failed" },
>         { ERROR_USER_EXISTS, "org.freedesktop.Accounts.Error.UserExists" },
>         { ERROR_USER_DOES_NOT_EXIST, "org.freedesktop.Accounts.Error.UserDoesNotExist" },
>         { ERROR_PERMISSION_DENIED, "org.freedesktop.Accounts.Error.PermissionDenied" },
>         { ERROR_NOT_SUPPORTED, "org.freedesktop.Accounts.Error.NotSupported" }
> };
> 
> GQuark
> error_quark (void)
> {
>         static volatile gsize quark_volatile = 0;
> 
>         g_dbus_error_register_error_domain ("accounts_error",
>                                             &quark_volatile,
>                                             accounts_error_entries,
>                                             G_N_ELEMENTS (accounts_error_entries));
> 
>         return (GQuark) quark_volatile;
> }
> #define ENUM_ENTRY(NAME, DESC) { NAME, "" #NAME "", DESC }
> 
>@@ -113,61 +114,62 @@ GType
> error_get_type (void)
> {
>   static GType etype = 0;
> 
>   if (etype == 0)
>     {
>       static const GEnumValue values[] =
>         {
>           ENUM_ENTRY (ERROR_FAILED, "Failed"),
>           ENUM_ENTRY (ERROR_USER_EXISTS, "UserExists"),
>           ENUM_ENTRY (ERROR_USER_DOES_NOT_EXIST, "UserDoesntExist"),
>           ENUM_ENTRY (ERROR_PERMISSION_DENIED, "PermissionDenied"),
>           ENUM_ENTRY (ERROR_NOT_SUPPORTED, "NotSupported"),
>           { 0, 0, 0 }
>         };
>       g_assert (NUM_ERRORS == G_N_ELEMENTS (values) - 1);
>       etype = g_enum_register_static ("Error", values);
>     }
>   return etype;
> }
> 
> #ifndef HAVE_FGETPWENT
> #include "fgetpwent.c"
> #endif
> 
> #ifndef MAX_LOCAL_USERS
> #define MAX_LOCAL_USERS 50
> #endif
> 
> static struct passwd *
>-entry_generator_fgetpwent (GHashTable   *users,
>+entry_generator_fgetpwent (Daemon       *daemon,
>+                           GHashTable   *users,
>                            gpointer     *state,
>                            struct spwd **spent)
> {
>         struct passwd *pwent;
> 
>         struct {
>                 struct spwd spbuf;
>                 char buf[1024];
>         } *shadow_entry_buffers;
> 
>         struct {
>                 FILE *fp;
>                 GHashTable *users;
>         } *generator_state;
> 
>         /* First iteration */
>         if (*state == NULL) {
>                 GHashTable *shadow_users = NULL;
>                 FILE *fp;
>                 struct spwd *shadow_entry;
> 
>                 fp = fopen (PATH_SHADOW, "r");
>                 if (fp == NULL) {
>                         g_warning ("Unable to open %s: %s", PATH_SHADOW, g_strerror (errno));
>                         return NULL;
>                 }
> 
>                 shadow_users = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
> 
>                 do {
>@@ -206,61 +208,62 @@ entry_generator_fgetpwent (GHashTable   *users,
>                 generator_state->users = shadow_users;
> 
>                 *state = generator_state;
>         }
> 
>         /* Every iteration */
>         generator_state = *state;
> 
>         if (g_hash_table_size (users) < MAX_LOCAL_USERS) {
>                 pwent = fgetpwent (generator_state->fp);
>                 if (pwent != NULL) {
>                         shadow_entry_buffers = g_hash_table_lookup (generator_state->users, pwent->pw_name);
> 
>                         if (shadow_entry_buffers != NULL) {
>                             *spent = &shadow_entry_buffers->spbuf;
>                             return pwent;
>                         }
>                 }
>         }
> 
>         /* Last iteration */
>         fclose (generator_state->fp);
>         g_hash_table_unref (generator_state->users);
>         g_free (generator_state);
>         *state = NULL;
> 
>         return NULL;
> }
> 
> static struct passwd *
>-entry_generator_cachedir (GHashTable   *users,
>+entry_generator_cachedir (Daemon       *daemon,
>+                          GHashTable   *users,
>                           gpointer     *state,
>                           struct spwd **shadow_entry)
> {
>         struct passwd *pwent;
>         const gchar *name;
>         GError *error = NULL;
>         gchar *filename;
>         gboolean regular;
>         GHashTableIter iter;
>         GKeyFile *key_file;
>         User *user;
>         GDir *dir;
> 
>         /* First iteration */
>         if (*state == NULL) {
>                 *state = g_dir_open (USERDIR, 0, &error);
>                 if (error != NULL) {
>                         if (!g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
>                                 g_warning ("couldn't list user cache directory: %s", USERDIR);
>                         g_error_free (error);
>                         return NULL;
>                 }
>         }
> 
>         /* Every iteration */
> 
>         /*
>          * Use names of files of regular type to lookup information
>          * about each user. Loop until we find something valid.
>          */
>@@ -278,145 +281,194 @@ entry_generator_cachedir (GHashTable   *users,
>                 if (regular) {
>                         pwent = getpwnam (name);
>                         if (pwent == NULL) {
>                                 g_debug ("user '%s' in cache dir but not present on system", name);
>                         } else {
>                                 *shadow_entry = getspnam (pwent->pw_name);
> 
>                                 return pwent;
>                         }
>                 }
>         }
> 
>         /* Last iteration */
>         g_dir_close (dir);
> 
>         /* Update all the users from the files in the cache dir */
>         g_hash_table_iter_init (&iter, users);
>         while (g_hash_table_iter_next (&iter, (gpointer *)&name, (gpointer *)&user)) {
>                 filename = g_build_filename (USERDIR, name, NULL);
>                 key_file = g_key_file_new ();
>                 if (g_key_file_load_from_file (key_file, filename, 0, NULL))
>                         user_update_from_keyfile (user, key_file);
>                 g_key_file_unref (key_file);
>                 g_free (filename);
>         }
> 
>         *state = NULL;
>         return NULL;
> }
> 
>+static struct passwd *
>+entry_generator_requested_users (Daemon       *daemon,
>+                                 GHashTable   *users,
>+                                 gpointer     *state,
>+                                 struct spwd **shadow_entry)
>+{
>+        struct passwd *pwent;
>+        GList *node;
>+
>+        /* First iteration */
>+        if (*state == NULL) {
>+                *state = daemon->priv->explicitly_requested_users;
>+        }
>+
>+        /* Every iteration */
>+
>+        if (g_hash_table_size (users) < MAX_LOCAL_USERS) {
>+                node = *state;
>+                while (node != NULL) {
>+                        const char *name;
>+
>+                        name = node->data;
>+                        node = node->next;
>+
>+                        *state = node;
>+
>+                        if (!g_hash_table_lookup (users, name)) {
>+                                pwent = getpwnam (name);
>+                                if (pwent == NULL) {
>+                                        g_debug ("user '%s' requested previously but not present on system", name);
>+                                } else {
>+                                        *shadow_entry = getspnam (pwent->pw_name);
>+
>+                                        return pwent;
>+                                }
>+                        }
>+                }
>+        }
>+
>+        /* Last iteration */
>+
>+        *state = NULL;
>+        return NULL;
>+}
>+
> static void
> load_entries (Daemon             *daemon,
>               GHashTable         *users,
>+              gboolean            allow_system_users,
>               EntryGeneratorFunc  entry_generator)
> {
>         gpointer generator_state = NULL;
>         struct passwd *pwent;
>         struct spwd *spent = NULL;
>         User *user = NULL;
> 
>         g_assert (entry_generator != NULL);
> 
>         for (;;) {
>                 spent = NULL;
>-                pwent = entry_generator (users, &generator_state, &spent);
>+                pwent = entry_generator (daemon, users, &generator_state, &spent);
>                 if (pwent == NULL)
>                         break;
> 
>                 /* Skip system users... */
>-                if (!user_classify_is_human (pwent->pw_uid, pwent->pw_name, pwent->pw_shell, spent? spent->sp_pwdp : NULL)) {
>+                if (!allow_system_users && !user_classify_is_human (pwent->pw_uid, pwent->pw_name, pwent->pw_shell, spent? spent->sp_pwdp : NULL)) {
>                         g_debug ("skipping user: %s", pwent->pw_name);
>                         continue;
>                 }
> 
>                 /* ignore duplicate entries */
>                 if (g_hash_table_lookup (users, pwent->pw_name)) {
>                         continue;
>                 }
> 
>                 user = g_hash_table_lookup (daemon->priv->users, pwent->pw_name);
>                 if (user == NULL) {
>                         user = user_new (daemon, pwent->pw_uid);
>                 } else {
>                         g_object_ref (user);
>                 }
> 
>                 /* freeze & update users not already in the new list */
>                 g_object_freeze_notify (G_OBJECT (user));
>                 user_update_from_pwent (user, pwent, spent);
> 
>                 g_hash_table_insert (users, g_strdup (user_get_user_name (user)), user);
>                 g_debug ("loaded user: %s", user_get_user_name (user));
>         }
> 
>         /* Generator should have cleaned up */
>         g_assert (generator_state == NULL);
> }
> 
> static GHashTable *
> create_users_hash_table (void)
> {
>         return g_hash_table_new_full (g_str_hash,
>                                       g_str_equal,
>                                       g_free,
>                                       g_object_unref);
> }
> 
> static void
> reload_users (Daemon *daemon)
> {
>         GHashTable *users;
>         GHashTable *old_users;
>         GHashTable *local;
>         GHashTableIter iter;
>         gpointer name;
>         User *user;
> 
>         /* Track the users that we saw during our (re)load */
>         users = create_users_hash_table ();
> 
>         /*
>          * NOTE: As we load data from all the sources, notifies are
>          * frozen in load_entries() and then thawed as we process
>          * them below.
>          */
> 
>         /* Load the local users into our hash table */
>-        load_entries (daemon, users, entry_generator_fgetpwent);
>+        load_entries (daemon, users, FALSE, entry_generator_fgetpwent);
>         local = g_hash_table_new (g_str_hash, g_str_equal);
>         g_hash_table_iter_init (&iter, users);
>         while (g_hash_table_iter_next (&iter, &name, NULL))
>                 g_hash_table_add (local, name);
> 
>+        /* and add users to hash table that were explicitly requested  */
>+        load_entries (daemon, users, TRUE, entry_generator_requested_users);
>+
>         /* Now add/update users from other sources, possibly non-local */
>-        load_entries (daemon, users, entry_generator_cachedir);
>+        load_entries (daemon, users, FALSE, entry_generator_cachedir);
> 
>         wtmp_helper_update_login_frequencies (users);
> 
>         /* Mark which users are local, which are not */
>         g_hash_table_iter_init (&iter, users);
>         while (g_hash_table_iter_next (&iter, &name, (gpointer *)&user))
>                 user_update_local_account_property (user, g_hash_table_lookup (local, name) != NULL);
> 
>         g_hash_table_destroy (local);
> 
>         /* Swap out the users */
>         old_users = daemon->priv->users;
>         daemon->priv->users = users;
> 
>         /* Remove all the old users */
>         g_hash_table_iter_init (&iter, old_users);
>         while (g_hash_table_iter_next (&iter, &name, (gpointer *)&user)) {
>                 if (!g_hash_table_lookup (users, name)) {
>                         user_unregister (user);
>                         accounts_accounts_emit_user_deleted (ACCOUNTS_ACCOUNTS (daemon),
>                                                              user_get_object_path (user));
>                 }
>         }
> 
>         /* Register all the new users */
>         g_hash_table_iter_init (&iter, users);
>         while (g_hash_table_iter_next (&iter, &name, (gpointer *)&user)) {
>                 if (!g_hash_table_lookup (old_users, name)) {
>                         user_register (user);
>                         accounts_accounts_emit_user_added (ACCOUNTS_ACCOUNTS (daemon),
>@@ -603,60 +655,62 @@ daemon_init (Daemon *daemon)
>         daemon->priv->shadow_monitor = setup_monitor (daemon,
>                                                       PATH_SHADOW,
>                                                       on_users_monitor_changed);
>         daemon->priv->group_monitor = setup_monitor (daemon,
>                                                      PATH_GROUP,
>                                                      on_users_monitor_changed);
> 
>         daemon->priv->wtmp_monitor = setup_monitor (daemon,
>                                                     wtmp_helper_get_path_for_monitor (),
>                                                     on_users_monitor_changed);
> 
>         daemon->priv->gdm_monitor = setup_monitor (daemon,
>                                                    PATH_GDM_CUSTOM,
>                                                    on_gdm_monitor_changed);
>         reload_users_timeout (daemon);
>         queue_reload_autologin (daemon);
> }
> 
> static void
> daemon_finalize (GObject *object)
> {
>         Daemon *daemon;
> 
>         g_return_if_fail (IS_DAEMON (object));
> 
>         daemon = DAEMON (object);
> 
>         if (daemon->priv->bus_connection != NULL)
>                 g_object_unref (daemon->priv->bus_connection);
> 
>+        g_list_free_full (daemon->priv->explicitly_requested_users, g_free);
>+
>         g_hash_table_destroy (daemon->priv->users);
> 
>         g_hash_table_unref (daemon->priv->extension_ifaces);
> 
>         G_OBJECT_CLASS (daemon_parent_class)->finalize (object);
> }
> 
> static gboolean
> register_accounts_daemon (Daemon *daemon)
> {
>         GError *error = NULL;
> 
>         daemon->priv->authority = polkit_authority_get_sync (NULL, &error);
> 
>         if (daemon->priv->authority == NULL) {
>                 if (error != NULL) {
>                         g_critical ("error getting polkit authority: %s", error->message);
>                         g_error_free (error);
>                 }
>                 goto error;
>         }
> 
>         daemon->priv->bus_connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, &error);
>         if (daemon->priv->bus_connection == NULL) {
>                 if (error != NULL) {
>                         g_critical ("error getting system bus: %s", error->message);
>                         g_error_free (error);
>                 }
>                 goto error;
>         }
>@@ -726,84 +780,90 @@ add_new_user_for_pwent (Daemon        *daemon,
>         user_register (user);
> 
>         g_hash_table_insert (daemon->priv->users,
>                              g_strdup (user_get_user_name (user)),
>                              user);
> 
>         accounts_accounts_emit_user_added (ACCOUNTS_ACCOUNTS (daemon), user_get_object_path (user));
> 
>         return user;
> }
> 
> User *
> daemon_local_find_user_by_id (Daemon *daemon,
>                               uid_t   uid)
> {
>         User *user;
>         struct passwd *pwent;
> 
>         pwent = getpwuid (uid);
>         if (pwent == NULL) {
>                 g_debug ("unable to lookup uid %d", (int)uid);
>                 return NULL;
>         }
> 
>         user = g_hash_table_lookup (daemon->priv->users, pwent->pw_name);
> 
>         if (user == NULL) {
>                 struct spwd *spent;
>                 spent = getspnam (pwent->pw_name);
>                 user = add_new_user_for_pwent (daemon, pwent, spent);
>+
>+                daemon->priv->explicitly_requested_users = g_list_append (daemon->priv->explicitly_requested_users,
>+                                                                          g_strdup (pwent->pw_name));
>         }
> 
>         return user;
> }
> 
> User *
> daemon_local_find_user_by_name (Daemon      *daemon,
>                                 const gchar *name)
> {
>         User *user;
>         struct passwd *pwent;
> 
>         pwent = getpwnam (name);
>         if (pwent == NULL) {
>                 g_debug ("unable to lookup name %s: %s", name, g_strerror (errno));
>                 return NULL;
>         }
> 
>         user = g_hash_table_lookup (daemon->priv->users, pwent->pw_name);
> 
>         if (user == NULL) {
>                 struct spwd *spent;
>                 spent = getspnam (pwent->pw_name);
>                 user = add_new_user_for_pwent (daemon, pwent, spent);
>+
>+                daemon->priv->explicitly_requested_users = g_list_append (daemon->priv->explicitly_requested_users,
>+                                                                          g_strdup (pwent->pw_name));
>         }
> 
>         return user;
> }
> 
> User *
> daemon_local_get_automatic_login_user (Daemon *daemon)
> {
>         return daemon->priv->autologin;
> }
> 
> static gboolean
> daemon_find_user_by_id (AccountsAccounts      *accounts,
>                         GDBusMethodInvocation *context,
>                         gint64                 uid)
> {
>         Daemon *daemon = (Daemon*)accounts;
>         User *user;
> 
>         user = daemon_local_find_user_by_id (daemon, uid);
> 
>         if (user) {
>                 accounts_accounts_complete_find_user_by_id (NULL, context, user_get_object_path (user));
>         }
>         else {
>                 throw_error (context, ERROR_FAILED, "Failed to look up user with uid %d.", (int)uid);
>         }
> 
>         return TRUE;
> }
>-- 
>2.12.0

Actions: View | Splinter Review

Attachments on bug 100422: 130488

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.