From 6832fd75a0b41e376e80f287f8ad3cfe599ea3d1 Mon Sep 17 00:00:00 2001 From: Stephan de Wit <33954429+swhite2@users.noreply.github.com> Date: Tue, 22 Mar 2022 13:38:26 +0100 Subject: [PATCH] unbound: implement custom forwarders over current dot setup (#5606) This PR pulls query forwarding over the current dot setup, so visually nothing changes. All API calls are redirected to new Forward functions, which slightly modifies what is returned based on whether "Query Forwarding" or "DNS over TLS" is selected from the menu. This way backwards compatibility is preserved. As an addition, a user is now able to specify a specific domain for a forward zone as well. Meaning that queries for this specific domain will skip a catch-all (".") domain (if specified), and instead use the server specified for this domain. Entering a forward zone with a catch-all domain (".") in both Query Forwading and DNS over TLS is considered a duplicate by Unbound, so a static warning for this has been attached in the grid - however, it might be possible for a user to be warned dynamically over this. --- plist | 1 + .../Unbound/Api/SettingsController.php | 108 ++++++++++++++++-- .../OPNsense/Unbound/DotController.php | 2 + .../OPNsense/Unbound/ForwardController.php | 42 +++++++ .../OPNsense/Unbound/forms/dialogDot.xml | 14 ++- .../OPNsense/Unbound/forms/forwarding.xml | 17 +++ .../app/models/OPNsense/Unbound/Menu/Menu.xml | 3 +- .../OPNsense/Unbound/Migrations/M1_0_2.php | 55 +++++++++ .../app/models/OPNsense/Unbound/Unbound.xml | 20 +++- .../mvc/app/views/OPNsense/Unbound/dot.volt | 81 ++++++++++++- src/opnsense/scripts/system/nameservers.php | 52 +++++++++ .../conf/actions.d/actions_system.conf | 6 + .../templates/OPNsense/Unbound/core/dot.conf | 53 ++++++--- src/www/services_unbound.php | 20 +--- 14 files changed, 425 insertions(+), 49 deletions(-) create mode 100755 src/opnsense/mvc/app/controllers/OPNsense/Unbound/ForwardController.php create mode 100755 src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/forwarding.xml create mode 100755 src/opnsense/mvc/app/models/OPNsense/Unbound/Migrations/M1_0_2.php create mode 100755 src/opnsense/scripts/system/nameservers.php diff --git a/plist b/plist index 116260f61..c11728b8c 100644 --- a/plist +++ b/plist @@ -574,6 +574,7 @@ /usr/local/opnsense/mvc/app/models/OPNsense/Unbound/Menu/Menu.xml /usr/local/opnsense/mvc/app/models/OPNsense/Unbound/Migrations/M1_0_0.php /usr/local/opnsense/mvc/app/models/OPNsense/Unbound/Migrations/M1_0_1.php +/usr/local/opnsense/mvc/app/models/OPNsense/Unbound/Migrations/M1_0_2.php /usr/local/opnsense/mvc/app/models/OPNsense/Unbound/Unbound.php /usr/local/opnsense/mvc/app/models/OPNsense/Unbound/Unbound.xml /usr/local/opnsense/mvc/app/views/OPNsense/CaptivePortal/clients.volt diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/Api/SettingsController.php b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/Api/SettingsController.php index 4e6e26c97..4a2da8810 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/Api/SettingsController.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/Api/SettingsController.php @@ -30,38 +30,126 @@ namespace OPNsense\Unbound\Api; use OPNsense\Base\ApiMutableModelControllerBase; +use OPNsense\Core\Backend; +use OPNsense\Core\Config; class SettingsController extends ApiMutableModelControllerBase { protected static $internalModelClass = '\OPNsense\Unbound\Unbound'; protected static $internalModelName = 'unbound'; - public function searchDotAction() - { - return $this->searchBase('dots.dot', array('enabled', 'server', 'port', 'verify')); + private string $type = "dot"; + + public function toggleSystemForwardAction() { + if ($this->request->isPost() && $this->request->hasPost('forwarding')) { + $this->sessionClose(); + Config::getInstance()->lock(); + $config = Config::getInstance()->object(); + + $val = $this->request->getPost('forwarding')['enabled']; + + /* Write to config exactly as legacy would */ + $config->unbound->forwarding = !empty($val); + if ($val != "1") { + /* legacy uses isset() */ + unset($config->unbound->forwarding); + } + + /* save and release lock */ + Config::getInstance()->save(); + } + } - public function getDotAction($uuid = null) + public function getSystemForwardAction() { + $config = Config::getInstance()->object(); + return array("forwarding" => + array( "enabled" => + empty($config->unbound->forwarding) ? 0 : 1 + ) + ); + } + + public function getNameserversAction() { + if ($this->request->isGet()) { + $backend = new Backend(); + $nameservers = json_decode(trim($backend->configdRun("system list nameservers"))); + + if ($nameservers !== null) { + $result = array(); + $config = Config::getInstance()->object(); + if (isset($config->system->dnsallowoverride)) { + foreach ($nameservers->dynamic as $dynamic) { + $result[] = $dynamic; + } + } + foreach ($nameservers->static as $static) { + $result[] = $static; + } + + return $result; + } + } + return array("message" => "Unable to run configd action"); + } + + /* + * Catch all Dot API endpoints and redirect them to Forward for + * backwards compatibility and infer the type from the request. + * If no type is provided, default to dot. + */ + public function __call($method, $args) + { + if (substr($method, -6) == 'Action') { + $fn = preg_replace('/Dot/', 'Forward', $method); + if (method_exists(get_class($this), $fn)) { + if (preg_match("/forward/i", $this->request->getHTTPReferer())) { + $this->type = "forward"; + } + return $this->$fn(...$args); + } + } + } + + public function searchForwardAction() + { + $filter_fn = function ($record) { + return $record->type == $this->type; + }; + + return $this->searchBase( + 'dots.dot', + array('enabled', 'server', 'port', 'verify', 'type', 'domain'), + null, + $filter_fn + ); + } + + public function getForwardAction($uuid = null) { return $this->getBase('dot', 'dots.dot', $uuid); } - public function addDotAction() + public function addForwardAction() { - return $this->addBase('dot', 'dots.dot'); + return $this->addBase('dot', 'dots.dot', + [ "type" => $this->type ] + ); } - public function delDotAction($uuid) + public function delForwardAction($uuid) { return $this->delBase('dots.dot', $uuid); } - public function setDotAction($uuid) + public function setForwardAction($uuid) { - return $this->setBase('dot', 'dots.dot', $uuid); + return $this->setBase('dot', 'dots.dot', $uuid, + [ "type" => $this->type ] + ); } - public function toggleDotAction($uuid, $enabled = null) + public function toggleForwardAction($uuid, $enabled = null) { return $this->toggleBase('dots.dot', $uuid, $enabled); } diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/DotController.php b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/DotController.php index 418564372..81d732619 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/DotController.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/DotController.php @@ -34,6 +34,8 @@ class DotController extends IndexController { public function indexAction() { + $this->view->selected_forward = ""; + $this->view->forwardingForm = $this->getForm('forwarding'); $this->view->formDialogEdit = $this->getForm('dialogDot'); $this->view->pick('OPNsense/Unbound/dot'); } diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/ForwardController.php b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/ForwardController.php new file mode 100755 index 000000000..bdfbf365b --- /dev/null +++ b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/ForwardController.php @@ -0,0 +1,42 @@ +view->selected_forward = "forward"; + $this->view->forwardingForm = $this->getForm('forwarding'); + $this->view->formDialogEdit = $this->getForm('dialogDot'); + $this->view->pick('OPNsense/Unbound/dot'); + } +} diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/dialogDot.xml b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/dialogDot.xml index 95f37a894..0dac22c94 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/dialogDot.xml +++ b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/dialogDot.xml @@ -5,6 +5,15 @@ checkbox Enable or disable this server. + + dot.domain + + text + + If a domain is entered here, queries for this specific domain will be forwarded to the specified server. + Leave blank to forward all queries to the specified server (default). + + dot.server @@ -21,6 +30,9 @@ dot.verify text - Verify if CN in certificate matches this value. + + Verify if CN in certificate matches this value. Please note that if this field is omitted, + Unbound will not perform any certificate verification. Therefore, man-in-the-middle attacks are still possible. + diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/forwarding.xml b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/forwarding.xml new file mode 100755 index 000000000..c30f41293 --- /dev/null +++ b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/forwarding.xml @@ -0,0 +1,17 @@ +
+ + forwarding.enabled + + checkbox + + + The configured system nameservers will be used to forward queries to. + This will override any entry in the grid below. + + + + forwarding.info + info + + +
diff --git a/src/opnsense/mvc/app/models/OPNsense/Unbound/Menu/Menu.xml b/src/opnsense/mvc/app/models/OPNsense/Unbound/Menu/Menu.xml index 8161bbda5..6c4af6003 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Unbound/Menu/Menu.xml +++ b/src/opnsense/mvc/app/models/OPNsense/Unbound/Menu/Menu.xml @@ -8,7 +8,8 @@ - + + diff --git a/src/opnsense/mvc/app/models/OPNsense/Unbound/Migrations/M1_0_2.php b/src/opnsense/mvc/app/models/OPNsense/Unbound/Migrations/M1_0_2.php new file mode 100755 index 000000000..3348612a2 --- /dev/null +++ b/src/opnsense/mvc/app/models/OPNsense/Unbound/Migrations/M1_0_2.php @@ -0,0 +1,55 @@ +object(); + + $node = $config->OPNsense->unboundplus; + + if (!empty($node->dots) && !empty($node->dots->dot)) { + foreach ($model->dots->dot->iterateItems() as $dot) { + $dot->type = "dot"; + } + } + } +} diff --git a/src/opnsense/mvc/app/models/OPNsense/Unbound/Unbound.xml b/src/opnsense/mvc/app/models/OPNsense/Unbound/Unbound.xml index c8e171bca..74a093e8e 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Unbound/Unbound.xml +++ b/src/opnsense/mvc/app/models/OPNsense/Unbound/Unbound.xml @@ -1,7 +1,7 @@ //OPNsense/unboundplus Unbound configuration - 1.0.1 + 1.0.2 unbound.enable @@ -56,12 +56,30 @@ N + + + unbound.forwarding + + Y 1 + + Y + dot + + DNS over TLS + Forward + + + + N + /^(?:(?:[a-z0-9]|[a-z0-9][a-z0-9\-]*[a-z0-9])\.)*(?:[a-z0-9]|[a-z0-9][a-z0-9\-]*[a-z0-9])$/i + A valid domain must be specified. + Y diff --git a/src/opnsense/mvc/app/views/OPNsense/Unbound/dot.volt b/src/opnsense/mvc/app/views/OPNsense/Unbound/dot.volt index ef2fb615c..7129ba867 100644 --- a/src/opnsense/mvc/app/views/OPNsense/Unbound/dot.volt +++ b/src/opnsense/mvc/app/views/OPNsense/Unbound/dot.volt @@ -27,6 +27,49 @@ + - -
+
+ {# include base forwarding form #} + {{ partial("layout_partials/base_form",['fields':forwardingForm,'id':'frm_ForwardingSettings'])}} +
+ +
+ + + {% if (selected_forward|default("") == "") %} + {% endif %} @@ -121,8 +186,14 @@ +
{{ lang._('Enabled') }}{{ lang._('Domain') }} {{ lang._('Address') }} {{ lang._('Port') }}{{ lang._('Hostname') }}{{ lang._('Edit') }} | {{ lang._('Delete') }} {{ lang._('ID') }}
+
+ {{ lang._('Please note that entries without a specific domain (and thus all domains) specified in both Custom Forwarding and DNS over TLS + are considered duplicates, DNS over TLS will be preferred. If "Use System Nameservers" is checked, Unbound will use the DNS servers entered + in System->Settings->General or those obtained via DHCP or PPP on WAN if the "Allow DNS server list to be overridden by DHCP/PPP on WAN" is checked.') }} +

- - - - /> - - - - @@ -379,14 +367,10 @@ include_once("head.inc"); - ','');?> + ' Unbound resolver.');?>