From 0e44b7b5418c400af7358a5391f350f2f737929e Mon Sep 17 00:00:00 2001 From: Assaf Muller Date: Wed, 7 May 2014 18:05:42 +0300 Subject: [PATCH] Make sure that gateway is in CIDR range by default Git commit c3706fa2 introduced the force_gateway_on_subnet option that verified that the defined gateway is in the CIDR range of a newly created or updated subnet. However, the default value was False for backwards compatability reasons. The default will change to True and the option will be marked as deprecated. For IPv6, the gateway must be in the CIDR only if the gateway is not a link local address. DocImpact Change-Id: I04fd1caec6da5dceee3f736b3f91f2468150ba2a Closes-Bug: #1304181 --- etc/neutron.conf | 6 ++- neutron/common/config.py | 8 ++- neutron/db/db_base_plugin_v2.py | 24 ++++++--- neutron/tests/unit/ml2/test_ml2_plugin.py | 6 +-- neutron/tests/unit/nuage/test_nuage_plugin.py | 8 ++- .../unit/oneconvergence/test_nvsd_plugin.py | 6 +++ neutron/tests/unit/test_db_plugin.py | 50 +++++++++++-------- neutron/tests/unit/test_l3_plugin.py | 2 - 8 files changed, 72 insertions(+), 38 deletions(-) diff --git a/etc/neutron.conf b/etc/neutron.conf index ec77905e165..456d52c8977 100644 --- a/etc/neutron.conf +++ b/etc/neutron.conf @@ -97,8 +97,10 @@ lock_path = $state_path/lock # Attention: the following parameter MUST be set to False if Neutron is # being used in conjunction with nova security groups # allow_overlapping_ips = False -# Ensure that configured gateway is on subnet -# force_gateway_on_subnet = False +# Ensure that configured gateway is on subnet. For IPv6, validate only if +# gateway is not a link local address. Deprecated, to be removed during the +# K release, at which point the check will be mandatory. +# force_gateway_on_subnet = True # Default maximum number of items returned in a single response, # value == infinite and value < 0 means no max limit, and value must diff --git a/neutron/common/config.py b/neutron/common/config.py index 0a8232fa02b..78e6a9f54d9 100644 --- a/neutron/common/config.py +++ b/neutron/common/config.py @@ -80,8 +80,12 @@ core_opts = [ help=_("Allow overlapping IP support in Neutron")), cfg.StrOpt('host', default=utils.get_hostname(), help=_("The hostname Neutron is running on")), - cfg.BoolOpt('force_gateway_on_subnet', default=False, - help=_("Ensure that configured gateway is on subnet")), + cfg.BoolOpt('force_gateway_on_subnet', default=True, + help=_("Ensure that configured gateway is on subnet. " + "For IPv6, validate only if gateway is not a link " + "local address. Deprecated, to be removed during the " + "K release, at which point the check will be " + "mandatory.")), cfg.BoolOpt('notify_nova_on_port_status_changes', default=True, help=_("Send notification to nova when port status changes")), cfg.BoolOpt('notify_nova_on_port_data_changes', default=True, diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 4d804f559d8..6ad0f27e6c9 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -491,8 +491,16 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, return True return False - @staticmethod - def _check_subnet_ip(cidr, ip_address): + @classmethod + def _check_gateway_in_subnet(cls, cidr, gateway): + """Validate that the gateway is on the subnet.""" + ip = netaddr.IPAddress(gateway) + if ip.version == 4 or (ip.version == 6 and not ip.is_link_local()): + return cls._check_subnet_ip(cidr, gateway) + return True + + @classmethod + def _check_subnet_ip(cls, cidr, ip_address): """Validate that the IP address is on the subnet.""" ip = netaddr.IPAddress(ip_address) net = netaddr.IPNetwork(cidr) @@ -549,8 +557,8 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, filter = {'network_id': [network_id]} subnets = self.get_subnets(context, filters=filter) for subnet in subnets: - if NeutronDbPluginV2._check_subnet_ip(subnet['cidr'], - fixed['ip_address']): + if self._check_subnet_ip(subnet['cidr'], + fixed['ip_address']): found = True subnet_id = subnet['id'] break @@ -579,8 +587,8 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, # Ensure that the IP is valid on the subnet if (not found and - not NeutronDbPluginV2._check_subnet_ip( - subnet['cidr'], fixed['ip_address'])): + not self._check_subnet_ip(subnet['cidr'], + fixed['ip_address'])): msg = _('IP address %s is not a valid IP for the defined ' 'subnet') % fixed['ip_address'] raise n_exc.InvalidInput(error_message=msg) @@ -1099,8 +1107,8 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, if attributes.is_attr_set(s.get('gateway_ip')): self._validate_ip_version(ip_ver, s['gateway_ip'], 'gateway_ip') if (cfg.CONF.force_gateway_on_subnet and - not NeutronDbPluginV2._check_subnet_ip(s['cidr'], - s['gateway_ip'])): + not self._check_gateway_in_subnet( + s['cidr'], s['gateway_ip'])): error_message = _("Gateway is not valid on subnet") raise n_exc.InvalidInput(error_message=error_message) # Ensure the gateway IP is not assigned to any port diff --git a/neutron/tests/unit/ml2/test_ml2_plugin.py b/neutron/tests/unit/ml2/test_ml2_plugin.py index 9bab5382c8b..9dd5a800b7d 100644 --- a/neutron/tests/unit/ml2/test_ml2_plugin.py +++ b/neutron/tests/unit/ml2/test_ml2_plugin.py @@ -461,7 +461,7 @@ class TestFaultyMechansimDriver(Ml2PluginV2FaultyDriverTestCase): 'name': 'subnet1', 'tenant_id': network['network']['tenant_id'], - 'gateway_ip': '10.0.2.1'}} + 'gateway_ip': '10.0.20.1'}} req = self.new_create_request('subnets', data) res = req.get_response(self.api) self.assertEqual(500, res.status_int) @@ -488,7 +488,7 @@ class TestFaultyMechansimDriver(Ml2PluginV2FaultyDriverTestCase): 'name': 'subnet1', 'tenant_id': network['network']['tenant_id'], - 'gateway_ip': '10.0.2.1'}} + 'gateway_ip': '10.0.20.1'}} subnet_req = self.new_create_request('subnets', data) subnet_res = subnet_req.get_response(self.api) self.assertEqual(201, subnet_res.status_int) @@ -519,7 +519,7 @@ class TestFaultyMechansimDriver(Ml2PluginV2FaultyDriverTestCase): 'name': 'subnet1', 'tenant_id': network['network']['tenant_id'], - 'gateway_ip': '10.0.2.1'}} + 'gateway_ip': '10.0.20.1'}} subnet_req = self.new_create_request('subnets', data) subnet_res = subnet_req.get_response(self.api) self.assertEqual(201, subnet_res.status_int) diff --git a/neutron/tests/unit/nuage/test_nuage_plugin.py b/neutron/tests/unit/nuage/test_nuage_plugin.py index afbc91bb9a6..426122912ec 100644 --- a/neutron/tests/unit/nuage/test_nuage_plugin.py +++ b/neutron/tests/unit/nuage/test_nuage_plugin.py @@ -216,6 +216,10 @@ class TestNuageSubnetsV2(NuagePluginV2TestCase, self.skipTest("Plugin does not support " "Neutron Subnet no-gateway option") + def test_create_subnet_nonzero_cidr(self): + self.skipTest("Plugin does not support " + "Neutron Subnet no-gateway option") + def test_create_subnet_with_none_gateway_fully_allocated(self): self.skipTest("Plugin does not support Neutron " "Subnet no-gateway option") @@ -235,7 +239,9 @@ class TestNuagePluginPortBinding(NuagePluginV2TestCase, class TestNuagePortsV2(NuagePluginV2TestCase, test_db_plugin.TestPortsV2): - pass + def test_no_more_port_exception(self): + self.skipTest("Plugin does not support " + "Neutron Subnet no-gateway option") class TestNuageL3NatTestCase(NuagePluginV2TestCase, diff --git a/neutron/tests/unit/oneconvergence/test_nvsd_plugin.py b/neutron/tests/unit/oneconvergence/test_nvsd_plugin.py index 564b8c56a81..d186f1a4d0c 100644 --- a/neutron/tests/unit/oneconvergence/test_nvsd_plugin.py +++ b/neutron/tests/unit/oneconvergence/test_nvsd_plugin.py @@ -87,6 +87,12 @@ class TestOneConvergencePluginSubnetsV2(test_plugin.TestSubnetsV2, def test_update_subnet_ipv6_inconsistent_address_attribute(self): self.skipTest("NVSD Plugin does not support IPV6.") + def test_create_subnet_ipv6_out_of_cidr_global(self): + self.skipTest("NVSD Plugin does not support IPV6.") + + def test_create_subnet_ipv6_out_of_cidr_lla(self): + self.skipTest("NVSD Plugin does not support IPV6.") + class TestOneConvergencePluginPortsV2(test_plugin.TestPortsV2, test_bindings.PortBindingsTestCase, diff --git a/neutron/tests/unit/test_db_plugin.py b/neutron/tests/unit/test_db_plugin.py index c196b8afbd0..50d73ff4137 100644 --- a/neutron/tests/unit/test_db_plugin.py +++ b/neutron/tests/unit/test_db_plugin.py @@ -1144,7 +1144,7 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s data['port']['fixed_ips']) def test_no_more_port_exception(self): - with self.subnet(cidr='10.0.0.0/32') as subnet: + with self.subnet(cidr='10.0.0.0/32', gateway_ip=None) as subnet: id = subnet['subnet']['network_id'] res = self._create_port(self.fmt, id) data = self.deserialize(self.fmt, res) @@ -2583,7 +2583,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): self.subnet(cidr='14.129.122.5/22'), self.subnet(cidr='15.129.122.5/24'), self.subnet(cidr='16.129.122.5/28'), - self.subnet(cidr='17.129.122.5/32') + self.subnet(cidr='17.129.122.5/32', gateway_ip=None) ) as subs: # the API should accept and correct these for users self.assertEqual(subs[0]['subnet']['cidr'], '10.0.0.0/8') @@ -2724,15 +2724,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): allocation_pools) def test_create_subnet_gw_values(self): - # Gateway not in subnet - gateway = '100.0.0.1' cidr = '10.0.0.0/24' - allocation_pools = [{'start': '10.0.0.1', - 'end': '10.0.0.254'}] - expected = {'gateway_ip': gateway, - 'cidr': cidr, - 'allocation_pools': allocation_pools} - self._test_create_subnet(expected=expected, gateway_ip=gateway) # Gateway is last IP in range gateway = '10.0.0.254' allocation_pools = [{'start': '10.0.0.1', @@ -2751,8 +2743,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): self._test_create_subnet(expected=expected, gateway_ip=gateway) - def test_create_subnet_gw_outside_cidr_force_on_returns_400(self): - cfg.CONF.set_override('force_gateway_on_subnet', True) + def test_create_subnet_gw_outside_cidr_returns_400(self): with self.network() as network: self._create_subnet(self.fmt, network['network']['id'], @@ -2760,8 +2751,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): webob.exc.HTTPClientError.code, gateway_ip='100.0.0.1') - def test_create_subnet_gw_of_network_force_on_returns_400(self): - cfg.CONF.set_override('force_gateway_on_subnet', True) + def test_create_subnet_gw_of_network_returns_400(self): with self.network() as network: self._create_subnet(self.fmt, network['network']['id'], @@ -2769,8 +2759,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): webob.exc.HTTPClientError.code, gateway_ip='10.0.0.0') - def test_create_subnet_gw_bcast_force_on_returns_400(self): - cfg.CONF.set_override('force_gateway_on_subnet', True) + def test_create_subnet_gw_bcast_returns_400(self): with self.network() as network: self._create_subnet(self.fmt, network['network']['id'], @@ -3038,6 +3027,28 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): ipv6_ra_mode=mode, ipv6_address_mode=mode) + def test_create_subnet_ipv6_out_of_cidr_global(self): + gateway_ip = '2000::1' + cidr = '2001::/64' + + with testlib_api.ExpectedException( + webob.exc.HTTPClientError) as ctx_manager: + self._test_create_subnet( + gateway_ip=gateway_ip, cidr=cidr, ip_version=6, + ipv6_ra_mode=constants.DHCPV6_STATEFUL, + ipv6_address_mode=constants.DHCPV6_STATEFUL) + self.assertEqual(ctx_manager.exception.code, + webob.exc.HTTPClientError.code) + + def test_create_subnet_ipv6_out_of_cidr_lla(self): + gateway_ip = 'fe80::1' + cidr = '2001::/64' + + self._test_create_subnet( + gateway_ip=gateway_ip, cidr=cidr, ip_version=6, + ipv6_ra_mode=constants.IPV6_SLAAC, + ipv6_address_mode=constants.IPV6_SLAAC) + def test_create_subnet_ipv6_attributes_no_dhcp_enabled(self): gateway_ip = 'fe80::1' cidr = 'fe80::/80' @@ -3122,7 +3133,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): def test_update_subnet_no_gateway(self): with self.subnet() as subnet: - data = {'subnet': {'gateway_ip': '11.0.0.1'}} + data = {'subnet': {'gateway_ip': '10.0.0.1'}} req = self.new_update_request('subnets', data, subnet['subnet']['id']) res = self.deserialize(self.fmt, req.get_response(self.api)) @@ -3136,7 +3147,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): def test_update_subnet(self): with self.subnet() as subnet: - data = {'subnet': {'gateway_ip': '11.0.0.1'}} + data = {'subnet': {'gateway_ip': '10.0.0.1'}} req = self.new_update_request('subnets', data, subnet['subnet']['id']) res = self.deserialize(self.fmt, req.get_response(self.api)) @@ -3182,8 +3193,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): self.assertEqual(res.status_int, webob.exc.HTTPClientError.code) - def test_update_subnet_gw_outside_cidr_force_on_returns_400(self): - cfg.CONF.set_override('force_gateway_on_subnet', True) + def test_update_subnet_gw_outside_cidr_returns_400(self): with self.network() as network: with self.subnet(network=network) as subnet: data = {'subnet': {'gateway_ip': '100.0.0.1'}} diff --git a/neutron/tests/unit/test_l3_plugin.py b/neutron/tests/unit/test_l3_plugin.py index 4eb80d0d33d..324f8bf8ade 100644 --- a/neutron/tests/unit/test_l3_plugin.py +++ b/neutron/tests/unit/test_l3_plugin.py @@ -880,8 +880,6 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): try_overlapped_cidr('10.0.1.0/24') # another subnet with overlapped cidr including s1 try_overlapped_cidr('10.0.0.0/16') - # another subnet with overlapped cidr included by s1 - try_overlapped_cidr('10.0.1.1/32') # clean-up self._router_interface_action('remove', r['router']['id'],