Bug 224415
| Summary: | 460.status-mail-rejects and 520.pfdenied appear broken | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Chris Hutchinson <portmaster> |
| Component: | conf | Assignee: | Kristof Provost <kp> |
| Status: | Closed FIXED | ||
| Severity: | Affects Many People | CC: | des, kp, per, sigsys |
| Priority: | --- | Keywords: | regression |
| Version: | CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
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. 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
(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. (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). (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.) (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. 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 (In reply to commit-hook from comment #7) Thanks! (So whining in the PR does help.:-) (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. (In reply to per from comment #8) I would phrase that as "Bringing it to the attention of the right committer.". (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". (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. (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. (In reply to Kristof Provost from comment #13) OK - thanks again! 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 A commit references this bug: Author: kp Date: Thu Jan 30 09:56:56 UTC 2020 New revision: 357289 URL: https://svnweb.freebsd.org/changeset/base/357289 Log: MFC r356816: 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 Changes: _U stable/12/ stable/12/usr.sbin/periodic/etc/security/520.pfdenied A commit references this bug: Author: kp Date: Thu Jan 30 09:56:57 UTC 2020 New revision: 357290 URL: https://svnweb.freebsd.org/changeset/base/357290 Log: MFC r356816: 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 Changes: _U stable/11/ stable/11/etc/periodic/security/520.pfdenied Committed Jan 16 22:08:05 UTC 2020 as r356816. ^Triage: assign to committer that resolved. |
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