Merge lp:~mpontillo/maas/bug-1384334-add-dnssec-config-option into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Add configuration option for dnssec-validation.

Adds a configuration option for DNSSEC validation.

Requires packaging to update /etc/bind/named.conf.options, so that the dnssec-validation option can move to a MAAS-controlled configuration file.

[Mike Pontillo (mpontillo)]

This change depends on a packaging change that I still need to propose, so that it will properly remove the dnssec-validation directive from /etc/bind/named.conf.options.

[Mike Pontillo (mpontillo)]

Proposed https://code.launchpad.net/~mpontillo/maas/packaging-dnssec-validation-fix/+merge/257610 which is the prerequisite for this fix to work.

[Gavin Panella (allenap)]

Looks good.

review: Approve

[Mike Pontillo (mpontillo)]

Thanks Gavin. I'm going to hold off on merging this until the packaging change is approved. (and I make the minor changes based on your comments)

=== modified file 'etc/maas/templates/dns/named.conf.options.inside.maas.template'
--- etc/maas/templates/dns/named.conf.options.inside.maas.template	2014-11-14 21:39:55 +0000
+++ etc/maas/templates/dns/named.conf.options.inside.maas.template	2015-04-28 21:54:03 +0000
@@ -6,6 +6,8 @@
 };
 {{endif}}
 
+dnssec-validation {{dnssec_validation}};
+
 allow-query { any; };
 allow-recursion { trusted; };
 allow-query-cache { trusted; };

=== modified file 'src/maasserver/dns/config.py'
--- src/maasserver/dns/config.py	2015-03-26 00:12:12 +0000
+++ src/maasserver/dns/config.py	2015-04-28 21:54:03 +0000
@@ -197,7 +197,9 @@
     # expect this side-effect from calling dns_update_all_zones_now(), and
     # some that call it for this side-effect alone. At present all it does is
     # set the upstream DNS servers, nothing to do with serving zones at all!
-    bind_write_options(upstream_dns=get_upstream_dns())
+    bind_write_options(
+        upstream_dns=get_upstream_dns(),
+        dnssec_validation=get_dnssec_validation())
 
     # Nor should we be rewriting ACLs that are related only to allowing
     # recursive queries to the upstream DNS servers. Again, this is legacy,
@@ -357,6 +359,14 @@
     return [] if upstream_dns is None else upstream_dns.split()
 
 
+def get_dnssec_validation():
+    """Return the configuration option for DNSSEC validation.
+
+    :return: "on", "off", or "auto"
+    """
+    return Config.objects.get_config("dnssec_validation")
+
+
 def get_trusted_networks():
     """Return the CIDR representation of all the Networks we know about.
 

=== modified file 'src/maasserver/dns/tests/test_config.py'
--- src/maasserver/dns/tests/test_config.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/dns/tests/test_config.py	2015-04-28 21:54:03 +0000
@@ -503,7 +503,8 @@
         dns_update_all_zones_now()
         self.assertThat(
             bind_write_options,
-            MockCalledOnceWith(upstream_dns=[random_ip]))
+            MockCalledOnceWith(
+                dnssec_validation='auto', upstream_dns=[random_ip]))
 
     def test_dns_update_all_zones_now_writes_trusted_networks_parameter(self):
         self.patch(settings, 'DNS_CONNECT', True)

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2015-04-17 07:26:25 +0000
+++ src/maasserver/forms.py	2015-04-28 21:54:03 +0000
@@ -1292,6 +1292,7 @@
     enlistment_domain = get_config_field('enlistment_domain')
     http_proxy = get_config_field('http_proxy')
     upstream_dns = get_config_field('upstream_dns')
+    dnssec_validation = get_config_field('dnssec_validation')
     ntp_server = get_config_field('ntp_server')
 
 

=== modified file 'src/maasserver/forms_settings.py'
--- src/maasserver/forms_settings.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/forms_settings.py	2015-04-28 21:54:03 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012-2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Configuration items definition and utilities."""
@@ -28,6 +28,7 @@
 from maasserver.models.config import (
     Config,
     DEFAULT_OS,
+    DNSSEC_VALIDATION_CHOICES,
 )
 from maasserver.utils.forms import compose_invalid_choice_text
 from maasserver.utils.osystems import (
@@ -107,6 +108,19 @@
     return field
 
 
+def make_dnssec_validation_field(*args, **kwargs):
+    """Build and return the make_dnssec_validation_field field."""
+    field = forms.ChoiceField(
+        initial=Config.objects.get_config('dnssec_validation'),
+        choices=DNSSEC_VALIDATION_CHOICES,
+        error_messages={
+            'invalid_choice': compose_invalid_choice_text(
+                'dnssec_validation', DNSSEC_VALIDATION_CHOICES)
+        },
+        **kwargs)
+    return field
+
+
 CONFIG_ITEMS = {
     'check_compatibility': {
         'default': False,
@@ -197,6 +211,19 @@
                 "server config.")
         }
     },
