[sugar] Initial Security Patches

[Noah Kantrowitz]

Michael Stone wrote:
> Simon,
>
> On Tue, 2007-07-31 at 14:20 +0100, Simon McVittie wrote:
>   
>> On Mon, 30 Jul 2007 at 16:21:36 -0400, Michael Stone wrote:
>>     
>>>      @dbus.service.signal(_DBUS_OWNER_IFACE, signature="s")
>>>      def CurrentActivityChanged(self, activity_id):
>>> -        pass
>>> +        if os.path.exists('/etc/olpc-security'):
>>> +            self.rainbow.activity_changed(activity_id)
>>>       
>> Minor: It's conventional for D-Bus methods to be named in CamelCase.
>>     
>
> I don't care much whether we follow this convention or the PEP-8
> convention:
>
>     "Function names should be lowercase, with words separated by
>     underscores as necessary to improve readability."
>   
I changed the two new methods in rainbow, but left "create_activity" to
avoid breaking sugar.
>   
>> More major: This looks like an example of the same method/signal
>> naming inversion we have in Telepathy's streamed media interfaces (which we
>> consider to be a bug). Methods should be verbs, like ChangeActivity;
>> signals should be events, like ActivityChanged. Can't Rainbow just
>> listen for _DBUS_OWNER_IFACE::CurrentActivityChanged? If you're implementing
>> Rainbow in Python, that's spelt:
>>
>>     bus = dbus.SessionBus()
>>     bus.add_signal_receiver(some_callback, 'CurrentActivityChanged',
>>                             _DBUS_OWNER_IFACE)
>>     
>
> Rainbow runs as root; hence, it is statically prevented from connecting to the
> DBus session bus by the DBus access control checks. Options include:
>
>   1) Do what we presently do.
>   2) Modify DBus to make an exception to this access check.
>   3) Have Sugar also send the signal on the system bus.
>
> We leaned towards number (1) because it is minimally invasive and because it
> suggests a good future path for gating activity-foregrounding request should
> this prove necessary.
>
> No. (2), though technically possible and in some ways, convenient, is not going
> to fly with the DBus upstream maintainers because it's not a good general
> solution. 
>
> If (3) is dramatically more palatable than (1), I don't mind switching now.
>
>
>   
2 is actually fairly easy, its just a few lines in our session.conf (the
session bus daemon config file). The reason I avoided it is more that it
breaks the design behind the system/session split more than 1.
>> If you only want to respond to CurrentActivityChanged from the process
>> or object that ought to be emitting it, you can also specify a bus name
>> or object path, or you can use connect_to_signal() on a proxy for that
>> object.
>>     
>
> This is good to know; however, insofar as we care, we actually care about
> preventing unauthorized processes from sending the signal. In fact, "Sugar"
> understood broadly is the only group of processes that will be allowed to send
> messages to Rainbow.
>
>   
So far yes, P_IDENT may prove more interesting though depending on how
we coordinate the activity with presence service and rainbow.
>> If you must use a method call for some technical reason, it'd be less
>> astonishing if it was called something like ChangeCurrentActivity.
>>     
>
> Not a problem.
>
>   
Already done in the latest patch.
>> Proxy objects trigger an Introspect() call every time they're created,
>> so you may want to cache this proxy object for use in future launches.
>>     
>
> Okay.
>   
Ditto.
>
> Thanks again,
>
>
> Michael
>
>
> _______________________________________________
> Sugar mailing list
> Sugar at lists.laptop.org
> http://lists.laptop.org/listinfo/sugar
>
>