Merge lp:~gmb/maas/restrict-dynamic-range-to-slash-16-bug-1382190 into lp:~maas-committers/maas/trunk

[Graham Binns]

Restrict the dynamic range on node group interfaces to /16 or smaller for IPv4 networks. Allow any size of dynamic range for IPv6 networks.

This is to allow us to generate DNS mappings for all the addresses in the v4 networks. We need to restrict the size of the network so as to keep the size of the zone files reasonable.

[Julian Edwards (julian-edwards)]

Potential problem highlighted below.

review: Needs Information

[Graham Binns (gmb)]

On 30 October 2014 02:35, Julian Edwards <email address hidden> wrote:
>> + if ip_range.size > 256 ** 2:
>> + raise ValidationError(ERROR_MESSAGE_DYNAMIC_RANGE_TOO_LARGE)
>> +
>
> This check assumes that the IP values will be on exact boundaries. You could straddle more than a /16 and it won't notice, even though the IP range is under 65536 addresses.

How? I don't see the assumption. For example:

In [12]: netaddr.IPRange('10.0.0.55', '10.1.0.55').size > 256 ** 2
Out[12]: True

Am I missing something?

> I am not familiar with your GENERATE branch so I don't know if this is going to break it or not, but I'm raising the point for your consideration.

Well, if you tried to put an oversize network through it, you'll get
an AssertionError. Actually, I think that should change so that
creating the GENERATEs becomes a no-op in that case (otherwise, how do
we handle networks > /16 that people already have — say if they've
been testing with the 1.7 betas), and I'll take care of that as a
drive-by in this branch.

[Graham Binns (gmb)]

On 30 October 2014 02:35, Julian Edwards <email address hidden> wrote:
> Review: Needs Information
>
> Potential problem highlighted below.
>
> Diff comments:
>
>> === modified file 'src/maasserver/forms.py'
>> --- src/maasserver/forms.py 2014-10-22 19:49:03 +0000
>> +++ src/maasserver/forms.py 2014-10-29 21:06:23 +0000
>> @@ -46,6 +46,7 @@
>> "ZoneForm",
>> ]
>>
>> +from netaddr import IPRange
>> import collections
>> import json
>> import pipes
>> @@ -1305,6 +1306,36 @@
>> "in that range.")
>>
>>
>> +ERROR_MESSAGE_DYNAMIC_RANGE_TOO_LARGE = (
>> + "Dynamic address range cannot be larger than a /16 (65536 addresses) ")
>> +
>> +
>> +def validate_new_dynamic_range_size(instance, ip_range_low, ip_range_high):
>> + """Check that a ip address range is of a manageable size.
>> +
>> + :raises ValidationError: If the ip range is larger than a /16
>> + network.
>> + """
>> + # Return early if the instance is not already managed, it currently
>> + # has no static IP range, or the static IP range hasn't changed.
>
> s/static/dynamic/

Gah! Fixed.

>> +
>> def test_calls_get_duplicate_fqdns_when_appropriate(self):
>> # Check for duplicate FQDNs if the NodeGroupInterface has a
>
> You need a happy-path test for an IPv4 range as well, I think.

You're right, I do. Added.

[Julian Edwards (julian-edwards)]

Approved as discussed on the hangout.

review: Approve

[Julian Edwards (julian-edwards)]

On Thursday 30 Oct 2014 07:46:26 you wrote:
> On 30 October 2014 02:35, Julian Edwards <email address hidden>
wrote:
> >> + if ip_range.size > 256 ** 2:
> >> + raise ValidationError(ERROR_MESSAGE_DYNAMIC_RANGE_TOO_LARGE)
> >> +
> >
> > This check assumes that the IP values will be on exact boundaries. You
> > could straddle more than a /16 and it won't notice, even though the IP
> > range is under 65536 addresses.
> How? I don't see the assumption. For example:
>
> In [12]: netaddr.IPRange('10.0.0.55', '10.1.0.55').size > 256 ** 2
> Out[12]: True
>
> Am I missing something?

Yes :)

>
> > I am not familiar with your GENERATE branch so I don't know if this is
> > going to break it or not, but I'm raising the point for your
> > consideration.
> Well, if you tried to put an oversize network through it, you'll get
> an AssertionError. Actually, I think that should change so that
> creating the GENERATEs becomes a no-op in that case (otherwise, how do
> we handle networks > /16 that people already have — say if they've
> been testing with the 1.7 betas), and I'll take care of that as a
> drive-by in this branch.

