From 3fab338ef4e1ae0efcfaee455ae04881aa013083 Mon Sep 17 00:00:00 2001 From: Bruno Wagner Date: Fri, 24 Jul 2015 16:24:13 -0300 Subject: [style] Fixed pep8 warnings Fixed pep8 warnings to prepare the keymanager for CI --- src/leap/keymanager/openpgp.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'src/leap/keymanager/openpgp.py') diff --git a/src/leap/keymanager/openpgp.py b/src/leap/keymanager/openpgp.py index 794a0ec..5d91dc9 100644 --- a/src/leap/keymanager/openpgp.py +++ b/src/leap/keymanager/openpgp.py @@ -114,8 +114,11 @@ class TempGPGWrapper(object): publkeys = filter( lambda pubkey: pubkey.key_id not in privids, publkeys) - listkeys = lambda: self._gpg.list_keys() - listsecretkeys = lambda: self._gpg.list_keys(secret=True) + def listkeys(): + return self._gpg.list_keys() + + def listsecretkeys(): + return self._gpg.list_keys(secret=True) self._gpg = GPG(binary=self._gpgbinary, homedir=tempfile.mkdtemp()) -- cgit v1.2.3 From d12f883eed4a8205440e8a4422605be1a1cfe914 Mon Sep 17 00:00:00 2001 From: Bruno Wagner Date: Mon, 3 Aug 2015 17:37:09 -0300 Subject: [style] Re-added lambdas to openpgp on keymanager --- src/leap/keymanager/openpgp.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'src/leap/keymanager/openpgp.py') diff --git a/src/leap/keymanager/openpgp.py b/src/leap/keymanager/openpgp.py index 5d91dc9..bbdedb2 100644 --- a/src/leap/keymanager/openpgp.py +++ b/src/leap/keymanager/openpgp.py @@ -111,14 +111,10 @@ class TempGPGWrapper(object): # and we want to count the keys afterwards. privids = map(lambda privkey: privkey.key_id, privkeys) - publkeys = filter( - lambda pubkey: pubkey.key_id not in privids, publkeys) + publkeys = filter(lambda pubkey: pubkey.key_id not in privids, publkeys) - def listkeys(): - return self._gpg.list_keys() - - def listsecretkeys(): - return self._gpg.list_keys(secret=True) + listkeys = lambda: self._gpg.list_keys() + listsecretkeys = lambda: self._gpg.list_keys(secret=True) self._gpg = GPG(binary=self._gpgbinary, homedir=tempfile.mkdtemp()) -- cgit v1.2.3 From 711fce95bf8f65c0f5a4eaddb4023eda29bc16ad Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Mon, 17 Aug 2015 19:22:14 -0400 Subject: [style] pep8 fix --- src/leap/keymanager/openpgp.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/leap/keymanager/openpgp.py') diff --git a/src/leap/keymanager/openpgp.py b/src/leap/keymanager/openpgp.py index bbdedb2..794a0ec 100644 --- a/src/leap/keymanager/openpgp.py +++ b/src/leap/keymanager/openpgp.py @@ -111,7 +111,8 @@ class TempGPGWrapper(object): # and we want to count the keys afterwards. privids = map(lambda privkey: privkey.key_id, privkeys) - publkeys = filter(lambda pubkey: pubkey.key_id not in privids, publkeys) + publkeys = filter( + lambda pubkey: pubkey.key_id not in privids, publkeys) listkeys = lambda: self._gpg.list_keys() listsecretkeys = lambda: self._gpg.list_keys(secret=True) -- cgit v1.2.3 From 6d823d4e94e70d19a31894f973754ff05d557493 Mon Sep 17 00:00:00 2001 From: Ruben Pollan Date: Mon, 21 Sep 2015 23:15:33 +0200 Subject: [feat] more verbosity in get_key wrong address log --- src/leap/keymanager/openpgp.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/leap/keymanager/openpgp.py') diff --git a/src/leap/keymanager/openpgp.py b/src/leap/keymanager/openpgp.py index 794a0ec..b8b47d0 100644 --- a/src/leap/keymanager/openpgp.py +++ b/src/leap/keymanager/openpgp.py @@ -321,7 +321,9 @@ class OpenPGPScheme(EncryptionScheme): raise errors.KeyNotFound(address) leap_assert( address in doc.content[KEY_ADDRESS_KEY], - 'Wrong address in key data.') + 'Wrong address in key %s. Expected %s, found %s.' + % (doc.content[KEY_ID_KEY], address, + doc.content[KEY_ADDRESS_KEY])) key = build_key_from_dict(OpenPGPKey, doc.content) key._gpgbinary = self._gpgbinary return key -- cgit v1.2.3 From cce42536ac2c7588a9df2f6e012bb8991f8bbd75 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 10 Sep 2015 10:36:53 -0400 Subject: [refactor] refactor key parsing so that it can be tested without needing to instantiate the whole OpenPGPScheme object, that receives a soledad instance. --- src/leap/keymanager/openpgp.py | 130 +++++++++++++++++++---------------------- 1 file changed, 59 insertions(+), 71 deletions(-) (limited to 'src/leap/keymanager/openpgp.py') diff --git a/src/leap/keymanager/openpgp.py b/src/leap/keymanager/openpgp.py index b8b47d0..069a78e 100644 --- a/src/leap/keymanager/openpgp.py +++ b/src/leap/keymanager/openpgp.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # openpgp.py -# Copyright (C) 2013 LEAP +# Copyright (C) 2013-2015 LEAP # # 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 @@ -251,7 +251,7 @@ class OpenPGPScheme(EncryptionScheme): leap_assert(is_address(address), 'Not an user address: %s' % address) def _gen_key(_): - with self._temporary_gpgwrapper() as gpg: + with TempGPGWrapper(gpgbinary=self._gpgbinary) as gpg: # TODO: inspect result, or use decorator params = gpg.gen_key_input( key_type='RSA', @@ -348,37 +348,25 @@ class OpenPGPScheme(EncryptionScheme): # TODO: add more checks for correct key data. leap_assert(key_data is not None, 'Data does not represent a key.') - with self._temporary_gpgwrapper() as gpg: - # TODO: inspect result, or use decorator - gpg.import_keys(key_data) - privkey = None - pubkey = None + priv_info, privkey = process_ascii_key( + key_data, self._gpgbinary, secret=True) + pub_info, pubkey = process_ascii_key( + key_data, self._gpgbinary, secret=False) - try: - privkey = gpg.list_keys(secret=True).pop() - except IndexError: - pass - try: - pubkey = gpg.list_keys(secret=False).pop() # unitary keyring - except IndexError: - return (None, None) - - openpgp_privkey = None - if privkey is not None: - # build private key - openpgp_privkey = self._build_key_from_gpg( - privkey, - gpg.export_keys(privkey['fingerprint'], secret=True)) - leap_check(pubkey['fingerprint'] == privkey['fingerprint'], - 'Fingerprints for public and private key differ.', - errors.KeyFingerprintMismatch) - - # build public key - openpgp_pubkey = self._build_key_from_gpg( - pubkey, - gpg.export_keys(pubkey['fingerprint'], secret=False)) - - return (openpgp_pubkey, openpgp_privkey) + if not pubkey: + return (None, None) + + openpgp_privkey = None + if privkey: + # build private key + openpgp_privkey = self._build_key_from_gpg(priv_info, privkey) + leap_check(pub_info['fingerprint'] == priv_info['fingerprint'], + 'Fingerprints for public and private key differ.', + errors.KeyFingerprintMismatch) + # build public key + openpgp_pubkey = self._build_key_from_gpg(pub_info, pubkey) + + return (openpgp_pubkey, openpgp_privkey) def put_ascii_key(self, key_data, address): """ @@ -439,7 +427,7 @@ class OpenPGPScheme(EncryptionScheme): 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 self._temporary_gpgwrapper() as gpg: + 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() @@ -577,24 +565,7 @@ class OpenPGPScheme(EncryptionScheme): :return: An instance of the key. :rtype: OpenPGPKey """ - expiry_date = None - if key['expires']: - expiry_date = datetime.fromtimestamp(int(key['expires'])) - address = [] - for uid in key['uids']: - address.append(_parse_address(uid)) - - return OpenPGPKey( - address, - gpgbinary=self._gpgbinary, - key_id=key['keyid'], - fingerprint=key['fingerprint'], - key_data=key_data, - private=True if key['type'] == 'sec' else False, - length=int(key['length']), - expiry_date=expiry_date, - refreshed_at=datetime.now(), - ) + return build_gpg_key(key, key_data, self._gpgbinary) def delete_key(self, key): """ @@ -654,21 +625,6 @@ class OpenPGPScheme(EncryptionScheme): # Data encryption, decryption, signing and verifying # - def _temporary_gpgwrapper(self, keys=None): - """ - Return a gpg wrapper that implements the context manager protocol and - contains C{keys}. - - :param keys: keys to conform the keyring. - :type key: list(OpenPGPKey) - - :return: a TempGPGWrapper instance - :rtype: TempGPGWrapper - """ - # TODO do here checks on key_data - return TempGPGWrapper( - keys=keys, gpgbinary=self._gpgbinary) - @staticmethod def _assert_gpg_result_ok(result): """ @@ -713,7 +669,7 @@ class OpenPGPScheme(EncryptionScheme): leap_assert_type(sign, OpenPGPKey) leap_assert(sign.private is True) keys.append(sign) - with self._temporary_gpgwrapper(keys) as gpg: + with TempGPGWrapper(keys, self._gpgbinary) as gpg: result = gpg.encrypt( data, pubkey.fingerprint, default_key=sign.key_id if sign else None, @@ -755,7 +711,7 @@ class OpenPGPScheme(EncryptionScheme): leap_assert_type(verify, OpenPGPKey) leap_assert(verify.private is False) keys.append(verify) - with self._temporary_gpgwrapper(keys) as gpg: + with TempGPGWrapper(keys, self._gpgbinary) as gpg: try: result = gpg.decrypt( data, passphrase=passphrase, always_trust=True) @@ -783,7 +739,7 @@ class OpenPGPScheme(EncryptionScheme): :return: Whether C{data} was encrypted using this wrapper. :rtype: bool """ - with self._temporary_gpgwrapper() as gpg: + with TempGPGWrapper(gpgbinary=self._gpgbinary) as gpg: gpgutil = GPGUtilities(gpg) return gpgutil.is_encrypted_asym(data) @@ -814,7 +770,7 @@ class OpenPGPScheme(EncryptionScheme): # result.fingerprint - contains the fingerprint of the key used to # sign. - with self._temporary_gpgwrapper(privkey) as gpg: + with TempGPGWrapper(privkey, self._gpgbinary) as gpg: result = gpg.sign(data, default_key=privkey.key_id, digest_algo=digest_algo, clearsign=clearsign, detach=detach, binary=binary) @@ -849,7 +805,7 @@ class OpenPGPScheme(EncryptionScheme): """ leap_assert_type(pubkey, OpenPGPKey) leap_assert(pubkey.private is False) - with self._temporary_gpgwrapper(pubkey) as gpg: + with TempGPGWrapper(pubkey, self._gpgbinary) as gpg: result = None if detached_sig is None: result = gpg.verify(data) @@ -867,3 +823,35 @@ class OpenPGPScheme(EncryptionScheme): rfprint = result.fingerprint kfprint = gpgpubkey['fingerprint'] return valid and rfprint == kfprint + + +def process_ascii_key(key_data, gpgbinary, secret=False): + with TempGPGWrapper(gpgbinary=gpgbinary) as gpg: + try: + gpg.import_keys(key_data) + info = gpg.list_keys(secret=secret).pop() + key = gpg.export_keys(info['fingerprint'], secret=secret) + except IndexError: + info = {} + key = None + return info, key + + +def build_gpg_key(key_info, key_data, gpgbinary=None): + expiry_date = None + if key_info['expires']: + expiry_date = datetime.fromtimestamp(int(key_info['expires'])) + address = [] + for uid in key_info['uids']: + address.append(_parse_address(uid)) + + return OpenPGPKey( + address, + gpgbinary=gpgbinary, + key_id=key_info['keyid'], + fingerprint=key_info['fingerprint'], + key_data=key_data, + private=True if key_info['type'] == 'sec' else False, + length=int(key_info['length']), + expiry_date=expiry_date, + refreshed_at=datetime.now()) -- cgit v1.2.3 From 9a9c53eea49092e80737c84a2f850dd682c33ae3 Mon Sep 17 00:00:00 2001 From: Ruben Pollan Date: Tue, 29 Sep 2015 16:36:20 +0200 Subject: [feat] self-repair the keyring if keys get duplicated In some cases in the past keys got stored twice in different documents. Hopefully this issue is solved now, this tries to self-repair the keyring if encounters that. This is not really solving the problem, if it keeps happening we need to investigate the source. - Resolves: #7498 --- src/leap/keymanager/openpgp.py | 143 +++++++++++++++++++++++++++-------------- 1 file changed, 93 insertions(+), 50 deletions(-) (limited to 'src/leap/keymanager/openpgp.py') diff --git a/src/leap/keymanager/openpgp.py b/src/leap/keymanager/openpgp.py index 069a78e..d648137 100644 --- a/src/leap/keymanager/openpgp.py +++ b/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 # -- cgit v1.2.3