From 06f7a9c25dff9dc6d5da1ecdfa0ab59d2cb0c3ab Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 1 Oct 2012 13:56:32 +0200 Subject: support only downloading long keyids (160bit) in add_key_from_keyserver() --- apt/auth.py | 12 +++++++++--- tests/test_auth.py | 3 ++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/apt/auth.py b/apt/auth.py index e088ae85..b523d36f 100644 --- a/apt/auth.py +++ b/apt/auth.py @@ -35,6 +35,10 @@ import apt_pkg from apt_pkg import gettext as _ +class AptKeyError(Exception): + pass + + class TrustedKey(object): """Represents a trusted key.""" @@ -79,7 +83,7 @@ def _call_apt_key_script(*args, **kwargs): output, stderr = proc.communicate(content) if proc.returncode: - raise SystemError("The apt-key script failed with return code %s:\n" + raise AptKeyError("The apt-key script failed with return code %s:\n" "%s\n" "stdout: %s\n" "stderr: %s" % (proc.returncode, " ".join(cmd), @@ -99,9 +103,9 @@ def add_key_from_file(filename): filename -- the absolute path to the public GnuPG key file """ if not os.path.abspath(filename): - raise SystemError("An absolute path is required: %s" % filename) + raise AptKeyError("An absolute path is required: %s" % filename) if not os.access(filename, os.R_OK): - raise SystemError("Key file cannot be accessed: %s" % filename) + raise AptKeyError("Key file cannot be accessed: %s" % filename) _call_apt_key_script("add", filename) def add_key_from_keyserver(keyid, keyserver): @@ -111,6 +115,8 @@ def add_key_from_keyserver(keyid, keyserver): keyid -- the identifier of the key, e.g. 0x0EB12DSA keyserver -- the URL or hostname of the key server """ + if len(keyid) < 160/8: + raise AptKeyError("Only v4 keyids (160bit) are supported") _call_apt_key_script("adv", "--quiet", "--keyserver", keyserver, "--recv", keyid) diff --git a/tests/test_auth.py b/tests/test_auth.py index 99c40db5..7c43d473 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -190,7 +190,8 @@ class TestAuthKeys(unittest.TestCase): self._start_keyserver() self.addCleanup(self._stop_keyserver) - apt.auth.add_key_from_keyserver("46925553", "hkp://localhost:19191") + apt.auth.add_key_from_keyserver( + "A1BD8E9D78F7FE5C3E65D8AF8B48AD6246925553", "hkp://localhost:19191") ret = apt.auth.list_keys() self.assertEqual(len(ret), 1) -- cgit v1.2.3 From f40644d4860dea6461e8eea7c363212eac58fd6a Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 1 Oct 2012 14:16:19 +0200 Subject: check fingerprint after downloading a key and before adding it --- apt/auth.py | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++--- tests/test_auth.py | 16 ++++++++++++ 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/apt/auth.py b/apt/auth.py index b523d36f..01c84635 100644 --- a/apt/auth.py +++ b/apt/auth.py @@ -27,6 +27,7 @@ import atexit import os import os.path +import shutil import subprocess import sys import tempfile @@ -112,13 +113,76 @@ def add_key_from_keyserver(keyid, keyserver): """Import a GnuPG key file to trust repositores signed by it. Keyword arguments: - keyid -- the identifier of the key, e.g. 0x0EB12DSA + keyid -- the long keyid (fingerprint) of the key, e.g. + A1BD8E9D78F7FE5C3E65D8AF8B48AD6246925553 keyserver -- the URL or hostname of the key server """ + tmp_keyring_dir = tempfile.mkdtemp() + try: + _add_key_from_keyserver(keyid, keyserver, tmp_keyring_dir) + except: + raise + finally: + print tmp_keyring_dir + shutil.rmtree(tmp_keyring_dir) + +def _add_key_from_keyserver(keyid, keyserver, tmp_keyring_dir): if len(keyid) < 160/8: - raise AptKeyError("Only v4 keyids (160bit) are supported") - _call_apt_key_script("adv", "--quiet", "--keyserver", keyserver, - "--recv", keyid) + raise AptKeyError("Only long keyids (v4, 160bit) are supported") + # create a temp keyring dir + tmp_secret_keyring = os.path.join(tmp_keyring_dir, "secring.gpg") + tmp_keyring = os.path.join(tmp_keyring_dir, "pubring.gpg") + # default options for gpg + gpg_default_options = [ + "gpg", + "--no-default-keyring", "--no-options", + "--homedir", tmp_keyring_dir, + ] + # download the key to a temp keyring first + res = subprocess.call(gpg_default_options + [ + "--secret-keyring", tmp_secret_keyring, + "--keyring", tmp_keyring, + "--keyserver", keyserver, + "--recv", keyid, + ]) + if res != 0: + raise AptKeyError("recv from '%s' failed for '%s'" % ( + keyserver, keyid)) + # now export again using the long key id (to ensure that there is + # really only this one key in our keyring) and not someone MITM us + tmp_export_keyring = os.path.join(tmp_keyring_dir, "export-keyring.gpg") + res = subprocess.call(gpg_default_options + [ + "--keyring", tmp_keyring, + "--output", tmp_export_keyring, + "--export", keyid, + ]) + if res != 0: + raise AptKeyError("export of '%s' failed", keyid) + # now verify the fingerprint, this is probably redundant as we + # exported by the fingerprint in the previous command but its + # still good paranoia + output = subprocess.Popen( + gpg_default_options + [ + "--keyring", tmp_export_keyring, + "--fingerprint", + "--batch", + "--with-colons", + ], + stdout=subprocess.PIPE, + universal_newlines=True).communicate()[0] + got_fingerprint=None + for line in output.splitlines(): + if line.startswith("fpr:"): + got_fingerprint = line.split(":")[9] + # stop after the first to ensure no subkey trickery + break + signing_key_fingerprint = keyid + if got_fingerprint != signing_key_fingerprint: + raise AptKeyError( + "Fingerprints do not match, not importing: '%s' != '%s'" % ( + signing_key_fingerprint, got_fingerprint)) + # finally add it + add_key_from_file(tmp_export_keyring) def add_key(content): """Import a GnuPG key to trust repositores signed by it. diff --git a/tests/test_auth.py b/tests/test_auth.py index 7c43d473..4e37b3d3 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -185,6 +185,22 @@ class TestAuthKeys(unittest.TestCase): self.assertEqual(key.keyid, "46925553") self.assertEqual(key.date, "2012-04-27") + def test_add_key_from_keyserver_too_short(self): + """Ensure that short keyids are not imported""" + with self.assertRaises(apt.auth.AptKeyError): + apt.auth.add_key_from_keyserver("46925553", "hkp://localhost:19191") + + def test_add_key_from_server_mitm(self): + """Verify that the key fingerprint is verified after download""" + self._start_keyserver() + self.addCleanup(self._stop_keyserver) + with self.assertRaises(apt.auth.AptKeyError) as cm: + apt.auth.add_key_from_keyserver( + "0101010178F7FE5C3E65D8AF8B48AD6246925553", + "hkp://localhost:19191") + self.assertTrue( + str(cm.exception).startswith("Fingerprints do not match")) + def testAddKeyFromServer(self): """Install a GnuPG key from a remote server.""" self._start_keyserver() -- cgit v1.2.3 From 649ae227b85f1d9d69cd186515589d47acd0ee46 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 1 Oct 2012 14:18:56 +0200 Subject: apt/auth.py: proper cleanup --- apt/auth.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apt/auth.py b/apt/auth.py index 01c84635..4d4d50e9 100644 --- a/apt/auth.py +++ b/apt/auth.py @@ -123,8 +123,7 @@ def add_key_from_keyserver(keyid, keyserver): except: raise finally: - print tmp_keyring_dir - shutil.rmtree(tmp_keyring_dir) + shutil.rmtree(tmp_keyring_dir) def _add_key_from_keyserver(keyid, keyserver, tmp_keyring_dir): if len(keyid) < 160/8: -- cgit v1.2.3 From c117c260d1a652d5ebcc4c33a88945002fbb4364 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 2 Oct 2012 11:06:51 +0200 Subject: apt/auth.py: fix trailing whitespace --- apt/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apt/auth.py b/apt/auth.py index 4d4d50e9..742b5cc2 100644 --- a/apt/auth.py +++ b/apt/auth.py @@ -123,7 +123,7 @@ def add_key_from_keyserver(keyid, keyserver): except: raise finally: - shutil.rmtree(tmp_keyring_dir) + shutil.rmtree(tmp_keyring_dir) def _add_key_from_keyserver(keyid, keyserver, tmp_keyring_dir): if len(keyid) < 160/8: -- cgit v1.2.3 From 7f97f163de6189c9a770fa5d723ddac85a2b976c Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 2 Oct 2012 11:09:29 +0200 Subject: add changelog entry --- debian/changelog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/debian/changelog b/debian/changelog index 161cacfd..11a230b8 100644 --- a/debian/changelog +++ b/debian/changelog @@ -7,6 +7,10 @@ python-apt (0.8.8) UNRELEASED; urgency=low * merged lp:~sampo555/python-apt/fix_1042916 reuse existing but disabled sources.list entries instead of duplicating them. Thanks to "sampo555", LP: #1042916 + * lp:~mvo/python-apt/recv-key-lp1016643: + - Only support long (v4) keyids when downloading keys and + check the keys fingerprint before importing. This avoids + man-in-the-middle attacks (LP: #1016643) [ James Hunt ] * python/cache.cc: PkgCacheGetIsMultiArch(): Return calculated -- cgit v1.2.3