diff options
| -rw-r--r-- | keymanager/changes/bug-7498_multiple_keys | 1 | ||||
| -rw-r--r-- | keymanager/src/leap/keymanager/openpgp.py | 143 | ||||
| -rw-r--r-- | keymanager/src/leap/keymanager/tests/__init__.py | 7 | ||||
| -rw-r--r-- | keymanager/src/leap/keymanager/tests/test_openpgp.py | 104 |
4 files changed, 204 insertions, 51 deletions
diff --git a/keymanager/changes/bug-7498_multiple_keys b/keymanager/changes/bug-7498_multiple_keys new file mode 100644 index 00000000..90cf675f --- /dev/null +++ b/keymanager/changes/bug-7498_multiple_keys @@ -0,0 +1 @@ +- self-repair the keyring if keys get duplicated (Closes: #7485) diff --git a/keymanager/src/leap/keymanager/openpgp.py b/keymanager/src/leap/keymanager/openpgp.py index 069a78e3..d6481377 100644 --- a/keymanager/src/leap/keymanager/openpgp.py +++ b/keymanager/src/leap/keymanager/openpgp.py @@ -22,6 +22,7 @@ import os import re import shutil import tempfile +import traceback import io @@ -41,6 +42,8 @@ from leap.keymanager.keys import ( TYPE_ADDRESS_PRIVATE_INDEX, KEY_ADDRESS_KEY, KEY_ID_KEY, + KEY_FINGERPRINT_KEY, + KEY_REFRESHED_AT_KEY, KEYMANAGER_ACTIVE_TYPE, ) @@ -422,41 +425,42 @@ class OpenPGPScheme(EncryptionScheme): :rtype: Deferred """ def check_and_put(docs, key): - if len(docs) == 1: - doc = docs.pop() - oldkey = build_key_from_dict(OpenPGPKey, doc.content) - if key.fingerprint == oldkey.fingerprint: - # in case of an update of the key merge them with gnupg - with TempGPGWrapper(gpgbinary=self._gpgbinary) as gpg: - gpg.import_keys(oldkey.key_data) - gpg.import_keys(key.key_data) - gpgkey = gpg.list_keys(secret=key.private).pop() - mergedkey = self._build_key_from_gpg( - gpgkey, - gpg.export_keys(gpgkey['fingerprint'], - secret=key.private)) - mergedkey.validation = max( - [key.validation, oldkey.validation]) - mergedkey.last_audited_at = oldkey.last_audited_at - mergedkey.refreshed_at = key.refreshed_at - mergedkey.encr_used = key.encr_used or oldkey.encr_used - mergedkey.sign_used = key.sign_used or oldkey.sign_used - doc.set_json(mergedkey.get_json()) - d = self._soledad.put_doc(doc) - else: - logger.critical( - "Can't put a key whith the same key_id and different " - "fingerprint: %s, %s" - % (key.fingerprint, oldkey.fingerprint)) - d = defer.fail( - errors.KeyFingerprintMismatch(key.fingerprint)) + deferred_repair = defer.succeed(None) + if len(docs) == 0: + return self._soledad.create_doc_from_json(key.get_json()) elif len(docs) > 1: + deferred_repair = self._repair_key_docs(docs, key.key_id) + + doc = docs[0] + oldkey = build_key_from_dict(OpenPGPKey, doc.content) + if key.fingerprint != oldkey.fingerprint: logger.critical( - "There is more than one key with the same key_id %s" - % (key.key_id,)) - d = defer.fail(errors.KeyAttributesDiffer(key.key_id)) - else: - d = self._soledad.create_doc_from_json(key.get_json()) + "Can't put a key whith the same key_id and different " + "fingerprint: %s, %s" + % (key.fingerprint, oldkey.fingerprint)) + return defer.fail( + errors.KeyFingerprintMismatch(key.fingerprint)) + + # in case of an update of the key merge them with gnupg + with TempGPGWrapper(gpgbinary=self._gpgbinary) as gpg: + gpg.import_keys(oldkey.key_data) + gpg.import_keys(key.key_data) + gpgkey = gpg.list_keys(secret=key.private).pop() + mergedkey = self._build_key_from_gpg( + gpgkey, + gpg.export_keys(gpgkey['fingerprint'], + secret=key.private)) + mergedkey.validation = max( + [key.validation, oldkey.validation]) + mergedkey.last_audited_at = oldkey.last_audited_at + mergedkey.refreshed_at = key.refreshed_at + mergedkey.encr_used = key.encr_used or oldkey.encr_used + mergedkey.sign_used = key.sign_used or oldkey.sign_used + doc.set_json(mergedkey.get_json()) + deferred_put = self._soledad.put_doc(doc) + + d = defer.gatherResults([deferred_put, deferred_repair]) + d.addCallback(lambda res: res[0]) return d d = self._soledad.get_from_index( @@ -533,14 +537,21 @@ class OpenPGPScheme(EncryptionScheme): self.KEY_TYPE, key_id, '1' if private else '0') - d.addCallback(get_doc, key_id) + d.addCallback(get_doc, key_id, activedoc) return d - def get_doc(doclist, key_id): - leap_assert( - len(doclist) is 1, - 'There is %d keys for id %s!' % (len(doclist), key_id)) - return doclist.pop() + def get_doc(doclist, key_id, activedoc): + if len(doclist) == 0: + logger.warning('There is no key for id %s! Self-repairing it.' + % (key_id)) + d = self._soledad.delete_doc(activedoc) + d.addCallback(lambda _: None) + return d + elif len(doclist) > 1: + d = self._repair_key_docs(doclist, key_id) + d.addCallback(lambda _: doclist[0]) + return d + return doclist[0] d = self._soledad.get_from_index( TYPE_ADDRESS_PRIVATE_INDEX, @@ -598,18 +609,20 @@ class OpenPGPScheme(EncryptionScheme): def delete_key(docs): if len(docs) == 0: raise errors.KeyNotFound(key) - if len(docs) > 1: - logger.critical("There is more than one key for key_id %s" - % key.key_id) - - doc = None - for d in docs: - if d.content['fingerprint'] == key.fingerprint: - doc = d - break - if doc is None: + elif len(docs) > 1: + logger.warning("There is more than one key for key_id %s" + % key.key_id) + + has_deleted = False + deferreds = [] + for doc in docs: + if doc.content['fingerprint'] == key.fingerprint: + d = self._soledad.delete_doc(doc) + deferreds.append(d) + has_deleted = True + if not has_deleted: raise errors.KeyNotFound(key) - return self._soledad.delete_doc(doc) + return defer.gatherResults(deferreds) d = self._soledad.get_from_index( TYPE_ID_PRIVATE_INDEX, @@ -621,6 +634,36 @@ class OpenPGPScheme(EncryptionScheme): d.addCallback(delete_key) return d + def _repair_key_docs(self, doclist, key_id): + """ + If there is more than one key for a key id try to self-repair it + + :return: a Deferred that will be fired once all the deletions are + completed + :rtype: Deferred + """ + logger.error("BUG ---------------------------------------------------") + logger.error("There is more than one key with the same key_id %s:" + % (key_id,)) + + def log_key_doc(doc): + logger.error("\t%s: %s" % (doc.content[KEY_ADDRESS_KEY], + doc.content[KEY_FINGERPRINT_KEY])) + + doclist.sort(key=lambda doc: doc.content[KEY_REFRESHED_AT_KEY], + reverse=True) + log_key_doc(doclist[0]) + deferreds = [] + for doc in doclist[1:]: + log_key_doc(doc) + d = self._soledad.delete_doc(doc) + deferreds.append(d) + + logger.error("") + logger.error(traceback.extract_stack()) + logger.error("BUG (please report above info) ------------------------") + return defer.gatherResults(deferreds, consumeErrors=True) + # # Data encryption, decryption, signing and verifying # diff --git a/keymanager/src/leap/keymanager/tests/__init__.py b/keymanager/src/leap/keymanager/tests/__init__.py index 9b95e1ac..cd612c43 100644 --- a/keymanager/src/leap/keymanager/tests/__init__.py +++ b/keymanager/src/leap/keymanager/tests/__init__.py @@ -66,9 +66,15 @@ class KeyManagerWithSoledadTestCase(unittest.TestCase, BaseLeapTest): for private in [True, False]: d = km.get_all_keys(private=private) d.addCallback(delete_keys) + d.addCallback(check_deleted, private) deferreds.append(d) return gatherResults(deferreds) + def check_deleted(_, private): + d = km.get_all_keys(private=private) + d.addCallback(lambda keys: self.assertEqual(keys, [])) + return d + # wait for the indexes to be ready for the tear down d = km._wrapper_map[OpenPGPKey].deferred_indexes d.addCallback(get_and_delete_keys) @@ -91,6 +97,7 @@ class KeyManagerWithSoledadTestCase(unittest.TestCase, BaseLeapTest): # key 24D18DDF: public key "Leap Test Key <leap@leap.se>" +KEY_ID = "2F455E2824D18DDF" KEY_FINGERPRINT = "E36E738D69173C13D709E44F2F455E2824D18DDF" PUBLIC_KEY = """ -----BEGIN PGP PUBLIC KEY BLOCK----- diff --git a/keymanager/src/leap/keymanager/tests/test_openpgp.py b/keymanager/src/leap/keymanager/tests/test_openpgp.py index 5f85c74b..bae83db7 100644 --- a/keymanager/src/leap/keymanager/tests/test_openpgp.py +++ b/keymanager/src/leap/keymanager/tests/test_openpgp.py @@ -21,12 +21,15 @@ Tests for the OpenPGP support on Key Manager. """ -from twisted.internet.defer import inlineCallbacks +from datetime import datetime +from mock import Mock +from twisted.internet.defer import inlineCallbacks, gatherResults, succeed from leap.keymanager import ( KeyNotFound, openpgp, ) +from leap.keymanager.keys import TYPE_ID_PRIVATE_INDEX from leap.keymanager.openpgp import OpenPGPKey from leap.keymanager.tests import ( KeyManagerWithSoledadTestCase, @@ -34,6 +37,7 @@ from leap.keymanager.tests import ( ADDRESS_2, KEY_FINGERPRINT, PUBLIC_KEY, + KEY_ID, PUBLIC_KEY_2, PRIVATE_KEY, PRIVATE_KEY_2, @@ -247,6 +251,104 @@ class OpenPGPCryptoTestCase(KeyManagerWithSoledadTestCase): validsign = pgp.verify(data, pubkey, detached_sig=signature) self.assertTrue(validsign) + @inlineCallbacks + def test_self_repair_three_keys(self): + pgp = openpgp.OpenPGPScheme( + self._soledad, gpgbinary=self.gpg_binary_path) + yield pgp.put_ascii_key(PUBLIC_KEY, ADDRESS) + + get_from_index = self._soledad.get_from_index + delete_doc = self._soledad.delete_doc + + def my_get_from_index(*args): + if (args[0] == TYPE_ID_PRIVATE_INDEX and + args[2] == KEY_ID): + k1 = OpenPGPKey(ADDRESS, key_id="1", + refreshed_at=datetime(2005, 1, 1)) + k2 = OpenPGPKey(ADDRESS, key_id="2", + refreshed_at=datetime(2007, 1, 1)) + k3 = OpenPGPKey(ADDRESS, key_id="3", + refreshed_at=datetime(2001, 1, 1)) + d1 = self._soledad.create_doc_from_json(k1.get_json()) + d2 = self._soledad.create_doc_from_json(k2.get_json()) + d3 = self._soledad.create_doc_from_json(k3.get_json()) + return gatherResults([d1, d2, d3]) + return get_from_index(*args) + + self._soledad.get_from_index = my_get_from_index + self._soledad.delete_doc = Mock(return_value=succeed(None)) + + key = yield pgp.get_key(ADDRESS, private=False) + + try: + self.assertEqual(key.key_id, "2") + self.assertEqual(self._soledad.delete_doc.call_count, 2) + finally: + self._soledad.get_from_index = get_from_index + self._soledad.delete_doc = delete_doc + + @inlineCallbacks + def test_self_repair_no_keys(self): + pgp = openpgp.OpenPGPScheme( + self._soledad, gpgbinary=self.gpg_binary_path) + yield pgp.put_ascii_key(PUBLIC_KEY, ADDRESS) + + get_from_index = self._soledad.get_from_index + delete_doc = self._soledad.delete_doc + + def my_get_from_index(*args): + if (args[0] == TYPE_ID_PRIVATE_INDEX and + args[2] == KEY_ID): + return succeed([]) + return get_from_index(*args) + + self._soledad.get_from_index = my_get_from_index + self._soledad.delete_doc = Mock(return_value=succeed(None)) + + try: + yield self.assertFailure(pgp.get_key(ADDRESS, private=False), + KeyNotFound) + self.assertEqual(self._soledad.delete_doc.call_count, 1) + finally: + self._soledad.get_from_index = get_from_index + self._soledad.delete_doc = delete_doc + + @inlineCallbacks + def test_self_repair_put_keys(self): + pgp = openpgp.OpenPGPScheme( + self._soledad, gpgbinary=self.gpg_binary_path) + + get_from_index = self._soledad.get_from_index + delete_doc = self._soledad.delete_doc + + def my_get_from_index(*args): + if (args[0] == TYPE_ID_PRIVATE_INDEX and + args[2] == KEY_ID): + k1 = OpenPGPKey(ADDRESS, key_id="1", + fingerprint=KEY_FINGERPRINT, + refreshed_at=datetime(2005, 1, 1)) + k2 = OpenPGPKey(ADDRESS, key_id="2", + fingerprint=KEY_FINGERPRINT, + refreshed_at=datetime(2007, 1, 1)) + k3 = OpenPGPKey(ADDRESS, key_id="3", + fingerprint=KEY_FINGERPRINT, + refreshed_at=datetime(2001, 1, 1)) + d1 = self._soledad.create_doc_from_json(k1.get_json()) + d2 = self._soledad.create_doc_from_json(k2.get_json()) + d3 = self._soledad.create_doc_from_json(k3.get_json()) + return gatherResults([d1, d2, d3]) + return get_from_index(*args) + + self._soledad.get_from_index = my_get_from_index + self._soledad.delete_doc = Mock(return_value=succeed(None)) + + try: + yield pgp.put_ascii_key(PUBLIC_KEY, ADDRESS) + self.assertEqual(self._soledad.delete_doc.call_count, 2) + finally: + self._soledad.get_from_index = get_from_index + self._soledad.delete_doc = delete_doc + def _assert_key_not_found(self, pgp, address, private=False): d = pgp.get_key(address, private=private) return self.assertFailure(d, KeyNotFound) |