diff --git a/doc/source/contributor/internals/openvswitch_firewall.rst b/doc/source/contributor/internals/openvswitch_firewall.rst index abcdd74e8fe..406ecc441a4 100644 --- a/doc/source/contributor/internals/openvswitch_firewall.rst +++ b/doc/source/contributor/internals/openvswitch_firewall.rst @@ -146,8 +146,8 @@ connections into separate conntrack zones. table=0, priority=100,in_port=1 actions=load:0x1->NXM_NX_REG5[],load:0x284->NXM_NX_REG6[],resubmit(,71) table=0, priority=100,in_port=2 actions=load:0x2->NXM_NX_REG5[],load:0x284->NXM_NX_REG6[],resubmit(,71) - table=0, priority=90,dl_dst=fa:16:3e:a4:22:10 actions=load:0x1->NXM_NX_REG5[],load:0x284->NXM_NX_REG6[],resubmit(,81) - table=0, priority=90,dl_dst=fa:16:3e:24:57:c7 actions=load:0x2->NXM_NX_REG5[],load:0x284->NXM_NX_REG6[],resubmit(,81) + table=0, priority=90,dl_vlan=0x284,dl_dst=fa:16:3e:a4:22:10 actions=load:0x1->NXM_NX_REG5[],load:0x284->NXM_NX_REG6[],resubmit(,81) + table=0, priority=90,dl_vlan=0x284,dl_dst=fa:16:3e:24:57:c7 actions=load:0x2->NXM_NX_REG5[],load:0x284->NXM_NX_REG6[],resubmit(,81) table=0, priority=0 actions=NORMAL Following ``table 71`` implements arp spoofing protection, ip spoofing @@ -264,8 +264,8 @@ remaining egress connections are sent to normal switching. :: - table=73, priority=100,dl_dst=fa:16:3e:a4:22:10 actions=load:0x1->NXM_NX_REG5[],resubmit(,81) - table=73, priority=100,dl_dst=fa:16:3e:24:57:c7 actions=load:0x2->NXM_NX_REG5[],resubmit(,81) + table=73, priority=100,reg6=0x284,dl_dst=fa:16:3e:a4:22:10 actions=load:0x1->NXM_NX_REG5[],resubmit(,81) + table=73, priority=100,reg6=0x284,dl_dst=fa:16:3e:24:57:c7 actions=load:0x2->NXM_NX_REG5[],resubmit(,81) table=73, priority=90,ct_state=+new-est,reg5=0x1 actions=ct(commit,zone=NXM_NX_REG6[0..15]),NORMAL table=73, priority=90,ct_state=+new-est,reg5=0x2 actions=ct(commit,zone=NXM_NX_REG6[0..15]),NORMAL table=73, priority=80,reg5=0x1 actions=NORMAL diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index c86bab4642e..71eebe24dfb 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -408,6 +408,15 @@ class OVSFirewallDriver(firewall.FirewallDriver): for table in ovs_consts.OVS_FIREWALL_TABLES: self.int_br.br.add_flow(table=table, priority=0, actions='drop') + def get_ovs_port(self, port_id): + ovs_port = self.int_br.br.get_vif_port_by_id(port_id) + if not ovs_port: + raise exceptions.OVSFWPortNotFound(port_id=port_id) + return ovs_port + + def _get_port_vlan_tag(self, port_name): + return get_tag_from_other_config(self.int_br.br, port_name) + def get_ofport(self, port): port_id = port['device'] return self.sg_port_map.ports.get(port_id) @@ -418,15 +427,11 @@ class OVSFirewallDriver(firewall.FirewallDriver): If ofport is nonexistent, create and return one. """ port_id = port['device'] - ovs_port = self.int_br.br.get_vif_port_by_id(port_id) - if not ovs_port: - raise exceptions.OVSFWPortNotFound(port_id=port_id) - + ovs_port = self.get_ovs_port(port_id) try: of_port = self.sg_port_map.ports[port_id] except KeyError: - port_vlan_id = get_tag_from_other_config( - self.int_br.br, ovs_port.port_name) + port_vlan_id = self._get_port_vlan_tag(ovs_port.port_name) of_port = OFPort(port, ovs_port, port_vlan_id) self.sg_port_map.create_port(of_port, port) else: @@ -442,6 +447,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): def prepare_port_filter(self, port): if not firewall.port_sec_enabled(port): + self._initialize_egress_no_port_security(port['device']) return old_of_port = self.get_ofport(port) of_port = self.get_or_create_ofport(port) @@ -462,8 +468,10 @@ class OVSFirewallDriver(firewall.FirewallDriver): """ if not firewall.port_sec_enabled(port): self.remove_port_filter(port) + self._initialize_egress_no_port_security(port['device']) return elif not self.is_port_managed(port): + self._remove_egress_no_port_security(port['device']) self.prepare_port_filter(port) return old_of_port = self.get_ofport(port) @@ -519,6 +527,15 @@ class OVSFirewallDriver(firewall.FirewallDriver): self.conj_ip_manager.sg_removed(sg_id) self.sg_port_map.delete_sg(sg_id) + def process_trusted_ports(self, port_ids): + """Pass packets from these ports directly to ingress pipeline.""" + for port_id in port_ids: + self._initialize_egress_no_port_security(port_id) + + def remove_trusted_ports(self, port_ids): + for port_id in port_ids: + self._remove_egress_no_port_security(port_id) + def filter_defer_apply_on(self): self._deferred = True @@ -559,6 +576,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): table=ovs_consts.TRANSIENT_TABLE, priority=90, dl_dst=port.mac, + dl_vlan='0x%x' % port.vlan_tag, actions='set_field:{:d}->reg{:d},' 'set_field:{:d}->reg{:d},' 'strip_vlan,resubmit(,{:d})'.format( @@ -585,6 +603,50 @@ class OVSFirewallDriver(firewall.FirewallDriver): actions='normal' ) + def _initialize_egress_no_port_security(self, port_id): + ovs_port = self.get_ovs_port(port_id) + try: + vlan_tag = self._get_port_vlan_tag(ovs_port.port_name) + except exceptions.OVSFWTagNotFound: + # It's a patch port, don't set anything + return + self._add_flow( + table=ovs_consts.TRANSIENT_TABLE, + priority=100, + in_port=ovs_port.ofport, + actions='set_field:%d->reg%d,' + 'set_field:%d->reg%d,' + 'resubmit(,%d)' % ( + ovs_port.ofport, + ovsfw_consts.REG_PORT, + vlan_tag, + ovsfw_consts.REG_NET, + ovs_consts.ACCEPT_OR_INGRESS_TABLE) + ) + self._add_flow( + table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, + priority=80, + reg_port=ovs_port.ofport, + actions='normal' + ) + + def _remove_egress_no_port_security(self, port_id): + ovs_port = self.get_ovs_port(port_id) + try: + # Test if it's a patch port + self._get_port_vlan_tag(ovs_port.port_name) + except exceptions.OVSFWTagNotFound: + # It's a patch port, don't do anything + return + self._delete_flows( + table=ovs_consts.TRANSIENT_TABLE, + in_port=ovs_port.ofport + ) + self._delete_flows( + table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, + reg_port=ovs_port.ofport + ) + def _initialize_egress(self, port): """Identify egress traffic and send it to egress base""" self._initialize_egress_ipv6_icmp(port) @@ -695,6 +757,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, priority=100, dl_dst=port.mac, + reg_net=port.vlan_tag, actions='set_field:{:d}->reg{:d},resubmit(,{:d})'.format( port.ofport, ovsfw_consts.REG_PORT, @@ -935,14 +998,15 @@ class OVSFirewallDriver(firewall.FirewallDriver): self._strict_delete_flow( priority=90, table=ovs_consts.TRANSIENT_TABLE, - dl_dst=port.mac) + dl_dst=port.mac, + dl_vlan=port.vlan_tag) self._strict_delete_flow( priority=100, table=ovs_consts.TRANSIENT_TABLE, in_port=port.ofport) self._delete_flows(reg_port=port.ofport) self._delete_flows(table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, - dl_dst=port.mac) + dl_dst=port.mac, reg_net=port.vlan_tag) def delete_flows_for_ip_addresses( self, ip_addresses, direction, ethertype, vlan_tag): diff --git a/neutron/tests/common/conn_testers.py b/neutron/tests/common/conn_testers.py index 82d705d8496..f59dc083ab6 100644 --- a/neutron/tests/common/conn_testers.py +++ b/neutron/tests/common/conn_testers.py @@ -22,6 +22,8 @@ from oslo_utils import uuidutils from neutron.agent import firewall from neutron.common import constants as n_consts from neutron.common import utils as common_utils +from neutron.plugins.ml2.drivers.openvswitch.agent.common import ( + constants as ovs_consts) from neutron.plugins.ml2.drivers.openvswitch.agent.openflow.ovs_ofctl import ( br_int) from neutron.tests.common import machine_fixtures @@ -360,6 +362,12 @@ class ConnectionTester(fixtures.Fixture): "At least one packet got reply from %s namespace to %s " "address." % self._get_namespace_and_address(direction))) + def set_peer_port_as_patch_port(self): + pass + + def set_peer_port_as_vm_port(self): + pass + class OVSBaseConnectionTester(ConnectionTester): @@ -419,9 +427,47 @@ class OVSConnectionTester(OVSBaseConnectionTester): def set_vm_tag(self, tag): self.set_tag(self._vm.port.name, self.bridge, tag) + self._vm.port.vlan_tag = tag def set_peer_tag(self, tag): self.set_tag(self._peer.port.name, self.bridge, tag) + self._peer.port.vlan_tag = tag + + def set_peer_port_as_patch_port(self): + """As packets coming from tunneling bridges are always tagged with + local VLAN tag, this flows will simulate the behavior. + """ + self.bridge.add_flow( + table=ovs_consts.LOCAL_SWITCHING, + priority=110, + vlan_tci=0, + in_port=self.bridge.get_port_ofport(self._peer.port.name), + actions='mod_vlan_vid:0x%x,' + 'resubmit(,%d)' % ( + self._peer.port.vlan_tag, + ovs_consts.LOCAL_SWITCHING) + ) + self.bridge.add_flow( + table=ovs_consts.TRANSIENT_TABLE, + priority=4, + dl_vlan='0x%x' % self._peer.port.vlan_tag, + actions='strip_vlan,normal' + ) + + def set_peer_port_as_vm_port(self): + """Remove flows simulating traffic from tunneling bridges. + + This method is opposite to set_peer_port_as_patch_port(). + """ + self.bridge.delete_flows( + table=ovs_consts.LOCAL_SWITCHING, + vlan_tci=0, + in_port=self.bridge.get_port_ofport(self._peer.port.name), + ) + self.bridge.delete_flows( + table=ovs_consts.TRANSIENT_TABLE, + dl_vlan='0x%x' % self._peer.port.vlan_tag, + ) class OVSTrunkConnectionTester(OVSBaseConnectionTester): diff --git a/neutron/tests/common/net_helpers.py b/neutron/tests/common/net_helpers.py index 0258434368b..f2fd878051e 100644 --- a/neutron/tests/common/net_helpers.py +++ b/neutron/tests/common/net_helpers.py @@ -737,6 +737,7 @@ class OVSPortFixture(PortFixture): hybrid_plug=False): super(OVSPortFixture, self).__init__(bridge, namespace, mac, port_id) self.hybrid_plug = hybrid_plug + self.vlan_tag = None def _create_bridge_fixture(self): return OVSBridgeFixture() diff --git a/neutron/tests/fullstack/test_securitygroup.py b/neutron/tests/fullstack/test_securitygroup.py index a18754f2606..e89346ac836 100644 --- a/neutron/tests/fullstack/test_securitygroup.py +++ b/neutron/tests/fullstack/test_securitygroup.py @@ -111,6 +111,8 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): 5. a remote security group member addition works, and 6. an established connection stops by deleting a SG rule. 7. test other protocol functionality by using SCTP protocol + 8. test two vms with same mac on the same host in different + networks """ index_to_sg = [0, 0, 1] if self.firewall_driver == 'iptables_hybrid': @@ -272,3 +274,100 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): self.assert_connection( vms[1].namespace, vms[0].namespace, vms[0].ip, 3366, net_helpers.NetcatTester.SCTP) + + # 8. test two vms with same mac on the same host in different networks + self._test_overlapping_mac_addresses() + + def _create_vm_on_host( + self, project_id, network_id, sg_id, host, mac_address=None): + if mac_address: + port = self.safe_client.create_port( + project_id, network_id, host.hostname, + security_groups=[sg_id], mac_address=mac_address) + else: + port = self.safe_client.create_port( + project_id, network_id, host.hostname, + security_groups=[sg_id]) + + return self.useFixture( + machine.FakeFullstackMachine( + host, network_id, project_id, self.safe_client, + neutron_port=port)) + + def _create_three_vms_first_has_static_mac( + self, project_id, allowed_port, subnet_cidr): + """Create three vms. + + First VM has a static mac and is placed on first host. Second VM is + placed on the first host and third VM is placed on second host. + """ + network = self.safe_client.create_network(project_id) + self.safe_client.create_subnet( + project_id, network['id'], subnet_cidr) + sg = self.safe_client.create_security_group(project_id) + + self.safe_client.create_security_group_rule( + project_id, sg['id'], + direction='ingress', + ethertype=constants.IPv4, + protocol=constants.PROTO_NAME_TCP, + port_range_min=allowed_port, port_range_max=allowed_port) + + vms = [self._create_vm_on_host( + project_id, network['id'], sg['id'], self.environment.hosts[0], + mac_address="fa:16:3e:de:ad:fe")] + + if self.firewall_driver == 'iptables_hybrid': + # iptables lack isolation between agents, use only a single host + vms.extend([ + self._create_vm_on_host( + project_id, network['id'], sg['id'], + self.environment.hosts[0]) + for _ in range(2)]) + else: + vms.extend([ + self._create_vm_on_host( + project_id, network['id'], sg['id'], host) + for host in self.environment.hosts[:2]]) + + map(lambda vm: vm.block_until_boot(), vms) + return vms + + def verify_connectivity_between_vms(self, src_vm, dst_vm, protocol, port): + self.assert_connection( + src_vm.namespace, dst_vm.namespace, dst_vm.ip, port, + protocol) + + def verify_no_connectivity_between_vms( + self, src_vm, dst_vm, protocol, port): + self.assert_no_connection( + src_vm.namespace, dst_vm.namespace, dst_vm.ip, port, protocol) + + def _test_overlapping_mac_addresses(self): + project1 = uuidutils.generate_uuid() + p1_allowed = 4444 + + project2 = uuidutils.generate_uuid() + p2_allowed = 4445 + + p1_vms = self._create_three_vms_first_has_static_mac( + project1, p1_allowed, '20.0.2.0/24') + p2_vms = self._create_three_vms_first_has_static_mac( + project2, p2_allowed, '20.0.3.0/24') + + have_connectivity = [ + (p1_vms[0], p1_vms[1], p1_allowed), + (p1_vms[1], p1_vms[2], p1_allowed), + (p2_vms[0], p2_vms[1], p2_allowed), + (p2_vms[1], p2_vms[2], p2_allowed), + ] + + for vm1, vm2, port in have_connectivity: + self.verify_connectivity_between_vms( + vm1, vm2, net_helpers.NetcatTester.TCP, port) + self.verify_connectivity_between_vms( + vm2, vm1, net_helpers.NetcatTester.TCP, port) + self.verify_no_connectivity_between_vms( + vm1, vm2, net_helpers.NetcatTester.TCP, port + 1) + self.verify_no_connectivity_between_vms( + vm2, vm1, net_helpers.NetcatTester.TCP, port + 1) diff --git a/neutron/tests/functional/agent/test_firewall.py b/neutron/tests/functional/agent/test_firewall.py index 349d54eca25..aa8d8707d3e 100644 --- a/neutron/tests/functional/agent/test_firewall.py +++ b/neutron/tests/functional/agent/test_firewall.py @@ -111,6 +111,8 @@ class BaseFirewallTestCase(base.BaseSudoTestCase): # FIXME(jlibosva): We should consider to call prepare_port_filter with # deferred bridge depending on its performance self.firewall.prepare_port_filter(self.src_port_desc) + # Traffic coming from patch-port is always VLAN tagged + self.tester.set_peer_port_as_patch_port() def initialize_iptables(self): cfg.CONF.set_override('enable_ipset', self.enable_ipset, @@ -542,6 +544,8 @@ class FirewallTestCase(BaseFirewallTestCase): self.assertEqual(packets_received, 0) def test_remote_security_groups(self): + self.tester.set_peer_port_as_vm_port() + remote_sg_id = 'remote_sg_id' peer_port_desc = self._create_port_description( self.tester.peer_port_id, 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 f37ff7dbf3b..153965a5d00 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -476,6 +476,7 @@ class TestOVSFirewallDriver(base.BaseTestCase): self.port_ofport, TESTING_VLAN_TAG, ovs_consts.BASE_INGRESS_TABLE), dl_dst=self.port_mac, + dl_vlan='0x%x' % TESTING_VLAN_TAG, priority=90, table=ovs_consts.TRANSIENT_TABLE) filter_rule = mock.call( @@ -498,8 +499,10 @@ class TestOVSFirewallDriver(base.BaseTestCase): 'security_groups': [1], 'port_security_enabled': False} self._prepare_security_group() - self.firewall.prepare_port_filter(port_dict) - self.assertFalse(self.mock_bridge.br.add_flow.called) + with mock.patch.object( + self.firewall, 'initialize_port_flows') as m_init_flows: + self.firewall.prepare_port_filter(port_dict) + self.assertFalse(m_init_flows.called) def test_prepare_port_filter_initialized_port(self): port_dict = {'device': 'port-id', @@ -602,3 +605,51 @@ class TestOVSFirewallDriver(base.BaseTestCase): self.firewall._cleanup_stale_sg() sg_removed_mock.assert_called_once_with(1) delete_sg_mock.assert_called_once_with(1) + + def test_get_ovs_port(self): + ovs_port = self.firewall.get_ovs_port('port_id') + self.assertEqual(self.fake_ovs_port, ovs_port) + + def test_get_ovs_port_non_existent(self): + self.mock_bridge.br.get_vif_port_by_id.return_value = None + with testtools.ExpectedException(exceptions.OVSFWPortNotFound): + self.firewall.get_ovs_port('port_id') + + def test__initialize_egress_no_port_security_sends_to_egress(self): + self.mock_bridge.br.db_get_val.return_value = {'tag': TESTING_VLAN_TAG} + self.firewall._initialize_egress_no_port_security('port_id') + expected_call = mock.call( + table=ovs_consts.TRANSIENT_TABLE, + priority=100, + in_port=self.fake_ovs_port.ofport, + actions='set_field:%d->reg%d,' + 'set_field:%d->reg%d,' + 'resubmit(,%d)' % ( + self.fake_ovs_port.ofport, + ovsfw_consts.REG_PORT, + TESTING_VLAN_TAG, + ovsfw_consts.REG_NET, + ovs_consts.ACCEPT_OR_INGRESS_TABLE) + ) + calls = self.mock_bridge.br.add_flow.call_args_list + self.assertIn(expected_call, calls) + + def test__initialize_egress_no_port_security_no_tag(self): + self.mock_bridge.br.db_get_val.return_value = {} + self.firewall._initialize_egress_no_port_security('port_id') + self.assertFalse(self.mock_bridge.br.add_flow.called) + + def test__remove_egress_no_port_security_deletes_flow(self): + self.mock_bridge.br.db_get_val.return_value = {'tag': TESTING_VLAN_TAG} + self.firewall._remove_egress_no_port_security('port_id') + expected_call = mock.call( + table=ovs_consts.TRANSIENT_TABLE, + in_port=self.fake_ovs_port.ofport, + ) + calls = self.mock_bridge.br.delete_flows.call_args_list + self.assertIn(expected_call, calls) + + def test__remove_egress_no_port_security_no_tag(self): + self.mock_bridge.br.db_get_val.return_value = {} + self.firewall._remove_egress_no_port_security('port_id') + self.assertFalse(self.mock_bridge.br.delete_flows.called)