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 <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

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."""

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'))

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 = {
+    return {'forwarders': forwarders}
-        'forwarders': forwarders,
-        'enable_dnssec': dnssec_enabled,
-    }
-    return conf
 
 
 def _set_forwarders(forwarders):
@@ -100,24 +91,14 @@ def _set_forwarders(forwarders):
     conf_file.close()
 
 
-def _set_dnssec(choice):
+def _remove_dnssec():
-    """Enable or disable DNSSEC."""
+    """Remove DNSSEC options."""
     data = [line.strip() for line in open(CONFIG_FILE, 'r', encoding='utf-8')]
 
-    if choice:
+    with open(CONFIG_FILE, 'w', encoding='utf-8') as file_handle:
-        conf_file = open(CONFIG_FILE, 'w', encoding='utf-8')
         for line in data:
-            if re.match(r'//\s*dnssec-enable\s+yes;', line):
+            if not re.match(r'^\s*dnssec-enable\s+yes;', line):
-                line = line.lstrip('/')
+                file_handle.write(line + '\n')
-            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()
 
 
 def get_served_domains():

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():
     """

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

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'] \
+        if old_config['forwarders'] != data['forwarders']:
-           or old_config['enable_dnssec'] != data['enable_dnssec']:
+            privileged.configure(data['forwarders'])
-            privileged.configure(data['forwarders'], data['enable_dnssec'])
             messages.success(self.request, _('Configuration updated'))
 
         return super().form_valid(form)