From cce4120e5dec5e82721e7e6849d829b5e02568b5 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Tue, 17 Nov 2020 14:33:47 +0000 Subject: [PATCH] Revert "[Security] fix allowed-address-pair 0.0.0.0/0 issue" This reverts commit 551130e9f4e8c89821d73881ad8e90a0208adb3e. Conflict: neutron/tests/functional/agent/test_firewall.py Change-Id: I574fb07b3bd7813723377121c63eeec028e3c838 Closes-Bug: #1903531 --- neutron/agent/linux/ipset_manager.py | 2 +- neutron/agent/linux/iptables_firewall.py | 2 +- .../agent/linux/openvswitch_firewall/rules.py | 6 ----- .../api/rpc/handlers/securitygroups_rpc.py | 6 ++--- neutron/db/securitygroups_rpc_base.py | 23 +++++++------------ .../tests/functional/agent/test_firewall.py | 3 +-- .../openvswitch_firewall/test_firewall.py | 9 ++++---- .../linux/openvswitch_firewall/test_rules.py | 3 +-- .../unit/agent/linux/test_ipset_manager.py | 18 +++++---------- .../agent/linux/test_iptables_firewall.py | 22 +++++++----------- .../unit/agent/test_securitygroups_rpc.py | 22 +++++------------- .../rpc/handlers/test_securitygroups_rpc.py | 22 ++++-------------- 12 files changed, 43 insertions(+), 95 deletions(-) diff --git a/neutron/agent/linux/ipset_manager.py b/neutron/agent/linux/ipset_manager.py index 49b236f48dd..d3caf59784f 100644 --- a/neutron/agent/linux/ipset_manager.py +++ b/neutron/agent/linux/ipset_manager.py @@ -44,7 +44,7 @@ class IpsetManager(object): /1's to represent the /0. """ sanitized_addresses = [] - for ip, _mac in addresses: + for ip in addresses: ip = netaddr.IPNetwork(ip) if ip.prefixlen == 0: if ip.version == 4: diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py index 756d2d590c2..1edc42a9d7e 100644 --- a/neutron/agent/linux/iptables_firewall.py +++ b/neutron/agent/linux/iptables_firewall.py @@ -534,7 +534,7 @@ class IptablesFirewallDriver(firewall.FirewallDriver): ethertype = rule['ethertype'] port_ips = port.get('fixed_ips', []) - for ip, _mac in self.sg_members[remote_group_id][ethertype]: + for ip in self.sg_members[remote_group_id][ethertype]: if ip not in port_ips: ip_rule = rule.copy() direction_ip_prefix = firewall.DIRECTION_IP_PREFIX[ diff --git a/neutron/agent/linux/openvswitch_firewall/rules.py b/neutron/agent/linux/openvswitch_firewall/rules.py index 94bbd874de3..86f2ad70d61 100644 --- a/neutron/agent/linux/openvswitch_firewall/rules.py +++ b/neutron/agent/linux/openvswitch_firewall/rules.py @@ -297,9 +297,6 @@ def create_flows_for_ip_address(ip_address, direction, ethertype, """Create flows from a rule and an ip_address derived from remote_group_id """ - ip_address, mac_address = ip_address - net = netaddr.IPNetwork(str(ip_address)) - any_src_ip = net.prefixlen == 0 # Group conj_ids per priority. conj_id_lists = [[] for i in range(4)] @@ -324,9 +321,6 @@ def create_flows_for_ip_address(ip_address, direction, ethertype, flow_template[FLOW_FIELD_FOR_IPVER_AND_DIRECTION[( ip_ver, direction)]] = ip_prefix - if any_src_ip: - flow_template['dl_src'] = mac_address - result = [] for offset, conj_id_list in enumerate(conj_id_lists): if not conj_id_list: diff --git a/neutron/api/rpc/handlers/securitygroups_rpc.py b/neutron/api/rpc/handlers/securitygroups_rpc.py index 478723ab41a..01ff3b3decd 100644 --- a/neutron/api/rpc/handlers/securitygroups_rpc.py +++ b/neutron/api/rpc/handlers/securitygroups_rpc.py @@ -326,10 +326,8 @@ class SecurityGroupServerAPIShim(sg_rpc_base.SecurityGroupInfoAPIMixin): filters = {'security_group_ids': tuple(remote_group_ids)} for p in self.rcache.get_resources('Port', filters): - allowed_ips = [(str(addr.ip_address), str(addr.mac_address)) - for addr in p.allowed_address_pairs] - port_ips = [(str(addr.ip_address), str(p.mac_address)) - for addr in p.fixed_ips] + allowed_ips + port_ips = [str(addr.ip_address) + for addr in p.fixed_ips + p.allowed_address_pairs] for sg_id in p.security_group_ids: if sg_id in ips_by_group: ips_by_group[sg_id].update(set(port_ips)) diff --git a/neutron/db/securitygroups_rpc_base.py b/neutron/db/securitygroups_rpc_base.py index 369c15fd0f8..858a9461264 100644 --- a/neutron/db/securitygroups_rpc_base.py +++ b/neutron/db/securitygroups_rpc_base.py @@ -224,7 +224,7 @@ class SecurityGroupInfoAPIMixin(object): context, sg_info['sg_member_ips'].keys()) for sg_id, member_ips in ips.items(): for ip in member_ips: - ethertype = 'IPv%d' % netaddr.IPNetwork(ip[0]).version + ethertype = 'IPv%d' % netaddr.IPNetwork(ip).version if ethertype in sg_info['sg_member_ips'][sg_id]: sg_info['sg_member_ips'][sg_id][ethertype].add(ip) return sg_info @@ -253,8 +253,7 @@ class SecurityGroupInfoAPIMixin(object): port['security_group_source_groups'].append(remote_group_id) base_rule = rule - ip_list = [ip[0] for ip in ips[remote_group_id]] - for ip in ip_list: + for ip in ips[remote_group_id]: if ip in port.get('fixed_ips', []): continue ip_rule = base_rule.copy() @@ -397,11 +396,9 @@ class SecurityGroupServerRpcMixin(SecurityGroupInfoAPIMixin, # Join the security group binding table directly to the IP allocation # table instead of via the Port table skip an unnecessary intermediary - query = context.session.query( - sg_binding_sgid, - models_v2.IPAllocation.ip_address, - aap_models.AllowedAddressPair.ip_address, - aap_models.AllowedAddressPair.mac_address) + query = context.session.query(sg_binding_sgid, + models_v2.IPAllocation.ip_address, + aap_models.AllowedAddressPair.ip_address) query = query.join(models_v2.IPAllocation, ip_port == sg_binding_port) # Outerjoin because address pairs may be null and we still want the @@ -413,12 +410,8 @@ class SecurityGroupServerRpcMixin(SecurityGroupInfoAPIMixin, # Each allowed address pair IP record for a port beyond the 1st # will have a duplicate regular IP in the query response since # the relationship is 1-to-many. Dedup with a set - for security_group_id, ip_address, allowed_addr_ip, mac in query: - # Since port mac will not be used further, but in order to align - # the data structure we directly set None to it to avoid bother - # the ports table. - ips_by_group[security_group_id].add((ip_address, None)) + for security_group_id, ip_address, allowed_addr_ip in query: + ips_by_group[security_group_id].add(ip_address) if allowed_addr_ip: - ips_by_group[security_group_id].add( - (allowed_addr_ip, mac)) + ips_by_group[security_group_id].add(allowed_addr_ip) return ips_by_group diff --git a/neutron/tests/functional/agent/test_firewall.py b/neutron/tests/functional/agent/test_firewall.py index 27a90cd41a3..3f0d96ce7c5 100644 --- a/neutron/tests/functional/agent/test_firewall.py +++ b/neutron/tests/functional/agent/test_firewall.py @@ -585,8 +585,7 @@ class FirewallTestCase(BaseFirewallTestCase): [remote_sg_id], self.net_id) - vm_sg_members = {'IPv4': [ - (self.tester.peer_ip_address, self.tester.peer_mac_address)]} + vm_sg_members = {'IPv4': [self.tester.peer_ip_address]} peer_sg_rules = [{'ethertype': 'IPv4', 'direction': 'egress', 'protocol': 'icmp'}] self.firewall.update_security_group_rules(remote_sg_id, peer_sg_rules) diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py index 661a0555910..c7d8263e16e 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -351,10 +351,9 @@ class TestConjIPFlowManager(base.BaseTestCase): def test_update_flows_for_vlan_no_ports_but_members(self): remote_group = self.driver.sg_port_map.get_sg.return_value remote_group.ports = set() - remote_group.members = {constants.IPv4: [ - ('10.22.3.4', 'fa:16:3e:aa:bb:cc'), ]} + remote_group.members = {constants.IPv4: ['10.22.3.4']} remote_group.get_ethertype_filtered_addresses.return_value = [ - ('10.22.3.4', 'fa:16:3e:aa:bb:cc'), ] + '10.22.3.4'] with mock.patch.object(self.manager.conj_id_map, 'get_conj_id') as get_conj_id_mock: get_conj_id_mock.return_value = self.conj_id @@ -367,7 +366,7 @@ class TestConjIPFlowManager(base.BaseTestCase): def test_update_flows_for_vlan(self): remote_group = self.driver.sg_port_map.get_sg.return_value remote_group.get_ethertype_filtered_addresses.return_value = [ - ('10.22.3.4', 'fa:16:3e:aa:bb:cc'), ] + '10.22.3.4'] with mock.patch.object(self.manager.conj_id_map, 'get_conj_id') as get_conj_id_mock: get_conj_id_mock.return_value = self.conj_id @@ -1040,7 +1039,7 @@ class TestOVSFirewallDriver(base.BaseTestCase): def test_delete_flow_for_ip_using_cookie_any(self): with mock.patch.object(self.firewall, '_delete_flows') as \ mock_delete_flows: - self.firewall.delete_flow_for_ip(('10.1.2.3', None), + self.firewall.delete_flow_for_ip('10.1.2.3', constants.INGRESS_DIRECTION, constants.IPv4, 100, [0]) _, kwargs = mock_delete_flows.call_args diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py index d2b8daa5110..34140ad264a 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py @@ -349,8 +349,7 @@ class TestCreateFlowsForIpAddress(base.BaseTestCase): conj_ids = [12, 20] flows = rules.create_flows_for_ip_address( - ('192.168.0.1', 'fa:16:3e:aa:bb:cc'), - constants.EGRESS_DIRECTION, constants.IPv4, + '192.168.0.1', constants.EGRESS_DIRECTION, constants.IPv4, 0x123, conj_ids) self.assertEqual(2, len(flows)) diff --git a/neutron/tests/unit/agent/linux/test_ipset_manager.py b/neutron/tests/unit/agent/linux/test_ipset_manager.py index 5eca4949284..bb7984638dd 100644 --- a/neutron/tests/unit/agent/linux/test_ipset_manager.py +++ b/neutron/tests/unit/agent/linux/test_ipset_manager.py @@ -20,12 +20,8 @@ TEST_SET_ID = 'fake_sgid' ETHERTYPE = 'IPv4' TEST_SET_NAME = ipset_manager.IpsetManager.get_name(TEST_SET_ID, ETHERTYPE) TEST_SET_NAME_NEW = TEST_SET_NAME + ipset_manager.SWAP_SUFFIX -FAKE_IPS = [('10.0.0.1', 'fa:16:3e:aa:bb:c1'), - ('10.0.0.2', 'fa:16:3e:aa:bb:c2'), - ('10.0.0.3', 'fa:16:3e:aa:bb:c3'), - ('10.0.0.4', 'fa:16:3e:aa:bb:c4'), - ('10.0.0.5', 'fa:16:3e:aa:bb:c5'), - ('10.0.0.6', 'fa:16:3e:aa:bb:c6')] +FAKE_IPS = ['10.0.0.1', '10.0.0.2', '10.0.0.3', '10.0.0.4', + '10.0.0.5', '10.0.0.6'] class BaseIpsetManagerTest(base.BaseTestCase): @@ -153,15 +149,13 @@ class IpsetManagerTestCase(BaseIpsetManagerTest): self.verify_mock_calls() def test_set_members_adding_all_zero_ipv4(self): - self.expect_set([('0.0.0.0/0', 'fa:16:3e:aa:bb:c1'), ]) - self.ipset.set_members(TEST_SET_ID, ETHERTYPE, - [('0.0.0.0/0', 'fa:16:3e:aa:bb:c1'), ]) + self.expect_set(['0.0.0.0/0']) + self.ipset.set_members(TEST_SET_ID, ETHERTYPE, ['0.0.0.0/0']) self.verify_mock_calls() def test_set_members_adding_all_zero_ipv6(self): - self.expect_set([('::/0', 'fa:16:3e:aa:bb:c1'), ]) - self.ipset.set_members(TEST_SET_ID, ETHERTYPE, - [('::/0', 'fa:16:3e:aa:bb:c1'), ]) + self.expect_set(['::/0']) + self.ipset.set_members(TEST_SET_ID, ETHERTYPE, ['::/0']) self.verify_mock_calls() def test_destroy(self): diff --git a/neutron/tests/unit/agent/linux/test_iptables_firewall.py b/neutron/tests/unit/agent/linux/test_iptables_firewall.py index 1d676b81548..94e43d2da89 100644 --- a/neutron/tests/unit/agent/linux/test_iptables_firewall.py +++ b/neutron/tests/unit/agent/linux/test_iptables_firewall.py @@ -1526,16 +1526,12 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase): if ethertype == "IPv4": ethertype = "ipv4" - members_add = {'IPv4': [('10.0.0.2', 'fa:16:3e:aa:bb:c1'), - ('10.0.0.3', 'fa:16:3e:aa:bb:c2')]} - members_after_delete = { - 'IPv4': [('10.0.0.3', 'fa:16:3e:aa:bb:c2'), ]} + members_add = {'IPv4': ['10.0.0.2', '10.0.0.3']} + members_after_delete = {'IPv4': ['10.0.0.3']} else: ethertype = "ipv6" - members_add = {'IPv6': [('fe80::2', 'fa:16:3e:aa:bb:c3'), - ('fe80::3', 'fa:16:3e:aa:bb:c4')]} - members_after_delete = { - 'IPv6': [('fe80::3', 'fa:16:3e:aa:bb:c4'), ]} + members_add = {'IPv6': ['fe80::2', 'fe80::3']} + members_after_delete = {'IPv6': ['fe80::3']} with mock.patch.dict(self.firewall.ipconntrack._device_zone_map, {port['network_id']: ct_zone}): @@ -2307,12 +2303,10 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): self.firewall.ipset.assert_has_calls(calls, True) def test_sg_rule_expansion_with_remote_ips(self): - other_ips = [('10.0.0.2', 'fa:16:3e:aa:bb:c1'), - ('10.0.0.3', 'fa:16:3e:aa:bb:c2'), - ('10.0.0.4', 'fa:16:3e:aa:bb:c3')] + other_ips = ['10.0.0.2', '10.0.0.3', '10.0.0.4'] self.firewall.sg_members = {'fake_sgid': { - 'IPv4': [(FAKE_IP['IPv4'], 'fa:16:3e:aa:bb:c4'), ] + other_ips, - 'IPv6': [(FAKE_IP['IPv6'], 'fa:16:3e:aa:bb:c5'), ]}} + 'IPv4': [FAKE_IP['IPv4']] + other_ips, + 'IPv6': [FAKE_IP['IPv6']]}} port = self._fake_port() rule = self._fake_sg_rule_for_ethertype(_IPv4, FAKE_SGID) @@ -2321,7 +2315,7 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): self.assertEqual(list(rules), [dict(list(rule.items()) + [('source_ip_prefix', '%s/32' % ip)]) - for ip, _mac in other_ips]) + for ip in other_ips]) def test_build_ipv4v6_mac_ip_list(self): mac_oth = 'ffff-ff0f-ffff' diff --git a/neutron/tests/unit/agent/test_securitygroups_rpc.py b/neutron/tests/unit/agent/test_securitygroups_rpc.py index fb57a038e15..c3418cbbdca 100644 --- a/neutron/tests/unit/agent/test_securitygroups_rpc.py +++ b/neutron/tests/unit/agent/test_securitygroups_rpc.py @@ -317,10 +317,8 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase): sg_member_ips = self.rpc.security_group_info_for_devices( ctx, devices=devices)['sg_member_ips'] expected_member_ips = [ - ('10.0.1.0/24', '00:00:00:00:00:01'), - ('11.0.0.1', '00:00:00:00:00:01'), - (port['port']['fixed_ips'][0]['ip_address'], - None)] + '10.0.1.0/24', '11.0.0.1', + port['port']['fixed_ips'][0]['ip_address']] self.assertEqual(sorted(expected_member_ips), sorted(sg_member_ips[sg_id]['IPv4'])) self._delete('ports', port_id) @@ -524,7 +522,7 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase): 'remote_group_id': sg2_id} ]}, 'sg_member_ips': {sg2_id: { - 'IPv4': set([(port_ip2, None), ]), + 'IPv4': set([port_ip2]), 'IPv6': set(), }} } @@ -2918,9 +2916,7 @@ class TestSecurityGroupAgentEnhancedRpcWithIptables( self.devices_info1 = {'security_groups': {'security_group1': rule1}, 'sg_member_ips': { 'security_group1': { - 'IPv4': [('10.0.0.3/32', - 'fa:16:3e:aa:bb:c1'), ], - 'IPv6': []}}, + 'IPv4': ['10.0.0.3/32'], 'IPv6': []}}, 'devices': devices_info1} devices_info2 = collections.OrderedDict([ ('tap_port1', self._device('tap_port1', @@ -2935,19 +2931,13 @@ class TestSecurityGroupAgentEnhancedRpcWithIptables( self.devices_info2 = {'security_groups': {'security_group1': rule1}, 'sg_member_ips': { 'security_group1': { - 'IPv4': [('10.0.0.3/32', - 'fa:16:3e:aa:bb:c1'), - ('10.0.0.4/32', - 'fa:16:3e:aa:bb:c2')], + 'IPv4': ['10.0.0.3/32', '10.0.0.4/32'], 'IPv6': []}}, 'devices': devices_info2} self.devices_info3 = {'security_groups': {'security_group1': rule2}, 'sg_member_ips': { 'security_group1': { - 'IPv4': [('10.0.0.3/32', - 'fa:16:3e:aa:bb:c1'), - ('10.0.0.4/32', - 'fa:16:3e:aa:bb:c2')], + 'IPv4': ['10.0.0.3/32', '10.0.0.4/32'], 'IPv6': []}}, 'devices': devices_info2} diff --git a/neutron/tests/unit/api/rpc/handlers/test_securitygroups_rpc.py b/neutron/tests/unit/api/rpc/handlers/test_securitygroups_rpc.py index 339c80ca2a6..6099327b367 100644 --- a/neutron/tests/unit/api/rpc/handlers/test_securitygroups_rpc.py +++ b/neutron/tests/unit/api/rpc/handlers/test_securitygroups_rpc.py @@ -13,7 +13,6 @@ # under the License. import mock -import netaddr from neutron_lib import context from oslo_utils import uuidutils @@ -129,30 +128,19 @@ class SecurityGroupServerAPIShimTestCase(base.BaseTestCase): def test_security_group_info_for_devices(self): s1 = self._make_security_group_ovo() - mac_1 = 'fa:16:3e:aa:bb:c1' - p1 = self._make_port_ovo(ip='1.1.1.1', - mac_address=netaddr.EUI(mac_1), - security_group_ids={s1.id}) - mac_2 = 'fa:16:3e:aa:bb:c2' + p1 = self._make_port_ovo(ip='1.1.1.1', security_group_ids={s1.id}) p2 = self._make_port_ovo( ip='2.2.2.2', - mac_address=netaddr.EUI(mac_2), security_group_ids={s1.id}, security=psec.PortSecurity(port_security_enabled=False)) - mac_3 = 'fa:16:3e:aa:bb:c3' - p3 = self._make_port_ovo(ip='3.3.3.3', - mac_address=netaddr.EUI(mac_3), - security_group_ids={s1.id}, + p3 = self._make_port_ovo(ip='3.3.3.3', security_group_ids={s1.id}, device_owner='network:dhcp') ids = [p1.id, p2.id, p3.id] info = self.shim.security_group_info_for_devices(self.ctx, ids) - self.assertIn(('1.1.1.1', str(netaddr.EUI(mac_1))), - info['sg_member_ips'][s1.id]['IPv4']) - self.assertIn(('2.2.2.2', str(netaddr.EUI(mac_2))), - info['sg_member_ips'][s1.id]['IPv4']) - self.assertIn(('3.3.3.3', str(netaddr.EUI(mac_3))), - info['sg_member_ips'][s1.id]['IPv4']) + self.assertIn('1.1.1.1', info['sg_member_ips'][s1.id]['IPv4']) + self.assertIn('2.2.2.2', info['sg_member_ips'][s1.id]['IPv4']) + self.assertIn('3.3.3.3', info['sg_member_ips'][s1.id]['IPv4']) self.assertIn(p1.id, info['devices'].keys()) self.assertIn(p2.id, info['devices'].keys()) # P3 is a trusted port so it doesn't have rules