diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index f91b11df9e7..a514833a3a2 100644 --- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py @@ -738,20 +738,22 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, if port.ofport != -1: self.int_br.delete_flows(in_port=port.ofport) - @staticmethod - def setup_arp_spoofing_protection(bridge, vif, port_details): + @classmethod + def setup_arp_spoofing_protection(cls, bridge, vif, port_details): # clear any previous flows related to this port in our ARP table bridge.delete_flows(table=constants.ARP_SPOOF_TABLE, in_port=vif.ofport) if not port_details.get('port_security_enabled', True): bridge.delete_flows(table=constants.LOCAL_SWITCHING, in_port=vif.ofport, proto='arp') + cls.set_allowed_macs_for_port(bridge, vif.ofport, allow_all=True) LOG.info(_LI("Skipping ARP spoofing rules for port '%s' because " "it has port security disabled"), vif.port_name) return if port_details['device_owner'].startswith('network:'): bridge.delete_flows(table=constants.LOCAL_SWITCHING, in_port=vif.ofport, proto='arp') + cls.set_allowed_macs_for_port(bridge, vif.ofport, allow_all=True) LOG.debug("Skipping ARP spoofing rules for network owned port " "'%s'.", vif.port_name) return @@ -760,9 +762,14 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, # collect all of the addresses and cidrs that belong to the port addresses = [f['ip_address'] for f in port_details['fixed_ips']] + mac_addresses = {vif.vif_mac} if port_details.get('allowed_address_pairs'): addresses += [p['ip_address'] for p in port_details['allowed_address_pairs']] + mac_addresses |= {p['mac_address'] + for p in port_details['allowed_address_pairs']} + + cls.set_allowed_macs_for_port(bridge, vif.ofport, mac_addresses) if any(netaddr.IPNetwork(ip).prefixlen == 0 for ip in addresses): # don't try to install protection because a /0 prefix allows any # address anyway and the ARP_SPA can only match on /1 or more. @@ -775,9 +782,13 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, for ip in addresses: if netaddr.IPNetwork(ip).version != 4: continue + if cfg.CONF.AGENT.enable_mac_spoofing_protection: + action = "resubmit(,%s)" % constants.MAC_SPOOF_TABLE + else: + action = "NORMAL" bridge.add_flow(table=constants.ARP_SPOOF_TABLE, priority=2, proto='arp', arp_spa=ip, in_port=vif.ofport, - actions="NORMAL") + actions=action) # drop any ARPs in this table that aren't explicitly allowed bridge.add_flow(table=constants.ARP_SPOOF_TABLE, priority=1, @@ -791,6 +802,34 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, priority=10, proto='arp', in_port=vif.ofport, actions=("resubmit(,%s)" % constants.ARP_SPOOF_TABLE)) + @staticmethod + def set_allowed_macs_for_port(br, port, mac_addresses=None, + allow_all=False): + if not cfg.CONF.AGENT.enable_mac_spoofing_protection: + return + if allow_all: + br.delete_flows(table=constants.LOCAL_SWITCHING, in_port=port) + br.delete_flows(table=constants.MAC_SPOOF_TABLE, in_port=port) + return + mac_addresses = mac_addresses or [] + for address in mac_addresses: + br.add_flow(table=constants.MAC_SPOOF_TABLE, priority=2, + eth_src=address, in_port=port, + actions="NORMAL") + # normalize so we can see if macs are the same + mac_addresses = {netaddr.EUI(mac) for mac in mac_addresses} + flows = br.dump_flows_for_table(constants.MAC_SPOOF_TABLE).splitlines() + for flow in flows: + if 'dl_src' not in flow or 'in_port=%s' % port not in flow: + continue + flow_mac = flow.split('dl_src=')[1].split(' ')[0].split(',')[0] + if netaddr.EUI(flow_mac) not in mac_addresses: + br.delete_flows(table=constants.MAC_SPOOF_TABLE, + in_port=port, eth_src=flow_mac) + br.add_flow(table=constants.LOCAL_SWITCHING, + priority=9, in_port=port, + actions=("resubmit(,%s)" % constants.MAC_SPOOF_TABLE)) + def port_unbound(self, vif_id, net_uuid=None): '''Unbind port. @@ -1757,6 +1796,7 @@ def create_agent_config_map(config): def main(): cfg.CONF.register_opts(ip_lib.OPTS) config.register_root_helper(cfg.CONF) + config.register_spoofing_opts(cfg.CONF) common_config.init(sys.argv[1:]) common_config.setup_logging() q_utils.log_opt_values(LOG) diff --git a/neutron/plugins/openvswitch/common/constants.py b/neutron/plugins/openvswitch/common/constants.py index 40fa8f0f07f..425cf60885c 100644 --- a/neutron/plugins/openvswitch/common/constants.py +++ b/neutron/plugins/openvswitch/common/constants.py @@ -62,6 +62,9 @@ CANARY_TABLE = 23 # Table for ARP poison/spoofing prevention rules ARP_SPOOF_TABLE = 24 +# Table for MAC spoof filtering +MAC_SPOOF_TABLE = 25 + # type for ARP reply in ARP header ARP_REPLY = '0x2' diff --git a/neutron/tests/functional/agent/test_ovs_flows.py b/neutron/tests/functional/agent/test_ovs_flows.py index cccdac90f39..ebaefa07461 100644 --- a/neutron/tests/functional/agent/test_ovs_flows.py +++ b/neutron/tests/functional/agent/test_ovs_flows.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +from oslo_config import cfg + from neutron.agent.linux import ip_lib from neutron.cmd.sanity import checks from neutron.plugins.openvswitch.agent import ovs_neutron_agent as ovsagt @@ -52,6 +54,26 @@ class ARPSpoofTestCase(test_ovs_lib.OVSBridgeTestBase, self.dst_p.addr.add('%s/24' % self.dst_addr) self.pinger.assert_ping(self.dst_addr) + def test_mac_spoof_blocks_wrong_mac(self): + self._setup_arp_spoof_for_port(self.src_p.name, [self.src_addr]) + self._setup_arp_spoof_for_port(self.dst_p.name, [self.dst_addr]) + self.src_p.addr.add('%s/24' % self.src_addr) + self.dst_p.addr.add('%s/24' % self.dst_addr) + self.pinger.assert_ping(self.dst_addr) + # changing the allowed mac should stop the port from working + self._setup_arp_spoof_for_port(self.src_p.name, [self.src_addr], + mac='00:11:22:33:44:55') + self.pinger.assert_no_ping(self.dst_addr) + + def test_mac_spoof_disabled_allows_wrong_mac(self): + cfg.CONF.set_override('enable_mac_spoofing_protection', False, 'AGENT') + self._setup_arp_spoof_for_port(self.src_p.name, [self.src_addr], + mac='00:11:22:33:44:55') + self._setup_arp_spoof_for_port(self.dst_p.name, [self.dst_addr]) + self.src_p.addr.add('%s/24' % self.src_addr) + self.dst_p.addr.add('%s/24' % self.dst_addr) + self.pinger.assert_ping(self.dst_addr) + def test_arp_spoof_doesnt_block_ipv6(self): self.src_addr = '2000::1' self.dst_addr = '2000::2' @@ -122,18 +144,24 @@ class ARPSpoofTestCase(test_ovs_lib.OVSBridgeTestBase, self.pinger.assert_ping(self.dst_addr) def _setup_arp_spoof_for_port(self, port, addrs, psec=True, - device_owner='nobody'): + device_owner='nobody', mac=None): of_port_map = self.br.get_vif_port_to_ofport_map() class VifPort(object): ofport = of_port_map[port] port_name = port + # ugly hack to deal with the fact that we can't retrieve macs + # from OVSDB in kilo + vif_mac = mac or ( + self.src_p.link.address if port == self.src_p.name + else self.dst_p.link.address) ip_addr = addrs.pop() details = {'port_security_enabled': psec, 'fixed_ips': [{'ip_address': ip_addr}], 'device_owner': device_owner, 'allowed_address_pairs': [ - dict(ip_address=ip) for ip in addrs]} + dict(ip_address=ip, mac_address=VifPort.vif_mac) + for ip in addrs]} ovsagt.OVSNeutronAgent.setup_arp_spoofing_protection( self.br, VifPort(), details) diff --git a/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py index 565b38ebe28..56728cd115d 100644 --- a/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py @@ -46,6 +46,7 @@ FAKE_IP2 = '10.0.0.2' class FakeVif(object): ofport = 99 port_name = 'name' + vif_mac = '00:22:44:66:88:AA' class CreateAgentConfigMap(base.BaseTestCase): @@ -1185,6 +1186,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): fake_details = {'fixed_ips': [], 'device_owner': 'nobody'} self.agent.prevent_arp_spoofing = True int_br = mock.Mock() + int_br.dump_flows_for_table.return_value = '' self.agent.setup_arp_spoofing_protection(int_br, vif, fake_details) int_br.delete_flows.assert_has_calls( [mock.call(table=mock.ANY, in_port=vif.ofport)]) @@ -1203,18 +1205,21 @@ class TestOvsNeutronAgent(base.BaseTestCase): 'device_owner': 'nobody', 'fixed_ips': [{'ip_address': '192.168.44.100'}, {'ip_address': '192.168.44.101'}], - 'allowed_address_pairs': [{'ip_address': '192.168.44.102/32'}, - {'ip_address': '192.168.44.103/32'}] + 'allowed_address_pairs': [{'ip_address': '192.168.44.102/32', + 'mac_address': FAKE_MAC}, + {'ip_address': '192.168.44.103/32', + 'mac_address': FAKE_MAC}] } self.agent.prevent_arp_spoofing = True int_br = mock.Mock() + int_br.dump_flows_for_table.return_value = '' self.agent.setup_arp_spoofing_protection(int_br, vif, fake_details) # make sure all addresses are allowed for addr in ('192.168.44.100', '192.168.44.101', '192.168.44.102/32', '192.168.44.103/32'): int_br.add_flow.assert_any_call( table=constants.ARP_SPOOF_TABLE, in_port=vif.ofport, - proto='arp', actions='NORMAL', arp_spa=addr, priority=mock.ANY) + proto='arp', actions=mock.ANY, arp_spa=addr, priority=mock.ANY) def test__get_ofport_moves(self): previous = {'port1': 1, 'port2': 2}