Launchpad itself

Merge lp:~benji/launchpad/bug-580035 into lp:launchpad

  1. bug-580035
  2. Merge into devel
Proposed by Benji York on 2010-08-27
Status: Merged
Approved by: Abel Deuring on 2010-08-27
Approved revision: no longer in the source branch.
Merged at revision: 11466
Proposed branch: lp:~benji/launchpad/bug-580035
Merge into: lp:launchpad
Diff against target: 343 lines (+65/-68)
7 files modified
lib/canonical/launchpad/interfaces/gpghandler.py (+4/-3)
lib/canonical/launchpad/mail/handlers.py (+15/-2)
lib/canonical/launchpad/mail/helpers.py (+3/-3)
lib/canonical/launchpad/mail/tests/test_handlers.py (+17/-33)
lib/canonical/launchpad/mail/tests/test_helpers.py (+4/-14)
lib/canonical/launchpad/utilities/gpghandler.py (+6/-3)
lib/lp/bugs/tests/bugs-emailinterface.txt (+16/-10)
To merge this branch: bzr merge lp:~benji/launchpad/bug-580035
Reviewer Review Type Date Requested Status
Abel Deuring (community) code 2010-08-27 Approve on 2010-08-27
Review via email: mp+33911@code.launchpad.net

Description of the Change

During QA this branch caused exceptions when processing GPG signed
emails.

I found that my understanding of the data structures involved was faulty
so the tests I wrote were not representative. I reworked the tests to
be realistic and added the data I needed to fix the bug to
PymeSignature.

You can run all the email tests that pertain to authentication that I
could find with this command:

    bin/test -ct emailauthentication.txt -t bugs-emailinterface.txt \
        -t signed_messages -t test_handlers -t test_helpers

