[sugar] [PATCH] services/presence/: identify Buddies by "key ID" (pubkey hash), not whole key.

Simon McVittie simon.mcvittie at collabora.co.uk
Mon May 28 12:59:11 EDT 2007


I thought I should send this one in for dcbw's approval, since it's a
behaviour change...

"""
services/presence/: identify Buddies by "key ID" (pubkey hash), not whole key.

This allows us to create Buddy objects as soon as we see a contact on the
server. For contacts not on trusted servers, or seen in anonymous MUCs, we
create a Buddy identified by JID instead (so we have some way to talk
about the anonymous contact within the Sugar API).

The concept of "trusted server" means a server which we trust to validate that
users with a keyID as the username part of their JID do in fact have that key.
Currently we just pretend that olpc.collabora.co.uk does this - in future, the
school servers will do this validation by using key rather than password
authentication.

Also create Buddy object paths based on the keyID or JID (for easier debugging).
"""

This will allow me to improve the Sugar-visible API by mapping from
Buddies to handles (and back) in a synchronous way, so we can track
buddies entering and leaving activities in a sane way, for instance. Also, we
can interoperate with non-OLPC servers correctly (with a Buddy object
present even for non-OLPC users).

I'm assuming here that the key-ID is "sufficiently unique" across all
trusted servers. It's a SHA-1 of the public key, so basically the
same strength as GnuPG key fingerprints and git object hashes.
(Actually, for hysterical raisins, it's a SHA-1 of Base64(public_key) -
we should probably change this before we ship.)

The actual function used for the key-ID can be changed (if it
is, old and new versions of Sugar will be incompatible, but that's not
really a problem yet) so if any crypto gurus want to specify something
different, now would be a really good time. As currently implemented, its
output must be short enough to put in the JID (for which a hex SHA-1 is
somewhat long already).

---
 services/presence/buddy.py           |   49 +++++++++-------
 services/presence/presenceservice.py |   48 +++++++++------
 services/presence/pstest.py          |   10 ++-
 services/presence/server_plugin.py   |  108 ++++++++++++++++++++++++++++++++--
 4 files changed, 166 insertions(+), 49 deletions(-)

diff --git a/services/presence/buddy.py b/services/presence/buddy.py
index 1b45fd5..b858b41 100644
--- a/services/presence/buddy.py
+++ b/services/presence/buddy.py
@@ -37,6 +37,7 @@ _PROP_CURACT = "current-activity"
 _PROP_COLOR = "color"
 _PROP_OWNER = "owner"
 _PROP_VALID = "valid"
+_PROP_OBJID = 'objid'
 
 # Will go away soon
 _PROP_IP4_ADDRESS = "ip4-address"
@@ -90,15 +91,14 @@ class Buddy(ExportedGObject):
     }
 
     __gproperties__ = {
-        _PROP_KEY          : (str, None, None, None,
-                              gobject.PARAM_READWRITE |
-                              gobject.PARAM_CONSTRUCT_ONLY),
+        _PROP_KEY          : (str, None, None, None, gobject.PARAM_READWRITE),
         _PROP_ICON         : (object, None, None, gobject.PARAM_READWRITE),
         _PROP_NICK         : (str, None, None, None, gobject.PARAM_READWRITE),
         _PROP_COLOR        : (str, None, None, None, gobject.PARAM_READWRITE),
         _PROP_CURACT       : (str, None, None, None, gobject.PARAM_READWRITE),
         _PROP_VALID        : (bool, None, None, False, gobject.PARAM_READABLE),
         _PROP_OWNER        : (bool, None, None, False, gobject.PARAM_READABLE),
+        _PROP_OBJID        : (str, None, None, None, gobject.PARAM_READABLE),
         _PROP_IP4_ADDRESS  : (str, None, None, None, gobject.PARAM_READWRITE)
     }
 
@@ -106,16 +106,16 @@ class Buddy(ExportedGObject):
         """Initialize the Buddy object
 
         bus -- connection to the D-Bus session bus
-        object_id -- the activity's unique identifier
+        object_id -- the buddy's unique identifier, either based on their
+            key-ID or JID
         kwargs -- used to initialize the object's properties
 
         constructs a DBUS "object path" from the _BUDDY_PATH
         and object_id
         """
-        if not object_id or not isinstance(object_id, int):
-            raise ValueError("object id must be a valid number")
 
