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
(cherry picked from commit 18c578aa10)
tags/13.0.3
Brian Haley 2 months ago
parent
commit
569b3fddab

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

@@ -120,26 +120,32 @@ def merge_port_ranges(rule_conj_list):
120 120
     # item means a removal.
121 121
     result = []
122 122
     rule_tmpl = rule_conj_list[0][0]
123
-    cur_conj = set()
123
+    cur_conj = {}
124 124
     cur_range_min = None
125 125
     for port, m, conj_id in port_ranges:
126 126
         if m == 'min':
127
+            if conj_id in cur_conj:
128
+                cur_conj[conj_id] += 1
129
+                continue
127 130
             if cur_conj and cur_range_min != port:
128 131
                 rule = rule_tmpl.copy()
129 132
                 rule['port_range_min'] = cur_range_min
130 133
                 rule['port_range_max'] = port - 1
131
-                result.append((rule, list(cur_conj)))
134
+                result.append((rule, list(cur_conj.keys())))
132 135
             cur_range_min = port
133
-            cur_conj.add(conj_id)
136
+            cur_conj[conj_id] = 1
134 137
         else:
138
+            if cur_conj[conj_id] > 1:
139
+                cur_conj[conj_id] -= 1
140
+                continue
135 141
             if cur_range_min <= port:
136 142
                 rule = rule_tmpl.copy()
137 143
                 rule['port_range_min'] = cur_range_min
138 144
                 rule['port_range_max'] = port
139
-                result.append((rule, list(cur_conj)))
145
+                result.append((rule, list(cur_conj.keys())))
140 146
                 # The next port range without 'port' starts from (port + 1)
141 147
                 cur_range_min = port + 1
142
-            cur_conj.remove(conj_id)
148
+            del cur_conj[conj_id]
143 149
 
144 150
     if (len(result) == 1 and result[0][0]['port_range_min'] == 1 and
145 151
             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):
420 420
         self.assertEqual(len(expected), len(result))
421 421
         for (range_min, range_max, conj_ids), result1 in zip(
422 422
                 expected, result):
423
-            self.assertEqual(range_min, result1[0]['port_range_min'])
424
-            self.assertEqual(range_max, result1[0]['port_range_max'])
423
+            self.assertEqual(range_min, result1[0].get('port_range_min'))
424
+            self.assertEqual(range_max, result1[0].get('port_range_max'))
425 425
             self.assertEqual(conj_ids, set(result1[1]))
426 426
 
427 427
     def test__assert_mergeable_rules(self):
@@ -489,6 +489,15 @@ class TestMergeRules(base.BaseTestCase):
489 489
                 (30, 40, {10, 12, 4}),
490 490
                 (41, 65535, {10, 12})], result)
491 491
 
492
+    def test_merge_port_ranges_no_port_ranges_same_conj_id(self):
493
+        result = rules.merge_port_ranges(
494
+            [(dict(self.rule_tmpl), 10),
495
+             (dict(self.rule_tmpl), 12),
496
+             (dict([('port_range_min', 30), ('port_range_max', 30)] +
497
+                   self.rule_tmpl), 10)])
498
+        self._test_merge_port_ranges_helper([
499
+                (None, None, {10, 12})], result)
500
+
492 501
     def test_merge_port_ranges_nonoverlapping(self):
493 502
         result = rules.merge_port_ranges(
494 503
             [(dict([('port_range_min', 30), ('port_range_max', 40)] +

Loading…
Cancel
Save