Merge lp:~cjwatson/launchpad/handle-more-gpg-exceptions into lp:launchpad

Proposed by Colin Watson on 2019-08-23
Status: Merged
Merged at revision: 19040
Proposed branch: lp:~cjwatson/launchpad/handle-more-gpg-exceptions
Merge into: lp:launchpad
Diff against target: 469 lines (+302/-23)
9 files modified
lib/lp/registry/model/codeofconduct.py (+2/-1)
lib/lp/registry/tests/test_codeofconduct.py (+194/-0)
lib/lp/services/gpg/doc/gpg-signatures.txt (+2/-2)
lib/lp/services/gpg/handler.py (+0/-9)
lib/lp/services/gpg/interfaces.py (+1/-7)
lib/lp/services/mail/incoming.py (+5/-3)
lib/lp/services/mail/tests/test_incoming.py (+96/-0)
lib/lp/soyuz/doc/fakepackager.txt (+1/-1)
lib/lp/testing/fixture.py (+1/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/handle-more-gpg-exceptions
Reviewer Review Type Date Requested Status
William Grant code 2019-08-23 Approve on 2019-08-29
Review via email: mp+371763@code.launchpad.net

Commit message

Handle more GPG exceptions when verifying code of conduct signatures or authenticating incoming email.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/model/codeofconduct.py'
2--- lib/lp/registry/model/codeofconduct.py 2018-05-06 08:52:34 +0000
3+++ lib/lp/registry/model/codeofconduct.py 2019-08-23 19:42:27 +0000
4@@ -43,6 +43,7 @@
5 )
6 from lp.services.gpg.interfaces import (
7 GPGKeyExpired,
8+ GPGKeyNotFoundError,
9 GPGVerificationError,
10 IGPGHandler,
11 )
12@@ -272,7 +273,7 @@
13
14 try:
15 sig = gpghandler.getVerifiedSignature(sane_signedcode)
16- except (GPGVerificationError, GPGKeyExpired) as e:
17+ except (GPGVerificationError, GPGKeyExpired, GPGKeyNotFoundError) as e:
18 return str(e)
19
20 if not sig.fingerprint:
21
22=== added file 'lib/lp/registry/tests/test_codeofconduct.py'
23--- lib/lp/registry/tests/test_codeofconduct.py 1970-01-01 00:00:00 +0000
24+++ lib/lp/registry/tests/test_codeofconduct.py 2019-08-23 19:42:27 +0000
25@@ -0,0 +1,194 @@
26+# Copyright 2019 Canonical Ltd. This software is licensed under the
27+# GNU Affero General Public License version 3 (see the file LICENSE).
28+
29+"""Test codes of conduct."""
30+
31+from __future__ import absolute_import, print_function, unicode_literals
32+
33+__metaclass__ = type
34+
35+from textwrap import dedent
36+
37+from testtools.matchers import (
38+ ContainsDict,
39+ Equals,
40+ MatchesRegex,
41+ )
42+from zope.component import getUtility
43+
44+from lp.registry.interfaces.codeofconduct import (
45+ ICodeOfConductSet,
46+ ISignedCodeOfConductSet,
47+ )
48+from lp.services.config import config
49+from lp.services.gpg.handler import PymeSignature
50+from lp.services.gpg.interfaces import (
51+ GPGKeyExpired,
52+ GPGKeyNotFoundError,
53+ GPGVerificationError,
54+ IGPGHandler,
55+ )
56+from lp.services.mail.sendmail import format_address
57+from lp.testing import TestCaseWithFactory
58+from lp.testing.fixture import ZopeUtilityFixture
59+from lp.testing.layers import ZopelessDatabaseLayer
60+
61+
62+class FakePymeKey:
63+
64+ def __init__(self, fingerprint):
65+ self.fingerprint = fingerprint
66+
67+
68+class FakeGPGHandlerBadSignature:
69+
70+ def getVerifiedSignature(self, content, signature=None):
71+ raise GPGVerificationError("Bad signature.")
72+
73+
74+class FakeGPGHandlerExpired:
75+
76+ def __init__(self, key):
77+ self.key = key
78+
79+ def getVerifiedSignature(self, content, signature=None):
80+ raise GPGKeyExpired(self.key)
81+
82+
83+class FakeGPGHandlerNotFound:
84+
85+ def __init__(self, fingerprint):
86+ self.fingerprint = fingerprint
87+
88+ def getVerifiedSignature(self, content, signature=None):
89+ raise GPGKeyNotFoundError(self.fingerprint)
90+
91+
92+class FakeGPGHandlerGood:
93+
94+ def __init__(self, signature):
95+ self.signature = signature
96+
97+ def getVerifiedSignature(self, content, signature=None):
98+ return self.signature
99+
100+
101+class TestSignedCodeOfConductSet(TestCaseWithFactory):
102+
103+ layer = ZopelessDatabaseLayer
104+
105+ def test_verifyAndStore_bad_signature(self):
106+ self.useFixture(ZopeUtilityFixture(
107+ FakeGPGHandlerBadSignature(), IGPGHandler))
108+ user = self.factory.makePerson()
109+ self.assertEqual(
110+ "Bad signature.",
111+ getUtility(ISignedCodeOfConductSet).verifyAndStore(
112+ user, "signed data"))
113+
114+ def test_verifyAndStore_expired(self):
115+ key = FakePymeKey("0" * 40)
116+ self.useFixture(ZopeUtilityFixture(
117+ FakeGPGHandlerExpired(key), IGPGHandler))
118+ user = self.factory.makePerson()
119+ self.assertEqual(
120+ "%s has expired" % key.fingerprint,
121+ getUtility(ISignedCodeOfConductSet).verifyAndStore(
122+ user, "signed data"))
123+
124+ def test_verifyAndStore_not_found(self):
125+ fingerprint = "0" * 40
126+ self.useFixture(ZopeUtilityFixture(
127+ FakeGPGHandlerNotFound(fingerprint), IGPGHandler))
128+ user = self.factory.makePerson()
129+ self.assertEqual(
130+ "No GPG key found with the given content: %s" % fingerprint,
131+ getUtility(ISignedCodeOfConductSet).verifyAndStore(
132+ user, "signed data"))
133+
134+ def test_verifyAndStore_unregistered(self):
135+ fingerprint = "0" * 40
136+ signature = PymeSignature(fingerprint, "plain data")
137+ self.useFixture(ZopeUtilityFixture(
138+ FakeGPGHandlerGood(signature), IGPGHandler))
139+ user = self.factory.makePerson()
140+ self.assertThat(
141+ getUtility(ISignedCodeOfConductSet).verifyAndStore(
142+ user, "signed data"),
143+ MatchesRegex(r"^The key you used.*is not registered"))
144+
145+ def test_verifyAndStore_wrong_owner(self):
146+ other_user = self.factory.makePerson()
147+ gpgkey = self.factory.makeGPGKey(other_user)
148+ signature = PymeSignature(gpgkey.fingerprint, "plain data")
149+ self.useFixture(ZopeUtilityFixture(
150+ FakeGPGHandlerGood(signature), IGPGHandler))
151+ user = self.factory.makePerson()
152+ self.assertThat(
153+ getUtility(ISignedCodeOfConductSet).verifyAndStore(
154+ user, "signed data"),
155+ MatchesRegex(r"^You.*do not seem to be the owner"))
156+
157+ def test_verifyAndStore_deactivated(self):
158+ user = self.factory.makePerson()
159+ gpgkey = self.factory.makeGPGKey(user)
160+ gpgkey.active = False
161+ signature = PymeSignature(gpgkey.fingerprint, "plain data")
162+ self.useFixture(ZopeUtilityFixture(
163+ FakeGPGHandlerGood(signature), IGPGHandler))
164+ self.assertThat(
165+ getUtility(ISignedCodeOfConductSet).verifyAndStore(
166+ user, "signed data"),
167+ MatchesRegex(r"^The OpenPGP key used.*has been deactivated"))
168+
169+ def test_verifyAndStore_bad_plain_data(self):
170+ user = self.factory.makePerson()
171+ gpgkey = self.factory.makeGPGKey(user)
172+ signature = PymeSignature(gpgkey.fingerprint, "plain data")
173+ self.useFixture(ZopeUtilityFixture(
174+ FakeGPGHandlerGood(signature), IGPGHandler))
175+ self.assertThat(
176+ getUtility(ISignedCodeOfConductSet).verifyAndStore(
177+ user, "signed data"),
178+ MatchesRegex(
179+ r"^The signed text does not match the Code of Conduct"))
180+
181+ def test_verifyAndStore_good(self):
182+ user = self.factory.makePerson()
183+ gpgkey = self.factory.makeGPGKey(user)
184+ signature = PymeSignature(
185+ gpgkey.fingerprint,
186+ getUtility(ICodeOfConductSet).current_code_of_conduct.content)
187+ self.useFixture(ZopeUtilityFixture(
188+ FakeGPGHandlerGood(signature), IGPGHandler))
189+ self.assertIsNone(
190+ getUtility(ISignedCodeOfConductSet).verifyAndStore(
191+ user, "signed data"))
192+ [notification] = self.assertEmailQueueLength(1)
193+ self.assertThat(dict(notification), ContainsDict({
194+ "From": Equals(format_address(
195+ "Launchpad Code Of Conduct System",
196+ config.canonical.noreply_from_address)),
197+ "To": Equals(user.preferredemail.email),
198+ "Subject": Equals(
199+ "Your Code of Conduct signature has been acknowledged"),
200+ }))
201+ self.assertEqual(
202+ dedent("""\
203+
204+ Hello
205+
206+ Your Code of Conduct Signature was modified.
207+
208+ User: '%(user)s'
209+ Digitally Signed by %(fingerprint)s
210+
211+
212+ Thanks,
213+
214+ The Launchpad Team
215+ """) % {
216+ 'user': user.display_name,
217+ 'fingerprint': gpgkey.fingerprint,
218+ },
219+ notification.get_payload(decode=True))
220
221=== modified file 'lib/lp/services/gpg/doc/gpg-signatures.txt'
222--- lib/lp/services/gpg/doc/gpg-signatures.txt 2019-03-07 15:05:17 +0000
223+++ lib/lp/services/gpg/doc/gpg-signatures.txt 2019-08-23 19:42:27 +0000
224@@ -42,7 +42,7 @@
225 ... -----END PGP SIGNATURE-----
226 ... """
227
228- >>> master_sig = gpghandler.verifySignature(content)
229+ >>> master_sig = gpghandler.getVerifiedSignature(content)
230 >>> print master_sig.fingerprint
231 A419AE861E88BC9E04B9C26FBA2B9389DFD20543
232
233@@ -62,7 +62,7 @@
234 ... """
235 >>>
236
237- >>> subkey_sig = gpghandler.verifySignature(content)
238+ >>> subkey_sig = gpghandler.getVerifiedSignature(content)
239 >>> print subkey_sig.fingerprint
240 A419AE861E88BC9E04B9C26FBA2B9389DFD20543
241
242
243=== modified file 'lib/lp/services/gpg/handler.py'
244--- lib/lp/services/gpg/handler.py 2019-03-22 19:22:54 +0000
245+++ lib/lp/services/gpg/handler.py 2019-08-23 19:42:27 +0000
246@@ -130,15 +130,6 @@
247 if os.path.exists(filename):
248 os.remove(filename)
249
250- def verifySignature(self, content, signature=None):
251- """See IGPGHandler."""
252- try:
253- return self.getVerifiedSignature(content, signature)
254- except (GPGVerificationError, GPGKeyExpired):
255- # Swallow GPG verification errors
256- pass
257- return None
258-
259 def getVerifiedSignatureResilient(self, content, signature=None):
260 """See IGPGHandler."""
261 errors = []
262
263=== modified file 'lib/lp/services/gpg/interfaces.py'
264--- lib/lp/services/gpg/interfaces.py 2019-03-22 19:22:54 +0000
265+++ lib/lp/services/gpg/interfaces.py 2019-08-23 19:42:27 +0000
266@@ -223,13 +223,6 @@
267 If the firgerprint cannot be sanitized return None.
268 """
269
270- def verifySignature(content, signature=None):
271- """See `getVerifiedSignature`.
272-
273- Suppress all exceptions and simply return None if the could not
274- be verified.
275- """
276-
277 def getURLForKeyInServer(fingerprint, action=None, public=False):
278 """Return the URL for that fingerprint on the configured keyserver.
279
280@@ -263,6 +256,7 @@
281
282 :raise GPGVerificationError: if the signature cannot be verified.
283 :raise GPGKeyExpired: if the signature was made with an expired key.
284+ :raise GPGKeyNotFoundError: if the key was not found on the keyserver.
285 :return: a `PymeSignature` object.
286 """
287
288
289=== modified file 'lib/lp/services/mail/incoming.py'
290--- lib/lp/services/mail/incoming.py 2019-07-25 15:00:18 +0000
291+++ lib/lp/services/mail/incoming.py 2019-08-23 19:42:27 +0000
292@@ -1,4 +1,4 @@
293-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
294+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
295 # GNU Affero General Public License version 3 (see the file LICENSE).
296
297 """Functions dealing with mails coming into Launchpad."""
298@@ -26,6 +26,8 @@
299 from lp.registry.interfaces.person import IPerson
300 from lp.services.features import getFeatureFlag
301 from lp.services.gpg.interfaces import (
302+ GPGKeyExpired,
303+ GPGKeyNotFoundError,
304 GPGVerificationError,
305 IGPGHandler,
306 )
307@@ -314,8 +316,8 @@
308 sig = gpghandler.getVerifiedSignature(
309 canonicalise_line_endings(mail.signedContent), signature)
310 log.debug("got signature %r" % sig)
311- except GPGVerificationError as e:
312- # verifySignature failed to verify the signature.
313+ except (GPGVerificationError, GPGKeyExpired, GPGKeyNotFoundError) as e:
314+ # getVerifiedSignature failed to verify the signature.
315 message = "Signature couldn't be verified: %s" % e
316 log.debug(message)
317 raise InvalidSignature(message)
318
319=== modified file 'lib/lp/services/mail/tests/test_incoming.py'
320--- lib/lp/services/mail/tests/test_incoming.py 2016-03-23 17:55:39 +0000
321+++ lib/lp/services/mail/tests/test_incoming.py 2019-08-23 19:42:27 +0000
322@@ -17,6 +17,11 @@
323 from zope.security.management import setSecurityPolicy
324
325 from lp.services.config import config
326+from lp.services.gpg.interfaces import (
327+ GPGKeyExpired,
328+ GPGKeyNotFoundError,
329+ IGPGHandler,
330+ )
331 from lp.services.log.logger import BufferLogger
332 from lp.services.mail import helpers
333 from lp.services.mail.handlers import mail_handlers
334@@ -34,6 +39,7 @@
335 from lp.testing import TestCaseWithFactory
336 from lp.testing.dbuser import switch_dbuser
337 from lp.testing.factory import GPGSigningContext
338+from lp.testing.fixture import ZopeUtilityFixture
339 from lp.testing.gpgkeys import import_secret_test_key
340 from lp.testing.layers import LaunchpadZopelessLayer
341 from lp.testing.mail_helpers import pop_notifications
342@@ -52,6 +58,30 @@
343 return True
344
345
346+class FakePymeKey:
347+
348+ def __init__(self, fingerprint):
349+ self.fingerprint = fingerprint
350+
351+
352+class FakeGPGHandlerExpired:
353+
354+ def __init__(self, key):
355+ self.key = key
356+
357+ def getVerifiedSignature(self, content, signature=None):
358+ raise GPGKeyExpired(self.key)
359+
360+
361+class FakeGPGHandlerNotFound:
362+
363+ def __init__(self, fingerprint):
364+ self.fingerprint = fingerprint
365+
366+ def getVerifiedSignature(self, content, signature=None):
367+ raise GPGKeyNotFoundError(self.fingerprint)
368+
369+
370 class IncomingTestCase(TestCaseWithFactory):
371
372 layer = LaunchpadZopelessLayer
373@@ -86,6 +116,72 @@
374 "(7, 58, u'No data')",
375 body)
376
377+ def test_expired_key(self):
378+ """An expired key should not be handled as an OOPS.
379+
380+ It should produce a message explaining to the user what went wrong.
381+ """
382+ person = self.factory.makePerson()
383+ key = FakePymeKey("0" * 40)
384+ self.useFixture(ZopeUtilityFixture(
385+ FakeGPGHandlerExpired(key), IGPGHandler))
386+ transaction.commit()
387+ email_address = person.preferredemail.email
388+ invalid_body = (
389+ '-----BEGIN PGP SIGNED MESSAGE-----\n'
390+ 'Hash: SHA1\n\n'
391+ 'Body\n'
392+ '-----BEGIN PGP SIGNATURE-----\n'
393+ 'Not a signature.\n'
394+ '-----END PGP SIGNATURE-----\n')
395+ ctrl = MailController(
396+ email_address, 'to@example.com', 'subject', invalid_body,
397+ bulk=False)
398+ ctrl.send()
399+ handleMail()
400+ self.assertEqual([], self.oopses)
401+ [notification] = pop_notifications()
402+ body = notification.get_payload()[0].get_payload(decode=True)
403+ self.assertIn(
404+ "An error occurred while processing a mail you sent to "
405+ "Launchpad's email\ninterface.\n\n\n"
406+ "Error message:\n\nSignature couldn't be verified: "
407+ "%s\nhas expired" % key.fingerprint,
408+ body)
409+
410+ def test_key_not_found(self):
411+ """A key not found on the keyserver should not be handled as an OOPS.
412+
413+ It should produce a message explaining to the user what went wrong.
414+ """
415+ person = self.factory.makePerson()
416+ fingerprint = "0" * 40
417+ self.useFixture(ZopeUtilityFixture(
418+ FakeGPGHandlerNotFound(fingerprint), IGPGHandler))
419+ transaction.commit()
420+ email_address = person.preferredemail.email
421+ invalid_body = (
422+ '-----BEGIN PGP SIGNED MESSAGE-----\n'
423+ 'Hash: SHA1\n\n'
424+ 'Body\n'
425+ '-----BEGIN PGP SIGNATURE-----\n'
426+ 'Not a signature.\n'
427+ '-----END PGP SIGNATURE-----\n')
428+ ctrl = MailController(
429+ email_address, 'to@example.com', 'subject', invalid_body,
430+ bulk=False)
431+ ctrl.send()
432+ handleMail()
433+ self.assertEqual([], self.oopses)
434+ [notification] = pop_notifications()
435+ body = notification.get_payload()[0].get_payload(decode=True)
436+ self.assertIn(
437+ "An error occurred while processing a mail you sent to "
438+ "Launchpad's email\ninterface.\n\n\n"
439+ "Error message:\n\nSignature couldn't be verified: "
440+ "No GPG key found with the given content:\n%s" % fingerprint,
441+ body)
442+
443 def test_mail_too_big(self):
444 """Much-too-big mail should generate a bounce, not an OOPS.
445
446
447=== modified file 'lib/lp/soyuz/doc/fakepackager.txt'
448--- lib/lp/soyuz/doc/fakepackager.txt 2019-04-28 17:14:55 +0000
449+++ lib/lp/soyuz/doc/fakepackager.txt 2019-08-23 19:42:27 +0000
450@@ -172,7 +172,7 @@
451 >>> from zope.component import getUtility
452 >>> from lp.services.gpg.interfaces import IGPGHandler
453 >>> gpghandler = getUtility(IGPGHandler)
454- >>> sig = gpghandler.verifySignature(content)
455+ >>> sig = gpghandler.getVerifiedSignature(content)
456
457 >>> sig.fingerprint == packager.gpg_key_fingerprint[2:]
458 True
459
460=== modified file 'lib/lp/testing/fixture.py'
461--- lib/lp/testing/fixture.py 2019-05-22 14:57:45 +0000
462+++ lib/lp/testing/fixture.py 2019-08-23 19:42:27 +0000
463@@ -13,6 +13,7 @@
464 'Urllib2Fixture',
465 'ZopeAdapterFixture',
466 'ZopeEventHandlerFixture',
467+ 'ZopeUtilityFixture',
468 'ZopeViewReplacementFixture',
469 ]
470