git: 16d39eadf73b - main - blacklistd: Don't remove a ruleset if we have already added it

The branch main has been updated by emaste:

URL: https://cgit.FreeBSD.org/src/commit/?id=16d39eadf73b34009f5bd6e9f1aee0353903849b

commit 16d39eadf73b34009f5bd6e9f1aee0353903849b
Author:     Jose Luis Duran <jlduran@gmail.com>
AuthorDate: 2022-10-12 16:42:18 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2022-10-12 19:47:44 +0000

    blacklistd: Don't remove a ruleset if we have already added it
    
    The noted argument is wrong - if it's already been deleted then the id we
    have for it is invalid.
    Because we don't track deletions to the ruleset, working it out is
    problematic at best.
    
    Instead, if we have already added the rule treat it as a non-op.
    
    This is a valid use case because we might receive a burst of messages
    in the downstream application for the same address and process them
    one by one. It's not the job of the downstream application to track
    blacklistd state.
    
    Obtained from:  https://github.com/zoulasc/blocklist/commit/959b18a6047c6facd100e5bb8a759eb597c65df2
---
 contrib/blacklist/bin/blacklistd.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/contrib/blacklist/bin/blacklistd.c b/contrib/blacklist/bin/blacklistd.c
index 5e5a6dce1adc..714abcbcaf0e 100644
--- a/contrib/blacklist/bin/blacklistd.c
+++ b/contrib/blacklist/bin/blacklistd.c
@@ -228,24 +228,19 @@ process(bl_t bl)
 	case BL_ADD:
 		dbi.count++;
 		dbi.last = ts.tv_sec;
-		if (dbi.id[0]) {
+		if (c.c_nfail != -1 && dbi.count >= c.c_nfail) {
 			/*
-			 * We should not be getting this since the rule
-			 * should have blocked the address. A possible
-			 * explanation is that someone removed that rule,
-			 * and another would be that we got another attempt
-			 * before we added the rule. In anycase, we remove
-			 * and re-add the rule because we don't want to add
-			 * it twice, because then we'd lose track of it.
+			 * No point in re-adding the rule.
+			 * It might exist already due to latency in processing
+			 * and removing the rule is the wrong thing to do as
+			 * it allows a window to attack again.
 			 */
-			(*lfun)(LOG_DEBUG, "rule exists %s", dbi.id);
-			(void)run_change("rem", &c, dbi.id, 0);
-			dbi.id[0] = '\0';
-		}
-		if (c.c_nfail != -1 && dbi.count >= c.c_nfail) {
-			int res = run_change("add", &c, dbi.id, sizeof(dbi.id));
-			if (res == -1)
-				goto out;
+			if (dbi.id[0] == '\0') {
+				int res = run_change("add", &c,
+				    dbi.id, sizeof(dbi.id));
+				if (res == -1)
+					goto out;
+			}
 			sockaddr_snprintf(rbuf, sizeof(rbuf), "%a",
 			    (void *)&rss);
 			(*lfun)(LOG_INFO,