Merge lp:~jimpop/mailman/forwarded_for into lp:mailman/2.1

Proposed by Jim Popovitch
Status: Merged
Merge reported by: Mark Sapiro
Merged at revision: not available
Proposed branch: lp:~jimpop/mailman/forwarded_for
Merge into: lp:mailman/2.1
Diff against target: 83 lines (+23/-10)
4 files modified
Mailman/Cgi/listinfo.py (+4/-3)
Mailman/Cgi/options.py (+10/-3)
Mailman/Cgi/subscribe.py (+4/-3)
Mailman/Utils.py (+5/-1)
To merge this branch: bzr merge lp:~jimpop/mailman/forwarded_for
Reviewer Review Type Date Requested Status
Mark Sapiro Approve
Review via email: mp+262717@code.launchpad.net

Commit message

Support for HTTP_X_FORWARDED_FOR and HTTP_FORWARDED_FOR (RFC 7239)

Description of the change

This branch adds support for HTTP_X_FORWARDED_FOR and HTTP_FORWARDED_FOR, it also removes REMOTE_HOST as that is never reliable (i.e. the data for REMOTE_HOST is defined elsewhere and subject to change). This branch also identifies the remote IP in "Possible malformed path attack" errors.

To post a comment you must log in.
Revision history for this message
Mark Sapiro (msapiro) wrote :

Looks good Jim. Thank you.

review: Approve
lp:~jimpop/mailman/forwarded_for updated
1569. By Jim Popovitch

Fixed improper typo/formatting for syslog call

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

I unfortunately found a small typo that needed attention, so I've
updated Mailman/Utils.py.

On Tue, Jun 23, 2015 at 1:34 PM, Mark Sapiro <email address hidden> wrote:
> Review: Approve
>
> Looks good Jim. Thank you.
> --
> https://code.launchpad.net/~jimpop/mailman/forwarded_for/+merge/262717
> You are the owner of lp:~jimpop/mailman/forwarded_for.

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

On 06/23/2015 10:45 AM, jimpop wrote:
> I unfortunately found a small typo that needed attention, so I've
> updated Mailman/Utils.py.

I missed that too, but it's fixed now.

--
Mark Sapiro <email address hidden> The highway is for gamblers,
San Francisco Bay Area, California better use your sense - B. Dylan

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

Wait!!! :-)

