June 2014 - tor-commits - lists.torproject.org

[bridgedb/master] Add unittest for MailMessage.getRecipient() w/ a bad email address.
by isis＠torproject.org 06 Jun '14

06 Jun '14

commit 3fda7f6365fa8445398563391f5ddcda1247f5e7 Author: Isis Lovecruft <isis(a)torproject.org> Date: Wed May 21 15:01:07 2014 +0000 Add unittest for MailMessage.getRecipient() w/ a bad email address. This should still return our configured EMAIL_SMTP_FROM_ADDR email address. --- lib/bridgedb/test/test_email_server.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/bridgedb/test/test_email_server.py b/lib/bridgedb/test/test_email_server.py index f58b36f..d7f2922 100644 --- a/lib/bridgedb/test/test_email_server.py +++ b/lib/bridgedb/test/test_email_server.py @@ -323,6 +323,17 @@ class MailMessageTests(unittest.TestCase): recipient = self.message.getRecipient(incoming) self.assertEqual(recipient, self.context.fromAddr) + def test_MailMessage_getRecipient_bad_address(self): + """MailMessage.getRecipient() for an incoming email sent to a malformed + email address should log an smtp.AddressError and then return our + configured email address. + """ + self._getIncomingLines() + self.message.lines[1] = 'To: ><@><<<>>.foo' + incoming = self.message.getIncomingMessage() + recipient = self.message.getRecipient(incoming) + self.assertEqual(recipient, self.context.fromAddr) + def test_MailMessage_reply_noFrom(self): """A received email without a "From:" or "Sender:" header shouldn't receive a response.

1 0

[bridgedb/master] Simplify MailResponse class by not implementing smtp.IMessage.
by isis＠torproject.org 06 Jun '14

06 Jun '14

commit cbbbbf8aefc5061be6f28c4c09104ebc9d18d6a9 Author: Isis Lovecruft <isis(a)torproject.org> Date: Wed May 21 04:14:22 2014 +0000 Simplify MailResponse class by not implementing smtp.IMessage. We aren't using it as an SMTP message delivery class anywhere, so there's no need to have the extra methods. * REMOVE bridgedb.email.server.MailResponse.lineReceived() * REMOVE bridgedb.email.server.MailResponse.eomReceived() * REMOVE bridgedb.email.server.MailResponse.connectionLost() * REMOVE zope.interface implementation declaration of twisted.mail.smtp.IMessage by b.e.s.MailResponse. --- lib/bridgedb/email/server.py | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/lib/bridgedb/email/server.py b/lib/bridgedb/email/server.py index 09df526..077bc17 100644 --- a/lib/bridgedb/email/server.py +++ b/lib/bridgedb/email/server.py @@ -286,9 +286,6 @@ class MailResponse(object): :cvar mailfile: An in-memory file for storing the formatted headers and body of the response email. """ - - implements(smtp.IMessage) - _buff = buffer if NEW_BUFFER_INTERFACE else unicode mailfile = io.BytesIO if NEW_BUFFER_INTERFACE else io.StringIO @@ -437,33 +434,6 @@ class MailResponse(object): body, _ = gpgSignMessage(self.gpgContext, body) self.writelines(body) - # The following methods implement the IMessage interface. - - def lineReceived(self, line): - """Called when we receive a line from an underlying transport.""" - self.write(line) - - def eomRecieved(self): - """Called when we receive an EOM. - - :rtype: :api:`twisted.internet.defer.Deferred` - :returns: A ``Deferred`` which has already been callbacked with the - entire response email contents retrieved from - :meth:`readContents`. - """ - contents = self.readContents() - if not self.closed: - self.connectionLost() - return defer.succeed(contents) - - def connectionLost(self): - """Called if we die partway through reading a message. - - Truncate the :cvar:`mailfile` to null length, then close it. - """ - self.mailfile.truncate(0) - self.mailfile.close() - class MailMessage(object): """Plugs into the Twisted Mail and receives an incoming message.

1 0

[bridgedb/master] Add 'pragma:' directive to MailMessage.reply() errback function.
by isis＠torproject.org 06 Jun '14

06 Jun '14

commit 8a71b50e9d7e81de011cb5c92d578e6b69e9fa0f Author: Isis Lovecruft <isis(a)torproject.org> Date: Wed May 21 04:25:29 2014 +0000 Add 'pragma:' directive to MailMessage.reply() errback function. We can't really test it, since it's a nested function. However, we shouldn't need to, all it does is log the twisted.python.failure.Failure. --- lib/bridgedb/email/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bridgedb/email/server.py b/lib/bridgedb/email/server.py index 2684d00..dd5b4fb 100644 --- a/lib/bridgedb/email/server.py +++ b/lib/bridgedb/email/server.py @@ -559,7 +559,7 @@ class MailMessage(object): """ logging.info("Got an email; deciding whether to reply.") - def _replyEB(fail): + def _replyEB(fail): # pragma: no cover """Errback for a :api:`twisted.mail.smtp.SMTPSenderFactory`. :param fail: A :api:`twisted.python.failure.Failure` which occurred during