+    'dnssec_validation': {
+        'default': 'auto',
+        'form': make_dnssec_validation_field,
+        'form_kwargs': {
+            'label': (
+                "Enable DNSSEC validation of upstream zones"),
+            'required': False,
+            'help_text': (
+                "Only used when MAAS is running its own DNS server. This "
+                "value is used as the value of 'dnssec_validation' in the DNS "
+                "server config.")
+        }
+    },
     'ntp_server': {
         'default': None,
         'form': forms.CharField,

=== modified file 'src/maasserver/management/commands/edit_named_options.py'
--- src/maasserver/management/commands/edit_named_options.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/management/commands/edit_named_options.py	2015-04-28 21:54:03 +0000
@@ -46,9 +46,10 @@
     help = (
         "Edit the named.conf.options file so that it includes the "
         "named.conf.options.inside.maas file, which contains the "
-        "'forwarders' setting.  A backup of the old file will be made "
-        "with the suffix '.maas-YYYY-MM-DDTHH:MM:SS.mmmmmm'.  This "
-        "program must be run as root.")
+        "'forwarders' and 'dnssec-validation' settings.  A backup "
+        "of the old file will be made with the suffix "
+        "'.maas-YYYY-MM-DDTHH:MM:SS.mmmmmm'.  This program must be run as "
+        "root.")
 
     def read_file(self, config_path):
         """Open the named file and return its contents as a string."""
@@ -95,6 +96,16 @@
         if 'forwarders' in options_block:
             del options_block['forwarders']
 
+    def remove_dnssec_validation(self, options_block):
+        """Remove existing dnssec-validation from the options block.
+
+        It's a syntax error to have more than one in the combined
+        configuration for named so we just remove whatever was there.
+        There is no data loss due to the backup file made later.
+        """
+        if 'dnssec-validation' in options_block:
+            del options_block['dnssec-validation']
+
     def back_up_existing_file(self, config_path):
         now = datetime.now().isoformat()
         backup_destination = config_path + '.' + now
@@ -116,6 +127,7 @@
         # Modify the config.
         self.set_up_include_statement(options_block, config_path)
         self.remove_forwarders(options_block)
+        self.remove_dnssec_validation(options_block)
         new_content = MakeISC(config_dict)
 
         # Back up and write new file.

=== modified file 'src/maasserver/models/config.py'
--- src/maasserver/models/config.py	2015-03-27 06:41:39 +0000
+++ src/maasserver/models/config.py	2015-04-28 21:54:03 +0000
@@ -33,6 +33,11 @@
 
 
 DEFAULT_OS = UbuntuOS()
+DNSSEC_VALIDATION_CHOICES = [
+    ("auto", "Automatic (use default root key)"),
+    ("yes", "Yes (manually configured root key)"),
+    ("no", "No (Disable DNSSEC; useful when upstream DNS is misconfigured)")
+]
 
 
 def get_default_config():
@@ -53,6 +58,7 @@
         'default_distro_series': DEFAULT_OS.get_default_release(),
         'http_proxy': None,
         'upstream_dns': None,
+        'dnssec_validation': "auto",
         'ntp_server': 'ntp.ubuntu.com',
         # RPC configuration.
         'rpc_region_certificate': None,

