Merge "Revert "[Security] fix allowed-address-pair 0.0.0.0/0 issue"" into stable/rocky

This commit is contained in:
Zuul 2020-11-18 01:56:25 +00:00 committed by Gerrit Code Review
commit 4bd90a00ce
12 changed files with 42 additions and 94 deletions

View File

@ -44,7 +44,7 @@ class IpsetManager(object):
/1's to represent the /0. /1's to represent the /0.
""" """
sanitized_addresses = [] sanitized_addresses = []
for ip, _mac in addresses: for ip in addresses:
ip = netaddr.IPNetwork(ip) ip = netaddr.IPNetwork(ip)
if ip.prefixlen == 0: if ip.prefixlen == 0:
if ip.version == 4: if ip.version == 4:

View File

@ -542,7 +542,7 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
ethertype = rule['ethertype'] ethertype = rule['ethertype']
port_ips = port.get('fixed_ips', []) 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: if ip not in port_ips:
ip_rule = rule.copy() ip_rule = rule.copy()
direction_ip_prefix = firewall.DIRECTION_IP_PREFIX[ direction_ip_prefix = firewall.DIRECTION_IP_PREFIX[

View File

@ -296,9 +296,6 @@ def create_flows_for_ip_address(ip_address, direction, ethertype,
"""Create flows from a rule and an ip_address derived from """Create flows from a rule and an ip_address derived from
remote_group_id 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. # Group conj_ids per priority.
conj_id_lists = [[] for i in range(4)] conj_id_lists = [[] for i in range(4)]
@ -323,9 +320,6 @@ def create_flows_for_ip_address(ip_address, direction, ethertype,
flow_template[FLOW_FIELD_FOR_IPVER_AND_DIRECTION[( flow_template[FLOW_FIELD_FOR_IPVER_AND_DIRECTION[(
ip_ver, direction)]] = ip_prefix ip_ver, direction)]] = ip_prefix
if any_src_ip:
flow_template['dl_src'] = mac_address
result = [] result = []
for offset, conj_id_list in enumerate(conj_id_lists): for offset, conj_id_list in enumerate(conj_id_lists):
if not conj_id_list: if not conj_id_list:

View File

@ -326,10 +326,8 @@ class SecurityGroupServerAPIShim(sg_rpc_base.SecurityGroupInfoAPIMixin):
filters = {'security_group_ids': tuple(remote_group_ids)} filters = {'security_group_ids': tuple(remote_group_ids)}
for p in self.rcache.get_resources('Port', filters): for p in self.rcache.get_resources('Port', filters):
allowed_ips = [(str(addr.ip_address), str(addr.mac_address)) port_ips = [str(addr.ip_address)
for addr in p.allowed_address_pairs] for addr in p.fixed_ips + p.allowed_address_pairs]
port_ips = [(str(addr.ip_address), str(p.mac_address))
for addr in p.fixed_ips] + allowed_ips
for sg_id in p.security_group_ids: for sg_id in p.security_group_ids:
if sg_id in ips_by_group: if sg_id in ips_by_group:
ips_by_group[sg_id].update(set(port_ips)) ips_by_group[sg_id].update(set(port_ips))

View File

@ -224,7 +224,7 @@ class SecurityGroupInfoAPIMixin(object):
context, sg_info['sg_member_ips'].keys()) context, sg_info['sg_member_ips'].keys())
for sg_id, member_ips in ips.items(): for sg_id, member_ips in ips.items():
for ip in member_ips: 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]: if ethertype in sg_info['sg_member_ips'][sg_id]:
sg_info['sg_member_ips'][sg_id][ethertype].add(ip) sg_info['sg_member_ips'][sg_id][ethertype].add(ip)
return sg_info return sg_info
@ -253,8 +253,7 @@ class SecurityGroupInfoAPIMixin(object):
port['security_group_source_groups'].append(remote_group_id) port['security_group_source_groups'].append(remote_group_id)
base_rule = rule base_rule = rule
ip_list = [ip[0] for ip in ips[remote_group_id]] for ip in ips[remote_group_id]:
for ip in ip_list:
if ip in port.get('fixed_ips', []): if ip in port.get('fixed_ips', []):
continue continue
ip_rule = base_rule.copy() ip_rule = base_rule.copy()
@ -397,11 +396,9 @@ class SecurityGroupServerRpcMixin(SecurityGroupInfoAPIMixin,
# Join the security group binding table directly to the IP allocation # Join the security group binding table directly to the IP allocation
# table instead of via the Port table skip an unnecessary intermediary # table instead of via the Port table skip an unnecessary intermediary
query = context.session.query( query = context.session.query(sg_binding_sgid,
sg_binding_sgid, models_v2.IPAllocation.ip_address,
models_v2.IPAllocation.ip_address, aap_models.AllowedAddressPair.ip_address)
aap_models.AllowedAddressPair.ip_address,
aap_models.AllowedAddressPair.mac_address)
query = query.join(models_v2.IPAllocation, query = query.join(models_v2.IPAllocation,
ip_port == sg_binding_port) ip_port == sg_binding_port)
# Outerjoin because address pairs may be null and we still want the # 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 # Each allowed address pair IP record for a port beyond the 1st
# will have a duplicate regular IP in the query response since # will have a duplicate regular IP in the query response since
# the relationship is 1-to-many. Dedup with a set # the relationship is 1-to-many. Dedup with a set
for security_group_id, ip_address, allowed_addr_ip, mac in query: for security_group_id, ip_address, allowed_addr_ip in query:
# Since port mac will not be used further, but in order to align ips_by_group[security_group_id].add(ip_address)
# 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))
if allowed_addr_ip: if allowed_addr_ip:
ips_by_group[security_group_id].add( ips_by_group[security_group_id].add(allowed_addr_ip)
(allowed_addr_ip, mac))
return ips_by_group return ips_by_group

View File

@ -584,8 +584,7 @@ class FirewallTestCase(BaseFirewallTestCase):
[remote_sg_id], [remote_sg_id],
self.net_id) self.net_id)
vm_sg_members = {'IPv4': [ vm_sg_members = {'IPv4': [self.tester.peer_ip_address]}
(self.tester.peer_ip_address, self.tester.peer_mac_address)]}
peer_sg_rules = [{'ethertype': 'IPv4', 'direction': 'egress', peer_sg_rules = [{'ethertype': 'IPv4', 'direction': 'egress',
'protocol': 'icmp'}] 'protocol': 'icmp'}]
self.firewall.update_security_group_rules(remote_sg_id, peer_sg_rules) self.firewall.update_security_group_rules(remote_sg_id, peer_sg_rules)

