Colin Watson has proposed merging lp:~cjwatson/launchpad/email-force-strong-auth into lp:launchpad.
Commit message: Allow rejecting unsigned email from a given user. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cjwatson/launchpad/email-force-strong-auth/+merge/343790 This is a useful strategy when handling incoming email spam from addresses whose associated users are legitimately active (and so can't just be suspended) but which are being forged by spammers. -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/email-force-strong-auth into lp:launchpad.
=== modified file 'lib/lp/registry/browser/person.py' --- lib/lp/registry/browser/person.py 2017-06-01 12:58:53 +0000 +++ lib/lp/registry/browser/person.py 2018-04-23 08:41:01 +0000 @@ -1,4 +1,4 @@ -# Copyright 2009-2017 Canonical Ltd. This software is licensed under the +# Copyright 2009-2018 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). """Person-related view classes.""" @@ -1231,7 +1231,9 @@ label = "Review person" field_names = [ 'name', 'display_name', - 'personal_standing', 'personal_standing_reason'] + 'personal_standing', 'personal_standing_reason', + 'require_strong_email_authentication', + ] custom_widget( 'personal_standing_reason', TextAreaWidget, height=5, width=60) === modified file 'lib/lp/registry/browser/tests/person-admin-views.txt' --- lib/lp/registry/browser/tests/person-admin-views.txt 2015-10-01 10:25:19 +0000 +++ lib/lp/registry/browser/tests/person-admin-views.txt 2018-04-23 08:41:01 +0000 @@ -18,7 +18,8 @@ [] >>> view.field_names ['name', 'display_name', - 'personal_standing', 'personal_standing_reason'] + 'personal_standing', 'personal_standing_reason', + 'require_strong_email_authentication'] >>> view.label 'Review person' @@ -50,6 +51,8 @@ <DBItem PersonalStanding.POOR, ...> >>> print user.personal_standing_reason Zaphod's just this guy. + >>> user.require_strong_email_authentication + False Non administrators cannot access the +review view === modified file 'lib/lp/registry/configure.zcml' --- lib/lp/registry/configure.zcml 2017-06-16 11:10:39 +0000 +++ lib/lp/registry/configure.zcml 2018-04-23 08:41:01 +0000 @@ -1,4 +1,4 @@ -<!-- Copyright 2009-2017 Canonical Ltd. This software is licensed under the +<!-- Copyright 2009-2018 Canonical Ltd. This software is licensed under the GNU Affero General Public License version 3 (see the file LICENSE). --> @@ -1028,7 +1028,8 @@ interface="lp.registry.interfaces.person.IPersonLimitedView"/> <require permission="launchpad.View" - interface="lp.registry.interfaces.person.IPersonViewRestricted"/> + interface="lp.registry.interfaces.person.IPersonViewRestricted + lp.registry.interfaces.person.IPersonModerate"/> <require permission="launchpad.View" interface="lp.registry.interfaces.person.ITeamPublic"/> @@ -1047,6 +1048,7 @@ <require permission="launchpad.Moderate" interface="lp.registry.interfaces.person.IPersonModerateRestricted" + set_schema="lp.registry.interfaces.person.IPersonModerate" set_attributes="name"/> <require permission="launchpad.Special" === modified file 'lib/lp/registry/interfaces/person.py' --- lib/lp/registry/interfaces/person.py 2018-04-17 00:26:25 +0000 +++ lib/lp/registry/interfaces/person.py 2018-04-23 08:41:01 +0000 @@ -542,7 +542,7 @@ description=_("The reason the person's standing is what it is.")) -class IPersonSettings(Interface): +class IPersonSettingsViewRestricted(Interface): """Settings for a person (not a team!) that are used relatively rarely. We store these attributes on a separate object, PersonSettings, to which @@ -554,6 +554,9 @@ class, too. We also may want TeamSettings and PersonTeamSettings in the future. + + These attributes need launchpad.View to see, and launchpad.Edit to + change. """ selfgenerated_bugnotifications = Bool( @@ -569,6 +572,24 @@ required=False, default=False) +class IPersonSettingsModerate(Interface): + """Settings for a person (not a team!) that are used relatively rarely. + + These attributes need launchpad.View to see, and launchpad.Moderate to + change. + """ + + require_strong_email_authentication = Bool( + title=_("Require strong authentication for incoming emails"), + description=_( + "If this option is set, Launchpad will only accept incoming " + "emails from you if it can authenticate them using GPG or DKIM. " + "Launchpad administrators may set this if one of your email " + "addresses is being forged as the sender address for incoming " + "spam."), + required=False, default=False) + + class IPersonPublic(IPrivacy): """Public attributes for a Person. @@ -703,7 +724,8 @@ IHasMergeProposals, IHasMugshot, IHasLocation, IHasRequestedReviews, IObjectWithLocation, IHasBugs, IHasRecipes, IHasTranslationImports, - IPersonSettings, IQuestionsPerson, IHasGitRepositories): + IPersonSettingsViewRestricted, IQuestionsPerson, + IHasGitRepositories): """IPerson attributes that require launchpad.View permission.""" account = Object(schema=IAccount) accountID = Int(title=_('Account ID'), required=True, readonly=True) @@ -1865,6 +1887,10 @@ """ +class IPersonModerate(IPersonSettingsModerate): + """IPerson attributes that the user can see and moderators can change.""" + + class IPersonModerateRestricted(Interface): """IPerson attributes that require launchpad.Moderate permission.""" @@ -1884,10 +1910,14 @@ as_of='devel') +class IPersonSettings(IPersonSettingsViewRestricted, IPersonSettingsModerate): + """A person's settings.""" + + class IPerson(IPersonPublic, IPersonLimitedView, IPersonViewRestricted, - IPersonEditRestricted, IPersonModerateRestricted, - IPersonSpecialRestricted, IHasStanding, ISetLocation, - IHeadingContext): + IPersonEditRestricted, IPersonModerate, + IPersonModerateRestricted, IPersonSpecialRestricted, + IPersonSettings, IHasStanding, ISetLocation, IHeadingContext): """A Person.""" export_as_webservice_entry(plural_name='people') === modified file 'lib/lp/registry/model/person.py' --- lib/lp/registry/model/person.py 2018-04-17 00:26:25 +0000 +++ lib/lp/registry/model/person.py 2018-04-23 08:41:01 +0000 @@ -427,6 +427,8 @@ expanded_notification_footers = BoolCol(notNull=False, default=False) + require_strong_email_authentication = BoolCol(notNull=False, default=False) + def readonly_settings(message, interface): """Make an object that disallows writes to values on the interface. === modified file 'lib/lp/registry/tests/test_person.py' --- lib/lp/registry/tests/test_person.py 2018-04-10 11:10:09 +0000 +++ lib/lp/registry/tests/test_person.py 2018-04-23 08:41:01 +0000 @@ -509,9 +509,9 @@ self.assertEqual(to_person, job.to_person) self.assertEqual(requester, job.requester) - def test_selfgenerated_bugnotifications_none_by_default(self): + def test_selfgenerated_bugnotifications_false_by_default(self): # Default for new accounts is to not get any - # self-generated bug notifications by default. + # self-generated bug notifications. user = self.factory.makePerson() self.assertFalse(user.selfgenerated_bugnotifications) @@ -520,6 +520,23 @@ user = self.factory.makePerson() self.assertFalse(user.expanded_notification_footers) + def test_require_strong_email_authentication_false_by_default(self): + # Default for new accounts is to not require strong email + # authentication. + user = self.factory.makePerson() + self.assertFalse(user.require_strong_email_authentication) + + def test_require_strong_email_authentication_permissions(self): + # A person cannot set require_strong_email_authentication on their + # own account, but a registry expert can. + user = self.factory.makePerson() + with person_logged_in(user): + self.assertRaises( + Unauthorized, setattr, + user, 'require_strong_email_authentication', True) + with celebrity_logged_in('registry_experts'): + user.require_strong_email_authentication = True + def test_canAccess__anonymous(self): # Anonymous users cannot call Person.canAccess() person = self.factory.makePerson() === added file 'lib/lp/services/mail/errortemplates/person-requires-signature.txt' --- lib/lp/services/mail/errortemplates/person-requires-signature.txt 1970-01-01 00:00:00 +0000 +++ lib/lp/services/mail/errortemplates/person-requires-signature.txt 2018-04-23 08:41:01 +0000 @@ -0,0 +1,10 @@ +Launchpad only accepts signed email from your address. + +If you want to use the Launchpad email interface, you will need to go here +to import your OpenPGP key and then use it to sign your messages: + + %(import_url)s + +If you did not knowingly send email to Launchpad, then spammers may be +forging messages as if they were sent from your address, perhaps due to +a compromised address book, and you can safely ignore this message. === modified file 'lib/lp/services/mail/helpers.py' --- lib/lp/services/mail/helpers.py 2015-09-11 16:28:53 +0000 +++ lib/lp/services/mail/helpers.py 2018-04-23 08:41:01 +0000 @@ -1,4 +1,4 @@ -# Copyright 2009-2015 Canonical Ltd. This software is licensed under the +# Copyright 2009-2018 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). __metaclass__ = type @@ -178,8 +178,7 @@ return person_term.value -def ensure_not_weakly_authenticated(signed_msg, context, - error_template='not-signed.txt'): +def ensure_not_weakly_authenticated(signed_msg, context): """Make sure that the current principal is not weakly authenticated. NB: While handling an email, the authentication state is stored partly in === modified file 'lib/lp/services/mail/incoming.py' --- lib/lp/services/mail/incoming.py 2015-03-13 19:05:50 +0000 +++ lib/lp/services/mail/incoming.py 2018-04-23 08:41:01 +0000 @@ -1,4 +1,4 @@ -# Copyright 2009-2013 Canonical Ltd. This software is licensed under the +# Copyright 2009-2018 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). """Functions dealing with mails coming into Launchpad.""" @@ -48,6 +48,7 @@ from lp.services.mail.notification import send_process_error_notification from lp.services.mail.sendmail import do_paranoid_envelope_to_validation from lp.services.mail.signedmessage import signed_message_from_string +from lp.services.webapp import canonical_url from lp.services.webapp.errorlog import ( ErrorReportingUtility, ScriptRequest, @@ -56,7 +57,10 @@ get_current_principal, setupInteraction, ) -from lp.services.webapp.interfaces import IPlacelessAuthUtility +from lp.services.webapp.interfaces import ( + ILaunchBag, + IPlacelessAuthUtility, + ) # Match '\n' and '\r' line endings. That is, all '\r' that are not # followed by a '\n', and all '\n' that are not preceded by a '\r'. @@ -268,11 +272,20 @@ if dkim_trusted_address: log.debug('accepting dkim strongly authenticated mail') setupInteraction(principal, dkim_trusted_address) - return principal else: log.debug("attempt gpg authentication for %r" % person) - return _gpgAuthenticateEmail(mail, principal, person, - signature_timestamp_checker) + principal = _gpgAuthenticateEmail( + mail, principal, person, signature_timestamp_checker) + + if (IWeaklyAuthenticatedPrincipal.providedBy(principal) and + person.require_strong_email_authentication): + import_url = canonical_url( + getUtility(ILaunchBag).user, view_name='+editpgpkeys') + error_message = get_error_message( + 'person-requires-signature.txt', import_url=import_url) + raise IncomingEmailError(error_message) + + return principal def _gpgAuthenticateEmail(mail, principal, person, === modified file 'lib/lp/services/mail/tests/test_signedmessage.py' --- lib/lp/services/mail/tests/test_signedmessage.py 2016-03-23 17:55:39 +0000 +++ lib/lp/services/mail/tests/test_signedmessage.py 2018-04-23 08:41:01 +0000 @@ -1,4 +1,4 @@ -# Copyright 2009-2016 Canonical Ltd. This software is licensed under the +# Copyright 2009-2018 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). """Test the SignedMessage class.""" @@ -19,6 +19,7 @@ from lp.registry.interfaces.person import IPersonSet from lp.services.gpg.interfaces import IGPGHandler +from lp.services.mail.helpers import IncomingEmailError from lp.services.mail.incoming import ( authenticateEmail, canonicalise_line_endings, @@ -46,16 +47,15 @@ def test_unsigned_message(self): # An unsigned message will not have a signature nor signed content, - # and generates a weakly authenticated principle. + # and generates a weakly authenticated principal. sender = self.factory.makePerson() email_message = self.factory.makeEmailMessage(sender=sender) msg = signed_message_from_string(email_message.as_string()) self.assertIs(None, msg.signedContent) self.assertIs(None, msg.signature) - principle = authenticateEmail(msg) - self.assertEqual(sender, principle.person) - self.assertTrue( - IWeaklyAuthenticatedPrincipal.providedBy(principle)) + principal = authenticateEmail(msg) + self.assertEqual(sender, principal.person) + self.assertTrue(IWeaklyAuthenticatedPrincipal.providedBy(principal)) self.assertIs(None, msg.signature) def _get_clearsigned_for_person(self, sender, body=None): @@ -78,24 +78,22 @@ def test_clearsigned_message_wrong_sender(self): # If the message is signed, but the key doesn't belong to the sender, - # the principle is set to the sender, but weakly authenticated. + # the principal is set to the sender, but weakly authenticated. sender = self.factory.makePerson() msg = self._get_clearsigned_for_person(sender) - principle = authenticateEmail(msg) + principal = authenticateEmail(msg) self.assertIsNot(None, msg.signature) - self.assertEqual(sender, principle.person) - self.assertTrue( - IWeaklyAuthenticatedPrincipal.providedBy(principle)) + self.assertEqual(sender, principal.person) + self.assertTrue(IWeaklyAuthenticatedPrincipal.providedBy(principal)) def test_clearsigned_message(self): # The test keys belong to Sample Person. sender = getUtility(IPersonSet).getByEmail('t...@canonical.com') msg = self._get_clearsigned_for_person(sender) - principle = authenticateEmail(msg) + principal = authenticateEmail(msg) self.assertIsNot(None, msg.signature) - self.assertEqual(sender, principle.person) - self.assertFalse( - IWeaklyAuthenticatedPrincipal.providedBy(principle)) + self.assertEqual(sender, principal.person) + self.assertFalse(IWeaklyAuthenticatedPrincipal.providedBy(principal)) def test_trailing_whitespace(self): # Trailing whitespace should be ignored when verifying a message's @@ -106,11 +104,10 @@ 'And tabs\t\t\n' 'Also mixed. \t ') msg = self._get_clearsigned_for_person(sender, body) - principle = authenticateEmail(msg) + principal = authenticateEmail(msg) self.assertIsNot(None, msg.signature) - self.assertEqual(sender, principle.person) - self.assertFalse( - IWeaklyAuthenticatedPrincipal.providedBy(principle)) + self.assertEqual(sender, principal.person) + self.assertFalse(IWeaklyAuthenticatedPrincipal.providedBy(principal)) def _get_detached_message_for_person(self, sender): # Return a signed message that contains a detached signature. @@ -147,21 +144,49 @@ def test_detached_signature_message_wrong_sender(self): # If the message is signed, but the key doesn't belong to the sender, - # the principle is set to the sender, but weakly authenticated. + # the principal is set to the sender, but weakly authenticated. sender = self.factory.makePerson() msg = self._get_detached_message_for_person(sender) - principle = authenticateEmail(msg) + principal = authenticateEmail(msg) self.assertIsNot(None, msg.signature) - self.assertEqual(sender, principle.person) - self.assertTrue( - IWeaklyAuthenticatedPrincipal.providedBy(principle)) + self.assertEqual(sender, principal.person) + self.assertTrue(IWeaklyAuthenticatedPrincipal.providedBy(principal)) def test_detached_signature_message(self): # Test a detached correct signature. sender = getUtility(IPersonSet).getByEmail('t...@canonical.com') msg = self._get_detached_message_for_person(sender) - principle = authenticateEmail(msg) - self.assertIsNot(None, msg.signature) - self.assertEqual(sender, principle.person) - self.assertFalse( - IWeaklyAuthenticatedPrincipal.providedBy(principle)) + principal = authenticateEmail(msg) + self.assertIsNot(None, msg.signature) + self.assertEqual(sender, principal.person) + self.assertFalse(IWeaklyAuthenticatedPrincipal.providedBy(principal)) + + def test_require_strong_email_authentication_and_signed(self): + sender = getUtility(IPersonSet).getByEmail('t...@canonical.com') + sender.require_strong_email_authentication = True + msg = self._get_clearsigned_for_person(sender) + principal = authenticateEmail(msg) + self.assertIsNot(None, msg.signature) + self.assertEqual(sender, principal.person) + self.assertFalse(IWeaklyAuthenticatedPrincipal.providedBy(principal)) + + def test_require_strong_email_authentication_and_unsigned(self): + sender = getUtility(IPersonSet).getByEmail('t...@canonical.com') + sender.require_strong_email_authentication = True + email_message = self.factory.makeEmailMessage(sender=sender) + msg = signed_message_from_string(email_message.as_string()) + error = self.assertRaises(IncomingEmailError, authenticateEmail, msg) + expected_message = ( + "Launchpad only accepts signed email from your address.\n\n" + "If you want to use the Launchpad email interface, you will need " + "to go here\n" + "to import your OpenPGP key and then use it to sign your " + "messages:\n\n" + " http://launchpad.dev/~%s/+editpgpkeys\n\n" + "If you did not knowingly send email to Launchpad, then spammers " + "may be\n" + "forging messages as if they were sent from your address, perhaps " + "due to\n" + "a compromised address book, and you can safely ignore this " + "message.\n" % sender.name) + self.assertEqual(expected_message, error.message)
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp