From a71dbd142a31eca4705cd0faee1eb9a4c4d43e93 Mon Sep 17 00:00:00 2001 From: Aliaksei Palkanau Date: Fri, 6 Apr 2012 02:12:02 +0300 Subject: [PATCH 1/5] Use GPG to verify the uploader of the package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Arno Töll --- bin/debexpo_importer.py | 70 ++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/bin/debexpo_importer.py b/bin/debexpo_importer.py index 2fea55bd..44a0a20b 100755 --- a/bin/debexpo_importer.py +++ b/bin/debexpo_importer.py @@ -361,24 +361,46 @@ class Importer(object): self.send_email(email, [self.user.email], package=self.changes['Source'], dsc_url=dsc_url, rfs_url=rfs_url) + def _generate_fake_user_if_not_found(self, maintainer_realname, maintainer_email_address): + """ + 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 + """ + 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) + + 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() - 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 + """ + gpg_addr_pattern = re.compile('"' + '(?P.+)' + '\s+' + '<(?P.+?)>' + '"') + addr_matcher = gpg_addr_pattern.search(gpg_out) + if addr_matcher is not None: + maintainer_realname, maintainer_email_address = addr_matcher.group('name'), addr_matcher.group('email') + log.debug("GPG signature matches user %s" % (maintainer_email_address)) + self._find_user_by_email_address(maintainer_email_address) def main(self): """ @@ -402,26 +424,22 @@ class Importer(object): except Exception as e: # XXX: The user won't ever see this message. The changes file was # invalid, we don't know whom send it to - self._reject("Your changes file appears invalid. Refusing your upload\n%s" % (e.message)) - - + self._reject("Your changes file appears invalid. Refusing your upload\n%s" % (e.message)) - # 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() - - # 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) 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)) - - # XXX: Replace self.user by something which was verified by GPG! + self._determine_uploader_by_gpg(gpg_out) + else: + self._determine_uploader_by_changedby_field() + self._generate_fake_user_if_not_found() + # XXX: Replace self.user by something which was verified by GPG! + self.user_id = self.user.id if self.user_id == -1: self._reject('Couldn\'t find user %s. Exiting.' % self.user.email) log.debug("User found in database. Has id: %s", self.user.id) -- GitLab From a52cd6fba58f2cdd0c740b435f150c0725377cc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arno=20T=C3=B6ll?= Date: Mon, 16 Apr 2012 20:50:58 -0400 Subject: [PATCH 2/5] * Refactor the GPG verification into lib/gnupg, check all uids of the keyring, accept uploads only if an user could be obtained by the GPG signature Implement a form validator to verify that the imported key matches the * profile mail address * Update CHANGELOG --- CHANGELOG | 39 ++++++++++++++++++++------------- bin/debexpo_importer.py | 45 +++++++++++++++++---------------------- debexpo/controllers/my.py | 4 ++-- debexpo/lib/gnupg.py | 38 ++++++++++++++++++++++++++++----- debexpo/lib/validators.py | 17 ++++++++++++++- 5 files changed, 94 insertions(+), 49 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 33ba5416..266d2619 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 44a0a20b..a13108f3 100755 --- a/bin/debexpo_importer.py +++ b/bin/debexpo_importer.py @@ -361,21 +361,13 @@ class Importer(object): self.send_email(email, [self.user.email], package=self.changes['Source'], dsc_url=dsc_url, rfs_url=rfs_url) - def _generate_fake_user_if_not_found(self, maintainer_realname, maintainer_email_address): - """ - 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 - """ - 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) - 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_by_changedby_field(self): """ @@ -391,16 +383,14 @@ class Importer(object): """ Create a user object based on the gpg output """ - gpg_addr_pattern = re.compile('"' - '(?P.+)' - '\s+' - '<(?P.+?)>' - '"') - addr_matcher = gpg_addr_pattern.search(gpg_out) - if addr_matcher is not None: - maintainer_realname, maintainer_email_address = addr_matcher.group('name'), addr_matcher.group('email') - log.debug("GPG signature matches user %s" % (maintainer_email_address)) - self._find_user_by_email_address(maintainer_email_address) + + 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): """ @@ -424,24 +414,27 @@ class Importer(object): except Exception as e: # XXX: The user won't ever see this message. The changes file was # invalid, we don't know whom send it to - self._reject("Your changes file appears invalid. Refusing your upload\n%s" % (e.message)) + self._reject("Your changes file appears invalid. Refusing your upload\n%s" % (e.message)) 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)) - self._determine_uploader_by_gpg(gpg_out) + self._determine_uploader_by_gpg(gpg_uids) else: self._determine_uploader_by_changedby_field() - self._generate_fake_user_if_not_found() - # XXX: Replace self.user by something which was verified by GPG! - self.user_id = self.user.id - 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 9bca89e0..e02b1260 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 d532cc01..42a53ba3 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 @@ -119,7 +120,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. @@ -134,10 +135,26 @@ class GnuPG(object): (output, _) = self._run(stdin=key) output = unicode(output, errors='replace') lines = (output.split('\n')) + key_id = None + user_ids = [] + gpg_addr_pattern = re.compile('(pub\s+\S+\s+\S+\s+|uid\s+)' + '(?P.+)' + '\s+' + '<(?P.+?)>' + '$') + for line in lines: - if line.startswith('pub'): + if not key_id and line.startswith('pub'): # get only the 2nd column of the 1st matching line - return line.split()[1] + key_id = line.split()[1] + addr_matcher = gpg_addr_pattern.search(line) + if addr_matcher is not None: + user_ids.append( (addr_matcher.group('name'), addr_matcher.group('email')) ) + if line.startswith('sub'): + break + + return (key_id, user_ids) + except (AttributeError, IndexError): log.error("Failed to extract key id from gpg output: '%s'" % output) @@ -149,7 +166,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): @@ -164,7 +181,18 @@ 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('"' + '(?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): diff --git a/debexpo/lib/validators.py b/debexpo/lib/validators.py index a2218740..83c88e5f 100644 --- a/debexpo/lib/validators.py +++ b/debexpo/lib/validators.py @@ -78,14 +78,29 @@ 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("No user id in key %s does match the email address the user configured") + 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()) if keystrength < requiredkeystrength: -- GitLab From a37a318ebd00b9fd4042f329fab007ae66fe96c2 Mon Sep 17 00:00:00 2001 From: Baptiste BEAUPLAT Date: Thu, 15 Nov 2018 22:29:39 +0100 Subject: [PATCH 3/5] Check in MyController test that gpg keys are not added when the email mismatch --- debexpo/tests/functional/test_my.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/debexpo/tests/functional/test_my.py b/debexpo/tests/functional/test_my.py index d8b0aba0..ecc31984 100644 --- a/debexpo/tests/functional/test_my.py +++ b/debexpo/tests/functional/test_my.py @@ -155,6 +155,8 @@ P6YA/0SM1Yi/F2maISv8k44MzRAdGf2yFabwsfdCH+RLD6YO 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'}) -- GitLab From 4adcd6d43ac9d779fd736b5bd1fbf618c67ea891 Mon Sep 17 00:00:00 2001 From: Baptiste BEAUPLAT Date: Thu, 15 Nov 2018 22:52:04 +0100 Subject: [PATCH 4/5] Explicit log message when user fails to upload a GPG key matching its email --- debexpo/lib/validators.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/debexpo/lib/validators.py b/debexpo/lib/validators.py index 418e33d8..b8d8cab8 100644 --- a/debexpo/lib/validators.py +++ b/debexpo/lib/validators.py @@ -94,7 +94,8 @@ class GpgKey(formencode.validators.FieldStorageUploadConverter): if user.email == email: break else: - log.debug("No user id in key %s does match the email address the user configured") + 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) """ -- GitLab From 8af8a6db7127227fc6c737628c25c699acd087ce Mon Sep 17 00:00:00 2001 From: Baptiste BEAUPLAT Date: Thu, 15 Nov 2018 22:56:52 +0100 Subject: [PATCH 5/5] Use safe UID regexp when checking GPG signature --- debexpo/lib/gnupg.py | 6 +----- debexpo/tests/test_gnupg.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/debexpo/lib/gnupg.py b/debexpo/lib/gnupg.py index a51a5cbb..6382ac69 100644 --- a/debexpo/lib/gnupg.py +++ b/debexpo/lib/gnupg.py @@ -177,11 +177,7 @@ class GnuPG(object): """ args = ('--verify', signed_file) (out, return_code) = self._run(args=args, pubring=pubring) - gpg_addr_pattern = re.compile('"' - '(?P.+)' - '\s+' - '<(?P.+?)>' - '"') + 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) diff --git a/debexpo/tests/test_gnupg.py b/debexpo/tests/test_gnupg.py index 94968cf6..2da4f41c 100644 --- a/debexpo/tests/test_gnupg.py +++ b/debexpo/tests/test_gnupg.py @@ -123,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. -- GitLab