View File

@ -350,10 +350,9 @@ class TestConjIPFlowManager(base.BaseTestCase):
def test_update_flows_for_vlan_no_ports_but_members(self): def test_update_flows_for_vlan_no_ports_but_members(self):
remote_group = self.driver.sg_port_map.get_sg.return_value remote_group = self.driver.sg_port_map.get_sg.return_value
remote_group.ports = set() remote_group.ports = set()
remote_group.members = {constants.IPv4: [ remote_group.members = {constants.IPv4: ['10.22.3.4']}
('10.22.3.4', 'fa:16:3e:aa:bb:cc'), ]}
remote_group.get_ethertype_filtered_addresses.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, with mock.patch.object(self.manager.conj_id_map,
'get_conj_id') as get_conj_id_mock: 'get_conj_id') as get_conj_id_mock:
get_conj_id_mock.return_value = self.conj_id get_conj_id_mock.return_value = self.conj_id
@ -366,7 +365,7 @@ class TestConjIPFlowManager(base.BaseTestCase):
def test_update_flows_for_vlan(self): def test_update_flows_for_vlan(self):
remote_group = self.driver.sg_port_map.get_sg.return_value remote_group = self.driver.sg_port_map.get_sg.return_value
remote_group.get_ethertype_filtered_addresses.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, with mock.patch.object(self.manager.conj_id_map,
'get_conj_id') as get_conj_id_mock: 'get_conj_id') as get_conj_id_mock:
get_conj_id_mock.return_value = self.conj_id get_conj_id_mock.return_value = self.conj_id

