Launchpad itself

Merge lp:~mbp/launchpad/dkim into lp:launchpad/db-devel

  1. dkim
  2. Merge into db-devel
Proposed by Martin Pool
Status: Superseded
Proposed branch: lp:~mbp/launchpad/dkim
Merge into: lp:launchpad/db-devel
Diff against target: 1080 lines (+288/-135)
13 files modified
lib/canonical/buildd/test_buildd_recipe (+1/-1)
lib/canonical/launchpad/doc/emailauthentication.txt (+16/-10)
lib/canonical/launchpad/mail/handlers.py (+55/-49)
lib/canonical/launchpad/mail/helpers.py (+20/-2)
lib/canonical/launchpad/mail/tests/test_handlers.py (+61/-0)
lib/lp/bugs/tests/bugs-emailinterface.txt (+38/-35)
lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+13/-13)
lib/lp/code/interfaces/sourcepackagerecipe.py (+1/-1)
lib/lp/code/model/tests/test_recipebuilder.py (+6/-4)
lib/lp/services/mail/incoming.py (+28/-5)
lib/lp/services/mail/tests/incomingmail.txt (+24/-13)
lib/lp/services/mail/tests/test_incoming.py (+24/-1)
utilities/migrater/file-ownership.txt (+1/-1)
To merge this branch: bzr merge lp:~mbp/launchpad/dkim
Reviewer Review Type Date Requested Status
Jonathan Lange (community) Approve
Graham Binns (community) code Approve
Launchpad code reviewers Pending
Review via email: mp+35985@code.launchpad.net

This proposal has been superseded by a proposal from 2010-11-24.

Commit message

[bug=316272,643200,643219] [r=gmb,jml][ui=none][bug=316272,643219] Make DKIM authentication actually work

Description of the change

This fixes a few more things related to authentication of incoming mail:

 * gpg signature timestamp checks should cover all mail, not just that to malone (bug 643200)

 * checks on mail to new@ should make sure it's strongly authenticated, not specifically that it has a gpg signature (bug 643219)

 * there was no test that gpg mail with implausible timestamps was actually rejected afaics

I haven't interactively tested this and I'm not utterly confident in the existing test coverage, so please review carefully and test it interactively yourself if you know how.

MaloneHandler.process was a bit large so I split out the code that decides what if any commands will be executed.

Thanks

To post a comment you must log in.
Revision history for this message
Māris Fogels (mars) wrote :

Hi Martin,

I asked Deryck about who may have domain expertise in this:

<mars> deryck, I was wondering if someone on your team would have knowledge in that domain that could help Martin out?
<deryck> mars, perhaps gmb or allenap. But the code team should have some expertise there, too. abentley wrote BaseMailer, I believe.
<mars> deryck, great. thank you for the help
<deryck> np

I suggest you ping one of the people above. They are on either side of your day.

Revision history for this message
Jonathan Lange (jml) wrote :

Hey Martin,

I like the refactoring that extracts extractAndAuthenticateCommands.

As a general point, we insist on capitalized sentences with full stops in our comments and docstrings.

I'm finding it hard to grok test_NonGPGNewBug. Your comment there is helpful, but I think it would help me trust the test more if there were reasons for why you aren't doing the normal thing and why you are logging in directly instead.

Also, you'll need to change all of the first person references. Nothing more frustrating to wonder who "I" is.

I also don't know how to make a GPG signature with crazy timestamps. It would be nice to do that instead of checking that the checker is called. The test you've got is OK to land though.

jml

Revision history for this message
Jonathan Lange (jml) wrote :

I was going to say, I think I'd appreciate a second review from someone who knows mail (or bug mail) better before this lands.

review: Approve
Revision history for this message
Martin Pool (mbp) wrote :

I'll add this text, is it more clear:

        """Mail authenticated other than by gpg can create bugs.

        The incoming mail layer is responsible for authenticating the mail,
        and setting the current principal to the sender of the mail, either
        weakly or non-weakly authenticated. At the layer of the handler,
        which this class is testing, we shouldn't care by what mechanism we
        decided to act on behalf of the mail sender, only that we did.

        In bug 643219, Launchpad had a problem where the MaloneHandler code
        was puncturing that abstraction and directly looking at the GPG
        signature; this test checks it's fixed.
        """

Revision history for this message
Graham Binns (gmb) wrote :

Sorry Martin, I didn't get time to review this today. I'll try to take
a look at it tomorrow morning.

--
Graham Binns
http://grahambinns.com

Revision history for this message
Graham Binns (gmb) wrote :

I'm happy for this to land (apologies for taking so long to review it). I'm not convinced that I'm competent enough to test this interactively locally, so I suggest that we get the LP team to give the tyres a good kicking when it gets onto staging instead.

review: Approve (code)
Revision history for this message
Martin Pool (mbp) wrote :

@gmb Could you please land this for me? Thanks.

Revision history for this message
Graham Binns (gmb) wrote :

On 29 September 2010 03:31, Martin Pool <email address hidden> wrote:
> @gmb Could you please land this for me?  Thanks.

Sure thing.

Revision history for this message
Martin Pool (mbp) wrote :
Download full text (6.3 KiB)

OK, this is now updated with the following changes based on the full-testsuite failure, and the tests in bugs-emailinterface.txt now pass.

 * There is a lack of coherence between some things checking for a .signature and some checking the current security principal. the email doctests had some trouble with this; specifically they didn't make the principal weaklyauthenticated before testing that condition. This is now fixed.

 * Typically for doctests, state is run together across tests, so I cleared the mail queue so we'd get a more obvious failure.

 * The not-gpg-signed error template was misnamed and will become more so in future, so I renamed it. The error is not so much about being unsigned, but being unauthenticated when trying to create a new bug.

Here's the incremental diff:

=== renamed file 'lib/canonical/launchpad/mail/errortemplates/not-gpg-signed.txt' => 'lib/canonical/launchpad/mail/errortemplates/unauthenticated-bug-creation.txt'
=== modified file 'lib/canonical/launchpad/mail/handlers.py'
--- lib/canonical/launchpad/mail/handlers.py 2010-09-21 05:21:10 +0000
+++ lib/canonical/launchpad/mail/handlers.py 2010-10-01 08:16:02 +0000
@@ -94,12 +94,15 @@
         commands = self.getCommands(signed_msg)
         to_user, to_host = to_addr.split('@')
         add_comment_to_bug = False
+ if to_user.lower() == 'new':
+ # Special failure message for unauthenticated attempts to create
+ # new bugs.
+ ensure_not_weakly_authenticated(signed_msg, CONTEXT,
+ 'unauthenticated-bug-creation.txt')
         if len(commands) > 0:
             ensure_not_weakly_authenticated(signed_msg, CONTEXT)
         if to_user.lower() == 'new':
             # A submit request.
- ensure_not_weakly_authenticated(signed_msg, CONTEXT,
- 'not-gpg-signed.txt')
             commands.insert(0, BugEmailCommands.get('bug', ['new']))
         elif to_user.isdigit():
             # A comment to a bug. We set add_comment_to_bug to True so

=== modified file 'lib/canonical/launchpad/mail/helpers.py'
--- lib/canonical/launchpad/mail/helpers.py 2010-09-20 06:40:41 +0000
+++ lib/canonical/launchpad/mail/helpers.py 2010-10-01 08:16:02 +0000
@@ -211,7 +211,11 @@
 def ensure_not_weakly_authenticated(signed_msg, context,
                                     error_template='not-signed.txt',
                                     no_key_template='key-not-registered.txt'):
- """Make sure that the current principal is not weakly authenticated."""
+ """Make sure that the current principal is not weakly authenticated.
+
+ NB: This mixes checking properties of the message with properties of the
+ current principal; it's assumed that this message was just received.
+ """
     cur_principal = get_current_principal()
     # The security machinery doesn't know about
     # IWeaklyAuthenticatedPrincipal yet, so do a manual

=== modified file 'lib/lp/bugs/tests/bugs-emailinterface.txt'
--- lib/lp/bugs/tests/bugs-emailinterface.txt 2010-10-01 07:07:03 +0000
+++ lib/lp/bugs/tests/bugs-emailinterface.txt 2010-10-01 08:16:02 +0000
@@ -339,13 +339,15 @@
     ... IWeaklyAuthenticatedPrincipal)
...

Read more...

Revision history for this message
Martin Pool (mbp) wrote :

I'll run the tests for this here; if I could get an additional review that would be nice.

Revision history for this message
Martin Pool (mbp) wrote :

There are further failures in emailauthentication.txt, apparently.

I tried running ./bin/test mail but that didn't seem to get this before.

Revision history for this message
Jonathan Lange (jml) wrote :
Download full text (4.1 KiB)

On Fri, Oct 1, 2010 at 9:20 AM, Martin Pool <email address hidden> wrote:
> OK, this is now updated with the following changes based on the full-testsuite failure, and the tests in bugs-emailinterface.txt now pass.
>
>  * There is a lack of coherence between some things checking for a .signature and some checking the current security principal.  the email doctests had some trouble with this; specifically they didn't make the principal weaklyauthenticated before testing that condition.  This is now fixed.
>
>  * Typically for doctests, state is run together across tests, so I cleared the mail queue so we'd get a more obvious failure.
>
>  * The not-gpg-signed error template was misnamed and will become more so in future, so I renamed it.  The error is not so much about being unsigned, but being unauthenticated when trying to create a new bug.
>
> Here's the incremental diff:
>

Thanks for persisting with this. There are a couple of points where
I'm not clear on what's going on and a couple of minor tweaks.

> === renamed file 'lib/canonical/launchpad/mail/errortemplates/not-gpg-signed.txt' => 'lib/canonical/launchpad/mail/errortemplates/unauthenticated-bug-creation.txt'
> === modified file 'lib/canonical/launchpad/mail/handlers.py'
> --- lib/canonical/launchpad/mail/handlers.py    2010-09-21 05:21:10 +0000
> +++ lib/canonical/launchpad/mail/handlers.py    2010-10-01 08:16:02 +0000
> @@ -94,12 +94,15 @@
>         commands = self.getCommands(signed_msg)
>         to_user, to_host = to_addr.split('@')
>         add_comment_to_bug = False
> +        if to_user.lower() == 'new':
> +            # Special failure message for unauthenticated attempts to create
> +            # new bugs.
> +            ensure_not_weakly_authenticated(signed_msg, CONTEXT,
> +                'unauthenticated-bug-creation.txt')

Why do you need to two clauses for "to_user.lower() == 'new'"? I can't
figure it out from the code. Perhaps there should be a comment.

>         if len(commands) > 0:
>             ensure_not_weakly_authenticated(signed_msg, CONTEXT)
>         if to_user.lower() == 'new':
>             # A submit request.
> -            ensure_not_weakly_authenticated(signed_msg, CONTEXT,
> -                'not-gpg-signed.txt')
>             commands.insert(0, BugEmailCommands.get('bug', ['new']))
>         elif to_user.isdigit():
>             # A comment to a bug. We set add_comment_to_bug to True so
>

