From nobody Wed Oct 12 19:48:09 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Mnjqt1Hq8z4dssw; Wed, 12 Oct 2022 19:48:10 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Mnjqt0mSVz3m8B; Wed, 12 Oct 2022 19:48:10 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1665604090; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Avoaf2BUgEKaojxXV4/NH32iVOcwy6Hu2/6aeOVC6SA=; b=Zno/EDXSaSYeRbjqaPoyAjmE6k1aOSA4RdCTpE/WGcvH8/FH9sc+Ezh0Gum6kinrdTmwH4 Lg2z1ZIxbP6NZRKEWuo4/gkxhuzeNTx9Lk3SHzuzfHX/fQiq6gG1bi2hOziyFbJ30vlupl QoZ838PnJzNnrY39ppAZMUp8KtPc/6eG/2JMtTwrr5IzOm3OsOkUUHNAeVcoBDQFIOPjUo OBJ1V386gTbQdhkmJKv4yKvRWg3AGjr1J26xvME8KLNeh9GNofJdPYs1L03aSwJZefuOfy X1d2JldnPd+tg6Qc9s//syjMw9Wyc1YxnXDoGM7INrl74BmL3Be5wIkF4qd+yA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Mnjqs6vYRz1Q9g; Wed, 12 Oct 2022 19:48:09 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 29CJm9oa032442; Wed, 12 Oct 2022 19:48:09 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 29CJm9iX032441; Wed, 12 Oct 2022 19:48:09 GMT (envelope-from git) Date: Wed, 12 Oct 2022 19:48:09 GMT Message-Id: <202210121948.29CJm9iX032441@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Ed Maste Subject: git: 16d39eadf73b - main - blacklistd: Don't remove a ruleset if we have already added it List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: emaste X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 16d39eadf73b34009f5bd6e9f1aee0353903849b Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1665604090; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Avoaf2BUgEKaojxXV4/NH32iVOcwy6Hu2/6aeOVC6SA=; b=hO3IeozHpObXwtWRDvyWR8akVM7VnL1cmabfr/j/PlzgRmCMq0RYj7NXEzFwFZwl1o4k0P 1uT2DC4sLFZ2EL2w2w9TNDJF3jIYyVZ7jjUeMSL/GFzWCZXFVcKRkHx8PaQaKTtg70UQUI MIZGish3xoOKDNPfeHnzV9WhM2OZU50zdGI4eoADJDyDVIZHGZZDuaOCEvPfldJg4BrXrJ Uw0kQkX8m34jmFd7Ij6yIMBZcTe9HbbSwxtMFne/5/YWwUPxjmIY4p7Y2BvLKeYQ66V6P1 Vw4tHzGD65bzmY+As5sIQ+UpJmNCMgGqJHPd8Dw32IYgOeSS1KxPRH05QOg5/g== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1665604090; a=rsa-sha256; cv=none; b=xJYA+Fc6stfcSdgQazr64i+CkF/3vhhGu3du1pQk0fkJSemZpWwII12G5SmLuc5oLu/iSx y6Ltqufn0Wax3hPrl4q0QuZYYkWN0EizK7wlqEyBHiZNpfpW0JEHs7dv7syVtxG4/m4+cY 9rG8fwUuGdBbIRTyH6GIaood6OO5/CQMK8GSybelrvUsWhQ4oPT9HcWfEBKFoa+FHGyYmm odk3bDNYgiOmUOCQAgTLjCbzq9AGV1PS0gij5c6jmWjJBs0Q07IY4HFPfkFoY8DhAKPgPi UE2NJWo7Ut+MgUIWtCRSrlcRdHKmFnMJdxhz1tdAvdsgWOnmmYvAeBmkLJW3Ww== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by emaste: URL: https://cgit.FreeBSD.org/src/commit/?id=16d39eadf73b34009f5bd6e9f1aee0353903849b commit 16d39eadf73b34009f5bd6e9f1aee0353903849b Author: Jose Luis Duran AuthorDate: 2022-10-12 16:42:18 +0000 Commit: Ed Maste 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,