commit 2f0011194012a2482f79603c310028736e9ff3c8 Author: Brian Haley Date: Mon Jan 8 15:50:40 2024 -0500 Disallow subnet cidr of :: without PD Do not allow the subnet cidr of :: to be used when creating a subnet, except in the case IPv6 prefix delegation has been specified in the request. Closes-bug: #2028159 Change-Id: I480e9a117513996f3c070acd4ba39c2b9fe9c0f1 diff --git a/doc/source/admin/config-ipv6.rst b/doc/source/admin/config-ipv6.rst index 8f2395f6fb..65841eeb3e 100644 --- a/doc/source/admin/config-ipv6.rst +++ b/doc/source/admin/config-ipv6.rst @@ -723,18 +723,19 @@ First, create a network and IPv6 subnet: +---------------------------+--------------------------------------+ $ openstack subnet create --ip-version 6 --ipv6-ra-mode slaac \ - --ipv6-address-mode slaac --use-default-subnet-pool \ + --ipv6-address-mode slaac --use-prefix-delegation \ --network ipv6-pd ipv6-pd-1 +------------------------+--------------------------------------+ | Field | Value | +------------------------+--------------------------------------+ - | allocation_pools | ::2-::ffff:ffff:ffff:ffff | + | allocation_pools | ::1-::ffff:ffff:ffff:ffff | | cidr | ::/64 | | created_at | 2017-01-25T19:31:53Z | | description | | | dns_nameservers | | + | dns_publish_fixed_ip | None | | enable_dhcp | True | - | gateway_ip | ::1 | + | gateway_ip | :: | | headers | | | host_routes | | | id | 1319510d-c92c-4532-bf5d-8bcf3da761a1 | @@ -747,9 +748,8 @@ First, create a network and IPv6 subnet: | revision_number | 2 | | service_types | | | subnetpool_id | prefix_delegation | - | tags | [] | + | tags | | | updated_at | 2017-01-25T19:31:53Z | - | use_default_subnetpool | True | +------------------------+--------------------------------------+ The subnet is initially created with a temporary CIDR before one can be diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 972de5b260..3fdd68dc8c 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -640,7 +640,7 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon, "the ip_version '%(ip_version)s'") % data raise exc.InvalidInput(error_message=msg) - def _validate_subnet(self, context, s, cur_subnet=None): + def _validate_subnet(self, context, s, cur_subnet=None, is_pd=False): """Validate a subnet spec.""" # This method will validate attributes which may change during @@ -658,6 +658,7 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon, has_cidr = False if validators.is_attr_set(s.get('cidr')): self._validate_ip_version(ip_ver, s['cidr'], 'cidr') + net = netaddr.IPNetwork(s['cidr']) has_cidr = True # TODO(watanabe.isao): After we found a way to avoid the re-sync @@ -666,15 +667,21 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon, dhcp_was_enabled = cur_subnet.enable_dhcp else: dhcp_was_enabled = False + # A subnet cidr of '::' is invalid, unless the caller has + # indicated they are doing Prefix Delegation, + # see https://bugs.launchpad.net/neutron/+bug/2028159 + if (has_cidr and ip_ver == constants.IP_VERSION_6 and + net.network == netaddr.IPAddress('::') and not + is_pd): + error_message = _("IPv6 subnet '::' is not supported") + raise exc.InvalidInput(error_message=error_message) if has_cidr and s.get('enable_dhcp') and not dhcp_was_enabled: - subnet_prefixlen = netaddr.IPNetwork(s['cidr']).prefixlen error_message = _("Subnet has a prefix length that is " "incompatible with DHCP service enabled") - if ((ip_ver == 4 and subnet_prefixlen > 30) or - (ip_ver == 6 and subnet_prefixlen > 126)): + if ((ip_ver == 4 and net.prefixlen > 30) or + (ip_ver == 6 and net.prefixlen > 126)): raise exc.InvalidInput(error_message=error_message) - net = netaddr.IPNetwork(s['cidr']) if net.is_multicast(): error_message = _("Multicast IP subnet is not supported " "if enable_dhcp is True") @@ -929,9 +936,11 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon, raise exc.BadRequest(resource='subnets', msg=msg) validate = True + is_pd = False if subnetpool_id: self.ipam.validate_pools_with_subnetpool(s) if subnetpool_id == constants.IPV6_PD_POOL_ID: + is_pd = True if has_cidr: # We do not currently support requesting a specific # cidr with IPv6 prefix delegation. Set the subnetpool_id @@ -949,7 +958,7 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon, raise exc.BadRequest(resource='subnets', msg=msg) if validate: - self._validate_subnet(context, s) + self._validate_subnet(context, s, is_pd=is_pd) with db_api.CONTEXT_WRITER.using(context): network = self._get_network(context, @@ -1006,7 +1015,8 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon, # Fill 'network_id' field with the current value since this is expected # by _validate_segment() in ipam_pluggable_backend. s['network_id'] = subnet_obj.network_id - self._validate_subnet(context, s, cur_subnet=subnet_obj) + is_pd = s['subnetpool_id'] == constants.IPV6_PD_POOL_ID + self._validate_subnet(context, s, cur_subnet=subnet_obj, is_pd=is_pd) db_pools = [netaddr.IPRange(p.start, p.end) for p in subnet_obj.allocation_pools] diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index bb301b9263..ecc79b556a 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -455,7 +455,7 @@ class NeutronDbPluginV2TestCase(testlib_api.WebTestCase): data = {'subnet': {'network_id': net_id, 'ip_version': constants.IP_VERSION_4, 'tenant_id': tenant_id}} - if cidr: + if cidr and cidr is not constants.ATTR_NOT_SPECIFIED: data['subnet']['cidr'] = cidr for arg in ('ip_version', 'tenant_id', 'subnetpool_id', 'prefixlen', 'enable_dhcp', 'allocation_pools', 'segment_id', @@ -823,7 +823,9 @@ class NeutronDbPluginV2TestCase(testlib_api.WebTestCase): as_admin=False): if project_id: tenant_id = project_id - cidr = netaddr.IPNetwork(cidr) if cidr else None + if (cidr is not None and + cidr != constants.ATTR_NOT_SPECIFIED): + cidr = netaddr.IPNetwork(cidr) if (gateway_ip is not None and gateway_ip != constants.ATTR_NOT_SPECIFIED): gateway_ip = netaddr.IPAddress(gateway_ip) @@ -1007,7 +1009,7 @@ class NeutronDbPluginV2TestCase(testlib_api.WebTestCase): if (k == 'gateway_ip' and ipv6_zero_gateway and keys[k][-3:] == "::0"): self.assertEqual(keys[k][:-1], resource[res_name][k]) - else: + elif keys[k] != constants.ATTR_NOT_SPECIFIED: self.assertEqual(keys[k], resource[res_name][k]) @@ -3316,7 +3318,6 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): def _test_create_subnet(self, network=None, expected=None, **kwargs): keys = kwargs.copy() - keys.setdefault('cidr', '10.0.0.0/24') keys.setdefault('ip_version', constants.IP_VERSION_4) keys.setdefault('enable_dhcp', True) with self.subnet(network=network, **keys) as subnet: @@ -4055,6 +4056,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): @testtools.skipIf(tools.is_bsd(), 'bug/1484837') def test_create_subnet_ipv6_pd_gw_values(self): cidr = constants.PROVISIONAL_IPV6_PD_PREFIX + subnetpool_id = constants.IPV6_PD_POOL_ID # Gateway is last IP in IPv6 DHCPv6 Stateless subnet gateway = '::ffff:ffff:ffff:ffff' allocation_pools = [{'start': '::1', @@ -4062,10 +4064,17 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): expected = {'gateway_ip': gateway, 'cidr': cidr, 'allocation_pools': allocation_pools} + # We do not specify a CIDR in the API call for a PD subnet, as it + # is unsupported. Instead we specify the subnetpool_id as + # "prefix_delegation" which is what happens via OSC's + # --use-prefix-delegation argument. But the expected result is a + # subnet object with the "::/64" PD prefix. Same comment applies below. self._test_create_subnet(expected=expected, gateway_ip=gateway, - cidr=cidr, ip_version=constants.IP_VERSION_6, + cidr=constants.ATTR_NOT_SPECIFIED, + ip_version=constants.IP_VERSION_6, ipv6_ra_mode=constants.DHCPV6_STATELESS, - ipv6_address_mode=constants.DHCPV6_STATELESS) + ipv6_address_mode=constants.DHCPV6_STATELESS, + subnetpool_id=subnetpool_id) # Gateway is first IP in IPv6 DHCPv6 Stateless subnet gateway = '::1' allocation_pools = [{'start': '::2', @@ -4074,18 +4083,22 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): 'cidr': cidr, 'allocation_pools': allocation_pools} self._test_create_subnet(expected=expected, gateway_ip=gateway, - cidr=cidr, ip_version=constants.IP_VERSION_6, + cidr=constants.ATTR_NOT_SPECIFIED, + ip_version=constants.IP_VERSION_6, ipv6_ra_mode=constants.DHCPV6_STATELESS, - ipv6_address_mode=constants.DHCPV6_STATELESS) + ipv6_address_mode=constants.DHCPV6_STATELESS, + subnetpool_id=subnetpool_id) # If gateway_ip is not specified and the subnet is using prefix # delegation, until the CIDR is assigned, this value should be first # IP from the subnet expected = {'gateway_ip': str(netaddr.IPNetwork(cidr).network), 'cidr': cidr} self._test_create_subnet(expected=expected, - cidr=cidr, ip_version=constants.IP_VERSION_6, + cidr=constants.ATTR_NOT_SPECIFIED, + ip_version=constants.IP_VERSION_6, ipv6_ra_mode=constants.IPV6_SLAAC, - ipv6_address_mode=constants.IPV6_SLAAC) + ipv6_address_mode=constants.IPV6_SLAAC, + subnetpool_id=subnetpool_id) def test_create_subnet_gw_outside_cidr_returns_201(self): with self.network() as network: @@ -4182,14 +4195,21 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): allocation_pools=allocation_pools) @testtools.skipIf(tools.is_bsd(), 'bug/1484837') - def test_create_subnet_with_v6_pd_allocation_pool(self): + def test_create_subnet_with_v6_pd_allocation_pool_returns_400(self): gateway_ip = '::1' cidr = constants.PROVISIONAL_IPV6_PD_PREFIX allocation_pools = [{'start': '::2', 'end': '::ffff:ffff:ffff:fffe'}] - self._test_create_subnet(gateway_ip=gateway_ip, - cidr=cidr, ip_version=constants.IP_VERSION_6, - allocation_pools=allocation_pools) + # Creating a subnet object with the "::/64" PD prefix is invalid + # unless the subnetpool_id is also passed as "prefix_delegation" + with testlib_api.ExpectedException( + webob.exc.HTTPClientError) as ctx_manager: + self._test_create_subnet(gateway_ip=gateway_ip, + cidr=cidr, + ip_version=constants.IP_VERSION_6, + allocation_pools=allocation_pools) + self.assertEqual(webob.exc.HTTPClientError.code, + ctx_manager.exception.code) def test_create_subnet_with_large_allocation_pool(self): gateway_ip = '10.0.0.1' @@ -4400,10 +4420,10 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): for mode, value in modes.items(): new_subnet[mode] = value if expect_success: - plugin._validate_subnet(ctx, new_subnet, cur_subnet) + plugin._validate_subnet(ctx, new_subnet, cur_subnet, True) else: self.assertRaises(lib_exc.InvalidInput, plugin._validate_subnet, - ctx, new_subnet, cur_subnet) + ctx, new_subnet, cur_subnet, True) def test_create_subnet_ipv6_ra_modes(self): # Test all RA modes with no address mode specified diff --git a/neutron/tests/unit/db/test_ipam_pluggable_backend.py b/neutron/tests/unit/db/test_ipam_pluggable_backend.py index 4099c1f4e6..169125a381 100644 --- a/neutron/tests/unit/db/test_ipam_pluggable_backend.py +++ b/neutron/tests/unit/db/test_ipam_pluggable_backend.py @@ -375,6 +375,7 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase): context = mock.Mock() pluggable_backend = ipam_pluggable_backend.IpamPluggableBackend() with self.subnet(cidr=constants.PROVISIONAL_IPV6_PD_PREFIX, + subnetpool_id=constants.IPV6_PD_POOL_ID, ip_version=constants.IP_VERSION_6) as subnet: subnet = subnet['subnet'] fixed_ips = [{'subnet_id': subnet['id'],