Re: User authentication to services

 On Thu, Jun 17, 2004 at 09:15:07PM -0400, Havoc Pennington wrote:
> Hi,
> 
> Thanks for the patch, looks good overall. Some small comments:

Many thanks for your help. HAL will be happy to be able to get the sender's uid 
:)

> - D-BUS code generally avoids assignment and declaration at once, 
>   i.e.:
>    char *service;
>    service = NULL;
>   is preferred.

Fixed.

> - I'd probably capitalize all of "UID"

Fixed.

> - the BaseName property replicates the functionality of 
>   the GetServiceOwner, so we need to decide between these two.

I've replaced it by GetProperty and changed:
  dbus/glib/dbus-gproxy.c:dbus_gproxy_new_for_service_owner() to call
  it instead. (Hmm, needs to be tested if it still works)

Is it ok, this way? If yes, I will change the spec too.

> - bus_connection_get_name() should not be able to return NULL for
>   a connection that owns a service, and we know this one does.
>   So you should _dbus_assert (base_name != NULL) rather than 
>   setting an error in that case I'm pretty sure.

Fixed.

> - the name "GetServiceProperty" isn't a bit funny; it's a 
>   property of the connection, not the service. 
>   Suggest just "GetProperty"

Renamed.
Huh, so many new names in the last days. I need to get used to :)

> - dbus_bus_get_service_uid() should be consistent with 
>   dbus_connection_get_unix_user() (in naming, and 
>   in dbus_uint32_t vs. unsigned long)

Fixed to return usigned long, but on the wire we still send 32bit, ok?

> - though I'm not sure we'll keep all of dbus-bus.[ch]
>   in the long run, this seems fine for now
> 
> - can't return -1 to uint32 without a cast; just use 
>   DBUS_UID_UNSET probably

Fixed.

Thanks,
Kay
Index: bus/driver.c
===================================================================
RCS file: /cvs/dbus/dbus/bus/driver.c,v
retrieving revision 1.49
diff -u -r1.49 driver.c
--- bus/driver.c        9 Jun 2004 18:15:09 -0000       1.49
+++ bus/driver.c        18 Jun 2004 02:15:11 -0000
@@ -757,64 +757,94 @@
 }
 
 static dbus_bool_t
-bus_driver_handle_get_service_owner (DBusConnection *connection,
-                                    BusTransaction *transaction,
-                                    DBusMessage    *message,
-                                    DBusError      *error)
+bus_driver_handle_get_service_property (DBusConnection *connection,
+                                          BusTransaction *transaction,
+                                          DBusMessage    *message,
+                                          DBusError      *error)
 {
-  char *text;
-  const char *base_name;
+  char *service;
+  char *property;
   DBusString str;
   BusRegistry *registry;
-  BusService *service;
+  BusService *serv;
+  DBusConnection *conn;
   DBusMessage *reply;
-  
+  unsigned long uid;
+  const char *base_name;
+
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
 
   registry = bus_connection_get_registry (connection);
 
-  text = NULL;
   reply = NULL;
 
   if (! dbus_message_get_args (message, error,
-                              DBUS_TYPE_STRING, &text,
+                              DBUS_TYPE_STRING, &service,
+                              DBUS_TYPE_STRING, &property,
                               DBUS_TYPE_INVALID))
       goto failed;
 
-  _dbus_string_init_const (&str, text);
-  service = bus_registry_lookup (registry, &str);
-  if (service == NULL)
+  _dbus_verbose ("asked for %s on %s\n", property, service);
+
+  _dbus_string_init_const (&str, service);
+  serv = bus_registry_lookup (registry, &str);
+  if (serv == NULL)
     {
       dbus_set_error (error, 
                      DBUS_ERROR_SERVICE_HAS_NO_OWNER,
-                     "Could not get owner of service '%s': no such service", 
text);
+                     "Could not get owner of service '%s': no such service", 
service);
       goto failed;
     }
 
