Merge ~andreserl/maas:lp1774206_trusted_acls into maas:master

[Andres Rodriguez]

LP: #1774206 - Add config option 'dns_trusted_acl'.

MAAS DNS has a 'trusted' ACL that lists the networks that are allowed to use MAAS for DNS resolution. This option allows to specify other networks (or IPs/ACLs) not known to MAAS that can use it for DNS resolution.

110fdce... by Andres Rodriguez on 2018-08-09

LP: #1774206 - Add ability to add extra items to the 'trusted' ACL

864a786... by Andres Rodriguez on 2018-08-09

Fix lint

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1774206_trusted_acls lp:~andreserl/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/3586/console
COMMIT: 4de9e12c92cac6afccf19ebc5784ea63a6e97f46

review: Needs Fixing

[Blake Rouse (blake-rouse)]

Looks good. Well tested to!

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1774206_trusted_acls lp:~andreserl/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 864a786d09f692b7595d26aa4067728d024a61ea

review: Approve

diff --git a/src/maasserver/dns/config.py b/src/maasserver/dns/config.py
index da2968e..e5dba64 100644
--- a/src/maasserver/dns/config.py
+++ b/src/maasserver/dns/config.py
@@ -121,15 +121,26 @@ def get_dnssec_validation():
     return Config.objects.get_config("dnssec_validation")
 
 
+def get_trusted_acls():
+    """Return the configuration option for trusted ACLs.
+
+    :return: A list of CIDR-format subnet, IPs or names.
+    """
+    items = Config.objects.get_config("dns_trusted_acl")
+    return [] if items is None else items.split()
+
+
 def get_trusted_networks():
