search

meeting-room-booking-system / mrbs-code

MRBS application code
Other
127 stars 63 forks source link

no default LDAP auth logs #631

Open jberanek opened 5 years ago

jberanek commented 5 years ago

Hi,

It's apparently necessary to set $ldap_debug in order to get the LDAP authorization code to log any errors. That's a bad thing not just from the basic debugging perspective, but it means it's that much harder to notice LDAP bruteforce attempts (and react to them using fail2ban or whatever).

Please make the basic error messages distinct from debug messages there and log the former unconditionally.

TIA.

Reported by: shallot

Original Ticket: mrbs/bugs/426

jberanek commented 5 years ago

Won't removing the error control operators (Bugs #425) solve this problem?

Original comment by: campbell-m

jberanek commented 5 years ago

Protecting against brute-force attacks is up to the LDAP server, surely?

Original comment by: jberanek

jberanek commented 5 years ago

Having removed the suppression operator from the ldap_bind() I can see that it now raises errors for invalid credentials. I think this is potentially a problem because

  1. It could possibly enable a DoS attack as the log file fills up with failed login attempts
  2. As things stand in MRBS the error message will also include the failed username and password, exposing the passwords of innocent users who accidentally make a small error in their credentials.

I propose to restore the @ operator on ldap_bind() calls and then in the event of an error, call trigger_error() raising the original error, unless the original error was LDAP_INVALID_CREDENTIALS. This would be the default behaviour but optionally you could configure MRBS to flag all ldap_bind() errors.

Original comment by: campbell-m

jberanek commented 5 years ago

I've now committed this change in 633efd.

Original comment by: campbell-m

jberanek commented 5 years ago

(Missed this communication earlier, sorry.)

Protecting against attacks on the machine with MRBS is much more useful because if you do it just on the LDAP server, whenever it would react, it would kill all LDAP connections from MRBS as a whole, and prevent both legitimate as well as abusive usage. Whereas, the MRBS log can point to the actual attacker's IP address which can then be blocked instead.

Log-based DoS is a fact of life for any service that accepts and validates unknown input. If the attacker POSTs e.g. 20 bytes for the username field and 20 bytes for the password field, just logging the most basic sentence "Access denied from 111.111.111.111" with a timestamp has probably already caused more bytes to be logged than there were in the input. But that doesn't mean that hiding security incidents is a good idea for the users.

Original comment by: *anonymous

jberanek commented 5 years ago

Presumably a log would also be useful for other authentication schemes? Would it be better to have a new table, eg login_attempts, and record all login attempts there, including whether the login was successful or not and the associated IP address? It should also have some kind of automatic pruning mechanism.

Original comment by: campbell-m

jberanek commented 5 years ago

Yes, it would probably be a good idea for all other authentication schemes, too.

Extra SQL tables could be a good idea, too, though that does come with a fair bit of overhead - log files are very much a known quantity and automatic pruning mechanisms are par for the course, whereas with a custom SQL table it's not as easy to just offload it to standard logrotate.

Original comment by: *anonymous