openstack
/
neutron
Code Issues Proposed changes

[Security] fix allowed-address-pair 0.0.0.0/0 issue 

When add allowed-address-pair 0.0.0.0/0 to one port, it will
unexpectedly open all others' protocol under same security
group. IPv6 has the same problem.

The root cause is the openflow rules calculation of the
security group, it will unexpectedly allow all IP(4&6)
traffic to get through.

For openvswitch openflow firewall, this patch adds a source
mac address match for the allowed-address-pair which has
prefix lenght 0, that means all ethernet packets from this
mac will be accepted. It exactly will meet the request of
accepting any IP address from the configured VM.

Test result shows that the remote security group and
allowed address pair works:
1. Port has 0.0.0.0/0 allowed-address-pair clould send any
   IP (src) packet out.
2. Port has x.x.x.x/y allowed-address-pair could be accepted
   for those VMs under same security group.
3. Ports under same network can reach each other (remote
   security group).
4. Protocol port number could be accessed only when there
   has related rule.

Closes-bug: #1867119
Change-Id: I2e3aa7c400d7bb17cc117b65faaa160b41013dde
changes/32/712632/12
LIU Yulong 2020-03-13 18:18:04 +08:00
parent b319d64388
commit 00298fe6e8
12 changed files with 94 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
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:

2
neutron/agent/linux/iptables_firewall.py
View File

@ -601,7 +601,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
neutron/agent/linux/openvswitch_firewall/rules.py
View File

@ -297,6 +297,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)]
@ -321,6 +324,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:

6
neutron/api/rpc/handlers/securitygroups_rpc.py
View File

@ -327,8 +327,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))

23
neutron/db/securitygroups_rpc_base.py
View File

@ -228,7 +228,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
@ -257,7 +257,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()
@ -408,9 +409,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
@ -422,10 +425,14 @@ 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
@db_api.retry_if_session_inactive()

3
neutron/tests/functional/agent/test_firewall.py
View File

@ -585,7 +585,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)

7
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

3
neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py
View File

@ -349,7 +349,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))

18
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):

22
neutron/tests/unit/agent/linux/test_iptables_firewall.py
View File

@ -1527,12 +1527,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}):
@ -2304,10 +2308,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)
@ -2316,7 +2322,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'

22
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)
@ -525,7 +527,7 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase):
'stateful': True}
]},
'sg_member_ips': {sg2_id: {
'IPv4': set([port_ip2]),
'IPv4': set([(port_ip2, None), ]),
'IPv6': set(),
}}
}
@ -2922,7 +2924,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',
@ -2937,13 +2941,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}

22
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
@ -128,19 +129,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