Bug 236312 - blacklist issues, related to base sshd
Summary: blacklist issues, related to base sshd
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: Normal Affects Some People
Assignee: Conrad Meyer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-06 06:13 UTC by Conrad Meyer
Modified: 2019-11-20 01:12 UTC (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Conrad Meyer freebsd_committer 2019-03-06 06:13:47 UTC
I wrote a bit more about it here: https://lists.freebsd.org/pipermail/freebsd-bugs/2019-February/087172.html

The salient parts are:

1. blacklistd in FreeBSD has the novel BAD_USER signal (added in fork from NetBSD).  It ignores this signal entirely.  It should not.  (sshd notifies about this signal in multiple locations.)  A conservative and easy fix here would be to treat it the same as an AUTHFAIL.

2. I believe I was mistaken about CLOEXEC and confused the distinction between exec and fork.  CLOFORK would, indeed, be problematic.  CLOEXEC is ok.

3. sendmsg() failure handling and retry is probably broken in a capability environment or chroot/jail.  Hope blacklistd started before your daemon!

4. libblacklist is probably initialized later than it should be in FreeBSD sshd.

5. FreeBSD sshd is missing a catch-all notify in cleanup_exit() on code 255.

I wanted to file a bug to track these issues before I forgot.
Comment 1 Kurt Lidl freebsd_committer 2019-03-11 19:59:22 UTC
I will look into this report.

The lack-of-handling with regards to the BAD_USER message is indeed entirely my fault.  I have some not-ready-for commit code that handles these messages either by treating the found username as "known to be bad" and immediately blacklists the remote address.  The missing code is where the bad username isn't on the list and needs to be treated like a regular failed login attempt.
Comment 2 Conrad Meyer freebsd_committer 2019-03-28 22:21:07 UTC
Do we have any updates on this, Kurt?  Would it be ok as a first-step, timely mitigation to treat BAD_USER the same as other authentication failures?  Of course, the other issues are still outstanding.  But that one seems trivial to fix in a good enough way.
Comment 3 Conrad Meyer freebsd_committer 2019-04-10 17:38:34 UTC
Sorry to nag, but any updates?
Comment 4 Kurt Lidl freebsd_committer 2019-04-15 15:27:17 UTC
Re-examinging the handling of the BAD_USER code shows that while not optimal, the current ignoring of these messages isn't harmful.

Let me explain about that code, it's unfinished state, and why it doesn't really matter that no special handling takes place.

The BAD_USER code handling is an attempt to inject some "better" handling for probing type attacks against ssh.  The blacklistd daemon just (simplying the descripting a bit), handles counting up failed attempts from a certain IP address to a certain destination port.  The "failed attempts" of type BL_ADD mean generically, that a bad authentication attempt occurred, and one should be added to the count for the IP address.  Generally speaking, the "username" for the failed authentication is unknown to blacklistd.

With sshd, there is an addition distinction made.  If sshd figured out that the username is invalid (but a username was received), it will send the BAD_USER message, along with the username.  The code in blacklistd (this is the part that is unimplemented) should compare this username to a known-bad list of usernames, and if the username is on the list, immediately blacklist the IP address, without waiting for the configured number of bad attempts in the blacklistd.conf file to be reached.  It is reasonable to consider the BL_BADUSER handlings a an "early out" for known-bad usernames.  The patches to add blacklistd support for sendmail do the same thing -- they have a concept of a bad username, and can signal that to blacklistd.

Even without any BL_BADUSER handling, the normal attempts at logging in will still get BL_ADD messages sent, so the number of failed counts will increase, ultimately causing the blacklisting to occur.

So, for example, on my machines, I have the username "admin" compiled into the static list of bad users that gets checked against BL_BADUSER messages.  So when someone attempts to login via ssh, my blacklistd gets the following messages:

BL_BADUSER "admin" remote_ip_address
BL_ADD remote_ip_address (address is blocked)

These are the messages that will be received for the first login attempt of "admin"/"some_password".  When the BL_BADUSER messages is received, the count for the remote_ip_address is set to the (trigger - 1) value.  When the BL_ADD message is received, the count is incremented, the trigger value is received,  and the remote_ip_address is blacklistd.

For machines without the BL_BADUSER handling (ie, everybody running the mainline FreeBSD blacklistd sources), the BL_BADUSER message is ignored, so the messages received look more like this, for a unknown username:

BL_BADUSER "admin" remote_ip_address
BL_ADD remote_ip_address
BL_BADUSER "admin" remote_ip_address
BL_ADD remote_ip_address
BL_BADUSER "admin" remote_ip_address
BL_ADD remote_ip_address (address is blocked)

So, the attacker get three full username/password attacks against the daemon.

In the case of the attacker using a valid username on the system, you'd have:
BL_ADD remote_ip_address
BL_ADD remote_ip_address
BL_ADD remote_ip_address (address is blocked)

I could post the code I have that has a compiled in static list of users, and there's a flag to turn off the processing of the bad-username code.  I have been hesitant to commit this code, since I really wanted to only ever release a version of the BL_BADUSER code that that did dynamic list handling, but as everyone can see, it's been a long time and I haven't progressed beyond writing a grammer that parses what I think the configuration file ought to look at, in isolation of the actual blacklist daemon code.
Comment 5 Conrad Meyer freebsd_committer 2019-04-15 18:38:41 UTC
Thanks for the update Kurt, I appreciate it.

(In reply to Kurt Lidl from comment #4)
> Even without any BL_BADUSER handling, the normal attempts at logging in will
> still get BL_ADD messages sent, so the number of failed counts will
> increase, ultimately causing the blacklisting to occur.

I think I see.  This is the BLACKLIST_NOTIFY(AUTH_FAIL) logic in userauth_finish()?

> For machines without the BL_BADUSER handling (ie, everybody running the
> mainline FreeBSD blacklistd sources), the BL_BADUSER message is ignored, so
> the messages received look more like this, for a unknown username:
> 
> BL_BADUSER "admin" remote_ip_address
> BL_ADD remote_ip_address
> BL_BADUSER "admin" remote_ip_address
> BL_ADD remote_ip_address
> BL_BADUSER "admin" remote_ip_address
> BL_ADD remote_ip_address (address is blocked)

Right, ok.

> I could post the code I have that has a compiled in static list of users,
> and there's a flag to turn off the processing of the bad-username code.  I
> have been hesitant to commit this code, since I really wanted to only ever
> release a version of the BL_BADUSER code that that did dynamic list
> handling,

I agree that makes a lot of sense.  I don't think a compile-time static list
is particularly useful.

> but as everyone can see, it's been a long time and I haven't
> progressed beyond writing a grammer that parses what I think the
> configuration file ought to look at, in isolation of the actual blacklist
> daemon code.

BadUsers is just a list of strings, right?  Would something like libucl be adequate for representing that instead of a hand-rolled parser?  Something like seems like an adequate syntax for a configuration file to me:

section "Invalid Users" {
    [
        "admin",
        "foo",
        "bar"
    ]
}
Comment 6 László Károlyi 2019-04-15 18:42:20 UTC
If I may add, I'd like to use a wildcard there.

In my view, a probe with any user and password (doesn't matter if the user is existing or not), should be blocked after reaching the configured threshold.
Comment 7 Kurt Lidl freebsd_committer 2019-04-15 19:13:33 UTC
(In reply to László Károlyi from comment #6)

That's what the current behavior is.  All that BL_BADUSER does is a shortcut to blocking before the normal number of attempts for users that are "known bad" and commonly guessed at by anonymous probers.

If you put a wildcard into that list, you'd be blocking any bad username after just one attempt.

Without the wildcard in the badusers list, you would get blocking after the configured limit of authentication failures, whether or not the username that was being tried was a valid username or not.
Comment 8 Kurt Lidl freebsd_committer 2019-04-15 19:26:08 UTC
(In reply to Conrad Meyer from comment #5)

What we really want is two types of data for the bad-user processing code.

The first maps a protocol/port (e.g. tcp/22) to a named list of usernames.

The second has name for a list, and a list of usersnames for that list.

So you might have:

tcp/22 = ssh_users;

tcp/25 = mail_users;
tcp/587 = mail_users;

ssh_users = { admin pi user };

mail_users = { nobody admin spamtrap };

Obviously, we want to be able to reuse a list for multiple protocol/port combinations,  and we also need to have separate lists for different purposes.

The yacc/lex parser I wrote digests a more complex syntax than I outlined above, but as far as I know, it does it properly.  The only thing that it doesn't handle that I think it ought to handle is a username with an embedded space in it.  Yes, I know, ssh clients won't send such a thing, but " 0101" is an actual username that I get failed login attempt for, and I'd like to just punt those on the first try, rather than giving them three attempts.
Comment 9 Conrad Meyer freebsd_committer 2019-04-16 15:05:48 UTC
(In reply to Kurt Lidl from comment #8)
> (In reply to Conrad Meyer from comment #5)
> 
> What we really want is two types of data for the bad-user processing code.
> 
> The first maps a protocol/port (e.g. tcp/22) to a named list of usernames.
> 
> The second has name for a list, and a list of usersnames for that list.
> …

That seems readily doable in UCL as well, right?

protocol-userlist-mapping {
  tcp {
    22 = "ssh_users",
    25 = "mail_users",
    587 = "mail_users",
  }
}

userlists {
  ssh_users = [ "admin", "pi", "user" ],
  mail_users = [ "nobody", "admin", "spamtrap" ],
}

> …
> Obviously, we want to be able to reuse a list for multiple protocol/port
> combinations,  and we also need to have separate lists for different
> purposes.

Right; this is readily achievable, although UCL cannot validate for us that every userlist referenced in a mapping is present (or that every userlist is addressed by a mapping); that would be enforced by the application.

> The yacc/lex parser I wrote digests a more complex syntax than I outlined
> above, but as far as I know, it does it properly.  The only thing that it
> doesn't handle that I think it ought to handle is a username with an
> embedded space in it.  Yes, I know, ssh clients won't send such a thing, but
> " 0101" is an actual username that I get failed login attempt for, and I'd
> like to just punt those on the first try, rather than giving them three
> attempts.

It's easy to embed spaces in UCL strings, at least :-).

I'm really hesitant to add a new yacc/lex parser and novel configuration format to the system in 2019.  Part of the motivation for UCL was that every old BSD program from the 80s had a different configuration syntax (e.g., cron), and that was unnecessarily difficult for new users and sysadmins a like.  So one goal is to at least be able to configure the system with a consistent format.

(CC'd allanjude@; please correct me if I'm wrong re: motivations for and abilities of UCL :-).)
Comment 10 Conrad Meyer freebsd_committer 2019-10-13 17:35:22 UTC
Thanks for the fish, Kurt.
Comment 11 László Károlyi 2019-10-14 09:31:09 UTC
What happened here?