-        self._object_path = _BUDDY_PATH + str(object_id)
+        self._object_id = object_id
+        self._object_path = dbus.ObjectPath(_BUDDY_PATH + object_id)
 
         self._activities = {}   # Activity ID -> Activity
         self._activity_sigids = {}
@@ -130,9 +130,6 @@ class Buddy(ExportedGObject):
         self._color = None
         self._ip4_address = None
 
-        if not kwargs.get(_PROP_KEY):
-            raise ValueError("key required")
-
         _ALLOWED_INIT_PROPS = [_PROP_NICK, _PROP_KEY, _PROP_ICON,
                                _PROP_CURACT, _PROP_COLOR, _PROP_IP4_ADDRESS]
         for (key, value) in kwargs.items():
@@ -158,7 +155,9 @@ class Buddy(ExportedGObject):
 
         pspec -- property specifier with a "name" attribute
         """
-        if pspec.name == _PROP_KEY:
+        if pspec.name == _PROP_OBJID:
+            return self._object_id
+        elif pspec.name == _PROP_KEY:
             return self._key
         elif pspec.name == _PROP_ICON:
             return self._icon
@@ -422,32 +421,40 @@ class Buddy(ExportedGObject):
         """
         changed = False
         changed_props = {}
-        if _PROP_NICK in properties.keys():
+        if _PROP_NICK in properties:
             nick = properties[_PROP_NICK]
             if nick != self._nick:
                 self._nick = nick
                 changed_props[_PROP_NICK] = nick
                 changed = True
-        if _PROP_COLOR in properties.keys():
+        if _PROP_COLOR in properties:
             color = properties[_PROP_COLOR]
             if color != self._color:
                 self._color = color
                 changed_props[_PROP_COLOR] = color
                 changed = True
-        if _PROP_CURACT in properties.keys():
+        if _PROP_CURACT in properties:
             curact = properties[_PROP_CURACT]
             if curact != self._current_activity:
                 self._current_activity = curact
                 changed_props[_PROP_CURACT] = curact
                 changed = True
-        if _PROP_IP4_ADDRESS in properties.keys():
+        if _PROP_IP4_ADDRESS in properties:
             ip4addr = properties[_PROP_IP4_ADDRESS]
             if ip4addr != self._ip4_address:
                 self._ip4_address = ip4addr
                 changed_props[_PROP_IP4_ADDRESS] = ip4addr
                 changed = True
-
-        if not changed or not len(changed_props.keys()):
+        if _PROP_KEY in properties:
+            # don't allow key to be set more than once
+            if self._key is None:
+                key = properties[_PROP_KEY]
+                if key is not None:
+                    self._key = key
+                    changed_props[_PROP_KEY] = key
+                    changed = True
+
+        if not changed or not changed_props:
             return
 
         # Try emitting PropertyChanged before updating validity
@@ -558,13 +565,11 @@ class ShellOwner(GenericOwner):
     _SHELL_OWNER_INTERFACE = "org.laptop.Shell.Owner"
     _SHELL_PATH = "/org/laptop/Shell"
 
