openstack
/
neutron
Code Issues Proposed changes

Stop killing conntrack state without CT Zone 

The conntrack clearing code was belligerenty killing connections
without a conntrack zone specifier when it couldn't get the zone
for a given device. This means it would kill all connections based
on an IP address match, which meant hitting innocent bystanders
in other tenant networks with overlapping IP addresses.

This bad fallback was being triggered every time because it was
using the wrong identifier for a port to look up the zone.

This patch fixes the port lookup and adjusts the fallback behavior
to never clear conntrack entries if we can't find the conntrack
zone for a port.

This triggered the bug below (in the cases I root-caused) by
killing a metadata connection right in the middle of retrieving
a key.

Closes-Bug: #1668958
Change-Id: Ia4ee9b3305e89c958ac927980d80119c53ea519b
(cherry picked from commit ff3132d8d4)
(cherry picked from commit 5a0700ee9f)
This commit is contained in:
Kevin Benton 2017-03-03 10:57:57 -08:00 committed by Jakub Libosvar
parent 7cbb44836b
commit 4f01368001
2 changed files with 30 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
neutron/agent/linux/ip_conntrack.py
View File

@ -74,15 +74,20 @@ class IpConntrackManager(object):
cmd = self._generate_conntrack_cmd_by_rule(rule, self.namespace)
ethertype = rule.get('ethertype')
for device_info in device_info_list:
zone_id = self._device_zone_map.get(device_info['device'], None)
zone_id = self._device_zone_map.get(
self._port_key(device_info['device']), None)
if not zone_id:
LOG.debug("No zone for device %(dev)s. Will not try to "
"clear conntrack state. Zone map: %(zm)s",
{'dev': device_info['device'],
'zm': self._device_zone_map})
continue
ips = device_info.get('fixed_ips', [])
for ip in ips:
net = netaddr.IPNetwork(ip)
if str(net.version) not in ethertype:
continue
ip_cmd = [str(net.ip)]
if zone_id:
ip_cmd.extend(['-w', zone_id])
ip_cmd = [str(net.ip), '-w', zone_id]
if remote_ip and str(
netaddr.IPNetwork(remote_ip).version) in ethertype:
if rule.get('direction') == 'ingress':
@ -134,13 +139,17 @@ class IpConntrackManager(object):
self._device_zone_map[short_port_id] = int(match.group('zone'))
LOG.debug("Populated conntrack zone map: %s", self._device_zone_map)
def get_device_zone(self, port_id):
@staticmethod
def _port_key(port_id):
# we have to key the device_zone_map based on the fragment of the port
# UUID that shows up in the interface name. This is because the initial
# map is populated strictly based on interface names that we don't know
# the full UUID of.
short_port_id = port_id[:(n_const.LINUX_DEV_LEN -
n_const.LINUX_DEV_PREFIX_LEN)]
return port_id[:(n_const.LINUX_DEV_LEN -
n_const.LINUX_DEV_PREFIX_LEN)]
def get_device_zone(self, port_id):
short_port_id = self._port_key(port_id)
try:
return self._device_zone_map[short_port_id]
except KeyError:
@ -149,8 +158,7 @@ class IpConntrackManager(object):
def _free_zones_from_removed_ports(self):
"""Clears any entries from the zone map of removed ports."""
existing_ports = [
port['device'][:(n_const.LINUX_DEV_LEN -
n_const.LINUX_DEV_PREFIX_LEN)]
self._port_key(port['device'])
for port in (list(self.filtered_ports.values()) +
list(self.unfiltered_ports.values()))
]

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

@ -1107,6 +1107,9 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
self.firewall.filter_defer_apply_on()
self.firewall.sg_rules['fake_sg_id'] = []
self.firewall.filter_defer_apply_off()
if not ct_zone:
self.assertFalse(self.utils_exec.called)
return
cmd = ['conntrack', '-D']
if protocol:
cmd.extend(['-p', protocol])
@ -1123,8 +1126,7 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
else:
cmd.extend(['-s', 'fe80::1'])
if ct_zone:
cmd.extend(['-w', ct_zone])
cmd.extend(['-w', ct_zone])
calls = [
mock.call(cmd, run_as_root=True, check_exit_code=True,
extra_ok_codes=[1])]
@ -1193,6 +1195,9 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
self.firewall.filtered_ports[port['device']] = new_port
self.firewall.filter_defer_apply_off()
if not ct_zone:
self.assertFalse(self.utils_exec.called)
return
calls = self._get_expected_conntrack_calls(
[('ipv4', '10.0.0.1'), ('ipv6', 'fe80::1')], ct_zone)
self.utils_exec.assert_has_calls(calls)
@ -1266,8 +1271,9 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
remote_ip_direction = '-s' if direction == '-d' else '-d'
conntrack_cmd = ['conntrack', '-D', '-f', ethertype,
direction, ips[ethertype][0]]
if ct_zone:
conntrack_cmd.extend(['-w', 10])
if not ct_zone:
continue
conntrack_cmd.extend(['-w', 10])
conntrack_cmd.extend([remote_ip_direction, ips[ethertype][1]])
calls.append(mock.call(conntrack_cmd,
@ -1486,7 +1492,9 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
self.firewall.ipconntrack._device_zone_map.pop('tapfake_dev', None)
self.firewall.remove_port_filter(port)
if not ct_zone:
self.assertFalse(self.utils_exec.called)
return
calls = self._get_expected_conntrack_calls(
[('ipv4', '10.0.0.1'), ('ipv6', 'fe80::1')], ct_zone)
self.utils_exec.assert_has_calls(calls)