To post a comment you must log in.
Abel Deuring (adeuring) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/interfaces/gpghandler.py'
2--- lib/canonical/launchpad/interfaces/gpghandler.py 2010-08-20 20:31:18 +0000
3+++ lib/canonical/launchpad/interfaces/gpghandler.py 2010-08-27 13:45:57 +0000
4@@ -38,7 +38,7 @@
5 self.fingerprint = fingerprint
6 self.pubkey = pubkey
7 super(GPGKeyNotFoundError, self).__init__(
8- "No GPG key found with the given content: %s" % (fingerprint,))
9+ "No GPG key found with the given content: %s" % (fingerprint, ))
10
11
12 class GPGKeyRevoked(Exception):
13@@ -47,7 +47,7 @@
14 def __init__(self, key):
15 self.key = key
16 super(GPGKeyRevoked, self).__init__(
17- "%s has been publicly revoked" % (key.keyid,))
18+ "%s has been publicly revoked" % (key.keyid, ))
19
20
21 class GPGKeyExpired(Exception):
22@@ -55,7 +55,7 @@
23
24 def __init__(self, key):
25 self.key = key
26- super(GPGKeyExpired, self).__init__("%s has expired" % (key.keyid,))
27+ super(GPGKeyExpired, self).__init__("%s has expired" % (key.keyid, ))
28
29
30 class SecretGPGKeyImportDetected(Exception):
31@@ -275,6 +275,7 @@
32
33 fingerprint = Attribute("Signer Fingerprint.")
34 plain_data = Attribute("Plain Signed Text.")
35+ timestamp = Attribute("The time at which the message was signed.")
36
37
38 class IPymeKey(Interface):
39
40=== modified file 'lib/canonical/launchpad/mail/handlers.py'
41--- lib/canonical/launchpad/mail/handlers.py 2010-08-20 20:31:18 +0000
42+++ lib/canonical/launchpad/mail/handlers.py 2010-08-27 13:45:57 +0000
43@@ -32,6 +32,7 @@
44 ISpecificationSet,
45 QuestionStatus,
46 )
47+from canonical.launchpad.interfaces.gpghandler import IGPGHandler
48 from canonical.launchpad.mail.commands import (
49 BugEmailCommands,
50 get_error_message,
51@@ -58,6 +59,16 @@
52 )
53
54
55+def extract_signature_timestamp(signed_msg):
56+ # break import cycle
57+ from canonical.launchpad.mail.incoming import (
58+ canonicalise_line_endings)
59+ signature = getUtility(IGPGHandler).getVerifiedSignature(
60+ canonicalise_line_endings(signed_msg.signedContent),
61+ signed_msg.signature)
62+ return signature.timestamp
63+
64+
65 class MaloneHandler:
66 """Handles emails sent to Malone.
67
68@@ -77,7 +88,8 @@
69 name, args in parse_commands(content,
70 BugEmailCommands.names())]
71
72- def process(self, signed_msg, to_addr, filealias=None, log=None):
73+ def process(self, signed_msg, to_addr, filealias=None, log=None,
74+ extract_signature_timestamp=extract_signature_timestamp):
75 """See IMailHandler."""
76 commands = self.getCommands(signed_msg)
77 user, host = to_addr.split('@')
78@@ -89,7 +101,8 @@
79 CONTEXT = 'bug report'
80 ensure_not_weakly_authenticated(signed_msg, CONTEXT)
81 if signature is not None:
82- ensure_sane_signature_timestamp(signature, CONTEXT)
83+ ensure_sane_signature_timestamp(
84+ extract_signature_timestamp(signed_msg), CONTEXT)
85
86 if user.lower() == 'new':
87 # A submit request.
88
89=== modified file 'lib/canonical/launchpad/mail/helpers.py'
90--- lib/canonical/launchpad/mail/helpers.py 2010-08-20 20:31:18 +0000
91+++ lib/canonical/launchpad/mail/helpers.py 2010-08-27 13:45:57 +0000
92@@ -230,7 +230,7 @@
93 raise IncomingEmailError(error_message)
94
95
96-def ensure_sane_signature_timestamp(signature, context,
97+def ensure_sane_signature_timestamp(timestamp, context,
98 error_template='old-signature.txt'):
99 """Ensure the signature was generated recently but not in the future."""
100 fourty_eight_hours = 48 * 60 * 60
101@@ -239,7 +239,7 @@
102 fourty_eight_hours_ago = now - fourty_eight_hours
103 ten_minutes_in_the_future = now + ten_minutes
104
105- if (signature.timestamp < fourty_eight_hours_ago
106- or signature.timestamp > ten_minutes_in_the_future):
107+ if (timestamp < fourty_eight_hours_ago
108+ or timestamp > ten_minutes_in_the_future):
109 error_message = get_error_message(error_template, context=context)
110 raise IncomingEmailError(error_message)
111
112=== modified file 'lib/canonical/launchpad/mail/tests/test_handlers.py'
113--- lib/canonical/launchpad/mail/tests/test_handlers.py 2010-08-20 20:31:18 +0000
114+++ lib/canonical/launchpad/mail/tests/test_handlers.py 2010-08-27 13:45:57 +0000
115@@ -9,6 +9,7 @@
116 import unittest
117
118 from canonical.database.sqlbase import commit
119+from canonical.launchpad.ftests import import_secret_test_key
120 from canonical.launchpad.mail.commands import BugEmailCommand
121 from canonical.launchpad.mail.handlers import MaloneHandler
122 from canonical.testing import LaunchpadFunctionalLayer
123@@ -17,6 +18,7 @@
124 person_logged_in,
125 TestCaseWithFactory,
126 )
127+from lp.testing.factory import GPGSigningContext
128
129
130 class TestMaloneHandler(TestCaseWithFactory):
131@@ -68,39 +70,21 @@
132
133 layer = LaunchpadFunctionalLayer
134
135- def test_far_past_gpg_signature_timestamp(self):
136- # If an email message's GPG signature's timestamp is too far in the
137- # past and the message contains a command, the command will fail and
138- # send the user an email telling them what happened.
139-
140- msg = self.factory.makeSignedMessage(body=' security no')
141- now = time.time()
142- one_week = 60 * 60 * 24 * 7
143- msg.signature = FakeSignature(timestamp=now-one_week)
144- handler = MaloneHandler()
145- with person_logged_in(self.factory.makePerson()):
146- success = handler.process(msg, msg['To'])
147- commit()
148- email = get_last_email()
149- self.assertEqual(email['subject'], 'Submit Request Failure')
150- self.assertIn(BAD_SIGNATURE_TIMESTAMP_MESSAGE, email['body'])
151-
152- def test_far_future_gpg_signature_timestamp(self):
153- # If an email message's GPG signature's timestamp is too far in the
154- # future and the message contains a command, the command will fail and
155- # send the user an email telling them what happened.
156-
157- msg = self.factory.makeSignedMessage(body=' security no')
158- now = time.time()
159- one_week = 60 * 60 * 24 * 7
160- msg.signature = FakeSignature(timestamp=now+one_week)
161- handler = MaloneHandler()
162- with person_logged_in(self.factory.makePerson()):
163- success = handler.process(msg, msg['To'])
164- commit()
165- email = get_last_email()
166- self.assertEqual(email['subject'], 'Submit Request Failure')
167- self.assertIn(BAD_SIGNATURE_TIMESTAMP_MESSAGE, email['body'])
168+ def test_good_signature_timestamp(self):
169+ # An email message's GPG signature's timestamp checked to be sure it
170+ # isn't too far in the future or past. This test shows that a
171+ # signature with a timestamp of appxoimately now will be accepted.
172+ signing_context = GPGSigningContext(
173+ import_secret_test_key().fingerprint, password='test')
174+ msg = self.factory.makeSignedMessage(
175+ body=' security no', signing_context=signing_context)
176+ handler = MaloneHandler()
177+ with person_logged_in(self.factory.makePerson()):
178+ success = handler.process(msg, msg['To'])
179+ commit()
180+ # Since there were no commands in the poorly-timestamped message, no
181+ # error emails were generated.
182+ self.assertEqual(stub.test_emails, [])
183
184 def test_bad_timestamp_but_no_commands(self):
185 # If an email message's GPG signature's timestamp is too far in the
186
187=== modified file 'lib/canonical/launchpad/mail/tests/test_helpers.py'
188--- lib/canonical/launchpad/mail/tests/test_helpers.py 2010-08-20 20:31:18 +0000
189+++ lib/canonical/launchpad/mail/tests/test_helpers.py 2010-08-27 13:45:57 +0000
190@@ -91,12 +91,6 @@
191 parse_commands(' command:', ['command']))
192
193
194-class FakeSignature:
195-
196- def __init__(self, timestamp):
197- self.timestamp = timestamp
198-
199-
200 class TestEnsureSaneSignatureTimestamp(unittest.TestCase):
201 """Tests for ensure_sane_signature_timestamp"""
202
203@@ -104,35 +98,31 @@
204 # signature timestamps shouldn't be too old
205 now = time.time()
206 one_week = 60 * 60 * 24 * 7
207- signature = FakeSignature(timestamp=now-one_week)
208 self.assertRaises(
209 IncomingEmailError, ensure_sane_signature_timestamp,
210- signature, 'bug report')
211+ now-one_week, 'bug report')
212
213 def test_future_timestamp(self):
214 # signature timestamps shouldn't be (far) in the future
215 now = time.time()
216 one_week = 60 * 60 * 24 * 7
217- signature = FakeSignature(timestamp=now+one_week)
218 self.assertRaises(
219 IncomingEmailError, ensure_sane_signature_timestamp,
220- signature, 'bug report')
221+ now+one_week, 'bug report')
222
223 def test_near_future_timestamp(self):
224 # signature timestamps in the near future are OK
225 now = time.time()
226 one_minute = 60
227- signature = FakeSignature(timestamp=now+one_minute)
228 # this should not raise an exception
229- ensure_sane_signature_timestamp(signature, 'bug report')
230+ ensure_sane_signature_timestamp(now+one_minute, 'bug report')
231
232 def test_recent_timestamp(self):
233 # signature timestamps in the recent past are OK
234 now = time.time()
235 one_hour = 60 * 60
236- signature = FakeSignature(timestamp=now-one_hour)
237 # this should not raise an exception
238- ensure_sane_signature_timestamp(signature, 'bug report')
239+ ensure_sane_signature_timestamp(now-one_hour, 'bug report')
240
241
242 class TestEnsureNotWeaklyAuthenticated(TestCaseWithFactory):
243
244=== modified file 'lib/canonical/launchpad/utilities/gpghandler.py'
245--- lib/canonical/launchpad/utilities/gpghandler.py 2010-08-20 20:31:18 +0000
246+++ lib/canonical/launchpad/utilities/gpghandler.py 2010-08-27 13:45:57 +0000
247@@ -218,8 +218,10 @@
248 "Unable to map subkey: %s" % signature.fpr)
249
250 # return the signature container
251- return PymeSignature(fingerprint=key.fingerprint,
252- plain_data=plain.getvalue())
253+ return PymeSignature(
254+ fingerprint=key.fingerprint,
255+ plain_data=plain.getvalue(),
256+ timestamp=signature.timestamp)
257
258 def importPublicKey(self, content):
259 """See IGPGHandler."""
260@@ -550,10 +552,11 @@
261 """See IPymeSignature."""
262 implements(IPymeSignature)
263
264- def __init__(self, fingerprint=None, plain_data=None):
265+ def __init__(self, fingerprint=None, plain_data=None, timestamp=None):
266 """Initialized a signature container."""
267 self.fingerprint = fingerprint
268 self.plain_data = plain_data
269+ self.timestamp = timestamp
270
271
272 class PymeKey:
273
274=== modified file 'lib/lp/bugs/tests/bugs-emailinterface.txt'
275--- lib/lp/bugs/tests/bugs-emailinterface.txt 2010-08-18 18:23:37 +0000
276+++ lib/lp/bugs/tests/bugs-emailinterface.txt 2010-08-27 13:45:57 +0000
277@@ -50,16 +50,11 @@
278 signed, so that the system can verify the sender. But to avoid having
279 to sign each email, we'll create a class which fakes a signed email:
280
281- >>> import time
282- >>> class MockSignature(object):
283- ... def __init__(self):
284- ... self.timestamp = time.time()
285-
286 >>> import email.Message
287 >>> class MockSignedMessage(email.Message.Message):
288 ... def __init__(self, *args, **kws):
289 ... email.Message.Message.__init__(self, *args, **kws)
290- ... self.signature = MockSignature()
291+ ... self.signature = 'fake'
292 ... @property
293 ... def signedMessage(self):
294 ... return self
295@@ -82,9 +77,14 @@
296 ... msg['Message-Id'] = factory.makeUniqueRFC822MsgId()
297 ... return msg
298
299+ >>> import time
300+ >>> def fake_extract_signature_timestamp(signed_msg):
301+ ... return time.time()
302+
303 >>> def process_email(raw_mail):
304 ... msg = construct_email(raw_mail)
305- ... handler.process(msg, msg['To'])
306+ ... handler.process(msg, msg['To'],
307+ ... extract_signature_timestamp=fake_extract_signature_timestamp)
308
309 >>> process_email(submit_mail)
310
311@@ -410,7 +410,9 @@
312 ... signature = None
313 >>> msg = email.message_from_string(
314 ... comment_mail, _class=MockUnsignedMessage)
315- >>> handler.process(msg, msg['To'])
316+ >>> handler.process(
317+ ... msg, msg['To'],
318+ ... extract_signature_timestamp=fake_extract_signature_timestamp)
319 True
320 >>> commit()
321
322@@ -482,7 +484,9 @@
323 ... return construct_email(edit_mail)
324
325 >>> def submit_command_email(msg):
326- ... handler.process(msg, msg['To'])
327+ ... handler.process(
328+ ... msg, msg['To'],
329+ ... extract_signature_timestamp=fake_extract_signature_timestamp)
330 ... commit()
331 ... sync(bug)
332
333@@ -1756,7 +1760,9 @@
334 >>> msg = signed_message_from_string(submit_mail)
335 >>> import email.Utils
336 >>> msg['Message-Id'] = email.Utils.make_msgid()
337- >>> handler.process(msg, msg['To'])
338+ >>> handler.process(
339+ ... msg, msg['To'],
340+ ... extract_signature_timestamp=fake_extract_signature_timestamp)
341 True
342 >>> print_latest_email()
343 Subject: Submit Request Failure