Date: Mon, 17 Apr 2023 23:05:21 -0700 From: Doug Hardie <bc979@lafn.org> To: questions@freebsd.org Subject: Re: Blacklistd Issues - Problem Identified Message-ID: <24171551-4181-49C8-B1DE-2C3D9A00DC4C@sermon-archive.info> In-Reply-To: <4E4A4B99-D8DF-4C5C-9700-C56F354A9991@sermon-archive.info> References: <C632EC86-6745-42F9-A5EE-FE604C7A8599@sermon-archive.info> <8B1C1DCE-75CA-4CE9-A589-329519FB792E@sermon-archive.info> <4E4A4B99-D8DF-4C5C-9700-C56F354A9991@sermon-archive.info>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_AD600E8F-ED7D-4073-86DF-9275C4AD4955 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii -- Doug > On Apr 17, 2023, at 16:42, Doug Hardie <bc979@lafn.org> wrote: >=20 > After digging through the code for blacklistd I find that postfix and = my web server call blacklistd with a type of 1 (BL_ADD) and sure enough, = blacklistd calls the helper to add the pf rule. However. sshd calls = with type 4 (BL_BADUSER) and there is a note in the handling of that = type that says "Ignore for now". And that it does, i.e., nothing. So = the problem is in sshd using a type that is not implemented, or in = backlistd which does not implement the BADUSER type. I wonder if = Release 13.2 will fix either of those. >=20 The following patch is a temporary fix for the problem: --- blacklistd.c.orig 2023-04-17 22:58:47.552759000 -0700 +++ blacklistd.c 2023-04-17 22:46:32.069666000 -0700 @@ -225,6 +225,7 @@ if (c.c_nfail !=3D -1) dbi.count =3D c.c_nfail - 1; /*FALLTHROUGH*/ + case BL_BADUSER: case BL_ADD: dbi.count++; dbi.last =3D ts.tv_sec; @@ -260,9 +261,9 @@ dbi.count =3D 0; dbi.last =3D 0; break; - case BL_BADUSER: - /* ignore for now */ - break; +// case BL_BADUSER: +// /* ignore for now */ +// break; default: (*lfun)(LOG_ERR, "unknown message %d", bi->bi_type);=20 } Basically the BADUSER call from sshd is moved to the ADD function. So = instead of what was supposed to be an immediate shutdown on one bad = authentication regardless of the conf settings, it now follows the = config settings rule. I am not convinced that sshd should use the = BADUSER call. It causes a single typo to lock you out. It seems to me = that it should use the ADD function so the admin gets to chose the = proper number of bad authentications before lockout. I'd submit a PR on this, but all the PRs I have submitted have been left = to wither on the vine. -- Doug --Apple-Mail=_AD600E8F-ED7D-4073-86DF-9275C4AD4955 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii <html><head><meta http-equiv=3D"content-type" content=3D"text/html; = charset=3Dus-ascii"></head><body style=3D"overflow-wrap: break-word; = -webkit-nbsp-mode: space; line-break: after-white-space;"><br><div> <div>-- Doug</div> </div> <div><br><blockquote type=3D"cite"><div>On Apr 17, 2023, at 16:42, Doug = Hardie <bc979@lafn.org> wrote:</div><br = class=3D"Apple-interchange-newline"><div><div>After digging through the = code for blacklistd I find that postfix and my web server call = blacklistd with a type of 1 (BL_ADD) and sure enough, blacklistd calls = the helper to add the pf rule. However. sshd calls with type 4 = (BL_BADUSER) and there is a note in the handling of that type that says = "Ignore for now". And that it does, i.e., nothing. So the = problem is in sshd using a type that is not implemented, or in backlistd = which does not implement the BADUSER type. I wonder if Release = 13.2 will fix either of = those.<br><br></div></div></blockquote><br></div><div>The following = patch is a temporary fix for the = problem:</div><div><br></div><div><div>--- blacklistd.c.orig<span = class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>2023-04-17 22:58:47.552759000 -0700</div><div>+++ = blacklistd.c<span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>2023-04-17 22:46:32.069666000 -0700</div><div>@@ -225,6 +225,7 = @@</div><div> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>if (c.c_nfail !=3D = -1)</div><div> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>dbi.count =3D = c.c_nfail - 1;</div><div> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> = </span>/*FALLTHROUGH*/</div><div>+<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>case = BL_BADUSER:</div><div> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>case = BL_ADD:</div><div> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> = </span>dbi.count++;</div><div> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>dbi.last =3D = ts.tv_sec;</div><div>@@ -260,9 +261,9 @@</div><div> <span = class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>dbi.count =3D 0;</div><div> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>dbi.last =3D = 0;</div><div> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>break;</div><div>-<span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>case = BL_BADUSER:</div><div>-<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>/* ignore for now = */</div><div>-<span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>break;</div><div>+//<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>case = BL_BADUSER:</div><div>+//<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>/* ignore for now = */</div><div>+//<span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>break;</div><div> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>default:</div><div> <span = class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>(*lfun)(LOG_ERR, "unknown message %d", = bi->bi_type); </div><div> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> = </span>}</div><div><br></div><div><br></div><div>Basically the BADUSER = call from sshd is moved to the ADD function. So instead of what = was supposed to be an immediate shutdown on one bad authentication = regardless of the conf settings, it now follows the config settings = rule. I am not convinced that sshd should use the BADUSER call. = It causes a single typo to lock you out. It seems to me that = it should use the ADD function so the admin gets to chose the proper = number of bad authentications before = lockout.</div><div><br></div><div>I'd submit a PR on this, but all the = PRs I have submitted have been left to wither on the = vine.</div><div><br></div><div>-- = Doug</div><div><br></div></div><br></body></html>= --Apple-Mail=_AD600E8F-ED7D-4073-86DF-9275C4AD4955--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?24171551-4181-49C8-B1DE-2C3D9A00DC4C>