Merge lp:~jimpop/mailman/dmarc-reject into lp:mailman/2.1

Proposed by Barry Warsaw
Status: Merged
Merged at revision: 1456
Proposed branch: lp:~jimpop/mailman/dmarc-reject
Merge into: lp:mailman/2.1
Diff against target: 196 lines (+131/-0)
5 files modified
Mailman/Gui/Privacy.py (+24/-0)
Mailman/Handlers/Moderate.py (+19/-0)
Mailman/MailList.py (+2/-0)
Mailman/Utils.py (+83/-0)
Mailman/versions.py (+3/-0)
To merge this branch: bzr merge lp:~jimpop/mailman/dmarc-reject
Reviewer Review Type Date Requested Status
Mark Sapiro Approve
Review via email: mp+215591@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Mark Sapiro (msapiro) wrote :

The merge is clean. I will be reviewing/testing further tomorrow.

Thanks greatly Jim, and I'm sorry it took the recent "wake-up" call to get action on this. It would have been better if I'd done this sooner.

review: Approve
Revision history for this message
Jim Popovitch (jimpop) wrote :

> The merge is clean. I will be reviewing/testing further tomorrow.
>
> Thanks greatly Jim, and I'm sorry it took the recent "wake-up" call to get
> action on this. It would have been better if I'd done this sooner.

Awesome, I am glad to see this headed in this direction. Mark, no need for any regret, I'm just glad to contribute something that is hopefully helpful.

Revision history for this message
Mark Sapiro (msapiro) wrote :

I have merged this branch as a starting point. I am refactoring a few things including removing the Hold option and adding Munge From and Wrap Message options so the choices are "Accept, Wrap Message, Munge From, Reject and Discard" and the list owner is not allowed to set an option with a lower value than the site default.

I am also keeping the general Wrap Message and Munge From list options which will apply to messages From: domains without DMARC reject/quarantine and all messages if this setting is Accept.

I want to thank you again for this work Jim and also point out one problem I found.

The code in Moderate includes

    if sender:
        if Utils.IsDmarcProhibited(sender):

but sender may not be the From: address. I made this

    dn, addr = email.Utils.parseaddr(msg.get('from'))
    if addr:
        if Utils.IsDmarcProhibited(addr)

Revision history for this message
Jim Popovitch (jimpop) wrote :

Thanks for feedback Mark. Should I push that change to dmarc-reject, or let that branch die now that the code is being merged?

Revision history for this message
Mark Sapiro (msapiro) wrote :

Wait until I publish the merged 2.1 branch. Then if you think this branch is still useful, fix it. Otherwise let it die.

Revision history for this message
Jim Popovitch (jimpop) wrote :

IMO, the dmarc-reject branch is useless after this merge, so I will let it die.

