openstack
/
neutron
Code Issues Proposed changes
Browse Source

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

changes/58/746858/1
Zuul 1 month ago
committed by Gerrit Code Review
parent
94fa5f9197 64a0916d2b
commit
9aa973bb5e
12 changed files with 94 additions and 42 deletions
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. +1
    -1
      neutron/agent/linux/ipset_manager.py
  2. +1
    -1
      neutron/agent/linux/iptables_firewall.py
  3. +6
    -0
      neutron/agent/linux/openvswitch_firewall/rules.py
  4. +4
    -2
      neutron/api/rpc/handlers/securitygroups_rpc.py
  5. +15
    -8
      neutron/db/securitygroups_rpc_base.py
  6. +2
    -1
      neutron/tests/functional/agent/test_firewall.py
  7. +4
    -3
      neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py
  8. +2
    -1
      neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py
  9. +12
    -6
      neutron/tests/unit/agent/linux/test_ipset_manager.py
  10. +14
    -8
      neutron/tests/unit/agent/linux/test_iptables_firewall.py
  11. +16
    -6
      neutron/tests/unit/agent/test_securitygroups_rpc.py
  12. +17
    -5
      neutron/tests/unit/api/rpc/handlers/test_securitygroups_rpc.py

+ 1
- 1
neutron/agent/linux/ipset_manager.py View File

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


+ 1
- 1
neutron/agent/linux/iptables_firewall.py View File

@@ -542,7 +542,7 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
ethertype = rule['ethertype']
port_ips = port.get('fixed_ips', [])

for ip in self.sg_members[remote_group_id][ethertype]:
for ip, _mac 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[


+ 6
- 0
neutron/agent/linux/openvswitch_firewall/rules.py View File

@@ -296,6 +296,9 @@ 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)]
@@ -320,6 +323,9 @@ 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:


+ 4
- 2
neutron/api/rpc/handlers/securitygroups_rpc.py View File

@@ -326,8 +326,10 @@ class SecurityGroupServerAPIShim(sg_rpc_base.SecurityGroupInfoAPIMixin):

filters = {'security_group_ids': tuple(remote_group_ids)}
for p in self.rcache.get_resources('Port', filters):
port_ips = [str(addr.ip_address)
for addr in p.fixed_ips + p.allowed_address_pairs]
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
for sg_id in p.security_group_ids:
if sg_id in ips_by_group:
ips_by_group[sg_id].update(set(port_ips))


+ 15
- 8
neutron/db/securitygroups_rpc_base.py View File

@@ -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).version
ethertype = 'IPv%d' % netaddr.IPNetwork(ip[0]).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,7 +253,8 @@ class SecurityGroupInfoAPIMixin(object):

port['security_group_source_groups'].append(remote_group_id)
base_rule = rule
for ip in ips[remote_group_id]:
ip_list = [ip[0] for ip in ips[remote_group_id]]
for ip in ip_list:
if ip in port.get('fixed_ips', []):
continue
ip_rule = base_rule.copy()
@@ -396,9 +397,11 @@ 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)
query = context.session.query(
sg_binding_sgid,
models_v2.IPAllocation.ip_address,
aap_models.AllowedAddressPair.ip_address,
aap_models.AllowedAddressPair.mac_address)
query = query.join(models_v2.IPAllocation,
ip_port == sg_binding_port)
# Outerjoin because address pairs may be null and we still want the
@@ -410,8 +413,12 @@ 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 in query:
ips_by_group[security_group_id].add(ip_address)
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))
if allowed_addr_ip:
ips_by_group[security_group_id].add(allowed_addr_ip)
ips_by_group[security_group_id].add(
(allowed_addr_ip, mac))
return ips_by_group

+ 2
- 1
neutron/tests/functional/agent/test_firewall.py View File

@@ -584,7 +584,8 @@ class FirewallTestCase(BaseFirewallTestCase):
[remote_sg_id],
self.net_id)

vm_sg_members = {'IPv4': [self.tester.peer_ip_address]}
vm_sg_members = {'IPv4': [
(self.tester.peer_ip_address, self.tester.peer_mac_address)]}
peer_sg_rules = [{'ethertype': 'IPv4', 'direction': 'egress',
'protocol': 'icmp'}]
self.firewall.update_security_group_rules(remote_sg_id, peer_sg_rules)


+ 4
- 3
neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py View File