1 0

[bridgedb/master] Add unittest for MailMessage.getRecipient() w/ email address that isn't ours.
by isis＠torproject.org 06 Jun '14

06 Jun '14

commit 27107de9b94072bed64ceb29160a783a1624360e Author: Isis Lovecruft <isis(a)torproject.org> Date: Wed May 21 14:58:32 2014 +0000 Add unittest for MailMessage.getRecipient() w/ email address that isn't ours. An email sent to 'notbridgedb(a)yikezors.net' that somehow ends up in our queue should return the EMAIL_SMTP_FROM_ADDRESS that we're configured to use. --- lib/bridgedb/test/test_email_server.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/bridgedb/test/test_email_server.py b/lib/bridgedb/test/test_email_server.py index 7805458..f58b36f 100644 --- a/lib/bridgedb/test/test_email_server.py +++ b/lib/bridgedb/test/test_email_server.py @@ -312,6 +312,17 @@ class MailMessageTests(unittest.TestCase): ] self.message.lines = lines + def test_MailMessage_getRecipient_notbridgedb_at_yikezors_dot_net(self): + """MailMessage.getRecipient() for an incoming email sent to any email + address other than the one we're listening for should return our + configured address, not the one in the incoming email. + """ + self._getIncomingLines() + self.message.lines[1] = 'To: notbridgedb(a)yikezors.net' + incoming = self.message.getIncomingMessage() + recipient = self.message.getRecipient(incoming) + self.assertEqual(recipient, self.context.fromAddr) + def test_MailMessage_reply_noFrom(self): """A received email without a "From:" or "Sender:" header shouldn't receive a response.

1 0

[bridgedb/master] Add unittest for MailMessage.getRecipient() which exposes 2.5 bugs.
by isis＠torproject.org 06 Jun '14

06 Jun '14

commit f1b7d03cf1e2dfbb9e11897d0bb07b43f5ad33e5 Author: Isis Lovecruft <isis(a)torproject.org> Date: Wed May 21 15:02:17 2014 +0000 Add unittest for MailMessage.getRecipient() which exposes 2.5 bugs. Bug #1: ------- BridgeDB's current code will accept an incoming email with a 'To: givemebridges(a)serious.ly' header. However, BridgeDB's reply will still contain: 'From: bridges(a)torproject.org' Obviously, it *shouldn't* be possible for any email whose SMTP RCPT TO domain is 'serious.ly' to actually end up in BridgeDB's mail queue. Though, if the outside SMTP layer is sent to '[bridges|ponticum].torproject.org' (with MAIL FROM a gmail/yahoo address), these messages still end up in BridgeDB's mail queue. The following netcat session demonstrates that this is possible: ∃!isisⒶwintermute:(master *$=)~ ∴ torsocks nc bridges.torproject.org 25 220 ponticum.torproject.org ESMTP Postfix (Debian/GNU) HELO ponticum.torproject.org 250 ponticum.torproject.org MAIL FROM: isisgrimalkin(a)gmail.com 250 2.1.0 Ok RCPT TO: bridges(a)bridges.torproject.org 250 2.1.5 Ok DATA 354 End data with <CR><LF>.<CR><LF> From: isislovecruft(a)gmail.com To: givemebridgesrightnow(a)serious.ly Subject: mwhahaha get transport obfs3 . 250 2.0.0 Ok: queued as F03972834F QUIT 221 2.0.0 Bye This request resulted in the following debug logs: _______________________________________________________________________________ 15:30:31 DEBUG L690:server.validateFrom() ORIGIN: "'<bridgedb@ponticum>'" 15:30:31 DEBUG L699:server.validateFrom() Got canonical domain: 'ponticum' 15:30:31 DEBUG L495:server.lineReceived() > Received: from ponticum (ponticum [127.0.0.1]) for <bridges@bridgedb>; Wed, 21 May 2014 15:30:31 +0000 15:30:31 DEBUG L495:server.lineReceived() > From isisgrimalkin(a)gmail.com Wed May 21 15:30:31 2014 15:30:31 DEBUG L495:server.lineReceived() > X-Original-To: bridges(a)bridges.torproject.org 15:30:31 DEBUG L495:server.lineReceived() > Delivered-To: bridgedb(a)ponticum.torproject.org 15:30:31 DEBUG L495:server.lineReceived() > Received: from ponticum.torproject.org (kpebetka.net [95.79.25.182]) 15:30:31 DEBUG L495:server.lineReceived() > by ponticum.torproject.org (Postfix) with SMTP id F03972834F 15:30:31 DEBUG L495:server.lineReceived() > for <bridges(a)bridges.torproject.org>; Wed, 21 May 2014 15:29:18 +0000 (UTC) 15:30:31 DEBUG L495:server.lineReceived() > From: isislovecruft(a)gmail.com 15:30:31 DEBUG L495:server.lineReceived() > To: givemebridgesrightnow(a)serious.ly 15:30:31 DEBUG L495:server.lineReceived() > Subject: mwhahaha 15:30:31 DEBUG L495:server.lineReceived() > X-DKIM-Authentication-Results: dunno 15:30:31 DEBUG L495:server.lineReceived() > Date: Wed, 21 May 2014 15:30:31 -0000 15:30:31 DEBUG L495:server.lineReceived() > Message-Id: <1400686231.135135.6548@ponticum> 15:30:31 DEBUG L495:server.lineReceived() > 15:30:31 DEBUG L495:server.lineReceived() > get transport obfs3 15:30:31 DEBUG L495:server.lineReceived() > 15:30:31 INFO L611:server.reply() Got an email; deciding whether to reply. 15:30:31 INFO L646:server.reply() Client requested email translation: en 15:30:31 DEBUG L70:request.determineBridg() Email request was valid. 15:30:31 DEBUG L160:request.withPluggableT() Parsing 'transport' line: 'get transport obfs3' 15:30:31 INFO L169:request.withPluggableT() Email requested transport type: 'obfs3' 15:30:31 DEBUG L81:request.determineBridg() Generating hashring filters for request. 15:30:31 INFO L420:Dist.getBridgesForEmai() Attempting to return for 3 bridges for isislovecruft(a)gmail.com... 15:30:31 DEBUG L445:Dist.getBridgesForEmai() Cache hit frozenset([<function filterBridgesByTransport(obfs3,<class 'ipaddr.IPv4Address'>)>]) 15:30:31 DEBUG L75:Dist.getNumBridgesPerA() Returning 3 bridges from ring of len: 492 15:30:31 DEBUG L1034:Bridges.getBridges() Got duplicate bridge 'edfa2fd66533da52f40424bbe917bd03c8378c2d' in main hashring for position 'eda7f69f7c08bd80861c3afa2921168a007d9ae5'. 15:30:31 DEBUG L1034:Bridges.getBridges() Got duplicate bridge 'ed0b2fd66f398afbf10424bb911790faca9ddb8e' in main hashring for position 'eda7f69f7c08bd80861c3afa2921168a007d9ae5'. 15:30:31 DEBUG L183:server.generateRespons() Email contents: From: bridges(a)torproject.org To: isislovecruft(a)gmail.com Message-ID: <20140521153031.21456.73227139.10726(a)ponticum.torproject.org> In-Reply-To: <1400686231.135135.6548@ponticum> Content-Type: text/plain; charset="utf-8" Date: Wed, 21 May 2014 15:30:31 +0000 Subject: Re: mwhahaha Hey, isislovecruft! [This is an automated message; please do not reply.] Here are your bridges: obfs3 10.1.1.1:1111 d14133856abbba8a65607baebf692162c567bf41 obfs3 10.2.2.2:2222 86f45ab5dcef80a4b1abfcc43579e76f1d0b25a4 obfs3 10.3.3.3:3333 5d55daabd91e041e74f62dcfab1a29c8bb32f0b2 To enter bridges into Tor Browser, follow the instructions on the Tor Browser download page [0] to start Tor Browser. When the 'Tor Network Settings' dialogue pops up, click 'Configure' and follow the wizard until it asks: > Does your Internet Service Provider (ISP) block or otherwise censor connections > to the Tor network? Select 'Yes' and then click 'Next'. To configure your new bridges, copy and paste the bridge lines into the text input box. Finally, click 'Connect', and you should be good to go! If you experience trouble, try clicking the 'Help' button in the 'Tor Network Settings' wizard for further assistance. [0]: https://www.torproject.org/projects/torbrowser.html.en#downloads-beta COMMANDs: (combine COMMANDs to specify multiple options simultaneously) get bridges Request vanilla bridges. get transport [TYPE] Request a Pluggable Transport by TYPE. get help Displays this message. get key Get a copy of BridgeDB's public GnuPG key. get ipv6 Request IPv6 bridges. Currently supported transport TYPEs: obfs2 obfs3 scramblesuit -- <3 BridgeDB ---------------------------------------------------------------------- Public Keys: https://bridges.torproject.org/keys This email was generated with rainbows, unicorns, and sparkles for isislovecruft(a)gmail.com on Wednesday, 21 May, 2014 at 15:30:31. 15:30:31 INFO L655:server.reply() Sending reply to isislovecruft(a)gmail.com _______________________________________________________________________________ Which obviously brings us to the other bug. Bug #2: ------- BridgeDB will accept an email from an arbitrary gmail/yahoo email address at the SMTP layer, and then send the reply to a *different* arbitrary gmail/yahoo email address taken from the contents of the email headers. As you can see in the example above, the SMTP command 'MAIL FROM: isisgrimalkin(a)gmail.com' combined with a 'From: isislovecruft(a)gmail.com' in the email headers within the SMTP DATA segment caused the reply to be sent the reply to the later, when it came from the former. The person reading the response from BridgeDB also has no way to know who originally emailed BridgeDB to cause this email to end up in her inbox. I'm not exactly certain if this is a bug or a feature. While it could be used for sending some junk to an arbitrary gmail/yahoo address, it could also be used as a sort of "Dear BridgeDB, can I have some bridges? Asking for a friend." mechanism. Bug #2.5: --------- Also, "dunno" certainly isn't a valid DKIM signature. --- lib/bridgedb/test/test_email_server.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/bridgedb/test/test_email_server.py b/lib/bridgedb/test/test_email_server.py index d7f2922..32c40ac 100644 --- a/lib/bridgedb/test/test_email_server.py +++ b/lib/bridgedb/test/test_email_server.py @@ -323,6 +323,17 @@ class MailMessageTests(unittest.TestCase): recipient = self.message.getRecipient(incoming) self.assertEqual(recipient, self.context.fromAddr) + def test_MailMessage_getRecipient_givemebridges_at_seriously(self): + """MailMessage.getRecipient() for an incoming email sent to any email + address other than the one we're listening for should return our + configured address, not the one in the incoming email. + """ + self._getIncomingLines() + self.message.lines[1] = 'To: givemebridges(a)serious.ly' + incoming = self.message.getIncomingMessage() + recipient = self.message.getRecipient(incoming) + self.assertEqual(recipient, self.context.fromAddr) + def test_MailMessage_getRecipient_bad_address(self): """MailMessage.getRecipient() for an incoming email sent to a malformed email address should log an smtp.AddressError and then return our

