Bug 224415 - 460.status-mail-rejects and 520.pfdenied appear broken
Summary: 460.status-mail-rejects and 520.pfdenied appear broken
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-bugs mailing list
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2017-12-18 01:48 UTC by Chris Hutchinson
Modified: 2020-01-20 03:37 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Hutchinson 2017-12-18 01:48:52 UTC
On a fresh build/install on
FreeBSD dev-box 12.0-CURRENT FreeBSD 12.0-CURRENT #0: Wed Dec 13 06:07:59 PST 2017 root@dev-box:/usr/obj/usr/src/amd64.amd64/sys/DEVBOX (r326056)

460.status-mail-rejects returns:
Checking for rejected mail hosts:
usage: fetch [-146AadFlMmnPpqRrsUv] [-B bytes] [--bind-address=host]
       [--ca-cert=file] [--ca-path=dir] [--cert=file] [--crl=file]
       [-i file] [--key=file] [-N file] [--no-passive] [--no-proxy=list]
       [--no-sslv3] [--no-tlsv1] [--no-verify-hostname] [--no-verify-peer]
       [-o file] [--referer=URL] [-S bytes] [-T seconds]
       [--user-agent=agent-string] [-w seconds] URL ...
       fetch [-146AadFlMmnPpqRrsUv] [-B bytes] [--bind-address=host]
       [--ca-cert=file] [--ca-path=dir] [--cert=file] [--crl=file]
       [-i file] [--key=file] [-N file] [--no-passive] [--no-proxy=list]
       [--no-sslv3] [--no-tlsv1] [--no-verify-hostname] [--no-verify-peer]
       [-o file] [--referer=URL] [-S bytes] [-T seconds]
       [--user-agent=agent-string] [-w seconds] -h host -f file [-c dir]

in the daily logs

and 520.pfdenied
doesn't even show up.

Both worked fine on 9, and early 11.

Thanks!

--Chris
Comment 1 Dag-Erling Smørgrav freebsd_committer 2018-10-09 09:31:14 UTC
Can you still reproduce any of this?

There is no way 460.status-mail-rejects would ever run fetch(1), so there must be something wrong on your system.

As for 520.pfdenied, it is part of the security report, not the daily report.
Comment 2 sigsys 2018-11-04 20:44:32 UTC
I noticed the problem with 520.pfdenied not reporting denied packets anymore as well. Seems like the problem appeared when support for blacklistd anchors was added.

Here's a fix:

diff --git a/usr.sbin/periodic/etc/security/520.pfdenied b/usr.sbin/periodic/etc/security/520.pfdenied
index e3021ce857c..69d9df78436 100755
--- a/usr.sbin/periodic/etc/security/520.pfdenied
+++ b/usr.sbin/periodic/etc/security/520.pfdenied
@@ -46,7 +46,7 @@ then
 	TMP=`mktemp -t security`
 	for _a in "" $(pfctl -a "blacklistd" -sA 2>/dev/null)
 	do
-		pfctl -a ${_a} -sr -v -z 2>/dev/null | \
+		pfctl -a "${_a}" -sr -v -z 2>/dev/null | \
 		nawk '{if (/^block/) {buf=$0; getline; gsub(" +"," ",$0); if ($5 > 0) print buf$0;} }' >> ${TMP}
 	done
 	if [ -s ${TMP} ]; then
