Launchpad itself

Merge lp:~mbp/launchpad/dkim into lp:launchpad

  1. dkim
  2. Merge into devel
Proposed by Martin Pool
Status: Merged
Approved by: Stuart Bishop
Approved revision: no longer in the source branch.
Merge reported by: Martin Pool
Merged at revision: not available
Proposed branch: lp:~mbp/launchpad/dkim
Merge into: lp:launchpad
Diff against target: 491 lines (+359/-8)
5 files modified
lib/canonical/launchpad/mail/incoming.py (+89/-2)
lib/lp/services/mail/tests/test_dkim.py (+258/-0)
lib/lp/services/mail/tests/test_signedmessage.py (+6/-6)
setup.py (+3/-0)
versions.cfg (+3/-0)
To merge this branch: bzr merge lp:~mbp/launchpad/dkim
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Approve
Review via email: mp+26284@code.launchpad.net

Description of the change

progress towards treating DKIM mail as authenticated

see https://dev.launchpad.net/LEP/DKIMAuthenticatedMail
and https://bugs.edge.launchpad.net/launchpad-registry/+bug/316272

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

There is an upstream bug in python-dkim, https://bugs.edge.launchpad.net/ubuntu/+source/pydkim/+bug/587783

Revision history for this message
Stuart Bishop (stub) wrote :
Download full text (4.1 KiB)

Thanks for this. I'll certainly appreciate this feature :-)

=== modified file 'lib/canonical/launchpad/mail/incoming.py'

+def _authenticateDkim(mail):
+ """"Attempt DKIM authentication of email; return True if known authentic"""
+
+ try:
+ dkim_result = dkim.verify(mail.as_string(), dkim_log)
+ except dkim.DKIMException, e:
+ log.warning('DKIM error: %r' % (e,))
+ dkim_result = False
+ except dns.exception.DNSException, e:
+ # many of them have lame messages, thus %r
+ log.warning('DNS exception: %r' % (e,))
+ dkim_result = False

So this is failing DKIM authentication on DNS glitches, and DKIMException (invalid signature, messed up DNS etc.). I think users need to be informed of this. A the moment, they will just get the standard 'unsigned email' response with no hint what went wrong.

This might be too much refactoring for now, so perhaps a bug is in order and we can worry about it if it is a genuine problem. I think we would have to annotate the 'mail' object and have a higher level report failures to the user if strong authentication is required but not available.

+ if mail.signature:
+ log.info('message has gpg signature, therefore not treating DKIM as conclusive')
+ return False

I think this logic is better left to the callsite. Just return success/fail and leave it up to authenticateEmail what signature types should take precidence.

=== added file 'lib/lp/services/mail/tests/test_dkim.py'

+# reuse the handler that records log events
+from lp.services.sshserver.tests.test_events import ListHandler

You don't seem to be using this. If you do, ideally this should be moved somewhere like lp.testing.

+ def monkeypatch_dns(self):
+ self._dns_responses = {}
+ def my_lookup(name):
+ try:
+ return self._dns_responses[name]
+ except KeyError:
+ raise dns.resolver.NXDOMAIN()
+ orig_dnstxt = dkim.dnstxt
+ dkim.dnstxt = my_lookup
+ def restore():
+ dkim.dnstxt = orig_dnstxt
+ self.addCleanup(restore)