1 0

[bridgedb/master] Cleanup b.e.server.MailDelivery unittests.
by isis＠torproject.org 06 Jun '14

06 Jun '14

commit ec15fd9506548c06869c4d65f22ba0550545da6b Author: Isis Lovecruft <isis(a)torproject.org> Date: Thu May 29 18:17:24 2014 +0000 Cleanup b.e.server.MailDelivery unittests. --- lib/bridgedb/test/test_email_server.py | 148 +++++++++++++++++++++++++------- 1 file changed, 118 insertions(+), 30 deletions(-) diff --git a/lib/bridgedb/test/test_email_server.py b/lib/bridgedb/test/test_email_server.py index 32c40ac..7015339 100644 --- a/lib/bridgedb/test/test_email_server.py +++ b/lib/bridgedb/test/test_email_server.py @@ -32,6 +32,10 @@ from bridgedb.test.util import TestCaseMixin from twisted.python import log from twisted.internet import defer from twisted.internet import reactor +from twisted.mail.smtp import SMTPBadRcpt +from twisted.mail.smtp import SMTPBadSender +from twisted.mail.smtp import User +from twisted.mail.smtp import Address from twisted.test import proto_helpers from twisted.trial import unittest @@ -417,68 +421,152 @@ class MailMessageTests(unittest.TestCase): return ret -class MailDeliveryTest(unittest.TestCase): +class MailDeliveryTests(unittest.TestCase): """Unittests for :class:`email.server.MailDelivery`.""" def setUp(self): + """Set up our :class:`server.MailDelivery` instance, and reset the + following ``TestCase`` attributes to ``None``: + - ``helo`` + - ``proto`` + - ``origin`` + - ``user`` + """ self.config = _createConfig() self.context = _createMailContext(self.config) self.delivery = server.MailDelivery() - self.helo = ('fubar.example.com', '127.0.0.1') - self.origin = server.smtp.Address('user(a)example.com') - self.users = [server.smtp.User('bridges', self.helo, None, self.origin),] - def tets_MailDelivery(self): + self.helo = None + self.proto = None + self.origin = None + self.user = None + + def tearDown(self): + """Reset all TestCase instance attributes between each test run.""" + self.helo = None + self.proto = None + self.origin = None + self.user = None + + def _createProtocolWithHost(self, host): + """Mock a Protocol which has a ``host`` attribute. + + We don't currently use any of the ``IProtocol`` methods of the + returned ``twisted.test.proto_helpers.AccumulatingProtocol``, and so + this could be any class, although a mocked ``IProtocol`` implementer + was chosen for completeness and realism's sakes. + + :param str host: A domain name or IP address. + :rtype: :api:`twisted.test.proto_helpers.AccumulatingProtocol` + :returns: A Protocol instance which has its ``host`` attribute set to + the given **host**, so that an :api:`twisted.mail.smtp.User` can + be constructed with it. + """ + self.proto = proto_helpers.AccumulatingProtocol() + self.proto.host = host + + def _createUser(self, username, domain, ipaddress): + """Create a ``twisted.mail.smtp.User`` for testing. + + :param str username: The local part of the client's email address. + :param str domain: The host part of the client's email address. + :param str ipaddress: The IP address of the client's mail server. + """ + self.helo = (domain, ipaddress) + self._createProtocolWithHost(domain) + self.origin = Address('@'.join((username, domain,))) + self.user = User(username, self.helo, self.proto, self.origin) + + def _setUpMAILFROM(self): + """Set up the parameters for emulating a connected client sending a + SMTP 'MAIL FROM:' command to us. + + The default is to emulate sending: ``MAIL FROM: client(a)example.com``. + """ + self.helo = ('localhost', '127.0.0.1') + self.origin = server.smtp.Address('client(a)example.com') + self.delivery.setBridgeDBContext(self.context) + + def _setUpRCPTTO(self, username=None): + """Set up the parameters for emulating a connected client sending a + SMTP 'RCPT TO:' command to us. + + The default is to emulate sending: ``RCPT TO: bridges@localhost``. + """ + name = username if username is not None else self.config.EMAIL_USERNAME + self._createUser(name, 'localhost', '127.0.0.1') + self.delivery.setBridgeDBContext(self.context) + + def test_MailDelivery_init(self): + """After calling :meth:`server.MailDelivery.__init__`, we should have a + :class:`server.MailDelivery` object instance. + """ self.assertIsInstance(self.delivery, server.MailDelivery) def test_MailDelivery_setBridgeDBContext(self): + """Calling :meth:`server.MailDelivery.setBridgeDBContext` should set + the :ivar:`MailDelivery.context` attribute. + + The ``MailDelivery.context`` should be a :class:`server.MailContext`, + and it should have relevant settings from the config file stored + within it. + """ self.delivery.setBridgeDBContext(self.context) + self.assertIsInstance(self.delivery.context, server.MailContext) + self.assertEqual(self.delivery.context.smtpFromAddr, + self.config.EMAIL_SMTP_FROM_ADDR) def test_MailDelivery_receivedHeader(self): - self.delivery.setBridgeDBContext(self.context) - hdr = self.delivery.receivedHeader(self.helo, self.origin, self.users) - self.assertTrue(hdr) - self.assertSubstring("Received: from fubar.example.com", hdr) + """The email resulting from a MailDelivery, the latter received from + ``'client(a)example.com'`` should contain a header stating: + ``'Received: from example.com'``. + """ + self._createUser('client', 'example.com', '127.0.0.1') + hdr = self.delivery.receivedHeader(self.helo, self.origin, [self.user,]) + self.assertSubstring("Received: from example.com", hdr) def test_MailDelivery_validateFrom(self): """A valid origin should be stored as ``MailDelivery.fromCanonical``.""" - self.delivery.setBridgeDBContext(self.context) + self._setUpMAILFROM() self.delivery.validateFrom(self.helo, self.origin) self.assertEqual(self.delivery.fromCanonical, 'example.com') def test_MailDelivery_validateFrom_unsupportedDomain(self): """A domain not in our canon should raise a SMTPBadSender.""" - self.delivery.setBridgeDBContext(self.context) - helo = ('yo.mama', '0.0.0.0') + self._setUpMAILFROM() origin = server.smtp.Address('throwing.pickles(a)yo.mama') - self.assertRaises(server.smtp.SMTPBadSender, - self.delivery.validateFrom, helo, origin) + self.assertRaises(SMTPBadSender, + self.delivery.validateFrom, self.helo, origin) - def test_MailDelivery_validateFrom_badOriginType(self): - """A non t.m.smtp.Address origin should raise cause an Exception.""" - self.delivery.setBridgeDBContext(self.context) - helo = ('yo.mama', '0.0.0.0') + def test_MailDelivery_validateFrom_origin_notAdressType(self): + """A non ``twisted.mail.smtp.Address`` origin should raise an + AttributeError exception. + """ + self._setUpMAILFROM() origin = 'throwing.pickles(a)yo.mama' - self.delivery.validateFrom(helo, origin) + self.delivery.validateFrom(self.helo, origin) def test_MailDelivery_validateTo(self): """Should return a callable that results in a MailMessage.""" - self.delivery.setBridgeDBContext(self.context) - ret = self.delivery.validateTo(self.users[0]) - self.assertIsInstance(ret, types.FunctionType) + self._setUpRCPTTO() + validated = self.delivery.validateTo(self.user) + self.assertIsInstance(validated, types.FunctionType) + self.assertIsInstance(validated(), server.MailMessage) def test_MailDelivery_validateTo_plusAddress(self): """Should return a callable that results in a MailMessage.""" - self.delivery.setBridgeDBContext(self.context) - user = server.smtp.User('bridges+ar', self.helo, None, self.origin) - ret = self.delivery.validateTo(user) - self.assertIsInstance(ret, types.FunctionType) + self._setUpRCPTTO('bridges+ar') + validated = self.delivery.validateTo(self.user) + self.assertIsInstance(validated, types.FunctionType) + self.assertIsInstance(validated(), server.MailMessage) def test_MailDelivery_validateTo_badUsername(self): - self.delivery.setBridgeDBContext(self.context) - user = server.smtp.User('yo.mama', self.helo, None, self.origin) - self.assertRaises(server.smtp.SMTPBadRcpt, - self.delivery.validateTo, user) + """A :class:`server.MailDelivery` which sends a SMTP + ``RCPT TO: yo.mama@localhost`` should raise a + ``twisted.mail.smtp.SMTPBadRcpt`` exception. + """ + self._setUpRCPTTO('yo.mama') + self.assertRaises(SMTPBadRcpt, self.delivery.validateTo, self.user) class SMTPTestCaseMixin(TestCaseMixin):

