diff --git a/neutron/agent/linux/openvswitch_firewall/constants.py b/neutron/agent/linux/openvswitch_firewall/constants.py index 00d142b4079..9d071bf9370 100644 --- a/neutron/agent/linux/openvswitch_firewall/constants.py +++ b/neutron/agent/linux/openvswitch_firewall/constants.py @@ -35,7 +35,14 @@ CT_MARK_INVALID = '0x1' REG_PORT = 5 REG_NET = 6 -PROTOCOLS_WITH_PORTS = (constants.PROTO_NAME_TCP, constants.PROTO_NAME_UDP) +PROTOCOLS_WITH_PORTS = (constants.PROTO_NAME_SCTP, + constants.PROTO_NAME_TCP, + constants.PROTO_NAME_UDP) + +# Only map protocols that need special handling +REVERSE_IP_PROTOCOL_MAP_WITH_PORTS = { + constants.IP_PROTOCOL_MAP[proto]: proto for proto in + PROTOCOLS_WITH_PORTS} ethertype_to_dl_type_map = { constants.IPv4: n_const.ETHERTYPE_IP, diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index ddda3193995..e021896ca0e 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -82,11 +82,27 @@ class SecurityGroup(object): self.ports = set() def update_rules(self, rules): - """Separate raw and remote rules.""" - self.raw_rules = [rule for rule in rules - if 'remote_group_id' not in rule] - self.remote_rules = [rule for rule in rules - if 'remote_group_id' in rule] + """Separate raw and remote rules. + If a rule has a protocol field, it is normalized to a number + here in order to ease later processing. + """ + self.raw_rules = [] + self.remote_rules = [] + for rule in rules: + protocol = rule.get('protocol') + if protocol is not None: + if protocol.isdigit(): + rule['protocol'] = int(protocol) + elif (rule.get('ethertype') == lib_const.IPv6 and + protocol == lib_const.PROTO_NAME_ICMP): + rule['protocol'] = lib_const.PROTO_NUM_IPV6_ICMP + else: + rule['protocol'] = lib_const.IP_PROTOCOL_MAP.get( + protocol, protocol) + if 'remote_group_id' in rule: + self.remote_rules.append(rule) + else: + self.raw_rules.append(rule) def get_ethertype_filtered_addresses(self, ethertype): return self.members.get(ethertype, []) diff --git a/neutron/agent/linux/openvswitch_firewall/rules.py b/neutron/agent/linux/openvswitch_firewall/rules.py index 2f5e87002ef..e2c96aac9c4 100644 --- a/neutron/agent/linux/openvswitch_firewall/rules.py +++ b/neutron/agent/linux/openvswitch_firewall/rules.py @@ -92,21 +92,17 @@ def create_protocol_flows(direction, flow_template, port, rule): flow_template.copy(), port) protocol = rule.get('protocol') - if protocol: - if (rule.get('ethertype') == n_consts.IPv6 and - protocol == n_consts.PROTO_NAME_ICMP): - flow_template['nw_proto'] = n_consts.PROTO_NUM_IPV6_ICMP - else: - flow_template['nw_proto'] = n_consts.IP_PROTOCOL_MAP.get( - protocol, protocol) + if protocol is not None: + flow_template['nw_proto'] = protocol flows = create_port_range_flows(flow_template, rule) return flows or [flow_template] def create_port_range_flows(flow_template, rule): - protocol = rule.get('protocol') - if protocol not in ovsfw_consts.PROTOCOLS_WITH_PORTS: + protocol = ovsfw_consts.REVERSE_IP_PROTOCOL_MAP_WITH_PORTS.get( + rule.get('protocol')) + if protocol is None: return [] flows = [] src_port_match = '{:s}_src'.format(protocol) diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py index 6c6cebc2eac..abf61004445 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -56,7 +56,7 @@ class TestSecurityGroup(base.BaseTestCase): self.sg = ovsfw.SecurityGroup('123') self.sg.members = {'type': [1, 2, 3, 4]} - def test_update_rules(self): + def test_update_rules_split(self): rules = [ {'foo': 'bar', 'rule': 'all'}, {'bar': 'foo'}, {'remote_group_id': '123456', 'foo': 'bar'}] @@ -67,6 +67,29 @@ class TestSecurityGroup(base.BaseTestCase): self.assertEqual(expected_raw_rules, self.sg.raw_rules) self.assertEqual(expected_remote_rules, self.sg.remote_rules) + def test_update_rules_protocols(self): + rules = [ + {'foo': 'bar', 'protocol': constants.PROTO_NAME_ICMP, + 'ethertype': constants.IPv4}, + {'foo': 'bar', 'protocol': constants.PROTO_NAME_ICMP, + 'ethertype': constants.IPv6}, + {'foo': 'bar', 'protocol': constants.PROTO_NAME_IPV6_ICMP_LEGACY, + 'ethertype': constants.IPv6}, + {'foo': 'bar', 'protocol': constants.PROTO_NAME_TCP}, + {'foo': 'bar', 'protocol': '94'}, + {'foo': 'bar', 'protocol': 'baz'}, + {'foo': 'no_proto'}] + self.sg.update_rules(rules) + + self.assertEqual({'foo': 'no_proto'}, self.sg.raw_rules.pop()) + protos = [rule['protocol'] for rule in self.sg.raw_rules] + self.assertEqual([constants.PROTO_NUM_ICMP, + constants.PROTO_NUM_IPV6_ICMP, + constants.PROTO_NUM_IPV6_ICMP, + constants.PROTO_NUM_TCP, + 94, + 'baz'], protos) + def test_get_ethertype_filtered_addresses(self): addresses = self.sg.get_ethertype_filtered_addresses('type') expected_addresses = [1, 2, 3, 4] diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py index 4acfead28bb..ce40ed6a9bf 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py @@ -183,7 +183,7 @@ class TestCreateProtocolFlows(base.BaseTestCase): self.assertEqual(expected_flows, flows) def test_create_protocol_flows_ingress(self): - rule = {'protocol': constants.PROTO_NAME_TCP} + rule = {'protocol': constants.PROTO_NUM_TCP} expected_flows = [{ 'table': ovs_consts.RULES_INGRESS_TABLE, 'actions': 'output:1', @@ -193,7 +193,7 @@ class TestCreateProtocolFlows(base.BaseTestCase): firewall.INGRESS_DIRECTION, rule, expected_flows) def test_create_protocol_flows_egress(self): - rule = {'protocol': constants.PROTO_NAME_TCP} + rule = {'protocol': constants.PROTO_NUM_TCP} expected_flows = [{ 'table': ovs_consts.RULES_EGRESS_TABLE, 'actions': 'resubmit(,{:d})'.format( @@ -215,7 +215,7 @@ class TestCreateProtocolFlows(base.BaseTestCase): def test_create_protocol_flows_icmp6(self): rule = {'ethertype': constants.IPv6, - 'protocol': constants.PROTO_NAME_ICMP} + 'protocol': constants.PROTO_NUM_IPV6_ICMP} expected_flows = [{ 'table': ovs_consts.RULES_EGRESS_TABLE, 'actions': 'resubmit(,{:d})'.format( @@ -227,7 +227,7 @@ class TestCreateProtocolFlows(base.BaseTestCase): def test_create_protocol_flows_port_range(self): rule = {'ethertype': constants.IPv4, - 'protocol': constants.PROTO_NAME_TCP, + 'protocol': constants.PROTO_NUM_TCP, 'port_range_min': 22, 'port_range_max': 23} expected_flows = [{ @@ -251,7 +251,7 @@ class TestCreatePortRangeFlows(base.BaseTestCase): def test_create_port_range_flows_with_source_and_destination(self): rule = { - 'protocol': constants.PROTO_NAME_TCP, + 'protocol': constants.PROTO_NUM_TCP, 'source_port_range_min': 123, 'source_port_range_max': 124, 'port_range_min': 10, @@ -265,7 +265,7 @@ class TestCreatePortRangeFlows(base.BaseTestCase): def test_create_port_range_flows_with_source(self): rule = { - 'protocol': constants.PROTO_NAME_TCP, + 'protocol': constants.PROTO_NUM_TCP, 'source_port_range_min': 123, 'source_port_range_max': 124, } @@ -277,7 +277,7 @@ class TestCreatePortRangeFlows(base.BaseTestCase): def test_create_port_range_flows_with_destination(self): rule = { - 'protocol': constants.PROTO_NAME_TCP, + 'protocol': constants.PROTO_NUM_TCP, 'port_range_min': 10, 'port_range_max': 11, } @@ -288,14 +288,14 @@ class TestCreatePortRangeFlows(base.BaseTestCase): def test_create_port_range_flows_without_port_range(self): rule = { - 'protocol': constants.PROTO_NAME_TCP, + 'protocol': constants.PROTO_NUM_TCP, } expected_flows = [] self._test_create_port_range_flows_helper(expected_flows, rule) def test_create_port_range_with_icmp_protocol(self): rule = { - 'protocol': 'icmp', + 'protocol': constants.PROTO_NUM_ICMP, 'port_range_min': 10, 'port_range_max': 11, }