This might have been nicer using Mocker (http://labix.org/mocker), which is available as a dependency. I wouldn't worry about changing now though - this is fine and understandable.

+ def test_dkim_valid_strict(self):
+ # FIXME: this fails because some re-wrapping happens after the
+ # message comes in (probably going too/from SignedMessage)
+ # and pydkim correctly complains about that: we would need to
+ # validate against the raw bytes
+ return
+ signed_message = self.fake_signing(plain_content,
+ canonicalize=(dkim.Simple, dkim.Simple))
+ self._dns_responses['example._domainkey.canonical.com.'] = \
+ sample_dns
+ principal = authenticateEmail(signed_message_from_string(signed_message))
+ self.assertStronglyAuthenticated(principal, signed_message)

This test is still disabled. Does this need to be fixed before landing, as we would see this situation in real email too? If not, at a minimum the comment needs to change to an XXX referencing a Launchpad bug.

+ def test_d...

Read more...

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

> Thanks for this. I'll certainly appreciate this feature :-)

Thanks for the review.

>
> === modified file 'lib/canonical/launchpad/mail/incoming.py'
>
> +def _authenticateDkim(mail):
> + """"Attempt DKIM authentication of email; return True if known
> authentic"""
> +
> + try:
> + dkim_result = dkim.verify(mail.as_string(), dkim_log)
> + except dkim.DKIMException, e:
> + log.warning('DKIM error: %r' % (e,))
> + dkim_result = False
> + except dns.exception.DNSException, e:
> + # many of them have lame messages, thus %r
> + log.warning('DNS exception: %r' % (e,))
> + dkim_result = False
>
> So this is failing DKIM authentication on DNS glitches, and DKIMException
> (invalid signature, messed up DNS etc.). I think users need to be informed of
> this. A the moment, they will just get the standard 'unsigned email' response
> with no hint what went wrong.
>
> This might be too much refactoring for now, so perhaps a bug is in order and
> we can worry about it if it is a genuine problem. I think we would have to
> annotate the 'mail' object and have a higher level report failures to the user
> if strong authentication is required but not available.

I think for now we shouldn't send any mail on DKIM failures, because that would be a likely way for this to go wrong by spamming people. Let's get some experience on real data first by monitoring how many fail and why.

There are a few possibilities:

* message is gpg signed but has broken dkim
* message has a broken signature but it's ok for it to be low-trusted anyhow
* message left the user ok but was broken somewhere in transit
* message was broken in our DC on the way to the incoming queue (we don't know this won't happen)
* message failed to verify because of a transient dns error

If the user expects their messages to be trusted and they aren't, there should be a way for them to find out why. Conversely if they don't care about DKIM (and often it won't be under the user's control to fix) we shouldn't bother them about it. For the moment I think we should let them ask a question in the first case.

>
>
> + if mail.signature:
> + log.info('message has gpg signature, therefore not treating DKIM as
> conclusive')
> + return False
>
> I think this logic is better left to the callsite. Just return success/fail
> and leave it up to authenticateEmail what signature types should take
> precidence.

ok

>
> === added file 'lib/lp/services/mail/tests/test_dkim.py'
>
> +# reuse the handler that records log events
> +from lp.services.sshserver.tests.test_events import ListHandler
>
> You don't seem to be using this. If you do, ideally this should be moved
> somewhere like lp.testing.

ok

>
>
> + def monkeypatch_dns(self):
> + self._dns_responses = {}
> + def my_lookup(name):
> + try:
> + return self._dns_responses[name]
> + except KeyError:
> + raise dns.resolver.NXDOMAIN()
> + orig_dnstxt = dkim.dnstxt
> + dkim.dnstxt = my_lookup
> + def restore():
> + dkim.dnstxt = orig_dnstxt
> + self.addCleanup(restore)
>
> Thi...

Read more...

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

bug 587783 now has what I think is a good clean patch, and a script to demonstrate it, and I've just sent it upstream.

There is also a source package branch there that can build a .deb of the fixed version or I guess that can be turned into an egg if prefered.

There was some list discussion of this <https://lists.launchpad.net/launchpad-dev/msg03533.html>

The point was also raised that we might want a whitelist of domains who are trusted to have meaningful DKIM headers, so I'll add that and do stub's tweaks.

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

>
> >
> >
> > + if mail.signature:
> > + log.info('message has gpg signature, therefore not treating DKIM as
> > conclusive')
> > + return False
> >
> > I think this logic is better left to the callsite. Just return success/fail
> > and leave it up to authenticateEmail what signature types should take
> > precidence.
>
> ok

done

>
> >
> > === added file 'lib/lp/services/mail/tests/test_dkim.py'
> >
> > +# reuse the handler that records log events
> > +from lp.services.sshserver.tests.test_events import ListHandler
> >
> > You don't seem to be using this. If you do, ideally this should be moved
> > somewhere like lp.testing.
>
> ok

fixed

> >
> > + def test_dkim_valid_strict(self):
> > + # FIXME: this fails because some re-wrapping happens after the
> > + # message comes in (probably going too/from SignedMessage)
> > + # and pydkim correctly complains about that: we would need to
> > + # validate against the raw bytes
> > + return
> > + signed_message = self.fake_signing(plain_content,
> > + canonicalize=(dkim.Simple, dkim.Simple))
> > + self._dns_responses['example._domainkey.canonical.com.'] = \
> > + sample_dns
> > + principal =
> > authenticateEmail(signed_message_from_string(signed_message))
> > + self.assertStronglyAuthenticated(principal, signed_message)
> >
> > This test is still disabled. Does this need to be fixed before landing, as
> we
> > would see this situation in real email too? If not, at a minimum the comment
> > needs to change to an XXX referencing a Launchpad bug.
>
> Yes, this needs to be fixed before landing; it's due in part to bug 587783.

fixed; there was a bug in python-dkim but I also need to expose the raw bytes of the message for DKIM validation, not the ISignedMessage which doesn't give the contents back byte-for-byte

>
> > + def test_dkim_body_mismatch(self):
> > + # The message message has a syntactically valid DKIM signature that
> > + # doesn't actually correspond to the sender. We log something
> about
> > + # this but we don't want to drop the message.
> > + #
> > + # XXX: This test relies on having real DNS service to look up the
> > + # signing key.
> >
> > Is the XXX comment still valid now you are monkey patching the DNS
> responses?
> > We might have to fix this before landing - I don't know if this is an
> allowed
> > dependency in our tests.
>
> It's out of date.

fixed

I think the main things to do now are

 * add a config option for a list of trusted domains and test it
 * possibly get a final check-over from @elmo
 * get the dependency included -- if you want, you could start that now in parallel?
 * deploy to staging and test there?

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

A couple more things:

 * We need to check the From address matches the signature; at the moment we don't but I now have a failing test for it
 * python-dkim only checks the first signature so a multiply signed message may be seen as untrusted. I think this is fine for now.

Revision history for this message
Stuart Bishop (stub) wrote :

lp:~stub/launchpad/dkim contains the changes for building the patched pydkim with buildout

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

<https://code.edge.launchpad.net/~mbp/ubuntu/lucid/pydkim/details> gives pydkim the ability to return details about the signatures that it verified.

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

Sorry, could you please re-try with the import restored?

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

the equivalent was merged by stub

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/mail/incoming.py'
--- lib/canonical/launchpad/mail/incoming.py 2010-06-04 16:15:38 +0000
+++ lib/canonical/launchpad/mail/incoming.py 2010-07-12 20:52:02 +0000
@@ -11,9 +11,13 @@
11from cStringIO import StringIO as cStringIO11from cStringIO import StringIO as cStringIO
12from email.utils import getaddresses, parseaddr12from email.utils import getaddresses, parseaddr
13import email.errors13import email.errors
14import logging
14import re15import re
15import sys16import sys
1617
18import dkim
19import dns.exception
20
17import transaction21import transaction
18from zope.component import getUtility22from zope.component import getUtility
19from zope.interface import directlyProvides, directlyProvidedBy23from zope.interface import directlyProvides, directlyProvidedBy
@@ -65,10 +69,83 @@
65 """The account for the person sending this email is inactive."""69 """The account for the person sending this email is inactive."""
6670
6771
68def authenticateEmail(mail):72def extract_address_domain(address):
73 realname, email_address = email.utils.parseaddr(address)
74 return email_address.split('@')[1]
75
76
77_trusted_dkim_domains = [
78 'gmail.com', 'google.com', 'mail.google.com', 'canonical.com']
79
80
81def _isDkimDomainTrusted(domain):
82 # XXX: really this should come from a dynamically-modifiable
83 # configuration, but we don't have such a thing yet.
84 #
85 # Being listed here means that we trust the domain not to be an open relay
86 # or to allow arbitrary intra-domain spoofing.
87 return domain in _trusted_dkim_domains
88
89
90def _authenticateDkim(signed_message, raw_mail):
91 """"Attempt DKIM authentication of email; return True if known authentic
92
93 :param signed_message: ISignedMessage
94 :param raw_mail: byte string of the raw contents: needed because
95 DKIM may be in strict mode and we want to avoid any header rewrapping
96 involved in parsing and regenerating the message.
97 """
98
99 log = logging.getLogger('mail-authenticate-dkim')
100 log.setLevel(logging.DEBUG)
101 # uncomment this for easier test debugging
102 # log.addHandler(logging.FileHandler('/tmp/dkim.log'))
103
104 dkim_log = cStringIO()
105 log.info('Attempting DKIM authentication of message %s from %s'
106 % (signed_message['Message-ID'], signed_message['From']))
107 signing_details = []
108 try:
109 # NB: if this fails with a keyword argument error, you need the
110 # python-dkim 0.3-3.2 that adds it
111 dkim_result = dkim.verify(raw_mail, dkim_log, details=signing_details)
112 except dkim.DKIMException, e:
113 log.warning('DKIM error: %r' % (e,))
114 dkim_result = False
115 except dns.exception.DNSException, e:
116 # many of them have lame messages, thus %r
117 log.warning('DNS exception: %r' % (e,))
118 dkim_result = False
119 else:
120 log.info('DKIM verification result=%s' % (dkim_result,))
121 log.debug('DKIM debug log: %s' % (dkim_log.getvalue(),))
122 if not dkim_result:
123 return False
124 # in addition to the dkim signature being valid, we have to check that it
125 # was actually signed by the user's domain.
126 if len(signing_details) != 1:
127 log.errors('expected exactly one DKIM details record: %r' % (signing_details,))
128 return False
129 signing_domain = signing_details[0]['d']
130 from_domain = extract_address_domain(signed_message['From'])
131 if signing_domain != from_domain:
132 log.warning("DKIM signing domain %s doesn't match From address %s; "
133 "disregarding signature"
134 % (signing_domain, from_domain))
135 return False
136 if not _isDkimDomainTrusted(signing_domain):
137 log.warning("valid DKIM signature from untrusted domain %s"
138 % (signing_domain,))
139 return False
140 return True
141
142
143def authenticateEmail(mail, raw_mail):
69 """Authenticates an email by verifying the PGP signature.144 """Authenticates an email by verifying the PGP signature.
70145
71 The mail is expected to be an ISignedMessage.146 The mail is expected to be an ISignedMessage.
147
148 raw_mail is just a byte string of the uninterpreted mail.
72 """149 """
73150
74 signature = mail.signature151 signature = mail.signature
@@ -89,6 +166,16 @@
89 raise InactiveAccount(166 raise InactiveAccount(
90 "Mail from a user with an inactive account.")167 "Mail from a user with an inactive account.")
91168
169 dkim_result = _authenticateDkim(mail, raw_mail)
170
171 if dkim_result:
172 if mail.signature is not None:
173 log.info('message has gpg signature, therefore not treating DKIM '
174 'success as conclusive')
175 else:
176 setupInteraction(principal, email_addr)
177 return principal
178
92 if signature is None:179 if signature is None:
93 # Mark the principal so that application code can check that the180 # Mark the principal so that application code can check that the
94 # user was weakly authenticated.181 # user was weakly authenticated.
@@ -255,7 +342,7 @@
255 continue342 continue
256343
257 try:344 try:
258 principal = authenticateEmail(mail)345 principal = authenticateEmail(mail, raw_mail)
259 except InvalidSignature, error:346 except InvalidSignature, error:
260 _handle_user_error(error, mail)347 _handle_user_error(error, mail)
261 continue348 continue
262349
=== added file 'lib/lp/services/mail/tests/test_dkim.py'
--- lib/lp/services/mail/tests/test_dkim.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/mail/tests/test_dkim.py 2010-07-12 20:52:02 +0000
@@ -0,0 +1,258 @@
1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Test DKIM-signed messages"""
5
6__metaclass__ = type
7
8import logging
9import unittest
10
11from StringIO import StringIO
12
13import dkim
14import dns.resolver
15
16from zope.component import getUtility
17
18from canonical.launchpad.mail import (
19 incoming,
20 signed_message_from_string,
21 )
22from canonical.launchpad.mail.incoming import (
23 authenticateEmail, )
24from canonical.launchpad.interfaces.mail import IWeaklyAuthenticatedPrincipal
25from lp.testing import TestCaseWithFactory
26from canonical.testing.layers import DatabaseFunctionalLayer
27
28
29# sample private key made with 'openssl genrsa' and public key using 'openssl
30# rsa -pubout'. Not really the key for canonical.com ;-)
31sample_privkey = """\
32-----BEGIN RSA PRIVATE KEY-----
33MIIBOwIBAAJBANmBe10IgY+u7h3enWTukkqtUD5PR52Tb/mPfjC0QJTocVBq6Za/
34PlzfV+Py92VaCak19F4WrbVTK5Gg5tW220MCAwEAAQJAYFUKsD+uMlcFu1D3YNaR
35EGYGXjJ6w32jYGJ/P072M3yWOq2S1dvDthI3nRT8MFjZ1wHDAYHrSpfDNJ3v2fvZ
36cQIhAPgRPmVYn+TGd59asiqG1SZqh+p+CRYHW7B8BsicG5t3AiEA4HYNOohlgWan
378tKgqLJgUdPFbaHZO1nDyBgvV8hvWZUCIQDDdCq6hYKuKeYUy8w3j7cgJq3ih922
382qNWwdJCfCWQbwIgTY0cBvQnNe0067WQIpj2pG7pkHZR6qqZ9SE+AjNTHX0CIQCI
39Mgq55Y9MCq5wqzy141rnxrJxTwK9ABo3IAFMWEov3g==
40-----END RSA PRIVATE KEY-----
41"""
42
43sample_pubkey = """\
44-----BEGIN PUBLIC KEY-----
45MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBANmBe10IgY+u7h3enWTukkqtUD5PR52T
46b/mPfjC0QJTocVBq6Za/PlzfV+Py92VaCak19F4WrbVTK5Gg5tW220MCAwEAAQ==
47-----END PUBLIC KEY-----
48"""
49
50sample_dns = """\
51k=rsa; \
52p=MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBANmBe10IgY+u7h3enWTukkqtUD5PR52T\
53b/mPfjC0QJTocVBq6Za/PlzfV+Py92VaCak19F4WrbVTK5Gg5tW220MCAwEAAQ=="""
54
55
56plain_content = """\
57From: Foo Bar <foo.bar@canonical.com>
58Date: Fri, 1 Apr 2010 00:00:00 +1000
59Subject: yet another comment
60To: 1@bugs.staging.launchpad.net
61
62 importance critical
63
64Why isn't this fixed yet?"""
65
66
67class TestDKIM(TestCaseWithFactory):
68 """Messages can be strongly authenticated by DKIM."""
69
70 layer = DatabaseFunctionalLayer
71
72 def setUp(self):
73 # Login with admin roles as we aren't testing access here.
74 TestCaseWithFactory.setUp(self, 'admin@canonical.com')
75 self._log_output = StringIO()
76 handler = logging.StreamHandler(self._log_output)
77 logger = logging.getLogger('mail-authenticate-dkim')
78 logger.addHandler(handler)
79 self.addCleanup(lambda: logger.removeHandler(handler))
80 self.monkeypatch_dns()
81
82 def fake_signing(self, plain_message, canonicalize=None):
83 if canonicalize is None:
84 canonicalize = (dkim.Relaxed, dkim.Relaxed)
85 dkim_line = dkim.sign(plain_message,
86 selector='example',
87 domain='canonical.com',
88 privkey=sample_privkey,
89 debuglog=self._log_output,
90 canonicalize=canonicalize
91 )
92 assert dkim_line[-1] == '\n'
93 return dkim_line + plain_message
94
95 def monkeypatch_dns(self):
96 self._dns_responses = {}
97 def my_lookup(name):
98 try:
99 return self._dns_responses[name]
100 except KeyError:
101 raise dns.resolver.NXDOMAIN()
102 orig_dnstxt = dkim.dnstxt
103 dkim.dnstxt = my_lookup
104 def restore():
105 dkim.dnstxt = orig_dnstxt
106 self.addCleanup(restore)
107
108 def get_dkim_log(self):
109 return self._log_output.getvalue()
110
111 def assertStronglyAuthenticated(self, principal, signed_message):
112 if IWeaklyAuthenticatedPrincipal.providedBy(principal):
113 self.fail('expected strong authentication; got weak:\n'
114 + self.get_dkim_log() + '\n\n' + signed_message)
115
116 def assertWeaklyAuthenticated(self, principal, signed_message):
117 if not IWeaklyAuthenticatedPrincipal.providedBy(principal):
118 self.fail('expected weak authentication; got strong:\n'
119 + self.get_dkim_log() + '\n\n' + signed_message)
120
121 def assertDkimLogContains(self, substring):
122 l = self.get_dkim_log()
123 if l.find(substring) == -1:
124 self.fail("didn't find %r in log: %s" % (substring, l))
125
126 def test_dkim_garbage_pubkey(self):
127 signed_message = self.fake_signing(plain_content)
128 self._dns_responses['example._domainkey.canonical.com.'] = \
129 'aothuaonu'
130 principal = authenticateEmail(signed_message_from_string(signed_message),
131 signed_message)
132 self.assertWeaklyAuthenticated(principal, signed_message)
133 self.assertEqual(principal.person.preferredemail.email,
134 'foo.bar@canonical.com')
135 self.assertDkimLogContains('invalid format in _domainkey txt record')
136
137 def test_dkim_valid_strict(self):
138 signed_message = self.fake_signing(plain_content,
139 canonicalize=(dkim.Simple, dkim.Simple))
140 self._dns_responses['example._domainkey.canonical.com.'] = \
141 sample_dns
142 principal = authenticateEmail(signed_message_from_string(signed_message),
143 signed_message)
144 self.assertStronglyAuthenticated(principal, signed_message)
145 self.assertEqual(principal.person.preferredemail.email,
146 'foo.bar@canonical.com')
147
148 def test_dkim_valid(self):
149 signed_message = self.fake_signing(plain_content)
150 self._dns_responses['example._domainkey.canonical.com.'] = \
151 sample_dns
152 principal = authenticateEmail(signed_message_from_string(signed_message),
153 signed_message)
154 self.assertStronglyAuthenticated(principal, signed_message)
155 self.assertEqual(principal.person.preferredemail.email,
156 'foo.bar@canonical.com')
157
158 def test_dkim_untrusted_signer(self):
159 # Valid signature from an untrusted domain -> untrusted
160 signed_message = self.fake_signing(plain_content)
161 self._dns_responses['example._domainkey.canonical.com.'] = \
162 sample_dns
163 saved_domains = incoming._trusted_dkim_domains[:]
164 def restore():
165 incoming._trusted_dkim_domains = saved_domains
166 self.addCleanup(restore)
167 incoming._trusted_dkim_domains = []
168 principal = authenticateEmail(signed_message_from_string(signed_message),
169 signed_message)
170 self.assertWeaklyAuthenticated(principal, signed_message)
171 self.assertEqual(principal.person.preferredemail.email,
172 'foo.bar@canonical.com')
173
174 def test_dkim_signing_irrelevant(self):
175 # It's totally valid for a message to be signed by a domain other than
176 # that of the From-sender, if that domain is relaying the message.
177 # However, we shouldn't then trust the purported sender, because they
178 # might have just made it up rather than relayed it.
179 tweaked_message = plain_content.replace('foo.bar@canonical.com',
180 'steve.alexander@ubuntulinux.com')
181 signed_message = self.fake_signing(tweaked_message)
182 self._dns_responses['example._domainkey.canonical.com.'] = \
183 sample_dns
184 principal = authenticateEmail(signed_message_from_string(signed_message),
185 signed_message)
186 self.assertWeaklyAuthenticated(principal, signed_message)
187 # should come from From, not the dkim signature
188 self.assertEqual(principal.person.preferredemail.email,
189 'steve.alexander@ubuntulinux.com')
190
191 def test_dkim_changed_from_address(self):
192 # if the address part of the message has changed, it's detected. we
193 # still treat this as weakly authenticated by the purported From-header
194 # sender, though perhaps in future we would prefer to reject these
195 # messages.
196 signed_message = self.fake_signing(plain_content)
197 self._dns_responses['example._domainkey.canonical.com.'] = \
198 sample_dns
199 fiddled_message = signed_message.replace('From: Foo Bar <foo.bar@canonical.com>',
200 'From: Carlos <carlos@canonical.com>')
201 principal = authenticateEmail(signed_message_from_string(fiddled_message),
202 fiddled_message)
203 self.assertWeaklyAuthenticated(principal, fiddled_message)
204 # should come from From, not the dkim signature
205 self.assertEqual(principal.person.preferredemail.email,
206 'carlos@canonical.com')
207
208 def test_dkim_changed_from_realname(self):
209 # if the real name part of the message has changed, it's detected
210 signed_message = self.fake_signing(plain_content)
211 self._dns_responses['example._domainkey.canonical.com.'] = \
212 sample_dns
213 fiddled_message = signed_message.replace('From: Foo Bar <foo.bar@canonical.com>',
214 'From: Evil Foo <foo.bar@canonical.com>')
215 principal = authenticateEmail(signed_message_from_string(fiddled_message),
216 fiddled_message)
217 # we don't care about the real name for determining the principal
218 self.assertWeaklyAuthenticated(principal, fiddled_message)
219 self.assertEqual(principal.person.preferredemail.email,
220 'foo.bar@canonical.com')
221
222 def test_dkim_nxdomain(self):
223 # if there's no DNS entry for the pubkey
224 # it should be handled decently
225 signed_message = self.fake_signing(plain_content)
226 principal = authenticateEmail(signed_message_from_string(signed_message),
227 signed_message)
228 self.assertWeaklyAuthenticated(principal, signed_message)
229 self.assertEqual(principal.person.preferredemail.email,
230 'foo.bar@canonical.com')
231
232 def test_dkim_message_unsigned(self):
233 # degenerate case: no signature treated as weakly authenticated
234 principal = authenticateEmail(signed_message_from_string(plain_content),
235 plain_content)
236 self.assertWeaklyAuthenticated(principal, plain_content)
237 self.assertEqual(principal.person.preferredemail.email,
238 'foo.bar@canonical.com')
239 # the library doesn't log anything if there's no header at all
240
241 def test_dkim_body_mismatch(self):
242 # The message message has a syntactically valid DKIM signature that
243 # doesn't actually correspond to what was signed. We log something about
244 # this but we don't want to drop the message.
245 signed_message = self.fake_signing(plain_content)
246 signed_message += 'blah blah'
247 self._dns_responses['example._domainkey.canonical.com.'] = \
248 sample_dns
249 principal = authenticateEmail(signed_message_from_string(signed_message),
250 signed_message)
251 self.assertWeaklyAuthenticated(principal, signed_message)
252 self.assertEqual(principal.person.preferredemail.email,
253 'foo.bar@canonical.com')
254 self.assertDkimLogContains('body hash mismatch')
255
256
257def test_suite():
258 return unittest.TestLoader().loadTestsFromName(__name__)
0259
=== modified file 'lib/lp/services/mail/tests/test_signedmessage.py'
--- lib/lp/services/mail/tests/test_signedmessage.py 2009-06-30 16:56:07 +0000
+++ lib/lp/services/mail/tests/test_signedmessage.py 2010-07-12 20:52:02 +0000
@@ -28,7 +28,7 @@
28from canonical.testing.layers import DatabaseFunctionalLayer28from canonical.testing.layers import DatabaseFunctionalLayer
2929
30class TestSignedMessage(TestCaseWithFactory):30class TestSignedMessage(TestCaseWithFactory):
31 """Test SignedMessage class correctly extracts the GPG signatures."""31 """Test SignedMessage class correctly extracts and verifies the GPG signatures."""
3232
33 layer = DatabaseFunctionalLayer33 layer = DatabaseFunctionalLayer
3434
@@ -45,7 +45,7 @@
45 msg = signed_message_from_string(email_message.as_string())45 msg = signed_message_from_string(email_message.as_string())
46 self.assertIs(None, msg.signedContent)46 self.assertIs(None, msg.signedContent)
47 self.assertIs(None, msg.signature)47 self.assertIs(None, msg.signature)
48 principle = authenticateEmail(msg)48 principle = authenticateEmail(msg, msg.as_string())
49 self.assertEqual(sender, principle.person)49 self.assertEqual(sender, principle.person)
50 self.assertTrue(50 self.assertTrue(
51 IWeaklyAuthenticatedPrincipal.providedBy(principle))51 IWeaklyAuthenticatedPrincipal.providedBy(principle))
@@ -73,7 +73,7 @@
73 # the principle is set to the sender, but weakly authenticated.73 # the principle is set to the sender, but weakly authenticated.
74 sender = self.factory.makePerson()74 sender = self.factory.makePerson()
75 msg = self._get_clearsigned_for_person(sender)75 msg = self._get_clearsigned_for_person(sender)
76 principle = authenticateEmail(msg)76 principle = authenticateEmail(msg, msg.as_string())
77 self.assertIsNot(None, msg.signature)77 self.assertIsNot(None, msg.signature)
78 self.assertEqual(sender, principle.person)78 self.assertEqual(sender, principle.person)
79 self.assertTrue(79 self.assertTrue(
@@ -83,7 +83,7 @@
83 # The test keys belong to Sample Person.83 # The test keys belong to Sample Person.
84 sender = getUtility(IPersonSet).getByEmail('test@canonical.com')84 sender = getUtility(IPersonSet).getByEmail('test@canonical.com')
85 msg = self._get_clearsigned_for_person(sender)85 msg = self._get_clearsigned_for_person(sender)
86 principle = authenticateEmail(msg)86 principle = authenticateEmail(msg, msg.as_string())
87 self.assertIsNot(None, msg.signature)87 self.assertIsNot(None, msg.signature)
88 self.assertEqual(sender, principle.person)88 self.assertEqual(sender, principle.person)
89 self.assertFalse(89 self.assertFalse(
@@ -127,7 +127,7 @@
127 # the principle is set to the sender, but weakly authenticated.127 # the principle is set to the sender, but weakly authenticated.
128 sender = self.factory.makePerson()128 sender = self.factory.makePerson()
129 msg = self._get_detached_message_for_person(sender)129 msg = self._get_detached_message_for_person(sender)
130 principle = authenticateEmail(msg)130 principle = authenticateEmail(msg, msg.as_string())
131 self.assertIsNot(None, msg.signature)131 self.assertIsNot(None, msg.signature)
132 self.assertEqual(sender, principle.person)132 self.assertEqual(sender, principle.person)
133 self.assertTrue(133 self.assertTrue(
@@ -137,7 +137,7 @@
137 # Test a detached correct signature.137 # Test a detached correct signature.
138 sender = getUtility(IPersonSet).getByEmail('test@canonical.com')138 sender = getUtility(IPersonSet).getByEmail('test@canonical.com')
139 msg = self._get_detached_message_for_person(sender)139 msg = self._get_detached_message_for_person(sender)
140 principle = authenticateEmail(msg)140 principle = authenticateEmail(msg, msg.as_string())
141 self.assertIsNot(None, msg.signature)141 self.assertIsNot(None, msg.signature)
142 self.assertEqual(sender, principle.person)142 self.assertEqual(sender, principle.person)
143 self.assertFalse(143 self.assertFalse(
144144
=== modified file 'setup.py'
--- setup.py 2010-07-06 08:58:00 +0000
+++ setup.py 2010-07-12 20:52:02 +0000
@@ -29,6 +29,8 @@
29 'chameleon.core',29 'chameleon.core',
30 'chameleon.zpt',30 'chameleon.zpt',
31 'cssutils',31 'cssutils',
32 # Required for pydkim
33 'dnspython',
32 'feedvalidator',34 'feedvalidator',
33 'funkload',35 'funkload',
34 'launchpadlib',36 'launchpadlib',
@@ -51,6 +53,7 @@
51 'paramiko',53 'paramiko',
52 'python-memcached',54 'python-memcached',
53 'pyasn1',55 'pyasn1',
56 'pydkim',
54 'python-openid',57 'python-openid',
55 'pytz',58 'pytz',
56 # This appears to be a broken indirect dependency from zope.security:59 # This appears to be a broken indirect dependency from zope.security:
5760
=== modified file 'versions.cfg'
--- versions.cfg 2010-07-09 15:10:07 +0000
+++ versions.cfg 2010-07-12 20:52:02 +0000
@@ -13,6 +13,8 @@
13# Required by Windmill to run on 2.413# Required by Windmill to run on 2.4
14ctypes = 1.0.214ctypes = 1.0.2
15docutils = 0.515docutils = 0.5
16# Required by pydkim
17dnspython = 1.7.1
16elementtree = 1.2.6-2005031618elementtree = 1.2.6-20050316
17epydoc = 3.0.119epydoc = 3.0.1
18feedvalidator = 0.0.0DEV-r104920feedvalidator = 0.0.0DEV-r1049
@@ -47,6 +49,7 @@
47PasteDeploy = 1.3.349PasteDeploy = 1.3.3
48pyasn1 = 0.0.9a50pyasn1 = 0.0.9a
49pycrypto = 2.0.151pycrypto = 2.0.1
52pydkim = 0.3-bug-587783-2
50pyOpenSSL = 0.1053pyOpenSSL = 0.10
51python-memcached = 1.4554python-memcached = 1.45
52python-openid = 2.2.155python-openid = 2.2.1