1 0

[bridgedb/master] Merge branch 'fix/12086-rcptto-other-domain' into fix/9874-automate-email-tests
by isis＠torproject.org 06 Jun '14

06 Jun '14

commit 80fef967c99e23834cd1a283292c9636753a3852 Merge: ec15fd9 5e62580 Author: Isis Lovecruft <isis(a)torproject.org> Date: Thu May 29 19:43:34 2014 +0000 Merge branch 'fix/12086-rcptto-other-domain' into fix/9874-automate-email-tests lib/bridgedb/email/server.py | 71 ++++++++++++++++++++++++-------- lib/bridgedb/test/test_email_server.py | 18 ++++---- 2 files changed, 62 insertions(+), 27 deletions(-)

1 0

[bridgedb/master] Simplify client parameter in b.e.server.createResponseBody().
by isis＠torproject.org 06 Jun '14

06 Jun '14

commit 8a4d6c9e9c4124fae450e148d257756df1f5e3a4 Author: Isis Lovecruft <isis(a)torproject.org> Date: Wed May 28 22:48:43 2014 +0000 Simplify client parameter in b.e.server.createResponseBody(). We can use the str() method on ``twisted.mail.smtp.Address`` objects to recreate the full email address. In other words, these are equivalent: clientAddr = '@'.join([client.local, client.domain]) and clientAddr = str(client) except that the latter looks nicer, so we should use that instead. --- lib/bridgedb/email/server.py | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/lib/bridgedb/email/server.py b/lib/bridgedb/email/server.py index dd5b4fb..79b4a59 100644 --- a/lib/bridgedb/email/server.py +++ b/lib/bridgedb/email/server.py @@ -81,7 +81,7 @@ def checkDKIM(message, rules): return False return True -def createResponseBody(lines, context, clientAddress, lang='en'): +def createResponseBody(lines, context, client, lang='en'): """Parse the **lines** from an incoming email request and determine how to respond. @@ -89,8 +89,8 @@ def createResponseBody(lines, context, clientAddress, lang='en'): client. :type context: class:`MailContext` :param context: The context which contains settings for the email server. - :type clientAddress: :api:`twisted.mail.smtp.Address` - :param clientAddress: The client's email address which should be in the + :type client: :api:`twisted.mail.smtp.Address` + :param client: The client's email address which should be in the :header:`To:` header of the response email. :param str lang: The 2-5 character locale code to use for translating the email. This is obtained from a client sending a email to a valid plus @@ -103,9 +103,7 @@ def createResponseBody(lines, context, clientAddress, lang='en'): string containing the (optionally translated) body for the email response which we should send out. """ - clientAddr = '@'.join([clientAddress.local, clientAddress.domain]) t = translations.installTranslations(lang) - bridges = None try: bridgeRequest = request.determineBridgeRequestOptions(lines) @@ -113,26 +111,26 @@ def createResponseBody(lines, context, clientAddress, lang='en'): # The request was invalid, respond with a help email which explains # valid email commands: if not bridgeRequest.isValid(): - raise EmailRequestedHelp("Email request from %r was invalid." - % clientAddr) + raise EmailRequestedHelp("Email request from '%s' was invalid." + % str(client)) # Otherwise they must have requested bridges: interval = context.schedule.getInterval(time.time()) bridges = context.distributor.getBridgesForEmail( - clientAddr, + str(client), interval, context.nBridges, countryCode=None, bridgeFilterRules=bridgeRequest.filters) except EmailRequestedHelp as error: logging.info(error) - return templates.buildWelcomeText(t, clientAddress) + return templates.buildWelcomeText(t, client) except EmailRequestedKey as error: logging.info(error) - return templates.buildKeyMessage(t, clientAddress) + return templates.buildKeyMessage(t, client) except TooSoonEmail as error: logging.info("Got a mail too frequently: %s." % error) - return templates.buildSpamWarning(t, clientAddress) + return templates.buildSpamWarning(t, client) except (IgnoreEmail, BadEmail) as error: logging.info(error) # Don't generate a response if their email address is unparsable or @@ -146,8 +144,8 @@ def createResponseBody(lines, context, clientAddress, lang='en'): includeFingerprint=context.includeFingerprints, addressClass=bridgeRequest.addressClass, transport=transport, - request=clientAddr) for b in bridges) - return templates.buildAnswerMessage(t, clientAddress, answer) + request=str(client)) for b in bridges) + return templates.buildAnswerMessage(t, client, answer) def generateResponse(fromAddress, clientAddress, body, subject=None, messageID=None, gpgContext=None):