<gmb> bigjools: So, you found a nice, interesting edge case… The GENERATE
stuff expects everythign to be in the same /16, so we need to enforce that
(fixing it is horrid, and if users want a dynamic range to span two /16s then
fuck ‘em, frankly). It doesn’t blow up… it but for 10.0.0.55 - 10.1.0.54, say,
you’ll get GENERATEs for 10.0.0.0 - 10.0.255.55. Which is surprising.

[MAAS Lander (maas-lander)]

The attempt to merge lp:~gmb/maas/restrict-dynamic-range-to-slash-16-bug-1382190 into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:2 http://security.ubuntu.com trusty-security Release [59.7 kB]
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [59.7 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Get:5 http://security.ubuntu.com trusty-security/main Sources [48.3 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:6 http://security.ubuntu.com trusty-security/universe Sources [11.2 kB]
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [151 kB]
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [50.4 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [134 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [89.3 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [352 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [216 kB]
Get:13 http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en [159 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Fetched 1,332 kB in 2s (458 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools gjs ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make pep8 postgresql pyflakes python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mimeparse python-mock python-netaddr python-netifaces python-nose python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-...

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2014-10-22 19:49:03 +0000
+++ src/maasserver/forms.py	2014-10-30 11:08:14 +0000
@@ -152,6 +152,8 @@
 from metadataserver.models import CommissioningScript
 from netaddr import (
     IPAddress,
+    IPNetwork,
+    IPRange,
     valid_ipv6,
     )
 from provisioningserver.logger import get_maas_logger
@@ -160,7 +162,10 @@
     NoConnectionsAvailable,
     NoSuchOperatingSystem,
     )
-from provisioningserver.utils.network import make_network
+from provisioningserver.utils.network import (
+    ip_range_within_network,
+    make_network,
+    )
 
 
 maaslog = get_maas_logger()
@@ -1305,6 +1310,39 @@
     "in that range.")
 
 
+ERROR_MESSAGE_DYNAMIC_RANGE_SPANS_SLASH_16S = (
+    "All addresses in the dynamic range must be within the same /16 "
+    "network.")
+
+
+def validate_new_dynamic_range_size(instance, ip_range_low, ip_range_high):
+    """Check that a ip address range is of a manageable size.
+
+    :raises ValidationError: If the ip range is larger than a /16
+        network.
+    """
+    # Return early if the instance is not already managed, its dynamic
+    # IP range hasn't changed, or the new values are blank.
+    if not instance.is_managed:
+        return True
+    # Deliberately vague check to allow for empty strings.
+    if (not ip_range_low and not ip_range_high):
+        return True
+    if (ip_range_low == instance.ip_range_low and
+       ip_range_high == instance.ip_range_high):
+        return True
+    ip_range = IPRange(ip_range_low, ip_range_high)
+
+    # Allow any size of dynamic range for v6 networks, but limit v4
+    # networks to /16s.
+    if ip_range.version == 6:
+        return True
+
+    slash_16_network = IPNetwork("%s/16" % IPAddress(ip_range.first))
+    if not ip_range_within_network(ip_range, slash_16_network):
+        raise ValidationError(ERROR_MESSAGE_DYNAMIC_RANGE_SPANS_SLASH_16S)
+
+
 def validate_new_static_ip_ranges(instance, static_ip_range_low,
                                   static_ip_range_high):
     """Check that new static IP ranges don't exclude allocated addresses.
@@ -1552,6 +1590,15 @@
                 netmask = 'ffff:ffff:ffff:ffff::'
             cleaned_data['subnet_mask'] = unicode(netmask)
 
+        dynamic_range_low = cleaned_data.get('ip_range_low')
+        dynamic_range_high = cleaned_data.get('ip_range_high')
+        try:
+            validate_new_dynamic_range_size(
+                self.instance, dynamic_range_low, dynamic_range_high)
+        except forms.ValidationError as exception:
+            set_form_error(self, 'ip_range_low', exception.message)
+            set_form_error(self, 'ip_range_high', exception.message)
+
         static_ip_range_low = cleaned_data.get('static_ip_range_low')
         static_ip_range_high = cleaned_data.get('static_ip_range_high')
         try:

=== modified file 'src/maasserver/tests/test_forms_nodegroupinterface.py'
--- src/maasserver/tests/test_forms_nodegroupinterface.py	2014-10-10 04:55:03 +0000
+++ src/maasserver/tests/test_forms_nodegroupinterface.py	2014-10-30 11:08:14 +0000
@@ -21,6 +21,7 @@
     NODEGROUPINTERFACE_MANAGEMENT,
     )
 from maasserver.forms import (
+    ERROR_MESSAGE_DYNAMIC_RANGE_SPANS_SLASH_16S,
     ERROR_MESSAGE_STATIC_RANGE_IN_USE,
     NodeGroupInterfaceForm,
     )
@@ -221,6 +222,63 @@
             [ERROR_MESSAGE_STATIC_RANGE_IN_USE],
             form._errors['static_ip_range_high'])
 
+    def test_rejects_ipv4_dynamic_ranges_across_multiple_slash_16s(self):
+        # Even if a dynamic range is < 65536 addresses, it can't cross
+        # two /16 networks.
+        network = IPNetwork("10.1.0.0/8")
+        nodegroup = factory.make_NodeGroup(
+            status=NODEGROUP_STATUS.ACCEPTED,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS,
+            network=network, static_ip_range_low=None,
+            static_ip_range_high=None)
+        [interface] = nodegroup.get_managed_interfaces()
+        form = NodeGroupInterfaceForm(
+            data={
+                'ip_range_low': '10.1.255.255',
+                'ip_range_high': '10.2.0.1',
+                },
+            instance=interface)
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            [ERROR_MESSAGE_DYNAMIC_RANGE_SPANS_SLASH_16S],
+            form._errors['ip_range_low'])
+        self.assertEqual(
+            [ERROR_MESSAGE_DYNAMIC_RANGE_SPANS_SLASH_16S],
+            form._errors['ip_range_low'])
+
+    def test_allows_sane_ipv4_dynamic_range_size(self):
+        network = IPNetwork("10.1.0.0/8")
+        nodegroup = factory.make_NodeGroup(
+            status=NODEGROUP_STATUS.ACCEPTED,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS,
+            network=network, static_ip_range_low=None,
+            static_ip_range_high=None)
+        [interface] = nodegroup.get_managed_interfaces()
+        form = NodeGroupInterfaceForm(
+            data={
+                'ip_range_low': '10.0.0.1',
+                'ip_range_high': '10.0.1.255',
+                },
+            instance=interface)
+        self.assertTrue(form.is_valid())
+
+    def test_allows_any_size_ipv6_dynamic_range(self):
+        network = factory.make_ipv6_network(slash=64)
+        nodegroup = factory.make_NodeGroup(
+            status=NODEGROUP_STATUS.ACCEPTED,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS,
+            network=network)
+        [interface] = nodegroup.get_managed_interfaces()
+        form = NodeGroupInterfaceForm(
+            data={
+                'ip_range_low': IPAddress(network.first).format(),
+                'ip_range_high': IPAddress(network.last).format(),
+                'static_ip_range_low': '',
+                'static_ip_range_high': '',
+                },
+            instance=interface)
+        self.assertTrue(form.is_valid())
+
     def test_calls_get_duplicate_fqdns_when_appropriate(self):
         # Check for duplicate FQDNs if the NodeGroupInterface has a
         # NodeGroup and is managing DNS.

=== modified file 'src/provisioningserver/dns/tests/test_zoneconfig.py'
--- src/provisioningserver/dns/tests/test_zoneconfig.py	2014-10-29 19:30:18 +0000
+++ src/provisioningserver/dns/tests/test_zoneconfig.py	2014-10-30 11:08:14 +0000
@@ -37,7 +37,6 @@
 from provisioningserver.dns.zoneconfig import (
     DNSForwardZoneConfig,
     DNSReverseZoneConfig,
-    intersect_iprange,
     )
 from testtools.matchers import (
     Contains,
@@ -443,9 +442,9 @@
         # The other tests in this TestCase rely on
         # get_expected_generate_directives(), which is quite dense. Here
         # we test get_GENERATE_directives() explicitly.
-        ip_range = IPRange('192.168.0.1', '192.168.2.128')
+        ip_range = IPRange('192.168.0.55', '192.168.2.128')
         expected_directives = [
-            ("1-255", "$.0.168.192.in-addr.arpa.", "192-168-0-$.domain."),
+            ("55-255", "$.0.168.192.in-addr.arpa.", "192-168-0-$.domain."),
             ("0-255", "$.1.168.192.in-addr.arpa.", "192-168-1-$.domain."),
             ("0-128", "$.2.168.192.in-addr.arpa.", "192-168-2-$.domain."),
             ]
@@ -545,15 +544,21 @@
         self.expectThat(directives, HasLength(2))
         self.assertItemsEqual(expected_generate_directives, directives)
 
-    def test_rejects_network_larger_than_slash_16(self):
+    def test_ignores_network_larger_than_slash_16(self):
         network = IPNetwork("%s/15" % factory.make_ipv4_address())
-        error = self.assertRaises(
-            AssertionError, DNSReverseZoneConfig.get_GENERATE_directives,
-            network, None)
         self.assertEqual(
-            "Cannot generate reverse zone mapping for any network larger "
-            "than /16.",
-            unicode(error))
+            [],
+            DNSReverseZoneConfig.get_GENERATE_directives(
+                network, factory.make_string()))
+
+    def test_ignores_networks_that_span_slash_16s(self):
+        # If the upper and lower bounds of a range span two /16 networks
+        # (but contain between them no more than 65536 addresses),
+        # get_GENERATE_directives() will return early
+        ip_range = IPRange('10.0.0.55', '10.1.0.54')
+        directives = DNSReverseZoneConfig.get_GENERATE_directives(
+            ip_range, factory.make_string())
+        self.assertEqual([], directives)
 
     def test_sorts_output_by_hostname(self):
         network = IPNetwork("10.0.0.1/23")
@@ -579,9 +584,9 @@
         # The other tests in this TestCase rely on
         # get_expected_generate_directives(), which is quite dense. Here
         # we test get_GENERATE_directives() explicitly.
-        ip_range = IPRange('192.168.0.1', '192.168.2.128')
+        ip_range = IPRange('192.168.0.55', '192.168.2.128')
         expected_directives = [
-            ("1-255", "192-168-0-$", "192.168.0.$"),
+            ("55-255", "192-168-0-$", "192.168.0.$"),
             ("0-255", "192-168-1-$", "192.168.1.$"),
             ("0-128", "192-168-2-$", "192.168.2.$"),
             ]
@@ -665,15 +670,20 @@
                 network)
             self.assertIsEqual(network.size / 256, len(directives))
 
-    def test_rejects_network_larger_than_slash_16(self):
+    def test_ignores_network_larger_than_slash_16(self):
         network = IPNetwork("%s/15" % factory.make_ipv4_address())
-        error = self.assertRaises(
-            AssertionError, DNSForwardZoneConfig.get_GENERATE_directives,
-            network)
         self.assertEqual(
-            "Cannot generate reverse zone mapping for any network larger "
-            "than /16.",
-            unicode(error))
+            [],
+            DNSForwardZoneConfig.get_GENERATE_directives(network))
+
+    def test_ignores_networks_that_span_slash_16s(self):
+        # If the upper and lower bounds of a range span two /16 networks
+        # (but contain between them no more than 65536 addresses),
+        # get_GENERATE_directives() will return early
+        ip_range = IPRange('10.0.0.55', '10.1.0.54')
+        directives = DNSForwardZoneConfig.get_GENERATE_directives(
+            ip_range)
+        self.assertEqual([], directives)
 
     def test_sorts_output(self):
         network = IPNetwork("10.0.0.0/23")
@@ -690,30 +700,3 @@
         self.expectThat(
             directives[1], Equals(
                 ("0-255", expected_hostname % "1", expected_address % "1")))
-
-
-class TestIntersectIPRange(MAASTestCase):
-    """Tests for `intersect_iprange()`."""
-
-    def test_finds_intersection_between_two_ranges(self):
-        range_1 = IPRange('10.0.0.1', '10.0.0.255')
-        range_2 = IPRange('10.0.0.128', '10.0.0.200')
-        intersect = intersect_iprange(range_1, range_2)
-        self.expectThat(
-            IPAddress(intersect.first), Equals(IPAddress('10.0.0.128')))
-        self.expectThat(
-            IPAddress(intersect.last), Equals(IPAddress('10.0.0.200')))
-
-    def test_ignores_non_intersecting_ranges(self):
-        range_1 = IPRange('10.0.0.1', '10.0.0.255')
-        range_2 = IPRange('10.0.1.128', '10.0.1.200')
-        self.assertIsNone(intersect_iprange(range_1, range_2))
-
-    def test_finds_partial_intersection(self):
-        range_1 = IPRange('10.0.0.1', '10.0.0.128')
-        range_2 = IPRange('10.0.0.64', '10.0.0.200')
-        intersect = intersect_iprange(range_1, range_2)
-        self.expectThat(
-            IPAddress(intersect.first), Equals(IPAddress('10.0.0.64')))
-        self.expectThat(
-            IPAddress(intersect.last), Equals(IPAddress('10.0.0.128')))

=== modified file 'src/provisioningserver/dns/zoneconfig.py'
--- src/provisioningserver/dns/zoneconfig.py	2014-10-29 19:26:07 +0000
+++ src/provisioningserver/dns/zoneconfig.py	2014-10-30 11:08:14 +0000
@@ -25,7 +25,7 @@
 
 from netaddr import (
     IPAddress,
-    IPRange,
+    IPNetwork,
     spanning_cidr,
     )
 from netaddr.core import AddrFormatError
@@ -35,19 +35,10 @@
     report_missing_config_dir,
     )
 from provisioningserver.utils.fs import incremental_write
-
-
-def intersect_iprange(network, iprange):
-    """Return the intersection between two IPNetworks or IPRanges.
-
-    IPSet is notoriously inefficient so we intersect ourselves here.
-    """
-    if network.last >= iprange.first and network.first <= iprange.last:
-        first = max(network.first, iprange.first)
-        last = min(network.last, iprange.last)
-        return IPRange(first, last)
-    else:
-        return None
+from provisioningserver.utils.network import (
+    intersect_iprange,
+    ip_range_within_network,
+    )
 
 
 def get_fqdn_or_ip_address(target):
@@ -255,9 +246,13 @@
     def get_GENERATE_directives(cls, dynamic_range):
         """Return the GENERATE directives for the forward zone of a network.
         """
-        assert dynamic_range.size <= 256 ** 2, (
-            "Cannot generate reverse zone mapping for any network larger than "
-            "/16.")
+        slash_16 = IPNetwork("%s/16" % IPAddress(dynamic_range.first))
+        if (dynamic_range.size > 256 ** 2 or
+           not ip_range_within_network(dynamic_range, slash_16)):
+            # We can't issue a sane set of $GENERATEs for any network
+            # larger than a /16, or for one that spans two /16s, so we
+            # don't try.
+            return []
 
         generate_directives = set()
         subnets, prefix, _ = get_details_for_ip_range(dynamic_range)
@@ -387,9 +382,13 @@
     @classmethod
     def get_GENERATE_directives(cls, dynamic_range, domain):
         """Return the GENERATE directives for the reverse zone of a network."""
-        assert dynamic_range.size <= 256 ** 2, (
-            "Cannot generate reverse zone mapping for any network larger than "
-            "/16.")
+        slash_16 = IPNetwork("%s/16" % IPAddress(dynamic_range.first))
+        if (dynamic_range.size > 256 ** 2 or
+           not ip_range_within_network(dynamic_range, slash_16)):
+            # We can't issue a sane set of $GENERATEs for any network
+            # larger than a /16, or for one that spans two /16s, so we
+            # don't try.
+            return []
 
         generate_directives = set()
         subnets, prefix, rdns_suffix = get_details_for_ip_range(dynamic_range)

=== modified file 'src/provisioningserver/utils/network.py'
--- src/provisioningserver/utils/network.py	2014-09-10 16:20:31 +0000
+++ src/provisioningserver/utils/network.py	2014-10-30 11:08:14 +0000
@@ -20,6 +20,8 @@
     'get_all_interface_addresses',
     'make_network',
     'resolve_hostname',
+    'intersect_iprange',
+    'ip_range_within_network',
     ]
 
 
@@ -35,6 +37,7 @@
 from netaddr import (
     IPAddress,
     IPNetwork,
+    IPRange,
     )
 import netifaces
 from provisioningserver.utils.shell import call_and_check
@@ -196,3 +199,27 @@
         IPAddress(sockaddr[0])
         for family, socktype, proto, canonname, sockaddr in address_info
         }
+
+
+def intersect_iprange(network, iprange):
+    """Return the intersection between two IPNetworks or IPRanges.
+
+    IPSet is notoriously inefficient so we intersect ourselves here.
+    """
+    if network.last >= iprange.first and network.first <= iprange.last:
+        first = max(network.first, iprange.first)
+        last = min(network.last, iprange.last)
+        return IPRange(first, last)
+    else:
+        return None
+
+
+def ip_range_within_network(ip_range, network):
+    """Check that the whole of a given IP range is within a given network."""
+    # Make sure that ip_range is an IPRange and not an IPNetwork,
+    # otherwise this won't work.
+    if isinstance(ip_range, IPNetwork):
+        ip_range = IPRange(
+            IPAddress(network.first), IPAddress(network.last))
+    return all([
+        intersect_iprange(cidr, network) for cidr in ip_range.cidrs()])

=== modified file 'src/provisioningserver/utils/tests/test_network.py'
--- src/provisioningserver/utils/tests/test_network.py	2014-09-19 10:10:49 +0000
+++ src/provisioningserver/utils/tests/test_network.py	2014-10-30 11:08:14 +0000
@@ -28,6 +28,7 @@
 from netaddr import (
     IPAddress,
     IPNetwork,
+    IPRange,
     )
 import netifaces
 from netifaces import (
@@ -43,9 +44,12 @@
     find_mac_via_arp,
     get_all_addresses_for_interface,
     get_all_interface_addresses,
+    intersect_iprange,
+    ip_range_within_network,
     make_network,
     resolve_hostname,
     )
+from testtools.matchers import Equals
 
 
 class TestMakeNetwork(MAASTestCase):
@@ -433,3 +437,53 @@
         self.assertRaises(
             KeyError,
             resolve_hostname, factory.make_hostname(), 4)
+
+
+class TestIntersectIPRange(MAASTestCase):
+    """Tests for `intersect_iprange()`."""
+
+    def test_finds_intersection_between_two_ranges(self):
+        range_1 = IPRange('10.0.0.1', '10.0.0.255')
+        range_2 = IPRange('10.0.0.128', '10.0.0.200')
+        intersect = intersect_iprange(range_1, range_2)
+        self.expectThat(
+            IPAddress(intersect.first), Equals(IPAddress('10.0.0.128')))
+        self.expectThat(
+            IPAddress(intersect.last), Equals(IPAddress('10.0.0.200')))
+
+    def test_ignores_non_intersecting_ranges(self):
+        range_1 = IPRange('10.0.0.1', '10.0.0.255')
+        range_2 = IPRange('10.0.1.128', '10.0.1.200')
+        self.assertIsNone(intersect_iprange(range_1, range_2))
+
+    def test_finds_partial_intersection(self):
+        range_1 = IPRange('10.0.0.1', '10.0.0.128')
+        range_2 = IPRange('10.0.0.64', '10.0.0.200')
+        intersect = intersect_iprange(range_1, range_2)
+        self.expectThat(
+            IPAddress(intersect.first), Equals(IPAddress('10.0.0.64')))
+        self.expectThat(
+            IPAddress(intersect.last), Equals(IPAddress('10.0.0.128')))
+
+
+class TestIPRangeWithinNetwork(MAASTestCase):
+
+    def test_returns_true_when_ip_range_is_within_network(self):
+        ip_range = IPRange('10.0.0.55', '10.0.255.55')
+        ip_network = IPNetwork('10.0.0.0/16')
+        self.assertTrue(ip_range_within_network(ip_range, ip_network))
+
+    def test_returns_false_when_ip_range_is_within_network(self):
+        ip_range = IPRange('192.0.0.55', '192.0.255.55')
+        ip_network = IPNetwork('10.0.0.0/16')
+        self.assertFalse(ip_range_within_network(ip_range, ip_network))
+
+    def test_returns_false_when_ip_range_is_partially_within_network(self):
+        ip_range = IPRange('10.0.0.55', '10.1.0.55')
+        ip_network = IPNetwork('10.0.0.0/16')
+        self.assertFalse(ip_range_within_network(ip_range, ip_network))
+
+    def test_works_with_two_ip_networks(self):
+        network_1 = IPNetwork('10.0.0.0/16')
+        network_2 = IPNetwork('10.0.0.0/24')
+        self.assertTrue(ip_range_within_network(network_2, network_1))