-    """Return the CIDR representation of all the Subnets we know about.
+    """Return the CIDR representation of all the subnets we know about
+    combined with the list from get_trusted_acls().
 
     :return: A list of CIDR-format subnet specifications.
     """
-    return [
+    known_subnets = [
         str(subnet.cidr)
         for subnet in Subnet.objects.all()
     ]
+    return list(set(known_subnets + get_trusted_acls()))
 
 
 def get_resource_name_for_subnet(subnet):
diff --git a/src/maasserver/dns/tests/test_config.py b/src/maasserver/dns/tests/test_config.py
index 036c144..cd933fd 100644
--- a/src/maasserver/dns/tests/test_config.py
+++ b/src/maasserver/dns/tests/test_config.py
@@ -19,6 +19,7 @@ from maasserver.dns.config import (
     dns_update_all_zones,
     get_internal_domain,
     get_resource_name_for_subnet,
+    get_trusted_acls,
     get_trusted_networks,
     get_upstream_dns,
 )
@@ -330,6 +331,17 @@ class TestDNSConfigModifications(TestDNSServer):
             compose_config_path(DNSConfig.target_file_name),
             FileContains(matcher=Contains(trusted_network)))
 
+    def test_dns_update_all_zones_writes_trusted_networks_params_extra(self):
+        self.patch(settings, 'DNS_CONNECT', True)
+        extra_trusted_network = factory.make_ipv6_network()
+        get_trusted_acls_patch = self.patch(
+            dns_config_module, 'get_trusted_acls')
+        get_trusted_acls_patch.return_value = [extra_trusted_network.cidr]
+        dns_update_all_zones()
+        self.assertThat(
+            compose_config_path(DNSConfig.target_file_name),
+            FileContains(matcher=Contains(str(extra_trusted_network))))
+
     def test_dns_config_has_NS_record(self):
         self.patch(settings, 'DNS_CONNECT', True)
         ip = factory.make_ipv4_address()
@@ -443,6 +455,34 @@ class TestGetUpstreamDNS(MAASServerTestCase):
         self.assertEqual(addresses, get_upstream_dns())
 
 
+class TestGetTrustedAcls(MAASServerTestCase):
+    """Test for maasserver/dns/config.py:get_trusted_acls()"""
+
+    def setUp(self):
+        super(TestGetTrustedAcls, self).setUp()
+        self.useFixture(RegionConfigurationFixture())
+
+    def test__returns_empty_string_if_no_networks(self):
+        self.assertEqual([], get_trusted_acls())
+
+    def test__returns_single_network(self):
+        subnet = factory.make_ipv6_network()
+        Config.objects.set_config('dns_trusted_acl', str(subnet))
+        expected = [str(subnet)]
+        self.assertEqual(expected, get_trusted_acls())
+
+    def test__returns_many_networks(self):
+        subnets = [
+            str(factory.make_ipv4_network()) for _ in range(
+                random.randint(1, 5))]
+        actual_subnets = ' '.join(subnets)
+        Config.objects.set_config('dns_trusted_acl', str(actual_subnets))
+        expected = [subnet for subnet in subnets]
+        # Note: This test was seen randomly failing because the networks were
+        # in an unexpected order...
+        self.assertItemsEqual(expected, get_trusted_acls())
+
+
 class TestGetTrustedNetworks(MAASServerTestCase):
     """Test for maasserver/dns/config.py:get_trusted_networks()"""
 
diff --git a/src/maasserver/fields.py b/src/maasserver/fields.py
index 2ff9a53..b139fa3 100644
--- a/src/maasserver/fields.py
+++ b/src/maasserver/fields.py
@@ -690,6 +690,70 @@ class HostListFormField(forms.CharField):
             return host
 
 
+class SubnetListFormField(forms.CharField):
+    """Accepts a space/comma separated list of hostnames, Subnets or IPs.
+
+    This field normalizes the list to a space-separated list.
+    """
+    separators = re.compile('[,\s]+')
+
+    # Regular expressions to sniff out things that look like IP addresses;
+    # additional and more robust validation ought to be done to make sure.
+    pt_ipv4 = r"(?: \d{1,3} [.] \d{1,3} [.] \d{1,3} [.] \d{1,3} )"
+    pt_ipv6 = r"(?: (|[0-9A-Fa-f]{1,4}) [:] (|[0-9A-Fa-f]{1,4}) [:] (.*))"
+    pt_ip = re.compile(
+        r"^ (?: %s | %s ) $" % (pt_ipv4, pt_ipv6), re.VERBOSE)
+    pt_subnet = re.compile(
+        r"^ (?: %s | %s ) \/\d+$" % (pt_ipv4, pt_ipv6), re.VERBOSE)
+
+    def clean(self, value):
+        if value is None:
+            return None
+        else:
+            values = map(str.strip, self.separators.split(value))
+            values = (value for value in values if len(value) != 0)
+            values = map(self._clean_addr_or_host, values)
+            return ' '.join(values)
+
+    def _clean_addr_or_host(self, value):
+        looks_like_ip = self.pt_ip.match(value) is not None
+        looks_like_subnet = self.pt_subnet.match(value) is not None
+        if looks_like_subnet:
+            return self._clean_subnet(value)
+        elif looks_like_ip:
+            return self._clean_addr(value)
+        else:
+            return self._clean_host(value)
+
+    def _clean_addr(self, value):
+        try:
+            addr = IPAddress(value)
+        except ValueError:
+            return
+        except AddrFormatError as error:
+            raise ValidationError(
+                "Invalid IP address: %s." % value)
+        else:
+            return str(addr)
+
+    def _clean_subnet(self, value):
+        try:
+            cidr = IPNetwork(value)
+        except AddrFormatError:
+            raise ValidationError(
+                "Invalid network: %s." % value)
+        else:
+            return str(cidr)
+
+    def _clean_host(self, host):
+        try:
+            validate_hostname(host)
+        except ValidationError as error:
+            raise ValidationError("Invalid hostname: " + error.message)
+        else:
+            return host
+
+
 class CaseInsensitiveChoiceField(forms.ChoiceField):
     """ChoiceField that allows the input to be case insensitive."""
 
diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py
index 6c20906..52939a3 100644
--- a/src/maasserver/forms/__init__.py
+++ b/src/maasserver/forms/__init__.py
@@ -1501,6 +1501,7 @@ class DNSForm(ConfigForm):
     """Settings page, DNS section."""
     upstream_dns = get_config_field('upstream_dns')
     dnssec_validation = get_config_field('dnssec_validation')
+    dns_trusted_acl = get_config_field('dns_trusted_acl')
 
 
 class NTPForm(ConfigForm):
diff --git a/src/maasserver/forms/settings.py b/src/maasserver/forms/settings.py
index 0420b33..a6ce778 100644
--- a/src/maasserver/forms/settings.py
+++ b/src/maasserver/forms/settings.py
@@ -21,6 +21,7 @@ from maasserver.bootresources import IMPORT_RESOURCES_SERVICE_PERIOD
 from maasserver.fields import (
     HostListFormField,
     IPListFormField,
+    SubnetListFormField,
 )
 from maasserver.models import BootResource
 from maasserver.models.config import (
@@ -357,6 +358,22 @@ CONFIG_ITEMS = {
                 "provided by the set upstream DNS.")
         }
     },
+    'dns_trusted_acl': {
+        'default': None,
+        'form': SubnetListFormField,
+        'form_kwargs': {
+            'label': (
+                "List of external networks (not previously known), that will "
+                "be allowed to use MAAS for DNS resolution."),
+            'required': False,
+            'help_text': (
+                "MAAS keeps a list of networks that are allowed to use MAAS "
+                "for DNS resolution. This option allows to add extra "
+                "networks (not previously known) to the trusted ACL where "
+                "this list of networks is kept. It also supports specifying "
+                "IPs or ACL names.")
+        }
+    },
     'ntp_servers': {
         'default': None,
         'form': HostListFormField,
diff --git a/src/maasserver/models/config.py b/src/maasserver/models/config.py
index 4c86449..914313f 100644
--- a/src/maasserver/models/config.py
+++ b/src/maasserver/models/config.py
@@ -78,6 +78,7 @@ def get_default_config():
         # DNS settings
         'upstream_dns': None,
         'dnssec_validation': "auto",
+        'dns_trusted_acl': None,
         'maas_internal_domain': 'maas-internal',
         # NTP settings
         'ntp_servers': 'ntp.ubuntu.com',
diff --git a/src/maasserver/tests/test_fields.py b/src/maasserver/tests/test_fields.py
index 10addd9..ffaab8e 100644
--- a/src/maasserver/tests/test_fields.py
+++ b/src/maasserver/tests/test_fields.py
@@ -29,6 +29,7 @@ from maasserver.fields import (
     MODEL_NAME_VALIDATOR,
     NodeChoiceField,
     register_mac_type,
+    SubnetListFormField,
     URLOrPPAFormField,
     URLOrPPAValidator,
     validate_mac,
@@ -705,6 +706,112 @@ class TestNodeChoiceField(MAASServerTestCase):
         self.assertEqual(node, node_field.clean(node.system_id))
 
 
+class TestSubnetListFormField(MAASTestCase):
+
+    def test_accepts_none(self):
+        self.assertIsNone(SubnetListFormField().clean(None))
+
+    def test_accepts_single_ip(self):
+        ip = factory.make_ip_address()
+        self.assertEqual(ip, SubnetListFormField().clean(ip))
+
+    def test_accepts_space_separated_ips(self):
+        ips = [factory.make_ip_address() for _ in range(5)]
+        input = ' '.join(ips)
+        self.assertEqual(input, SubnetListFormField().clean(input))
+
+    def test_accepts_comma_separated_ips(self):
+        ips = [factory.make_ip_address() for _ in range(5)]
+        input = ','.join(ips)
+        self.assertEqual(' '.join(ips), SubnetListFormField().clean(input))
+
+    def test_accepts_single_subnet(self):
+        subnet = str(factory.make_ipv4_network())
+        self.assertEqual(subnet, SubnetListFormField().clean(subnet))
+
+    def test_accepts_space_separated_subnets(self):
+        subnets = [str(factory.make_ipv6_network()) for _ in range(5)]
+        input = ' '.join(subnets)
+        self.assertEqual(input, SubnetListFormField().clean(input))
+
+    def test_accepts_comma_separated_subnets(self):
+        subnets = [str(factory.make_ipv4_network()) for _ in range(5)]
+        input = ','.join(subnets)
+        self.assertEqual(' '.join(subnets), SubnetListFormField().clean(input))
+
+    def test_separators_dont_conflict_with_ipv4_address(self):
+        self.assertIsNone(re.search(
+            SubnetListFormField.separators, factory.make_ipv4_address()))
+
+    def test_separators_dont_conflict_with_ipv6_address(self):
+        self.assertIsNone(re.search(
+            SubnetListFormField.separators, factory.make_ipv6_address()))
+
+    def test_accepts_hostname(self):
+        hostname = factory.make_hostname()
+        self.assertEqual(hostname, SubnetListFormField().clean(hostname))
+
+    def test_accepts_space_separated_hostnames(self):
+        hostnames = factory.make_hostname(), factory.make_hostname()
+        input = ' '.join(hostnames)
+        self.assertEqual(input, SubnetListFormField().clean(input))
+
+    def test_accepts_comma_separated_hostnames(self):
+        hostnames = factory.make_hostname(), factory.make_hostname()
+        input = ','.join(hostnames)
+        self.assertEqual(
+            ' '.join(hostnames), SubnetListFormField().clean(input))
+
+    def test_accepts_misc(self):
+        servers = {
+            "::1",
+            "1::",
+            "1::2",
+            "1:2::3",
+            "1::2:3",
+            "1:2::3:4",
+            "::127.0.0.1",
+        }
+        input = ','.join(servers)
+        self.assertEqual(' '.join(servers), SubnetListFormField().clean(input))
+
+    def test_rejects_invalid_ipv4_address(self):
+        input = "%s 12.34.56.999" % factory.make_hostname()
+        error = self.assertRaises(
+            ValidationError, SubnetListFormField().clean, input)
+        self.assertThat(error.message, Equals(
+            "Invalid IP address: 12.34.56.999."))
+
+    def test_rejects_invalid_ipv6_address(self):
+        input = "%s fe80::abcde" % factory.make_hostname()
+        error = self.assertRaises(
+            ValidationError, SubnetListFormField().clean, input)
+        self.assertThat(error.message, Equals(
+            "Invalid IP address: fe80::abcde."))
+
+    def test_rejects_invalid_ipv4_subnet(self):
+        input = "%s 10.10.10.300/24" % factory.make_ipv4_network()
+        error = self.assertRaises(
+            ValidationError, SubnetListFormField().clean, input)
+        self.assertThat(error.message, Equals(
+            "Invalid network: 10.10.10.300/24."))
+
+    def test_rejects_invalid_ipv6_subnet(self):
+        input = "%s 100::/300" % factory.make_ipv6_network()
+        error = self.assertRaises(
+            ValidationError, SubnetListFormField().clean, input)
+        self.assertThat(error.message, Equals(
+            "Invalid network: 100::/300."))
+
+    def test_rejects_invalid_hostname(self):
+        input = "%s abc-.foo" % factory.make_hostname()
+        error = self.assertRaises(
+            ValidationError, SubnetListFormField().clean, input)
+        self.assertThat(error.message, Equals(
+            "Invalid hostname: Label cannot start or end with "
+            "hyphen: 'abc-'."))
+
+
 class TestVersionedTextFileField(MAASServerTestCase):
 
     def test_creates_new(self):
diff --git a/src/maasserver/triggers/system.py b/src/maasserver/triggers/system.py
index d6df1d6..1db361c 100644
--- a/src/maasserver/triggers/system.py
+++ b/src/maasserver/triggers/system.py
@@ -1335,8 +1335,8 @@ DNS_INTERFACE_UPDATE = dedent("""\
 
 # Triggered when a config is inserted. Increments the zone serial and notifies
 # that DNS needs to be updated. Only watches for inserts on config
