On Thu, Sep 12, 2013 at 10:49 AM, Phil Pennock <[email protected]> wrote: >> The (very small) patch is attached. It builds without error for me. >> Look towards the end of this message for session captures illustrating >> its function. > > If I try 2000 AUTH commands in one session, this will only capture the > last one, right?
In the $authenticated_fail_id variable, yes. It is only indicative of the last one which failed. > I think the core code change is probably right, but I'd look at adding a > failure_log option to the generic authenticator support so that logs can > be written as each auth attempt fails. I confirmed that it already does this. I telnet'd to a server and issued: EHLO tlyons AUTH PLAIN AG1lMkBleGFtcGxlLmNvbQB3cm9uZw== RSET EHLO tlyons AUTH PLAIN AG1mMkBleGFtcGxlLmNvbQB3cm9uZw== QUIT ...and this was what was logged, showing that multiple auth id's had been tried: 2013-09-12 18:05:37 plain authenticator failed for tlyons.ivenue.net (tlyons) [192.168.100.166]: 535 Incorrect authentication data ([email protected]) 2013-09-12 18:05:55 plain authenticator failed for tlyons.ivenue.net (tlyons) [192.168.100.166]: 535 Incorrect authentication data ([email protected]) > I'd also check to see if it's safest to string-escape the failed auth id > immediately, so that it can't contain newlines etc, so it's always safe > for logging. The protection against this is normally that _valid_ auth > ids are centrally allocated and trusted to not be malicious. But it It's already been stringified by existing code. A short while before the case() where it copies that id into my new global authenticated_fail_id variable: /* The value of authenticated_id is stored in the spool file and printed in log lines. It must not contain binary zeros or newline characters. In normal use, it never will, but when playing around or testing, this error can (did) happen. To guard against this, ensure that the id contains only printing characters. */ if (set_id != NULL) set_id = string_printing(set_id); Having said that, string_printing() doesn't strip the characters, it converts them to something safe to print. Trying ^I, ^M and (randomly) ^O, I saw this: LOG: quit after auth failed (set_id=me2\[email protected]) LOG: quit after auth failed (set_id=me2\[email protected]) LOG: quit after auth failed (set_id=me2\[email protected]) > might be that the string handling will always handle this safely. Easy Could be. The log messages generated at auth time for each of the lines from the QUIT acl above was the same: LOG: plain authenticator failed for mailc-ef.linkedin.com (tlyons) [199.101.162.50]: 535 Incorrect authentication data (set_id=me2\[email protected]) LOG: plain authenticator failed for mailc-ef.linkedin.com (tlyons) [199.101.162.50]: 535 Incorrect authentication data (set_id=me2\[email protected]) LOG: plain authenticator failed for mailc-ef.linkedin.com (tlyons) [199.101.162.50]: 535 Incorrect authentication data (set_id=me2\[email protected]) > test: send an authid to your patched install which has embedded > newlines and colons, etc. After all, the only character that can't be > inside a PLAIN authid is the ASCII NUL. It does work with the colon too: LOG: plain authenticator failed for mailc-ef.linkedin.com (tlyons) [199.101.162.50]: 535 Incorrect authentication data (set_id=me2:@example.com) LOG: quit after auth failed (set_id=me2:@example.com) > Then for the examples, based only on the code you already have, I'd look > to applying this check into the AUTH ACL too, by the rate-limits. Heck, > if it were me, I'd have a bounded exponential back-off timer, so that > each evaluation of AUTH doubles a delay timer, as well as being > otherwise rate-limited. It seems better to do this in the ACLs. It wouldn't be too hard to modify the existing Lena logic for repelling brute force attacks (shown below) to do that. Maybe make the delay be 20+2^(0$acl_c_authnomail) ...or something like that....assuming that delay is an expanded option. Result set: 20, 24, 36, 84, 276 Delay result set: 0, 24, 36, 84, 276 acl_check_auth: drop message = Authentication is allowed only once per message in order to slow down bruteforce cracking set acl_m_auth = ${eval10:0$acl_m_auth+1} condition = ${if >{$acl_m_auth}{2}} delay = 22s drop message = Dropped for bruteforce cracking attempt set acl_c_authnomail = ${eval10:0$acl_c_authnomail+1} condition = ${if >{$acl_c_authnomail}{4}} log_message = X-Brute-Force: $sender_host_address ...Todd -- The total budget at all receivers for solving senders' problems is $0. If you want them to accept your mail and manage it the way you want, send it the way the spec says to. --John Levine -- ## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
