User passwords in clear in "admin logfile"

[hoffmann]

I am sorry, but this looks like a showstopper before going to the long "pre-v2 break". I even thought it had been discussed (and solved) some time ago, but it still seems to be with us.

When a user authentication fails, the admin e-mail address defined in indico.conf gets a message like

2014-05-05 08:11:21,792 indico.auth.ldap - ERROR LDAPAuthentication.py:337 -- Username: abidrahmank - invalid credentials
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/indico-1.1.2-py2.7.egg/MaKaC/authentication/LDAPAuthentication.py", line 329, in check
    ldapc.openAsUser(userName, password)
  File "/usr/local/lib/python2.7/dist-packages/indico-1.1.2-py2.7.egg/MaKaC/authentication/LDAPAuthentication.py", line 248, in openAsUser
    self.l.simple_bind_s(dn, password)
  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 208, in simple_bind_s
    resp_type, resp_data, resp_msgid, resp_ctrls = self.result3(msgid,all=1,timeout=self.timeout)
  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 469, in result3
    resp_ctrl_classes=resp_ctrl_classes
  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 476, in result4
    ldap_result = self._ldap_call(self._l.result4,msgid,all,timeout,add_ctrls,add_intermediates,add_extop)
  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 99, in _ldap_call
    result = func(*args,**kwargs)
INVALID_CREDENTIALS: {'desc': 'Invalid credentials'}

Additional information:
URL: https://www.cta-observatory.org/indico/signIn.py
Params: {'signIn': 'Login', 'login': '***', 'password': '***', 'returnURL': 'https://www.cta-observatory.org/indico/conferenceDisplay.py?confId=661'}
IP: 193.48.172.166
User Agent: Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.2; WOW64; Trident/6.0; Touch; MAARJS)
Referer: https://www.cta-observatory.org/indico/signIn.py?returnURL=https%3A%2F%2Fwww.cta-observatory.org%2Findico%2FconferenceDisplay.py%3FconfId%3D661

The attribute values for login and password have obviously been replaced in this ticket, but they go in clear into the e-mail message.

It seems to be sufficient to fix this in the _getRequestInfo() method of IndicoMailFormatter: ​https://indico-software.org/browser/indico/indico/MaKaC/common/logger.py?rev=master#L57

(Sorry, I still do not manage to reference code correctly in this Trac.)

[hoffmann]

Here is my patch (wrt v1.1.2) - sorry, cannot deliver a cleaner contribution through git/hub right now.

Index: MaKaC/common/logger.py
===================================================================
--- MaKaC/common/logger.py	(revision 5932)
+++ MaKaC/common/logger.py	(working copy)
@@ -44,7 +44,11 @@
             return ''
         info = ['Additional information:']
         info.append('URL: %s' % rh.getRequestURL())
-        info.append('Params: %s' % rh._getTruncatedParams())
+	# obfuscate password parameter, if used in failing request
+	params = rh._getTruncatedParams()
+	if "password" in params:
+		params['password'] = '***'
+        info.append('Params: %s' % params)
         info.append('IP: %s' % rh._req.remote_ip)
         info.append('User Agent: %s' % rh._req.headers_in.get('User-Agent', 'n/a'))
         info.append('Referer: %s' % rh._req.headers_in.get('Referer', 'n/a'))

[pferreir]

This has been fixed in 1.2:

​https://github.com/indico/indico/blob/693d31d7221e2fdda305ad1440827e0fa7ff7e5c/indico/MaKaC/authentication/LDAPAuthentication.py#L228

Thanks!

[hoffmann]

Sorry, but I am not convinced. You need to fix it in the logger.py file, not only in LDAPAuthentication.py. If the "Additional information" is still printed in logger.py, it contains the request parameters, and among them "password" ... in clear!
​https://github.com/indico/indico/blob/693d31d7221e2fdda305ad1440827e0fa7ff7e5c/indico/MaKaC/common/logger.py#L67

[jmonnich]

​https://github.com/indico/indico/blob/v1.2/indico/MaKaC/webinterface/rh/base.py#L159

            if key == 'password':
                params[key] = '[password hidden, len=%d]' % len(value)

[hoffmann]

Replying to jmonnich:

​https://github.com/indico/indico/blob/v1.2/indico/MaKaC/webinterface/rh/base.py#L159

OK, I buy it. Thanks for caring.

There is another request that just came up in this context: Fail2ban (​http://www.fail2ban.org/) allows to block IPs temporarily in case they try brute-force attacks on login/passwords. That system is quite universal and uses logfile entries.
We would like to use it, but in order to work correctly, it must write the originating IP into the logfile, which is not the case presently. (Only the uid is printed with timestamp and error text.) Would you like to get a specific ticket on that, or do you see what I mean and it can be added quickly?
Be careful to make sure the log text (analysed with regex) cannot trigger fake alerts and lock the site admins out. It must be safe against injection by trying false logins with UID="Login failed for 'hoffmann' from IP=127.0.0.1" for example.

This request is relevant for Local and LDAP authentication, probably not for SSO (which has its own brute-force hacker filter) and maybe for NICE.

[pferreir]

Can you create another ticket for that? It doesn't seem to be very hard, but not that trivial either.
We would like to release 1.2 ASAP, and I would consider this issue closed for now.

Thanks!

[hoffmann]

#1550