[src/trunk]: src/external/bsd/blacklist/bin blacklist: Don't remove a ruleset...

[roy <roy%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/0db20533eced
branches:  trunk
changeset: 745753:0db20533eced
user:      roy <roy%NetBSD.org@localhost>
date:      Wed Mar 11 02:33:18 2020 +0000

description:
blacklist: 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.

diffstat:

 external/bsd/blacklist/bin/blacklistd.c |  31 +++++++++++++------------------
 1 files changed, 13 insertions(+), 18 deletions(-)

diffs (54 lines):

diff -r 9afeaf35bcce -r 0db20533eced external/bsd/blacklist/bin/blacklistd.c
--- a/external/bsd/blacklist/bin/blacklistd.c   Wed Mar 11 02:12:08 2020 +0000
+++ b/external/bsd/blacklist/bin/blacklistd.c   Wed Mar 11 02:33:18 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: blacklistd.c,v 1.41 2020/03/11 02:12:08 roy Exp $      */
+/*     $NetBSD: blacklistd.c,v 1.42 2020/03/11 02:33:18 roy Exp $      */
 
 /*-
  * Copyright (c) 2015 The NetBSD Foundation, Inc.
@@ -32,7 +32,7 @@
 #include "config.h"
 #endif
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: blacklistd.c,v 1.41 2020/03/11 02:12:08 roy Exp $");
+__RCSID("$NetBSD: blacklistd.c,v 1.42 2020/03/11 02:33:18 roy Exp $");
 
 #include <sys/types.h>
 #include <sys/socket.h>
@@ -230,24 +230,19 @@
        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,