Add PolicyKit authentication D-Bus method

New org.freedesktop.SystemToolsBackends.Authenticate interface, which can be called on all modules. This method takes no arguments, and returns a boolean, or error for cancelled or failed. It's implemented via dispatcher_filter_func(), calling dispatch_auth_message().

New check_polkit_auth() function to centralize PolicyKit authorizations handling, returning error when needed. We now require PolicyKit instead of it being an optional dependency, and need version 0.94 so that we can remove the PID checking workaround.

PolicyKit action for SelfConfig is now really used, so set it to "yes" since settings that can be changed that way are minor (just like the old behavior).

5 files changed, 136 insertions, 121 deletions

diff --git a/Makefile.am b/Makefile.am
index 6736b3e..2b7babe 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -60,10 +60,8 @@ systemd_DATA = org.freedesktop.SystemToolsBackends.conf
 
 policy_in_files = org.freedesktop.SystemToolsBackends.policy.in
 
-if HAVE_POLKIT
 policydir = $(POLKIT_POLICY_DIR)
 policy_DATA = $(policy_in_files:.policy.in=.policy)
 
 @INTLTOOL_POLICY_RULE@
 
-endif
diff --git a/configure.in b/configure.in
index 23f2a4e..21d263f 100644
--- a/configure.in
+++ b/configure.in
@@ -12,7 +12,7 @@ IT_PROG_INTLTOOL([0.40.0])
 DBUS_REQUIRED=1.1.2
 DBUS_GLIB_REQUIRED=0.74
 GLIB_REQUIRED=2.4.0
-POLICYKIT_REQUIRED=0.92
+POLICYKIT_REQUIRED=0.94
 GIO_REQUIRED=2.15.2
 
 dnl get prefix in $prefix, yes, this sucks
@@ -115,6 +115,7 @@ AC_SUBST([DBUS_LIBS])
 
 dnl check for the dispatcher deps
 PKG_CHECK_MODULES(DISPATCHER, [
+			   polkit-gobject-1 >= $POLICYKIT_REQUIRED
 			   dbus-glib-1 >= $DBUS_GLIB_REQUIRED
 			   glib-2.0    >= $GLIB_REQUIRED
 			   ])
@@ -122,17 +123,6 @@ DISPATCHER_CFLAGS="-DDBUS_API_SUBJECT_TO_CHANGE $DISPATCHER_CFLAGS"
 AC_SUBST(DISPATCHER_LIBS)
 AC_SUBST(DISPATCHER_CFLAGS)
 
-dnl check for PolicyKit
-PKG_CHECK_MODULES(POLKIT, polkit-gobject-1 >= $POLICYKIT_REQUIRED, have_polkit=yes, have_polkit=no)
-
-if test "$have_polkit" = "yes"; then
-  AC_DEFINE(HAVE_POLKIT, [1], [whether PolicyKit was found])
-fi
-
-AC_SUBST(POLKIT_LIBS)
-AC_SUBST(POLKIT_CFLAGS)
-AM_CONDITIONAL(HAVE_POLKIT, test x$have_polkit = xyes)
-
 dnl check for Gio
 PKG_CHECK_MODULES(GIO, gio-2.0 >= $GIO_REQUIRED, have_gio=yes, have_gio=no)
 
@@ -176,17 +166,19 @@ Utils/Makefile
 SystemToolsBackends.pl
 test-backends
 services/Makefile
-services/org.freedesktop.SystemToolsBackends.GroupsConfig.service
+services/org.freedesktop.SystemToolsBackends.GroupConfig2.service
+services/org.freedesktop.SystemToolsBackends.GroupsConfig2.service
 services/org.freedesktop.SystemToolsBackends.HostsConfig.service
 services/org.freedesktop.SystemToolsBackends.IfacesConfig.service
 services/org.freedesktop.SystemToolsBackends.NFSConfig.service
 services/org.freedesktop.SystemToolsBackends.NTPConfig.service
 services/org.freedesktop.SystemToolsBackends.Platform.service