@@ -323,9 +323,10 @@ 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']}
remote_group.members = {constants.IPv4: [
('10.22.3.4', 'fa:16:3e:aa:bb:cc'), ]}
remote_group.get_ethertype_filtered_addresses.return_value = [
'10.22.3.4']
('10.22.3.4', 'fa:16:3e:aa:bb:cc'), ]
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
@@ -338,7 +339,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']
('10.22.3.4', 'fa:16:3e:aa:bb:cc'), ]
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


+ 2
- 1
neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py View File

@@ -350,7 +350,8 @@ class TestCreateFlowsForIpAddress(base.BaseTestCase):

conj_ids = [12, 20]
flows = rules.create_flows_for_ip_address(
'192.168.0.1', constants.EGRESS_DIRECTION, constants.IPv4,
('192.168.0.1', 'fa:16:3e:aa:bb:cc'),
constants.EGRESS_DIRECTION, constants.IPv4,
0x123, conj_ids)

self.assertEqual(2, len(flows))


+ 12
- 6
neutron/tests/unit/agent/linux/test_ipset_manager.py View File

@@ -20,8 +20,12 @@ 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', '10.0.0.2', '10.0.0.3', '10.0.0.4',
'10.0.0.5', '10.0.0.6']
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')]


class BaseIpsetManagerTest(base.BaseTestCase):
@@ -149,13 +153,15 @@ class IpsetManagerTestCase(BaseIpsetManagerTest):
self.verify_mock_calls()

def test_set_members_adding_all_zero_ipv4(self):
self.expect_set(['0.0.0.0/0'])
self.ipset.set_members(TEST_SET_ID, ETHERTYPE, ['0.0.0.0/0'])
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.verify_mock_calls()

def test_set_members_adding_all_zero_ipv6(self):
self.expect_set(['::/0'])
self.ipset.set_members(TEST_SET_ID, ETHERTYPE, ['::/0'])
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.verify_mock_calls()

def test_destroy(self):


+ 14
- 8
neutron/tests/unit/agent/linux/test_iptables_firewall.py View File

@@ -1492,12 +1492,16 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):

if ethertype == "IPv4":
ethertype = "ipv4"
members_add = {'IPv4': ['10.0.0.2', '10.0.0.3']}
members_after_delete = {'IPv4': ['10.0.0.3']}
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'), ]}
else:
ethertype = "ipv6"
members_add = {'IPv6': ['fe80::2', 'fe80::3']}
members_after_delete = {'IPv6': ['fe80::3']}
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'), ]}

with mock.patch.dict(self.firewall.ipconntrack._device_zone_map,
{port['network_id']: ct_zone}):
@@ -2269,10 +2273,12 @@ 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', '10.0.0.3', '10.0.0.4']
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')]
self.firewall.sg_members = {'fake_sgid': {
'IPv4': [FAKE_IP['IPv4']] + other_ips,
'IPv6': [FAKE_IP['IPv6']]}}
'IPv4': [(FAKE_IP['IPv4'], 'fa:16:3e:aa:bb:c4'), ] + other_ips,
'IPv6': [(FAKE_IP['IPv6'], 'fa:16:3e:aa:bb:c5'), ]}}

port = self._fake_port()
rule = self._fake_sg_rule_for_ethertype(_IPv4, FAKE_SGID)
@@ -2281,7 +2287,7 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase):
self.assertEqual(list(rules),
[dict(list(rule.items()) +
[('source_ip_prefix', '%s/32' % ip)])
for ip in other_ips])
for ip, _mac in other_ips])

def test_build_ipv4v6_mac_ip_list(self):
mac_oth = 'ffff-ff0f-ffff'


+ 16
- 6
neutron/tests/unit/agent/test_securitygroups_rpc.py View File

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



+ 17
- 5
neutron/tests/unit/api/rpc/handlers/test_securitygroups_rpc.py View File

@@ -13,6 +13,7 @@
# under the License.

import mock
import netaddr
from neutron_lib import context
from oslo_utils import uuidutils

@@ -126,19 +127,30 @@ class SecurityGroupServerAPIShimTestCase(base.BaseTestCase):

def test_security_group_info_for_devices(self):
s1 = self._make_security_group_ovo()
p1 = self._make_port_ovo(ip='1.1.1.1', security_group_ids={s1.id})
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'
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))
p3 = self._make_port_ovo(ip='3.3.3.3', security_group_ids={s1.id},
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},
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', 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(('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(p1.id, info['devices'].keys())
self.assertIn(p2.id, info['devices'].keys())
# P3 is a trusted port so it doesn't have rules


Write Preview
Loading…
Cancel
Save