openstack
/
neutron
Code Issues Proposed changes

OVS: Add mac spoofing filtering to flows 

The mac-spoofing filtering done by iptables was
not adequate. See the bug report and change
I39dc0e23fc118ede19ef2d986b29fc5a8e48ff78 for
more information.

This patch adds flows to the OVS agent to block
any traffic from the VM that isn't in the allowed
address pairs macs or the mac address field of
the port.

Conflicts:
    This had to be heavily modified to work with Kilo due to
    the lack of a pluggable openflow interface that was present
    in master, mitaka, and liberty.

Closes-Bug: #1558658
Change-Id: I02984b21872e0f183db7404c10d8180dbd89075f
(cherry-picked from 997d7b03fb)
This commit is contained in:
Kevin Benton 2016-03-28 22:21:53 -07:00
parent d626eb24a3
commit d48147b0fc
4 changed files with 84 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
View File

@ -734,20 +734,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
@ -756,9 +758,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.
@ -771,9 +778,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,
@ -787,6 +798,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.
@ -1752,6 +1791,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)

3
neutron/plugins/openvswitch/common/constants.py
View File

@ -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'

32
neutron/tests/functional/agent/test_ovs_flows.py
View File

@ -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)

11
neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py
View File

@ -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):
@ -1165,6 +1166,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)])
@ -1183,18 +1185,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}