-    def __init__(self, ps, bus, object_id, test=False):
+    def __init__(self, ps, bus):
         """Initialize the ShellOwner instance
 
         ps -- presenceservice.PresenceService object
         bus -- a connection to the D-Bus session bus
-        object_id -- the activity's unique identifier
-        test -- ignored
 
         Retrieves initial property values from the profile
         module.  Loads the buddy icon from file as well.
@@ -584,8 +589,8 @@ class ShellOwner(GenericOwner):
         icon = f.read()
         f.close()
 
-        GenericOwner.__init__(self, ps, bus, object_id, key=key,
-                nick=nick, color=color, icon=icon, server=server,
+        GenericOwner.__init__(self, ps, bus, psutils.pubkey_to_keyid(key),
+                key=key, nick=nick, color=color, icon=icon, server=server,
                 key_hash=key_hash, registered=registered)
 
         # Ask to get notifications on Owner object property changes in the
diff --git a/services/presence/presenceservice.py b/services/presence/presenceservice.py
index bf058d3..4f84a3b 100644
--- a/services/presence/presenceservice.py
+++ b/services/presence/presenceservice.py
@@ -1,4 +1,5 @@
 # Copyright (C) 2007, Red Hat, Inc.
+# Copyright (C) 2007 Collabora Ltd. <http://www.collabora.co.uk/>
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -33,6 +34,7 @@ from sugar import util
 
 from buddy import Buddy, ShellOwner
 from activity import Activity
+from psutils import pubkey_to_keyid
 
 _PRESENCE_SERVICE = "org.laptop.Sugar.Presence"
 _PRESENCE_INTERFACE = "org.laptop.Sugar.Presence"
@@ -57,15 +59,17 @@ class PresenceService(ExportedGObject):
 
     def _create_owner(self):
         # Overridden by TestPresenceService
-        return ShellOwner(self, self._session_bus, self._get_next_object_id())
+        return ShellOwner(self, self._session_bus)
 
     def __init__(self):
         self._next_object_id = 0
         self._connected = False
 
-        self._buddies = {}      # key -> Buddy
+        self._buddies = {}              # identifier -> Buddy
+        self._buddies_by_pubkey = {}    # base64 public key -> Buddy
         self._handles_buddies = {}      # tp client -> (handle -> Buddy)
-        self._activities = {}   # activity id -> Activity
+
+        self._activities = {}           # activity id -> Activity
 
         self._session_bus = dbus.SessionBus()
         self._session_bus.add_signal_receiver(self._connection_disconnected_cb,
@@ -74,7 +78,10 @@ class PresenceService(ExportedGObject):
 
         # Create the Owner object
         self._owner = self._create_owner()
-        self._buddies[self._owner.props.key] = self._owner
+        key = self._owner.props.key
+        keyid = pubkey_to_keyid(key)
+        self._buddies['keyid/' + keyid] = self._owner
+        self._buddies_by_pubkey[key] = self._owner
 
         self._registry = ManagerRegistry()
         self._registry.LoadManagers()
@@ -133,31 +140,35 @@ class PresenceService(ExportedGObject):
         if self._connected != old_status:
             self.emit('connection-status', self._connected)
 
-    def _contact_online(self, tp, handle, props):
-        new_buddy = False
-        key = props["key"]
-        buddy = self._buddies.get(key)
-        if not buddy:
+    def get_buddy(self, objid):
+        buddy = self._buddies.get(objid)
+        if buddy is None:
+            _logger.debug('Creating new buddy at .../%s', objid)
             # we don't know yet this buddy
-            objid = self._get_next_object_id()
-            buddy = Buddy(self._session_bus, objid, key=key)
+            buddy = Buddy(self._session_bus, objid)
             buddy.connect("validity-changed", self._buddy_validity_changed_cb)
             buddy.connect("disappeared", self._buddy_disappeared_cb)
-            self._buddies[key] = buddy
+            self._buddies[objid] = buddy
+        return buddy
+
+    def _contact_online(self, tp, objid, handle, props):
+        _logger.debug('Handle %u, .../%s is now online', handle, objid)
+        buddy = self.get_buddy(objid)
 
         self._handles_buddies[tp][handle] = buddy
         # store the handle of the buddy for this CM
         buddy.add_telepathy_handle(tp, handle)
-
         buddy.set_properties(props)
 
     def _buddy_validity_changed_cb(self, buddy, valid):
         if valid:
             self.BuddyAppeared(buddy.object_path())
+            self._buddies_by_pubkey[buddy.props.key] = buddy
             _logger.debug("New Buddy: %s (%s)", buddy.props.nick,
                           buddy.props.color)
         else:
             self.BuddyDisappeared(buddy.object_path())
+            self._buddies_by_pubkey.pop(buddy.props.key, None)
             _logger.debug("Buddy left: %s (%s)", buddy.props.nick,
                           buddy.props.color)
 
@@ -166,16 +177,17 @@ class PresenceService(ExportedGObject):
             self.BuddyDisappeared(buddy.object_path())
             _logger.debug('Buddy left: %s (%s)', buddy.props.nick,
                           buddy.props.color)
-        self._buddies.pop(buddy.props.key)
+            self._buddies_by_pubkey.pop(buddy.props.key, None)
+        self._buddies.pop(buddy.props.objid, None)
 
     def _contact_offline(self, tp, handle):
         if not self._handles_buddies[tp].has_key(handle):
             return
 
         buddy = self._handles_buddies[tp].pop(handle)
-        key = buddy.props.key
-
         # the handle of the buddy for this CM is not valid anymore
+        # (this might trigger _buddy_disappeared_cb if they are not visible
+        # via any CM)
         buddy.remove_telepathy_handle(tp, handle)
 
     def _get_next_object_id(self):
@@ -326,8 +338,8 @@ class PresenceService(ExportedGObject):
                          in_signature="ay", out_signature="o",
                          byte_arrays=True)
     def GetBuddyByPublicKey(self, key):
-        if self._buddies.has_key(key):
-            buddy = self._buddies[key]
+        buddy = self._buddies_by_pubkey.get(key)
+        if buddy is not None:
             if buddy.props.valid:
                 return buddy.object_path()
         raise NotFoundError("The buddy was not found.")
diff --git a/services/presence/pstest.py b/services/presence/pstest.py
index 1900993..3054e48 100644
--- a/services/presence/pstest.py
+++ b/services/presence/pstest.py
@@ -26,6 +26,7 @@ from sugar import env, util
 
 from buddy import GenericOwner, _PROP_NICK, _PROP_CURACT, _PROP_COLOR
 from presenceservice import PresenceService
+from psutils import pubkey_to_keyid
 
 
 _logger = logging.getLogger('s-p-s.pstest')
@@ -37,7 +38,7 @@ class TestOwner(GenericOwner):
 
     __gtype_name__ = "TestOwner"
 
-    def __init__(self, ps, bus, object_id, test_num, randomize):
+    def __init__(self, ps, bus, test_num, randomize):
         self._cp = ConfigParser()
         self._section = "Info"
         self._test_activities = []
@@ -62,8 +63,9 @@ class TestOwner(GenericOwner):
         icon = _get_random_image()
 
         _logger.debug("pubkey is %s" % pubkey)
-        GenericOwner.__init__(self, ps, bus, object_id, key=pubkey, nick=nick,
-                color=color, icon=icon, registered=registered, key_hash=privkey_hash)
+        GenericOwner.__init__(self, ps, bus, pubkey_to_keyid(pubkey),
+                key=pubkey, nick=nick, color=color, icon=icon,
+                registered=registered, key_hash=privkey_hash)
 
         # Only do the random stuff if randomize is true
         if randomize:
@@ -169,7 +171,7 @@ class TestPresenceService(PresenceService):
         PresenceService.__init__(self)
 
     def _create_owner(self):
-        return TestOwner(self, self._session_bus, self._get_next_object_id(),
+        return TestOwner(self, self._session_bus,
                          self.__test_num, self.__randomize)
 
     def internal_get_activity(self, actid):
diff --git a/services/presence/server_plugin.py b/services/presence/server_plugin.py
index 26adba9..b020286 100644
--- a/services/presence/server_plugin.py
+++ b/services/presence/server_plugin.py
@@ -20,6 +20,7 @@
 import logging
 import os
 import sys
+from string import hexdigits
 try:
     # Python >= 2.5
     from hashlib import md5
@@ -42,6 +43,7 @@ from telepathy.constants import (HANDLE_TYPE_CONTACT,
     CONNECTION_STATUS_CONNECTING,
     CONNECTION_STATUS_REASON_AUTHENTICATION_FAILED,
     CONNECTION_STATUS_REASON_NONE_SPECIFIED,
+    CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES,
     PROPERTY_FLAG_WRITE)
 from sugar import util
 
@@ -105,8 +107,11 @@ class ServerPlugin(gobject.GObject):
         'contact-online':
             # Contact has come online and we've discovered all their buddy
             # properties.
-            # args: contact handle: int; dict {name: str => property: object}
-            (gobject.SIGNAL_RUN_FIRST, None, [object, object]),
+            # args:
+            #   contact identification (based on key ID or JID): str
+            #   contact handle: int or long
+            #   dict {name: str => property: object}
+            (gobject.SIGNAL_RUN_FIRST, None, [str, object, object]),
         'contact-offline':
             # Contact has gone offline.
             # args: contact handle
@@ -263,7 +268,7 @@ class ServerPlugin(gobject.GObject):
 
         account_info['server'] = self._owner.get_server()
 
-        khash = util.printable_hash(util._sha_data(self._owner.props.key))
+        khash = psutils.pubkey_to_keyid(self._owner.props.key)
         account_info['account'] = "%s@%s" % (khash, account_info['server'])
 
         account_info['password'] = self._owner.get_key_hash()
@@ -770,10 +775,13 @@ class ServerPlugin(gobject.GObject):
             return
 
         props['nick'] = aliases[0]
+
         jid = self._conn[CONN_INTERFACE].InspectHandles(HANDLE_TYPE_CONTACT,
                                                         [handle])[0]
         self._online_contacts[handle] = jid
-        self.emit("contact-online", handle, props)
+        objid = self.identify_contacts(None, [handle])[handle]
+
+        self.emit("contact-online", objid, handle, props)
 
         self._conn[CONN_INTERFACE_BUDDY_INFO].GetActivities(handle,
             reply_handler=lambda *args: self._contact_online_activities_cb(
@@ -841,7 +849,7 @@ class ServerPlugin(gobject.GObject):
                 handle not in self._subscribe_local_pending and
                 handle not in self._subscribe_remote_pending):
             # it's probably a channel-specific handle - can't create a Buddy
-            # object
+            # object for those yet
             return
 
         self._online_contacts[handle] = None
@@ -1063,3 +1071,93 @@ class ServerPlugin(gobject.GObject):
             if room == act_handle:
                 self.emit("activity-properties-changed", act_id, properties)
                 return
+
+    def _server_is_trusted(self, hostname):
+        """Return True if the server with the given hostname is trusted to
+        verify public-key ownership correctly, and only allows users to
+        register JIDs whose username part is either a public key fingerprint,
+        or of the wrong form to be a public key fingerprint (to allow for
+        ejabberd's admin at example.com address).
+
+        If we trust the server, we can skip verifying the key ourselves,
+        which leads to simplifications. In the current implementation we
+        never verify that people actually own the key they claim to, so
+        we will always give contacts on untrusted servers a JID- rather than
+        key-based identity.
+
+        For the moment we assume that the test server, olpc.collabora.co.uk,
+        does this verification.
+        """
+        return (hostname == 'olpc.collabora.co.uk')
+
+    def identify_contacts(self, tp_chan, handles):
+        """Work out the "best" unique identifier we can for the given handles,
+        in the context of the given channel (which may be None), using only
+        'fast' connection manager API (that does not involve network
+        round-trips).
+
+        For the XMPP server case, we proceed as follows:
+
+        * Find the owners of the given handles, if the channel has
+          channel-specific handles
+        * If the owner (globally-valid JID) is on a trusted server, return
+          'keyid/' plus the 'key fingerprint' (the user part of their JID,
+          currently implemented as the SHA-1 of the Base64 blob in
+          owner.key.pub)
+        * If the owner (globally-valid JID) cannot be found or is on an
+          untrusted server, return 'xmpp/' plus an escaped form of the JID
+
+        The idea is that we identify buddies by key-ID (i.e. by key, assuming
+        no collisions) if we can find it without making network round-trips,
+        but if that's not possible we just use their JIDs.
+
+        :Parameters:
+            `tp_chan` : telepathy.client.Channel or None
+                The channel in which the handles were found, or None if they
+                are known to be channel-specific handles
+            `handles` : iterable over (int or long)
+                The contacts' handles in that channel
+        :Returns:
+            A dict mapping the provided handles to the best available
+            unique identifier, which is a string that could be used as a
+            suffix to an object path
+        """
+        # we need to be able to index into handles, so force them to
+        # be a sequence
+        if not isinstance(handles, (tuple, list)):
+            handles = tuple(handles)
+
+        owners = handles
+
+        if tp_chan is not None and CHANNEL_INTERFACE_GROUP in tp_chan:
+
+            group = tp_chan[CHANNEL_INTERFACE_GROUP]
+            if group.GetFlags() & CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES:
+
+                owners = group.GetHandleOwners(handles)
+                for i, owner in enumerate(owners):
+                    if owner == 0:
+                        owners[i] = handles[i]
+
+        jids = self._conn[CONN_INTERFACE].InspectHandles(HANDLE_TYPE_CONTACT,
+                                                         owners)
+
+        ret = {}
+        for handle, jid in zip(handles, jids):
+            if '/' in jid:
+                # the contact is unidentifiable (in an anonymous MUC) - create
+                # a temporary identity for them, based on their room-JID
+                ret[handle] = 'xmpp/' + psutils.escape_identifier(jid)
+            else:
+                user, host = jid.split('@', 1)
+                if (self._server_is_trusted(host) and len(user) == 40 and
+                    user.strip(hexdigits) == ''):
+                    # they're on a trusted server and their username looks
+                    # like a key-ID
+                    ret[handle] = 'keyid/' + user.lower()
+                else:
+                    # untrusted server, or not the right format to be a
+                    # key-ID - identify the contact by their JID
+                    ret[handle] = 'xmpp/' + psutils.escape_identifier(jid)
+
+        return ret
-- 
1.5.2-rc3.GIT

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 266 bytes
Desc: Digital signature
Url : http://mailman.laptop.org/pipermail/sugar/attachments/20070528/994ed877/attachment-0001.pgp 


More information about the Sugar mailing list