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 c9c0bed7546842081f6faf47b2a0380cc9d1ee3c..0e1616d6d4623204d2b10fb83b26f014a8b64a40 100644 (file)
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -1079,7 +1079,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 74cb41bd20e2c5caaf42229bde2df482de9ef642..b6890c798052e90682959335ac7168edd87759e3 100644 (file)
--- 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 9fe7d55f04c92d28855fa3df761f095df3781953..eb5654ea9b3bebd6ea9707b539f166898a7ad115 100644 (file)
--- a/dbus/dbus-sysdeps-util-unix.c
+++ b/dbus/dbus-sysdeps-util-unix.c
@@ -992,14 +992,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 c572fcd0bb40b3fc99da27f1424051d07d7723e8..5e4634ff860dfa09cfd231a4b3f575de9866252f 100644 (file)
--- a/dbus/dbus-sysdeps-util-win.c
+++ b/dbus/dbus-sysdeps-util-win.c
@@ -649,6 +649,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
@@ -662,8 +669,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;
 }
 
@@ -707,13 +713,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 e7e36ad67f98a27dd10b97d2f49e9e5ef0cd3e42..33637337103af2f55a8bcd35f19a39f84a29357c 100644 (file)
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -298,7 +298,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 1ca21eb717138f89ba987fdf1fc7a12b2ab3dc9e..0093ee49f2bb1210f0691bedd3fef332c6b964a6 100644 (file)
--- a/dbus/dbus-userdb-util.c
+++ b/dbus/dbus-userdb-util.c
@@ -373,31 +373,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;
@@ -410,6 +414,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 fcb515c83ab4bf2d56ca1d310cc440bd812571e1..9026caa4ddfc6237042ebca04a97e77b5e96aaa4 100644 (file)
--- a/dbus/dbus-userdb.h
+++ b/dbus/dbus-userdb.h
@@ -100,7 +100,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 a1777bb2aee9aaf4d4e443c81549108af0352b12..79b926f2f4b6bf367c6a0fe2b7dcdafd3de0d4e0 100644 (file)
--- a/test/internals/misc-internals.c
+++ b/test/internals/misc-internals.c
@@ -935,7 +935,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");
@@ -946,8 +946,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),