gemato/openpgp: correctly handle duplicate keys vs unexpected keys

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 <robbat2@gentoo.org>
Closes: https://github.com/projg2/gemato/pull/32
Signed-off-by: Michał Górny <mgorny@gentoo.org>

1 files 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