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
1=== modified file 'Mailman/Gui/Privacy.py'
2--- Mailman/Gui/Privacy.py 2009-01-11 16:06:13 +0000
3+++ Mailman/Gui/Privacy.py 2014-04-14 01:04:55 +0000
4@@ -235,6 +235,30 @@
5 >rejection notice</a> to
6 be sent to moderated members who post to this list.""")),
7
8+ ('dmarc_moderation_action', mm_cfg.Radio,
9+ (_('Accept'), _('Hold'), _('Reject'), _('Discard')), 0,
10+ _("""Action to take when anyone posts to the
11+ list from a domain with a DMARC Reject/Quarantine Policy."""),
12+ _("""<ul><li><b>Hold</b> -- this holds the message for approval
13+ by the list moderators.
14+
15+ <p><li><b>Reject</b> -- this automatically rejects the message by
16+ sending a bounce notice to the post's author. The text of the
17+ bounce notice can be <a
18+ href="?VARHELP=privacy/sender/dmarc_moderation_notice"
19+ >configured by you</a>.
20+
21+ <p><li><b>Discard</b> -- this simply discards the message, with
22+ no notice sent to the post's author.
23+ </ul>""")),
24+
25+ ('dmarc_moderation_notice', mm_cfg.Text, (10, WIDTH), 1,
26+ _("""Text to include in any
27+ <a href="?VARHELP/privacy/sender/dmarc_moderation_action"
28+ >rejection notice</a> to
29+ be sent to anyone who posts to this list from a domain
30+ with DMARC Reject/Quarantine Policy.""")),
31+
32 _('Non-member filters'),
33
34 ('accept_these_nonmembers', mm_cfg.EmailListEx, (10, WIDTH), 1,
35
36=== modified file 'Mailman/Handlers/Moderate.py'
37--- Mailman/Handlers/Moderate.py 2013-11-19 04:52:17 +0000
38+++ Mailman/Handlers/Moderate.py 2014-04-14 01:04:55 +0000
39@@ -56,6 +56,25 @@
40 else:
41 sender = None
42 if sender:
43+ if Utils.IsDmarcProhibited(sender):
44+ # Note that for dmarc_moderation_action, 0 = Accept,
45+ # 1 = Hold, 2 = Reject, 3 = Discard
46+ if mlist.dmarc_moderation_action == 1:
47+ msgdata['sender'] = sender
48+ Hold.hold_for_approval(mlist, msg, msgdata,
49+ ModeratedMemberPost)
50+ elif mlist.dmarc_moderation_action == 2:
51+ # Reject
52+ text = mlist.dmarc_moderation_notice
53+ if text:
54+ text = Utils.wrap(text)
55+ else:
56+ # Use the default RejectMessage notice string
57+ text = None
58+ raise Errors.RejectMessage, text
59+ elif mlist.dmarc_moderation_action == 3:
60+ raise Errors.DiscardMessage
61+
62 # If the member's moderation flag is on, then perform the moderation
63 # action.
64 if mlist.getMemberOption(sender, mm_cfg.Moderate):
65
66=== modified file 'Mailman/MailList.py'
67--- Mailman/MailList.py 2013-09-28 23:08:15 +0000
68+++ Mailman/MailList.py 2014-04-14 01:04:55 +0000
69@@ -389,6 +389,8 @@
70 # 2==Discard
71 self.member_moderation_action = 0
72 self.member_moderation_notice = ''
73+ self.dmarc_moderation_action = mm_cfg.DEFAULT_DMARC_MODERATION_ACTION
74+ self.dmarc_moderation_notice = ''
75 self.accept_these_nonmembers = []
76 self.hold_these_nonmembers = []
77 self.reject_these_nonmembers = []
78
79=== modified file 'Mailman/Utils.py'
80--- Mailman/Utils.py 2013-12-07 01:19:28 +0000
81+++ Mailman/Utils.py 2014-04-14 01:04:55 +0000
82@@ -35,6 +35,7 @@
83 import base64
84 import random
85 import urlparse
86+import collections
87 import htmlentitydefs
88 import email.Header
89 import email.Iterators
90@@ -71,6 +72,13 @@
91 True = 1
92 False = 0
93
94+try:
95+ import dns.resolver
96+ from dns.exception import DNSException
97+ dns_resolver = True
98+except ImportError:
99+ dns_resolver = False
100+
101 EMPTYSTRING = ''
102 UEMPTYSTRING = u''
103 NL = '\n'
104@@ -1058,3 +1066,78 @@
105 else:
106 return False
107
108+
109+# This takes an email address, and returns True if DMARC policy is p=reject
110+def IsDmarcProhibited(email):
111+ if not dns_resolver:
112+ return False
113+
114+ email = email.lower()
115+ at_sign = email.find('@')
116+ if at_sign < 1:
117+ return False
118+ dmarc_domain = '_dmarc.' + email[at_sign+1:]
119+
120+ try:
121+ resolver = dns.resolver.Resolver()
122+ resolver.timeout = 3
123+ resolver.lifetime = 5
124+ txt_recs = resolver.query(dmarc_domain, dns.rdatatype.TXT)
125+ except (dns.resolver.NXDOMAIN, dns.resolver.NoAnswer):
126+ return False
127+ except DNSException, e:
128+ syslog('error', 'DNSException: Unable to query DMARC policy for %s (%s). %s',
129+ email, dmarc_domain, e.__class__)
130+ return False
131+ else:
132+# people are already being dumb, don't trust them to provide honest DNS
133+# where the answer section only contains what was asked for, nor to include
134+# CNAMEs before the values they point to.
135+ full_record = ""
136+ results_by_name = collections.defaultdict(list)
137+ cnames = {}
138+ want_names = set([dmarc_domain + '.'])
139+ for txt_rec in txt_recs.response.answer:
140+ if txt_rec.rdtype == dns.rdatatype.CNAME:
141+ cnames[txt_rec.name.to_text()] = txt_rec.items[0].target.to_text()
142+ if txt_rec.rdtype != dns.rdatatype.TXT:
143+ continue
144+ results_by_name[txt_rec.name.to_text()].append("".join(txt_rec.items[0].strings))
145+ expands = list(want_names)
146+ seen = set(expands)
147+ while expands:
148+ item = expands.pop(0)
149+ if item in cnames:
150+ if cnames[item] in seen:
151+ continue # cname loop
152+ expands.append(cnames[item])
153+ seen.add(cnames[item])
154+ want_names.add(cnames[item])
155+ want_names.discard(item)
156+
157+ if len(want_names) != 1:
158+ syslog('error', 'multiple DMARC entries in results for %s, processing each to be strict',
159+ dmarc_domain)
160+ for name in want_names:
161+ if name not in results_by_name:
162+ continue
163+ dmarcs = filter(lambda n: n.startswith('v=DMARC1;'), results_by_name[name])
164+ if len(dmarcs) == 0:
165+ return False
166+ if len(dmarcs) > 1:
167+ syslog('error', 'RRset of TXT records for %s has %d v=DMARC1 entries; testing them all',
168+ dmarc_domain, len(dmarc))
169+ for entry in dmarcs:
170+ if re.search(r'\bp=reject\b', entry, re.IGNORECASE):
171+ syslog('info', 'DMARC lookup for %s (%s) found p=reject in %s = %s',
172+ email, dmarc_domain, name, entry)
173+ return True
174+
175+ if re.search(r'\bp=quarantine\b', entry, re.IGNORECASE):
176+ syslog('info', 'DMARC lookup for %s (%s) found p=quarantine in %s = %s',
177+ email, dmarc_domain, name, entry)
178+ return True
179+
180+ return False
181+
182+
183
184=== modified file 'Mailman/versions.py'
185--- Mailman/versions.py 2013-09-28 23:08:15 +0000
186+++ Mailman/versions.py 2014-04-14 01:04:55 +0000
187@@ -388,6 +388,9 @@
188 # the current GUI description model. So, 0==Hold, 1==Reject, 2==Discard
189 add_only_if_missing('member_moderation_action', 0)
190 add_only_if_missing('member_moderation_notice', '')
191+ add_only_if_missing('dmarc_moderation_action',
192+ mm_cfg.DEFAULT_DMARC_MODERATION_ACTION)
193+ add_only_if_missing('dmarc_moderation_notice', '')
194 add_only_if_missing('new_member_options',
195 mm_cfg.DEFAULT_NEW_MEMBER_OPTIONS)
196 # Emergency moderation flag