First pass on OLPC security modifications to DBus

[Havoc Pennington]

Hi,

Thanks for the patch, here are some comments:

 - if we get one more of these besides selinux and olpc we will probably need
   some kind of extensible security module thing ;-)

 - isn't there some name for this feature other than "olpc"? something
   specific to the security architecture or whatever?

 - the config file setting should be like <fork/> where its presence
means "true"
   (if we allowed "false" to override later in the file, that would be
done with a
    <nofork/> or <noolpc/> but I think there's no need for that)
  The point here is <olpc/> rather than <olpc>on</olpc>

 - config file element needs a patch to the dbus-daemon.1.in man page

 - the header is wrong on olpc.c, both the Emacs magic and the license don't
   match the rest of dbus (olpc.h also)

 - since you didn't have indent-tabs-mode nil, be sure to M-x untabify
   to be sure none leaked in

 - caching the system connection from dbus_bus_get() in a static variable
   just raises questions and issues, I would not do it, just call dbus_bus_get
   every time (and remember to unref the result). This will also theoretically
   allow you to survive a system bus restart, for those who feel that is
   feasible.

 - bus_olpc_check_can_own() fails to set the DBusError in a few places...
   it looks like this is intentional based on how this function is called, but
   I'm not sure I understand the semantics? if there's no memory you
   should BUS_SET_OOM presumably?

 - your system bus service seems to use the path "/" which I would
consider bad practice,
   it essentially encodes in the API that the process offers only that
single object and
   API, while it would be reasonable to offer some other second API from the
   process as well - say a system service control API with methods like Start()
   and Stop(), just to speculate on a possible one. This is like
naming a global variable
   "the variable" instead of "security_manager" or something.

 - since bus_olpc_check_can_own connects to the system bus and blocks on it,
   you probably want to take make it an error if the config file has bus type
   "system" and also contains the <olpc/> element, or something...

 - stdint.h should not be in olpc.h, since olpc.c is all
linux-specific anyway it's
   fine in there, though using the dbus typedefs would be more consistent

 - I would add the new config file element to one of the test config
files so the parsing code
   at least gets tested

 - I'm not sure how to meaningfully unit test the rest of this, but it
seems straightforward
   so hopefully no big issues

We should carefully review for memory leaks and OOM handling and so
forth when you have a more final version of the patch, I did not step
through the code line-by-line yet.

Havoc