From d16b25da0962156601615587a612b57b918b0dc6 Mon Sep 17 00:00:00 2001 From: Ruben Pollan Date: Tue, 24 Oct 2017 11:50:30 +0200 Subject: [bug] make nicknym return key data There was a mismatch on the return values of Nicknym class and the expectations. As most of the code was expecting an OpenPGPKey, but it was returning the json as it came from the nickserver. Now is just returning the key data. --- src/leap/bitmask/core/mail_services.py | 5 ++-- src/leap/bitmask/keymanager/__init__.py | 35 ++++++++------------------ src/leap/bitmask/keymanager/nicknym.py | 23 ++++++++--------- src/leap/bitmask/keymanager/refresher.py | 17 +++++++++---- tests/integration/keymanager/test_refresher.py | 12 ++++----- 5 files changed, 41 insertions(+), 51 deletions(-) diff --git a/src/leap/bitmask/core/mail_services.py b/src/leap/bitmask/core/mail_services.py index 5337b313..31c27364 100644 --- a/src/leap/bitmask/core/mail_services.py +++ b/src/leap/bitmask/core/mail_services.py @@ -298,10 +298,9 @@ class KeymanagerContainer(Container): try: remote = yield keymanager._nicknym.fetch_key_with_address(userid) except Exception: - remote = {} + remote = "" - if (keymanager.OPENPGP_KEY not in remote or - key.key_data != remote[KeyManager.OPENPGP_KEY]): + if key.key_data != remote: yield keymanager.send_key() def _set_status(self, address, status, error=None, keys=None): diff --git a/src/leap/bitmask/keymanager/__init__.py b/src/leap/bitmask/keymanager/__init__.py index bc0c2185..45b7e582 100644 --- a/src/leap/bitmask/keymanager/__init__.py +++ b/src/leap/bitmask/keymanager/__init__.py @@ -18,16 +18,12 @@ Key Manager is a Nicknym agent for LEAP client. """ import fileinput -import json -import sys import tempfile from urlparse import urlparse from twisted.logger import Logger from twisted.internet import defer, task, reactor -from twisted.web import client -from twisted.web._responses import NOT_FOUND from leap.common import ca_bundle from leap.common.http import HTTPClient @@ -49,9 +45,6 @@ class KeyManager(object): log = Logger() - OPENPGP_KEY = 'openpgp' - PUBKEY_KEY = "user[public_key]" - def __init__(self, address, nickserver_uri, soledad, token=None, ca_cert_path=None, api_uri=None, api_version=None, uid=None, gpgbinary=None, combined_ca_bundle=None): @@ -203,23 +196,17 @@ class KeyManager(object): :rtype: Deferred """ - server_keys = yield self._nicknym.fetch_key_with_address(address) - - # insert keys in local database - if self.OPENPGP_KEY in server_keys: - # nicknym server is authoritative for its own domain, - # for other domains the key might come from key servers. - validation_level = ValidationLevels.Weak_Chain - _, domain = _split_email(address) - if (domain == _get_domain(self._nickserver_uri)): - validation_level = ValidationLevels.Provider_Trust - - yield self.put_raw_key( - server_keys[self.OPENPGP_KEY], - address=address, - validation=validation_level) - else: - raise KeyNotFound("No openpgp key found") + raw_key = yield self._nicknym.fetch_key_with_address(address) + + # nicknym server is authoritative for its own domain, + # for other domains the key might come from key servers. + validation_level = ValidationLevels.Weak_Chain + _, domain = _split_email(address) + if (domain == _get_domain(self._nickserver_uri)): + validation_level = ValidationLevels.Provider_Trust + + yield self.put_raw_key( + raw_key, address=address, validation=validation_level) def get_key(self, address, private=False, fetch_remote=True): """ diff --git a/src/leap/bitmask/keymanager/nicknym.py b/src/leap/bitmask/keymanager/nicknym.py index cbc4e25d..5342a7e4 100644 --- a/src/leap/bitmask/keymanager/nicknym.py +++ b/src/leap/bitmask/keymanager/nicknym.py @@ -38,6 +38,7 @@ class Nicknym(object): log = Logger() + OPENPGP_KEY = 'openpgp' PUBKEY_KEY = "user[public_key]" def __init__(self, nickserver_uri, ca_cert_path, token): @@ -99,12 +100,13 @@ class Nicknym(object): :param uri: The URI of the request. :type uri: str - :return: A deferred that will be fired with GET content as json (dict) + :return: A deferred that will be fired with GET content as raw key str :rtype: Deferred """ try: content = yield self._fetch_and_handle_404_from_nicknym(uri) json_content = json.loads(content) + key = json_content[self.OPENPGP_KEY] except KeyNotFound: raise except IOError as e: @@ -114,15 +116,12 @@ class Nicknym(object): except ValueError as v: self.log.warn('Invalid JSON data from key: %s' % (uri,)) raise KeyNotFound(v.message + ' - ' + uri), None, sys.exc_info()[2] + except KeyError: + raise KeyNotFound("No openpgp key found") except Exception as e: self.log.warn('Error retrieving key: %r' % (e,)) raise KeyNotFound(e.message), None, sys.exc_info()[2] - # Responses are now text/plain, although it's json anyway, but - # this will fail when it shouldn't - # leap_assert( - # res.headers['content-type'].startswith('application/json'), - # 'Content-type is not JSON.') - defer.returnValue(json_content) + defer.returnValue(key) def _fetch_and_handle_404_from_nicknym(self, uri): """ @@ -166,9 +165,8 @@ class Nicknym(object): :param address: The address bound to the keys. :type address: str - :return: A Deferred which fires when the key is in the storage, - or which fails with KeyNotFound if the key was not found on - nickserver. + :return: A Deferred with the raw key, or which fails with KeyNotFound + if the key was not found on nickserver. :rtype: Deferred """ @@ -183,9 +181,8 @@ class Nicknym(object): :param fingerprint: The fingerprint bound to the keys. :type fingerprint: str - :return: A Deferred which fires when the key is in the storage, - or which fails with KeyNotFound if the key was not found on - nickserver. + :return: A Deferred with the raw key, or which fails with KeyNotFound + if the key was not found on nickserver. :rtype: Deferred """ diff --git a/src/leap/bitmask/keymanager/refresher.py b/src/leap/bitmask/keymanager/refresher.py index d89a7508..9ccc81e1 100644 --- a/src/leap/bitmask/keymanager/refresher.py +++ b/src/leap/bitmask/keymanager/refresher.py @@ -106,17 +106,24 @@ class RandomRefreshPublicKey(object): if old_key is None: defer.returnValue(None) - old_updated_key = yield self._keymanger._nicknym.\ + updated_key_data = yield self._keymanger._nicknym.\ fetch_key_with_fingerprint(old_key.fingerprint) + updated_key, _ = self._openpgp.parse_key(updated_key_data, + old_key.address) - if old_updated_key.fingerprint != old_key.fingerprint: + if updated_key.fingerprint != old_key.fingerprint: self.log.error( ERROR_UNEQUAL_FINGERPRINTS % ( - old_key.fingerprint, old_updated_key.fingerprint)) + old_key.fingerprint, updated_key.fingerprint)) defer.returnValue(None) - yield self._maybe_unactivate_key(old_updated_key) - yield self._openpgp.put_key(old_updated_key) + updated_key.validation = old_key.validation + updated_key.last_audited_at = old_key.last_audited_at + updated_key.encr_used = old_key.encr_used + updated_key.sign_used = old_key.sign_used + + yield self._maybe_unactivate_key(updated_key) + yield self._openpgp.put_key(updated_key) # No new fetch by address needed, bc that will happen before sending an # email could be discussed since fetching before sending an email diff --git a/tests/integration/keymanager/test_refresher.py b/tests/integration/keymanager/test_refresher.py index 24ad4b05..47a27b4b 100644 --- a/tests/integration/keymanager/test_refresher.py +++ b/tests/integration/keymanager/test_refresher.py @@ -32,7 +32,7 @@ from leap.bitmask.keymanager.refresher import RandomRefreshPublicKey, \ ERROR_UNEQUAL_FINGERPRINTS from leap.bitmask.keymanager.testing import KeyManagerWithSoledadTestCase -from common import KEY_FINGERPRINT +from common import KEY_FINGERPRINT, PUBLIC_KEY_2, KEY_FINGERPRINT_2 ANOTHER_FP = 'ANOTHERFINGERPRINT' @@ -76,13 +76,13 @@ class RandomRefreshPublicKeyTestCase(KeyManagerWithSoledadTestCase): fingerprint=KEY_FINGERPRINT))) km._nicknym.fetch_key_with_fingerprint = \ - Mock(return_value=defer.succeed(OpenPGPKey( - fingerprint=ANOTHER_FP))) + Mock(return_value=defer.succeed(PUBLIC_KEY_2)) yield rf.maybe_refresh_key() - mock_logger_error.assert_called_with(ERROR_UNEQUAL_FINGERPRINTS % - (KEY_FINGERPRINT, ANOTHER_FP)) + mock_logger_error.assert_called_with( + ERROR_UNEQUAL_FINGERPRINTS % + (KEY_FINGERPRINT, KEY_FINGERPRINT_2)) @defer.inlineCallbacks def test_put_new_key_in_local_storage(self): @@ -95,7 +95,7 @@ class RandomRefreshPublicKeyTestCase(KeyManagerWithSoledadTestCase): OpenPGPKey(fingerprint=KEY_FINGERPRINT))) km._nicknym.fetch_key_with_fingerprint = Mock( - return_value=defer.succeed(OpenPGPKey(fingerprint=ANOTHER_FP))) + return_value=defer.succeed(PUBLIC_KEY_2)) yield rf.maybe_refresh_key() -- cgit v1.2.3