Rearrange calls in unbound.inc so config text can be generated without writing the actual file and a syntax check can be run. Rearrange services_unbound.php to wait until input checking is complete before saving things to global config and run syntax check as part of input checking. Implements #4411.

2 files changed, 126 insertions, 114 deletions

diff --git a/src/etc/inc/unbound.inc b/src/etc/inc/unbound.inc
index 5447c4c..3a09ea3 100644
--- a/src/etc/inc/unbound.inc
+++ b/src/etc/inc/unbound.inc
@@ -103,14 +103,54 @@ function unbound_optimization() {
 
 }
 
+function test_unbound_config($unboundcfg, &$output) {
+	global $g;
+
+	$cfgfile = "{$g['unbound_chroot_path']}/unbound.test.conf";
+	$unboundcfgtxt = unbound_generate_config_text($unboundcfg);
+	file_put_contents($cfgfile, $unboundcfgtxt);
+
+	$rv = 0;
+	exec("/usr/local/sbin/unbound-checkconf {$cfgfile} 2>&1", $output, $rv);
+	unlink_if_exists($cfgfile);
+
+	return $rv;
+}
+
+
 function unbound_generate_config() {
+
+	$unboundcfgtxt = unbound_generate_config_text();
+
+	// Configure static Host entries
+	unbound_add_host_entries();
+
+	// Configure Domain Overrides
+	unbound_add_domain_overrides();
+
+	// Configure Unbound statistics
+	$statistics = unbound_statistics();
+
+	// Configure Unbound access-lists
+	unbound_acls_config();
+
+	create_unbound_chroot_path();
+	file_put_contents("{$g['unbound_chroot_path']}/unbound.conf", $unboundcfgtxt);
+}
+
+
+function unbound_generate_config_text($unboundcfg=NULL) {
+
 	global $config, $g;
+	if (is_null($unboundcfg)) {
+		$unboundcfg = $config['unbound'];
+	}
 
 	// Setup optimization
 	$optimization = unbound_optimization();
 
 	// Setup DNSSEC support
-	if (isset($config['unbound']['dnssec'])) {
+	if (isset($unboundcfg['dnssec'])) {
 		$module_config = "validator iterator";
 		$anchor_file = "auto-trust-anchor-file: {$g['unbound_chroot_path']}/root.key";
 	} else {
@@ -133,8 +173,8 @@ EOF;
 
 	// Determine interfaces to run on
 	$bindints = "";
-	if (!empty($config['unbound']['active_interface'])) {
-		$active_interfaces = explode(",", $config['unbound']['active_interface']);
+	if (!empty($unboundcfg['active_interface'])) {
+		$active_interfaces = explode(",", $unboundcfg['active_interface']);
 		if (in_array("all", $active_interfaces, true)) {
 			$bindints .= "interface: 0.0.0.0\n";
 			$bindints .= "interface: ::0\n";
@@ -166,9 +206,9 @@ EOF;
 
 	// Determine interfaces to run on
 	$outgoingints = "";
-	if (!empty($config['unbound']['outgoing_interface'])) {
+	if (!empty($unboundcfg['outgoing_interface'])) {
 		$outgoingints = "# Outgoing interfaces to be used\n";
-		$outgoing_interfaces = explode(",", $config['unbound']['outgoing_interface']);
+		$outgoing_interfaces = explode(",", $unboundcfg['outgoing_interface']);
 		foreach ($outgoing_interfaces as $outif) {
 			$outip = get_interface_ip($outif);
 			if (is_ipaddr($outip)) {
@@ -182,7 +222,7 @@ EOF;
 	}
 
 	// Allow DNS Rebind for forwarded domains
-	if (isset($config['unbound']['domainoverrides']) && is_array($config['unbound']['domainoverrides'])) {
+	if (isset($unboundcfg['domainoverrides']) && is_array($unboundcfg['domainoverrides'])) {
 		if (!isset($config['system']['webgui']['nodnsrebindcheck'])) {
 			$private_domains = "# Set private domains in case authoritative name server returns a Private IP address\n";
 			$private_domains .= unbound_add_domain_overrides("private");
@@ -190,21 +230,9 @@ EOF;
 		$reverse_zones .= unbound_add_domain_overrides("reverse");
 	}
 
-	// Configure static Host entries
-	unbound_add_host_entries();
-
-	// Configure Domain Overrides
-	unbound_add_domain_overrides();
-
-	// Configure Unbound statistics
-	$statistics = unbound_statistics();
-
-	// Configure Unbound access-lists
-	unbound_acls_config();
-
 	// Add custom Unbound options
-	if ($config['unbound']['custom_options']) {
-		$custom_options_source = explode("\n", base64_decode($config['unbound']['custom_options']));
+	if ($unboundcfg['custom_options']) {
+		$custom_options_source = explode("\n", base64_decode($unboundcfg['custom_options']));
 		$custom_options = "# Unbound custom options\n";
 		foreach ($custom_options_source as $ent) {
 			$custom_options .= $ent."\n";
@@ -212,31 +240,31 @@ EOF;
 	}
 
 	// Server configuration variables
-	$port = (is_port($config['unbound']['port'])) ? $config['unbound']['port'] : "53";
-	$hide_identity = isset($config['unbound']['hideidentity']) ? "yes" : "no";
-	$hide_version = isset($config['unbound']['hideversion']) ? "yes" : "no";
-	$harden_dnssec_stripped = isset($config['unbound']['dnssecstripped']) ? "yes" : "no";
-	$prefetch = isset($config['unbound']['prefetch']) ? "yes" : "no";
-	$prefetch_key = isset($config['unbound']['prefetchkey']) ? "yes" : "no";
-	$outgoing_num_tcp = (!empty($config['unbound']['outgoing_num_tcp'])) ? $config['unbound']['outgoing_num_tcp'] : "10";
-	$incoming_num_tcp = (!empty($config['unbound']['incoming_num_tcp'])) ? $config['unbound']['incoming_num_tcp'] : "10";
-	$edns_buffer_size = (!empty($config['unbound']['edns_buffer_size'])) ? $config['unbound']['edns_buffer_size'] : "4096";
-	$num_queries_per_thread = (!empty($config['unbound']['num_queries_per_thread'])) ? $config['unbound']['num_queries_per_thread'] : "4096";
-	$jostle_timeout = (!empty($config['unbound']['jostle_timeout'])) ? $config['unbound']['jostle_timeout'] : "200";
-	$cache_max_ttl = (!empty($config['unbound']['cache_max_ttl'])) ? $config['unbound']['cache_max_ttl'] : "86400";
-	$cache_min_ttl = (!empty($config['unbound']['cache_min_ttl'])) ? $config['unbound']['cache_min_ttl'] : "0";
-	$infra_host_ttl = (!empty($config['unbound']['infra_host_ttl'])) ? $config['unbound']['infra_host_ttl'] : "900";
-	$infra_cache_numhosts = (!empty($config['unbound']['infra_cache_numhosts'])) ? $config['unbound']['infra_cache_numhosts'] : "10000";
-	$unwanted_reply_threshold = (!empty($config['unbound']['unwanted_reply_threshold'])) ? $config['unbound']['unwanted_reply_threshold'] : "0";
+	$port = (is_port($unboundcfg['port'])) ? $unboundcfg['port'] : "53";
+	$hide_identity = isset($unboundcfg['hideidentity']) ? "yes" : "no";
+	$hide_version = isset($unboundcfg['hideversion']) ? "yes" : "no";
+	$harden_dnssec_stripped = isset($unboundcfg['dnssecstripped']) ? "yes" : "no";
+	$prefetch = isset($unboundcfg['prefetch']) ? "yes" : "no";
+	$prefetch_key = isset($unboundcfg['prefetchkey']) ? "yes" : "no";
+	$outgoing_num_tcp = (!empty($unboundcfg['outgoing_num_tcp'])) ? $unboundcfg['outgoing_num_tcp'] : "10";
+	$incoming_num_tcp = (!empty($unboundcfg['incoming_num_tcp'])) ? $unboundcfg['incoming_num_tcp'] : "10";
+	$edns_buffer_size = (!empty($unboundcfg['edns_buffer_size'])) ? $unboundcfg['edns_buffer_size'] : "4096";
+	$num_queries_per_thread = (!empty($unboundcfg['num_queries_per_thread'])) ? $unboundcfg['num_queries_per_thread'] : "4096";
+	$jostle_timeout = (!empty($unboundcfg['jostle_timeout'])) ? $unboundcfg['jostle_timeout'] : "200";
+	$cache_max_ttl = (!empty($unboundcfg['cache_max_ttl'])) ? $unboundcfg['cache_max_ttl'] : "86400";
+	$cache_min_ttl = (!empty($unboundcfg['cache_min_ttl'])) ? $unboundcfg['cache_min_ttl'] : "0";
+	$infra_host_ttl = (!empty($unboundcfg['infra_host_ttl'])) ? $unboundcfg['infra_host_ttl'] : "900";
+	$infra_cache_numhosts = (!empty($unboundcfg['infra_cache_numhosts'])) ? $unboundcfg['infra_cache_numhosts'] : "10000";
+	$unwanted_reply_threshold = (!empty($unboundcfg['unwanted_reply_threshold'])) ? $unboundcfg['unwanted_reply_threshold'] : "0";
 	if ($unwanted_reply_threshold == "disabled") {
 		$unwanted_reply_threshold = "0";
 	}
-	$msg_cache_size = (!empty($config['unbound']['msgcachesize'])) ? $config['unbound']['msgcachesize'] : "4";
-	$verbosity = isset($config['unbound']['log_verbosity']) ? $config['unbound']['log_verbosity'] : 1;
-	$use_caps = isset($config['unbound']['use_caps']) ? "yes" : "no";
+	$msg_cache_size = (!empty($unboundcfg['msgcachesize'])) ? $unboundcfg['msgcachesize'] : "4";
+	$verbosity = isset($unboundcfg['log_verbosity']) ? $unboundcfg['log_verbosity'] : 1;
+	$use_caps = isset($unboundcfg['use_caps']) ? "yes" : "no";
 
 	// Set up forwarding if it is configured
-	if (isset($config['unbound']['forwarding'])) {
+	if (isset($unboundcfg['forwarding'])) {
 		$dnsservers = array();
 		if (isset($config['system']['dnsallowoverride'])) {
 			$ns = array_unique(get_nameservers());
@@ -356,10 +384,7 @@ include: {$g['unbound_chroot_path']}/remotecontrol.conf
 
 EOD;
 
-	create_unbound_chroot_path();
-	file_put_contents("{$g['unbound_chroot_path']}/unbound.conf", $unboundconf);
-
-	return 0;
+	return $unboundconf;
 }
 
 function unbound_remote_control_setup() {
diff --git a/src/usr/local/www/services_unbound.php b/src/usr/local/www/services_unbound.php
index c34f591..b743f14 100644
--- a/src/usr/local/www/services_unbound.php
+++ b/src/usr/local/www/services_unbound.php
@@ -74,54 +74,53 @@ if (!is_array($config['unbound'])) {
 
 $a_unboundcfg =& $config['unbound'];
 
-if (!is_array($config['unbound']['hosts'])) {
-	$config['unbound']['hosts'] = array();
+if (!is_array($a_unboundcfg['hosts'])) {
+	$a_unboundcfg['hosts'] = array();
 }
 
-$a_hosts =& $config['unbound']['hosts'];
+$a_hosts =& $a_unboundcfg['hosts'];
 
-if (!is_array($config['unbound']['domainoverrides'])) {
-	$config['unbound']['domainoverrides'] = array();
+if (!is_array($a_unboundcfg['domainoverrides'])) {
+	$a_unboundcfg['domainoverrides'] = array();
 }
 
-$a_domainOverrides = &$config['unbound']['domainoverrides'];
+$a_domainOverrides = &$a_unboundcfg['domainoverrides'];
 
-if (isset($config['unbound']['enable'])) {
+if (isset($a_unboundcfg['enable'])) {
 	$pconfig['enable'] = true;
 }
-if (isset($config['unbound']['dnssec'])) {
+if (isset($a_unboundcfg['dnssec'])) {
 	$pconfig['dnssec'] = true;
 }
-if (isset($config['unbound']['forwarding'])) {
+if (isset($a_unboundcfg['forwarding'])) {
 	$pconfig['forwarding'] = true;
 }
-if (isset($config['unbound']['regdhcp'])) {
+if (isset($a_unboundcfg['regdhcp'])) {
 	$pconfig['regdhcp'] = true;
 }
-if (isset($config['unbound']['regdhcpstatic'])) {
+if (isset($a_unboundcfg['regdhcpstatic'])) {
 	$pconfig['regdhcpstatic'] = true;
 }
-if (isset($config['unbound']['txtsupport'])) {
+if (isset($a_unboundcfg['txtsupport'])) {
 	$pconfig['txtsupport'] = true;
 }
 
-$pconfig['port'] = $config['unbound']['port'];
-$pconfig['custom_options'] = base64_decode($config['unbound']['custom_options']);
+$pconfig['port'] = $a_unboundcfg['port'];
+$pconfig['custom_options'] = base64_decode($a_unboundcfg['custom_options']);
 
-if (empty($config['unbound']['active_interface'])) {
+if (empty($a_unboundcfg['active_interface'])) {
 	$pconfig['active_interface'] = array();
 } else {
-	$pconfig['active_interface'] = explode(",", $config['unbound']['active_interface']);
+	$pconfig['active_interface'] = explode(",", $a_unboundcfg['active_interface']);
 }
 
-if (empty($config['unbound']['outgoing_interface'])) {
+if (empty($a_unboundcfg['outgoing_interface'])) {
 	$pconfig['outgoing_interface'] = array();
 } else {
-	$pconfig['outgoing_interface'] = explode(",", $config['unbound']['outgoing_interface']);
+	$pconfig['outgoing_interface'] = explode(",", $a_unboundcfg['outgoing_interface']);
 }
 
 if ($_POST) {
-
 	if ($_POST['apply']) {
 		$retval = services_unbound_configure();
 		$savemsg = get_std_save_message($retval);
@@ -136,76 +135,63 @@ if ($_POST) {
 		$pconfig = $_POST;
 		unset($input_errors);
 
-		if (isset($_POST['enable']) && isset($config['dnsmasq']['enable'])) {
-			if ($_POST['port'] == $config['dnsmasq']['port']) {
+		if (isset($pconfig['enable']) && isset($config['dnsmasq']['enable'])) {
+			if ($pconfig['port'] == $config['dnsmasq']['port']) {
 				$input_errors[] = "The DNS Forwarder is enabled using this port. Choose a non-conflicting port, or disable the DNS Forwarder.";
 			}
 		}
 
-		if (empty($_POST['active_interface'])) {
+		if (empty($pconfig['active_interface'])) {
 			$input_errors[] = "One or more Network Interfaces must be selected for binding.";
-		} else if (!isset($config['system']['dnslocalhost']) && (!in_array("lo0", $_POST['active_interface']) && !in_array("all", $_POST['active_interface']))) {
+		} else if (!isset($config['system']['dnslocalhost']) && (!in_array("lo0", $pconfig['active_interface']) && !in_array("all", $pconfig['active_interface']))) {
 			$input_errors[] = "This system is configured to use the DNS Resolver as its DNS server, so Localhost or All must be selected in Network Interfaces.";
 		}
 
-		if (empty($_POST['outgoing_interface'])) {
+		if (empty($pconfig['outgoing_interface'])) {
 			$input_errors[] = "One or more Outgoing Network Interfaces must be selected.";
 		}
 
-		if ($_POST['port']) {
-			if (is_port($_POST['port'])) {
-				$a_unboundcfg['port'] = $_POST['port'];
-			} else {
-				$input_errors[] = gettext("You must specify a valid port number.");
-			}
-		} else if (isset($config['unbound']['port'])) {
-			unset($config['unbound']['port']);
+		if ($pconfig['port'] && !is_port($pconfig['port'])) {
+			$input_errors[] = gettext("You must specify a valid port number.");
 		}
 
-		if (isset($_POST['enable'])) {
-			$a_unboundcfg['enable'] = true;
-		} else {
-			unset($a_unboundcfg['enable']);
-		}
-		if (isset($_POST['dnssec'])) {
-			$a_unboundcfg['dnssec'] = true;
-		} else {
-			unset($a_unboundcfg['dnssec']);
-		}
-		if (isset($_POST['forwarding'])) {
-			$a_unboundcfg['forwarding'] = true;
-		} else {
-			unset($a_unboundcfg['forwarding']);
-		}
-		if (isset($_POST['regdhcp'])) {
-			$a_unboundcfg['regdhcp'] = true;
-		} else {
-			unset($a_unboundcfg['regdhcp']);
-		}
-		if (isset($_POST['regdhcpstatic'])) {
-			$a_unboundcfg['regdhcpstatic'] = true;
-		} else {
-			unset($a_unboundcfg['regdhcpstatic']);
-		}
-		if (isset($_POST['txtsupport'])) {
-			$a_unboundcfg['txtsupport'] = true;
-		} else {
-			unset($a_unboundcfg['txtsupport']);
-		}
-		if (is_array($_POST['active_interface']) && !empty($_POST['active_interface'])) {
-			$a_unboundcfg['active_interface'] = implode(",", $_POST['active_interface']);
+		if (is_array($pconfig['active_interface']) && !empty($pconfig['active_interface'])) {
+			$display_active_interface = $pconfig['active_interface'];
+			$pconfig['active_interface'] = implode(",", $pconfig['active_interface']);
 		}
 
-		if (is_array($_POST['outgoing_interface']) && !empty($_POST['outgoing_interface'])) {
-			$a_unboundcfg['outgoing_interface'] = implode(",", $_POST['outgoing_interface']);
+		$display_custom_options = $pconfig['custom_options'];
+		$pconfig['custom_options'] = base64_encode(str_replace("\r\n", "\n", $pconfig['custom_options']));
+
+		if (is_array($pconfig['outgoing_interface']) && !empty($pconfig['outgoing_interface'])) {
+			$display_outgoing_interface = $pconfig['outgoing_interface'];
+			$pconfig['outgoing_interface'] = implode(",", $pconfig['outgoing_interface']);
 		}
 
-		$a_unboundcfg['custom_options'] = base64_encode(str_replace("\r\n", "\n", $_POST['custom_options']));
+		$test_output = array();
+		if (test_unbound_config($pconfig, $test_output)) {
+			$input_errors[] = gettext("The generated config file cannot be parsed by unbound. Please correct the following errors:");
+			$input_errors = array_merge($input_errors, $test_output);
+		}
 
 		if (!$input_errors) {
+			$a_unboundcfg['enable'] = isset($pconfig['enable']);
+			$a_unboundcfg['dnssec'] = isset($pconfig['dnssec']);
+			$a_unboundcfg['forwarding'] = isset($pconfig['forwarding']);
+			$a_unboundcfg['regdhcp'] = isset($pconfig['regdhcp']);
+			$a_unboundcfg['regdhcpstatic'] = isset($pconfig['regdhcpstatic']);
+			$a_unboundcfg['txtsupport'] = isset($pconfig['txtsupport']);
+			$a_unboundcfg['active_interface'] = $pconfig['active_interface'];
+			$a_unboundcfg['outgoing_interface'] = $pconfig['outgoing_interface'];
+			$a_unboundcfg['custom_options'] = $pconfig['custom_options'];
+
 			write_config("DNS Resolver configured.");
 			mark_subsystem_dirty('unbound');
 		}
+
+		$pconfig['active_interface'] = $display_active_interface;
+		$pconfig['outgoing_interface'] = $display_outgoing_interface;
+		$pconfig['custom_options'] = $display_custom_options;
 	}
 }
 
@@ -354,14 +340,14 @@ $section->addInput(new Form_Checkbox(
 
 $btnadvdns = new Form_Button(
 	'btnadvdns',
-	'Advanced'
+	'Custom options'
 );
 
 $btnadvdns->removeClass('btn-primary')->addClass('btn-default btn-sm');
 
 $section->addInput(new Form_StaticText(
-	'Advanced',
-	$btnadvdns . ' ' . 'Show advanced optionss'
+	'Custom options',
+	$btnadvdns . ' ' . 'Show custom options'
 ));
 
 $section->addInput(new Form_TextArea (
@@ -373,6 +359,7 @@ $section->addInput(new Form_TextArea (
 $form->add($section);
 print($form);
 ?>
+
 <script>
 //<![CDATA[
 events.push(function(){