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>

7 files changed, 34 insertions, 16 deletions

diff --git a/bus/connection.c b/bus/connection.c
index 7a482e2b..8a8ce5f4 100644
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -1009,7 +1009,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 a37be804..6c22a103 100644
--- a/bus/policy.c
+++ b/bus/policy.c
@@ -450,7 +450,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 bed6fd3e..961b3eff 100644
--- a/dbus/dbus-sysdeps-util-unix.c
+++ b/dbus/dbus-sysdeps-util-unix.c
@@ -987,14 +987,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 8c8fbed8..42b1acd2 100644
--- a/dbus/dbus-sysdeps-util-win.c
+++ b/dbus/dbus-sysdeps-util-win.c
@@ -647,6 +647,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
@@ -660,8 +667,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;
 }
 
@@ -705,13 +711,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 24fbec6a..93d85d28 100644
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -278,7 +278,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 170d233e..44c898ee 100644
--- a/dbus/dbus-userdb-util.c
+++ b/dbus/dbus-userdb-util.c
@@ -406,31 +406,35 @@ _dbus_user_database_get_gid (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;
@@ -443,6 +447,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;
         }
@@ -473,7 +478,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_assert_not_reached ("didn't get username");
@@ -484,7 +489,7 @@ _dbus_userdb_test (const char *test_data_dir)
   if (!_dbus_get_user_id (username, &uid))
     _dbus_assert_not_reached ("didn't get uid");
 
-  if (!_dbus_groups_from_uid (uid, &group_ids, &n_group_ids))
+  if (!_dbus_groups_from_uid (uid, &group_ids, &n_group_ids, &error))
     _dbus_assert_not_reached ("didn't get groups");
 
   printf ("    Current user: %s homedir: %s gids:",
diff --git a/dbus/dbus-userdb.h b/dbus/dbus-userdb.h
index b38e3d18..b161b395 100644
--- a/dbus/dbus-userdb.h
+++ b/dbus/dbus-userdb.h
@@ -111,7 +111,8 @@ dbus_bool_t _dbus_credentials_from_uid          (dbus_uid_t         user_id,
                                                  DBusCredentials   *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);
 DBUS_PRIVATE_EXPORT
 dbus_bool_t _dbus_is_console_user               (dbus_uid_t         uid,
                                                  DBusError         *error);