+services/org.freedesktop.SystemToolsBackends.SelfConfig2.service
 services/org.freedesktop.SystemToolsBackends.ServicesConfig.service
 services/org.freedesktop.SystemToolsBackends.SMBConfig.service
 services/org.freedesktop.SystemToolsBackends.TimeConfig.service
-services/org.freedesktop.SystemToolsBackends.UserConfig.service
-services/org.freedesktop.SystemToolsBackends.UsersConfig.service
+services/org.freedesktop.SystemToolsBackends.UserConfig2.service
+services/org.freedesktop.SystemToolsBackends.UsersConfig2.service
 org.freedesktop.SystemToolsBackends.service
 Net-DBus/Makefile
 Net-DBus/lib/Makefile
@@ -205,7 +197,6 @@ Configuration (BACKENDS):
   The backends will be installed in : ${scriptsdir}
   The files will be installed in    : ${filesdir}
   Using internal copy of Net::Dbus  : ${internalnetdbus}
-  Use PolicyKit                     : ${have_polkit}
   File notification                 : ${have_gio}
 
   DBus system.d directory           : ${DBUS_SYSTEMD_DIR}
diff --git a/dispatcher/Makefile.am b/dispatcher/Makefile.am
index d515077..c4aecc2 100644
--- a/dispatcher/Makefile.am
+++ b/dispatcher/Makefile.am
@@ -1,7 +1,6 @@
 INCLUDES = \
 	-Wall	\
 	$(DISPATCHER_CFLAGS) \
-	$(POLKIT_CFLAGS) \
 	$(GIO_CFLAGS)
 
 sbin_PROGRAMS = system-tools-backends
@@ -10,7 +9,7 @@ if HAVE_GIO
 file_monitor_sources = file-monitor.c file-monitor.h
 endif
 
-system_tools_backends_LDADD = $(DISPATCHER_LIBS) $(POLKIT_LIBS) $(GIO_LIBS)
+system_tools_backends_LDADD = $(DISPATCHER_LIBS) $(GIO_LIBS)
 system_tools_backends_SOURCES = \
 	$(file_monitor_sources) \
 	dispatcher.c  dispatcher.h	\
diff --git a/dispatcher/dispatcher.c b/dispatcher/dispatcher.c
index 15b8eb9..2bac840 100644
--- a/dispatcher/dispatcher.c
+++ b/dispatcher/dispatcher.c
@@ -31,9 +31,7 @@
 #include "config.h"
 #include "dispatcher.h"
 
-#ifdef HAVE_POLKIT
 #include <polkit/polkit.h>
-#endif
 
 #ifdef HAVE_GIO
 #include "file-monitor.h"
@@ -42,6 +40,7 @@
 #define DBUS_ADDRESS_ENVVAR "DBUS_SESSION_BUS_ADDRESS"
 #define DBUS_INTERFACE_STB "org.freedesktop.SystemToolsBackends"
 #define DBUS_INTERFACE_STB_PLATFORM "org.freedesktop.SystemToolsBackends.Platform"
+#define DBUS_INTERFACE_STB_AUTH "org.freedesktop.SystemToolsBackends.Authentication"
 #define DBUS_PATH_SELF_CONFIG "/org/freedesktop/SystemToolsBackends/SelfConfig2"
 
 #define STB_DISPATCHER_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), STB_TYPE_DISPATCHER, StbDispatcherPrivate))
@@ -65,9 +64,7 @@ struct StbDispatcherPrivate
   DBusConnection *connection;
   gchar *platform;
 
-#ifdef HAVE_POLKIT
   PolkitAuthority *polkit_authority;
-#endif
 
 #ifdef HAVE_GIO
   StbFileMonitor *file_monitor;
@@ -311,98 +308,149 @@ get_destination (DBusMessage *message)
   return destination;
 }
 