Comment 3 per 2020-01-16 11:59:44 UTC
(In reply to sigsys from comment #2)
Agreed, this is a regression in base r301226, still present in 12.1-RELEASE. Agreed on the fix too.
Comment 4 sigsys 2020-01-16 19:59:06 UTC
(In reply to per from comment #3)
BTW I've been using that fix on two hosts running pf (including one with blacklistd running) and the periodic reports are still fine whenever I look at them (on 11.3 and recent 12-STABLE).
Comment 5 per 2020-01-16 20:49:10 UTC
(In reply to sigsys from comment #4)
Yes, both the bug and the fix should be pretty obvious to anyone with a reasonable knowledge of shell scripting (clearly the intent is that the "" in the 'for' list should result in the first loop round executing the equivalent of 'pfctl -a "" -sr -v -z' to get the statistics for the "default anchor" / "main ruleset" - but since an unquoted variable set to "nothing" doesn't cause an argument to be passed, the resulting command is instead effectively 'pfctl -a -sr -v -z', which doesn't produce any statistics).

I have to wonder why your fix hasn't been applied in over a year's time - do we need to create a separate bug report for 520.pfdenied, so as to not be "blocked" by the challenged and unconfirmed report for 460.status-mail-rejects here? (FWIW, it has always worked fine for me, and still does in 12.1-RELEASE.)
Comment 6 sigsys 2020-01-16 21:36:32 UTC
(In reply to per from comment #5)
Everyone's too busy and/or that isn't their part of the tree. Also it's not a very serious bug tbf.

Dunno if just bumping the PR even helps much, maybe it should be mailed to whoever played with that script last.  It's probably the people mentioned in that commit you found.
Comment 7 commit-hook freebsd_committer 2020-01-16 22:08:09 UTC
A commit references this bug:

Author: kp
Date: Thu Jan 16 22:08:05 UTC 2020
New revision: 356816
URL: https://svnweb.freebsd.org/changeset/base/356816

Log:
  Fix pfdenied not returning any results

  When _a is empty we end up with an invalid invocation of pfctl, and no output.
  We must add quotes to make it clear to pfctl that we're passing an empty anchor
  name.

  PR:		224415
  Submitted by:	sigsys AT gmail.com
  MFC after:	2 weeks

Changes:
  head/usr.sbin/periodic/etc/security/520.pfdenied
Comment 8 per 2020-01-16 22:16:44 UTC
(In reply to commit-hook from comment #7)
Thanks! (So whining in the PR does help.:-)
Comment 9 sigsys 2020-01-16 22:50:53 UTC
(In reply to commit-hook from comment #7)
Great. Thanks!

I checked a bit for the other problem in this PR while at it, and the periodic file after 460.status-mail-rejects is 480.leapfile-ntpd, which does call fetch (via the rc.d/ntpd script) to update the leap-seconds file. There were a lot of commits to it since then, so the problem might not even be there anymore.
Comment 10 Kristof Provost freebsd_committer 2020-01-17 08:15:09 UTC
(In reply to per from comment #8)
I would phrase that as "Bringing it to the attention of the right committer.".
Comment 11 per 2020-01-17 17:53:20 UTC
(In reply to Kristof Provost from comment #10)
Obviously my comment was a bit tongue-in-cheek, but your comment got me thinking - just *how* are you supposed to bring it "to the attention of the right committer."? This time updating the PR clearly worked for that, but in general? I can't agree with sigsys' suggestion to e-mail whoever introduced a bug (if you can even find that out), since it is intrusive/disruptive and not tracked, and still may not reach "the right committer".
Comment 12 per 2020-01-17 18:05:02 UTC
(In reply to sigsys from comment #9)
Good find! (And not the first time I've come across mis-diagnosing due to assuming that the message / log entry preceding "badness" is useful for analyzing the "badness".) However 'fetch' is actually invoked with $ntp_leapfile_fetch_opts, which can be set in rc.conf etc, so it's entirely possible that this is a "user error", even though it sounds unlikely from the "fresh build/install" in the description. Anyway, I don't think any progress on that can be made without the OP's cooperation - i.e. as far as I'm concerned, this PR may be closed now.
Comment 13 Kristof Provost freebsd_committer 2020-01-18 09:35:59 UTC
(In reply to per from comment #11)
Good question without a good answer.

Certainly create a bug, with as much relevant information as you can. You may also want to post it to the relevant mailing list. Figuring out who's been touching the affected (or related) code and sending them a ping (one ping only) is fine too.

Anything you can do to make it easier to fix the bug (simple reproduction instructions are awesome!) will help too.

There are no surefire ways.
Comment 14 per 2020-01-18 16:17:25 UTC
(In reply to Kristof Provost from comment #13)
OK - thanks again!
Comment 15 Chris Hutchinson 2020-01-20 03:37:47 UTC
Hi, The OP checking in here. ;)
I didn't have the patience to wait for a resolution.
So I embarked on the journey myself. I'm working on
a project based on pf(4). Considering it manages to
acquire ~2 million (bad) IPs a week. Its daily report
via periodic is important to me.
My fix for the 520.pfdenied was to revert the changes
that added "blacklist" to the script, and all was well.
I'll spin up the recent commit(to 520.pfdenied), and
report back (if I find a problem).

Thanks to all, for *finally* tending to this pr(1). :)

--chris