Allow fail2ban protection

[DirkHoffmann]

# Ticket imported from Trac

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.)
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.

[DirkHoffmann]

From #1530 (for reference).

[DirkHoffmann]

Update: There are two places in the code that should trigger fail2ban:

• Login dialog (see User passwords in clear in "admin logfile" #1530 for reference) - exception for invalid credentials in https://github.com/DirkHoffmann/indico/blob/85529ca/indico/MaKaC/authentication/LDAPAuthentication.py#L224
• Account creation (see Passwords still exposed #1740 reference) - Somewhere in the process method of https://github.com/indico/indico/blob/3a2dba3/indico/MaKaC/webinterface/rh/base.py#L545

[DirkHoffmann]

Trying to figure how I can get the REMOTE_ADDR from within the log routine.
Seems that the good old environment variable can be used request.get_environ("REMOTE_ADDR"). But is that compatible with Windows? Present InDiCo code seems to use it in several places

• /MaKaC/webinterface/session/base.py
• /indico/web/wsgi/indico_wsgi_handler.py

But also req.remote_addr should work - which is used in two or three places elsewhere in the sources.

tbc…

[ThiefMaster]

Both files are obsolete since v1.2.

The correct way to get the IP address is request.remote_addr (with request being imported using from flask import request)

[DirkHoffmann]

OK, thanks for the information. Thus I should have all ingredients in hand to do the job.
On the time-scale of a couple of months, I can commit to implement it. So if someone else starts to work on the issue, let me know, in order to avoid us double work!

[DirkHoffmann]

Sorry, I have to ask for advice again. I have to implement something in v1.1(.2), so I concluded that the logger.exception() for INVALID_CREDENTIALS in LDAPAuthentication.py should be replaced by a lower level like logger.warning().
I see, you came to the same conclusion in the v1.2 sources, but you prefer logger.info() instead. I do not see info messages in /opt/indico/log/indico.log. So where do they go?

[pferreir]

Maybe your debug level is misconfigured (logging.conf)?

[DirkHoffmann]

You are probably right. I was looking in indico.conf, but the level is WARNING in logging.conf.
However it is NOTSET (i.e. "all message levels") specifically for [handler_indico], which should be the one used by LDAPAuthentication, shouldn't it?

[pferreir]

Yep. Indeed. We really have to set a better default. NOTSET is just plain silly 😟

Edit: See #1829

[DirkHoffmann]

I have a working solution for v1.1, working with
failregex = indico.auth.ldap: WARNING - Username: .* - invalid credentials. RemoteAddr: <HOST>$
in /etc/fail2ban/filter.d/indico.conf (Hehe, yet another indico.conf 😈 to confuse users.) Needed that urgently, and v1.2 is not yet installed on our system. (LDAP difficulties.)
I think I will pull-request it to v1.1 for the sake of it. (Shouldn't I?)
The way to do it in v1.2 and later will be completely different (due to refactoring), but if we could keep the messages compatible for fail2ban, that would be great. If you want to change nevertheless: The difficult part of designing a "good" message is to exclude all possible ways to inject a bunch of false error messages (by "clever" choice of user name or URL options) that would lock out sysadmins or anybody else.

[ThiefMaster]

Hm, I don't think it makes lots of sense to merge anything into v1.1 - that version is outdated and I doubt we'll release a v1.1.3 considering that we want people to update to v1.2...

[DirkHoffmann]

v1.1: I know, it is a philosophical question. But what is the alternative? Keeping forever a better version (which is in production) of v1.1 in my own git repository? Maybe it just feels bad for me, as I am not used to decentralised repos. But it feels indeed very bad to have the knowledge of the best solutions spread around the world and not one repo with the ideal, even if outdated, solution.