+static void
+return_error (StbDispatcher *dispatcher,
+	      DBusMessage   *message,
+	      const gchar   *error_name,
+	      const gchar   *error_message)
+{
+  DBusMessage *reply;
+  StbDispatcherPrivate *priv;
+
+  priv = STB_DISPATCHER_GET_PRIVATE (dispatcher);
+
+  DEBUG (dispatcher, "sending error %s from: %s", error_name, dbus_message_get_path (message));
+
+  reply = dbus_message_new_error (message, error_name, error_message);
+  dbus_connection_send (priv->connection, reply, NULL);
+  dbus_message_unref (reply);
+}
+
 static gboolean
-can_caller_do_action (StbDispatcher *dispatcher,
-		      DBusMessage   *message,
-		      const gchar   *name)
+check_polkit_auth (StbDispatcher *dispatcher,
+                   DBusMessage   *message,
+                   GError       **ret_error)
 {
-#ifdef HAVE_POLKIT
   StbDispatcherPrivate *priv;
   PolkitSubject *subject;
   PolkitAuthorizationResult *result;
+  GError *error;
+  gchar **path;
   gchar *action_id;
-  gulong caller_pid;
   gboolean retval;
-  GError *gerror = NULL;
-  DBusError dbus_error;
-  DBusMessage *call, *reply;
-  const gchar *connection_name;
-
-  /* Allow getting information */
-  if (dbus_message_has_member (message, "get"))
-    return TRUE;
-
-  /* Do not allow anything besides "set", "add" or "del" past this point */
-  if (!(dbus_message_has_member (message, "set")
-       || dbus_message_has_member (message, "add")
-       || dbus_message_has_member (message, "del")))
-    return FALSE;
 
   priv = STB_DISPATCHER_GET_PRIVATE (dispatcher);
 
-  if (name)
-    action_id = g_strdup_printf ("org.freedesktop.systemtoolsbackends.%s.set", name);
+  if (dbus_message_get_path_decomposed (message, &path)
+      && path[0] && path[1] && path[2] && path[3]
+      && strcmp (path[3], "SelfConfig2") == 0)
+    action_id = g_strdup_printf ("org.freedesktop.systemtoolsbackends.self.set");
   else
     action_id = g_strdup_printf ("org.freedesktop.systemtoolsbackends.set");
 
-  /* Get the caller's PID using the connection name */
-  call = dbus_message_new_method_call ("org.freedesktop.DBus",
-				       "/org/freedesktop/DBus",
-				       "org.freedesktop.DBus",
-				       "GetConnectionUnixProcessID");
-  connection_name = dbus_message_get_sender (message);
-  dbus_message_append_args (call, DBUS_TYPE_STRING, &connection_name, DBUS_TYPE_INVALID);
-
-  dbus_error_init (&dbus_error);
-
-  reply = dbus_connection_send_with_reply_and_block (priv->connection, call, -1, &dbus_error);
-  if (dbus_error_is_set (&dbus_error))
-    goto dbus_error;
-
-  dbus_message_get_args (reply, &dbus_error, DBUS_TYPE_UINT32, &caller_pid, DBUS_TYPE_INVALID);
-  if (dbus_error_is_set (&dbus_error))
-    goto dbus_error;
-
-  dbus_message_unref (call);
-  dbus_message_unref (reply);
-
-  /* We need to identify the subject using its PID
-   * because it's how PolkitLockButton works on the client side */
-  subject = polkit_unix_process_new (caller_pid);
+  subject = polkit_system_bus_name_new (dbus_message_get_sender (message));
   result = polkit_authority_check_authorization_sync (priv->polkit_authority, subject, action_id, NULL,
                                                       POLKIT_CHECK_AUTHORIZATION_FLAGS_ALLOW_USER_INTERACTION,
-                                                      NULL, &gerror);
+                                                      NULL, &error);
+  if (result)
+    {
+      retval = polkit_authorization_result_get_is_authorized (result);
+      g_object_unref (result);
+    }
+  else
+    {
+      retval = FALSE;
 
-  g_object_unref (subject);
+      if (error->code == POLKIT_ERROR_CANCELLED)
+          /* Clients need to hanle this case separately to avoid showing an error dialog */
+          return_error (dispatcher, message,
+                        "org.freedesktop.SystemToolsBackends.AuthenticationCancelled",
+                        "Authentication cancelled by user");
+      else
+          return_error (dispatcher, message,
+                        "org.freedesktop.SystemToolsBackends.AuthenticationFailed",
+                        error->message);
 
-  if (gerror)
-    {
-      g_critical ("%s", gerror->message);
-      g_error_free (gerror);
-      g_free (action_id);
+        g_warning ("Error checking PolicyKit authorization: %s", error->message);
 
-      return FALSE;
+        if (!ret_error)
+          g_error_free (error);
+        else
+          *ret_error = error;
     }
 
-  retval = polkit_authorization_result_get_is_authorized (result);
-
   DEBUG (dispatcher,
 	 (retval) ? "subject is allowed to do action '%s'" : "subject can't do action '%s'",
 	 action_id);
 
   g_free (action_id);
+  g_object_unref (subject);
+
+  if (path)
+    g_strfreev (path);
 
   return retval;
+}
+
+static gboolean
+can_caller_do_action (StbDispatcher *dispatcher,
+		      DBusMessage   *message)
+{
+  gboolean retval;
 
-  dbus_error:
-    g_critical ("Could not get PID of the caller: %s", dbus_error.message);
-    dbus_error_free (&dbus_error);
-    g_free (action_id);
+  /* Allow getting information */
+  if (dbus_message_has_member (message, "get"))
+    return TRUE;
 
+  /* Do not allow anything besides "set", "add" or "del" past this point */
+  if (!(dbus_message_has_member (message, "set")
+       || dbus_message_has_member (message, "add")
+       || dbus_message_has_member (message, "del")))
     return FALSE;
-#else
-  return TRUE;
-#endif /* HAVE_POLKIT */
+
+  /* Replies with an error if needed, in which case retval is FALSE */
+  retval = check_polkit_auth (dispatcher, message, NULL);
+
+  return retval;
+}
+
+/*
+ * Handles all messages with DBUS_INTERFACE_STB_AUTH, which check authorizations
+ * for all modules.
+ */
+static void
+dispatch_auth_message (StbDispatcher *dispatcher,
+                       DBusMessage   *message)
+{
+  StbDispatcherPrivate *priv;
+  GError *error = NULL;
+
+  priv = dispatcher->_priv;
+
+  if (!dbus_message_has_interface (message, DBUS_INTERFACE_STB_AUTH))
+    return;
+
+  if (dbus_message_has_member (message, "authenticate"))
+    {
+      DBusMessage *reply;
+      DBusMessageIter iter;
+      gboolean authorized;
+
+      /* Replies with an error if needed */
+      authorized = check_polkit_auth (dispatcher, message, &error);
+
+      if (!error)
+        {
+           /* Create a reply with the authorization result */
+           reply = dbus_message_new_method_return (message);
+
+           dbus_message_iter_init_append (reply, &iter);
+           dbus_message_iter_append_basic (&iter, DBUS_TYPE_BOOLEAN, &authorized);
+
+           dbus_connection_send (priv->connection, reply, NULL);
+           dbus_message_unref (reply);
+        }
+    }
 }
 
 static void
