[sugar] Initial Security Patches

[Marco Pesenti Gritti]

On 7/30/07, Michael Stone <michael at laptop.org> wrote:
> Dear Sugar developers,
>
> Attached are the first round of containerization patches for review. These
> patches cause Sugar to use Rainbow to launch activities and cause Sugar to
> notify Rainbow of foreground-activity-changed events, both conditional on the
> existence of the flag-file '/etc/olpc-security'. They apply cleanly and work
> for me on a 539 installation.


Hello,

thanks for the patches, some comments follow...

+        if os.path.exists('/etc/olpc-security'):

The way we are usually doing this is to initialize the proxy in a
try/except block and fallback if the service is not available. Are we
using /etc/olpc-security here because we want to ensure we never run
without a Rainbow service on the XO?

+            system_bus = dbus.SystemBus()
+            rainbow = system_bus.get_object('org.laptop.security.Rainbow', '/')
+            self.rainbow = dbus.Interface(rainbow,
+                               dbus_interface='org.laptop.security.Rainbow')

Please use self._rainbow here, I don't see a reason the proxy should be public.

     @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)

The os.path.exists('/etc/olpc-security') is a little ugly, let's at
least avoid doing it when it's not necessary. I think we can just make
self._rainbow None when '/etc/olpc-security' doesn't exist above and
just "if self._rainbow" here.

Is there any reason the dbus call is not async here?

+        if not os.path.exists('/etc/olpc-security'):
+            self._factory.create(self._activity_handle.get_dict(),
+                                 timeout=120 * 1000,
+                                 reply_handler=self._no_reply_handler,
+                                 error_handler=self._create_error_handler)

I don't want the behavior with and without bitfrost to be so
different. If we are going for separate processes we should get rid of
the sugar factory completely and just run separate processes also in
the non-bitfrost case. So we either just launch the activity
executable from Sugar (which is what I assume bitfrost is doing) or we
make bitfrost run even without vserver.

Also before landing this we need to fix activities which must run with
a single process (web-activity for example) to implement their own
single process mechanism.

Marco