pfSense

The issue with the way pfSense stops Unbound, is that the Unbound service takes longer to shut down when there are many local-zone's defined. In limited testing, it can take 10-20seconds.

sigkillbypid("{$g['varrun_path']}/unbound.pid", "TERM");

The code will then try to re-start Unbound and fail due to "could not open ports".
To fix this issue, the restart of Unbound can only be done, when the Unbound service has been fully stopped.

The command '/usr/local/sbin/unbound -c /var/unbound/unbound.conf' returned exit code '1', the output was '[1489349484] unbound[90597:0] error: bind: address already in use [1489349484] unbound[90597:0] fatal error: could not open ports'

Here is a draft change to the code that seems to fix this issue. The loop might need to be increased to account for slow systems, or increase the timeout to 2 seconds.

In /etc/inc/services.inc

function services_unbound_configure()

        // kill any running Unbound instance
        if (file_exists("{$g['varrun_path']}/unbound.pid")) {
                sigkillbypid("{$g['varrun_path']}/unbound.pid", "TERM");

                for ($i=1; $i <= 30; $i++) {
                        exec("/bin/pgrep unbound 2>&1", $output, $retval);

                        if ($retval != 1) {
                                log_error("DEBUG: Unbound - waiting 1 second to shutdown");
                                sleep(1);
                        } else {
                                break;
                        }
                }
        }

However, it seems that there are other places in the pfSense code (ie: OpenVPN) that also need to be patched for this issue... So maybe this code should be placed elsewhere to cover all other instances of restarting Unbound.

Can the devs chime in on a full solution to this issue please ...

note on my system it needs a lot more than one second to shutdown, probably around 10 seconds due to the over 1 million hosts in the pfblockerng file. So sleep will be unreliable.

Its a 30x loop in 1 sec increments and breaks on Unbound being fully shutdown.

for ($i=1; $i <= 30; $i++) {

Could probably use is_process_running() from util.inc instead of the exec(). That debug stuff should certainly be wrapper into if ($g['debug']) {}, noone wants tens of logs entries about this normally.

I left the log message just for testing... Shouldn't be included in final code...
The function is_process_running() uses pgrep, so it will work fine.

I pushed a commit for this (not a PR yet because I think there's more to be done) but in case anyone wants to use this for System Patches:
https://github.com/luckman212/pfsense/commit/7d01b12a991e084576d286a784528b788745f905

thx BBcan177

Can the devs chime in on this issue please?

Are there other functions in the code that also need to be patched to fully resolve this issue?

To "me too" this report from a different direction, I realized we were seeing this issue on our HA setup during the once daily Suricata rules update. The master server was unaffected but the backup router would end up with unbound not running. With the code change BBcan177's fix's loop triggered 18 times over 24 seconds but unbound does get started. We have both routers functioning as DNS servers so I would think that unbound isn't running dramatically differently on the backup router.

We didn't have the behavior of it stopping multiple times, just that it couldn't start. Also worth noting is that manual Suricata updates or firewall syncs don't cause this issue, and that we have no local-zones defined in unbound, just one "private-domain" entry. WAN IPs are all static, though we do get "pfSense package system has detected an IP change or dynamic WAN reconnection" from OpenVPN at any sync since OpenVPN apparently assigns itself an address when that package starts.

Also worth noting is this is running on a 4 CPU VM on a Xeon E5-2609 v3 so I would think CPU speed isn't an issue.

Be aware that if you attempt to add a patch for this manually, you also need to apply the patch from #7667 first.

ok thank you for fixing this, I will test the fix as soon as possible on my firewall.

Instead of using this stop command

mwexec("echo '/usr/local/sbin/unbound-control stop' | /usr/bin/su -m unbound", true);

I would recommend the following command (And also for the other Unbound start/reload etc...)

mwexec("/usr/sbin/chroot -u unbound -g unbound / /usr/local/sbin/unbound-control -c {$g['unbound_chroot_path']}/unbound.conf stop", true);

At a minimum, please consider just adding the for/loop to check if Unbound has stopped before restarting, as there is no negative issue with that section of the recent changes.

BBcan177 . wrote:

I would recommend the following command (And also for the other Unbound start/reload etc...)

mwexec("/usr/sbin/chroot -u unbound -g unbound / /usr/local/sbin/unbound-control -c {$g['unbound_chroot_path']}/unbound.conf stop", true);

Unless I'm missing something here, why would that be any better? Doing a chroot to / would essentially a no-op, all it's doing is changing the user which "su -m" will do as well.

At a minimum, please consider just adding the for/loop to check if Unbound has stopped before restarting, as there is no negative issue with that section of the recent changes.

I'm not sure it's seen enough testing to justify that claim. I'd like to see some of the users who had problems with the full set of changes applied try a patch with just the loop.