1 0

[bridgedb/master] Fix #12086; disallow emails not to our domain.
by isis＠torproject.org 06 Jun '14

06 Jun '14

commit 5e6258041d492c0748e03079f366bbb0d06d53f9 Author: Isis Lovecruft <isis(a)torproject.org> Date: Wed May 28 23:05:20 2014 +0000 Fix #12086; disallow emails not to our domain. At the SMTP layer, we previously only checked that the email address within the SMTP 'RCPT TO:' command (after being stripped of any '+' aliases) as well as the email address in the 'To:' header both had a username equal to the configured EMAIL_USERNAME. It didn't check that these email addresses were the same, nor did it check the domain at all. This refactors the ``bridgedb.email.server.MailDelivery.validateTo()`` method to check that the domain name in the SMTP 'RCPT TO:' command is either our domain or a subdomain of it. At the email layer, we now check that the email was sent to a domain which is either our domain or a subdomain of it. We also check that the username matches our configured username (after '+' aliases have been removed). * FIXES #12086, a bug where BridgeDB would accept emails addressed to addresses such as 'givemebridges(a)serious.ly'. * CHANGES MailDelivery.validateTo() to check domain names and usernames in the SMTP 'RCPT TO' command that an incoming email was received for. * RENAME MailMessage.getRecipient() → MailMessage.getMailFrom() in bridgedb.email.server module. * CHANGES MailMessage.getMailFrom() to check domain names and usernames in the 'To:' header of an incoming email. --- lib/bridgedb/email/server.py | 71 ++++++++++++++++++++++++-------- lib/bridgedb/test/test_email_server.py | 18 ++++---- 2 files changed, 62 insertions(+), 27 deletions(-) diff --git a/lib/bridgedb/email/server.py b/lib/bridgedb/email/server.py index 79b4a59..593713b 100644 --- a/lib/bridgedb/email/server.py +++ b/lib/bridgedb/email/server.py @@ -506,7 +506,7 @@ class MailMessage(object): else: return client - def getRecipient(self, incoming): + def getMailFrom(self, incoming): """Find our address in the recipients list of the **incoming** message. :type incoming: :api:`twisted.mail.smtp.rfc822.Message` @@ -516,21 +516,42 @@ class MailMessage(object): :return: Our address from the recipients list. If we can't find it return our default ``SMTP_FROM_ADDRESS`` from the config file. """ - ourAddress = self.context.fromAddr + logging.debug("Searching for our email address in 'To:' header...") + + ours = None try: - ourAddress = smtp.Address(ourAddress) + ourAddress = smtp.Address(self.context.fromAddr) allRecipients = incoming.getaddrlist("To") + for _, addr in allRecipients: recipient = smtp.Address(addr) - # See if the user looks familiar. We do a 'find' instead of - # compare because we might have a '+' address here. - if recipient.local.find(ourAddress.local) != -1: - return '@'.join([recipient.local, recipient.domain]) - except smtp.AddressError as error: - logging.warn(error) + if not (ourAddress.domain in recipient.domain): + logging.debug(("Not our domain (%s) or subdomain, skipping" + " email address: %s") + % (ourAddress.domain, str(recipient))) + continue + # The recipient's username should at least start with ours, + # but it still might be a '+' address. + if not recipient.local.startswith(ourAddress.local): + logging.debug(("Username doesn't begin with ours, skipping" + " email address: %s") % str(recipient)) + continue + # Ignore everything after the first '+', if there is one. + beforePlus = recipient.local.split('+', 1)[0] + if beforePlus == ourAddress.local: + ours = str(recipient) + if not ours: + raise BadEmail(allRecipients) - return ourAddress + except Exception as error: + logging.error(("Couldn't find our email address in incoming email " + "headers: %r" % error)) + # Just return the email address that we're configured to use: + ours = self.context.fromAddr + + logging.debug("Found our email address: %s." % ours) + return ours def getCanonicalDomain(self, domain): try: @@ -571,7 +592,7 @@ class MailMessage(object): d.addErrback(_replyEB) incoming = self.getIncomingMessage() - recipient = self.getRecipient(incoming) + recipient = self.getMailFrom(incoming) client = self.getClientAddress(incoming) if not client: @@ -693,13 +714,27 @@ class MailDelivery(object): :returns: A parameterless function which returns an instance of :class:`SMTPMessage`. """ - u = user.dest.local - # Hasplus? If yes, strip '+foo' - idx = u.find('+') - if idx != -1: - u = u[:idx] - if u != self.context.username: - raise smtp.SMTPBadRcpt(user) + logging.debug("Validating SMTP 'RCPT TO:' email address...") + + recipient = user.dest + ourAddress = smtp.Address(self.context.smtpFromAddr) + + if not (ourAddress.domain in recipient.domain): + logging.debug(("Not our domain (%s) or subdomain, skipping" + " SMTP 'RCPT TO' address: %s") + % (ourAddress.domain, str(recipient))) + raise smtp.SMTPBadRcpt(str(recipient)) + # The recipient's username should at least start with ours, + # but it still might be a '+' address. + if not recipient.local.startswith(ourAddress.local): + logging.debug(("Username doesn't begin with ours, skipping" + " SMTP 'RCPT TO' address: %s") % str(recipient)) + raise smtp.SMTPBadRcpt(str(recipient)) + # Ignore everything after the first '+', if there is one. + beforePlus = recipient.local.split('+', 1)[0] + if beforePlus != ourAddress.local: + raise smtp.SMTPBadRcpt(str(recipient)) + return lambda: MailMessage(self.context, self.fromCanonical) diff --git a/lib/bridgedb/test/test_email_server.py b/lib/bridgedb/test/test_email_server.py index 7015339..7aff08e 100644 --- a/lib/bridgedb/test/test_email_server.py +++ b/lib/bridgedb/test/test_email_server.py @@ -316,37 +316,37 @@ class MailMessageTests(unittest.TestCase): ] self.message.lines = lines - def test_MailMessage_getRecipient_notbridgedb_at_yikezors_dot_net(self): - """MailMessage.getRecipient() for an incoming email sent to any email + def test_MailMessage_getMailFrom_notbridgedb_at_yikezors_dot_net(self): + """MailMessage.getMailFrom() for an incoming email sent to any email address other than the one we're listening for should return our configured address, not the one in the incoming email. """ self._getIncomingLines() self.message.lines[1] = 'To: notbridgedb(a)yikezors.net' incoming = self.message.getIncomingMessage() - recipient = self.message.getRecipient(incoming) + recipient = str(self.message.getMailFrom(incoming)) self.assertEqual(recipient, self.context.fromAddr) - def test_MailMessage_getRecipient_givemebridges_at_seriously(self): - """MailMessage.getRecipient() for an incoming email sent to any email + def test_MailMessage_getMailFrom_givemebridges_at_seriously(self): + """MailMessage.getMailFrom() for an incoming email sent to any email address other than the one we're listening for should return our configured address, not the one in the incoming email. """ self._getIncomingLines() self.message.lines[1] = 'To: givemebridges(a)serious.ly' incoming = self.message.getIncomingMessage() - recipient = self.message.getRecipient(incoming) + recipient = str(self.message.getMailFrom(incoming)) self.assertEqual(recipient, self.context.fromAddr) - def test_MailMessage_getRecipient_bad_address(self): - """MailMessage.getRecipient() for an incoming email sent to a malformed + def test_MailMessage_getMailFrom_bad_address(self): + """MailMessage.getMailFrom() for an incoming email sent to a malformed email address should log an smtp.AddressError and then return our configured email address. """ self._getIncomingLines() self.message.lines[1] = 'To: ><@><<<>>.foo' incoming = self.message.getIncomingMessage() - recipient = self.message.getRecipient(incoming) + recipient = str(self.message.getMailFrom(incoming)) self.assertEqual(recipient, self.context.fromAddr) def test_MailMessage_reply_noFrom(self):