=== modified file 'src/maasserver/tests/test_commands_edit_named_options.py'
--- src/maasserver/tests/test_commands_edit_named_options.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/tests/test_commands_edit_named_options.py	2015-04-28 21:54:03 +0000
@@ -40,6 +40,15 @@
     };
 """)
 
+OPTIONS_FILE_WITH_DNSSEC = textwrap.dedent("""\
+    options {
+        directory "/var/cache/bind";
+        dnssec-validation auto;
+        auth-nxdomain no;    # conform to RFC1035
+        listen-on-v6 { any; };
+    };
+""")
+
 OPTIONS_FILE_WITH_FORWARDERS = textwrap.dedent("""\
     options {
         directory "/var/cache/bind";
@@ -103,6 +112,19 @@
             Not(FileContains(
                 matcher=Contains('forwarders'))))
 
+    def test_removes_existing_dnssec_validation_config(self):
+        options_file = self.make_file(contents=OPTIONS_FILE_WITH_DNSSEC)
+        call_command(
+            "edit_named_options", config_path=options_file,
+            stdout=self.stdout)
+
+        # Check that the file was re-written without forwarders (since
+        # that's now in the included file).
+        self.assertThat(
+            options_file,
+            Not(FileContains(
+                matcher=Contains('dnssec-validation'))))
+
     def test_normal_operation(self):
         options_file = self.make_file(contents=OPTIONS_FILE)
         call_command(

=== modified file 'src/provisioningserver/dns/actions.py'
--- src/provisioningserver/dns/actions.py	2015-03-25 15:33:23 +0000
+++ src/provisioningserver/dns/actions.py	2015-04-28 21:54:03 +0000
@@ -119,7 +119,7 @@
     dns_config.write_config(trusted_networks=trusted_networks)
 
 
-def bind_write_options(upstream_dns):
+def bind_write_options(upstream_dns, dnssec_validation):
     """Write BIND options.
 
     :param upstream_dns: A sequence of upstream DNS servers.
@@ -129,7 +129,8 @@
     assert not isinstance(upstream_dns, (bytes, unicode))
     assert isinstance(upstream_dns, collections.Sequence)
 
-    set_up_options_conf(upstream_dns=upstream_dns)
+    set_up_options_conf(
+        upstream_dns=upstream_dns, dnssec_validation=dnssec_validation)
 
 
 def bind_write_zones(zones):

=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py	2014-11-17 11:02:09 +0000
+++ src/provisioningserver/dns/config.py	2015-04-28 21:54:03 +0000
@@ -192,6 +192,7 @@
     # specify it. If it's not set, the substitution will fail with the default
     # template that uses this value.
     kwargs.setdefault("upstream_dns")
+    kwargs.setdefault("dnssec_validation", "auto")
     try:
         rendered = template.substitute(kwargs)
     except NameError as error:

=== modified file 'src/provisioningserver/dns/tests/test_actions.py'
--- src/provisioningserver/dns/tests/test_actions.py	2015-03-25 15:33:23 +0000
+++ src/provisioningserver/dns/tests/test_actions.py	2015-04-28 21:54:03 +0000
@@ -251,7 +251,10 @@
             factory.make_ipv4_address(),
             factory.make_ipv4_address(),
         ]
-        actions.bind_write_options(upstream_dns=upstream_dns)
+        dnssec_validation = random.choice(["auto", "yes", "no"])
+        expected_dnssec_validation = dnssec_validation
+        actions.bind_write_options(
+            upstream_dns=upstream_dns, dnssec_validation=dnssec_validation)
         expected_options_file = join(
             self.dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
         self.assertThat(expected_options_file, FileExists())
@@ -261,11 +264,15 @@
             %s;
         };
 
+        dnssec-validation %s;
+
         allow-query { any; };
         allow-recursion { trusted; };
         allow-query-cache { trusted; };
         """)
-        expected_options_content %= tuple(upstream_dns)
+        expected_options_content %= (
+            tuple(upstream_dns) + (expected_dnssec_validation,))
+
         self.assertThat(
             expected_options_file,
             FileContains(expected_options_content))