[Bug 36845] As well as ACLs for DBus calls, we need ACLs to filter which handlers get channels

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=36845

--- Comment #2 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2011-05-04 08:58:36 PDT ---
Review of attachment 46325:
 --> (https://bugs.freedesktop.org/review?bug=36845&attachment=46325)

::: mission-control-plugins/Makefile.am
@@ +56,3 @@

+if ENABLE_AEGIS
+libmission_control_plugins_la_SOURCES += builtin-aegis-acl.c

This should be in plugins or src, not in m-c-p. m-c-p should only contain
things we're willing to treat as stable C API (i.e. the GInterfaces for people
to implement, the GInterfaces representing what they'll be given, and an
absolute minimum of utility code).

I'd actually be tempted to move this back to plugins/, and just change the
build system to build it as a static convenience library (noinst_LTLIBRARIES),
to be linked into MC in src.

::: mission-control-plugins/builtin-aegis-acl.h
@@ +26,3 @@
+#include <sys/types.h>
+#include <sys/creds.h>
+#include <glib-object.h>

Don't include sys/types.h or sys/creds.h in this header unless you really need
to. They can almost certainly move to the .c file, when you've made the changes
below.

@@ +32,3 @@
+typedef struct {
+  GObject parent;
+} _AegisAcl;

C lets you point to a struct whose members you haven't defined, as long as you
tell it that it's a struct. The correct GObject idiom to make use of this is as
follows:

Near the top of the .h file:

    typedef struct _AegisAcl AegisAcl;

In the .c file if it's private (and here, it should be), or lower down the .h
file otherwise:

    struct _AegisAcl {
      ...
    };

In this particular case you don't necessarily need AegisAcl to appear in the
header at all: all MC cares about is that it's a GObject implementing
McpDBusAcl, so why not make aegis_acl_new() just return GObject* or
McpDBusAcl*? Then you can move both the structs, both the typedefs, and both
the sys/*.h into the implementation.

::: mission-control-plugins/dbus-acl.c
@@ +67,3 @@
+#if ENABLE_AEGIS
+#include "builtin-aegis-acl.h"
+#endif

Do this in src/plugin-loader.c instead.

Not directly relevant to this bug, but the logic for ENABLE_AEGIS is
astonishing. Normally, you're meant to #define the magic symbol to 1 to enable
something, or *leave it undefined* to disable - but you're defining it to 1 or
0, which violates least-astonishment for autoconf users.

@@ +148,3 @@
+  dbus_acls = g_list_prepend (dbus_acls, aegis_acl_new ());
+#endif
+

This entire function should have been in src/, really, but never mind.

Instead of this change, apply this pseudo-patch to src/plugin-loader.c:

>     mcp_read_dir (dir);
> +
> +   /* if enabled, the Aegis pseudo-plugin is part of the binary,
> +    * so you can't just bypass it by deleting the file */
> +   #if ENABLE_AEGIS
> +   mcp_add_object (aegis_acl_new ());
> +   #endif
> +
>      g_once_init_leave (&ready, 1);

(mcp_add_object() prepends, so that will treat the Aegis plugin as
highest-priority.)

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.