Browse Source

Fix KeyError in OVS firewall

When merging port ranges, the code never assumed the
conjunction ID might not be present in the set due to
already being removed.

In this case there were two security groups, both using
the same remote security group, but the first security
group does not define a port range and the second one does.
Or more generally, the first SG port range is a subset
of the second, as no port-range means the full range.

Change-Id: I17ab643abbd2ec21eda4ae1dfb9abf2d4b0657f2
Closes-bug: #1813007
tags/15.0.0.0b1
Brian Haley IWAMOTO Toshihiro 1 year ago
parent
commit
18c578aa10
2 changed files with 22 additions and 7 deletions
  1. +11
    -5
      neutron/agent/linux/openvswitch_firewall/rules.py
  2. +11
    -2
      neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py

+ 11
- 5
neutron/agent/linux/openvswitch_firewall/rules.py View File

@@ -121,26 +121,32 @@ def merge_port_ranges(rule_conj_list):
# item means a removal.
result = []
rule_tmpl = rule_conj_list[0][0]
cur_conj = set()
cur_conj = {}
cur_range_min = None
for port, m, conj_id in port_ranges:
if m == 'min':
if conj_id in cur_conj:
cur_conj[conj_id] += 1
continue
if cur_conj and cur_range_min != port:
rule = rule_tmpl.copy()
rule['port_range_min'] = cur_range_min
rule['port_range_max'] = port - 1
result.append((rule, list(cur_conj)))
result.append((rule, list(cur_conj.keys())))
cur_range_min = port
cur_conj.add(conj_id)
cur_conj[conj_id] = 1
else:
if cur_conj[conj_id] > 1:
cur_conj[conj_id] -= 1
continue
if cur_range_min <= port:
rule = rule_tmpl.copy()
rule['port_range_min'] = cur_range_min
rule['port_range_max'] = port
result.append((rule, list(cur_conj)))
result.append((rule, list(cur_conj.keys())))
# The next port range without 'port' starts from (port + 1)
cur_range_min = port + 1
cur_conj.remove(conj_id)
del cur_conj[conj_id]

if (len(result) == 1 and result[0][0]['port_range_min'] == 1 and
result[0][0]['port_range_max'] == 65535):


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

@@ -420,8 +420,8 @@ class TestMergeRules(base.BaseTestCase):
self.assertEqual(len(expected), len(result))
for (range_min, range_max, conj_ids), result1 in zip(
expected, result):
self.assertEqual(range_min, result1[0]['port_range_min'])
self.assertEqual(range_max, result1[0]['port_range_max'])
self.assertEqual(range_min, result1[0].get('port_range_min'))
self.assertEqual(range_max, result1[0].get('port_range_max'))
self.assertEqual(conj_ids, set(result1[1]))

def test__assert_mergeable_rules(self):
@@ -489,6 +489,15 @@ class TestMergeRules(base.BaseTestCase):
(30, 40, {10, 12, 4}),
(41, 65535, {10, 12})], result)

def test_merge_port_ranges_no_port_ranges_same_conj_id(self):
result = rules.merge_port_ranges(
[(dict(self.rule_tmpl), 10),
(dict(self.rule_tmpl), 12),
(dict([('port_range_min', 30), ('port_range_max', 30)] +
self.rule_tmpl), 10)])
self._test_merge_port_ranges_helper([
(None, None, {10, 12})], result)

def test_merge_port_ranges_nonoverlapping(self):
result = rules.merge_port_ranges(
[(dict([('port_range_min', 30), ('port_range_max', 40)] +


Loading…
Cancel
Save