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

[Jim Popovitch]

Support for HTTP_X_FORWARDED_FOR and HTTP_FORWARDED_FOR (RFC 7239)

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.

[Mark Sapiro (msapiro)]

Looks good Jim. Thank you.

review: Approve

[lp:~jimpop/mailman/forwarded_for]

1569. By Jim Popovitch on 2015-06-23

Fixed improper typo/formatting for syslog call

[Jim Popovitch (jimpop)]

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.

[Mark Sapiro (msapiro)]

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

[Jim Popovitch (jimpop)]

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.

[Mark Sapiro (msapiro)]

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

=== modified file 'Mailman/Cgi/listinfo.py'
--- Mailman/Cgi/listinfo.py	2015-04-24 00:42:33 +0000
+++ Mailman/Cgi/listinfo.py	2015-06-23 17:44:49 +0000
@@ -187,9 +187,10 @@
         'subscribe')
     if mm_cfg.SUBSCRIBE_FORM_SECRET:
         now = str(int(time.time()))
-        remote = os.environ.get('REMOTE_HOST',
-                                os.environ.get('REMOTE_ADDR',
-                                               'w.x.y.z'))
+        remote = os.environ.get('HTTP_FORWARDED_FOR',
+                 os.environ.get('HTTP_X_FORWARDED_FOR',
+                 os.environ.get('REMOTE_ADDR',
+                                'w.x.y.z'))
         # Try to accept a range in case of load balancers, etc.  (LP: #1447445)
         if remote.find('.') >= 0:
             # ipv4 - drop last octet

=== modified file 'Mailman/Cgi/options.py'
--- Mailman/Cgi/options.py	2015-01-23 00:09:03 +0000
+++ Mailman/Cgi/options.py	2015-06-23 17:44:49 +0000
@@ -193,7 +193,10 @@
                     mlist.HoldUnsubscription(user)
                     doc.addError(msga, tag='')
                 else:
-                    ip = os.environ.get('REMOTE_ADDR')
+                    ip = os.environ.get('HTTP_FORWARDED_FOR',
+                         os.environ.get('HTTP_X_FORWARDED_FOR',
+                         os.environ.get('REMOTE_ADDR',
+                                        'unidentified origin')))
                     mlist.ConfirmUnsubscription(user, userlang, remote=ip)
                     doc.addError(msgc, tag='')
                 mlist.Save()
@@ -264,9 +267,13 @@
             # So as not to allow membership leakage, prompt for the email
             # address and the password here.
             if mlist.private_roster <> 0:
+                remote = os.environ.get('HTTP_FORWARDED_FOR',
+                         os.environ.get('HTTP_X_FORWARDED_FOR',
+                         os.environ.get('REMOTE_ADDR',
+                                        'unidentified origin')))
                 syslog('mischief',
-                       'Login failure with private rosters: %s',
-                       user)
+                       'Login failure with private rosters: %s from %s',
+                       user, remote)
                 user = None
             # give an HTTP 401 for authentication failure
             print 'Status: 401 Unauthorized'

=== modified file 'Mailman/Cgi/subscribe.py'
--- Mailman/Cgi/subscribe.py	2015-04-24 00:42:33 +0000
+++ Mailman/Cgi/subscribe.py	2015-06-23 17:44:49 +0000
@@ -118,9 +118,10 @@
     # Canonicalize the full name
     fullname = Utils.canonstr(fullname, lang)
     # Who was doing the subscribing?
-    remote = os.environ.get('REMOTE_HOST',
-                            os.environ.get('REMOTE_ADDR',
-                                           'unidentified origin'))
+    remote = os.environ.get('HTTP_FORWARDED_FOR',
+             os.environ.get('HTTP_X_FORWARDED_FOR',
+             os.environ.get('REMOTE_ADDR',
+                            'unidentified origin')))
     # Are we checking the hidden data?
     if mm_cfg.SUBSCRIBE_FORM_SECRET:
         now = int(time.time())

=== modified file 'Mailman/Utils.py'
--- Mailman/Utils.py	2015-05-01 16:14:08 +0000
+++ Mailman/Utils.py	2015-06-23 17:44:49 +0000
@@ -262,7 +262,11 @@
     if path:
         if CRNLpat.search(path):
             path = CRNLpat.split(path)[0]
-            syslog('error', 'Warning: Possible malformed path attack.')
+            remote = os.environ.get('HTTP_FORWARDED_FOR',
+                     os.environ.get('HTTP_X_FORWARDED_FOR',
+                     os.environ.get('REMOTE_ADDR',
+                                    'unidentified origin')))
+            syslog('error', 'Warning: Possible malformed path attack domain=%s remote=%s', get_domain(), remote)
         return [p for p in path.split('/') if p]
     return None