[sugar] [PATCH] services/presence/: identify Buddies by "key ID" (pubkey hash), not whole key.
Morgan Collett
morgan at collabora.co.uk
Wed May 30 11:09:13 EDT 2007
FWIW, looks good to me.
Morgan
Simon McVittie wrote:
> 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
More information about the Sugar
mailing list