userdb: Add proper error reporting when getting groups from a uid

Previously, if dbus_connection_get_unix_user() succeeded but
_dbus_unix_groups_from_uid() failed, then bus_connection_get_unix_groups()
would incorrectly fail without setting the error indicator, resulting
in "(null)" being logged, which is rather unhelpful.

This also lets us distinguish between ENOMEM and other errors, such as
the uid not existing in the system's user database.

Fixes: 145fb99b (untitled refactoring commit, 2006-12-12)
Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343
Signed-off-by: Simon McVittie <smcv@collabora.com>

diff --git a/bus/connection.c b/bus/connection.c
index e3d876e43ed98b7124b527fe0ba6c3f2eedf7cc5..a95e1fad0852bea23b63518a81fe7fcc893df9a6 100644 (file)
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -1081,7 +1081,7 @@ bus_connection_get_unix_groups  (DBusConnection   *connection,
 
   if (dbus_connection_get_unix_user (connection, &uid))
     {
-      if (!_dbus_unix_groups_from_uid (uid, groups, n_groups))
+      if (!_dbus_unix_groups_from_uid (uid, groups, n_groups, error))
         {
           _dbus_verbose ("Did not get any groups for UID %lu\n",
                          uid);

diff --git a/bus/policy.c b/bus/policy.c
index 8c59cf0c84f68adbe1317b3c05cfe876aaabfaf8..6ef26a29c84383e29a0d489365f0ed712e65b2c4 100644 (file)
--- a/bus/policy.c
+++ b/bus/policy.c
@@ -452,7 +452,7 @@ bus_policy_allow_unix_user (BusPolicy        *policy,
   int n_group_ids;
 
   /* On OOM or error we always reject the user */
-  if (!_dbus_unix_groups_from_uid (uid, &group_ids, &n_group_ids))
+  if (!_dbus_unix_groups_from_uid (uid, &group_ids, &n_group_ids, NULL))
     {
       _dbus_verbose ("Did not get any groups for UID %lu\n",
                      uid);

diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c
index bda0198bac4eeb033cf0a5498f48317f00226c54..49367d439053abf616145179ba40d88c5f3e628a 100644 (file)
--- a/dbus/dbus-sysdeps-util-unix.c
+++ b/dbus/dbus-sysdeps-util-unix.c
@@ -947,14 +947,16 @@ _dbus_parse_unix_group_from_config (const DBusString  *groupname,
  * @param uid the UID
  * @param group_ids return location for array of group IDs
  * @param n_group_ids return location for length of returned array
+ * @param error error location
  * @returns #TRUE if the UID existed and we got some credentials
  */
 dbus_bool_t
 _dbus_unix_groups_from_uid (dbus_uid_t            uid,
                             dbus_gid_t          **group_ids,
-                            int                  *n_group_ids)
+                            int                  *n_group_ids,
+                            DBusError            *error)
 {
-  return _dbus_groups_from_uid (uid, group_ids, n_group_ids);
+  return _dbus_groups_from_uid (uid, group_ids, n_group_ids, error);
 }
 
 /**

diff --git a/dbus/dbus-sysdeps-util-win.c b/dbus/dbus-sysdeps-util-win.c
index 12d7b369fdab7b1392b96d6a9897a07d48759a48..befe1ba29eda650193d84ff9bd35995d3342e6b8 100644 (file)
--- a/dbus/dbus-sysdeps-util-win.c
+++ b/dbus/dbus-sysdeps-util-win.c
@@ -651,6 +651,13 @@ dbus_bool_t _dbus_windows_user_is_process_owner (const char *windows_sid)
   unix emulation functions - should be removed sometime in the future
  =====================================================================*/
 
+static void
+set_unix_uid_unsupported (DBusError *error)
+{
+  dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED,
+                  "UNIX user IDs not supported on Windows");
+}
+
 /**
  * Checks to see if the UNIX user ID is at the console.
  * Should always fail on Windows (set the error to
@@ -664,8 +671,7 @@ dbus_bool_t
 _dbus_unix_user_is_at_console (dbus_uid_t         uid,
                                DBusError         *error)
 {
-  dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED,
-                  "UNIX user IDs not supported on Windows\n");
+  set_unix_uid_unsupported (error);
   return FALSE;
 }
 
@@ -709,13 +715,16 @@ _dbus_parse_unix_user_from_config (const DBusString  *username,
  * @param uid the UID
  * @param group_ids return location for array of group IDs
  * @param n_group_ids return location for length of returned array
+ * @param error error location
  * @returns #TRUE if the UID existed and we got some credentials
  */
 dbus_bool_t
 _dbus_unix_groups_from_uid (dbus_uid_t            uid,
                             dbus_gid_t          **group_ids,
-                            int                  *n_group_ids)
+                            int                  *n_group_ids,
+                            DBusError            *error)
 {
+  set_unix_uid_unsupported (error);
   return FALSE;
 }
 

diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
index 91b6016f67f07167c50aceb8b69b72e21d87d2ec..786d9669c4702d09964d0e6d1e002d8a90d8c022 100644 (file)
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -302,7 +302,8 @@ dbus_bool_t _dbus_parse_unix_group_from_config  (const DBusString  *groupname,
                                                  dbus_gid_t        *gid_p);
 dbus_bool_t _dbus_unix_groups_from_uid          (dbus_uid_t         uid,
                                                  dbus_gid_t       **group_ids,
-                                                 int               *n_group_ids);
+                                                 int               *n_group_ids,
+                                                 DBusError         *error);
 dbus_bool_t _dbus_unix_user_is_at_console       (dbus_uid_t         uid,
                                                  DBusError         *error);
 dbus_bool_t _dbus_unix_user_is_process_owner    (dbus_uid_t         uid);

diff --git a/dbus/dbus-userdb-util.c b/dbus/dbus-userdb-util.c
index 4bb5c5338f75873c7c7247082540e7b39c5f23fa..558acff10381a7f221e032eae2638b669227c430 100644 (file)
--- a/dbus/dbus-userdb-util.c
+++ b/dbus/dbus-userdb-util.c
@@ -342,31 +342,35 @@ _dbus_user_database_lookup_group (DBusUserDatabase *db,
  * @param uid the UID
  * @param group_ids return location for array of group IDs
  * @param n_group_ids return location for length of returned array
+ * @param error error to fill in on failure
  * @returns #TRUE if the UID existed and we got some credentials
  */
 dbus_bool_t
 _dbus_groups_from_uid (dbus_uid_t         uid,
                        dbus_gid_t       **group_ids,
-                       int               *n_group_ids)
+                       int               *n_group_ids,
+                       DBusError         *error)
 {
   DBusUserDatabase *db;
   const DBusUserInfo *info;
   *group_ids = NULL;
   *n_group_ids = 0;
 
-  /* FIXME: this can't distinguish ENOMEM from other errors */
   if (!_dbus_user_database_lock_system ())
-    return FALSE;
+    {
+      _DBUS_SET_OOM (error);
+      return FALSE;
+    }
 
   db = _dbus_user_database_get_system ();
   if (db == NULL)
     {
+      _DBUS_SET_OOM (error);
       _dbus_user_database_unlock_system ();
       return FALSE;
     }
 
-  if (!_dbus_user_database_get_uid (db, uid,
-                                    &info, NULL))
+  if (!_dbus_user_database_get_uid (db, uid, &info, error))
     {
       _dbus_user_database_unlock_system ();
       return FALSE;
@@ -379,6 +383,7 @@ _dbus_groups_from_uid (dbus_uid_t         uid,
       *group_ids = dbus_new (dbus_gid_t, info->n_group_ids);
       if (*group_ids == NULL)
         {
+          _DBUS_SET_OOM (error);
          _dbus_user_database_unlock_system ();
           return FALSE;
         }

diff --git a/dbus/dbus-userdb.h b/dbus/dbus-userdb.h
index 1853a430772f68c10e06208712dcb23fbcf177cd..d37d2433222440b64033769da8017287a17a7ba9 100644 (file)
--- a/dbus/dbus-userdb.h
+++ b/dbus/dbus-userdb.h
@@ -102,7 +102,8 @@ dbus_bool_t _dbus_get_user_id_and_primary_group (const DBusString  *username,
                                                  dbus_gid_t        *gid_p);
 dbus_bool_t _dbus_groups_from_uid              (dbus_uid_t            uid,
                                                  dbus_gid_t          **group_ids,
-                                                 int                  *n_group_ids);
+                                                 int                  *n_group_ids,
+                                                 DBusError            *error);
 DBUS_PRIVATE_EXPORT
 dbus_bool_t _dbus_is_console_user               (dbus_uid_t         uid,
                                                  DBusError         *error);

diff --git a/test/internals/misc-internals.c b/test/internals/misc-internals.c
index 0dfbabe55c6e97969f292f9bc7d4992d00d732f8..3fc9d504dbc79e1f2e332599ac1598e2994a2367 100644 (file)
--- a/test/internals/misc-internals.c
+++ b/test/internals/misc-internals.c
@@ -979,7 +979,7 @@ _dbus_userdb_test (const char *test_data_dir)
   dbus_uid_t uid;
   unsigned long *group_ids;
   int n_group_ids, i;
-  DBusError error;
+  DBusError error = DBUS_ERROR_INIT;
 
   if (!_dbus_username_from_current_process (&username))
     _dbus_test_fatal ("didn't get username");
@@ -990,8 +990,8 @@ _dbus_userdb_test (const char *test_data_dir)
   if (!_dbus_get_user_id (username, &uid))
     _dbus_test_fatal ("didn't get uid");
 
-  if (!_dbus_groups_from_uid (uid, &group_ids, &n_group_ids))
-    _dbus_test_fatal ("didn't get groups");
+  if (!_dbus_groups_from_uid (uid, &group_ids, &n_group_ids, &error))
+    _dbus_test_fatal ("didn't get groups: %s: %s", error.name, error.message);
 
   _dbus_test_diag ("    Current user: %s homedir: %s gids:",
           _dbus_string_get_const_data (username),