From 158366feeab54af92d3d1220868aa24b732b63b8 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Mon, 19 Sep 2022 07:14:32 -0700 Subject: [PATCH] bind: Drop enabling DNSSEC (deprecated) as it is always enabled - As of bind 9.16, the option to enable DNSSEC 'dnssec-enable' is obsolete and has no effect[1]. The option 'dnssec-validation' controls DNSSEC validation and is set to 'auto' by default. 'auto' means that DNSSEC validation is enabled and default trust anchor is used for DNS root zone. DNSSEC signatures are also passed onto a client whenever available. Current stable, Debian Buster, has version 9.16[3]. - As of bind 9.18, the option to enable DNSSEC 'dnssec-enable' is not recognized and causes the daemon to fail to start[2]. Debian next, Debian Bookworm, has version 9.18[3]. Therefore, in testing and unstable, bind fails to start of installation from FreedomBox. - There is no use-case for changing the current default behavior. Links: 1) https://bind9.readthedocs.io/en/v9_16_32/reference.html#dnssec-validation-option 2) https://bind9.readthedocs.io/en/v9_18_6/reference.html 3) https://tracker.debian.org/pkg/bind9 Tests: - Run functional and unit tests. - Option to enable/disable DNSSEC is removed. - When bind is installed on testing without the patch, it fails to start. When the patch is applied, bind will be upgraded, the dnssec-enable option is removed from the configuration file /etc/bind/named.conf.options and bind is running. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/bind/__init__.py | 2 +- plinth/modules/bind/forms.py | 11 ++---- plinth/modules/bind/privileged.py | 37 +++++--------------- plinth/modules/bind/tests/test_bind.py | 12 ------- plinth/modules/bind/tests/test_functional.py | 31 ---------------- plinth/modules/bind/views.py | 5 ++- 6 files changed, 15 insertions(+), 83 deletions(-) diff --git a/plinth/modules/bind/__init__.py b/plinth/modules/bind/__init__.py index 326b64086..fe2f114a5 100644 --- a/plinth/modules/bind/__init__.py +++ b/plinth/modules/bind/__init__.py @@ -30,7 +30,7 @@ class BindApp(app_module.App): app_id = 'bind' - _version = 2 + _version = 3 def __init__(self): """Create components for the app.""" diff --git a/plinth/modules/bind/forms.py b/plinth/modules/bind/forms.py index 36cca46cc..95e87407e 100644 --- a/plinth/modules/bind/forms.py +++ b/plinth/modules/bind/forms.py @@ -1,7 +1,5 @@ # SPDX-License-Identifier: AGPL-3.0-or-later -""" -Forms for BIND module. -""" +"""Forms for BIND module.""" from django import forms from django.core.validators import validate_ipv46_address @@ -15,12 +13,9 @@ def validate_ips(ips): class BindForm(forms.Form): - """BIND configuration form""" + """BIND configuration form.""" + forwarders = forms.CharField( label=_('Forwarders'), required=False, validators=[validate_ips], help_text=_('A list DNS servers, separated by space, to which ' 'requests will be forwarded')) - - enable_dnssec = forms.BooleanField( - label=_('Enable DNSSEC'), required=False, - help_text=_('Enable Domain Name System Security Extensions')) diff --git a/plinth/modules/bind/privileged.py b/plinth/modules/bind/privileged.py index 562ee5199..0e1713229 100644 --- a/plinth/modules/bind/privileged.py +++ b/plinth/modules/bind/privileged.py @@ -28,9 +28,6 @@ forwarders { }; forward first; -dnssec-enable yes; -dnssec-validation auto; - auth-nxdomain no; # conform to RFC1035 listen-on-v6 { any; }; }; @@ -43,6 +40,8 @@ def setup(old_version: int): if old_version == 0: with open(CONFIG_FILE, 'w', encoding='utf-8') as conf_file: conf_file.write(DEFAULT_CONFIG) + elif old_version < 3: + _remove_dnssec() Path(ZONES_DIR).mkdir(exist_ok=True, parents=True) @@ -50,10 +49,9 @@ def setup(old_version: int): @privileged -def configure(forwarders: str, dnssec: bool): +def configure(forwarders: str): """Configure BIND.""" _set_forwarders(forwarders) - _set_dnssec(dnssec) action_utils.service_restart('named') @@ -62,22 +60,15 @@ def get_config(): data = [line.strip() for line in open(CONFIG_FILE, 'r', encoding='utf-8')] forwarders = '' - dnssec_enabled = False flag = False for line in data: if re.match(r'^\s*forwarders\s+{', line): flag = True - elif re.match(r'^\s*dnssec-enable\s+yes;', line): - dnssec_enabled = True elif flag and '//' not in line: forwarders = re.sub('[;]', '', line) flag = False - conf = { - 'forwarders': forwarders, - 'enable_dnssec': dnssec_enabled, - } - return conf + return {'forwarders': forwarders} def _set_forwarders(forwarders): @@ -100,24 +91,14 @@ def _set_forwarders(forwarders): conf_file.close() -def _set_dnssec(choice): - """Enable or disable DNSSEC.""" +def _remove_dnssec(): + """Remove DNSSEC options.""" data = [line.strip() for line in open(CONFIG_FILE, 'r', encoding='utf-8')] - if choice: - conf_file = open(CONFIG_FILE, 'w', encoding='utf-8') + with open(CONFIG_FILE, 'w', encoding='utf-8') as file_handle: for line in data: - if re.match(r'//\s*dnssec-enable\s+yes;', line): - line = line.lstrip('/') - conf_file.write(line + '\n') - conf_file.close() - else: - conf_file = open(CONFIG_FILE, 'w', encoding='utf-8') - for line in data: - if re.match(r'^\s*dnssec-enable\s+yes;', line): - line = '//' + line - conf_file.write(line + '\n') - conf_file.close() + if not re.match(r'^\s*dnssec-enable\s+yes;', line): + file_handle.write(line + '\n') def get_served_domains(): diff --git a/plinth/modules/bind/tests/test_bind.py b/plinth/modules/bind/tests/test_bind.py index 465fd3781..6131893a9 100644 --- a/plinth/modules/bind/tests/test_bind.py +++ b/plinth/modules/bind/tests/test_bind.py @@ -74,18 +74,6 @@ def test_set_forwarders(): assert conf['forwarders'] == '' -@pytest.mark.usefixtures('configuration_file') -def test_enable_dnssec(): - """Test that enabling DNSSEC works.""" - bind.privileged._set_dnssec(True) - conf = bind.privileged.get_config() - assert conf['enable_dnssec'] - - bind.privileged._set_dnssec(False) - conf = bind.privileged.get_config() - assert not conf['enable_dnssec'] - - @pytest.mark.usefixtures('bind_zones_folder') def test_get_correct_served_domains(): """ diff --git a/plinth/modules/bind/tests/test_functional.py b/plinth/modules/bind/tests/test_functional.py index fc43e6ab6..99f2c1e0c 100644 --- a/plinth/modules/bind/tests/test_functional.py +++ b/plinth/modules/bind/tests/test_functional.py @@ -23,45 +23,14 @@ class TestBindApp(functional.BaseAppTests): functional.set_forwarders(session_browser, '1.1.1.1 1.0.0.1') assert functional.get_forwarders(session_browser) == '1.1.1.1 1.0.0.1' - def test_enable_disable_dnssec(self, session_browser): - """Test enabling/disabling DNSSEC.""" - functional.app_enable(session_browser, 'bind') - _enable_dnssec(session_browser, False) - - _enable_dnssec(session_browser, True) - assert _get_dnssec(session_browser) - - _enable_dnssec(session_browser, False) - assert not _get_dnssec(session_browser) - @pytest.mark.backups def test_backup_restore(self, session_browser): """Test backup and restore.""" functional.app_enable(session_browser, 'bind') functional.set_forwarders(session_browser, '1.1.1.1') - _enable_dnssec(session_browser, False) functional.backup_create(session_browser, 'bind', 'test_bind') functional.set_forwarders(session_browser, '1.0.0.1') - _enable_dnssec(session_browser, True) functional.backup_restore(session_browser, 'bind', 'test_bind') assert functional.get_forwarders(session_browser) == '1.1.1.1' - assert not _get_dnssec(session_browser) - - -def _enable_dnssec(browser, enable): - """Enable/disable DNSSEC in bind configuration.""" - functional.nav_to_module(browser, 'bind') - if enable: - browser.check('enable_dnssec') - else: - browser.uncheck('enable_dnssec') - - functional.submit(browser, form_class='form-configuration') - - -def _get_dnssec(browser): - """Return whether DNSSEC is enabled/disabled in bind configuration.""" - functional.nav_to_module(browser, 'bind') - return browser.find_by_name('enable_dnssec').first.checked diff --git a/plinth/modules/bind/views.py b/plinth/modules/bind/views.py index 20ab92e85..d5de97387 100644 --- a/plinth/modules/bind/views.py +++ b/plinth/modules/bind/views.py @@ -56,9 +56,8 @@ class BindAppView(AppView): # pylint: disable=too-many-ancestors data = form.cleaned_data old_config = privileged.get_config() - if old_config['forwarders'] != data['forwarders'] \ - or old_config['enable_dnssec'] != data['enable_dnssec']: - privileged.configure(data['forwarders'], data['enable_dnssec']) + if old_config['forwarders'] != data['forwarders']: + privileged.configure(data['forwarders']) messages.success(self.request, _('Configuration updated')) return super().form_valid(form)