> === modified file 'lib/canonical/launchpad/mail/helpers.py'
> --- lib/canonical/launchpad/mail/helpers.py     2010-09-20 06:40:41 +0000
> +++ lib/canonical/launchpad/mail/helpers.py     2010-10-01 08:16:02 +0000
> @@ -211,7 +211,11 @@
>  def ensure_not_weakly_authenticated(signed_msg, context,
>                                     error_template='not-signed.txt',
>                                     no_key_template='key-not-registered.txt'):
> -    """Make sure that the current principal is not weakly authenticated."""
> +    """Make sure that the current principal is not weakly authenticated.
> +
> +    NB: This mixes checking properties of the message with properties of the
> +    current principal; it's assumed that this message was just received.
> +    "...

Read more...

Revision history for this message
Jonathan Lange (jml) wrote :

See recent comments.

review: Needs Fixing
Revision history for this message
Martin Pool (mbp) wrote :
Download full text (5.2 KiB)

On 1 October 2010 23:33, Jonathan Lange <email address hidden> wrote:
> On Fri, Oct 1, 2010 at 9:20 AM, Martin Pool <email address hidden> wrote:
>> OK, this is now updated with the following changes based on the full-testsuite failure, and the tests in bugs-emailinterface.txt now pass.
>>
>>  * There is a lack of coherence between some things checking for a .signature and some checking the current security principal.  the email doctests had some trouble with this; specifically they didn't make the principal weaklyauthenticated before testing that condition.  This is now fixed.
>>
>>  * Typically for doctests, state is run together across tests, so I cleared the mail queue so we'd get a more obvious failure.
>>
>>  * The not-gpg-signed error template was misnamed and will become more so in future, so I renamed it.  The error is not so much about being unsigned, but being unauthenticated when trying to create a new bug.
>>
>> Here's the incremental diff:
>>
>
> Thanks for persisting with this. There are a couple of points where
> I'm not clear on what's going on and a couple of minor tweaks.
>
>> === renamed file 'lib/canonical/launchpad/mail/errortemplates/not-gpg-signed.txt' => 'lib/canonical/launchpad/mail/errortemplates/unauthenticated-bug-creation.txt'
>> === modified file 'lib/canonical/launchpad/mail/handlers.py'
>> --- lib/canonical/launchpad/mail/handlers.py    2010-09-21 05:21:10 +0000
>> +++ lib/canonical/launchpad/mail/handlers.py    2010-10-01 08:16:02 +0000
>> @@ -94,12 +94,15 @@
>>         commands = self.getCommands(signed_msg)
>>         to_user, to_host = to_addr.split('@')
>>         add_comment_to_bug = False
>> +        if to_user.lower() == 'new':
>> +            # Special failure message for unauthenticated attempts to create
>> +            # new bugs.
>> +            ensure_not_weakly_authenticated(signed_msg, CONTEXT,
>> +                'unauthenticated-bug-creation.txt')
>
> Why do you need to two clauses for "to_user.lower() == 'new'"? I can't
> figure it out from the code. Perhaps there should be a comment.

I'll add a comment. It's because we need to check for authentication
on mails that either attempt to create a new bug or that have commands
that modify an existing bug, and we give a different error in either
case. After doing this, we must check the recipient address is
recognized, and that again includes the case of 'new@'.

Perhaps I should try harder to refactor this...?

>>         if len(commands) > 0:
>>             ensure_not_weakly_authenticated(signed_msg, CONTEXT)
>>         if to_user.lower() == 'new':
>>             # A submit request.
>> -            ensure_not_weakly_authenticated(signed_msg, CONTEXT,
>> -                'not-gpg-signed.txt')
>>             commands.insert(0, BugEmailCommands.get('bug', ['new']))
>>         elif to_user.isdigit():
>>             # A comment to a bug. We set add_comment_to_bug to True so
>>
>
>
>> === modified file 'lib/canonical/launchpad/mail/helpers.py'
>> --- lib/canonical/launchpad/mail/helpers.py     2010-09-20 06:40:41 +0000
>> +++ lib/canonical/launchpad/mail/helpers.py     2010-10-01 08:16:02 +0000

>> @@ -211,7 +211,11 @@
>>  def ensure_not_weakly_authenticat...

Read more...

Revision history for this message
Martin Pool (mbp) wrote :
Download full text (3.4 KiB)

On 8 October 2010 12:19, Martin Pool <email address hidden> wrote:
> On 1 October 2010 23:33, Jonathan Lange <email address hidden> wrote:
>> On Fri, Oct 1, 2010 at 9:20 AM, Martin Pool <email address hidden> wrote:

>>> === renamed file 'lib/canonical/launchpad/mail/errortemplates/not-gpg-signed.txt' => 'lib/canonical/launchpad/mail/errortemplates/unauthenticated-bug-creation.txt'
>>> === modified file 'lib/canonical/launchpad/mail/handlers.py'
>>> --- lib/canonical/launchpad/mail/handlers.py    2010-09-21 05:21:10 +0000
>>> +++ lib/canonical/launchpad/mail/handlers.py    2010-10-01 08:16:02 +0000
>>> @@ -94,12 +94,15 @@
>>>         commands = self.getCommands(signed_msg)
>>>         to_user, to_host = to_addr.split('@')
>>>         add_comment_to_bug = False
>>> +        if to_user.lower() == 'new':
>>> +            # Special failure message for unauthenticated attempts to create
>>> +            # new bugs.
>>> +            ensure_not_weakly_authenticated(signed_msg, CONTEXT,
>>> +                'unauthenticated-bug-creation.txt')
>>
>> Why do you need to two clauses for "to_user.lower() == 'new'"? I can't
>> figure it out from the code. Perhaps there should be a comment.
>
> I'll add a comment.  It's because we need to check for authentication
> on mails that either attempt to create a new bug or that have commands
> that modify an existing bug, and we give a different error in either
> case.   After doing this, we must check the recipient address is
> recognized, and that again includes the case of 'new@'.
>
> Perhaps I should try harder to refactor this...?

I think it's a bit simpler now. You could tear it apart entirely
which could be nice but this branch is old...

>>> @@ -211,7 +211,11 @@
>>>  def ensure_not_weakly_authenticated(signed_msg, context,
>>>                                     error_template='not-signed.txt',
>>>                                     no_key_template='key-not-registered.txt'):
>>> -    """Make sure that the current principal is not weakly authenticated."""
>>> +    """Make sure that the current principal is not weakly authenticated.
>>> +
>>> +    NB: This mixes checking properties of the message with properties of the
>>> +    current principal; it's assumed that this message was just received.
>>> +    """
>>
>> I can't say the expansion clears things up much for me.

Cleaner like this:

def ensure_not_weakly_authenticated(signed_msg, context,
                                    error_template='not-signed.txt',
                                    no_key_template='key-not-registered.txt'):
    """Make sure that the current principal is not weakly authenticated.

    NB: While handling an email, the authentication state is stored partly in
    properties of the message object, and partly in the current security
    principal. As a consequence this function will only work correctly if the
    message has just been passed through authenticateEmail -- you can't give
    it an arbitrary message object.
    """

>> Could you please use lp.testing.sampledata.USER_EMAIL instead of
>> '<email address hidden>'? We're trying to kill off some of our more
>> gratuitous magic literals.
>
> sure, thanks

I've removed all the occurr...

Read more...

Revision history for this message
Jonathan Lange (jml) wrote :

Thanks Martin.

From your comments, things look good, but I don't see any new revisions pushed up. As soon as you'll push them up I'll give them the once over and probably approve for landing.

Revision history for this message
Martin Pool (mbp) wrote :

> Thanks Martin.
>
> From your comments, things look good, but I don't see any new revisions pushed
> up. As soon as you'll push them up I'll give them the once over and probably
> approve for landing.

I don't know why they didn't show up in the merge proposal discussion thread on the web page, but the new revisions (from Friday the 15th) are in the branch, are visible in 'unmerged revisions' and in the diff.

Revision history for this message
Jonathan Lange (jml) :
review: Approve
Revision history for this message
Martin Pool (mbp) wrote :

Looking at it again, I'm not sure if the handling of new@ is really correct,
but the tests should tell us.

- Martin

On 19/10/2010 7:38 PM, "Jonathan Lange" <email address hidden> wrote:

Review: Approve

--
https://code.launchpad.net/~mbp/launchpad/dkim/+merge/35985
You are the owner of lp:~mbp/launch...

Revision history for this message
Jonathan Lange (jml) wrote :

Martin, what's the next step here?

Revision history for this message
Martin Pool (mbp) wrote :

On 5 November 2010 05:35, Jonathan Lange <email address hidden> wrote:
> Martin, what's the next step here?

I would like someone to review the currently proposed changes, and if
they are acceptable to sponsor landing. Thanks.

--
Martin

Revision history for this message
Jonathan Lange (jml) wrote :

I approved on 2010-10-19. Have there been changes since? If not, I'm happy to land this for you.

Revision history for this message
Martin Pool (mbp) wrote :

On 7 November 2010 22:34, Jonathan Lange <email address hidden> wrote:
> I approved on 2010-10-19. Have there been changes since? If not, I'm happy to land this for you.

Just confirming what I said on irc: there are no changes since. I'm
going to run the full test suite it in a VM, but if you could also
submit it in parallel with that, I'd appreciate it. If it fails,
please let me know.

--
Martin

Revision history for this message
Martin Pool (mbp) wrote :

My attempt to test it was thwarted by bug 629746 causing 'make check'
to fail. I'll try again with bin/test.

Revision history for this message
Jonathan Lange (jml) wrote :

Conflicts again!

Text conflict in lib/canonical/launchpad/mail/handlers.py
Text conflict in lib/lp/bugs/tests/bugs-emailinterface.txt
Text conflict in lib/lp/services/mail/tests/test_incoming.py

I can't run the tests on ec2 until they are fixed. Sorry.

Revision history for this message
Jonathan Lange (jml) wrote :

Still waiting for conflict resolution.

Revision history for this message
Martin Pool (mbp) wrote :

OK, now resolved, and I'm trying to persuade the tests to run.

Revision history for this message
Martin Pool (mbp) wrote :

This apparently has some doctest failures:

Failure in test lib/lp/services/mail/tests/incomingmail.txt
Traceback (most recent call last):
  File "/usr/lib/python2.6/unittest.py", line 279, in run
    testMethod()
  File "/usr/lib/python2.6/doctest.py", line 2152, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for incomingmail.txt
  File "lib/lp/services/mail/tests/incomingmail.txt", line 0

----------------------------------------------------------------------
File "lib/lp/services/mail/tests/incomingmail.txt", line 90, in incomingmail.txt
Failed example:
    handleMail(zopeless_transaction)
Differences (ndiff with -expected +actual):
    - WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...
    - WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...
    - WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...
    - WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...
    + ERROR:canonical.launchpad.mail:An exception was raised inside the handler:
    + http://localhost:58000/93/c2ab70bc-f39e-11df-af8f-52540048778d.txt
    + Traceback (most recent call last):
    + File "/home/mbp/launchpad/lp-branches/work/lib/lp/services/mail/incoming.py", line 370, in handleMail
    + principal = authenticateEmail(mail)
    + File "/home/mbp/launchpad/lp-branches/work/lib/lp/services/mail/incoming.py", line 221, in authenticateEmail
    + 'incoming mail verification')
    + File "/home/mbp/launchpad/lp-branches/work/lib/canonical/launchpad/mail/helpers.py", line 255, in ensure_sane_signature_timestamp
    + raise IncomingEmailError(error_message)
    + IncomingEmailError: The message you sent included commands to modify the incoming mail verification, but the
    + signature was (apparently) generated too far in the past or future.
    + <BLANKLINE>
....

Revision history for this message
Martin Pool (mbp) wrote :

More failures in http://sourcefrog.net/tmp/1y/launchpad-dkim-test-failures.txt

Revision history for this message
Jonathan Lange (jml) wrote :

I ran the tests against ec2 – you should have received the email – and have got the same errors.

Do you need help debugging them?

Revision history for this message
Martin Pool (mbp) wrote :

On 21 November 2010 05:28, Jonathan Lange <email address hidden> wrote:
> I ran the tests against ec2 – you should have received the email – and have got the same errors.
>
> Do you need help debugging them?

Thanks, I think I'm ok. Will try to do it Monday.

--
Martin

Revision history for this message
Martin Pool (mbp) wrote :
Download full text (14.7 KiB)

Thanks for all your patience and help. The updated branch now passes at least the two doctests that were identified as failing. The specific change is that I've gone back to the approach of passing in the timestamp-checking callback, but moved it to a more appropriate spot that (I think) covers all cases. This is imo cleaner than monkey patching, as I was doing.

I'm thinking of also retargeting this to devel. It has no database dependencies.

=== modified file 'lib/canonical/launchpad/doc/emailauthentication.txt'
--- lib/canonical/launchpad/doc/emailauthentication.txt 2010-10-18 22:24:59 +0000
+++ lib/canonical/launchpad/doc/emailauthentication.txt 2010-11-24 06:49:53 +0000
@@ -20,12 +20,18 @@
     >>> commit()
     >>> LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)

+For most of these tests, we don't care whether the timestamps are out of
+date:
+
+ >>> def accept_any_timestamp(timestamp, context_message):
+ ... pass
+
 Now Sample Person and Foo Bar have one OpenPGP key each. Next, let's get
 a test email that's signed and try to authenticate the user who sent it:

     >>> from canonical.launchpad.mail.ftests import read_test_message
     >>> msg = read_test_message('signed_detached.txt')
- >>> principal = authenticateEmail(msg)
+ >>> principal = authenticateEmail(msg, accept_any_timestamp)

 If the user isn't registered in Launchpad, None is return, if it
 succeeds the authenticated principal:
@@ -52,7 +58,7 @@
 message. Inline signatures are supported as well.

     >>> msg = read_test_message('signed_inline.txt')
- >>> principal = authenticateEmail(msg)
+ >>> principal = authenticateEmail(msg, accept_any_timestamp)
     >>> principal is not None
     True
     >>> name, addr = email.Utils.parseaddr(msg['From'])
@@ -65,7 +71,7 @@
 As well as signed multipart messages:

     >>> msg = read_test_message('signed_multipart.txt')
- >>> principal = authenticateEmail(msg)
+ >>> principal = authenticateEmail(msg, accept_any_timestamp)
     >>> principal is not None
     True
     >>> name, addr = email.Utils.parseaddr(msg['From'])
@@ -80,7 +86,7 @@
 to deal with it if we receive a dash escaped message.

     >>> msg = read_test_message('signed_dash_escaped.txt')
- >>> principal = authenticateEmail(msg)
+ >>> principal = authenticateEmail(msg, accept_any_timestamp)
     >>> principal is not None
     True
     >>> name, addr = email.Utils.parseaddr(msg['From'])
@@ -96,7 +102,7 @@
     >>> msg = read_test_message('signed_detached_invalid_signature.txt')
     >>> name, addr = email.Utils.parseaddr(msg['From'])
     >>> from_user = getUtility(IPersonSet).getByEmail(addr)
- >>> principal = authenticateEmail(msg)
+ >>> principal = authenticateEmail(msg, accept_any_timestamp)
     Traceback (most recent call last):
       ...
     InvalidSignature:...
@@ -131,7 +137,7 @@
     ... msg = email.message_from_string(
     ... line_ending.join(msg_lines), _class=SignedMessage)
     ... msg.parsed_string = msg.as_string()
- ... principal = authenticateEmail(msg)
+ ... principal = authenticateEmail(msg, accept_any_timestamp)
     ... authenticated_person = IPers...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/buildd/test_buildd_recipe'
--- lib/canonical/buildd/test_buildd_recipe 2010-07-23 16:05:55 +0000
+++ lib/canonical/buildd/test_buildd_recipe 2010-11-24 07:21:59 +0000
@@ -8,7 +8,7 @@
8country_code = 'us'8country_code = 'us'
9apt_cacher_ng_host = 'stumpy'9apt_cacher_ng_host = 'stumpy'
10distroseries_name = 'maverick'10distroseries_name = 'maverick'
11recipe_text = """# bzr-builder format 0.2 deb-version 0+{revno}11recipe_text = """# bzr-builder format 0.2 deb-version {debupstream}-0~{revno}
12http://bazaar.launchpad.dev/~ppa-user/+junk/wakeonlan"""12http://bazaar.launchpad.dev/~ppa-user/+junk/wakeonlan"""
1313
14def deb_line(host, suites):14def deb_line(host, suites):
1515
=== modified file 'lib/canonical/launchpad/doc/emailauthentication.txt'
--- lib/canonical/launchpad/doc/emailauthentication.txt 2010-10-18 22:24:59 +0000
+++ lib/canonical/launchpad/doc/emailauthentication.txt 2010-11-24 07:21:59 +0000
@@ -20,12 +20,18 @@
20 >>> commit()20 >>> commit()
21 >>> LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)21 >>> LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)
2222
23For most of these tests, we don't care whether the timestamps are out of
24date:
25
26 >>> def accept_any_timestamp(timestamp, context_message):
27 ... pass
28
23Now Sample Person and Foo Bar have one OpenPGP key each. Next, let's get29Now Sample Person and Foo Bar have one OpenPGP key each. Next, let's get
24a test email that's signed and try to authenticate the user who sent it:30a test email that's signed and try to authenticate the user who sent it:
2531
26 >>> from canonical.launchpad.mail.ftests import read_test_message32 >>> from canonical.launchpad.mail.ftests import read_test_message
27 >>> msg = read_test_message('signed_detached.txt')33 >>> msg = read_test_message('signed_detached.txt')
28 >>> principal = authenticateEmail(msg)34 >>> principal = authenticateEmail(msg, accept_any_timestamp)
2935
30If the user isn't registered in Launchpad, None is return, if it36If the user isn't registered in Launchpad, None is return, if it
31succeeds the authenticated principal:37succeeds the authenticated principal:
@@ -52,7 +58,7 @@
52message. Inline signatures are supported as well.58message. Inline signatures are supported as well.
5359
54 >>> msg = read_test_message('signed_inline.txt')60 >>> msg = read_test_message('signed_inline.txt')
55 >>> principal = authenticateEmail(msg)61 >>> principal = authenticateEmail(msg, accept_any_timestamp)
56 >>> principal is not None62 >>> principal is not None
57 True63 True
58 >>> name, addr = email.Utils.parseaddr(msg['From'])64 >>> name, addr = email.Utils.parseaddr(msg['From'])
@@ -65,7 +71,7 @@
65As well as signed multipart messages:71As well as signed multipart messages:
6672
67 >>> msg = read_test_message('signed_multipart.txt')73 >>> msg = read_test_message('signed_multipart.txt')
68 >>> principal = authenticateEmail(msg)74 >>> principal = authenticateEmail(msg, accept_any_timestamp)
69 >>> principal is not None75 >>> principal is not None
70 True76 True
71 >>> name, addr = email.Utils.parseaddr(msg['From'])77 >>> name, addr = email.Utils.parseaddr(msg['From'])
@@ -80,7 +86,7 @@
80to deal with it if we receive a dash escaped message.86to deal with it if we receive a dash escaped message.
8187
82 >>> msg = read_test_message('signed_dash_escaped.txt')88 >>> msg = read_test_message('signed_dash_escaped.txt')
83 >>> principal = authenticateEmail(msg)89 >>> principal = authenticateEmail(msg, accept_any_timestamp)
84 >>> principal is not None90 >>> principal is not None
85 True91 True
86 >>> name, addr = email.Utils.parseaddr(msg['From'])92 >>> name, addr = email.Utils.parseaddr(msg['From'])
@@ -96,7 +102,7 @@
96 >>> msg = read_test_message('signed_detached_invalid_signature.txt')102 >>> msg = read_test_message('signed_detached_invalid_signature.txt')
97 >>> name, addr = email.Utils.parseaddr(msg['From'])103 >>> name, addr = email.Utils.parseaddr(msg['From'])
98 >>> from_user = getUtility(IPersonSet).getByEmail(addr)104 >>> from_user = getUtility(IPersonSet).getByEmail(addr)
99 >>> principal = authenticateEmail(msg)105 >>> principal = authenticateEmail(msg, accept_any_timestamp)
100 Traceback (most recent call last):106 Traceback (most recent call last):
101 ...107 ...
102 InvalidSignature:...108 InvalidSignature:...
@@ -131,7 +137,7 @@
131 ... msg = email.message_from_string(137 ... msg = email.message_from_string(
132 ... line_ending.join(msg_lines), _class=SignedMessage)138 ... line_ending.join(msg_lines), _class=SignedMessage)
133 ... msg.parsed_string = msg.as_string()139 ... msg.parsed_string = msg.as_string()
134 ... principal = authenticateEmail(msg)140 ... principal = authenticateEmail(msg, accept_any_timestamp)
135 ... authenticated_person = IPerson(principal)141 ... authenticated_person = IPerson(principal)
136 ... print authenticated_person.preferredemail.email142 ... print authenticated_person.preferredemail.email
137 test@canonical.com143 test@canonical.com
@@ -156,7 +162,7 @@
156 Content-Type: multipart/mixed; boundary="--------------------EuxKj2iCbKjpUGkD"162 Content-Type: multipart/mixed; boundary="--------------------EuxKj2iCbKjpUGkD"
157 ...163 ...
158164
159 >>> principal = authenticateEmail(msg)165 >>> principal = authenticateEmail(msg, accept_any_timestamp)
160 >>> print IPerson(principal).displayname166 >>> print IPerson(principal).displayname
161 Sample Person167 Sample Person
162168
@@ -178,7 +184,7 @@
178 >>> from canonical.launchpad.interfaces.mail import (184 >>> from canonical.launchpad.interfaces.mail import (
179 ... IWeaklyAuthenticatedPrincipal)185 ... IWeaklyAuthenticatedPrincipal)
180 >>> msg = read_test_message('unsigned_multipart.txt')186 >>> msg = read_test_message('unsigned_multipart.txt')
181 >>> principal = authenticateEmail(msg)187 >>> principal = authenticateEmail(msg, accept_any_timestamp)
182 >>> IWeaklyAuthenticatedPrincipal.providedBy(principal)188 >>> IWeaklyAuthenticatedPrincipal.providedBy(principal)
183 True189 True
184190
@@ -191,7 +197,7 @@
191authenticated user:197authenticated user:
192198
193 >>> msg = read_test_message('signed_key_not_registered.txt')199 >>> msg = read_test_message('signed_key_not_registered.txt')
194 >>> principal = authenticateEmail(msg)200 >>> principal = authenticateEmail(msg, accept_any_timestamp)
195 >>> IWeaklyAuthenticatedPrincipal.providedBy(principal)201 >>> IWeaklyAuthenticatedPrincipal.providedBy(principal)
196 True202 True
197203
@@ -205,7 +211,7 @@
205principal.211principal.
206212
207 >>> msg = read_test_message('signed_inline.txt')213 >>> msg = read_test_message('signed_inline.txt')
208 >>> principal = authenticateEmail(msg)214 >>> principal = authenticateEmail(msg, accept_any_timestamp)
209 >>> IWeaklyAuthenticatedPrincipal.providedBy(principal)215 >>> IWeaklyAuthenticatedPrincipal.providedBy(principal)
210 False216 False
211217
212218
=== renamed file 'lib/canonical/launchpad/mail/errortemplates/not-gpg-signed.txt' => 'lib/canonical/launchpad/mail/errortemplates/unauthenticated-bug-creation.txt'
=== modified file 'lib/canonical/launchpad/mail/handlers.py'
--- lib/canonical/launchpad/mail/handlers.py 2010-11-08 13:11:30 +0000
+++ lib/canonical/launchpad/mail/handlers.py 2010-11-24 07:21:59 +0000
@@ -15,7 +15,6 @@
15from canonical.config import config15from canonical.config import config
16from canonical.database.sqlbase import rollback16from canonical.database.sqlbase import rollback
17from canonical.launchpad.helpers import get_email_template17from canonical.launchpad.helpers import get_email_template
18from canonical.launchpad.interfaces.gpghandler import IGPGHandler
19from canonical.launchpad.interfaces.mail import (18from canonical.launchpad.interfaces.mail import (
20 EmailProcessingError,19 EmailProcessingError,
21 IBugEditEmailCommand,20 IBugEditEmailCommand,
@@ -31,7 +30,6 @@
31 )30 )
32from canonical.launchpad.mail.helpers import (31from canonical.launchpad.mail.helpers import (
33 ensure_not_weakly_authenticated,32 ensure_not_weakly_authenticated,
34 ensure_sane_signature_timestamp,
35 get_main_body,33 get_main_body,
36 guess_bugtask,34 guess_bugtask,
37 IncomingEmailError,35 IncomingEmailError,
@@ -61,16 +59,6 @@
61 )59 )
6260
6361
64def extract_signature_timestamp(signed_msg):
65 # break import cycle
66 from lp.services.mail.incoming import (
67 canonicalise_line_endings)
68 signature = getUtility(IGPGHandler).getVerifiedSignature(
69 canonicalise_line_endings(signed_msg.signedContent),
70 signed_msg.signature)
71 return signature.timestamp
72
73
74class MaloneHandler:62class MaloneHandler:
75 """Handles emails sent to Malone.63 """Handles emails sent to Malone.
7664
@@ -90,47 +78,65 @@
90 name, args in parse_commands(content,78 name, args in parse_commands(content,
91 BugEmailCommands.names())]79 BugEmailCommands.names())]
9280
93 def process(self, signed_msg, to_addr, filealias=None, log=None,81 def extractAndAuthenticateCommands(self, signed_msg, to_addr):
94 extract_signature_timestamp=extract_signature_timestamp):82 """Extract commands and handle special destinations.
95 """See IMailHandler."""83
84 NB: The authentication is carried out against the current principal,
85 not directly against the message. authenticateEmail must previously
86 have been called on this thread.
87
88 :returns: (final_result, add_comment_to_bug, commands)
89 If final_result is non-none, stop processing and return this value
90 to indicate whether the message was dealt with or not.
91 If add_comment_to_bug, add the contents to the first bug
92 selected.
93 commands is a list of bug commands.
94 """
95 CONTEXT = 'bug report'
96 commands = self.getCommands(signed_msg)96 commands = self.getCommands(signed_msg)
97 user, host = to_addr.split('@')97 to_user, to_host = to_addr.split('@')
98 add_comment_to_bug = False98 add_comment_to_bug = False
99 signature = signed_msg.signature99 if len(commands) > 0:
100 # If there are any commands, we must have strong authentication.
101 # We send a different failure message for attempts to create a new
102 # bug.
103 if to_user.lower() == 'new':
104 ensure_not_weakly_authenticated(signed_msg, CONTEXT,
105 'unauthenticated-bug-creation.txt')
106 else:
107 ensure_not_weakly_authenticated(signed_msg, CONTEXT)
108 if to_user.lower() == 'new':
109 commands.insert(0, BugEmailCommands.get('bug', ['new']))
110 elif to_user.isdigit():
111 # A comment to a bug. We set add_comment_to_bug to True so
112 # that the comment gets added to the bug later. We don't add
113 # the comment now, since we want to let the 'bug' command
114 # handle the possible errors that can occur while getting
115 # the bug.
116 add_comment_to_bug = True
117 commands.insert(0, BugEmailCommands.get('bug', [to_user]))
118 elif to_user.lower() == 'help':
119 from_user = getUtility(ILaunchBag).user
120 if from_user is not None:
121 preferredemail = from_user.preferredemail
122 if preferredemail is not None:
123 to_address = str(preferredemail.email)
124 self.sendHelpEmail(to_address)
125 return True, False, None
126 elif to_user.lower() != 'edit':
127 # Indicate that we didn't handle the mail.
128 return False, False, None
129 return None, add_comment_to_bug, commands
130
131 def process(self, signed_msg, to_addr, filealias=None, log=None):
132 """See IMailHandler."""
100133
101 try:134 try:
102 if len(commands) > 0:135 (final_result, add_comment_to_bug,
103 CONTEXT = 'bug report'136 commands, ) = self.extractAndAuthenticateCommands(
104 ensure_not_weakly_authenticated(signed_msg, CONTEXT)137 signed_msg, to_addr)
105 if signature is not None:138 if final_result is not None:
106 ensure_sane_signature_timestamp(139 return final_result
107 extract_signature_timestamp(signed_msg), CONTEXT)
108
109 if user.lower() == 'new':
110 # A submit request.
111 commands.insert(0, BugEmailCommands.get('bug', ['new']))
112 if signature is None:
113 raise IncomingEmailError(
114 get_error_message('not-gpg-signed.txt'))
115 elif user.isdigit():
116 # A comment to a bug. We set add_comment_to_bug to True so
117 # that the comment gets added to the bug later. We don't add
118 # the comment now, since we want to let the 'bug' command
119 # handle the possible errors that can occur while getting
120 # the bug.
121 add_comment_to_bug = True
122 commands.insert(0, BugEmailCommands.get('bug', [user]))
123 elif user.lower() == 'help':
124 from_user = getUtility(ILaunchBag).user
125 if from_user is not None:
126 preferredemail = from_user.preferredemail
127 if preferredemail is not None:
128 to_address = str(preferredemail.email)
129 self.sendHelpEmail(to_address)
130 return True
131 elif user.lower() != 'edit':
132 # Indicate that we didn't handle the mail.
133 return False
134140
135 bug = None141 bug = None
136 bug_event = None142 bug_event = None
137143
=== modified file 'lib/canonical/launchpad/mail/helpers.py'
--- lib/canonical/launchpad/mail/helpers.py 2010-11-08 13:11:30 +0000
+++ lib/canonical/launchpad/mail/helpers.py 2010-11-24 07:21:59 +0000
@@ -211,7 +211,14 @@
211def ensure_not_weakly_authenticated(signed_msg, context,211def ensure_not_weakly_authenticated(signed_msg, context,
212 error_template='not-signed.txt',212 error_template='not-signed.txt',
213 no_key_template='key-not-registered.txt'):213 no_key_template='key-not-registered.txt'):
214 """Make sure that the current principal is not weakly authenticated."""214 """Make sure that the current principal is not weakly authenticated.
215
216 NB: While handling an email, the authentication state is stored partly in
217 properties of the message object, and partly in the current security
218 principal. As a consequence this function will only work correctly if the
219 message has just been passed through authenticateEmail -- you can't give
220 it an arbitrary message object.
221 """
215 cur_principal = get_current_principal()222 cur_principal = get_current_principal()
216 # The security machinery doesn't know about223 # The security machinery doesn't know about
217 # IWeaklyAuthenticatedPrincipal yet, so do a manual224 # IWeaklyAuthenticatedPrincipal yet, so do a manual
@@ -232,7 +239,18 @@
232239
233def ensure_sane_signature_timestamp(timestamp, context,240def ensure_sane_signature_timestamp(timestamp, context,
234 error_template='old-signature.txt'):241 error_template='old-signature.txt'):
235 """Ensure the signature was generated recently but not in the future."""242 """Ensure the signature was generated recently but not in the future.
243
244 If the timestamp is wrong, the message is rejected rather than just
245 treated as untrusted, so that the user gets a chance to understand
246 the problem. Therefore, this raises an error rather than returning
247 a value.
248
249 :param context: Short user-readable description of the place
250 the problem occurred.
251 :raises IncomingEmailError: if the timestamp is stale or implausible,
252 containing a message based on the context and template.
253 """
236 fourty_eight_hours = 48 * 60 * 60254 fourty_eight_hours = 48 * 60 * 60
237 ten_minutes = 10 * 60255 ten_minutes = 10 * 60
238 now = time.time()256 now = time.time()
239257
=== modified file 'lib/canonical/launchpad/mail/tests/test_handlers.py'
--- lib/canonical/launchpad/mail/tests/test_handlers.py 2010-10-26 15:47:24 +0000
+++ lib/canonical/launchpad/mail/tests/test_handlers.py 2010-11-24 07:21:59 +0000
@@ -6,6 +6,7 @@
6from doctest import DocTestSuite6from doctest import DocTestSuite
7import email7import email
8import time8import time
9import transaction
9import unittest10import unittest
1011
11from canonical.database.sqlbase import commit12from canonical.database.sqlbase import commit
@@ -42,6 +43,66 @@
42 self.assertEqual('bug', commands[0].name)43 self.assertEqual('bug', commands[0].name)
43 self.assertEqual(['foo'], commands[0].string_args)44 self.assertEqual(['foo'], commands[0].string_args)
4445
46 def test_NonGPGAuthenticatedNewBug(self):
47 """Mail authenticated other than by gpg can create bugs.
48
49 The incoming mail layer is responsible for authenticating the mail,
50 and setting the current principal to the sender of the mail, either
51 weakly or non-weakly authenticated. At the layer of the handler,
52 which this class is testing, we shouldn't care by what mechanism we
53 decided to act on behalf of the mail sender, only that we did.
54
55 In bug 643219, Launchpad had a problem where the MaloneHandler code
56 was puncturing that abstraction and directly looking at the GPG
57 signature; this test checks it's fixed.
58 """
59 # NB SignedMessage by default isn't actually signed, it just has the
60 # capability of knowing about signing.
61 message = self.factory.makeSignedMessage(body=' affects malone\nhi!')
62 self.assertEquals(message.signature, None)
63
64 # Pretend that the mail auth has given us a logged-in user.
65 handler = MaloneHandler()
66 with person_logged_in(self.factory.makePerson()):
67 mail_handled, add_comment_to_bug, commands = \
68 handler.extractAndAuthenticateCommands(message,
69 'new@bugs.launchpad.net')
70 self.assertEquals(mail_handled, None)
71 self.assertEquals(map(str, commands), [
72 'bug new',
73 'affects malone',
74 ])
75
76 def test_mailToHelpFromUnknownUser(self):
77 """Mail from people of no account to help@ is simply dropped.
78 """
79 message = self.factory.makeSignedMessage()
80 handler = MaloneHandler()
81 mail_handled, add_comment_to_bug, commands = \
82 handler.extractAndAuthenticateCommands(message,
83 'help@bugs.launchpad.net')
84 self.assertEquals(mail_handled, True)
85 self.assertEquals(self.getSentMail(), [])
86
87 def test_mailToHelp(self):
88 """Mail to help@ generates a help command."""
89 message = self.factory.makeSignedMessage()
90 handler = MaloneHandler()
91 with person_logged_in(self.factory.makePerson()):
92 mail_handled, add_comment_to_bug, commands = \
93 handler.extractAndAuthenticateCommands(message,
94 'help@bugs.launchpad.net')
95 self.assertEquals(mail_handled, True)
96 self.assertEquals(len(self.getSentMail()), 1)
97 # TODO: Check the right mail was sent. -- mbp 20100923
98
99 def getSentMail(self):
100 # Sending mail is (unfortunately) a side effect of parsing the
101 # commands, and unfortunately you must commit the transaction to get
102 # them sent.
103 transaction.commit()
104 return stub.test_emails[:]
105
45106
46class FakeSignature:107class FakeSignature:
47108
48109
=== modified file 'lib/lp/bugs/tests/bugs-emailinterface.txt'
--- lib/lp/bugs/tests/bugs-emailinterface.txt 2010-11-05 14:17:11 +0000
+++ lib/lp/bugs/tests/bugs-emailinterface.txt 2010-11-24 07:21:59 +0000
@@ -50,6 +50,8 @@
50signed, so that the system can verify the sender. But to avoid having50signed, so that the system can verify the sender. But to avoid having
51to sign each email, we'll create a class which fakes a signed email:51to sign each email, we'll create a class which fakes a signed email:
5252
53 >>> from lp.testing import sampledata
54
53 >>> import email.Message55 >>> import email.Message
54 >>> class MockSignedMessage(email.Message.Message):56 >>> class MockSignedMessage(email.Message.Message):
55 ... def __init__(self, *args, **kws):57 ... def __init__(self, *args, **kws):
@@ -77,14 +79,10 @@
77 ... msg['Message-Id'] = factory.makeUniqueRFC822MsgId()79 ... msg['Message-Id'] = factory.makeUniqueRFC822MsgId()
78 ... return msg80 ... return msg
7981
80 >>> import time
81 >>> def fake_extract_signature_timestamp(signed_msg):
82 ... return time.time()
83
84 >>> def process_email(raw_mail):82 >>> def process_email(raw_mail):
85 ... msg = construct_email(raw_mail)83 ... msg = construct_email(raw_mail)
86 ... handler.process(msg, msg['To'],84 ... handler.process(msg, msg['To'],
87 ... extract_signature_timestamp=fake_extract_signature_timestamp)85 ... )
8886
89 >>> process_email(submit_mail)87 >>> process_email(submit_mail)
9088
@@ -156,7 +154,7 @@
156If we would file a bug on Ubuntu instead, we would submit a mail like154If we would file a bug on Ubuntu instead, we would submit a mail like
157this:155this:
158156
159 >>> login('test@canonical.com')157 >>> login(sampledata.USER_EMAIL)
160 >>> submit_mail = """From: Sample Person <test@canonical.com>158 >>> submit_mail = """From: Sample Person <test@canonical.com>
161 ... To: new@bugs.canonical.com159 ... To: new@bugs.canonical.com
162 ... Date: Fri Jun 17 10:20:23 BST 2005160 ... Date: Fri Jun 17 10:20:23 BST 2005
@@ -342,13 +340,15 @@
342 >>> from canonical.launchpad.interfaces.mail import IWeaklyAuthenticatedPrincipal340 >>> from canonical.launchpad.interfaces.mail import IWeaklyAuthenticatedPrincipal
343 >>> from zope.interface import directlyProvides, directlyProvidedBy341 >>> from zope.interface import directlyProvides, directlyProvidedBy
344 >>> from zope.security.management import queryInteraction342 >>> from zope.security.management import queryInteraction
345 >>> participations = queryInteraction().participations343
346 >>> len(participations)344 >>> def simulate_receiving_untrusted_mail():
347 1345 ... participations = queryInteraction().participations
348 >>> current_principal = participations[0].principal346 ... assert len(participations) == 1
349 >>> directlyProvides(347 ... current_principal = participations[0].principal
350 ... current_principal, directlyProvidedBy(current_principal),348 ... directlyProvides(
351 ... IWeaklyAuthenticatedPrincipal)349 ... current_principal, directlyProvidedBy(current_principal),
350 ... IWeaklyAuthenticatedPrincipal)
351 >>> simulate_receiving_untrusted_mail()
352352
353Now we send a comment containing commands.353Now we send a comment containing commands.
354354
@@ -385,6 +385,8 @@
385385
386 >>> def print_latest_email():386 >>> def print_latest_email():
387 ... commit()387 ... commit()
388 ... if not stub.test_emails:
389 ... raise AssertionError("No emails queued!")
388 ... from_addr, to_addrs, raw_message = stub.test_emails[-1]390 ... from_addr, to_addrs, raw_message = stub.test_emails[-1]
389 ... sent_msg = email.message_from_string(raw_message)391 ... sent_msg = email.message_from_string(raw_message)
390 ... error_mail, original_mail = sent_msg.get_payload()392 ... error_mail, original_mail = sent_msg.get_payload()
@@ -411,7 +413,7 @@
411 ... comment_mail, _class=MockUnsignedMessage)413 ... comment_mail, _class=MockUnsignedMessage)
412 >>> handler.process(414 >>> handler.process(
413 ... msg, msg['To'],415 ... msg, msg['To'],
414 ... extract_signature_timestamp=fake_extract_signature_timestamp)416 ... )
415 True417 True
416 >>> commit()418 >>> commit()
417419
@@ -457,12 +459,9 @@
457 >>> added_message in bug_one.messages459 >>> added_message in bug_one.messages
458 True460 True
459461
460Unmark the principal:462In these tests, every time we log in, we're fully trusted again:
461463
462 >>> provided_interfaces = directlyProvidedBy(current_principal)464 >>> login(sampledata.USER_EMAIL)
463 >>> directlyProvides(
464 ... current_principal,
465 ... provided_interfaces - IWeaklyAuthenticatedPrincipal)
466465
467466
468Commands467Commands
@@ -485,7 +484,7 @@
485 >>> def submit_command_email(msg):484 >>> def submit_command_email(msg):
486 ... handler.process(485 ... handler.process(
487 ... msg, msg['To'],486 ... msg, msg['To'],
488 ... extract_signature_timestamp=fake_extract_signature_timestamp)487 ... )
489 ... commit()488 ... commit()
490 ... sync(bug)489 ... sync(bug)
491490
@@ -734,7 +733,7 @@
734 >>> 'Foo Bar' in [subscription.person.displayname733 >>> 'Foo Bar' in [subscription.person.displayname
735 ... for subscription in bug_four.subscriptions]734 ... for subscription in bug_four.subscriptions]
736 False735 False
737 >>> login('test@canonical.com')736 >>> login(sampledata.USER_EMAIL)
738 >>> submit_commands(bug_four, 'unsubscribe')737 >>> submit_commands(bug_four, 'unsubscribe')
739 >>> 'Sample Person' in [subscription.person.displayname738 >>> 'Sample Person' in [subscription.person.displayname
740 ... for subscription in bug_four.subscriptions]739 ... for subscription in bug_four.subscriptions]
@@ -793,9 +792,9 @@
793 ... for subscriber in bug_five.getIndirectSubscribers()])792 ... for subscriber in bug_five.getIndirectSubscribers()])
794 [u'Sample Person', u'Ubuntu Team']793 [u'Sample Person', u'Ubuntu Team']
795794
796(Log back in as test@canonical.com for the tests that follow.)795(Log back in for the tests that follow.)
797796
798 >>> login("test@canonical.com")797 >>> login(sampledata.USER_EMAIL)
799798
800If we specify a non-existant user, an error message will be sent:799If we specify a non-existant user, an error message will be sent:
801800
@@ -1108,7 +1107,7 @@
1108Attempting to set the milestone for a bug without sufficient1107Attempting to set the milestone for a bug without sufficient
1109permissions also elicits an error message:1108permissions also elicits an error message:
11101109
1111 >>> login('test@canonical.com')1110 >>> login(sampledata.USER_EMAIL)
1112 >>> bug = new_firefox_bug()1111 >>> bug = new_firefox_bug()
1113 >>> commit()1112 >>> commit()
11141113
@@ -1150,7 +1149,7 @@
1150 >>> ubuntu.bug_supervisor = sample_person1149 >>> ubuntu.bug_supervisor = sample_person
1151 >>> logout()1150 >>> logout()
11521151
1153 >>> login('test@canonical.com')1152 >>> login(sampledata.USER_EMAIL)
11541153
1155Like the web UI, we can assign a bug to nobody.1154Like the web UI, we can assign a bug to nobody.
11561155
@@ -1246,10 +1245,10 @@
1246 >>> login('foo.bar@canonical.com')1245 >>> login('foo.bar@canonical.com')
1247 >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')1246 >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
1248 >>> ubuntu.driver = getUtility(IPersonSet).getByEmail(1247 >>> ubuntu.driver = getUtility(IPersonSet).getByEmail(
1249 ... 'test@canonical.com')1248 ... sampledata.USER_EMAIL)
1250 >>> commit()1249 >>> commit()
1251 >>> LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)1250 >>> LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)
1252 >>> login('test@canonical.com')1251 >>> login(sampledata.USER_EMAIL)
1253 >>> sync(bug)1252 >>> sync(bug)
12541253
1255Now a new bugtask for the series will be create directly.1254Now a new bugtask for the series will be create directly.
@@ -1302,7 +1301,7 @@
1302 >>> ubuntu.driver = None1301 >>> ubuntu.driver = None
1303 >>> commit()1302 >>> commit()
1304 >>> LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)1303 >>> LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)
1305 >>> login('test@canonical.com')1304 >>> login(sampledata.USER_EMAIL)
13061305
1307 >>> bug = new_firefox_bug()1306 >>> bug = new_firefox_bug()
1308 >>> for bugtask in bug.bugtasks:1307 >>> for bugtask in bug.bugtasks:
@@ -1329,7 +1328,7 @@
1329 ... print driver.displayname1328 ... print driver.displayname
1330 Sample Person1329 Sample Person
13311330
1332 >>> login('test@canonical.com')1331 >>> login(sampledata.USER_EMAIL)
1333 >>> submit_commands(bug, 'affects /firefox/trunk')1332 >>> submit_commands(bug, 'affects /firefox/trunk')
13341333
1335 >>> for bugtask in bug.bugtasks:1334 >>> for bugtask in bug.bugtasks:
@@ -1367,7 +1366,7 @@
1367 ... print nomination.target.bugtargetdisplayname1366 ... print nomination.target.bugtargetdisplayname
1368 Evolution trunk1367 Evolution trunk
13691368
1370 >>> login('test@canonical.com')1369 >>> login(sampledata.USER_EMAIL)
13711370
1372Let's take on the upstream task on bug four as well. This time we'll1371Let's take on the upstream task on bug four as well. This time we'll
1373sneak in a 'subscribe' command between the 'affects' and the other1372sneak in a 'subscribe' command between the 'affects' and the other
@@ -1642,7 +1641,7 @@
1642The user is a bug supervisors of the upstream product1641The user is a bug supervisors of the upstream product
1643~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~1642~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
16441643
1645 >>> login('test@canonical.com')1644 >>> login(sampledata.USER_EMAIL)
1646 >>> bug_one = getUtility(IBugSet).get(1)1645 >>> bug_one = getUtility(IBugSet).get(1)
1647 >>> submit_commands(1646 >>> submit_commands(
1648 ... bug_one, 'status confirmed', 'assignee test@canonical.com')1647 ... bug_one, 'status confirmed', 'assignee test@canonical.com')
@@ -1746,6 +1745,7 @@
1746If none of the bug tasks can be chosen, an error message is sent to the1745If none of the bug tasks can be chosen, an error message is sent to the
1747user, telling him that he has to use the 'affects' command.1746user, telling him that he has to use the 'affects' command.
17481747
1748 >>> del stub.test_emails[:]
1749 >>> login('stuart.bishop@canonical.com')1749 >>> login('stuart.bishop@canonical.com')
1750 >>> submit_commands(1750 >>> submit_commands(
1751 ... bug_one, 'status new', 'assignee foo.bar@canonical.com')1751 ... bug_one, 'status new', 'assignee foo.bar@canonical.com')
@@ -1776,7 +1776,9 @@
1776him about the error. Let's start with trying to submit a bug without1776him about the error. Let's start with trying to submit a bug without
1777signing the mail:1777signing the mail:
17781778
1779 >>> login('test@canonical.com')1779 >>> del stub.test_emails[:]
1780 >>> login(sampledata.USER_EMAIL)
1781 >>> simulate_receiving_untrusted_mail()
17801782
1781 >>> from canonical.launchpad.mail import signed_message_from_string1783 >>> from canonical.launchpad.mail import signed_message_from_string
1782 >>> msg = signed_message_from_string(submit_mail)1784 >>> msg = signed_message_from_string(submit_mail)
@@ -1784,7 +1786,7 @@
1784 >>> msg['Message-Id'] = email.Utils.make_msgid()1786 >>> msg['Message-Id'] = email.Utils.make_msgid()
1785 >>> handler.process(1787 >>> handler.process(
1786 ... msg, msg['To'],1788 ... msg, msg['To'],
1787 ... extract_signature_timestamp=fake_extract_signature_timestamp)1789 ... )
1788 True1790 True
1789 >>> print_latest_email()1791 >>> print_latest_email()
1790 Subject: Submit Request Failure1792 Subject: Submit Request Failure
@@ -1797,6 +1799,7 @@
17971799
1798A submit without specifying on what we want to file the bug on:1800A submit without specifying on what we want to file the bug on:
17991801
1802 >>> login(sampledata.USER_EMAIL)
1800 >>> submit_mail_no_bugtask = """From: test@canonical.com1803 >>> submit_mail_no_bugtask = """From: test@canonical.com
1801 ... To: new@malone1804 ... To: new@malone
1802 ... Date: Fri Jun 17 10:20:23 BST 20051805 ... Date: Fri Jun 17 10:20:23 BST 2005
@@ -2003,7 +2006,7 @@
2003 >>> from canonical.launchpad.mailnotification import (2006 >>> from canonical.launchpad.mailnotification import (
2004 ... send_process_error_notification)2007 ... send_process_error_notification)
2005 >>> send_process_error_notification(2008 >>> send_process_error_notification(
2006 ... 'test@canonical.com', 'Some subject', 'Some error message.',2009 ... sampledata.USER_EMAIL, 'Some subject', 'Some error message.',
2007 ... msg, failing_command=['foo bar'])2010 ... msg, failing_command=['foo bar'])
20082011
2009The To and Subject headers got set to the values we provided:2012The To and Subject headers got set to the values we provided:
@@ -2067,7 +2070,7 @@
20672070
2068First, we create a new firefox bug.2071First, we create a new firefox bug.
20692072
2070 >>> login('test@canonical.com')2073 >>> login(sampledata.USER_EMAIL)
2071 >>> submit_mail = """From: Sample Person <test@canonical.com>2074 >>> submit_mail = """From: Sample Person <test@canonical.com>
2072 ... To: new@bugs.canonical.com2075 ... To: new@bugs.canonical.com
2073 ... Date: Fri Jun 17 10:20:23 BST 20062076 ... Date: Fri Jun 17 10:20:23 BST 2006
20742077
=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-11-19 00:59:48 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-11-24 07:21:59 +0000
@@ -135,13 +135,13 @@
135 Build schedule: Built daily135 Build schedule: Built daily
136 Owner: Master Chef136 Owner: Master Chef
137 Base branch: lp://dev/~chef/ratatouille/veggies137 Base branch: lp://dev/~chef/ratatouille/veggies
138 Debian version: 0\+\{revno\}138 Debian version: {debupstream}-0~{revno}
139 Daily build archive: Secret PPA139 Daily build archive: Secret PPA
140 Distribution series: Secret Squirrel140 Distribution series: Secret Squirrel
141 .*141 .*
142142
143 Recipe contents143 Recipe contents
144 # bzr-builder format 0.2 deb-version 0\+\{revno\}144 # bzr-builder format 0.2 deb-version {debupstream}-0~{revno}
145 lp://dev/~chef/ratatouille/veggies"""145 lp://dev/~chef/ratatouille/veggies"""
146 main_text = extract_text(find_main_content(browser.contents))146 main_text = extract_text(find_main_content(browser.contents))
147 self.assertTextMatchesExpressionIgnoreWhitespace(147 self.assertTextMatchesExpressionIgnoreWhitespace(
@@ -291,7 +291,7 @@
291291
292 browser = self.createRecipe(292 browser = self.createRecipe(
293 dedent('''293 dedent('''
294 # bzr-builder format 0.2 deb-version 0+{revno}294 # bzr-builder format 0.2 deb-version {debupstream}-0~{revno}
295 %(branch)s295 %(branch)s
296 merge %(package_branch)s296 merge %(package_branch)s
297 ''' % {297 ''' % {
@@ -345,7 +345,7 @@
345345
346 with recipe_parser_newest_version(145.115):346 with recipe_parser_newest_version(145.115):
347 recipe = dedent(u'''\347 recipe = dedent(u'''\
348 # bzr-builder format 145.115 deb-version 0+{revno}348 # bzr-builder format 145.115 deb-version {debupstream}-0~{revno}
349 %s349 %s
350 ''') % branch.bzr_identity350 ''') % branch.bzr_identity
351 browser = self.createRecipe(recipe, branch)351 browser = self.createRecipe(recipe, branch)
@@ -439,14 +439,14 @@
439 Build schedule: Built on request439 Build schedule: Built on request
440 Owner: Master Chef440 Owner: Master Chef
441 Base branch: lp://dev/~chef/ratatouille/meat441 Base branch: lp://dev/~chef/ratatouille/meat
442 Debian version: 0\+\{revno\}442 Debian version: {debupstream}-0~{revno}
443 Daily build archive:443 Daily build archive:
444 PPA 2444 PPA 2
445 Distribution series: Mumbly Midget445 Distribution series: Mumbly Midget
446 .*446 .*
447447
448 Recipe contents448 Recipe contents
449 # bzr-builder format 0.2 deb-version 0\+\{revno\}449 # bzr-builder format 0.2 deb-version {debupstream}-0~{revno}
450 lp://dev/~chef/ratatouille/meat"""450 lp://dev/~chef/ratatouille/meat"""
451 main_text = extract_text(find_main_content(browser.contents))451 main_text = extract_text(find_main_content(browser.contents))
452 self.assertTextMatchesExpressionIgnoreWhitespace(452 self.assertTextMatchesExpressionIgnoreWhitespace(
@@ -499,14 +499,14 @@
499 Build schedule: Built on request499 Build schedule: Built on request
500 Owner: Master Chef500 Owner: Master Chef
501 Base branch: lp://dev/~chef/ratatouille/meat501 Base branch: lp://dev/~chef/ratatouille/meat
502 Debian version: 0\+\{revno\}502 Debian version: {debupstream}-0~{revno}
503 Daily build archive:503 Daily build archive:
504 Secret PPA504 Secret PPA
505 Distribution series: Mumbly Midget505 Distribution series: Mumbly Midget
506 .*506 .*
507507
508 Recipe contents508 Recipe contents
509 # bzr-builder format 0.2 deb-version 0\+\{revno\}509 # bzr-builder format 0.2 deb-version {debupstream}-0~{revno}
510 lp://dev/~chef/ratatouille/meat"""510 lp://dev/~chef/ratatouille/meat"""
511 main_text = extract_text(find_main_content(browser.contents))511 main_text = extract_text(find_main_content(browser.contents))
512 self.assertTextMatchesExpressionIgnoreWhitespace(512 self.assertTextMatchesExpressionIgnoreWhitespace(
@@ -551,7 +551,7 @@
551 distroseries=self.squirrel, branches=[veggie_branch])551 distroseries=self.squirrel, branches=[veggie_branch])
552552
553 new_recipe_text = dedent(u'''\553 new_recipe_text = dedent(u'''\
554 # bzr-builder format 145.115 deb-version 0+{revno}554 # bzr-builder format 145.115 deb-version {debupstream}-0~{revno}
555 %s555 %s
556 ''') % recipe.base_branch.bzr_identity556 ''') % recipe.base_branch.bzr_identity
557557
@@ -639,14 +639,14 @@
639 Build schedule: Built on request639 Build schedule: Built on request
640 Owner: Master Chef640 Owner: Master Chef
641 Base branch: lp://dev/~chef/ratatouille/meat641 Base branch: lp://dev/~chef/ratatouille/meat
642 Debian version: 0\+\{revno\}642 Debian version: {debupstream}-0~{revno}
643 Daily build archive:643 Daily build archive:
644 Secret PPA644 Secret PPA
645 Distribution series: Mumbly Midget645 Distribution series: Mumbly Midget
646 .*646 .*
647647
648 Recipe contents648 Recipe contents
649 # bzr-builder format 0.2 deb-version 0\+\{revno\}649 # bzr-builder format 0.2 deb-version {debupstream}-0~{revno}
650 lp://dev/~chef/ratatouille/meat"""650 lp://dev/~chef/ratatouille/meat"""
651 main_text = extract_text(find_main_content(browser.contents))651 main_text = extract_text(find_main_content(browser.contents))
652 self.assertTextMatchesExpressionIgnoreWhitespace(652 self.assertTextMatchesExpressionIgnoreWhitespace(
@@ -690,7 +690,7 @@
690 Build schedule: Built on request690 Build schedule: Built on request
691 Owner: Master Chef691 Owner: Master Chef
692 Base branch: lp://dev/~chef/chocolate/cake692 Base branch: lp://dev/~chef/chocolate/cake
693 Debian version: 0\+\{revno\}693 Debian version: {debupstream}-0~{revno}
694 Daily build archive: Secret PPA694 Daily build archive: Secret PPA
695 Distribution series: Secret Squirrel695 Distribution series: Secret Squirrel
696696
@@ -700,7 +700,7 @@
700 Request build\(s\)700 Request build\(s\)
701701
702 Recipe contents702 Recipe contents
703 # bzr-builder format 0.2 deb-version 0\+\{revno\}703 # bzr-builder format 0.2 deb-version {debupstream}-0~{revno}
704 lp://dev/~chef/chocolate/cake""", self.getMainText(recipe))704 lp://dev/~chef/chocolate/cake""", self.getMainText(recipe))
705705
706 def test_index_no_builds(self):706 def test_index_no_builds(self):
707707
=== modified file 'lib/lp/code/interfaces/sourcepackagerecipe.py'
--- lib/lp/code/interfaces/sourcepackagerecipe.py 2010-11-05 18:30:21 +0000
+++ lib/lp/code/interfaces/sourcepackagerecipe.py 2010-11-24 07:21:59 +0000
@@ -63,7 +63,7 @@
6363
6464
65MINIMAL_RECIPE_TEXT = dedent(u'''\65MINIMAL_RECIPE_TEXT = dedent(u'''\
66 # bzr-builder format 0.2 deb-version 0+{revno}66 # bzr-builder format 0.2 deb-version {debupstream}-0~{revno}
67 %s67 %s
68 ''')68 ''')
6969
7070
=== modified file 'lib/lp/code/model/tests/test_recipebuilder.py'
--- lib/lp/code/model/tests/test_recipebuilder.py 2010-11-19 05:31:52 +0000
+++ lib/lp/code/model/tests/test_recipebuilder.py 2010-11-24 07:21:59 +0000
@@ -164,8 +164,9 @@
164 'author_name': u'Joe User',164 'author_name': u'Joe User',
165 'archive_purpose': 'PPA',165 'archive_purpose': 'PPA',
166 'ogrecomponent': 'universe',166 'ogrecomponent': 'universe',
167 'recipe_text': '# bzr-builder format 0.2 deb-version 0+{revno}\n'167 'recipe_text':
168 'lp://dev/~joe/someapp/pkg\n',168 '# bzr-builder format 0.2 deb-version {debupstream}-0~{revno}\n'
169 'lp://dev/~joe/someapp/pkg\n',
169 'archives': expected_archives,170 'archives': expected_archives,
170 'distroseries_name': job.build.distroseries.name,171 'distroseries_name': job.build.distroseries.name,
171 }, job._extraBuildArgs(distroarchseries))172 }, job._extraBuildArgs(distroarchseries))
@@ -231,8 +232,9 @@
231 'author_name': u'Joe User',232 'author_name': u'Joe User',
232 'archive_purpose': 'PPA',233 'archive_purpose': 'PPA',
233 'ogrecomponent': 'universe',234 'ogrecomponent': 'universe',
234 'recipe_text': '# bzr-builder format 0.2 deb-version 0+{revno}\n'235 'recipe_text':
235 'lp://dev/~joe/someapp/pkg\n',236 '# bzr-builder format 0.2 deb-version {debupstream}-0~{revno}\n'
237 'lp://dev/~joe/someapp/pkg\n',
236 'archives': expected_archives,238 'archives': expected_archives,
237 'distroseries_name': job.build.distroseries.name,239 'distroseries_name': job.build.distroseries.name,
238 }, job._extraBuildArgs(distroarchseries, logger))240 }, job._extraBuildArgs(distroarchseries, logger))
239241
=== modified file 'lib/lp/services/mail/incoming.py'
--- lib/lp/services/mail/incoming.py 2010-10-23 16:45:43 +0000
+++ lib/lp/services/mail/incoming.py 2010-11-24 07:21:59 +0000
@@ -37,6 +37,9 @@
37from canonical.launchpad.interfaces.mailbox import IMailBox37from canonical.launchpad.interfaces.mailbox import IMailBox
38from canonical.launchpad.mail.commands import get_error_message38from canonical.launchpad.mail.commands import get_error_message
39from canonical.launchpad.mail.handlers import mail_handlers39from canonical.launchpad.mail.handlers import mail_handlers
40from canonical.launchpad.mail.helpers import (
41 ensure_sane_signature_timestamp,
42 )
40from canonical.launchpad.mailnotification import (43from canonical.launchpad.mailnotification import (
41 send_process_error_notification,44 send_process_error_notification,
42 )45 )
@@ -61,6 +64,7 @@
61# Match trailing whitespace.64# Match trailing whitespace.
62trailing_whitespace = re.compile(r'[ \t]*((?=\r\n)|$)')65trailing_whitespace = re.compile(r'[ \t]*((?=\r\n)|$)')
6366
67
64def canonicalise_line_endings(text):68def canonicalise_line_endings(text):
65 r"""Canonicalise the line endings to '\r\n'.69 r"""Canonicalise the line endings to '\r\n'.
6670
@@ -159,14 +163,23 @@
159 return True163 return True
160164
161165
162def authenticateEmail(mail):166def authenticateEmail(mail,
167 signature_timestamp_checker=None):
163 """Authenticates an email by verifying the PGP signature.168 """Authenticates an email by verifying the PGP signature.
164169
165 The mail is expected to be an ISignedMessage.170 The mail is expected to be an ISignedMessage.
171
172 If this completes, it will set the current security principal to be the
173 message sender.
174
175 :param signature_timestamp_checker: This callable is
176 passed the message signature timestamp, and it can raise an exception if
177 it dislikes it (for example as a replay attack.) This parameter is
178 intended for use in tests. If None, ensure_sane_signature_timestamp
179 is used.
166 """180 """
167181
168 signature = mail.signature182 signature = mail.signature
169 signed_content = mail.signedContent
170183
171 name, email_addr = parseaddr(mail['From'])184 name, email_addr = parseaddr(mail['From'])
172 authutil = getUtility(IPlacelessAuthUtility)185 authutil = getUtility(IPlacelessAuthUtility)
@@ -206,11 +219,19 @@
206 gpghandler = getUtility(IGPGHandler)219 gpghandler = getUtility(IGPGHandler)
207 try:220 try:
208 sig = gpghandler.getVerifiedSignature(221 sig = gpghandler.getVerifiedSignature(
209 canonicalise_line_endings(signed_content), signature)222 canonicalise_line_endings(mail.signedContent), signature)
210 except GPGVerificationError, e:223 except GPGVerificationError, e:
211 # verifySignature failed to verify the signature.224 # verifySignature failed to verify the signature.
212 raise InvalidSignature("Signature couldn't be verified: %s" % str(e))225 raise InvalidSignature("Signature couldn't be verified: %s" % str(e))
213226
227 if signature_timestamp_checker is None:
228 signature_timestamp_checker = ensure_sane_signature_timestamp
229 # If this fails, we return an error to the user rather than just treating
230 # it as untrusted, so they can debug or understand the problem.
231 signature_timestamp_checker(
232 sig.timestamp,
233 'incoming mail verification')
234
214 for gpgkey in person.gpg_keys:235 for gpgkey in person.gpg_keys:
215 if gpgkey.fingerprint == sig.fingerprint:236 if gpgkey.fingerprint == sig.fingerprint:
216 break237 break
@@ -255,7 +276,8 @@
255 return request.oopsid276 return request.oopsid
256277
257278
258def handleMail(trans=transaction):279def handleMail(trans=transaction,
280 signature_timestamp_checker=None):
259 # First we define an error handler. We define it as a local281 # First we define an error handler. We define it as a local
260 # function, to avoid having to pass a lot of parameters.282 # function, to avoid having to pass a lot of parameters.
261 # pylint: disable-msg=W0631283 # pylint: disable-msg=W0631
@@ -358,7 +380,8 @@
358 continue380 continue
359381
360 try:382 try:
361 principal = authenticateEmail(mail)383 principal = authenticateEmail(
384 mail, signature_timestamp_checker)
362 except InvalidSignature, error:385 except InvalidSignature, error:
363 _handle_user_error(error, mail)386 _handle_user_error(error, mail)
364 continue387 continue
365388
=== modified file 'lib/lp/services/mail/tests/incomingmail.txt'
--- lib/lp/services/mail/tests/incomingmail.txt 2010-10-25 13:16:10 +0000
+++ lib/lp/services/mail/tests/incomingmail.txt 2010-11-24 07:21:59 +0000
@@ -62,6 +62,12 @@
62 ... msg.replace_header('To', '123@%s' % domain)62 ... msg.replace_header('To', '123@%s' % domain)
63 ... msgids[domain].append("<%s>" % sendmail(msg))63 ... msgids[domain].append("<%s>" % sendmail(msg))
6464
65handleMail will check the timestamp on signed messages, but the signatures
66on our testdata are old, and in these tests we don't care to be told.
67
68 >>> def accept_any_timestamp(timestamp, context_message):
69 ... pass
70
65Since the User gets authenticated using OpenPGP signatures we have to71Since the User gets authenticated using OpenPGP signatures we have to
66import the keys before handleMail is called.72import the keys before handleMail is called.
6773
@@ -72,6 +78,11 @@
72 >>> LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)78 >>> LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)
73 >>> zopeless_transaction = LaunchpadZopelessLayer.txn79 >>> zopeless_transaction = LaunchpadZopelessLayer.txn
7480
81 >>> handleMailForTest = lambda: handleMail(
82 ... zopeless_transaction,
83 ... signature_timestamp_checker=accept_any_timestamp)
84
85
75We temporarily override the error mails' From address, so that they will86We temporarily override the error mails' From address, so that they will
76pass through the authentication stage:87pass through the authentication stage:
7788
@@ -87,7 +98,7 @@
87discussed below; this output merely shows that we emit warnings when the98discussed below; this output merely shows that we emit warnings when the
88header is missing.)99header is missing.)
89100
90 >>> handleMail(zopeless_transaction)101 >>> handleMailForTest()
91 WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...102 WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...
92 WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...103 WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...
93 WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...104 WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...
@@ -139,7 +150,7 @@
139 >>> moin_change = read_test_message('moin-change.txt')150 >>> moin_change = read_test_message('moin-change.txt')
140 >>> moin_change['X-Launchpad-Original-To'] = '123@foo.com'151 >>> moin_change['X-Launchpad-Original-To'] = '123@foo.com'
141 >>> msgid = "<%s>" % sendmail(moin_change)152 >>> msgid = "<%s>" % sendmail(moin_change)
142 >>> handleMail(zopeless_transaction)153 >>> handleMailForTest()
143 >>> msgid not in foo_handler.handledMails154 >>> msgid not in foo_handler.handledMails
144 True155 True
145156
@@ -154,7 +165,7 @@
154165
155 >>> moin_change.replace_header('X-Launchpad-Original-To', '123@bar.com')166 >>> moin_change.replace_header('X-Launchpad-Original-To', '123@bar.com')
156 >>> msgid = "<%s>" % sendmail(moin_change)167 >>> msgid = "<%s>" % sendmail(moin_change)
157 >>> handleMail(zopeless_transaction)168 >>> handleMailForTest()
158 >>> msgid in bar_handler.handledMails169 >>> msgid in bar_handler.handledMails
159 True170 True
160171
@@ -176,13 +187,13 @@
176187
177 >>> msg = read_test_message('unsigned_inactive.txt')188 >>> msg = read_test_message('unsigned_inactive.txt')
178 >>> msgid = sendmail(msg, ['edit@malone-domain'])189 >>> msgid = sendmail(msg, ['edit@malone-domain'])
179 >>> handleMail(zopeless_transaction)190 >>> handleMailForTest()
180 >>> msgid not in foo_handler.handledMails191 >>> msgid not in foo_handler.handledMails
181 True192 True
182193
183 >>> msg = read_test_message('invalid_signed_inactive.txt')194 >>> msg = read_test_message('invalid_signed_inactive.txt')
184 >>> msgid = sendmail(msg, ['edit@malone-domain'])195 >>> msgid = sendmail(msg, ['edit@malone-domain'])
185 >>> handleMail(zopeless_transaction)196 >>> handleMailForTest()
186 >>> msgid not in foo_handler.handledMails197 >>> msgid not in foo_handler.handledMails
187 True198 True
188199
@@ -198,7 +209,7 @@
198 >>> msg['CC'] = '123@foo.com'209 >>> msg['CC'] = '123@foo.com'
199 >>> msg['X-Launchpad-Original-To'] = '123@bar.com'210 >>> msg['X-Launchpad-Original-To'] = '123@bar.com'
200 >>> msgid = '<%s>' % sendmail (msg, ['123@bar.com'])211 >>> msgid = '<%s>' % sendmail (msg, ['123@bar.com'])
201 >>> handleMail(zopeless_transaction)212 >>> handleMailForTest()
202 >>> msgid in bar_handler.handledMails213 >>> msgid in bar_handler.handledMails
203 True214 True
204215
@@ -238,7 +249,7 @@
238 ... doesn't matter249 ... doesn't matter
239 ... """)250 ... """)
240 >>> msgid = sendmail(msg, ['edit@malone-domain'])251 >>> msgid = sendmail(msg, ['edit@malone-domain'])
241 >>> handleMail(zopeless_transaction)252 >>> handleMailForTest()
242 ERROR:canonical.launchpad.mail:An exception was raised inside the handler:253 ERROR:canonical.launchpad.mail:An exception was raised inside the handler:
243 ...254 ...
244 TestOopsException255 TestOopsException
@@ -286,7 +297,7 @@
286 ... doesn't matter297 ... doesn't matter
287 ... """)298 ... """)
288 >>> msgid = sendmail(msg, ['edit@malone-domain'])299 >>> msgid = sendmail(msg, ['edit@malone-domain'])
289 >>> handleMail(zopeless_transaction)300 >>> handleMailForTest()
290 ERROR:canonical.launchpad.mail:An exception was raised inside the handler:301 ERROR:canonical.launchpad.mail:An exception was raised inside the handler:
291 ...302 ...
292 Unauthorized303 Unauthorized
@@ -352,7 +363,7 @@
352If we call handleMail(), we'll see some useful error messages printed363If we call handleMail(), we'll see some useful error messages printed
353out:364out:
354365
355 >>> handleMail(zopeless_transaction)366 >>> handleMailForTest()
356 ERROR:...:An exception was raised inside the handler: http://...367 ERROR:...:An exception was raised inside the handler: http://...
357 Traceback (most recent call last):368 Traceback (most recent call last):
358 ...369 ...
@@ -389,7 +400,7 @@
389 >>> len(stub.test_emails)400 >>> len(stub.test_emails)
390 2401 2
391402
392 >>> handleMail(zopeless_transaction)403 >>> handleMailForTest()
393 ERROR:...:Upload to Librarian failed...404 ERROR:...:Upload to Librarian failed...
394 ...405 ...
395 UploadFailed: ...Connection refused...406 UploadFailed: ...Connection refused...
@@ -416,7 +427,7 @@
416 >>> msg.replace_header('To', '123@foo.com')427 >>> msg.replace_header('To', '123@foo.com')
417 >>> msg['Return-Path'] = '<>'428 >>> msg['Return-Path'] = '<>'
418 >>> msgid = '<%s>' % sendmail(msg)429 >>> msgid = '<%s>' % sendmail(msg)
419 >>> handleMail(zopeless_transaction)430 >>> handleMailForTest()
420 >>> msgid in foo_handler.handledMails431 >>> msgid in foo_handler.handledMails
421 False432 False
422433
@@ -437,7 +448,7 @@
437 ... 'multipart/report; report-type=delivery-status;'448 ... 'multipart/report; report-type=delivery-status;'
438 ... ' boundary="boundary"')449 ... ' boundary="boundary"')
439 >>> msgid = '<%s>' % sendmail(msg)450 >>> msgid = '<%s>' % sendmail(msg)
440 >>> handleMail(zopeless_transaction)451 >>> handleMailForTest()
441 >>> msgid in foo_handler.handledMails452 >>> msgid in foo_handler.handledMails
442 False453 False
443454
@@ -452,7 +463,7 @@
452 >>> msg['Return-Path'] = '<not@empty.com>'463 >>> msg['Return-Path'] = '<not@empty.com>'
453 >>> msg['Precedence'] = 'bulk'464 >>> msg['Precedence'] = 'bulk'
454 >>> msgid = '<%s>' % sendmail(msg)465 >>> msgid = '<%s>' % sendmail(msg)
455 >>> handleMail(zopeless_transaction)466 >>> handleMailForTest()
456 >>> msgid in foo_handler.handledMails467 >>> msgid in foo_handler.handledMails
457 False468 False
458469
459470
=== modified file 'lib/lp/services/mail/tests/test_incoming.py'
--- lib/lp/services/mail/tests/test_incoming.py 2010-10-15 20:44:53 +0000
+++ lib/lp/services/mail/tests/test_incoming.py 2010-11-24 07:21:59 +0000
@@ -10,17 +10,23 @@
10from zope.security.management import setSecurityPolicy10from zope.security.management import setSecurityPolicy
1111
12from canonical.config import config12from canonical.config import config
13from canonical.launchpad.ftests import import_secret_test_key
13from canonical.launchpad.mail.ftests.helpers import testmails_path14from canonical.launchpad.mail.ftests.helpers import testmails_path
15from canonical.launchpad.mail import (
16 helpers,
17 )
14from canonical.launchpad.testing.systemdocs import LayeredDocFileSuite18from canonical.launchpad.testing.systemdocs import LayeredDocFileSuite
15from canonical.launchpad.webapp.authorization import LaunchpadSecurityPolicy19from canonical.launchpad.webapp.authorization import LaunchpadSecurityPolicy
20from canonical.testing.layers import LaunchpadZopelessLayer
16from lp.services.mail.incoming import (21from lp.services.mail.incoming import (
22 authenticateEmail,
17 handleMail,23 handleMail,
18 MailErrorUtility,24 MailErrorUtility,
19 )25 )
20from canonical.testing.layers import LaunchpadZopelessLayer
21from lp.services.mail.sendmail import MailController26from lp.services.mail.sendmail import MailController
22from lp.services.mail.stub import TestMailer27from lp.services.mail.stub import TestMailer
23from lp.testing import TestCaseWithFactory28from lp.testing import TestCaseWithFactory
29from lp.testing.factory import GPGSigningContext
24from lp.testing.mail_helpers import pop_notifications30from lp.testing.mail_helpers import pop_notifications
2531
2632
@@ -81,6 +87,23 @@
81 else:87 else:
82 self.assertEqual(old_oops.id, current_oops.id)88 self.assertEqual(old_oops.id, current_oops.id)
8389
90 def test_bad_signature_timestamp(self):
91 """If the signature is nontrivial future-dated, it's not trusted."""
92
93 signing_context = GPGSigningContext(
94 import_secret_test_key().fingerprint, password='test')
95 msg = self.factory.makeSignedMessage(signing_context=signing_context)
96 # It's not trivial to make a gpg signature with a bogus timestamp, so
97 # let's just treat everything as invalid, and trust that the regular
98 # implementation of extraction and checking of timestamps is correct,
99 # or at least tested.
100 def fail_all_timestamps(timestamp, context):
101 raise helpers.IncomingEmailError("fail!")
102 self.assertRaises(
103 helpers.IncomingEmailError,
104 authenticateEmail,
105 msg, fail_all_timestamps)
106
84107
85def setUp(test):108def setUp(test):
86 test._old_policy = setSecurityPolicy(LaunchpadSecurityPolicy)109 test._old_policy = setSecurityPolicy(LaunchpadSecurityPolicy)
87110
=== modified file 'utilities/migrater/file-ownership.txt'
--- utilities/migrater/file-ownership.txt 2010-11-16 12:56:01 +0000
+++ utilities/migrater/file-ownership.txt 2010-11-24 07:21:59 +0000
@@ -1047,7 +1047,7 @@
1047 ./mail/errortemplates/num-arguments-mismatch.txt1047 ./mail/errortemplates/num-arguments-mismatch.txt
1048 ./mail/errortemplates/no-such-bug.txt1048 ./mail/errortemplates/no-such-bug.txt
1049 ./mail/errortemplates/security-parameter-mismatch.txt1049 ./mail/errortemplates/security-parameter-mismatch.txt
1050 ./mail/errortemplates/not-gpg-signed.txt1050 ./mail/errortemplates/unauthenticated-bug-creation.txt
1051 ./mail/errortemplates/key-not-registered.txt1051 ./mail/errortemplates/key-not-registered.txt
1052 ./mail/errortemplates/oops.txt1052 ./mail/errortemplates/oops.txt
1053 ./mail/errortemplates/branchmergeproposal-exists.txt1053 ./mail/errortemplates/branchmergeproposal-exists.txt
10541054
=== modified file 'versions.cfg'

Subscribers

People subscribed via source and target branches

to status/vote changes: