Merge "Move Linuxbridge ARP spoofing to nat table PREROUTING chain"
This commit is contained in:
Zuul
Gerrit Code Review
commit
b354321399
@ -70,26 +70,33 @@ def chain_name(vif):
|
||||
@lockutils.synchronized('ebtables')
|
||||
def delete_arp_spoofing_protection(vifs):
|
||||
current_rules = ebtables(['-L']).splitlines()
|
||||
_delete_arp_spoofing_protection(vifs, current_rules)
|
||||
_delete_arp_spoofing_protection(vifs, current_rules, table='nat',
|
||||
chain='PREROUTING')
|
||||
|
||||
# TODO(haleyb) this can go away in "R" cycle, it's here to cleanup
|
||||
# old chains in the filter table
|
||||
current_rules = ebtables(['-L'], table='filter').splitlines()
|
||||
_delete_arp_spoofing_protection(vifs, current_rules, table='filter',
|
||||
chain='FORWARD')
|
||||
|
||||
|
||||
def _delete_arp_spoofing_protection(vifs, current_rules):
|
||||
def _delete_arp_spoofing_protection(vifs, current_rules, table, chain):
|
||||
# delete the jump rule and then delete the whole chain
|
||||
jumps = [vif for vif in vifs if vif_jump_present(vif, current_rules)]
|
||||
for vif in jumps:
|
||||
ebtables(['-D', 'FORWARD', '-i', vif, '-j',
|
||||
chain_name(vif), '-p', 'ARP'])
|
||||
ebtables(['-D', chain, '-i', vif, '-j',
|
||||
chain_name(vif), '-p', 'ARP'], table=table)
|
||||
for vif in vifs:
|
||||
if chain_exists(chain_name(vif), current_rules):
|
||||
ebtables(['-X', chain_name(vif)])
|
||||
_delete_mac_spoofing_protection(vifs, current_rules)
|
||||
ebtables(['-X', chain_name(vif)], table=table)
|
||||
_delete_mac_spoofing_protection(vifs, current_rules, table=table,
|
||||
chain=chain)
|
||||
|
||||
|
||||
@lockutils.synchronized('ebtables')
|
||||
def delete_unreferenced_arp_protection(current_vifs):
|
||||
def _delete_unreferenced_arp_protection(current_vifs, table, chain):
|
||||
# deletes all jump rules and chains that aren't in current_vifs but match
|
||||
# the spoof prefix
|
||||
current_rules = ebtables(['-L']).splitlines()
|
||||
current_rules = ebtables(['-L'], table=table).splitlines()
|
||||
to_delete = []
|
||||
for line in current_rules:
|
||||
# we're looking to find and turn the following:
|
||||
@ -101,7 +108,19 @@ def delete_unreferenced_arp_protection(current_vifs):
|
||||
to_delete.append(devname)
|
||||
LOG.info("Clearing orphaned ARP spoofing entries for devices %s",
|
||||
to_delete)
|
||||
_delete_arp_spoofing_protection(to_delete, current_rules)
|
||||
_delete_arp_spoofing_protection(to_delete, current_rules, table=table,
|
||||
chain=chain)
|
||||
|
||||
|
||||
@lockutils.synchronized('ebtables')
|
||||
def delete_unreferenced_arp_protection(current_vifs):
|
||||
_delete_unreferenced_arp_protection(current_vifs,
|
||||
table='nat', chain='PREROUTING')
|
||||
|
||||
# TODO(haleyb) this can go away in "R" cycle, it's here to cleanup
|
||||
# old chains in the filter table
|
||||
_delete_unreferenced_arp_protection(current_vifs,
|
||||
table='filter', chain='FORWARD')
|
||||
|
||||
|
||||
@lockutils.synchronized('ebtables')
|
||||
@ -119,12 +138,12 @@ def _install_arp_spoofing_protection(vif, addresses, current_rules):
|
||||
# packets until the allows are installed, but that's better than leaked
|
||||
# spoofed packets and ARP can handle losses.
|
||||
ebtables(['-F', vif_chain])
|
||||
for addr in addresses:
|
||||
for addr in sorted(addresses):
|
||||
ebtables(['-A', vif_chain, '-p', 'ARP', '--arp-ip-src', addr,
|
||||
'-j', 'ACCEPT'])
|
||||
# check if jump rule already exists, if not, install it
|
||||
if not vif_jump_present(vif, current_rules):
|
||||
ebtables(['-A', 'FORWARD', '-i', vif, '-j',
|
||||
ebtables(['-A', 'PREROUTING', '-i', vif, '-j',
|
||||
vif_chain, '-p', 'ARP'])
|
||||
|
||||
|
||||
@ -155,13 +174,13 @@ def _install_mac_spoofing_protection(vif, port_details, current_rules):
|
||||
ebtables(['-N', vif_chain, '-P', 'DROP'])
|
||||
# check if jump rule already exists, if not, install it
|
||||
if not _mac_vif_jump_present(vif, current_rules):
|
||||
ebtables(['-A', 'FORWARD', '-i', vif, '-j', vif_chain])
|
||||
ebtables(['-A', 'PREROUTING', '-i', vif, '-j', vif_chain])
|
||||
# we can't just feed all allowed macs at once because we can exceed
|
||||
# the maximum argument size. limit to 500 per rule.
|
||||
for chunk in (mac_addresses[i:i + 500]
|
||||
for i in range(0, len(mac_addresses), 500)):
|
||||
new_rule = ['-A', vif_chain, '-i', vif,
|
||||
'--among-src', ','.join(chunk), '-j', 'RETURN']
|
||||
'--among-src', ','.join(sorted(chunk)), '-j', 'RETURN']
|
||||
ebtables(new_rule)
|
||||
_delete_vif_mac_rules(vif, current_rules)
|
||||
|
||||
@ -185,17 +204,17 @@ def _delete_vif_mac_rules(vif, current_rules):
|
||||
ebtables(['-D', chain] + rule.split())
|
||||
|
||||
|
||||
def _delete_mac_spoofing_protection(vifs, current_rules):
|
||||
def _delete_mac_spoofing_protection(vifs, current_rules, table, chain):
|
||||
# delete the jump rule and then delete the whole chain
|
||||
jumps = [vif for vif in vifs
|
||||
if _mac_vif_jump_present(vif, current_rules)]
|
||||
for vif in jumps:
|
||||
ebtables(['-D', 'FORWARD', '-i', vif, '-j',
|
||||
_mac_chain_name(vif)])
|
||||
ebtables(['-D', chain, '-i', vif, '-j',
|
||||
_mac_chain_name(vif)], table=table)
|
||||
for vif in vifs:
|
||||
chain = _mac_chain_name(vif)
|
||||
if chain_exists(chain, current_rules):
|
||||
ebtables(['-X', chain])
|
||||
ebtables(['-X', chain], table=table)
|
||||
|
||||
|
||||
# Used to scope ebtables commands in testing
|
||||
@ -207,6 +226,7 @@ NAMESPACE = None
|
||||
retry=tenacity.retry_if_exception(lambda e: e.returncode == 255),
|
||||
reraise=True
|
||||
)
|
||||
def ebtables(comm):
|
||||
def ebtables(comm, table='nat'):
|
||||
execute = ip_lib.IPWrapper(NAMESPACE).netns.execute
|
||||
return execute(['ebtables', '--concurrent'] + comm, run_as_root=True)
|
||||
return execute(['ebtables', '-t', table, '--concurrent'] + comm,
|
||||
run_as_root=True)
|
||||
|
||||
@ -0,0 +1,168 @@
|
||||
# Copyright (c) 2018 Red Hat, Inc.
|
||||
# All Rights Reserved.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||
# you may not use this file except in compliance with the License.
|
||||
# You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS,
|
||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
import mock
|
||||
from neutron_lib import constants
|
||||
|
||||
from neutron.agent.common import utils
|
||||
from neutron.plugins.ml2.drivers.linuxbridge.agent import arp_protect
|
||||
from neutron.tests import base
|
||||
|
||||
|
||||
VIF = 'vif_tap0'
|
||||
PORT_NO_SEC = {'port_security_enabled': False}
|
||||
PORT_TRUSTED = {'device_owner': constants.DEVICE_OWNER_ROUTER_GW}
|
||||
PORT = {'fixed_ips': [{'ip_address': '10.1.1.1'}],
|
||||
'device_owner': 'nobody',
|
||||
'mac_address': '00:11:22:33:44:55'}
|
||||
PORT_ADDR_PAIR = {'fixed_ips': [{'ip_address': '10.1.1.1'}],
|
||||
'device_owner': 'nobody',
|
||||
'mac_address': '00:11:22:33:44:55',
|
||||
'allowed_address_pairs': [
|
||||
{'mac_address': '00:11:22:33:44:66',
|
||||
'ip_address': '10.1.1.2'}]}
|
||||
|
||||
|
||||
class TestLinuxBridgeARPSpoofing(base.BaseTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(TestLinuxBridgeARPSpoofing, self).setUp()
|
||||
self.execute = mock.patch.object(utils, "execute").start()
|
||||
|
||||
@mock.patch.object(arp_protect, "delete_arp_spoofing_protection")
|
||||
def test_port_no_security(self, dasp):
|
||||
arp_protect.setup_arp_spoofing_protection(VIF, PORT_NO_SEC)
|
||||
dasp.assert_called_with([VIF])
|
||||
|
||||
@mock.patch.object(arp_protect, "delete_arp_spoofing_protection")
|
||||
def test_port_trusted(self, dasp):
|
||||
arp_protect.setup_arp_spoofing_protection(VIF, PORT_TRUSTED)
|
||||
dasp.assert_called_with([VIF])
|
||||
|
||||
def _test_port_add_arp_spoofing(self, vif, port):
|
||||
mac_addresses = {port['mac_address']}
|
||||
ip_addresses = {p['ip_address'] for p in port['fixed_ips']}
|
||||
if port.get('allowed_address_pairs'):
|
||||
mac_addresses |= {p['mac_address']
|
||||
for p in port['allowed_address_pairs']}
|
||||
ip_addresses |= {p['ip_address']
|
||||
for p in port['allowed_address_pairs']}
|
||||
spoof_chain = arp_protect.SPOOF_CHAIN_PREFIX + vif
|
||||
mac_chain = arp_protect.MAC_CHAIN_PREFIX + vif
|
||||
|
||||
expected = [
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-L'],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.ANY,
|
||||
mock.ANY,
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-N',
|
||||
'neutronMAC-%s' % vif, '-P', 'DROP'],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.ANY,
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-A',
|
||||
'PREROUTING', '-i', vif, '-j', mac_chain],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-A',
|
||||
mac_chain, '-i', vif,
|
||||
'--among-src', '%s' % ','.join(sorted(mac_addresses)),
|
||||
'-j', 'RETURN'],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.ANY,
|
||||
mock.ANY,
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-N',
|
||||
spoof_chain, '-P', 'DROP'],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-F',
|
||||
spoof_chain],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
]
|
||||
for addr in sorted(ip_addresses):
|
||||
expected.extend([
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-A',
|
||||
spoof_chain, '-p', 'ARP',
|
||||
'--arp-ip-src', addr, '-j', 'ACCEPT'],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
])
|
||||
expected.extend([
|
||||
mock.ANY,
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-A',
|
||||
'PREROUTING', '-i', vif, '-j',
|
||||
spoof_chain, '-p', 'ARP'],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
])
|
||||
|
||||
arp_protect.setup_arp_spoofing_protection(vif, port)
|
||||
self.execute.assert_has_calls(expected)
|
||||
|
||||
def test_port_add_arp_spoofing(self):
|
||||
self._test_port_add_arp_spoofing(VIF, PORT)
|
||||
|
||||
def test_port_add_arp_spoofing_addr_pair(self):
|
||||
self._test_port_add_arp_spoofing(VIF, PORT_ADDR_PAIR)
|
||||
|
||||
@mock.patch.object(arp_protect, "chain_exists", return_value=True)
|
||||
@mock.patch.object(arp_protect, "vif_jump_present", return_value=True)
|
||||
def test_port_delete_arp_spoofing(self, ce, vjp):
|
||||
spoof_chain = arp_protect.SPOOF_CHAIN_PREFIX + VIF
|
||||
mac_chain = arp_protect.MAC_CHAIN_PREFIX + VIF
|
||||
expected = [
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-L'],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.ANY,
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-D',
|
||||
'PREROUTING', '-i', VIF, '-j', spoof_chain,
|
||||
'-p', 'ARP'],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-X',
|
||||
spoof_chain],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.ANY,
|
||||
mock.call(['ebtables', '-t', 'nat', '--concurrent', '-X',
|
||||
mac_chain],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.call(['ebtables', '-t', 'filter', '--concurrent', '-L'],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.ANY,
|
||||
mock.call(['ebtables', '-t', 'filter', '--concurrent', '-D',
|
||||
'FORWARD', '-i', VIF, '-j', spoof_chain,
|
||||
'-p', 'ARP'],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.call(['ebtables', '-t', 'filter', '--concurrent', '-X',
|
||||
spoof_chain],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
mock.ANY,
|
||||
mock.call(['ebtables', '-t', 'filter', '--concurrent', '-X',
|
||||
mac_chain],
|
||||
check_exit_code=True, extra_ok_codes=None,
|
||||
log_fail_as_error=True, run_as_root=True),
|
||||
]
|
||||
|
||||
arp_protect.delete_arp_spoofing_protection([VIF])
|
||||
self.execute.assert_has_calls(expected)
|
||||
Loading…
Reference in New Issue
Block a user