Merge lp:~jimpop/mailman/security-logging into lp:mailman/2.1

[Jim Popovitch]

Logging improvement for security related events (failed logins, etc.)

[Jim Popovitch (jimpop)]

Here's a patch that logs security related events (failed logins, etc).

[Mark Sapiro (msapiro)]

There are a couple of issues.

First, there is currently a log entry in mischief for an options page login failure, but only with private rosters. If we are going to be logging auth failures in a new security log, the options failure should be logged there too and unconditionally.

Second, the Changes to the owner notifications to add

text = "%s\nReason: %s" % (text, whence)

to the owner notifications for subscribe and unsubscribe are problematic. whence is not a 'Reason' and there is no i18n for 'Reason'

It would be better to just add whence as an additional replacement in the templates.

Finally, I don't see the need for this in the owner notices at all. This info is already in the subscribe log which I think is a better place for it.

review: Needs Fixing

[lp:~jimpop/mailman/security-logging]

1768. By Jim Popovitch on 2018-06-10

Changes based on feedback from Mark.

[Jim Popovitch (jimpop)]

Thanks Mark,

The auth failure is now always logged to security in options.py, and the mischief log entry remains as was (logging only if private roster).

The only reason I put the whence in the owner notification emails is so that it is crystal clear to all the admins and moderators why a subscriber was added or removed, i.e. "bin/remove_member", "email confirmation", "web confirmation". It keeps a large mod team informed, as well as saving mods from having to ask "why was this person removed?". etc. I like the idea of integrating it in the templates, so I've removed it from this merge and I'll do a new merge request for that piece (and you can decide on that at that time).

review: Needs Resubmitting

[Mark Sapiro (msapiro)]

I added a NEWS item and fixed a few small issues.

Thanks for the contribution.

review: Approve

[Mark Sapiro (msapiro)]

On 6/10/18 4:02 PM, Jim Popovitch wrote:
>
> The only reason I put the whence in the owner notification emails is so that it is crystal clear to all the admins and moderators why a subscriber was added or removed, i.e. "bin/remove_member", "email confirmation", "web confirmation". It keeps a large mod team informed, as well as saving mods from having to ask "why was this person removed?". etc. I like the idea of integrating it in the templates, so I've removed it from this merge and I'll do a new merge request for that piece (and you can decide on that at that time).

I was confused. I forgot that whence was those things in these cases. I
was focused on its being an IP address. No that I realize what it is, I
think it's fine to add it to the notice, but making it a replacement for
the templates is the way to go.

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

[Jim Popovitch (jimpop)]

I've gone ahead and created the new branch for the whence templates. The chief problem for me at this point is making sure that the placement of $(whence) is grammatically correct in each template.

https://code.launchpad.net/~jimpop/mailman/owner-notification-whence

[Mark Sapiro (msapiro)]

On 6/11/18 5:23 PM, Jim Popovitch wrote:
> I've gone ahead and created the new branch for the whence templates. The chief problem for me at this point is making sure that the placement of $(whence) is grammatically correct in each template.
>
> https://code.launchpad.net/~jimpop/mailman/owner-notification-whence

I understand the issue, and I wouldn't worry about it.

I'd say only do the 'en' templates and I'll post to mailman-i18n that
templates need to be updated to take advantage of this. There's no
problem if the template doesn't have %(whence)s in it.

My concerns are:

the 'en' adminsubscribeack.txt template adds a %(method)s replacement
and that doesn't seem to be provided.

There is no 'en' (or other) adminunsubscribeack.txt template.

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

[Jim Popovitch (jimpop)]

Thanks for pointing that out. I've re-pushed a complete new branch
with just the changes for the 2 "en" templates and MailList.py.

I left the branch as "Development" status (as opposed to Mature) for
now, and sent you a merge request.

Thanks!!

-Jim P.

=== modified file 'Mailman/Cgi/admin.py'
--- Mailman/Cgi/admin.py	2017-06-21 16:59:41 +0000
+++ Mailman/Cgi/admin.py	2018-06-10 23:02:31 +0000
@@ -119,6 +119,11 @@
         if cgidata.has_key('adminpw'):
             # This is a re-authorization attempt
             msg = Bold(FontSize('+1', _('Authorization failed.'))).Format()
+            remote = os.environ.get('HTTP_FORWARDED_FOR',
+                     os.environ.get('HTTP_X_FORWARDED_FOR',
+                     os.environ.get('REMOTE_ADDR',
+                                    'unidentified origin')))
+            syslog('security', 'Authorization failed (admin): list=%s: remote=%s', listname, remote)
         else:
             msg = ''
         Auth.loginpage(mlist, 'admin', msg=msg)
 
=== modified file 'Mailman/Cgi/admindb.py'
--- Mailman/Cgi/admindb.py	2017-06-24 21:34:48 +0000
+++ Mailman/Cgi/admindb.py	2018-06-10 23:02:31 +0000
@@ -159,6 +159,11 @@
         if cgidata.has_key('adminpw'):
             # This is a re-authorization attempt
             msg = Bold(FontSize('+1', _('Authorization failed.'))).Format()
