launchpad-reviewers team mailing list archive

[Martin Pool <martinpool@xxxxxxxxx>]

Martin Pool has proposed merging lp:~mbp/launchpad/dkim-flag into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~mbp/launchpad/dkim-flag/+merge/60288

This adds a feature-flag scram switch that can turn off DKIM incoming mail authentication.

I haven't yet merged per-mail-sender scopes so this is not sufficient to let us trust it for only particular users.  But it will let us turn it off in an emergency if necessary.

lint: clean

tested with './bin/test -m test_dkim'
-- 
https://code.launchpad.net/~mbp/launchpad/dkim-flag/+merge/60288
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mbp/launchpad/dkim-flag into lp:launchpad.

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2011-04-11 01:30:37 +0000
+++ lib/lp/services/features/flags.py	2011-05-07 20:34:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __all__ = [
@@ -41,6 +41,10 @@
      'float',
      'Sets the hard request timeout in milliseconds.',
      ''),
+    ('mail.dkim_authentication.disabled',
+     'boolean',
+     'Disable DKIM authentication checks on incoming mail.',
+     ''),
     ('malone.advanced-subscriptions.enabled',
      'boolean',
      'Enables advanced bug subscription features.',

=== modified file 'lib/lp/services/mail/incoming.py'
--- lib/lp/services/mail/incoming.py	2011-04-28 22:41:18 +0000
+++ lib/lp/services/mail/incoming.py	2011-05-07 20:34:39 +0000
@@ -51,6 +51,7 @@
 from canonical.launchpad.webapp.interfaces import IPlacelessAuthUtility
 from canonical.librarian.interfaces import UploadFailed
 from lp.registry.interfaces.person import IPerson
+from lp.services.features import getFeatureFlag
 from lp.services.mail.handlers import mail_handlers
 from lp.services.mail.sendmail import do_paranoid_envelope_to_validation
 from lp.services.mail.signedmessage import signed_message_from_string
@@ -116,6 +117,11 @@
 
     log = logging.getLogger('mail-authenticate-dkim')
     log.setLevel(logging.DEBUG)
+
+    if getFeatureFlag('mail.dkim_authentication.disabled'):
+        log.info('dkim authentication feature disabled')
+        return False
+
     # uncomment this for easier test debugging
     # log.addHandler(logging.FileHandler('/tmp/dkim.log'))
 

=== modified file 'lib/lp/services/mail/tests/test_dkim.py'
--- lib/lp/services/mail/tests/test_dkim.py	2010-10-12 16:53:10 +0000
+++ lib/lp/services/mail/tests/test_dkim.py	2011-05-07 20:34:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test DKIM-signed messages"""
@@ -14,6 +14,7 @@
 
 from canonical.launchpad.interfaces.mail import IWeaklyAuthenticatedPrincipal
 from canonical.launchpad.mail import signed_message_from_string
+from lp.services.features.testing import FeatureFixture
 from lp.services.mail import incoming
 from lp.services.mail.incoming import authenticateEmail
 from canonical.testing.layers import DatabaseFunctionalLayer
@@ -130,6 +131,14 @@
             'foo.bar@xxxxxxxxxxxxx')
         self.assertDkimLogContains('invalid format in _domainkey txt record')
 
+    def test_dkim_disabled(self):
+        """With disabling flag set, mail isn't trusted."""
+        self.useFixture(FeatureFixture({
+            'mail.dkim_authentication.disabled': 'true'}))
+        # A test that would normally pass will now fail
+        self.assertRaises(AssertionError,
+            self.test_dkim_valid_strict)
+
     def test_dkim_valid_strict(self):
         signed_message = self.fake_signing(plain_content,
             canonicalize=(dkim.Simple, dkim.Simple))