From 74295ed4d0295687919f00b2085893b57a4310f0 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Fri, 9 May 2014 18:03:48 -0700 Subject: [PATCH] Use ebtables to isolate dhcp traffic Iptables doesn't properly block the broadcast traffic crossing the bridge, so use ebtables instead. Removes test which is no longer valid since we are not using iptables anymore. Conflicts: nova/tests/network/test_linux_net.py Change-Id: I43e5f1fe9512dd3ec9595c7203bc46837cef3cad Closes-Bug: #1318104 (cherry picked from commit 1236b09076cca3b4b16538b055e52edde5a4feea) --- nova/network/linux_net.py | 63 +++---------- nova/tests/network/test_linux_net.py | 132 ++++----------------------- 2 files changed, 30 insertions(+), 165 deletions(-) diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 9440bfc1d35f..706a52aad90f 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -1638,29 +1638,14 @@ def isolate_dhcp_address(interface, address): % (interface, address)) rules.append('OUTPUT -p ARP -o %s --arp-ip-src %s -j DROP' % (interface, address)) + rules.append('FORWARD -p IPv4 -i %s --ip-protocol udp ' + '--ip-destination-port 67:68 -j DROP' + % interface) + rules.append('FORWARD -p IPv4 -o %s --ip-protocol udp ' + '--ip-destination-port 67:68 -j DROP' + % interface) # NOTE(vish): the above is not possible with iptables/arptables ensure_ebtables_rules(rules) - # block dhcp broadcast traffic across the interface - ipv4_filter = iptables_manager.ipv4['filter'] - ipv4_filter.add_rule('FORWARD', - ('-m physdev --physdev-in %s -d 255.255.255.255 ' - '-p udp --dport 67 -j %s' - % (interface, CONF.iptables_drop_action)), - top=True) - ipv4_filter.add_rule('FORWARD', - ('-m physdev --physdev-out %s -d 255.255.255.255 ' - '-p udp --dport 67 -j %s' - % (interface, CONF.iptables_drop_action)), - top=True) - # block ip traffic to address across the interface - ipv4_filter.add_rule('FORWARD', - ('-m physdev --physdev-in %s -d %s -j %s' - % (interface, address, CONF.iptables_drop_action)), - top=True) - ipv4_filter.add_rule('FORWARD', - ('-m physdev --physdev-out %s -s %s -j %s' - % (interface, address, CONF.iptables_drop_action)), - top=True) def remove_isolate_dhcp_address(interface, address): @@ -1670,38 +1655,14 @@ def remove_isolate_dhcp_address(interface, address): % (interface, address)) rules.append('OUTPUT -p ARP -o %s --arp-ip-src %s -j DROP' % (interface, address)) + rules.append('FORWARD -p IPv4 -i %s --ip-protocol udp ' + '--ip-destination-port 67:68 -j DROP' + % interface) + rules.append('FORWARD -p IPv4 -o %s --ip-protocol udp ' + '--ip-destination-port 67:68 -j DROP' + % interface) remove_ebtables_rules(rules) # NOTE(vish): the above is not possible with iptables/arptables - # block dhcp broadcast traffic across the interface - ipv4_filter = iptables_manager.ipv4['filter'] - - drop_actions = ['DROP'] - if CONF.iptables_drop_action != 'DROP': - drop_actions.append(CONF.iptables_drop_action) - - for drop_action in drop_actions: - ipv4_filter.remove_rule('FORWARD', - ('-m physdev --physdev-in %s ' - '-d 255.255.255.255 ' - '-p udp --dport 67 -j %s' - % (interface, drop_action)), - top=True) - ipv4_filter.remove_rule('FORWARD', - ('-m physdev --physdev-out %s ' - '-d 255.255.255.255 ' - '-p udp --dport 67 -j %s' - % (interface, drop_action)), - top=True) - - # block ip traffic to address across the interface - ipv4_filter.remove_rule('FORWARD', - ('-m physdev --physdev-in %s -d %s -j %s' - % (interface, address, drop_action)), - top=True) - ipv4_filter.remove_rule('FORWARD', - ('-m physdev --physdev-out %s -s %s -j %s' - % (interface, address, drop_action)), - top=True) def get_gateway_rules(bridge): diff --git a/nova/tests/network/test_linux_net.py b/nova/tests/network/test_linux_net.py index 9ab75fede35f..c391e6a718ac 100644 --- a/nova/tests/network/test_linux_net.py +++ b/nova/tests/network/test_linux_net.py @@ -634,13 +634,9 @@ class LinuxNetworkTestCase(test.NoDBTestCase): linux_net.IptablesManager()) self.stubs.Set(linux_net, 'binary_name', 'test') executes = [] - inputs = [] def fake_execute(*args, **kwargs): executes.append(args) - process_input = kwargs.get('process_input') - if process_input: - inputs.append(process_input) return "", "" self.stubs.Set(utils, 'execute', fake_execute) @@ -669,27 +665,26 @@ class LinuxNetworkTestCase(test.NoDBTestCase): iface, '--arp-ip-src', dhcp, '-j', 'DROP'), ('ebtables', '-t', 'filter', '-I', 'OUTPUT', '-p', 'ARP', '-o', iface, '--arp-ip-src', dhcp, '-j', 'DROP'), + ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-i', + iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', + '-j', 'DROP'), + ('ebtables', '-t', 'filter', '-I', 'FORWARD', '-p', 'IPv4', '-i', + iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', + '-j', 'DROP'), + ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-o', + iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', + '-j', 'DROP'), + ('ebtables', '-t', 'filter', '-I', 'FORWARD', '-p', 'IPv4', '-o', + iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', + '-j', 'DROP'), ('iptables-save', '-c'), ('iptables-restore', '-c'), ('ip6tables-save', '-c'), ('ip6tables-restore', '-c'), ] self.assertEqual(executes, expected) - expected_inputs = [ - '-A test-FORWARD -m physdev --physdev-in %s ' - '-d 255.255.255.255 -p udp --dport 67 -j DROP' % iface, - '-A test-FORWARD -m physdev --physdev-out %s ' - '-d 255.255.255.255 -p udp --dport 67 -j DROP' % iface, - '-A test-FORWARD -m physdev --physdev-in %s ' - '-d 192.168.1.1 -j DROP' % iface, - '-A test-FORWARD -m physdev --physdev-out %s ' - '-s 192.168.1.1 -j DROP' % iface, - ] - for inp in expected_inputs: - self.assertIn(inp, inputs[0]) executes = [] - inputs = [] @staticmethod def fake_remove(bridge, gateway): @@ -704,105 +699,14 @@ class LinuxNetworkTestCase(test.NoDBTestCase): iface, '--arp-ip-dst', dhcp, '-j', 'DROP'), ('ebtables', '-t', 'filter', '-D', 'OUTPUT', '-p', 'ARP', '-o', iface, '--arp-ip-src', dhcp, '-j', 'DROP'), - ('iptables-save', '-c'), - ('iptables-restore', '-c'), - ('ip6tables-save', '-c'), - ('ip6tables-restore', '-c'), + ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-i', + iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', + '-j', 'DROP'), + ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-o', + iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', + '-j', 'DROP'), ] self.assertEqual(executes, expected) - for inp in expected_inputs: - self.assertNotIn(inp, inputs[0]) - - def test_isolated_host_iptables_logdrop(self): - # Ensure that a different drop action for iptables doesn't change - # the drop action for ebtables. - self.flags(fake_network=False, - share_dhcp_address=True, - iptables_drop_action='LOGDROP') - - # NOTE(vish): use a fresh copy of the manager for each test - self.stubs.Set(linux_net, 'iptables_manager', - linux_net.IptablesManager()) - self.stubs.Set(linux_net, 'binary_name', 'test') - executes = [] - inputs = [] - - def fake_execute(*args, **kwargs): - executes.append(args) - process_input = kwargs.get('process_input') - if process_input: - inputs.append(process_input) - return "", "" - - self.stubs.Set(utils, 'execute', fake_execute) - - driver = linux_net.LinuxBridgeInterfaceDriver() - - @staticmethod - def fake_ensure(bridge, interface, network, gateway): - return bridge - - self.stubs.Set(linux_net.LinuxBridgeInterfaceDriver, - 'ensure_bridge', fake_ensure) - - iface = 'eth0' - dhcp = '192.168.1.1' - network = {'dhcp_server': dhcp, - 'bridge': 'br100', - 'bridge_interface': iface} - driver.plug(network, 'fakemac') - expected = [ - ('ebtables', '-t', 'filter', '-D', 'INPUT', '-p', 'ARP', '-i', - iface, '--arp-ip-dst', dhcp, '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-I', 'INPUT', '-p', 'ARP', '-i', - iface, '--arp-ip-dst', dhcp, '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-D', 'OUTPUT', '-p', 'ARP', '-o', - iface, '--arp-ip-src', dhcp, '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-I', 'OUTPUT', '-p', 'ARP', '-o', - iface, '--arp-ip-src', dhcp, '-j', 'DROP'), - ('iptables-save', '-c'), - ('iptables-restore', '-c'), - ('ip6tables-save', '-c'), - ('ip6tables-restore', '-c'), - ] - self.assertEqual(executes, expected) - expected_inputs = [ - ('-A test-FORWARD -m physdev --physdev-in %s ' - '-d 255.255.255.255 -p udp --dport 67 -j LOGDROP' % iface), - ('-A test-FORWARD -m physdev --physdev-out %s ' - '-d 255.255.255.255 -p udp --dport 67 -j LOGDROP' % iface), - ('-A test-FORWARD -m physdev --physdev-in %s ' - '-d 192.168.1.1 -j LOGDROP' % iface), - ('-A test-FORWARD -m physdev --physdev-out %s ' - '-s 192.168.1.1 -j LOGDROP' % iface), - ] - for inp in expected_inputs: - self.assertIn(inp, inputs[0]) - - executes = [] - inputs = [] - - @staticmethod - def fake_remove(bridge, gateway): - return - - self.stubs.Set(linux_net.LinuxBridgeInterfaceDriver, - 'remove_bridge', fake_remove) - - driver.unplug(network) - expected = [ - ('ebtables', '-t', 'filter', '-D', 'INPUT', '-p', 'ARP', '-i', - iface, '--arp-ip-dst', dhcp, '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-D', 'OUTPUT', '-p', 'ARP', '-o', - iface, '--arp-ip-src', dhcp, '-j', 'DROP'), - ('iptables-save', '-c'), - ('iptables-restore', '-c'), - ('ip6tables-save', '-c'), - ('ip6tables-restore', '-c'), - ] - self.assertEqual(executes, expected) - for inp in expected_inputs: - self.assertNotIn(inp, inputs[0]) def _test_initialize_gateway(self, existing, expected, routes=''): self.flags(fake_network=False)