View File

@ -350,8 +350,7 @@ class TestCreateFlowsForIpAddress(base.BaseTestCase):
conj_ids = [12, 20] conj_ids = [12, 20]
flows = rules.create_flows_for_ip_address( flows = rules.create_flows_for_ip_address(
('192.168.0.1', 'fa:16:3e:aa:bb:cc'), '192.168.0.1', constants.EGRESS_DIRECTION, constants.IPv4,
constants.EGRESS_DIRECTION, constants.IPv4,
0x123, conj_ids) 0x123, conj_ids)
self.assertEqual(2, len(flows)) self.assertEqual(2, len(flows))

View File

@ -20,12 +20,8 @@ TEST_SET_ID = 'fake_sgid'
ETHERTYPE = 'IPv4' ETHERTYPE = 'IPv4'
TEST_SET_NAME = ipset_manager.IpsetManager.get_name(TEST_SET_ID, ETHERTYPE) TEST_SET_NAME = ipset_manager.IpsetManager.get_name(TEST_SET_ID, ETHERTYPE)
TEST_SET_NAME_NEW = TEST_SET_NAME + ipset_manager.SWAP_SUFFIX TEST_SET_NAME_NEW = TEST_SET_NAME + ipset_manager.SWAP_SUFFIX
FAKE_IPS = [('10.0.0.1', 'fa:16:3e:aa:bb:c1'), FAKE_IPS = ['10.0.0.1', '10.0.0.2', '10.0.0.3', '10.0.0.4',
('10.0.0.2', 'fa:16:3e:aa:bb:c2'), '10.0.0.5', '10.0.0.6']
('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')]
class BaseIpsetManagerTest(base.BaseTestCase): class BaseIpsetManagerTest(base.BaseTestCase):
@ -153,15 +149,13 @@ class IpsetManagerTestCase(BaseIpsetManagerTest):
self.verify_mock_calls() self.verify_mock_calls()
def test_set_members_adding_all_zero_ipv4(self): def test_set_members_adding_all_zero_ipv4(self):
self.expect_set([('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, self.ipset.set_members(TEST_SET_ID, ETHERTYPE, ['0.0.0.0/0'])
[('0.0.0.0/0', 'fa:16:3e:aa:bb:c1'), ])
self.verify_mock_calls() self.verify_mock_calls()
def test_set_members_adding_all_zero_ipv6(self): def test_set_members_adding_all_zero_ipv6(self):
self.expect_set([('::/0', 'fa:16:3e:aa:bb:c1'), ]) self.expect_set(['::/0'])
self.ipset.set_members(TEST_SET_ID, ETHERTYPE, self.ipset.set_members(TEST_SET_ID, ETHERTYPE, ['::/0'])
[('::/0', 'fa:16:3e:aa:bb:c1'), ])
self.verify_mock_calls() self.verify_mock_calls()
def test_destroy(self): def test_destroy(self):

View File

@ -1492,16 +1492,12 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
if ethertype == "IPv4": if ethertype == "IPv4":
ethertype = "ipv4" ethertype = "ipv4"
members_add = {'IPv4': [('10.0.0.2', 'fa:16:3e:aa:bb:c1'), members_add = {'IPv4': ['10.0.0.2', '10.0.0.3']}
('10.0.0.3', 'fa:16:3e:aa:bb:c2')]} members_after_delete = {'IPv4': ['10.0.0.3']}
members_after_delete = {
'IPv4': [('10.0.0.3', 'fa:16:3e:aa:bb:c2'), ]}
else: else:
ethertype = "ipv6" ethertype = "ipv6"
members_add = {'IPv6': [('fe80::2', 'fa:16:3e:aa:bb:c3'), members_add = {'IPv6': ['fe80::2', 'fe80::3']}
('fe80::3', 'fa:16:3e:aa:bb:c4')]} members_after_delete = {'IPv6': ['fe80::3']}
members_after_delete = {
'IPv6': [('fe80::3', 'fa:16:3e:aa:bb:c4'), ]}
with mock.patch.dict(self.firewall.ipconntrack._device_zone_map, with mock.patch.dict(self.firewall.ipconntrack._device_zone_map,
{port['network_id']: ct_zone}): {port['network_id']: ct_zone}):
@ -2273,12 +2269,10 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase):
self.firewall.ipset.assert_has_calls(calls, True) self.firewall.ipset.assert_has_calls(calls, True)
def test_sg_rule_expansion_with_remote_ips(self): def test_sg_rule_expansion_with_remote_ips(self):
other_ips = [('10.0.0.2', 'fa:16:3e:aa:bb:c1'), other_ips = ['10.0.0.2', '10.0.0.3', '10.0.0.4']
('10.0.0.3', 'fa:16:3e:aa:bb:c2'),
('10.0.0.4', 'fa:16:3e:aa:bb:c3')]
self.firewall.sg_members = {'fake_sgid': { self.firewall.sg_members = {'fake_sgid': {
'IPv4': [(FAKE_IP['IPv4'], 'fa:16:3e:aa:bb:c4'), ] + other_ips, 'IPv4': [FAKE_IP['IPv4']] + other_ips,
'IPv6': [(FAKE_IP['IPv6'], 'fa:16:3e:aa:bb:c5'), ]}} 'IPv6': [FAKE_IP['IPv6']]}}
port = self._fake_port() port = self._fake_port()
rule = self._fake_sg_rule_for_ethertype(_IPv4, FAKE_SGID) rule = self._fake_sg_rule_for_ethertype(_IPv4, FAKE_SGID)
@ -2287,7 +2281,7 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase):
self.assertEqual(list(rules), self.assertEqual(list(rules),
[dict(list(rule.items()) + [dict(list(rule.items()) +
[('source_ip_prefix', '%s/32' % ip)]) [('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): def test_build_ipv4v6_mac_ip_list(self):
mac_oth = 'ffff-ff0f-ffff' mac_oth = 'ffff-ff0f-ffff'

View File

@ -317,10 +317,8 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase):
sg_member_ips = self.rpc.security_group_info_for_devices( sg_member_ips = self.rpc.security_group_info_for_devices(
ctx, devices=devices)['sg_member_ips'] ctx, devices=devices)['sg_member_ips']
expected_member_ips = [ expected_member_ips = [
('10.0.1.0/24', '00:00:00:00:00:01'), '10.0.1.0/24', '11.0.0.1',
('11.0.0.1', '00:00:00:00:00:01'), port['port']['fixed_ips'][0]['ip_address']]
(port['port']['fixed_ips'][0]['ip_address'],
None)]
self.assertEqual(sorted(expected_member_ips), self.assertEqual(sorted(expected_member_ips),
sorted(sg_member_ips[sg_id]['IPv4'])) sorted(sg_member_ips[sg_id]['IPv4']))
self._delete('ports', port_id) self._delete('ports', port_id)
@ -524,7 +522,7 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase):
'remote_group_id': sg2_id} 'remote_group_id': sg2_id}
]}, ]},
'sg_member_ips': {sg2_id: { 'sg_member_ips': {sg2_id: {
'IPv4': set([(port_ip2, None), ]), 'IPv4': set([port_ip2]),
'IPv6': set(), 'IPv6': set(),
}} }}
} }
@ -2949,9 +2947,7 @@ class TestSecurityGroupAgentEnhancedRpcWithIptables(
self.devices_info1 = {'security_groups': {'security_group1': rule1}, self.devices_info1 = {'security_groups': {'security_group1': rule1},
'sg_member_ips': { 'sg_member_ips': {
'security_group1': { 'security_group1': {
'IPv4': [('10.0.0.3/32', 'IPv4': ['10.0.0.3/32'], 'IPv6': []}},
'fa:16:3e:aa:bb:c1'), ],
'IPv6': []}},
'devices': devices_info1} 'devices': devices_info1}
devices_info2 = collections.OrderedDict([ devices_info2 = collections.OrderedDict([
('tap_port1', self._device('tap_port1', ('tap_port1', self._device('tap_port1',
@ -2966,19 +2962,13 @@ class TestSecurityGroupAgentEnhancedRpcWithIptables(
self.devices_info2 = {'security_groups': {'security_group1': rule1}, self.devices_info2 = {'security_groups': {'security_group1': rule1},
'sg_member_ips': { 'sg_member_ips': {
'security_group1': { 'security_group1': {
'IPv4': [('10.0.0.3/32', 'IPv4': ['10.0.0.3/32', '10.0.0.4/32'],
'fa:16:3e:aa:bb:c1'),
('10.0.0.4/32',
'fa:16:3e:aa:bb:c2')],
'IPv6': []}}, 'IPv6': []}},
'devices': devices_info2} 'devices': devices_info2}
self.devices_info3 = {'security_groups': {'security_group1': rule2}, self.devices_info3 = {'security_groups': {'security_group1': rule2},
'sg_member_ips': { 'sg_member_ips': {
'security_group1': { 'security_group1': {
'IPv4': [('10.0.0.3/32', 'IPv4': ['10.0.0.3/32', '10.0.0.4/32'],
'fa:16:3e:aa:bb:c1'),
('10.0.0.4/32',
'fa:16:3e:aa:bb:c2')],
'IPv6': []}}, 'IPv6': []}},
'devices': devices_info2} 'devices': devices_info2}

View File

@ -13,7 +13,6 @@
# under the License. # under the License.
import mock import mock
import netaddr
from neutron_lib import context from neutron_lib import context
from oslo_utils import uuidutils from oslo_utils import uuidutils
@ -127,30 +126,19 @@ class SecurityGroupServerAPIShimTestCase(base.BaseTestCase):
def test_security_group_info_for_devices(self): def test_security_group_info_for_devices(self):
s1 = self._make_security_group_ovo() s1 = self._make_security_group_ovo()
mac_1 = 'fa:16:3e:aa:bb:c1' p1 = self._make_port_ovo(ip='1.1.1.1', security_group_ids={s1.id})
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'
p2 = self._make_port_ovo( p2 = self._make_port_ovo(
ip='2.2.2.2', ip='2.2.2.2',
mac_address=netaddr.EUI(mac_2),
security_group_ids={s1.id}, security_group_ids={s1.id},
security=psec.PortSecurity(port_security_enabled=False)) 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', security_group_ids={s1.id},
p3 = self._make_port_ovo(ip='3.3.3.3',
mac_address=netaddr.EUI(mac_3),
security_group_ids={s1.id},
device_owner='network:dhcp') device_owner='network:dhcp')
ids = [p1.id, p2.id, p3.id] ids = [p1.id, p2.id, p3.id]
info = self.shim.security_group_info_for_devices(self.ctx, ids) info = self.shim.security_group_info_for_devices(self.ctx, ids)
self.assertIn(('1.1.1.1', str(netaddr.EUI(mac_1))), self.assertIn('1.1.1.1', info['sg_member_ips'][s1.id]['IPv4'])
info['sg_member_ips'][s1.id]['IPv4']) self.assertIn('2.2.2.2', info['sg_member_ips'][s1.id]['IPv4'])
self.assertIn(('2.2.2.2', str(netaddr.EUI(mac_2))), self.assertIn('3.3.3.3', info['sg_member_ips'][s1.id]['IPv4'])
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(p1.id, info['devices'].keys()) self.assertIn(p1.id, info['devices'].keys())
self.assertIn(p2.id, info['devices'].keys()) self.assertIn(p2.id, info['devices'].keys())
# P3 is a trusted port so it doesn't have rules # P3 is a trusted port so it doesn't have rules