Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move assignement of $pfb['dnsbl_iface'] into pfb_global() in pfblocker.inc #1105

Closed
wants to merge 1 commit into from

Conversation

Uglymotha
Copy link

@Uglymotha Uglymotha changed the title Move assignement of ['dnsbl_iface'] into pfb_global() in pfblocker.inc Move assignement of $pfb['dnsbl_iface'] into pfb_global() in pfblocker.inc Sep 2, 2021
Copy link
Contributor

@jim-p jim-p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a part of this pull request, you must increase PORTVERSION or add a PORTREVISION line in the package Makefile. Without this version increase, the package will not be rebuilt to include the new change.

See https://docs.netgate.com/pfsense/en/latest/references/developer-style-guide.html#ports-packages-specific-rules

@Uglymotha
Copy link
Author

As a part of this pull request, you must increase PORTVERSION or add a PORTREVISION line in the package Makefile. Without this version increase, the package will not be rebuilt to include the new change.

See https://docs.netgate.com/pfsense/en/latest/references/developer-style-guide.html#ports-packages-specific-rules

Done

@BBcan177
Copy link
Contributor

BBcan177 commented Sep 6, 2021

This change doesn't do anything.

The function sync_package_pfblockerng() calls these functions:

pfb_unbound_python()
pfb_unbound_dnsbl()
pfb_create_dnsbl()
pfb_update_unbound()

So that variable $pfb['dnsbl_iface'] should already be defined and defaults to "lo0' when not defined.

There is some other reason for that error to occur.

I would also suggest switching to Unbound Python mode and using Null Blocking Mode.

@Uglymotha
Copy link
Author

This change doesn't do anything.

The function sync_package_pfblockerng() calls these functions:

pfb_unbound_python()
pfb_unbound_dnsbl()
pfb_create_dnsbl()
pfb_update_unbound()

So that variable $pfb['dnsbl_iface'] should already be defined and defaults to "lo0' when not defined.

There is some other reason for that error to occur.

I would also suggest switching to Unbound Python mode and using Null Blocking Mode.

Let's just take the ballistics. pfb_sync_pckage() is called directly (on boot):

// Main pfBlockerNG function
function sync_package_pfblockerng($cron='') {
global $g, $config, $pfb, $pfbarr;
pfb_global();

    $pfb['conf_mod']                = FALSE;        // Flag to check for mods to the config.xml file. ('$pfb_config' array to hold changes)
    $pfb['filter_configure']        = FALSE;        // Flag to call filter_configure once

    // Detect boot process or package installation
    if (platform_booting() || $g['pfblockerng_install']) {
            // Create DNSBL NAT, VIP, Lighttpd service and certs if required on reboot.
            if ($pfb['dnsbl'] == 'on') {
                    pfb_create_dnsbl('enabled');
            }
            $log = 'Sync terminated during boot process.';
            pfb_logger("\n{$log}\nUPDATE PROCESS ENDED [ NOW ]\n", 1);
            log_error("[pfBlockerNG] {$log}");
            return;
    }

It calls pfb_global, where $pfb['dnsbl_iface'] is not set, ie empty. It then calls:

// Create DNSBL VIP and NAT rules, lighttpd conf and services
function pfb_create_dnsbl($mode) {
global $config, $pfb;
pfb_global();

    // Reload config.xml to get any recent changes
    $config = parse_config(true);

    $new_nat = $new_vip = $pfb_ex_nat = $pfb_ex_vip = $dnsbl_ex_nat = $dnsbl_ex_vip = array();
    $pfb['dnsbl_vip_changed'] = $pfbupdate = FALSE;
    $iface = escapeshellarg(get_real_interface($pfb['dnsbl_iface']));

Again, a call to pfb_global() and $pfb['dnsbl_iface'] is still empty, $iface is empty and the whole logic goes down the drain.
This is what you get when your program tries to maintain 3 different copies of the same settings, it gets messy.

@BBcan177
Copy link
Contributor

BBcan177 commented Sep 7, 2021

ok I see that there is another call to pfb_create_dnsbl(), I looked at the other call at the bottom of the sync function.

There are 5 variables that will need to be moved to pfb_global() (lines 6192-6197)

https://github.com/pfsense/FreeBSD-ports/blob/devel/net/pfSense-pkg-pfBlockerNG-devel/files/usr/local/pkg/pfblockerng/pfblockerng.inc#L6192-L6197

I have a PR that I plan on submitting today, so two choices:

  1. Move those 5 variables to pfb_global() and remove the bump to the makefile (I prefer to not have the user update the installation twice).

  2. I will address this in the new PR.

Thanks for reporting the bug.

@rbgarga
Copy link
Member

rbgarga commented Sep 10, 2021

Superseded by #1106

@rbgarga rbgarga closed this Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants