From 13aaa32f77c2852a97cb72fd306cfedd253c4c3f Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Fri, 19 Feb 2021 11:35:02 +0100 Subject: [PATCH] [OVS FW] Allow egress ICMPv6 only for know addresses Before that patch it was possible to send ICMPv6 packets like e.g. neutron_lib.constants.ICMPV6_TYPE_MLD_QUERY, neutron_lib.constants.ICMPV6_TYPE_RS, neutron_lib.constants.ICMPV6_TYPE_NS, neutron_lib.constants.ICMPV6_TYPE_NA And that could cause some security issues as instance could advertise that it owns IPv6 address which really don't belong to it. Now rules in table=71 which allows that traffic are "per mac/ipaddress" and are allowed only for fixed ips allocated to port and port's allowed_address_pairs. Closes-Bug: #1902917 Change-Id: I4749fdc6a6cabd253b971bf4010ff76f5593c59c (cherry picked from commit 4b5bcff64c4725b37f094dfd2613ec58f723d304) --- .../internals/openvswitch_firewall.rst | 20 ++++----- .../linux/openvswitch_firewall/firewall.py | 33 ++++++++------- .../openvswitch_firewall/test_firewall.py | 41 ++++++++++++++++++- 3 files changed, 69 insertions(+), 25 deletions(-) diff --git a/doc/source/contributor/internals/openvswitch_firewall.rst b/doc/source/contributor/internals/openvswitch_firewall.rst index 5809e88fe3d..903635b230f 100644 --- a/doc/source/contributor/internals/openvswitch_firewall.rst +++ b/doc/source/contributor/internals/openvswitch_firewall.rst @@ -245,16 +245,16 @@ solicitation and neighbour advertisement. :: - table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=130 actions=resubmit(,94) - table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=131 actions=resubmit(,94) - table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=132 actions=resubmit(,94) - table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=135 actions=resubmit(,94) - table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=136 actions=resubmit(,94) - table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=130 actions=resubmit(,94) - table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=131 actions=resubmit(,94) - table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=132 actions=resubmit(,94) - table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=135 actions=resubmit(,94) - table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=136 actions=resubmit(,94) + table=71, priority=95,icmp6,reg5=0x1,in_port=1,dl_src=fa:16:3e:a4:22:11,ipv6_src=fe80::11,icmp_type=130 actions=resubmit(,94) + table=71, priority=95,icmp6,reg5=0x1,in_port=1,dl_src=fa:16:3e:a4:22:11,ipv6_src=fe80::11,icmp_type=131 actions=resubmit(,94) + table=71, priority=95,icmp6,reg5=0x1,in_port=1,dl_src=fa:16:3e:a4:22:11,ipv6_src=fe80::11,icmp_type=132 actions=resubmit(,94) + table=71, priority=95,icmp6,reg5=0x1,in_port=1,dl_src=fa:16:3e:a4:22:11,ipv6_src=fe80::11,icmp_type=135 actions=resubmit(,94) + table=71, priority=95,icmp6,reg5=0x1,in_port=1,dl_src=fa:16:3e:a4:22:11,ipv6_src=fe80::11,icmp_type=136 actions=resubmit(,94) + table=71, priority=95,icmp6,reg5=0x2,in_port=2,dl_src=fa:16:3e:a4:22:22,ipv6_src=fe80::22,icmp_type=130 actions=resubmit(,94) + table=71, priority=95,icmp6,reg5=0x2,in_port=2,dl_src=fa:16:3e:a4:22:22,ipv6_src=fe80::22,icmp_type=131 actions=resubmit(,94) + table=71, priority=95,icmp6,reg5=0x2,in_port=2,dl_src=fa:16:3e:a4:22:22,ipv6_src=fe80::22,icmp_type=132 actions=resubmit(,94) + table=71, priority=95,icmp6,reg5=0x2,in_port=2,dl_src=fa:16:3e:a4:22:22,ipv6_src=fe80::22,icmp_type=135 actions=resubmit(,94) + table=71, priority=95,icmp6,reg5=0x2,in_port=2,dl_src=fa:16:3e:a4:22:22,ipv6_src=fe80::22,icmp_type=136 actions=resubmit(,94) Following rules implement ARP spoofing protection diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index 119a994572f..1aaa0fba618 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -898,19 +898,24 @@ class OVSFirewallDriver(firewall.FirewallDriver): self._initialize_egress(port) self._initialize_ingress(port) - def _initialize_egress_ipv6_icmp(self, port): - for icmp_type in firewall.ICMPV6_ALLOWED_EGRESS_TYPES: - self._add_flow( - table=ovs_consts.BASE_EGRESS_TABLE, - priority=95, - in_port=port.ofport, - reg_port=port.ofport, - dl_type=lib_const.ETHERTYPE_IPV6, - nw_proto=lib_const.PROTO_NUM_IPV6_ICMP, - icmp_type=icmp_type, - actions='resubmit(,%d)' % ( - ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE) - ) + def _initialize_egress_ipv6_icmp(self, port, allowed_pairs): + # NOTE(slaweq): should we include also fe80::/64 (link-local) subnet + # in the allowed pairs here? + for mac_addr, ip_addr in allowed_pairs: + for icmp_type in firewall.ICMPV6_ALLOWED_EGRESS_TYPES: + self._add_flow( + table=ovs_consts.BASE_EGRESS_TABLE, + priority=95, + in_port=port.ofport, + reg_port=port.ofport, + dl_type=lib_const.ETHERTYPE_IPV6, + nw_proto=lib_const.PROTO_NUM_IPV6_ICMP, + icmp_type=icmp_type, + dl_src=mac_addr, + ipv6_src=ip_addr, + actions='resubmit(,%d)' % ( + ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE) + ) def _initialize_egress_no_port_security(self, port_id, ovs_ports=None): try: @@ -983,7 +988,6 @@ class OVSFirewallDriver(firewall.FirewallDriver): def _initialize_egress(self, port): """Identify egress traffic and send it to egress base""" - self._initialize_egress_ipv6_icmp(port) # Apply mac/ip pairs for IPv4 allowed_pairs = port.allowed_pairs_v4.union( @@ -1016,6 +1020,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): # Apply mac/ip pairs for IPv6 allowed_pairs = port.allowed_pairs_v6.union( {(port.mac, ip_addr) for ip_addr in port.ipv6_addresses}) + self._initialize_egress_ipv6_icmp(port, allowed_pairs) for mac_addr, ip_addr in allowed_pairs: self._add_flow( table=ovs_consts.BASE_EGRESS_TABLE, 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 9b7c24909a3..dfea997430e 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -24,6 +24,7 @@ import testtools from neutron.agent.common import ovs_lib from neutron.agent.common import utils +from neutron.agent import firewall as agent_firewall from neutron.agent.linux.openvswitch_firewall import constants as ovsfw_consts from neutron.agent.linux.openvswitch_firewall import exceptions from neutron.agent.linux.openvswitch_firewall import firewall as ovsfw @@ -40,8 +41,14 @@ TESTING_SEGMENT = 1000 def create_ofport(port_dict, network_type=None, physical_network=None, segment_id=TESTING_SEGMENT): + allowed_pairs_v4 = ovsfw.OFPort._get_allowed_pairs( + port_dict, version=constants.IPv4) + allowed_pairs_v6 = ovsfw.OFPort._get_allowed_pairs( + port_dict, version=constants.IPv6) ovs_port = mock.Mock(vif_mac='00:00:00:00:00:00', ofport=1, - port_name="port-name") + port_name="port-name", + allowed_pairs_v4=allowed_pairs_v4, + allowed_pairs_v6=allowed_pairs_v6) return ovsfw.OFPort(port_dict, ovs_port, vlan_tag=TESTING_VLAN_TAG, segment_id=segment_id, network_type=network_type, @@ -981,6 +988,38 @@ class TestOVSFirewallDriver(base.BaseTestCase): with testtools.ExpectedException(exceptions.OVSFWPortNotHandled): self.firewall._remove_egress_no_port_security('foo') + def test__initialize_egress_ipv6_icmp(self): + port_dict = { + 'device': 'port-id', + 'security_groups': [1], + 'fixed_ips': ["10.0.0.1"], + 'allowed_address_pairs': [ + {'mac_address': 'aa:bb:cc:dd:ee:ff', + 'ip_address': '192.168.1.1'}, + {'mac_address': 'aa:bb:cc:dd:ee:ff', + 'ip_address': '2003::1'} + ]} + of_port = create_ofport(port_dict) + self.mock_bridge.br.db_get_val.return_value = {'tag': TESTING_VLAN_TAG} + self.firewall._initialize_egress_ipv6_icmp( + of_port, set([('aa:bb:cc:dd:ee:ff', '2003::1')])) + expected_calls = [] + for icmp_type in agent_firewall.ICMPV6_ALLOWED_EGRESS_TYPES: + expected_calls.append( + mock.call( + table=ovs_consts.BASE_EGRESS_TABLE, + priority=95, + in_port=TESTING_VLAN_TAG, + reg5=TESTING_VLAN_TAG, + dl_type='0x86dd', + nw_proto=constants.PROTO_NUM_IPV6_ICMP, + icmp_type=icmp_type, + dl_src='aa:bb:cc:dd:ee:ff', + ipv6_src='2003::1', + actions='resubmit(,%d)' % ( + ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE))) + self.mock_bridge.br.add_flow.assert_has_calls(expected_calls) + def test_process_trusted_ports_caches_port_id(self): vif_port = ovs_lib.VifPort('name', 1, 'id', 'mac', mock.ANY) with mock.patch.object(self.firewall.int_br.br, 'get_vifs_by_ids',