+            remote = os.environ.get('HTTP_FORWARDED_FOR',
+                     os.environ.get('HTTP_X_FORWARDED_FOR',
+                     os.environ.get('REMOTE_ADDR',
+                                    'unidentified origin')))
+            syslog('security', 'Authorization failed (admindb): list=%s: domain=%s', listname, remote)
         else:
             msg = ''
         Auth.loginpage(mlist, 'admindb', msg=msg)
 
=== modified file 'Mailman/Cgi/edithtml.py'
--- Mailman/Cgi/edithtml.py	2017-06-06 05:47:05 +0000
+++ Mailman/Cgi/edithtml.py	2018-06-10 23:02:31 +0000
@@ -126,6 +126,11 @@
         if cgidata.has_key('admlogin'):
             # This is a re-authorization attempt
             msg = Bold(FontSize('+1', _('Authorization failed.'))).Format()
+            remote = os.environ.get('HTTP_FORWARDED_FOR',
+                     os.environ.get('HTTP_X_FORWARDED_FOR',
+                     os.environ.get('REMOTE_ADDR',
+                                    'unidentified origin')))
+            syslog('security', 'Authorization failed (edithtml): list=%s: remote=%s', listname, remote)
         else:
             msg = ''
         Auth.loginpage(mlist, 'admin', msg=msg)
 
=== modified file 'Mailman/Cgi/options.py'
--- Mailman/Cgi/options.py	2018-02-04 16:41:19 +0000
+++ Mailman/Cgi/options.py	2018-06-10 23:02:31 +0000
@@ -288,13 +288,15 @@
         # message.
         if cgidata.has_key('password'):
             doc.addError(_('Authentication failed.'))
+            remote = os.environ.get('HTTP_FORWARDED_FOR',
+                     os.environ.get('HTTP_X_FORWARDED_FOR',
+                     os.environ.get('REMOTE_ADDR',
+                                    'unidentified origin')))
+            syslog('security', 'Authorization failed (private): user=%s 
+                                list=%s remote=%s', user, listname, remote)
             # 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 from %s',
                        user, remote)
 
=== modified file 'Mailman/Cgi/private.py'
--- Mailman/Cgi/private.py	2017-06-06 05:47:05 +0000
+++ Mailman/Cgi/private.py	2018-06-10 23:02:31 +0000
@@ -142,6 +142,12 @@
         if cgidata.has_key('submit'):
             # This is a re-authorization attempt
             message = Bold(FontSize('+1', _('Authorization failed.'))).Format()
+            remote = os.environ.get('HTTP_FORWARDED_FOR',
+                     os.environ.get('HTTP_X_FORWARDED_FOR',
+                     os.environ.get('REMOTE_ADDR',
+                                    'unidentified origin')))
+            syslog('security', 'Authorization failed (private): user=%s list=%s 
+                                remote=%s', username, listname, remote)
             # give an HTTP 401 for authentication failure
             print 'Status: 401 Unauthorized'
         # Are we processing a password reminder from the login screen?
 
=== modified file 'Mailman/Cgi/roster.py'
--- Mailman/Cgi/roster.py	2017-06-06 03:48:34 +0000
+++ Mailman/Cgi/roster.py	2018-06-10 23:02:31 +0000
@@ -118,6 +118,11 @@
         error_page_doc(doc, _('%(realname)s roster authentication failed.'))
         doc.AddItem(mlist.GetMailmanFooter())
         print doc.Format()
+        remote = os.environ.get('HTTP_FORWARDED_FOR',
+                 os.environ.get('HTTP_X_FORWARDED_FOR',
+                 os.environ.get('REMOTE_ADDR',
+                                'unidentified origin')))
+        syslog('security', 'Authorization failed (roster): list=%s: remote=%s', listname, remote)
         return
 
     # The document and its language
 
=== modified file 'Mailman/Utils.py'
--- Mailman/Utils.py	2018-06-05 19:14:08 +0000
+++ Mailman/Utils.py	2018-06-10 23:02:31 +0000
@@ -111,7 +111,11 @@
     # But first ensure the list name doesn't contain a path traversal
     # attack.
     if len(re.sub(mm_cfg.ACCEPTABLE_LISTNAME_CHARACTERS, '', listname)) > 0:
-        syslog('mischief', 'Hostile listname: %s', listname)
+        remote = os.environ.get('HTTP_FORWARDED_FOR',
+                 os.environ.get('HTTP_X_FORWARDED_FOR',
+                 os.environ.get('REMOTE_ADDR',
+                                'unidentified origin')))
+        syslog('mischief', 'Hostile listname: listname=%s remote=%s', listname, remote)
         return False
     basepath = Site.get_listpath(listname)
     for ext in ('.pck', '.pck.last', '.db', '.db.last'):