-  base_name = bus_connection_get_name (bus_service_get_primary_owner 
(service));
-  if (base_name == NULL)
-    {
-      dbus_set_error (error,
-                     DBUS_ERROR_FAILED,
-                     "Could not determine base service for '%s'", text);
-      goto failed;
-    }
-  _dbus_assert (*base_name == ':');
+  conn = bus_service_get_primary_owner (serv);
 
   reply = dbus_message_new_method_return (message);
   if (reply == NULL)
     goto oom;
 
-  if (! dbus_message_append_args (reply, 
-                                 DBUS_TYPE_STRING, base_name,
-                                 DBUS_TYPE_INVALID))
-    goto oom;
-  
+  if (strcmp ("UID", property) == 0)
+    {
+
+      if (!dbus_connection_get_unix_user (conn, &uid))
+        {
+          dbus_set_error (error,
+                          DBUS_ERROR_FAILED,
+                          "Could not determine UID for '%s'", service);
+          goto failed;
+        }
+    
+      _dbus_verbose (" found UID=%i\n", (dbus_uint32_t) uid);
+      if (! dbus_message_append_args (reply,
+                                      DBUS_TYPE_UINT32, (dbus_uint32_t) uid,
+                                      DBUS_TYPE_INVALID))
+        goto oom;
+    }
+  else if (strcmp ("BaseName", property) == 0)
+    {
+      base_name = bus_connection_get_name (conn);
+      _dbus_assert (base_name != NULL);
+      _dbus_assert (base_name[0] == ':');
+      
+      _dbus_verbose (" found base_name=%s\n", base_name);
+      if (! dbus_message_append_args (reply,
+                                      DBUS_TYPE_STRING, base_name,
+                                      DBUS_TYPE_INVALID))
+        goto oom;
+    }
+  else
+    {
+      dbus_set_error (error,
+                      DBUS_ERROR_FAILED,
+                      "Unknown property '%s'", property);
+      goto failed;
+    }  
+
   if (! bus_transaction_send_from_driver (transaction, connection, reply))
     goto oom;
 
   dbus_message_unref (reply);
-  dbus_free (text);
+  dbus_free (service);
+  dbus_free (property);
 
   return TRUE;
 
@@ -825,7 +855,8 @@
   _DBUS_ASSERT_ERROR_IS_SET (error);
   if (reply)
     dbus_message_unref (reply);
-  dbus_free (text);
+  dbus_free (service);
+  dbus_free (property);
   return FALSE;
 }
 
@@ -874,7 +905,7 @@
   { "ListServices", bus_driver_handle_list_services },
   { "AddMatch", bus_driver_handle_add_match },
   { "RemoveMatch", bus_driver_handle_remove_match },
-  { "GetServiceOwner", bus_driver_handle_get_service_owner },
+  { "GetProperty", bus_driver_handle_get_service_property },
   { "ReloadConfig", bus_driver_handle_reload_config }
 };
 
