Project

General

Profile

Actions
Copy link

Bug #14409

open

pfBlockerNG Cron Redundantly Updates pfSense Configuration When DNSBL is Disabled Due to Faulty Virtual IP Count

Added by LTC Tech 11 months ago. Updated 2 months ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
pfBlockerNG
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Plus Target Version:
Affected Version:
Affected Plus Version:
23.01
Affected Architecture:

Description

pfBlockerNG: 3.2.0_4
pfSense Plus: 23.01

Related forum post:
https://forum.netgate.com/topic/174231/pfblockerng-fills-pfsense-config-history

Even though pfBlockerNG's DNSBL is disabled it checks for the presence of Virtual IPs matching DNSBL's description. It expects to find one (IPv4 only) or two (IPv4 and IPv6) Virtual IPs. When it does not find any VIPs (as DNSBL is disabled) it flips the $pfbupdate flag causing later code to write redundant changes to the pfSense config. This floods pfSense's Config History under Diagnostics -> Backup & Restore quickly overwriting legitimate history.

Relevant code:
https://github.com/pfsense/FreeBSD-ports/blob/1669285e76c1cbbdf0e08a74603537a955839139/net/pfSense-pkg-pfBlockerNG/files/usr/local/pkg/pfblockerng/pfblockerng.inc#L2016

        // Validate DNSBL VIP address(es)
        $pfb['dnsbl_v6'] == 'on' ? $vip_count = 2 : $vip_count = 1;
        $result = array();
        foreach (array("inet {$pfb['dnsbl_vip']}", "inet6 ::{$pfb['dnsbl_vip']}") as $g_vip) {
            $g_vip = escapeshellarg($g_vip);
            exec("/sbin/ifconfig {$iface} | {$pfb['grep']} {$g_vip} 2>&1", $result, $retval);
        }
        if (count($result) != $vip_count) {
            $pfbupdate = TRUE;
        }

        // Update config.xml, if changes required
        if ($pfbupdate) {
            init_config_arr(array('nat', 'rule'));
            config_set_path('nat/rule', $new_nat);

            init_config_arr(array('virtualip', 'vip'));
            config_set_path('virtualip/vip', $new_vip);
            write_config('pfBlockerNG: saving DNSBL changes');
        }

I've come up with a patch that sets $vip_count to 0 if DNSBL is disabled. This causes the Virtual IP counts to match and avoids the config update.

--- a/src/usr/local/pkg/pfblockerng/pfblockerng.inc
+++ b/src/usr/local/pkg/pfblockerng/pfblockerng.inc
@@ -2013,7 +2013,12 @@
                }

                // Validate DNSBL VIP address(es)
-               $pfb['dnsbl_v6'] == 'on' ? $vip_count = 2 : $vip_count = 1;
+               if($mode == 'enabled') {
+                       $pfb['dnsbl_v6'] == 'on' ? $vip_count = 2 : $vip_count = 1;
+               }
+               else {
+                       $vip_count = 0;
+               }
                $result = array();
                foreach (array("inet {$pfb['dnsbl_vip']}", "inet6 ::{$pfb['dnsbl_vip']}") as $g_vip) {
                        $g_vip = escapeshellarg($g_vip);

I am not sure if this actually breaks DNSBL functionality as we don't use it. It may potentially break Virtual IP addition or removal.

Can someone with more knowledge of the code take a look?

Actions
Copy link
 #1

Updated by LTC Tech 11 months ago

Another quirk seems to be that there is some other bug that writes to config on cron until you toggle some DNSBL settings and save it. Setting the "Resolver Cache" checkbox off then saving and then setting it on again is sufficient.

Actions
Copy link
 #2

Updated by Sima Xi 2 months ago

I've come up with a patch that sets $vip_count to 0 if DNSBL is disabled. This causes the Virtual IP counts to match and avoids the config update.

[...]

I am not sure if this actually breaks DNSBL functionality as we don't use it. It may potentially break Virtual IP addition or removal.

Can someone with more knowledge of the code take a look?

That patch didn't stop the periodic updates for my system. So what I did instead was to skip that block of checks entirely when disabled:

--- pfblockerng.inc.orig        2024-02-14 16:42:24.057610000 +0800
+++ pfblockerng.inc     2024-02-14 16:43:10.935321000 +0800
@@ -2013,6 +2013,7 @@
                }

                // Validate DNSBL VIP address(es)
+       if ($mode == 'enabled') {
                $pfb['dnsbl_v6'] == 'on' ? $vip_count = 2 : $vip_count = 1;
                $result = array();
                foreach (array("inet {$pfb['dnsbl_vip']}", "inet6 ::{$pfb['dnsbl_vip']}") as $g_vip) {
@@ -2022,7 +2023,7 @@
                if (count($result) != $vip_count) {
                        $pfbupdate = TRUE;
                }
-
+       }
                // Update config.xml, if changes required
                if ($pfbupdate) {
                        init_config_arr(array('nat', 'rule'));

e.g:
        // Validate DNSBL VIP address(es)
    if ($mode == 'enabled') {
        $pfb['dnsbl_v6'] == 'on' ? $vip_count = 2 : $vip_count = 1;
        $result = array();
        foreach (array("inet {$pfb['dnsbl_vip']}", "inet6 ::{$pfb['dnsbl_vip']}") as $g_vip) {
            $g_vip = escapeshellarg($g_vip);
            exec("/sbin/ifconfig {$iface} | {$pfb['grep']} {$g_vip} 2>&1", $result, $retval);
        }
        if (count($result) != $vip_count) {
            $pfbupdate = TRUE;
        }
    }

Don't know whether this breaks anything. So do this at your own risk.

Actions
Copy link

Also available in: Atom PDF