-# upstream_dns, dnssec_validation, default_dns_ttl, windows_kms_host, and
-# maas_internal_domain.
+# upstream_dns, dnssec_validation, default_dns_ttl, windows_kms_host,
+# dns_trusted_acls and maas_internal_domain.
 DNS_CONFIG_INSERT = dedent("""\
     CREATE OR REPLACE FUNCTION sys_dns_config_insert()
     RETURNS trigger as $$
@@ -1344,6 +1344,7 @@ DNS_CONFIG_INSERT = dedent("""\
       -- Only care about the
       IF (NEW.name = 'upstream_dns' OR
           NEW.name = 'dnssec_validation' OR
+          NEW.name = 'dns_trusted_acl' OR
           NEW.name = 'default_dns_ttl' OR
           NEW.name = 'windows_kms_host' OR
           NEW.name = 'maas_internal_domain')
@@ -1359,15 +1360,18 @@ DNS_CONFIG_INSERT = dedent("""\
 
 # Triggered when a config is updated. Increments the zone serial and notifies
 # that DNS needs to be updated. Only watches for updates on config
-# upstream_dns, dnssec_validation, default_dns_ttl, and windows_kms_host.
+# upstream_dns, dnssec_validation, dns_trusted_acl, default_dns_ttl,
+# and windows_kms_host.
 DNS_CONFIG_UPDATE = dedent("""\
     CREATE OR REPLACE FUNCTION sys_dns_config_update()
     RETURNS trigger as $$
     BEGIN
-      -- Only care about the
+      -- Only care about the upstream_dns, default_dns_ttl,
+      -- dns_trusted_acl and windows_kms_host.
       IF (OLD.value != NEW.value AND (
           NEW.name = 'upstream_dns' OR
           NEW.name = 'dnssec_validation' OR
+          NEW.name = 'dns_trusted_acl' OR
           NEW.name = 'default_dns_ttl' OR
           NEW.name = 'windows_kms_host' OR
           NEW.name = 'maas_internal_domain'))
diff --git a/src/maasserver/triggers/tests/test_system_listener.py b/src/maasserver/triggers/tests/test_system_listener.py
index d00dd83..072f082 100644
--- a/src/maasserver/triggers/tests/test_system_listener.py
+++ b/src/maasserver/triggers/tests/test_system_listener.py
@@ -4218,6 +4218,30 @@ class TestDNSConfigListener(
 
     @wait_for_reactor
     @inlineCallbacks
+    def test_sends_message_for_config_dns_trusted_acl_insert(self):
+        dns_trusted_acl_new = factory.make_name('internal')
+        yield deferToDatabase(register_system_triggers)
+        yield self.capturePublication()
+        dv = DeferredValue()
+        listener = self.make_listener_without_delay()
+        listener.register(
+            "sys_dns", lambda *args: dv.set(args))
+        yield listener.startService()
+        try:
+            yield deferToDatabase(
+                Config.objects.set_config,
+                "dns_trusted_acl", dns_trusted_acl_new)
+            yield dv.get(timeout=2)
+            yield self.assertPublicationUpdated()
+        finally:
+            yield listener.stopService()
+        self.assertThat(
+            self.getCapturedPublication().source, Equals(
+                "configuration dns_trusted_acl set to %s"
+                % json.dumps(dns_trusted_acl_new)))
+
+    @wait_for_reactor
+    @inlineCallbacks
     def test_sends_message_for_config_upstream_dns_update(self):
         upstream_dns_old = factory.make_ip_address()
         upstream_dns_new = factory.make_ip_address()
@@ -4326,6 +4350,34 @@ class TestDNSConfigListener(
                 "configuration maas_internal_domain changed to %s"
                 % (json.dumps(maas_internal_domain_new))))
 
+    @wait_for_reactor
+    @inlineCallbacks
+    def test_sends_message_for_config_dns_trusted_acl_update(self):
+        dns_trusted_acl_old = factory.make_name('internal')
+        dns_trusted_acl_new = factory.make_name('internal_new')
+        yield deferToDatabase(register_system_triggers)
+        yield deferToDatabase(
+            Config.objects.set_config,
+            "dns_trusted_acl", dns_trusted_acl_old)
+        yield self.capturePublication()
+        dv = DeferredValue()
+        listener = self.make_listener_without_delay()
+        listener.register(
+            "sys_dns", lambda *args: dv.set(args))
+        yield listener.startService()
+        try:
+            yield deferToDatabase(
+                Config.objects.set_config,
+                "dns_trusted_acl", dns_trusted_acl_new)
+            yield dv.get(timeout=2)
+            yield self.assertPublicationUpdated()
+        finally:
+            yield listener.stopService()
+        self.assertThat(
+            self.getCapturedPublication().source, Equals(
+                "configuration dns_trusted_acl changed to %s"
+                % (json.dumps(dns_trusted_acl_new))))
+
 
 class TestDNSConfigListenerLegacy(
         MAASLegacyTransactionServerTestCase, TransactionalHelpersMixin,
diff --git a/src/maasserver/views/tests/test_settings.py b/src/maasserver/views/tests/test_settings.py
index 392a84e..5670b18 100644
--- a/src/maasserver/views/tests/test_settings.py
+++ b/src/maasserver/views/tests/test_settings.py
@@ -180,7 +180,10 @@ class SettingsTest(MAASServerTestCase):
         self.addCleanup(bootsources.signals.enable)
         bootsources.signals.disable()
         self.client.login(user=factory.make_admin())
-        new_upstream = "8.8.8.8"
+        new_upstream = "8.8.8.8 8.8.4.4"
+        new_ipv4_subnet = factory.make_ipv4_network()
+        new_ipv6_subnet = factory.make_ipv6_network()
+        new_subnets = "%s %s" % (new_ipv4_subnet, new_ipv6_subnet)
         response = self.client.post(
             reverse('settings_network'),
             get_prefixed_form_data(
@@ -188,12 +191,15 @@ class SettingsTest(MAASServerTestCase):
                 data={
                     'upstream_dns': new_upstream,
                     'dnssec_validation': 'no',
+                    'dns_trusted_acl': new_subnets,
                 }))
         self.assertEqual(
             http.client.FOUND, response.status_code, response.content)
         self.assertEqual(
             new_upstream, Config.objects.get_config('upstream_dns'))
         self.assertEqual('no', Config.objects.get_config('dnssec_validation'))
+        self.assertEqual(
+            new_subnets, Config.objects.get_config('dns_trusted_acl'))
 
     def test_settings_ntp_POST(self):
         # Disable boot source cache signals.
diff --git a/src/provisioningserver/dns/actions.py b/src/provisioningserver/dns/actions.py
index 97ad042..a955023 100644
--- a/src/provisioningserver/dns/actions.py
+++ b/src/provisioningserver/dns/actions.py
@@ -123,6 +123,7 @@ def bind_write_options(upstream_dns, dnssec_validation):
     """Write BIND options.
 
     :param upstream_dns: A sequence of upstream DNS servers.
+    :param dnssec_validation: Whether to enable DNSSec.
     """
     # upstream_dns was formerly specified as a single IP address. These
     # assertions are here to prevent code that assumes that slipping through.