From d14ffae4660621e57eeb41fdee8183d8dc8e7355 Mon Sep 17 00:00:00 2001 From: Stephan de Wit Date: Fri, 14 Oct 2022 17:02:23 +0200 Subject: [PATCH] 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 | 2 + src/etc/inc/plugins.inc.d/unbound.inc | 18 ++- src/etc/rc.syshook.d/start/90-dnsbl | 3 + .../Unbound/Api/ServiceController.php | 17 +-- .../OPNsense/Unbound/forms/dnsbl.xml | 12 +- .../app/models/OPNsense/Unbound/Unbound.xml | 4 + .../mvc/app/views/OPNsense/Unbound/dnsbl.volt | 9 +- src/opnsense/scripts/unbound/blocklists.py | 28 +++-- src/opnsense/scripts/unbound/wrapper.py | 45 +------ .../conf/actions.d/actions_unbound.conf | 6 +- .../templates/OPNsense/Unbound/core/+TARGETS | 1 + .../OPNsense/Unbound/core/blocklists.conf | 2 + .../OPNsense/Unbound/core/dnsbl_module.py | 117 ++++++++++++++++++ 13 files changed, 182 insertions(+), 82 deletions(-) create mode 100755 src/etc/rc.syshook.d/start/90-dnsbl create mode 100644 src/opnsense/service/templates/OPNsense/Unbound/core/dnsbl_module.py diff --git a/plist b/plist index c5369d0c2..a08d7c2db 100644 --- a/plist +++ b/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 diff --git a/src/etc/inc/plugins.inc.d/unbound.inc b/src/etc/inc/plugins.inc.d/unbound.inc index 7e020484d..9dbdab3dc 100644 --- a/src/etc/inc/plugins.inc.d/unbound.inc +++ b/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 diff --git a/src/etc/rc.syshook.d/start/90-dnsbl b/src/etc/rc.syshook.d/start/90-dnsbl new file mode 100755 index 000000000..d1a178720 --- /dev/null +++ b/src/etc/rc.syshook.d/start/90-dnsbl @@ -0,0 +1,3 @@ +#!/bin/sh + +/usr/local/sbin/configctl -dq unbound dnsbl diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/Api/ServiceController.php b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/Api/ServiceController.php index faf0442f5..eca989e1c 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/Api/ServiceController.php +++ b/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); } } diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/dnsbl.xml b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/dnsbl.xml index 11bdc6a17..a139ab82f 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/dnsbl.xml +++ b/src/opnsense/mvc/app/controllers/OPNsense/Unbound/forms/dnsbl.xml @@ -33,6 +33,16 @@ text true - Destination ip address for entries in the blocklist (leave empty to use default: 0.0.0.0) + + Destination ip address for entries in the blocklist (leave empty to use default: 0.0.0.0). + Not used when "Return NXDOMAIN" is checked. + + + + unbound.dnsbl.nxdomain + + checkbox + true + Use the DNS response code NXDOMAIN instead of a destination address. diff --git a/src/opnsense/mvc/app/models/OPNsense/Unbound/Unbound.xml b/src/opnsense/mvc/app/models/OPNsense/Unbound/Unbound.xml index d9bb735e4..489301594 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Unbound/Unbound.xml +++ b/src/opnsense/mvc/app/models/OPNsense/Unbound/Unbound.xml @@ -150,6 +150,10 @@ N ipv4 + + N + 0 + diff --git a/src/opnsense/mvc/app/views/OPNsense/Unbound/dnsbl.volt b/src/opnsense/mvc/app/views/OPNsense/Unbound/dnsbl.volt index ccaaa7b1a..594f5a84a 100644 --- a/src/opnsense/mvc/app/views/OPNsense/Unbound/dnsbl.volt +++ b/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 @@ }); - -
{{ partial("layout_partials/base_form",['fields':dnsblForm,'id':'frm_dnsbl_settings'])}}
diff --git a/src/opnsense/scripts/unbound/blocklists.py b/src/opnsense/scripts/unbound/blocklists.py index 2e308f666..8f3e7d99d 100755 --- a/src/opnsense/scripts/unbound/blocklists.py +++ b/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) diff --git a/src/opnsense/scripts/unbound/wrapper.py b/src/opnsense/scripts/unbound/wrapper.py index b2e48da6c..dba927664 100755 --- a/src/opnsense/scripts/unbound/wrapper.py +++ b/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) diff --git a/src/opnsense/service/conf/actions.d/actions_unbound.conf b/src/opnsense/service/conf/actions.d/actions_unbound.conf index 1c7fcd835..4d4bad4ef 100644 --- a/src/opnsense/service/conf/actions.d/actions_unbound.conf +++ b/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 diff --git a/src/opnsense/service/templates/OPNsense/Unbound/core/+TARGETS b/src/opnsense/service/templates/OPNsense/Unbound/core/+TARGETS index 8eb414f4b..611b0cbfe 100644 --- a/src/opnsense/service/templates/OPNsense/Unbound/core/+TARGETS +++ b/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 diff --git a/src/opnsense/service/templates/OPNsense/Unbound/core/blocklists.conf b/src/opnsense/service/templates/OPNsense/Unbound/core/blocklists.conf index 50a57991b..3ba0089fa 100644 --- a/src/opnsense/service/templates/OPNsense/Unbound/core/blocklists.conf +++ b/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] diff --git a/src/opnsense/service/templates/OPNsense/Unbound/core/dnsbl_module.py b/src/opnsense/service/templates/OPNsense/Unbound/core/dnsbl_module.py new file mode 100644 index 000000000..05294152d --- /dev/null +++ b/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()