Index: dbus/dbus-bus.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-bus.c,v
retrieving revision 1.29
diff -u -r1.29 dbus-bus.c
--- dbus/dbus-bus.c     9 Jun 2004 18:15:09 -0000       1.29
+++ dbus/dbus-bus.c     18 Jun 2004 02:15:12 -0000
@@ -627,6 +627,79 @@
 }
 
 /**
+ * Asks the bus to return the uid of a service.
+ *
+ * @param connection the connection
+ * @param service_name the service name
+ * @param error location to store the error
+ * @returns a result code, -1 if error is set
+ */ 
+unsigned long
+dbus_bus_get_unix_user (DBusConnection *connection,
+                        const char     *service,
+                        DBusError      *error)
+{
+  DBusMessage *message, *reply;
+  dbus_uint32_t uid;  
+
+  _dbus_return_val_if_fail (connection != NULL, DBUS_UID_UNSET);
+  _dbus_return_val_if_fail (service != NULL, DBUS_UID_UNSET);
+  _dbus_return_val_if_error_is_set (error, DBUS_UID_UNSET);
+  
+  message = dbus_message_new_method_call (DBUS_SERVICE_ORG_FREEDESKTOP_DBUS,
+                                          DBUS_PATH_ORG_FREEDESKTOP_DBUS,
+                                          DBUS_INTERFACE_ORG_FREEDESKTOP_DBUS,
+                                          "GetProperty");
+
+  if (message == NULL)
+    {
+      _DBUS_SET_OOM (error);
+      return DBUS_UID_UNSET;
+    }
+ 
+  if (!dbus_message_append_args (message,
+                                DBUS_TYPE_STRING, service,
+                                DBUS_TYPE_STRING, "UID",
+                                DBUS_TYPE_INVALID))
+    {
+      dbus_message_unref (message);
+      _DBUS_SET_OOM (error);
+      return DBUS_UID_UNSET;
+    }
+  
+  reply = dbus_connection_send_with_reply_and_block (connection, message, -1,
+                                                     error);
+  
+  dbus_message_unref (message);
+  
+  if (reply == NULL)
+    {
+      _DBUS_ASSERT_ERROR_IS_SET (error);
+      return DBUS_UID_UNSET;
+    }  
+
+  if (dbus_set_error_from_message (error, reply))
+    {
+      _DBUS_ASSERT_ERROR_IS_SET (error);
+      dbus_message_unref (reply);
+      return DBUS_UID_UNSET;
+    }
+  
+  if (!dbus_message_get_args (reply, error,
+                              DBUS_TYPE_UINT32, &uid,
+                              DBUS_TYPE_INVALID))
+    {
+      _DBUS_ASSERT_ERROR_IS_SET (error);
+      dbus_message_unref (reply);
+      return DBUS_UID_UNSET;
+    }
+
+  dbus_message_unref (reply);
+  
+  return (unsigned long) uid;
+}
+
+/**
  * Checks whether a certain service exists.
  *
  * @param connection the connection
Index: dbus/dbus-bus.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-bus.h,v
retrieving revision 1.9
diff -u -r1.9 dbus-bus.h
--- dbus/dbus-bus.h     2 Dec 2003 10:44:21 -0000       1.9
+++ dbus/dbus-bus.h     18 Jun 2004 02:15:12 -0000
@@ -45,6 +45,9 @@
 dbus_bool_t     dbus_bus_set_base_service (DBusConnection *connection,
                                           const char     *base_service);
 const char*     dbus_bus_get_base_service (DBusConnection *connection);
+unsigned long   dbus_bus_get_unix_user    (DBusConnection *connection,
+                                          const char     *service,
+                                           DBusError      *error);
 int             dbus_bus_acquire_service  (DBusConnection *connection,
                                           const char     *service_name,
                                           unsigned int    flags,
Index: glib/dbus-gproxy.c
===================================================================
RCS file: /cvs/dbus/dbus/glib/dbus-gproxy.c,v
retrieving revision 1.10
diff -u -r1.10 dbus-gproxy.c
--- glib/dbus-gproxy.c  2 Jun 2004 13:13:14 -0000       1.10
+++ glib/dbus-gproxy.c  18 Jun 2004 02:15:13 -0000
@@ -956,12 +956,13 @@
   request = dbus_message_new_method_call (DBUS_SERVICE_ORG_FREEDESKTOP_DBUS,
                                          DBUS_PATH_ORG_FREEDESKTOP_DBUS,
                                          DBUS_INTERFACE_ORG_FREEDESKTOP_DBUS,
-                                         "GetServiceOwner");
+                                         "GetProperty");
   if (request == NULL)
     g_error ("Out of memory");
   
   if (! dbus_message_append_args (request, 
                                  DBUS_TYPE_STRING, service_name, 
+                                 DBUS_TYPE_STRING, "BaseName", 
                                  DBUS_TYPE_INVALID))
     g_error ("Out of memory");