Thanks!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Mailman/Gui/Privacy.py'
--- Mailman/Gui/Privacy.py 2009-01-11 16:06:13 +0000
+++ Mailman/Gui/Privacy.py 2014-04-14 01:04:55 +0000
@@ -235,6 +235,30 @@
235 >rejection notice</a> to235 >rejection notice</a> to
236 be sent to moderated members who post to this list.""")),236 be sent to moderated members who post to this list.""")),
237237
238 ('dmarc_moderation_action', mm_cfg.Radio,
239 (_('Accept'), _('Hold'), _('Reject'), _('Discard')), 0,
240 _("""Action to take when anyone posts to the
241 list from a domain with a DMARC Reject/Quarantine Policy."""),
242 _("""<ul><li><b>Hold</b> -- this holds the message for approval
243 by the list moderators.
244
245 <p><li><b>Reject</b> -- this automatically rejects the message by
246 sending a bounce notice to the post's author. The text of the
247 bounce notice can be <a
248 href="?VARHELP=privacy/sender/dmarc_moderation_notice"
249 >configured by you</a>.
250
251 <p><li><b>Discard</b> -- this simply discards the message, with
252 no notice sent to the post's author.
253 </ul>""")),
254
255 ('dmarc_moderation_notice', mm_cfg.Text, (10, WIDTH), 1,
256 _("""Text to include in any
257 <a href="?VARHELP/privacy/sender/dmarc_moderation_action"
258 >rejection notice</a> to
259 be sent to anyone who posts to this list from a domain
260 with DMARC Reject/Quarantine Policy.""")),
261
238 _('Non-member filters'),262 _('Non-member filters'),
239263
240 ('accept_these_nonmembers', mm_cfg.EmailListEx, (10, WIDTH), 1,264 ('accept_these_nonmembers', mm_cfg.EmailListEx, (10, WIDTH), 1,
241265
=== modified file 'Mailman/Handlers/Moderate.py'
--- Mailman/Handlers/Moderate.py 2013-11-19 04:52:17 +0000
+++ Mailman/Handlers/Moderate.py 2014-04-14 01:04:55 +0000
@@ -56,6 +56,25 @@
56 else:56 else:
57 sender = None57 sender = None
58 if sender:58 if sender:
59 if Utils.IsDmarcProhibited(sender):
60 # Note that for dmarc_moderation_action, 0 = Accept,
61 # 1 = Hold, 2 = Reject, 3 = Discard
62 if mlist.dmarc_moderation_action == 1:
63 msgdata['sender'] = sender
64 Hold.hold_for_approval(mlist, msg, msgdata,
65 ModeratedMemberPost)
66 elif mlist.dmarc_moderation_action == 2:
67 # Reject
68 text = mlist.dmarc_moderation_notice
69 if text:
70 text = Utils.wrap(text)
71 else:
72 # Use the default RejectMessage notice string
73 text = None
74 raise Errors.RejectMessage, text
75 elif mlist.dmarc_moderation_action == 3:
76 raise Errors.DiscardMessage
77
59 # If the member's moderation flag is on, then perform the moderation78 # If the member's moderation flag is on, then perform the moderation
60 # action.79 # action.
61 if mlist.getMemberOption(sender, mm_cfg.Moderate):80 if mlist.getMemberOption(sender, mm_cfg.Moderate):
6281
=== modified file 'Mailman/MailList.py'
--- Mailman/MailList.py 2013-09-28 23:08:15 +0000
+++ Mailman/MailList.py 2014-04-14 01:04:55 +0000
@@ -389,6 +389,8 @@
389 # 2==Discard389 # 2==Discard
390 self.member_moderation_action = 0390 self.member_moderation_action = 0
391 self.member_moderation_notice = ''391 self.member_moderation_notice = ''
392 self.dmarc_moderation_action = mm_cfg.DEFAULT_DMARC_MODERATION_ACTION
393 self.dmarc_moderation_notice = ''
392 self.accept_these_nonmembers = []394 self.accept_these_nonmembers = []
393 self.hold_these_nonmembers = []395 self.hold_these_nonmembers = []
394 self.reject_these_nonmembers = []396 self.reject_these_nonmembers = []
395397
=== modified file 'Mailman/Utils.py'
--- Mailman/Utils.py 2013-12-07 01:19:28 +0000
+++ Mailman/Utils.py 2014-04-14 01:04:55 +0000
@@ -35,6 +35,7 @@
35import base6435import base64
36import random36import random
37import urlparse37import urlparse
38import collections
38import htmlentitydefs39import htmlentitydefs
39import email.Header40import email.Header
40import email.Iterators41import email.Iterators
@@ -71,6 +72,13 @@
71 True = 172 True = 1
72 False = 073 False = 0
7374
75try:
76 import dns.resolver
77 from dns.exception import DNSException
78 dns_resolver = True
79except ImportError:
80 dns_resolver = False
81
74EMPTYSTRING = ''82EMPTYSTRING = ''
75UEMPTYSTRING = u''83UEMPTYSTRING = u''
76NL = '\n'84NL = '\n'
@@ -1058,3 +1066,78 @@
1058 else:1066 else:
1059 return False1067 return False
10601068
1069
1070# This takes an email address, and returns True if DMARC policy is p=reject
1071def IsDmarcProhibited(email):
1072 if not dns_resolver:
1073 return False
1074
1075 email = email.lower()
1076 at_sign = email.find('@')
1077 if at_sign < 1:
1078 return False
1079 dmarc_domain = '_dmarc.' + email[at_sign+1:]
1080
1081 try:
1082 resolver = dns.resolver.Resolver()
1083 resolver.timeout = 3
1084 resolver.lifetime = 5
1085 txt_recs = resolver.query(dmarc_domain, dns.rdatatype.TXT)
1086 except (dns.resolver.NXDOMAIN, dns.resolver.NoAnswer):
1087 return False
1088 except DNSException, e:
1089 syslog('error', 'DNSException: Unable to query DMARC policy for %s (%s). %s',
1090 email, dmarc_domain, e.__class__)
1091 return False
1092 else:
1093# people are already being dumb, don't trust them to provide honest DNS
1094# where the answer section only contains what was asked for, nor to include
1095# CNAMEs before the values they point to.
1096 full_record = ""
1097 results_by_name = collections.defaultdict(list)
1098 cnames = {}
1099 want_names = set([dmarc_domain + '.'])
1100 for txt_rec in txt_recs.response.answer:
1101 if txt_rec.rdtype == dns.rdatatype.CNAME:
1102 cnames[txt_rec.name.to_text()] = txt_rec.items[0].target.to_text()
1103 if txt_rec.rdtype != dns.rdatatype.TXT:
1104 continue
1105 results_by_name[txt_rec.name.to_text()].append("".join(txt_rec.items[0].strings))
1106 expands = list(want_names)
1107 seen = set(expands)
1108 while expands:
1109 item = expands.pop(0)
1110 if item in cnames:
1111 if cnames[item] in seen:
1112 continue # cname loop
1113 expands.append(cnames[item])
1114 seen.add(cnames[item])
1115 want_names.add(cnames[item])
1116 want_names.discard(item)
1117
1118 if len(want_names) != 1:
1119 syslog('error', 'multiple DMARC entries in results for %s, processing each to be strict',
1120 dmarc_domain)
1121 for name in want_names:
1122 if name not in results_by_name:
1123 continue
1124 dmarcs = filter(lambda n: n.startswith('v=DMARC1;'), results_by_name[name])
1125 if len(dmarcs) == 0:
1126 return False
1127 if len(dmarcs) > 1:
1128 syslog('error', 'RRset of TXT records for %s has %d v=DMARC1 entries; testing them all',
1129 dmarc_domain, len(dmarc))
1130 for entry in dmarcs:
1131 if re.search(r'\bp=reject\b', entry, re.IGNORECASE):
1132 syslog('info', 'DMARC lookup for %s (%s) found p=reject in %s = %s',
1133 email, dmarc_domain, name, entry)
1134 return True
1135
1136 if re.search(r'\bp=quarantine\b', entry, re.IGNORECASE):
1137 syslog('info', 'DMARC lookup for %s (%s) found p=quarantine in %s = %s',
1138 email, dmarc_domain, name, entry)
1139 return True
1140
1141 return False
1142
1143
10611144
=== modified file 'Mailman/versions.py'
--- Mailman/versions.py 2013-09-28 23:08:15 +0000
+++ Mailman/versions.py 2014-04-14 01:04:55 +0000
@@ -388,6 +388,9 @@
388 # the current GUI description model. So, 0==Hold, 1==Reject, 2==Discard388 # the current GUI description model. So, 0==Hold, 1==Reject, 2==Discard
389 add_only_if_missing('member_moderation_action', 0)389 add_only_if_missing('member_moderation_action', 0)
390 add_only_if_missing('member_moderation_notice', '')390 add_only_if_missing('member_moderation_notice', '')
391 add_only_if_missing('dmarc_moderation_action',
392 mm_cfg.DEFAULT_DMARC_MODERATION_ACTION)
393 add_only_if_missing('dmarc_moderation_notice', '')
391 add_only_if_missing('new_member_options',394 add_only_if_missing('new_member_options',
392 mm_cfg.DEFAULT_NEW_MEMBER_OPTIONS)395 mm_cfg.DEFAULT_NEW_MEMBER_OPTIONS)
393 # Emergency moderation flag396 # Emergency moderation flag