unbound: rework DNSBL implementation to python module (#6083)

Replaces the current blocklist implementation to use python instead of relying on unbound-control. The latter had the drawback of a very long execution time to administrate the local-data entries both locally and in Unbound. The memory footprint was also considerably larger due to unbound internals, while the python module keeps it all in memory in a simple dictionary - reducing the total amount of memory consumption by more than a factor of 10. A drawback is a potential decrease in performance of ~15%, although most setups shouldn't be affected by this as most hardware which is capable of running this should be scaled towards its intended use.

The option of returning NXDOMAIN has also been added (fixes #6027), which in this implementation is a lot easier than what we would have to do if local-data were to be used.

plist

@@ -139,6 +139,7 @@
 /usr/local/etc/rc.syshook.d/start/25-syslog
 /usr/local/etc/rc.syshook.d/start/90-carp
 /usr/local/etc/rc.syshook.d/start/90-cron
+/usr/local/etc/rc.syshook.d/start/90-dnsbl
 /usr/local/etc/rc.syshook.d/start/95-beep
 /usr/local/etc/rc.syshook.d/stop/05-beep
 /usr/local/etc/rc.syshook.d/stop/80-freebsd
@@ -1069,6 +1070,7 @@
 /usr/local/opnsense/service/templates/OPNsense/Unbound/core/+TARGETS
 /usr/local/opnsense/service/templates/OPNsense/Unbound/core/advanced.conf
 /usr/local/opnsense/service/templates/OPNsense/Unbound/core/blocklists.conf
+/usr/local/opnsense/service/templates/OPNsense/Unbound/core/dnsbl_module.py
 /usr/local/opnsense/service/templates/OPNsense/Unbound/core/domainoverrides.conf
 /usr/local/opnsense/service/templates/OPNsense/Unbound/core/dot.conf
 /usr/local/opnsense/service/templates/OPNsense/Unbound/core/private_domains.conf

src/etc/inc/plugins.inc.d/unbound.inc

@@ -107,7 +107,9 @@ function unbound_generate_config()
         return;
     }
 
-    $dirs = array('/dev', '/etc', '/lib', '/run', '/usr', '/usr/local/sbin', '/var/db', '/var/run');
+    $pythonv = readlink('/usr/local/bin/python3');
+
+    $dirs = ['/dev', '/etc', '/lib', '/run', '/usr', '/usr/local/sbin', '/var/db', '/var/run', "/usr/local/lib/{$pythonv}"];
 
     foreach ($dirs as $dir) {
         mwexecf('/bin/mkdir -p %s', "/var/unbound{$dir}");
@@ -117,6 +119,13 @@ function unbound_generate_config()
         mwexecf('/sbin/mount -t devfs devfs %s', '/var/unbound/dev');
     }
 
+    $python_dir = "/usr/local/lib/{$pythonv}";
+    $chroot_python_dir = "/var/unbound{$python_dir}";
+
+    if (mwexec("/sbin/mount | grep {$chroot_python_dir}", true)) {
+        mwexec("/sbin/mount -r -t nullfs {$python_dir} {$chroot_python_dir}");
+    }
+
     $optimization = unbound_optimization();
 
     $module_config = '';
@@ -134,10 +143,10 @@ function unbound_generate_config()
         $module_config .= 'dns64 ';
     }
     if (isset($config['unbound']['dnssec'])) {
-        $module_config .= 'validator iterator';
+        $module_config .= 'python validator iterator';
         $anchor_file = 'auto-trust-anchor-file: /var/unbound/root.key';
     } else {
-        $module_config .= 'iterator';
+        $module_config .= 'python iterator';
     }
 
     if (!isset($config['system']['webgui']['nodnsrebindcheck'])) {
@@ -325,6 +334,9 @@ include: /var/unbound/etc/*.conf
 
 {$forward_conf}
 
+python:
+python-script: dnsbl_module.py
+
 remote-control:
     control-enable: yes
     control-interface: 127.0.0.1

src/etc/rc.syshook.d/start/90-dnsbl

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+/usr/local/sbin/configctl -dq unbound dnsbl

src/opnsense/mvc/app/controllers/OPNsense/Unbound/Api/ServiceController.php

@@ -44,20 +44,7 @@ class ServiceController extends ApiMutableServiceControllerBase
         $this->sessionClose();
         $backend = new Backend();
         $backend->configdRun('template reload ' . escapeshellarg(static::$internalServiceTemplate));
-        $response = json_decode(trim($backend->configdRun(static::$internalServiceName . ' dnsbl')), true);
-        if ($response !== null) {
-            $response['status'] = "OK";
-            $response['status_msg'] = sprintf(
-                gettext("Added %d and removed %d resource records."),
-                $response['additions'],
-                $response['removals']
-            );
-            return $response;
-        }
-
-        return array(
-            'status' => 'ERR',
-            'status_msg' => gettext('An error occurred during script execution. Check the logs for details'),
-        );
+        $response = $backend->configdRun(static::$internalServiceName . ' dnsbl');
+        return array('status' => $response);
     }
 }

src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/dnsbl.xml

@@ -33,6 +33,16 @@
         <label>Destination Address</label>
         <type>text</type>
         <advanced>true</advanced>
-        <help>Destination ip address for entries in the blocklist (leave empty to use default: 0.0.0.0)</help>
+        <help>
+            Destination ip address for entries in the blocklist (leave empty to use default: 0.0.0.0).
+            Not used when "Return NXDOMAIN" is checked.
+        </help>
+    </field>
+    <field>
+        <id>unbound.dnsbl.nxdomain</id>
+        <label>Return NXDOMAIN</label>
+        <type>checkbox</type>
+        <advanced>true</advanced>
+        <help>Use the DNS response code NXDOMAIN instead of a destination address.</help>
     </field>
 </form>

src/opnsense/mvc/app/models/OPNsense/Unbound/Unbound.xml

@@ -150,6 +150,10 @@
                 <NetMaskAllowed>N</NetMaskAllowed>
                 <AddressFamily>ipv4</AddressFamily>
             </address>
+            <nxdomain type="BooleanField">
+                <Required>N</Required>
+                <default>0</default>
+            </nxdomain>
         </dnsbl>
         <forwarding>
             <enabled type="BooleanField">

src/opnsense/mvc/app/views/OPNsense/Unbound/dnsbl.volt

@@ -40,11 +40,6 @@
                   dfObj.resolve();
               });
               return dfObj;
-          },
-          onAction: function(data, status) {
-              if (data['status'].toLowerCase().trim() == 'ok') {
-                  $("#responseMsg").removeClass("hidden").html(data['status_msg']);
-              }
           }
       });
 
@@ -52,15 +47,13 @@
    });
 </script>
 
-<div class="alert alert-info hidden" role="alert" id="responseMsg"></div>
-
 <div class="content-box" style="padding-bottom: 1.5em;">
     {{ partial("layout_partials/base_form",['fields':dnsblForm,'id':'frm_dnsbl_settings'])}}
     <div class="col-md-12">
         <hr />
         <button class="btn btn-primary" id="saveAct"
                 data-endpoint='/api/unbound/service/dnsbl'
-                data-label="{{ lang._('Download & Apply') }}"
+                data-label="{{ lang._('Apply') }}"
                 data-error-title="{{ lang._('Error updating blocklists') }}"
                 type="button">
         </button>

src/opnsense/scripts/unbound/blocklists.py

@@ -35,6 +35,7 @@ import time
 import fcntl
 from configparser import ConfigParser
 import requests
+import json
 
 def uri_reader(uri):
     req_opts = {
@@ -86,10 +87,14 @@ if __name__ == '__main__':
         r'?([\da-zA-Z]\.((xn\-\-[a-zA-Z\d]+)|([a-zA-Z\d]{2,})))$'
     )
     destination_address = '0.0.0.0'
+    rcode = 'NOERROR'
 
     startup_time = time.time()
     syslog.openlog('unbound', logoption=syslog.LOG_DAEMON, facility=syslog.LOG_LOCAL4)
-    blocklist_items = set()
+    blocklist_items = {
+        'data': {},
+        'config': {}
+    }
     if os.path.exists('/tmp/unbound-blocklists.conf'):
         cnf = ConfigParser()
         cnf.read('/tmp/unbound-blocklists.conf')
@@ -114,8 +119,11 @@ if __name__ == '__main__':
             syslog.syslog(syslog.LOG_NOTICE, 'blocklist download : exclude domains matching %s' % wp)
 
         # fetch all blocklists
-        if cnf.has_section('settings') and cnf.has_option('settings', 'address'):
-            destination_address = cnf.get('settings', 'address')
+        if cnf.has_section('settings'):
+            if cnf.has_option('settings', 'address'):
+                blocklist_items['config']['dst_addr'] = cnf.get('settings', 'address')
+            if cnf.has_option('settings', 'rcode'):
+                blocklist_items['config']['rcode'] = cnf.get('settings', 'rcode')
         if cnf.has_section('blocklists'):
             for blocklist in cnf['blocklists']:
                 file_stats = {'uri': cnf['blocklists'][blocklist], 'skip' : 0, 'blocklist': 0, 'lines' :0}
@@ -135,7 +143,9 @@ if __name__ == '__main__':
                         else:
                             if domain_pattern.match(domain):
                                 file_stats['blocklist'] += 1
-                                blocklist_items.add(entry)
+                                # We write an empty dictionary as value for now.
+                                # In the future we might want to add context per fqdn
+                                blocklist_items['data'][entry] = {}
                             else:
                                 file_stats['skip'] += 1
 
@@ -145,10 +155,14 @@ if __name__ == '__main__':
                 )
 
     # write out results
-    with open("/usr/local/etc/unbound.opnsense.d/dnsbl.conf", 'w') as unbound_outf:
+    if not os.path.exists('/var/unbound/data'):
+        os.makedirs('/var/unbound/data')
+    with open("/var/unbound/data/dnsbl.json.new", 'w') as unbound_outf:
         if blocklist_items:
-            for entry in blocklist_items:
-                unbound_outf.write("local-data: \"%s A %s\"\n" % (entry, destination_address))
+            json.dump(blocklist_items, unbound_outf)
+
+    # atomically replace the current dnsbl so unbound can pick up on it
+    os.replace('/var/unbound/data/dnsbl.json.new', '/var/unbound/data/dnsbl.json')
 
     syslog.syslog(syslog.LOG_NOTICE, "blocklist download done in %0.2f seconds (%d records)" % (
         time.time() - startup_time, len(blocklist_items)

src/opnsense/scripts/unbound/wrapper.py

@@ -42,19 +42,8 @@ def unbound_control_reader(action):
     for line in sp.stdout.strip().split("\n"):
         yield line
 
-def unbound_control_do(action, bulk_input):
-    p = subprocess.Popen(['/usr/local/sbin/unbound-control', '-c', '/var/unbound/unbound.conf', action],
-                         stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True)
-    for input in bulk_input:
-        p.stdin.write("%s\n" % input)
-
-    result = p.communicate()[0]
-    # return code is only available after communicate()
-    return (result, p.returncode)
-
 # parse arguments
 parser = argparse.ArgumentParser()
-parser.add_argument('-b', '--dnsbl', help='Update DNS blocklists', action="store_true", default=False)
 parser.add_argument('-c', '--cache', help='Dump cache', action="store_true", default=False)
 parser.add_argument('-i', '--infra', help='Dump infrastructure cache', action="store_true", default=False)
 parser.add_argument('-s', '--stats', help='Dump stats', action="store_true", default=False)
@@ -71,39 +60,7 @@ except:
     sys.exit(1)
 
 output = None
-if args.dnsbl:
-    dnsbl_files = {'new': '/usr/local/etc/unbound.opnsense.d/dnsbl.conf', 'cache': '/tmp/unbound_dnsbl.cache'}
-    dnsbl_contents = {}
-    syslog.openlog('unbound', logoption=syslog.LOG_DAEMON, facility=syslog.LOG_LOCAL4)
-    for filetype in dnsbl_files:
-        dnsbl_contents[filetype] = set()
-        if os.path.exists(dnsbl_files[filetype]):
-            with open(dnsbl_files[filetype], 'r') as current_f:
-                for line in current_f:
-                    if line.startswith('local-data:'):
-                        dnsbl_contents[filetype].add(line[11:].strip(' "\'\t\r\n'))
-
-    additions = dnsbl_contents['new'] - dnsbl_contents['cache']
-    removals = dnsbl_contents['cache'] - dnsbl_contents['new']
-    if removals:
-        # RR removals only accept domain names, so strip it again (xxx.xx 0.0.0.0 --> xxx.xx)
-        removals = {line.split(' ')[0].strip() for line in removals}
-        uc = unbound_control_do('local_datas_remove', removals)
-        syslog.syslog(syslog.LOG_NOTICE, 'unbound-control returned: %s' % uc[0])
-        if uc[1] != 0:
-            sys.exit(1)
-    if additions:
-        uc = unbound_control_do('local_datas', additions)
-        syslog.syslog(syslog.LOG_NOTICE, 'unbound-control returned: %s' % uc[0])
-        if uc[1] != 0:
-            sys.exit(1)
-
-    output = {'additions': len(additions), 'removals': len(removals)}
-
-    # finally, always save a cache to keep the current state
-    shutil.copyfile(dnsbl_files['new'], dnsbl_files['cache'])
-    syslog.syslog(syslog.LOG_NOTICE, 'got %d RR additions and %d RR removals' % (output['additions'], output['removals']))
-elif args.cache:
+if args.cache:
     output = list()
     for line in unbound_control_reader('dump_cache'):
         parts = re.split('^(\S+)\s+(?:([\d]*)\s+)?(IN)\s+(\S+)\s+(.*)$', line)

src/opnsense/service/conf/actions.d/actions_unbound.conf

@@ -66,11 +66,9 @@ type:script_output
 message:Checking Unbound configuration
 
 [dnsbl]
-command:
-    /usr/local/opnsense/scripts/unbound/blocklists.py &&
-    /usr/local/opnsense/scripts/unbound/wrapper.py -b
+command:/usr/local/opnsense/scripts/unbound/blocklists.py
 parameters:
-type:script_output
+type:script
 message:Updating Unbound DNSBLs
 description:Update Unbound DNSBLs
 

src/opnsense/service/templates/OPNsense/Unbound/core/+TARGETS

@@ -5,3 +5,4 @@ private_domains.conf:/var/unbound/private_domains.conf
 domainoverrides.conf:/usr/local/etc/unbound.opnsense.d/domainoverrides.conf
 root.min.hints:/var/unbound/root.hints
 unbound_dhcpd.conf:/usr/local/etc/unbound_dhcpd.conf
+dnsbl_module.py:/var/unbound/dnsbl_module.py

src/opnsense/service/templates/OPNsense/Unbound/core/blocklists.conf

@@ -37,6 +37,8 @@
 %}
 {%  if not helpers.empty('OPNsense.unboundplus.dnsbl.enabled') %}
 [settings]
+rcode={% if not helpers.empty('OPNsense.unboundplus.dnsbl.nxdomain') %}NXDOMAIN{%else%}NOERROR{%endif%}
+
 address={{OPNsense.unboundplus.dnsbl.address|default('0.0.0.0')}}
 
 [blocklists]

src/opnsense/service/templates/OPNsense/Unbound/core/dnsbl_module.py

@@ -0,0 +1,117 @@
+import os
+import json
+import time
+
+class ModuleContext:
+    def __init__(self, env):
+        self.env = env
+        self.dnsbl_path = '/data/dnsbl.json'
+        self.dst_addr = '0.0.0.0'
+        self.rcode = RCODE_NOERROR
+        self.dnsbl_mtime_cache = 0
+        self.dnsbl_update_time = 0
+        self.dnsbl_available = False
+
+        self.update_dnsbl()
+
+    def dnsbl_exists(self):
+        return os.path.isfile(self.dnsbl_path) and os.path.getsize(self.dnsbl_path) > 0
+
+    def load_dnsbl(self):
+        with open(self.dnsbl_path, 'r') as f:
+            try:
+                mod_env['dnsbl'] = json.load(f)
+                log_info('dnsbl_module: blocklist loaded. length is %d' % len(mod_env['dnsbl']['data']))
+                config = mod_env['dnsbl']['config']
+                self.dst_addr = config['dst_addr']
+                self.rcode = RCODE_NXDOMAIN if config['rcode'] == 'NXDOMAIN' else RCODE_NOERROR
+            except json.decoder.JSONDecodeError as e:
+                if not 'dnsbl' in mod_env:
+                    log_err("dnsbl_module: unable to bootstrap blocklist, this is likely due to a corrupted \
+                            file. Please re-apply the blocklist settings.")
+                    self.dnsbl_available = False
+                    return
+                else:
+                    log_err("dnsbl_module: error parsing blocklist: %s, reusing last known list" % e)
+
+        self.dnsbl_available = True
+
+    def dnssec_enabled(self):
+        return 'validator' in self.env.cfg.module_conf
+
+    def update_dnsbl(self):
+        if (time.time() - self.dnsbl_update_time) > 60:
+            self.dnsbl_update_time = time.time()
+            if not self.dnsbl_exists():
+                self.dnsbl_available = False
+                return
+            fstat = os.stat(self.dnsbl_path).st_mtime
+            if fstat != self.dnsbl_mtime_cache:
+                self.dnsbl_mtime_cache = fstat
+                log_info("dnsbl_module: updating blocklist.")
+                self.load_dnsbl()
+
+    def filter_query(self, id, qstate):
+        self.update_dnsbl()
+        qname = qstate.qinfo.qname_str
+        if self.dnsbl_available and qname.rstrip('.') in mod_env['dnsbl']['data']:
+            qstate.return_rcode = self.rcode
+
+            if self.rcode == RCODE_NXDOMAIN:
+                # exit early
+                qstate.ext_state[id] = MODULE_FINISHED
+                return True
+
+            qtype = qstate.qinfo.qtype
+            msg = DNSMessage(qname, RR_TYPE_A, RR_CLASS_IN, PKT_QR | PKT_RA | PKT_AA)
+            if (qtype == RR_TYPE_A) or (qtype == RR_TYPE_ANY):
+                msg.answer.append("%s 3600 IN A %s" % (qname, self.dst_addr))
+            if not msg.set_return_msg(qstate):
+                qstate.ext_state[id] = MODULE_ERROR
+                log_err("dnsbl_module: unable to create response for %s, dropping query" % qname)
+                return True
+
+            if self.dnssec_enabled():
+                qstate.return_msg.rep.security = 2
+            qstate.ext_state[id] = MODULE_FINISHED
+        else:
+            # Pass the query to validator/iterator
+            qstate.ext_state[id] = MODULE_WAIT_MODULE
+
+        return True
+
+def init_standard(id, env):
+    ctx = ModuleContext(env)
+    mod_env['context'] = ctx
+    return True
+
+def deinit(id):
+    return True
+
+def inform_super(id, qstate, superqstate, qdata):
+    return True
+
+def operate(id, event, qstate, qdata):
+    if (event == MODULE_EVENT_NEW) or (event == MODULE_EVENT_PASS):
+        ctx = mod_env['context']
+        return ctx.filter_query(id, qstate)
+
+    if event == MODULE_EVENT_MODDONE:
+        # Iterator finished, show response (if any)
+        qstate.ext_state[id] = MODULE_FINISHED
+        return True
+
+    log_err("pythonmod: bad event. Query was %s" % qstate.qinfo.qname_str)
+    qstate.ext_state[id] = MODULE_ERROR
+    return True
+
+
+try:
+    import unboundmodule
+    test_mode = False
+except ImportError:
+    test_mode = True
+
+if __name__ == '__main__' and test_mode:
+    # Runs when executed from the command line as opposed to embedded in Unbound. For future reference
+    exit()