I just found another. :-( It was a missing parenthesis.

These two last minute fixes are due to me having produced the diff
from my master copy that I build MM from (it has other odd things in
it that are specific to my installation), and then I copied/pasted the
changes into the new proposed branch. I really should have done this
differently, and I will next time. I should have dev'ed the changes
on a proposed branch, then produced a local patch to import into my
private production branch. Sorry for the hiccups.

-Jim P.

On Tue, Jun 23, 2015 at 2:24 PM, Mark Sapiro <email address hidden> wrote:
> On 06/23/2015 10:45 AM, jimpop wrote:
>> I unfortunately found a small typo that needed attention, so I've
>> updated Mailman/Utils.py.
>
>
> I missed that too, but it's fixed now.
>
> --
> Mark Sapiro <email address hidden> The highway is for gamblers,
> San Francisco Bay Area, California better use your sense - B. Dylan
>
> https://code.launchpad.net/~jimpop/mailman/forwarded_for/+merge/262717
> You are the owner of lp:~jimpop/mailman/forwarded_for.

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

On 06/23/2015 11:30 AM, jimpop wrote:
> Wait!!! :-)
>
> I just found another. :-( It was a missing parenthesis.

Fixed. That one I should have seen because it produces a compile error
on make install. <sigh>

--
Mark Sapiro <email address hidden> The highway is for gamblers,
San Francisco Bay Area, California better use your sense - B. Dylan

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Mailman/Cgi/listinfo.py'
2--- Mailman/Cgi/listinfo.py 2015-04-24 00:42:33 +0000
3+++ Mailman/Cgi/listinfo.py 2015-06-23 17:44:49 +0000
4@@ -187,9 +187,10 @@
5 'subscribe')
6 if mm_cfg.SUBSCRIBE_FORM_SECRET:
7 now = str(int(time.time()))
8- remote = os.environ.get('REMOTE_HOST',
9- os.environ.get('REMOTE_ADDR',
10- 'w.x.y.z'))
11+ remote = os.environ.get('HTTP_FORWARDED_FOR',
12+ os.environ.get('HTTP_X_FORWARDED_FOR',
13+ os.environ.get('REMOTE_ADDR',
14+ 'w.x.y.z'))
15 # Try to accept a range in case of load balancers, etc. (LP: #1447445)
16 if remote.find('.') >= 0:
17 # ipv4 - drop last octet
18
19=== modified file 'Mailman/Cgi/options.py'
20--- Mailman/Cgi/options.py 2015-01-23 00:09:03 +0000
21+++ Mailman/Cgi/options.py 2015-06-23 17:44:49 +0000
22@@ -193,7 +193,10 @@
23 mlist.HoldUnsubscription(user)
24 doc.addError(msga, tag='')
25 else:
26- ip = os.environ.get('REMOTE_ADDR')
27+ ip = os.environ.get('HTTP_FORWARDED_FOR',
28+ os.environ.get('HTTP_X_FORWARDED_FOR',
29+ os.environ.get('REMOTE_ADDR',
30+ 'unidentified origin')))
31 mlist.ConfirmUnsubscription(user, userlang, remote=ip)
32 doc.addError(msgc, tag='')
33 mlist.Save()
34@@ -264,9 +267,13 @@
35 # So as not to allow membership leakage, prompt for the email
36 # address and the password here.
37 if mlist.private_roster <> 0:
38+ remote = os.environ.get('HTTP_FORWARDED_FOR',
39+ os.environ.get('HTTP_X_FORWARDED_FOR',
40+ os.environ.get('REMOTE_ADDR',
41+ 'unidentified origin')))
42 syslog('mischief',
43- 'Login failure with private rosters: %s',
44- user)
45+ 'Login failure with private rosters: %s from %s',
46+ user, remote)
47 user = None
48 # give an HTTP 401 for authentication failure
49 print 'Status: 401 Unauthorized'
50
51=== modified file 'Mailman/Cgi/subscribe.py'
52--- Mailman/Cgi/subscribe.py 2015-04-24 00:42:33 +0000
53+++ Mailman/Cgi/subscribe.py 2015-06-23 17:44:49 +0000
54@@ -118,9 +118,10 @@
55 # Canonicalize the full name
56 fullname = Utils.canonstr(fullname, lang)
57 # Who was doing the subscribing?
58- remote = os.environ.get('REMOTE_HOST',
59- os.environ.get('REMOTE_ADDR',
60- 'unidentified origin'))
61+ remote = os.environ.get('HTTP_FORWARDED_FOR',
62+ os.environ.get('HTTP_X_FORWARDED_FOR',
63+ os.environ.get('REMOTE_ADDR',
64+ 'unidentified origin')))
65 # Are we checking the hidden data?
66 if mm_cfg.SUBSCRIBE_FORM_SECRET:
67 now = int(time.time())
68
69=== modified file 'Mailman/Utils.py'
70--- Mailman/Utils.py 2015-05-01 16:14:08 +0000
71+++ Mailman/Utils.py 2015-06-23 17:44:49 +0000
72@@ -262,7 +262,11 @@
73 if path:
74 if CRNLpat.search(path):
75 path = CRNLpat.split(path)[0]
76- syslog('error', 'Warning: Possible malformed path attack.')
77+ remote = os.environ.get('HTTP_FORWARDED_FOR',
78+ os.environ.get('HTTP_X_FORWARDED_FOR',
79+ os.environ.get('REMOTE_ADDR',
80+ 'unidentified origin')))
81+ syslog('error', 'Warning: Possible malformed path attack domain=%s remote=%s', get_domain(), remote)
82 return [p for p in path.split('/') if p]
83 return None
84

Subscribers

People subscribed via source and target branches