From fa0b0339c6e4dfde64fb6a6ce1cc68c3d071a889 Mon Sep 17 00:00:00 2001 From: "Robin H. Johnson" Date: Fri, 28 Apr 2023 17:06:00 -0700 Subject: gemato/openpgp: correctly handle duplicate keys vs unexpected keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old code path had a subtle behavior bug: if an expected key appeared twice in data from a WKD URL, it was then removed entirely. This happened at one point due to a GPG behavior: when using --export, if --keyring is passed twice, with different keyrings, but those keyrings both contain the key being exported (possibly with different signatures), then the export output will have duplicates of PGP packets present in both keyrings (e.g. UID). To avoid this, defer the removal of unexpected keys until the main import is completed. Signed-off-by: Robin H. Johnson Closes: https://github.com/projg2/gemato/pull/32 Signed-off-by: Michał Górny --- gemato/openpgp.py | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/gemato/openpgp.py b/gemato/openpgp.py index 4ede082..b1b8eb6 100644 --- a/gemato/openpgp.py +++ b/gemato/openpgp.py @@ -561,7 +561,7 @@ debug-level guru f'key {key}') return False addrs.update(uids) - keys = set(keys) + expected_keys = frozenset(keys) data = b'' proxies = {} @@ -588,23 +588,37 @@ debug-level guru data, raise_on_error=OpenPGPKeyRefreshError) - # we need to explicitly ensure all keys were fetched + imported_keys = set() for line in out.splitlines(): if line.startswith(b'[GNUPG:] IMPORT_OK'): fpr = line.split(b' ')[3].decode('ASCII') logging.debug( f'refresh_keys_wkd(): import successful for key: {fpr}') - if fpr in keys: - keys.remove(fpr) - else: - # we need to delete unexpected keys - exitst, out, err = self._spawn_gpg( - [GNUPG, '--batch', '--delete-keys', fpr], - raise_on_error=OpenPGPKeyRefreshError) - if keys: + imported_keys.add(fpr) + + # Need to explicitly ensure all keys were fetched + # However: + # - any key MAY appear 0 or more times. + # - expected keys SHOULD be present. + # - unexpected keys MAY also be present. + unexpected_keys = imported_keys.difference(expected_keys) + if unexpected_keys: + # we need to delete unexpected keys + logging.debug( + f'refresh_keys_wkd(): got unexpected key, will remove: ' + f'{unexpected_keys}') + # 128x 40-byte fingerprints = 5KiB commandline max + # If this contains a lot of keys, it should just blow up, but that + # saves complexity. + exitst, out, err = self._spawn_gpg( + [GNUPG, '--batch', '--delete-keys'] + list(unexpected_keys), + raise_on_error=OpenPGPKeyRefreshError) + + not_updated_keys = expected_keys.difference(imported_keys) + if not_updated_keys: logging.debug( f'refresh_keys_wkd(): failing due to non-updated keys: ' - f'{keys}') + f'{not_updated_keys}') return False return True -- cgit v1.2.3