1 0

[bridgedb/master] Add extra logging to b.e.server.MailResponse methods.
by isis＠torproject.org 06 Jun '14

06 Jun '14

commit e34c3f21d723ab1a9dea8b9fffa7ee2032a86614 Author: Isis Lovecruft <isis(a)torproject.org> Date: Thu May 29 19:59:50 2014 +0000 Add extra logging to b.e.server.MailResponse methods. --- lib/bridgedb/email/server.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/bridgedb/email/server.py b/lib/bridgedb/email/server.py index f2258a6..7904c36 100644 --- a/lib/bridgedb/email/server.py +++ b/lib/bridgedb/email/server.py @@ -332,13 +332,15 @@ class MailResponse(object): def rewind(self): """Rewind to the very beginning of the :cvar:`mailfile`.""" + logging.debug("Rewinding %s.mailfile..." % self.__class__.__name__) self.mailfile.seek(0) def write(self, line): """Any **line** written to me will have ``'\r\n'`` appended to it.""" if line.find('\n') != -1: # If **line** contains newlines, send it to :meth:`writelines` to - # break it up so that we can replace them with '\r\n': + # break it up so that we can replace them: + logging.debug("Found newlines in %r. Calling writelines()." % line) self.writelines(line) else: self.mailfile.write(self._buff(line + '\r\n')) @@ -406,7 +408,9 @@ class MailResponse(object): :param str body: The body of the response email. """ + logging.info("Writing email body...") if self.gpgContext: + logging.info("Attempting to sign email...") body, _ = gpgSignMessage(self.gpgContext, body) self.writelines(body)

1 0