Check validity of key signature

Check if a new fetched key was signed by a old key with the same address.

Please do not merge before: https://github.com/isislovecruft/python-gnupg/pull/150

- Resolves #8112

6 files changed, 130 insertions, 9 deletions

diff --git a/src/leap/bitmask/keymanager/keys.py b/src/leap/bitmask/keymanager/keys.py
index fd454480..622c1c68 100644
--- a/src/leap/bitmask/keymanager/keys.py
+++ b/src/leap/bitmask/keymanager/keys.py
@@ -160,6 +160,28 @@ class OpenPGPKey(object):
 
         return []
 
+    def is_signed_by(self, other_key):
+        """
+        Checks if current key was signed by another key. Rather than just
+        relying on the fingerprint being there, we use gpg's --check-sigs with
+        both keys being present in the keychain to check the signature
+        validity. By doing so, relying on the long key id instead of the
+        fingerprint is fine.
+
+        :param other_key: the other key.
+        :return: True if valid signature could be found.
+        :rtype: bool
+        """
+        keys = [self, other_key]
+        with TempGPGWrapper(keys=keys, gpgbinary=self._gpgbinary) as gpg:
+            certs = gpg.check_sigs(str(self.fingerprint)).certs
+            for uid, cur_certs in certs.iteritems():
+                if (parse_address(uid) in other_key.uids and
+                        other_key.fingerprint[-16:] in cur_certs):
+                    return True
+
+            return False
+
     def merge(self, newkey):
         if newkey.fingerprint != self.fingerprint:
             logger.critical(
diff --git a/src/leap/bitmask/keymanager/validation.py b/src/leap/bitmask/keymanager/validation.py
index 16a897e9..61adc0e1 100644
--- a/src/leap/bitmask/keymanager/validation.py
+++ b/src/leap/bitmask/keymanager/validation.py
@@ -121,9 +121,4 @@ def can_upgrade(new_key, old_key):
         return True
 
     # New key signed by the old key
-    # XXX: signatures are using key-ids instead of fingerprints
-    key_id = old_key.fingerprint[-16:]
-    if key_id in new_key.signatures:
-        return True
-
-    return False
+    return new_key.is_signed_by(old_key)
diff --git a/tests/integration/keymanager/__init__.py b/tests/integration/keymanager/__init__.py
new file mode 100644
index 00000000..e69de29b
--- /dev/null
+++ b/tests/integration/keymanager/__init__.py
diff --git a/tests/integration/keymanager/common.py b/tests/integration/keymanager/common.py
index 3371ceb8..f120223a 100644
--- a/tests/integration/keymanager/common.py
+++ b/tests/integration/keymanager/common.py
@@ -254,3 +254,77 @@ THx7N776fcYHGumbqUMYrxrcZSbNveE6SaK8fphRam1dewM0
 =a5gs
 -----END PGP PRIVATE KEY BLOCK-----
 """
+
+OLD_AND_NEW_KEY_ADDRESS = "test@key.de"
+OLD_PUB_KEY_FINGERPRINT = "580693B3E70AA98FEFA71F957DE22E18225F560F"
+OLD_PUB_KEY = """
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mQENBFdZNX0BCADtJykk30qD6fBrr43hLUPHb6GLX8L5Xoh5MKs6w/x/6KznrZ9l
+20tzEeuci5/PHsmJJd3mxrsc4bYjv5klLDR4dm3T1LLxK32mvGld74I+74lUPBO6
+rFiJb8DA1vV8ax5wXIsaxHJQ9/4Ovku+ZiQT8PnwIEr58+9Bz67tbHG2RAaY+iWS
+Lcag7CeoqUMkky9dqJr1f8Xo9wsgOXVUD4bm5p9xUqZaXR4TGYZsie9XJMg1tlyW
+zALZlLyXX7xTNP87EUtJAu9T9d+JmXfj82LKWbgDE8eKJoSFa26JyeWluivkMs5x
+mNs6AELNbvEqx6rvXw6FB8n6MZqdB3P5SMYjABEBAAG0FnRlc3Qga2V5IDx0ZXN0
+QGtleS5kZT6JATkEEwEIACMFAldZNX0CGwMHCwkIBwMCAQYVCAIJCgsEFgIDAQIe
+AQIXgAAKCRB94i4YIl9WD4jWB/9XHD/1HUHqzNTLcKUlNN698pdxrDoJEEdgMv5l
+aCx4VBHlS/VuvlEEJPuZHq2jl1Z83rmqrSUXhxaJA+ywjISNHHC1miwouAk14S1k
+kIqoMhg4oswxGndsf7lhj+Rr+P9S4AG6xrhYvENDbXld8Jmhftk/y17mFKsfBXh5
+9bhcEQcIWKwNL7GcQZk/xCOsBTXNVduhYP8b2CYy9XXDJykcQgDHCkoidxEpBx81
+2/3gdEmd3RZqnnwewNhLmttsGvWo5qSCyPCOs1AHBLt7amB0QpwxTRW+YDj4Jks6
+Za4FrF1FXEHVPWnTW2m2uBzG5ooMIRq5n+bMSbOnO93TwTOXuQENBFdZNX0BCADW
+JaeWoFfa0YY3btEwapPU7gVkSPGN8Fn1blzkkzAW4//ClKRGB15znBho49/alPf3
+dAYiiXd+fcnjTzprDVqKXZSh16YiYgUhddfBBWR+iyjT9usq5xuDhWADmVXbsgGY
+uTIlCFCaLJvetpYw6QztoBUvwWg71Q3eDX9c2lHG5aiZN3NFUK3N3920YAJUJDeB
+RNleTGHbS0Vq7ZPvo8nAL6U/10jP4zg0JVSH6bINOF83aHjbVrOraN1Kj5cKbdj7
+YspjrJCeHUA4B9BDcYdZZ0fcdnq+WO/n3akwY12Ob17yuQQqA/iz0MrsuIJaYedA
+kjwCPFkebD1AIbSYru0jABEBAAGJAR8EGAEIAAkFAldZNX0CGwwACgkQfeIuGCJf
+Vg+TTAf/Qj/slw6sfaN92np6MjGmp0OrMqQbWMI0CgQR8hqFUihyvZJUcSal4+KU
+6kca6raa0jdUl0Scp27T2m0HvuGBjOoHyY1Me5PluNnW+bCMlyasb0Np2Hn/61UB
+09W99h+h+9DKg3F0VWgmAz4HhEObMoCcbvEKKhSAsYjGK98T9p9QvBw042CJ/nT8
+ptemjS6pcWC4Gzsz4LuG/fzRsbdfcu/4ssS754vunLvghoLEDF8wYAiaa2ueHmqv
+kdHAlnG5kcUa9UNdpTf5m99ureLcJhlZVWfYAEOUHqbt0CAxPnTPgr3S6Aw7IU+F
+vE1b/YBvraAI0OqDyEHA0rYXB7XGFQ==
+=i5Hb
+-----END PGP PUBLIC KEY BLOCK-----
+"""
+
+# This key is signed by 580693B3E70AA98FEFA71F957DE22E18225F560F
+NEW_PUB_KEY_FINGERPRINT = "6042FD09AE96D41624A81CACEDBF665C9DE6A0EF"
+NEW_PUB_KEY = """
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mQENBFdZNWIBCAC4dyLC7R/azZqj3fWSNmgt56hT9YORXuXbab4hilmsJzhjRS3p
+/CdXQuNmfRntrvPhTeI1lMubY6MehgxN/TMDXeN+5F4U8Jam2/f3bbYXapBJIzYk
+6/QKURMhb6zu3CjGG5OGzRlHhScljFvTH1gDb1HVG5iA+/g9EBVpFabxa+5MufxN
+m7NcMjANBKYqZxT25vt6kbTYuYgHcb/pyNFOO8OF6wC2dK8D4HLJLkh7Uz7WOqK8
+eiTg9aWK/C2KjXzojcR84mvp5k+72jUbNKuVbOXNZNsSb061cbrtj8v5RxA5IQhj
+LKfy4svi+eFGaQqSm4ul8XF0HPya4TEO2gCJABEBAAG0FnRlc3Qga2V5IDx0ZXN0
+QGtleS5kZT6JATkEEwEIACMFAldZNWICGwMHCwkIBwMCAQYVCAIJCgsEFgIDAQIe
+AQIXgAAKCRDtv2Zcneag7x8ZCACZE3NVnVCh1yvbieoAnvdnnbVsWuHNx9USJvLO
+O5C8dU07b886ma0WNa3srLdH94xq/x0Avxv9oFN/czAg9lUyM4BHfsPhpXhiRdwG
+yDVH/4fFAZ2as0wX4VitxNw3JqvyfFPKfYsVnwyK3znr4dLO/7mvjGIBlyxTT67z
+kY14CYqgIeFbdxxIKw+wkLMfDEI86NfJPbH1GLaVQI9028uuSSkn51nbeTBrOlbl
+bkGblSYhBnLxtH4rTX2ukGppLQ8NnGuwCIw9ZPdEwtekl2GZioXgQEuKaqQPgTPF
+JhJQU+Fmjg8hWRAzAlb1qbJ1O0pUucRIF2fZYBg0nOUuVLGdiQEcBBMBCAAGBQJX
+WTXUAAoJEH3iLhgiX1YPAQUIAOuQhig4Rr/ayO6ndg/JNnyBaD4sv/2CqI9wszya
+ODCNVNmtlHo448189BjBItceuFHmuA6ZyKd1qWwM9WetWAmMwp+Bbus5HNheLhPR
+DXA3DN6UlgfAFkyojd/eWLmHaEURrTKpKpVBhzAHNQ2EqUGxgvbaMJoyswh2xxgZ
+12OoArMXD+b9DiOunseI1OdvbqwMoXjFS+TDc/7V6K212EOkjI7JirHv5qh2pjAs
+AHTJHE1HBU9rZROPmQfFlBmvOYAgrdgYAjOCemRdJnLSFaoT4AfipdRv5kp9Xfzw
+0UlgaNRSCL452i+xFBiP7gqDcGprCvtH2bDpFi18LZo1qja5AQ0EV1k1YgEIAMtR
+lrIE/nf+pXteCH/tAfMKE0ROyptV+/3Cu8MB/bJikvivJRwj9YGqnp1RMiGeTqUl
+sf19SaQzai9SJAcW/1TIC8xYCB5EZJTrR0i+o5vtc0GQ/9WCfOk3jS9BDUAf65lk
+HJpbUZIavjzCQrhMRRqmtgRznlFGfXXDnFaMGzr4bQ3qNJgvQawdnqni+T2YPmyo
+mKiyAJVkJZJbve5DLVNU3qaHBtc5eqzHfo9BkHcCRhco9+YR/MSKhNcRoauUcxoi
+IeOsqG3E1gwmWcjFGUtR20M5Ps5FDGFkwgKbXaWWMV9cvOYUqxpwxBa4h6nIayV/
+giSY2uQT5CzK01m2ADsAEQEAAYkBHwQYAQgACQUCV1k1YgIbDAAKCRDtv2Zcneag
+75DzB/4qJWvPL7ctI/yZ4W9OpBHmAyIWUfAQAcgprLua3SFGRepbRfNBUIA0IJuI
+NFxIMON/8cilbxVMDFn2ynhRrl/9oPaINkGJGdCgSUwF6j5/6Cdda0aSvUEZbmvd
+AQu/EyU91n9IR2Ic6AevI/Y+Y7ouIKSQ3L/ZQ+BxHeWh1PRxyyGqyiwc6W/pqApF
+boUUKB+AMk7hiJ826DuugCG6OG+jHCpQ4cnjUvcm2zNV7wp6KUIfDloHgG9rne4n
+MX1BrUFh3+YWaHHNd0pQb/iBG0ZpyW+LqfAxsQN2/VC1gP1G4xM0rmtFHGi+dKX2
+0R00fQK9pP8xQexesbAiR6gaANJX
+=AGIo
+-----END PGP PUBLIC KEY BLOCK-----
+"""
diff --git a/tests/integration/keymanager/test_openpgp.py b/tests/integration/keymanager/test_openpgp.py
index b16e52fd..d04f5d01 100644
--- a/tests/integration/keymanager/test_openpgp.py
+++ b/tests/integration/keymanager/test_openpgp.py
@@ -37,6 +37,9 @@ from common import (
     PUBLIC_KEY_2,
     PRIVATE_KEY,
     PRIVATE_KEY_2,
+    NEW_PUB_KEY,
+    OLD_AND_NEW_KEY_ADDRESS,
+    OLD_PUB_KEY
 )
 
 
@@ -339,6 +342,15 @@ class OpenPGPCryptoTestCase(KeyManagerWithSoledadTestCase):
         d = pgp.get_key(address, private=private)
         return self.assertFailure(d, KeyNotFound)
 
+    def test_key_is_signed_by(self):
+        pgp = openpgp.OpenPGPScheme(
+            self._soledad, gpgbinary=self.gpg_binary_path)
+        old_pubkey, old_privkey = pgp.parse_key(
+            OLD_PUB_KEY, OLD_AND_NEW_KEY_ADDRESS)
+        new_pubkey, new_privkey = pgp.parse_key(
+            NEW_PUB_KEY, OLD_AND_NEW_KEY_ADDRESS)
+        self.assertTrue(new_pubkey.is_signed_by(old_pubkey))
+
     @inlineCallbacks
     def _insert_key_docs(self, refreshed_at):
         for date in refreshed_at:
diff --git a/tests/integration/keymanager/test_validation.py b/tests/integration/keymanager/test_validation.py
index dccd0f64..aa297127 100644
--- a/tests/integration/keymanager/test_validation.py
+++ b/tests/integration/keymanager/test_validation.py
@@ -23,15 +23,20 @@ from twisted.internet.defer import inlineCallbacks
 
 from leap.bitmask.keymanager.errors import KeyNotValidUpgrade
 from leap.bitmask.keymanager.validation import ValidationLevels
+from leap.bitmask.keymanager.testing import KeyManagerWithSoledadTestCase
 
-from leap.bitmask.keymanager.testing import (
-    KeyManagerWithSoledadTestCase,
+from common import (
     ADDRESS,
     PUBLIC_KEY,
     ADDRESS_2,
     PUBLIC_KEY_2,
     PRIVATE_KEY_2,
-    KEY_FINGERPRINT
+    KEY_FINGERPRINT,
+    NEW_PUB_KEY,
+    NEW_PUB_KEY_FINGERPRINT,
+    OLD_AND_NEW_KEY_ADDRESS,
+    OLD_PUB_KEY_FINGERPRINT,
+    OLD_PUB_KEY
 )
 
 
@@ -81,6 +86,19 @@ class ValidationLevelsTestCase(KeyManagerWithSoledadTestCase):
         yield self.assertFailure(d, KeyNotValidUpgrade)
 
     @inlineCallbacks
+    def test_can_upgrade_key(self):
+        km = self._key_manager()
+
+        yield km.put_raw_key(OLD_PUB_KEY, OLD_AND_NEW_KEY_ADDRESS)
+        old_key = yield km.get_key(OLD_AND_NEW_KEY_ADDRESS, fetch_remote=False)
+        self.assertEquals(old_key.fingerprint, OLD_PUB_KEY_FINGERPRINT)
+
+        yield km.put_raw_key(NEW_PUB_KEY, OLD_AND_NEW_KEY_ADDRESS)
+
+        new_key = yield km.get_key(OLD_AND_NEW_KEY_ADDRESS, fetch_remote=False)
+        self.assertEquals(new_key.fingerprint, NEW_PUB_KEY_FINGERPRINT)
+
+    @inlineCallbacks
     def test_roll_back(self):
         km = self._key_manager()
         yield km.put_raw_key(EXPIRED_KEY_UPDATED, ADDRESS)