Page MenuHomeFreeBSD

MFV: blocklist (8aa81bf)
ClosedPublic

Authored by jlduran on Thu, Oct 2, 6:50 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 3, 10:44 PM
Unknown Object (File)
Fri, Oct 3, 9:47 PM
Unknown Object (File)
Fri, Oct 3, 9:28 AM
Unknown Object (File)
Thu, Oct 2, 11:11 PM
Unknown Object (File)
Thu, Oct 2, 8:39 PM
Unknown Object (File)
Thu, Oct 2, 7:59 PM
Subscribers

Details

Summary

Breaking changes:

  • Upstream commit 24932b6 ("blocklistd: log the conf file line number with bad protocol errors") breaks database compatibility with the current version. An error will be displayed: Key size mismatch 296 != 288 It is recommended to remove the old database file (/var/db/blacklistd.db), a new and compatible database will be created when the service starts.
  • Upstream commit ddf6d71 ("implement BLOCKLIST_BAD_USER as a "one-count" failure") introduced BLOCKLIST_BAD_USER with a one-count failure mechanism. BLOCKLIST_AUTH_FAIL was implemented with a two-count failure mechanism. Since we utilize BLOCKLIST_AUTH_FAIL, the number of failed attempts now doubles towards the maximum limit (nfails).

Take advantage of the renaming from blacklist to blocklist to introduce
these breaking changes.

Changes:

https://github.com/zoulasc/blocklist/compare/7093cd9...8aa81bf

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This is just the MFV commit.

For some reason, phabricator is not letting me upload the other commit.
Perhaps is best to open a pull request on GitHub as a workaround?

The draft can be found here:
https://github.com/jlduran/freebsd-src/tree/blocklist-full

I have tested building some ports with the old/new names, building on amd64/aarch64. I have yet to test pkgbase. There is a lot of duplication/symlinking, as I prefer to be rather careful with introducing breakage. At this point, blacklistd is not yet considered obsolete, but it should warn you about the renaming.

What can/should we do to handle the blacklistd.conf -> blocklistd.conf transition?

What can/should we do to handle the blacklistd.conf -> blocklistd.conf transition?

Out of creativity, I added the following comment at the beginning of the file:

# @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
# @ The file blacklistd.conf has been renamed to blocklistd.conf @
# @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@

Basically it reads /etc/blacklistd.conf if the user has blacklistd_enable="YES". Evidently, if the user configures blocklistd_enable="YES", it reads from /etc/blocklistd.conf.

The problem with this MFV, for those currently using blacklist, is a two-fold breakage:

  1. The database file /var/db/blacklistd.db will be now incompatible due to the extra "field", a line number for NetBSD's npf firewall (not applicable to FreeBSD). This is an "easy fix", just remove the file, and it'll be recreated.
  2. The column nfail in /etc/blacklistd.conf, usually associated to the number of failed attempts before blocking an IP address is now nfail * 2, because the penalty weight for BLOCKLIST_AUTH_FAIL is now twice as much. I opposed to this commit, citing the possibility of enumeration attacks, however I failed to understand the original motivation, so I just complied to the change. Upstream's argument is that the /etc/blocklistd.conf file should be adapted accordingly to match the desired behavior. I can understand that.

Regarding those two upstream commits mentioned in the "breaking changes" section, I'm indifferent. If you prefer to revert them for now, it's fine by me. I thought maybe the name change is already disruptive enough to introduce them.
The reason to keep both nomenclatures, stems from the ease of modification of the /usr/libexec/blacklistd-helper, which now also has a few warnings, depending on the firewall used (obviously, the warnings are not present in /usr/libexec/blocklistd-helper):

  1. If ipfw is used, it instructs the user to rename its blacklist sentinel file to blocklist:
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@  WARNING: rename /etc/ipfw-blacklist.rc to               @
@                  /etc/ipfw-blocklist.rc                  @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
  1. If pf is used, it instructs the user to rename the anchor(s) from blacklist to blocklist in its /etc/pf.conf file:
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@  WARNING: rename the blacklist anchor to blocklist       @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@

At the moment, it does not explicitly check for the anchor, and assumes it exists. Granted, this can be further improved.

  1. I really doubt someone is using ipf with blacklist, but it throws a warning.

In summary, I tried to keep everything separate, just adding warnings to anything blacklist-related, rather than complicating trying to unify the scripts.
Symlinking the library files, the header file, which also defines the old functions as aliases, and the enum. Again, blocklist.h can be further polished. Maybe when deprecating the functions, those too could throw a warning (those mentioned in libblacklist(3)) further down the road.
In my (very basic) testing, compiling src/ports WITH(OUT)_BLACKLIST(_SUPPORT) it turns out OK. When pkgbase-ing blocklist, a few files (debug lib32 files, etc.) end up in the wrong plist file. @ivy has generously offered help with this endeavor.
I was not able to submit the "main" commit to Phabricator, which is on the GitHub link in the first comment, if you would like to try it out.

This revision was not accepted when it landed; it landed in state Needs Review.Sun, Oct 12, 5:19 PM
This revision was automatically updated to reflect the committed changes.