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
This commit is contained in:
parent
60edb4c951
commit
ff3132d8d4
neutron
@ -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()))
|
||||
]
|
||||
|
@ -1131,6 +1131,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])
|
||||
@ -1147,8 +1150,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])]
|
||||
@ -1217,6 +1219,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)
|
||||
@ -1290,8 +1295,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,
|
||||
@ -1510,7 +1516,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)
|
||||
|
Loading…
x
Reference in New Issue
Block a user