diff --git a/CHANGELOG b/CHANGELOG index 33ba541676eee5e162bc7f5c8d87c8c985186e34..266d2619184595d46487df2b9e3d1ddd5769403b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,18 +1,27 @@ -debexpo (UNRELEASED + 1) - - * Support for sponsoring guidelines. Debian Developer are asked to register at - mentors.debian.net (if not done already) and provide there their personal - sponsoring guidelines. That's is a semi-structured description text to - point sponsors to individual developers who are interested to sponsor some - packages. - - * Provide public profile pages for both, maintainer and developers. For - maintainer their package list is being displayed among their profile data. - On the other hand, for Debian Developers their sponsoring guidelines are - being displayed. - Both can disable their profiles if desired. - - +debexpo (UNRELEASED) + + * Support for sponsoring guidelines. Debian Developer are asked to + register at mentors.debian.net (if not done already) and provide + there their personal sponsoring guidelines. That's is a + semi-structured description text to point sponsors to individual + developers who are interested to sponsor some packages. + + * Provide public profile pages for both, maintainer and developers. + For maintainer their package list is being displayed among their + profile data. On the other hand, for Debian Developers their + sponsoring guidelines are being displayed. Both can disable their + profiles if desired. + + * Determine Debexpo user ID by the GPG key ID. This means Debexpo will + now emulate behavior of official Debian archives to identify an + upload solely by its signature. Thanks to Aliaksei Palkanau who + provided a patch on which this improvement is based on. Previously + Debexpo used the Changed-By field to determine the uploader. + Moreover GPG keys are being verified upon import now. + Thanks to a patch by Vipin Nair the key strength must match Debian + keyring maintainer criterias (2048 bit RSA keys). + Finally at least one UID of the key to be imported must match the + configured profile email address. debexpo (v2) diff --git a/bin/debexpo_importer.py b/bin/debexpo_importer.py index 203f8d4b16fbd88d0fed5747801b4cd98547ce25..03b303293cd38a71f232c5af656d73daa12350f6 100755 --- a/bin/debexpo_importer.py +++ b/bin/debexpo_importer.py @@ -363,24 +363,36 @@ class Importer(object): return package_version + def _find_user_by_email_address(self, email_address): + """ + Searches user by email address + """ + # XXX: Maybe model is more appropriate place for such a method + self.user = meta.session.query(User).filter_by(email=email_address).filter_by(verification=None).first() + return self.user - def _determine_uploader(self): + def _determine_uploader_by_changedby_field(self): """ Create a user object based on the Changed-By entry - This object will also exist if the user was NOT found. This is useful - to have a user object to send reject mails to """ - if self.user_id is None: - maintainer_string = self.changes.get('Changed-By') - log.debug("Determining user from 'Changed-By:' field: %s" % maintainer_string) - maintainer_realname, maintainer_email_address = email.utils.parseaddr(maintainer_string) - log.debug("Changed-By's email address is: %s", maintainer_email_address) - self.user = meta.session.query(User).filter_by( - email=maintainer_email_address).filter_by(verification=None).first() - if self.user is None: - # generate user object, but only to send out reject message - self.user = User(id=-1, name=maintainer_realname, email=maintainer_email_address) - self.user_id = self.user.id + maintainer_string = self.changes.get('Changed-By') + log.debug("Determining user from 'Changed-By:' field: %s" % maintainer_string) + maintainer_realname, maintainer_email_address = email.utils.parseaddr(maintainer_string) + log.debug("Changed-By's email address is: %s", maintainer_email_address) + self._find_user_by_email_address(maintainer_email_address) + + def _determine_uploader_by_gpg(self, gpg_out): + """ + Create a user object based on the gpg output + """ + + for (name, email) in gpg_out: + log.debug("GPG signature matches user %s <%s>" % (name, email)) + if self._find_user_by_email_address(email): + log.debug("GPG signature mapped to user %s <%s>" % (self.user.name, self.user.email)) + return + + return None def main(self): """ @@ -409,7 +421,7 @@ class Importer(object): # Determine user from changed-by field # This might be temporary, the GPG check should replace the user later # At this stage it is only helpful to get an email address to send blame mails to - self._determine_uploader() + self._determine_uploader_by_changedby_field() # Checks whether the upload has a dsc file if not self.changes.get_dsc(): @@ -421,19 +433,23 @@ class Importer(object): " use the default flags or -S for a source only" " upload)") - # Next, find out whether the changes file was signed with a valid signature, if not reject immediately if not self.skip_gpg: + # Next, find out whether the changes file was signed with a valid signature, if not reject immediately if not signature.is_signed(self.changes_file): self._reject('Your upload does not appear to be signed') - (gpg_out, gpg_status) = signature.verify_sig_full(self.changes_file) + (gpg_out, gpg_uids, gpg_status) = signature.verify_sig_full(self.changes_file) if gpg_status != 0: self._reject('Your upload does not contain a valid signature. Output was:\n%s' % (gpg_out)) - log.debug("GPG signature matches user %s" % (self.user.email)) + self._determine_uploader_by_gpg(gpg_uids) - # XXX: Replace self.user by something which was verified by GPG! - - if self.user_id == -1: + if self.user is None: + # Creates a fake user object, but only if no user was found before + # This is useful to have a user object to send reject mails to + # generate user object, but only to send out reject message + self.user = User(id=-1, name=maintainer_realname, email=maintainer_email_address) self._reject('Couldn\'t find user %s. Exiting.' % self.user.email) + + self.user_id = self.user.id log.debug("User found in database. Has id: %s", self.user.id) self.files = self.changes.get_files() diff --git a/debexpo/controllers/my.py b/debexpo/controllers/my.py index d1846989c927e7d64614532c3b3628d969a48136..b4a942643d033e41b153221108f92274950d9bf6 100644 --- a/debexpo/controllers/my.py +++ b/debexpo/controllers/my.py @@ -113,7 +113,7 @@ class MyController(BaseController): if 'gpg' in self.form_result and self.form_result['gpg'] is not None: log.debug('Setting a new GPG key') self.user.gpg = self.form_result['gpg'].value - self.user.gpg_id = self.gnupg.parse_key_id(self.user.gpg) + (self.user.gpg_id, _) = self.gnupg.parse_key_id(self.user.gpg) temp = tempfile.NamedTemporaryFile(delete=True) temp.write(self.user.gpg) @@ -244,7 +244,7 @@ class MyController(BaseController): 'metrics' : self._metrics, }[request.params['form']]() except KeyError: - log.error('Could not find form name; defaulting to main page') + log.error('Could not find form name "%s"; defaulting to main page' % (request.params['form'])) pass log.debug('Populating template context') diff --git a/debexpo/lib/gnupg.py b/debexpo/lib/gnupg.py index 5e3a732ffc9ceef59fe11713ad3b259ed3c31436..6382ac6996234bba6f4b9cb1bb9e84da5e98c4a8 100644 --- a/debexpo/lib/gnupg.py +++ b/debexpo/lib/gnupg.py @@ -39,6 +39,7 @@ __license__ = 'MIT' import logging import os import subprocess +import re import pylons import re @@ -132,7 +133,7 @@ class GnuPG(object): def parse_key_id(self, key, email = None): """ - Returns the key id of the given GPG public key. + Returns the key id of the given GPG public key along with a list of user ids. ``key`` ASCII armored GPG public key. @@ -148,7 +149,7 @@ class GnuPG(object): output = unicode(output, errors='replace') keys = KeyData.read_from_gpg(output.splitlines()) for key in keys.values(): - return key.get_id() + return (key.get_id(), key.get_all_uid()) except (AttributeError, IndexError): log.error("Failed to extract key id from gpg output: '%s'" % output) @@ -160,7 +161,7 @@ class GnuPG(object): function which returns a boolean only """ - (_, status) = self.verify_sig_full(signed_file, pubring) + (_, _, status) = self.verify_sig_full(signed_file, pubring) return status == 0 def verify_sig_full(self, signed_file, pubring=None): @@ -175,7 +176,14 @@ class GnuPG(object): setting will be used (~/.gnupg/pubring.gpg)) """ args = ('--verify', signed_file) - return self._run(args=args, pubring=pubring) + (out, return_code) = self._run(args=args, pubring=pubring) + gpg_addr_pattern = re.compile(r"^gpg: Good signature from \"(?P.+?)\s*(?:\((?P.+)\))?\s*(?:<(?P.+)>)?\"") + user_ids = [] + for line in out.split("\n"): + addr_matcher = gpg_addr_pattern.search(line) + if addr_matcher is not None: + user_ids.append( (addr_matcher.group('name'), addr_matcher.group('email')) ) + return (out, user_ids, return_code) def add_signature(self, signature_file, pubring=None): @@ -341,6 +349,13 @@ class KeyData(object): short_fpr = self.format_short_fpr(self.pub[4]) return "{}/{}".format(short_id, short_fpr) + def get_all_uid(self): + user_ids = [] + for uid_object in self.uids.values(): + uid = uid_object.split() + user_ids.append((uid['name'], uid['email'])) + return user_ids + def keycheck(self): return KeycheckKeyResult(self) diff --git a/debexpo/lib/validators.py b/debexpo/lib/validators.py index e8ca244d796f3c3ec755a4701d79a2eab59cb706..b8d8cab855903420efa951ce8edb95cfbb8cc4e9 100644 --- a/debexpo/lib/validators.py +++ b/debexpo/lib/validators.py @@ -78,14 +78,30 @@ class GpgKey(formencode.validators.FieldStorageUploadConverter): 'properly configured to handle' + 'GPG keys'), value, c) - self.gpg_id = self.gnupg.parse_key_id(value.value) + + (self.gpg_id, user_ids) = self.gnupg.parse_key_id(value.value) if self.gpg_id is None: log.error("Failed to parse GPG key") raise formencode.Invalid(_('Invalid GPG key'), value, c) + + """ + allow only keys which match the user name + """ + + user = meta.session.query(User).get(session['user_id']) + for (uid, email) in user_ids: + if user.email == email: + break + else: + log.debug("User id {} [{}]: fail to upload GPG key: no uid matching email <{}>".format( + user.id, user.name, user.email)) + raise formencode.Invalid(_('None of your user IDs in key %s does match your profile mail address' % (self.gpg_id)), value, c) + """ Minimum Key Strength Check. """ + requiredkeystrength = int(config['debexpo.gpg_minkeystrength']) keystrength = self.gnupg.extract_key_strength(self.key_id()) keytype = self.gnupg.extract_key_type(self.key_id()) diff --git a/debexpo/tests/functional/test_my.py b/debexpo/tests/functional/test_my.py index 5a07f1eb17b64a743599a61d79d3d7b86f3e03be..ecc31984663786b85fc9321048efb1c7ef92d78e 100644 --- a/debexpo/tests/functional/test_my.py +++ b/debexpo/tests/functional/test_my.py @@ -9,7 +9,7 @@ import os import shutil class TestMyController(TestController): - _GPGKEY = """-----BEGIN PGP PUBLIC KEY BLOCK----- + _GPGKEY_WRONG_EMAIL = """-----BEGIN PGP PUBLIC KEY BLOCK----- mDMEW9b91RYJKwYBBAHaRw8BAQdAHtUIQWAsmPilu0JDMnLbpPQfT1i3z2IVMoDH rhlYkO+0JWRlYmV4cG8gdGVzdGluZyA8ZGViZXhwb0BleGFtcGxlLm9yZz6IkAQT @@ -22,7 +22,23 @@ AAoJEChiGOfHT5wRNK8A/115pc8+OwKDy1fGXGX3l0uq1wdfiJreG/9YZddx/JTI AQD4ZLpyUg+z6kJ+8YAmHFiOD9Ixv3QVvrfpBwnBVtJZBg== =N+9W -----END PGP PUBLIC KEY BLOCK-----""" - _GPG_ID = '256E/C74F9C11' + + _GPG_ID_WRONG_EMAIL = '256E/C74F9C11' + + _GPGKEY = """-----BEGIN PGP PUBLIC KEY BLOCK----- + +mDMEW+iERRYJKwYBBAHaRw8BAQdAZN+9IfILcMWaZ5bOx4Ykmum/1ZMaxZAw1YbI +KjEWWU60J0RlYmV4cG8gdGVzdGluZyBrZXkgPGVtYWlsQGV4YW1wbGUuY29tPoiQ +BBMWCAA4FiEEdhj55Cj1+6e2+jO1NU98o/QgaL4FAlvohEUCGwMFCwkIBwIGFQoJ +CAsCBBYCAwECHgECF4AACgkQNU98o/QgaL7I+wEAjY6np4hgEfkotEM0hpOo1LGF +sWWiO1OKhi/Nfg+WOoUA/0/DEcGfclpGhpB+unaqn0dLnMKDJeZAxINji7/Lz2gH +uDgEW+iERRIKKwYBBAGXVQEFAQEHQJwX6mLJZQMkBwKbyJa0+oz15wSiYHFONGYI +s9TdseYWAwEIB4h4BBgWCAAgFiEEdhj55Cj1+6e2+jO1NU98o/QgaL4FAlvohEUC +GwwACgkQNU98o/QgaL6XtAEAl+8Pqc8q6EWTudqgynVIpdraSuBrVSaEcxffKaT3 +P6YA/0SM1Yi/F2maISv8k44MzRAdGf2yFabwsfdCH+RLD6YO +=BYiE +-----END PGP PUBLIC KEY BLOCK-----""" + _GPG_ID= '256E/F42068BE' def _setup_gpg_env(self): self.homedir = tempfile.mkdtemp() @@ -121,6 +137,27 @@ AQD4ZLpyUg+z6kJ+8YAmHFiOD9Ixv3QVvrfpBwnBVtJZBg== user = meta.session.query(User).filter(User.email=='email@example.com').one() self.assertEquals(user.gpg, None) + def test__gpg_wrong_email(self): + response = self.app.post(url('my'), {'form': 'gpg'}) + self.assertEquals(response.status_int, 302) + self.assertTrue(response.location.endswith(url('login'))) + response = self.app.post(url('login'), self._AUTHDATA) + user = meta.session.query(User).filter(User.email=='email@example.com').one() + self.assertEquals(user.gpg, None) + + # upload GPG key + response = self.app.post(url('my'), {'form': 'gpg', + 'delete_gpg': 0, + 'commit': 'submit'}, + upload_files = [('gpg', 'mykey.asc', + self._GPGKEY_WRONG_EMAIL)]) + self.assertEquals(response.status_int, 200) + self.assertTrue('None of your user IDs in key {} does match your' + ' profile mail address'.format(self._GPG_ID_WRONG_EMAIL) + in response) + user = meta.session.query(User).filter(User.email=='email@example.com').one() + self.assertEquals(user.gpg, None) + def test__details(self): response = self.app.post(url('my'), {'form': 'details'}) self.assertEquals(response.status_int, 302) diff --git a/debexpo/tests/test_gnupg.py b/debexpo/tests/test_gnupg.py index de7a37fb9df07d0cac44d8541a27aaf70fe1a56c..2da4f41c9ed54c69a77f2c8591d0b9c793f17c30 100644 --- a/debexpo/tests/test_gnupg.py +++ b/debexpo/tests/test_gnupg.py @@ -59,6 +59,8 @@ AQD4ZLpyUg+z6kJ+8YAmHFiOD9Ixv3QVvrfpBwnBVtJZBg== """ test_gpg_key_id = '256E/C74F9C11' +test_gpg_key_name = 'debexpo testing' +test_gpg_key_email = 'debexpo@example.org' class TestGnuPGController(TestCase): @@ -94,7 +96,19 @@ class TestGnuPGController(TestCase): """ gnupg = self._get_gnupg() self.assertFalse(gnupg.is_unusable()) - self.assertEqual(gnupg.parse_key_id(test_gpg_key), test_gpg_key_id) + (gpg_key_id, _) = gnupg.parse_key_id(test_gpg_key) + self.assertEqual(gpg_key_id, test_gpg_key_id) + + def testParseKeyIDWithUID(self): + """ + Test the extraction of an uid from a given GPG key. + """ + gnupg = self._get_gnupg() + self.assertFalse(gnupg.is_unusable()) + (_, gpg_key_uids) = gnupg.parse_key_id(test_gpg_key) + (gpg_key_name, gpg_key_email) = gpg_key_uids[0] + self.assertEqual(gpg_key_name, test_gpg_key_name) + self.assertEqual(gpg_key_email, test_gpg_key_email) def testSignatureVerification(self): """ @@ -109,6 +123,21 @@ class TestGnuPGController(TestCase): assert os.path.exists(pubring) self.assertTrue(gnupg.verify_sig(signed_file, pubring)) + def testSignatureVerificationWithUID(self): + """ + Verify the signature in the file debexpo/tests/gpg/debian_announcement.gpg.asc. + """ + gnupg = self._get_gnupg() + self.assertFalse(gnupg.is_unusable()) + gpg_data_dir = os.path.join(os.path.dirname(__file__), 'gpg') + signed_file = os.path.join(gpg_data_dir, 'debian_announcement.gpg.asc') + pubring = os.path.join(gpg_data_dir, 'pubring.gpg') + assert os.path.exists(signed_file) + assert os.path.exists(pubring) + (out, uids, status) = gnupg.verify_sig_full(signed_file, pubring) + self.assertEquals(status, 0) + self.assertTrue(('debexpo testing', 'debexpo@example.org') in uids) + def testInvalidSignature(self): """ Test that verify_sig() fails for an unsigned file.