@@ -450,24 +498,6 @@ dispatch_stb_message (StbDispatcher *dispatcher,
 }
 
 static void
-return_error (StbDispatcher *dispatcher,
-	      DBusMessage   *message,
-	      const gchar   *error_name)
-{
-  DBusMessage *reply;
-  StbDispatcherPrivate *priv;
-
-  priv = STB_DISPATCHER_GET_PRIVATE (dispatcher);
-
-  DEBUG (dispatcher, "sending error %s from: %s", error_name, dbus_message_get_path (message));
-
-  reply = dbus_message_new_error (message, error_name,
-				  "No permissions to perform the task.");
-  dbus_connection_send (priv->connection, reply, NULL);
-  dbus_message_unref (reply);
-}
-
-static void
 dispatch_platform_message (StbDispatcher *dispatcher,
 			   DBusMessage   *message)
 {
@@ -505,26 +535,27 @@ dispatch_self_config (StbDispatcher *dispatcher,
 {
   StbDispatcherPrivate *priv;
   const gchar *sender;
-  uid_t uid, message_uid;
+  gulong uid, message_uid;
 
   priv = dispatcher->_priv;
   sender = dbus_message_get_sender (message);
   uid = dbus_bus_get_unix_user (priv->connection, sender, NULL);
 
-  /* Absolutely avoid UID 0 being allowed */
+  /* Absolutely avoid passing UID 0 */
   g_return_if_fail (uid > 0);
 
   if (dbus_message_get_args (message, NULL,
-                             DBUS_TYPE_UINT32, &uid,
+                             DBUS_TYPE_UINT32, &message_uid,
                              DBUS_TYPE_INVALID)
                              && message_uid == uid)
     {
       dbus_message_set_sender (message, sender);
-      dispatch_stb_message (dispatcher, message, dbus_message_get_serial (message));
-      dbus_message_unref (message);
+      dispatch_stb_message (dispatcher, message, 0);
     }
   else
-    return_error (dispatcher, message, DBUS_ERROR_ACCESS_DENIED);
+    return_error (dispatcher, message,
+                  "org.freedesktop.SystemToolsBackends.InvalidMessage",
+                  "Malformed message was sent");
 }
 
 static DBusHandlerResult
@@ -546,19 +577,19 @@ dispatcher_filter_func (DBusConnection *connection,
     dispatch_stb_message (dispatcher, message, 0);
   else if (dbus_message_has_interface (message, DBUS_INTERFACE_STB_PLATFORM))
     dispatch_platform_message (dispatcher, message);
+  else if (dbus_message_has_interface (message, DBUS_INTERFACE_STB_AUTH))
+    dispatch_auth_message (dispatcher, message);
   else if (dbus_message_has_path (message, DBUS_PATH_SELF_CONFIG))
     {
-      if (can_caller_do_action (dispatcher, message, "self"))
+      /* Error is returned from can_caller_do_action() in case of failure */
+      if (can_caller_do_action (dispatcher, message))
 	dispatch_self_config (dispatcher, message);
-      else
-	return_error (dispatcher, message, DBUS_ERROR_ACCESS_DENIED);
     }
   else if (dbus_message_has_interface (message, DBUS_INTERFACE_STB))
     {
-      if (can_caller_do_action (dispatcher, message, NULL))
+      /* Error is returned from can_caller_do_action() in case of failure */
+      if (can_caller_do_action (dispatcher, message))
 	dispatch_stb_message (dispatcher, message, 0);
-      else
-	return_error (dispatcher, message, DBUS_ERROR_ACCESS_DENIED);
     }
 
   return DBUS_HANDLER_RESULT_HANDLED;
@@ -607,9 +638,7 @@ stb_dispatcher_init (StbDispatcher *dispatcher)
   /* we're screwed if we don't have this */
   g_assert (priv->connection != NULL);
 
-#ifdef HAVE_POLKIT
   priv->polkit_authority = polkit_authority_get ();
-#endif
 
 #ifdef HAVE_GIO
   priv->file_monitor = stb_file_monitor_new ();
@@ -662,9 +691,7 @@ stb_dispatcher_finalize (GObject *object)
 
   dbus_connection_unref (priv->connection);
 
-#ifdef HAVE_POLKIT
   g_object_unref (priv->polkit_authority);
-#endif
 
 #ifdef HAVE_GIO
   g_object_unref (priv->file_monitor);
diff --git a/org.freedesktop.SystemToolsBackends.policy.in b/org.freedesktop.SystemToolsBackends.policy.in
index f45ef58..a53aefb 100644
--- a/org.freedesktop.SystemToolsBackends.policy.in
+++ b/org.freedesktop.SystemToolsBackends.policy.in
@@ -19,7 +19,7 @@
     <_message>You need to authenticate to modify your user account information</_message>
     <defaults>
       <allow_inactive>no</allow_inactive>
-      <allow_active>auth_self</allow_active>
+      <allow_active>yes</allow_active>
     </defaults>
   </action>
 </policyconfig>