From 7a067a97a81148db655d7481ec0eeb5cb57cd7f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harald=20Jens=C3=A5s?= Date: Tue, 22 Sep 2020 21:36:08 +0200 Subject: [PATCH] pxe filter - option to always block unknown hosts It is not always desired to open the DHCP server for any host just because introspection is active. Add an option to ensure that only nodes being introspected are added to the DHCP servers allow list. Also adds ethoib support in the dnsmasq PXE filter. Also fix a typo in ethoib_interfaces option help text. Change-Id: I4cd7f4f0a449dcc23897ec9288cb57ec9bd647d7 --- ironic_inspector/conf/iptables.py | 2 +- ironic_inspector/conf/pxe_filter.py | 9 + ironic_inspector/pxe_filter/base.py | 77 +++++++ ironic_inspector/pxe_filter/dnsmasq.py | 14 +- ironic_inspector/pxe_filter/iptables.py | 95 +++----- .../test/unit/test_dnsmasq_pxe_filter.py | 54 ++++- ironic_inspector/test/unit/test_iptables.py | 205 +++++++----------- ironic_inspector/test/unit/test_pxe_filter.py | 197 +++++++++++++++++ ...ter-eoib-mac-support-7567bbc7c6bf1878.yaml | 6 + ...-unknown-host-option-b84b2aa1f7f49a17.yaml | 17 ++ 10 files changed, 474 insertions(+), 202 deletions(-) create mode 100644 releasenotes/notes/dnsmasq-pxe-filter-eoib-mac-support-7567bbc7c6bf1878.yaml create mode 100644 releasenotes/notes/pxe-filter-add-deny-unknown-host-option-b84b2aa1f7f49a17.yaml diff --git a/ironic_inspector/conf/iptables.py b/ironic_inspector/conf/iptables.py index 7ecf61388..bb6e80b8d 100644 --- a/ironic_inspector/conf/iptables.py +++ b/ironic_inspector/conf/iptables.py @@ -26,7 +26,7 @@ _OPTS = [ help=_('iptables chain name to use.')), cfg.ListOpt('ethoib_interfaces', default=[], - help=_('List of Etherent Over InfiniBand interfaces ' + help=_('List of Ethernet Over InfiniBand interfaces ' 'on the Inspector host which are used for physical ' 'access to the DHCP network. Multiple interfaces would ' 'be attached to a bond or bridge specified in ' diff --git a/ironic_inspector/conf/pxe_filter.py b/ironic_inspector/conf/pxe_filter.py index 2e1d915ad..07af59c56 100644 --- a/ironic_inspector/conf/pxe_filter.py +++ b/ironic_inspector/conf/pxe_filter.py @@ -24,6 +24,15 @@ _OPTS = [ cfg.IntOpt('sync_period', default=15, min=0, help=_('Amount of time in seconds, after which repeat periodic ' 'update of the filter.')), + cfg.BoolOpt('deny_unknown_macs', default=False, + help=_('By default inspector will open the DHCP server for ' + 'any node when introspection is active. Opening DHCP ' + 'for unknown MAC addresses when introspection is ' + 'active allow for users to add nodes with no ports to ' + 'ironic and have ironic-inspector enroll ports based ' + 'on node introspection results. NOTE: If this option ' + 'is True, nodes must have at least one enrolled port ' + 'prior to introspection.')) ] diff --git a/ironic_inspector/pxe_filter/base.py b/ironic_inspector/pxe_filter/base.py index 437eb9342..b0a11fbb3 100644 --- a/ironic_inspector/pxe_filter/base.py +++ b/ironic_inspector/pxe_filter/base.py @@ -15,6 +15,8 @@ import contextlib import functools +import os +import re from automaton import exceptions as automaton_errors from automaton import machines @@ -27,13 +29,17 @@ import stevedore from ironic_inspector.common.i18n import _ from ironic_inspector.common import ironic as ir_utils +from ironic_inspector import node_cache from ironic_inspector.pxe_filter import interface +from ironic_inspector import utils CONF = cfg.CONF LOG = log.getLogger(__name__) _STEVEDORE_DRIVER_NAMESPACE = 'ironic_inspector.pxe_filter' +_EMAC_REGEX = 'EMAC=([0-9a-f]{2}(:[0-9a-f]{2}){5}) IMAC=.*' + class InvalidFilterDriverState(RuntimeError): """The fsm of the filter driver raised an error.""" @@ -96,6 +102,14 @@ class BaseFilter(interface.FilterDriver): def __init__(self): super(BaseFilter, self).__init__() + # Configuration check + if (CONF.pxe_filter.deny_unknown_macs + and CONF.processing.node_not_found_hook): + msg = _('Configuration error: [pxe_filter]deny_unknown_macs is' + 'enabled and [processing]node_not_found_hook is enabled.' + 'These options cannot both be enabled simultaneously.') + raise utils.Error(msg) + self.lock = semaphore.BoundedSemaphore() self.fsm.initialize(start_state=States.uninitialized) @@ -235,3 +249,66 @@ def driver(): :returns: the singleton PXE filter driver object. """ return _driver_manager().driver + + +def _ib_mac_to_rmac_mapping(ports): + """Update port InfiniBand MAC address to EthernetOverInfiniBand MAC + + On InfiniBand deployment we need to map between the baremetal host + InfiniBand MAC to the EoIB MAC. The EoIB MAC addresses are learned + automatically by the EoIB interfaces and those MACs are recorded to the + /sys/class/net//eth/neighs file. The InfiniBand GUID is + taken from the ironic port client-id extra attribute. The InfiniBand GUID + is the last 8 bytes of the client-id. The file format allows to map the + GUID to EoIB MAC. The filter rules based on those MACs get applied during a + driver.update() call + + :param ports: list of ironic ports + :returns: Nothing. + """ + ethoib_interfaces = CONF.iptables.ethoib_interfaces + for iface in ethoib_interfaces: + neighs_file = (os.path.join('/sys/class/net', iface, 'eth/neighs')) + try: + with open(neighs_file, 'r') as fd: + data = fd.read() + except IOError: + LOG.error('Interface %s is not Ethernet Over InfiniBand; ' + 'Skipping ...', iface) + continue + for port in ports: + client_id = port.extra.get('client-id') + if client_id: + # Note(moshele): The last 8 bytes in the client-id is + # the baremetal node InfiniBand GUID + guid = client_id[-23:] + p = re.compile(_EMAC_REGEX + guid) + match = p.search(data) + if match: + port.address = match.group(1) + + +def get_ironic_macs(ironic): + ports = [port for port in + ir_utils.call_with_retries(ironic.ports, limit=None, + fields=['address', 'extra'])] + _ib_mac_to_rmac_mapping(ports) + return {port.address for port in ports} + + +def get_inactive_macs(ironic): + ports = [port for port in + ir_utils.call_with_retries(ironic.ports, limit=None, + fields=['address', 'extra']) + if port.address not in node_cache.active_macs()] + _ib_mac_to_rmac_mapping(ports) + return {port.address for port in ports} + + +def get_active_macs(ironic): + ports = [port for port in + ir_utils.call_with_retries(ironic.ports, limit=None, + fields=['address', 'extra']) + if port.address in node_cache.active_macs()] + _ib_mac_to_rmac_mapping(ports) + return {port.address for port in ports} diff --git a/ironic_inspector/pxe_filter/dnsmasq.py b/ironic_inspector/pxe_filter/dnsmasq.py index 101734c22..0935728ed 100644 --- a/ironic_inspector/pxe_filter/dnsmasq.py +++ b/ironic_inspector/pxe_filter/dnsmasq.py @@ -84,11 +84,11 @@ class DnsmasqFilter(pxe_filter.BaseFilter): timestamp_start = timeutils.utcnow() # active_macs are the MACs for which introspection is active - active_macs = node_cache.active_macs() + active_macs = pxe_filter.get_active_macs(ironic) + # ironic_macs are all the MACs know to ironic (all ironic ports) - ironic_macs = set(port.address for port in - ir_utils.call_with_retries(ironic.ports, limit=None, - fields=['address'])) + ironic_macs = pxe_filter.get_ironic_macs(ironic) + denylist, allowlist = _get_deny_allow_lists() # removedlist are the MACs that are in either in allow or denylist, # but not kept in ironic (ironic_macs) any more @@ -233,7 +233,8 @@ def _configure_removedlist(macs): hostsdir = CONF.dnsmasq_pxe_filter.dhcp_hostsdir - if _should_enable_unknown_hosts(): + if (_should_enable_unknown_hosts() + and not CONF.pxe_filter.deny_unknown_macs): for mac in macs: if os.stat(os.path.join(hostsdir, mac)).st_size != _MAC_ALLOW_LEN: _add_mac_to_allowlist(mac) @@ -253,7 +254,8 @@ def _configure_unknown_hosts(): path = os.path.join(CONF.dnsmasq_pxe_filter.dhcp_hostsdir, _UNKNOWN_HOSTS_FILE) - if _should_enable_unknown_hosts(): + if (_should_enable_unknown_hosts() + and not CONF.pxe_filter.deny_unknown_macs): wildcard_filter = _ALLOW_UNKNOWN_HOSTS log_wildcard_filter = 'allow' else: diff --git a/ironic_inspector/pxe_filter/iptables.py b/ironic_inspector/pxe_filter/iptables.py index 1d9c5248d..5205882e7 100644 --- a/ironic_inspector/pxe_filter/iptables.py +++ b/ironic_inspector/pxe_filter/iptables.py @@ -12,23 +12,17 @@ # limitations under the License. import contextlib -import os -import re from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log -from ironic_inspector.common import ironic as ir_utils from ironic_inspector import node_cache from ironic_inspector.pxe_filter import base as pxe_filter - CONF = cfg.CONF LOG = log.getLogger(__name__) -_EMAC_REGEX = 'EMAC=([0-9a-f]{2}(:[0-9a-f]{2}){5}) IMAC=.*' - def _should_enable_dhcp(): """Check whether we should enable DHCP at all. @@ -46,6 +40,7 @@ class IptablesFilter(pxe_filter.BaseFilter): def __init__(self): super(IptablesFilter, self).__init__() self.denylist_cache = None + self.allowlist_cache = None self.enabled = True self.interface = CONF.iptables.dnsmasq_interface self.chain = CONF.iptables.firewall_chain @@ -66,6 +61,7 @@ class IptablesFilter(pxe_filter.BaseFilter): def reset(self): self.enabled = True self.denylist_cache = None + self.allowlist_cache = None for chain in (self.chain, self.new_chain): try: self._clean_up(chain) @@ -114,26 +110,44 @@ class IptablesFilter(pxe_filter.BaseFilter): self._disable_dhcp() return - to_deny = _get_denylist(ironic) - if to_deny == self.denylist_cache: - LOG.debug('Not updating iptables - no changes in MAC list %s', - to_deny) + if CONF.pxe_filter.deny_unknown_macs: + to_allow = pxe_filter.get_active_macs(ironic) + to_deny = None + else: + to_deny = pxe_filter.get_inactive_macs(ironic) + to_allow = None + + if to_deny == self.denylist_cache and to_allow == self.allowlist_cache: + LOG.debug('Not updating iptables - no changes in MAC lists %s %s', + to_deny, to_allow) return - LOG.debug('Adding active MAC\'s %s to the deny list', to_deny) + if CONF.pxe_filter.deny_unknown_macs: + LOG.debug('Adding MAC\'s %s to the allow list', to_allow) + else: + LOG.debug('Adding MAC\'s %s to the deny list', to_deny) with self._temporary_chain(self.new_chain, self.chain): # Force update on the next iteration if this attempt fails self.denylist_cache = None - # - Add active macs to the deny list, so that nova can boot them - for mac in to_deny: - self._iptables('-A', self.new_chain, '-m', 'mac', - '--mac-source', mac, '-j', 'DROP') - # - Add everything else to the allow list - self._iptables('-A', self.new_chain, '-j', 'ACCEPT') + self.allowlist_cache = None + # - Add active macs to allow list, so that they are introspected + if CONF.pxe_filter.deny_unknown_macs: + for mac in to_allow: + self._iptables('-A', self.new_chain, '-m', 'mac', + '--mac-source', mac, '-j', 'ACCEPT') + # - Add inactive macs to the deny list, so that nova can boot them + if not CONF.pxe_filter.deny_unknown_macs: + for mac in to_deny: + self._iptables('-A', self.new_chain, '-m', 'mac', + '--mac-source', mac, '-j', 'DROP') + # - Add everything else to the unknown list + policy = 'DROP' if CONF.pxe_filter.deny_unknown_macs else 'ACCEPT' + self._iptables('-A', self.new_chain, '-j', policy) # Cache result of successful iptables update self.enabled = True self.denylist_cache = to_deny + self.allowlist_cache = to_allow LOG.debug('The iptables filter was synchronized') @contextlib.contextmanager @@ -190,50 +204,3 @@ class IptablesFilter(pxe_filter.BaseFilter): # deny everything self._iptables('-A', self.new_chain, '-j', 'REJECT') self.enabled = False - - -def _ib_mac_to_rmac_mapping(ports): - """Update port InfiniBand MAC address to EthernetOverInfiniBand MAC - - On InfiniBand deployment we need to map between the baremetal host - InfiniBand MAC to the EoIB MAC. The EoIB MAC addresses are learned - automatically by the EoIB interfaces and those MACs are recorded to the - /sys/class/net//eth/neighs file. The InfiniBand GUID is - taken from the ironic port client-id extra attribute. The InfiniBand GUID - is the last 8 bytes of the client-id. The file format allows to map the - GUID to EoIB MAC. The filter rules based on those MACs get applied during a - driver.update() call - - :param ports: list of ironic ports - :returns: Nothing. - """ - ethoib_interfaces = CONF.iptables.ethoib_interfaces - for interface in ethoib_interfaces: - neighs_file = ( - os.path.join('/sys/class/net', interface, 'eth/neighs')) - try: - with open(neighs_file, 'r') as fd: - data = fd.read() - except IOError: - LOG.error('Interface %s is not Ethernet Over InfiniBand; ' - 'Skipping ...', interface) - continue - for port in ports: - client_id = port.extra.get('client-id') - if client_id: - # Note(moshele): The last 8 bytes in the client-id is - # the baremetal node InfiniBand GUID - guid = client_id[-23:] - p = re.compile(_EMAC_REGEX + guid) - match = p.search(data) - if match: - port.address = match.group(1) - - -def _get_denylist(ironic): - ports = [port for port in - ir_utils.call_with_retries(ironic.ports, limit=None, - fields=['address', 'extra']) - if port.address not in node_cache.active_macs()] - _ib_mac_to_rmac_mapping(ports) - return [port.address for port in ports] diff --git a/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py b/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py index c66af8ec7..8bd52d748 100644 --- a/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py +++ b/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py @@ -26,8 +26,10 @@ from oslo_config import cfg from ironic_inspector.common import ironic as ir_utils from ironic_inspector import node_cache +from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector.pxe_filter import dnsmasq from ironic_inspector.test import base as test_base +from ironic_inspector import utils CONF = cfg.CONF @@ -38,6 +40,15 @@ class DnsmasqTestBase(test_base.BaseTest): self.driver = dnsmasq.DnsmasqFilter() +class TestConfiguration(DnsmasqTestBase): + + def test_deny_unknown_macs_and_node_not_found_hook_bad(self): + CONF.set_override('deny_unknown_macs', True, 'pxe_filter') + CONF.set_override('node_not_found_hook', True, 'processing') + self.assertRaisesRegex(utils.Error, 'Configuration error:', + self.driver.__init__) + + class TestShouldEnableUnknownHosts(DnsmasqTestBase): def setUp(self): super(TestShouldEnableUnknownHosts, self).setUp() @@ -251,6 +262,23 @@ class TestMACHandlers(test_base.BaseTest): 'A %s record for all unknown hosts using wildcard mac ' 'created', 'deny') + def test__macs_unknown_hosts_deny_unknown(self): + self.mock_join.return_value = "%s/%s" % (self.dhcp_hostsdir, + dnsmasq._UNKNOWN_HOSTS_FILE) + self.mock_introspection_active.return_value = True + CONF.set_override('deny_unknown_macs', True, 'pxe_filter') + + dnsmasq._configure_unknown_hosts() + + self.mock_join.assert_called_once_with(self.dhcp_hostsdir, + dnsmasq._UNKNOWN_HOSTS_FILE) + self.mock__exclusive_write_or_pass.assert_called_once_with( + self.mock_join.return_value, + '%s' % dnsmasq._DENY_UNKNOWN_HOSTS) + self.mock_log.debug.assert_called_once_with( + 'A %s record for all unknown hosts using wildcard mac ' + 'created', 'deny') + def test__configure_removedlist_allowlist(self): self.mock_introspection_active.return_value = True self.mock_stat.return_value.st_size = dnsmasq._MAC_DENY_LEN @@ -271,6 +299,17 @@ class TestMACHandlers(test_base.BaseTest): self.mock__exclusive_write_or_pass.assert_called_once_with( self.mock_join.return_value, '%s,ignore\n' % self.mac) + def test__configure_removedlist_denylist_deny_unknown(self): + self.mock_introspection_active.return_value = True + self.mock_stat.return_value.st_size = dnsmasq._MAC_ALLOW_LEN + CONF.set_override('deny_unknown_macs', True, 'pxe_filter') + + dnsmasq._configure_removedlist({self.mac}) + + self.mock_join.assert_called_with(self.dhcp_hostsdir, self.mac) + self.mock__exclusive_write_or_pass.assert_called_once_with( + self.mock_join.return_value, '%s,ignore\n' % self.mac) + def test__allowlist_mac(self): dnsmasq._add_mac_to_allowlist(self.mac) @@ -372,6 +411,9 @@ class TestSync(DnsmasqTestBase): get_client_mock = self.useFixture( fixtures.MockPatchObject(ir_utils, 'get_client')).mock get_client_mock.return_value = self.mock_ironic + self.mock_get_active_macs = self.useFixture( + fixtures.MockPatchObject(pxe_filter, 'get_active_macs')).mock + self.mock_get_active_macs.return_value = set() self.mock_active_macs = self.useFixture( fixtures.MockPatchObject(node_cache, 'active_macs')).mock self.ironic_macs = {'new_mac', 'active_mac'} @@ -401,14 +443,15 @@ class TestSync(DnsmasqTestBase): self.mock__configure_unknown_hosts.assert_called_once_with() def test__sync(self): + self.mock_get_active_macs.return_value = {'active_mac'} self.driver._sync(self.mock_ironic) + self.mock_get_active_macs.assert_called_once_with(self.mock_ironic) self.mock__add_mac_to_allowlist.assert_called_once_with('active_mac') self.mock__add_mac_to_denylist.assert_called_once_with('new_mac') self.mock_ironic.ports.assert_called_once_with( - limit=None, fields=['address']) - self.mock_active_macs.assert_called_once_with() + fields=['address', 'extra'], limit=None) self.mock__get_deny_allow_lists.assert_called_once_with() self.mock__configure_unknown_hosts.assert_called_once_with() self.mock__configure_removedlist.assert_called_once_with({'gone_mac'}) @@ -424,14 +467,15 @@ class TestSync(DnsmasqTestBase): os_exc.SDKException('boom'), [mock.Mock(address=address) for address in self.ironic_macs] ] + self.mock_get_active_macs.return_value = {'active_mac'} self.driver._sync(self.mock_ironic) self.mock__add_mac_to_allowlist.assert_called_once_with('active_mac') self.mock__add_mac_to_denylist.assert_called_once_with('new_mac') - self.mock_ironic.ports.assert_called_with( - limit=None, fields=['address']) - self.mock_active_macs.assert_called_once_with() + self.mock_ironic.ports.assert_called_with(fields=['address', 'extra'], + limit=None) + self.mock_get_active_macs.assert_called_once_with(self.mock_ironic) self.mock__get_deny_allow_lists.assert_called_once_with() self.mock__configure_removedlist.assert_called_once_with({'gone_mac'}) self.mock_log.debug.assert_has_calls([ diff --git a/ironic_inspector/test/unit/test_iptables.py b/ironic_inspector/test/unit/test_iptables.py index 939811dfb..1fb7f9693 100644 --- a/ironic_inspector/test/unit/test_iptables.py +++ b/ironic_inspector/test/unit/test_iptables.py @@ -16,14 +16,13 @@ from unittest import mock import fixtures -from openstack import exceptions as os_exc from oslo_config import cfg from ironic_inspector import node_cache from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector.pxe_filter import iptables from ironic_inspector.test import base as test_base - +from ironic_inspector import utils CONF = cfg.CONF @@ -44,10 +43,14 @@ class TestIptablesDriver(test_base.NodeTest): fixtures.MockPatchObject(self.driver, '_iptables')).mock self.mock_should_enable_dhcp = self.useFixture( fixtures.MockPatchObject(iptables, '_should_enable_dhcp')).mock - self.mock__get_denylist = self.useFixture( - fixtures.MockPatchObject(iptables, '_get_denylist')).mock - self.mock__get_denylist.return_value = [] + self.mock_get_inactive_macs = self.useFixture( + fixtures.MockPatchObject(pxe_filter, 'get_inactive_macs')).mock + self.mock_get_inactive_macs.return_value = set() + self.mock_get_active_macs = self.useFixture( + fixtures.MockPatchObject(pxe_filter, 'get_active_macs')).mock + self.mock_get_active_macs.return_value = set() self.mock_ironic = mock.Mock() + self.mock_ironic.ports.return_value = [] def check_fsm(self, events): # assert the iptables.fsm.process_event() was called with the events @@ -143,7 +146,7 @@ class TestIptablesDriver(test_base.NodeTest): for (args, call) in zip(_iptables_expected_args, call_args_list): self.assertEqual(args, call[0]) - self.mock__get_denylist.assert_called_once_with(self.mock_ironic) + self.mock_get_inactive_macs.assert_called_once_with(self.mock_ironic) self.check_fsm([pxe_filter.Events.sync]) def test__iptables_args_ipv4(self): @@ -179,7 +182,8 @@ class TestIptablesDriver(test_base.NodeTest): self.driver = iptables.IptablesFilter() self.mock_iptables = self.useFixture( fixtures.MockPatchObject(self.driver, '_iptables')).mock - self.mock__get_denylist.return_value = ['AA:BB:CC:DD:EE:FF'] + self.mock_get_active_macs.return_value = ['AA:BB:CC:DD:EE:FF'] + self.mock_get_inactive_macs.return_value = ['FF:EE:DD:CC:BB:AA'] self.mock_should_enable_dhcp.return_value = True _iptables_expected_args = [ @@ -190,7 +194,7 @@ class TestIptablesDriver(test_base.NodeTest): ('-N', self.driver.new_chain), # deny ('-A', self.driver.new_chain, '-m', 'mac', '--mac-source', - self.mock__get_denylist.return_value[0], '-j', 'DROP'), + self.mock_get_inactive_macs.return_value[0], '-j', 'DROP'), ('-A', self.driver.new_chain, '-j', 'ACCEPT'), ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', expected_port, '-j', self.driver.new_chain), @@ -208,14 +212,14 @@ class TestIptablesDriver(test_base.NodeTest): for (args, call) in zip(_iptables_expected_args, call_args_list): self.assertEqual(args, call[0]) - self.mock__get_denylist.assert_called_once_with(self.mock_ironic) + self.mock_get_inactive_macs.assert_called_once_with(self.mock_ironic) # check caching self.mock_iptables.reset_mock() - self.mock__get_denylist.reset_mock() + self.mock_get_inactive_macs.reset_mock() self.driver.sync(self.mock_ironic) - self.mock__get_denylist.assert_called_once_with(self.mock_ironic) + self.mock_get_inactive_macs.assert_called_once_with(self.mock_ironic) self.assertFalse(self.mock_iptables.called) def test_sync_with_denylist_ipv4(self): @@ -226,18 +230,71 @@ class TestIptablesDriver(test_base.NodeTest): CONF.set_override('ip_version', '6', 'iptables') self._test_sync_with_denylist('547') + def _test_sync_with_allowlist(self, expected_port): + CONF.set_override('deny_unknown_macs', True, 'pxe_filter') + self.driver = iptables.IptablesFilter() + self.mock_iptables = self.useFixture( + fixtures.MockPatchObject(self.driver, '_iptables')).mock + self.mock_get_active_macs.return_value = ['AA:BB:CC:DD:EE:FF'] + self.mock_get_inactive_macs.return_value = ['FF:EE:DD:CC:BB:AA'] + self.mock_should_enable_dhcp.return_value = True + + _iptables_expected_args = [ + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + expected_port, '-j', self.driver.new_chain), + ('-F', self.driver.new_chain), + ('-X', self.driver.new_chain), + ('-N', self.driver.new_chain), + # deny + ('-A', self.driver.new_chain, '-m', 'mac', '--mac-source', + self.mock_get_active_macs.return_value[0], '-j', 'ACCEPT'), + ('-A', self.driver.new_chain, '-j', 'DROP'), + ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + expected_port, '-j', self.driver.new_chain), + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + expected_port, '-j', self.driver.chain), + ('-F', self.driver.chain), + ('-X', self.driver.chain), + ('-E', self.driver.new_chain, self.driver.chain) + ] + + self.driver.sync(self.mock_ironic) + self.check_fsm([pxe_filter.Events.sync]) + call_args_list = self.mock_iptables.call_args_list + + for (args, call) in zip(_iptables_expected_args, + call_args_list): + self.assertEqual(args, call[0]) + self.mock_get_active_macs.assert_called_once_with(self.mock_ironic) + + # check caching + + self.mock_iptables.reset_mock() + self.mock_get_active_macs.reset_mock() + self.driver.sync(self.mock_ironic) + self.mock_get_active_macs.assert_called_once_with(self.mock_ironic) + self.assertFalse(self.mock_iptables.called) + + def test_sync_with_allowlist_ipv4(self): + CONF.set_override('ip_version', '4', 'iptables') + self._test_sync_with_allowlist('67') + + def test_sync_with_allowlist_ipv6(self): + CONF.set_override('ip_version', '6', 'iptables') + self._test_sync_with_allowlist('547') + def _test__iptables_clean_cache_on_error(self, expected_port): self.driver = iptables.IptablesFilter() self.mock_iptables = self.useFixture( fixtures.MockPatchObject(self.driver, '_iptables')).mock - self.mock__get_denylist.return_value = ['AA:BB:CC:DD:EE:FF'] + self.mock_get_inactive_macs.return_value = ['AA:BB:CC:DD:EE:FF'] self.mock_should_enable_dhcp.return_value = True self.mock_iptables.side_effect = [None, None, RuntimeError('Oops!'), None, None, None, None, None, None] self.assertRaises(RuntimeError, self.driver.sync, self.mock_ironic) self.check_fsm([pxe_filter.Events.sync, pxe_filter.Events.reset]) - self.mock__get_denylist.assert_called_once_with(self.mock_ironic) + self.mock_get_inactive_macs.assert_called_once_with(self.mock_ironic) # check caching syncs_expected_args = [ @@ -249,7 +306,7 @@ class TestIptablesDriver(test_base.NodeTest): ('-N', self.driver.new_chain), # deny ('-A', self.driver.new_chain, '-m', 'mac', '--mac-source', - self.mock__get_denylist.return_value[0], '-j', 'DROP'), + self.mock_get_inactive_macs.return_value[0], '-j', 'DROP'), ('-A', self.driver.new_chain, '-j', 'ACCEPT'), ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', expected_port, '-j', self.driver.new_chain), @@ -262,7 +319,7 @@ class TestIptablesDriver(test_base.NodeTest): self.mock_iptables.reset_mock() self.mock_iptables.side_effect = None - self.mock__get_denylist.reset_mock() + self.mock_get_inactive_macs.reset_mock() self.mock_fsm.reset_mock() self.driver.sync(self.mock_ironic) self.check_fsm([pxe_filter.Events.sync]) @@ -271,7 +328,7 @@ class TestIptablesDriver(test_base.NodeTest): for (idx, (args, call)) in enumerate(zip(syncs_expected_args, call_args_list)): self.assertEqual(args, call[0], 'idx: %s' % idx) - self.mock__get_denylist.assert_called_once_with(self.mock_ironic) + self.mock_get_inactive_macs.assert_called_once_with(self.mock_ironic) def test__iptables_clean_cache_on_error_ipv4(self): CONF.set_override('ip_version', '4', 'iptables') @@ -291,6 +348,12 @@ class TestIptablesDriver(test_base.NodeTest): driver = iptables.IptablesFilter() self.assertEqual(driver._cmd_iptables, 'ip6tables') + def test_deny_unknown_macs_and_node_not_found_hook_bad(self): + CONF.set_override('deny_unknown_macs', True, 'pxe_filter') + CONF.set_override('node_not_found_hook', True, 'processing') + self.assertRaisesRegex(utils.Error, 'Configuration error:', + self.driver.__init__) + class Test_ShouldEnableDhcp(test_base.BaseTest): def setUp(self): @@ -311,113 +374,3 @@ class Test_ShouldEnableDhcp(test_base.BaseTest): def test__should_enable_dhcp_false(self): self.mock_introspection_active.return_value = False self.assertIs(False, iptables._should_enable_dhcp()) - - -class TestIBMapping(test_base.BaseTest): - def setUp(self): - super(TestIBMapping, self).setUp() - CONF.set_override('ethoib_interfaces', ['eth0'], 'iptables') - self.ib_data = ( - 'EMAC=02:00:02:97:00:01 IMAC=97:fe:80:00:00:00:00:00:00:7c:fe:90:' - '03:00:29:26:52\n' - 'EMAC=02:00:00:61:00:02 IMAC=61:fe:80:00:00:00:00:00:00:7c:fe:90:' - '03:00:29:24:4f\n' - ) - self.client_id = ('ff:00:00:00:00:00:02:00:00:02:c9:00:7c:fe:90:03:00:' - '29:24:4f') - self.ib_address = '7c:fe:90:29:24:4f' - self.ib_port = mock.Mock(address=self.ib_address, - extra={'client-id': self.client_id}, - spec=['address', 'extra']) - self.port = mock.Mock(address='aa:bb:cc:dd:ee:ff', - extra={}, spec=['address', 'extra']) - self.ports = [self.ib_port, self.port] - self.expected_rmac = '02:00:00:61:00:02' - self.fileobj = mock.mock_open(read_data=self.ib_data) - - def test_matching_ib(self): - with mock.patch('builtins.open', self.fileobj, - create=True) as mock_open: - iptables._ib_mac_to_rmac_mapping(self.ports) - - self.assertEqual(self.expected_rmac, self.ib_port.address) - self.assertEqual(self.ports, [self.ib_port, self.port]) - mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', - 'r') - - def test_ib_not_match(self): - self.ports[0].extra['client-id'] = 'foo' - with mock.patch('builtins.open', self.fileobj, - create=True) as mock_open: - iptables._ib_mac_to_rmac_mapping(self.ports) - - self.assertEqual(self.ib_address, self.ib_port.address) - self.assertEqual(self.ports, [self.ib_port, self.port]) - mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', - 'r') - - def test_open_no_such_file(self): - with mock.patch('builtins.open', - side_effect=IOError(), autospec=True) as mock_open: - iptables._ib_mac_to_rmac_mapping(self.ports) - - self.assertEqual(self.ib_address, self.ib_port.address) - self.assertEqual(self.ports, [self.ib_port, self.port]) - mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', - 'r') - - def test_no_interfaces(self): - CONF.set_override('ethoib_interfaces', [], 'iptables') - with mock.patch('builtins.open', self.fileobj, - create=True) as mock_open: - iptables._ib_mac_to_rmac_mapping(self.ports) - - self.assertEqual(self.ib_address, self.ib_port.address) - self.assertEqual(self.ports, [self.ib_port, self.port]) - mock_open.assert_not_called() - - -class TestGetDenylist(test_base.BaseTest): - def setUp(self): - super(TestGetDenylist, self).setUp() - self.mock__ib_mac_to_rmac_mapping = self.useFixture( - fixtures.MockPatchObject(iptables, '_ib_mac_to_rmac_mapping')).mock - self.mock_active_macs = self.useFixture( - fixtures.MockPatchObject(node_cache, 'active_macs')).mock - self.mock_ironic = mock.Mock() - - def test_active_port(self): - mock_ports_list = [ - mock.Mock(address='foo'), - mock.Mock(address='bar'), - ] - self.mock_ironic.ports.return_value = mock_ports_list - self.mock_active_macs.return_value = {'foo'} - - ports = iptables._get_denylist(self.mock_ironic) - # foo is an active address so we expect the denylist contain only bar - self.assertEqual(['bar'], ports) - self.mock_ironic.ports.assert_called_once_with( - limit=None, fields=['address', 'extra']) - self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( - [mock_ports_list[1]]) - - @mock.patch('time.sleep', lambda _x: None) - def test_retry_on_port_list_failure(self): - mock_ports_list = [ - mock.Mock(address='foo'), - mock.Mock(address='bar'), - ] - self.mock_ironic.ports.side_effect = [ - os_exc.SDKException('boom'), - mock_ports_list - ] - self.mock_active_macs.return_value = {'foo'} - - ports = iptables._get_denylist(self.mock_ironic) - # foo is an active address so we expect the denylist contain only bar - self.assertEqual(['bar'], ports) - self.mock_ironic.ports.assert_called_with( - limit=None, fields=['address', 'extra']) - self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( - [mock_ports_list[1]]) diff --git a/ironic_inspector/test/unit/test_pxe_filter.py b/ironic_inspector/test/unit/test_pxe_filter.py index 150d68b33..8846fcc46 100644 --- a/ironic_inspector/test/unit/test_pxe_filter.py +++ b/ironic_inspector/test/unit/test_pxe_filter.py @@ -17,10 +17,12 @@ from automaton import exceptions as automaton_errors from eventlet import semaphore import fixtures from futurist import periodics +from openstack import exceptions as os_exc from oslo_config import cfg import stevedore from ironic_inspector.common import ironic as ir_utils +from ironic_inspector import node_cache from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector.pxe_filter import interface from ironic_inspector.test import base as test_base @@ -292,3 +294,198 @@ class TestDriver(test_base.BaseTest): self.assertIs(self.mock_driver, ret) self.mock__driver_manager.assert_called_once_with() + + +class TestIBMapping(test_base.BaseTest): + def setUp(self): + super(TestIBMapping, self).setUp() + CONF.set_override('ethoib_interfaces', ['eth0'], 'iptables') + self.ib_data = ( + 'EMAC=02:00:02:97:00:01 IMAC=97:fe:80:00:00:00:00:00:00:7c:fe:90:' + '03:00:29:26:52\n' + 'EMAC=02:00:00:61:00:02 IMAC=61:fe:80:00:00:00:00:00:00:7c:fe:90:' + '03:00:29:24:4f\n' + ) + self.client_id = ('ff:00:00:00:00:00:02:00:00:02:c9:00:7c:fe:90:03:00:' + '29:24:4f') + self.ib_address = '7c:fe:90:29:24:4f' + self.ib_port = mock.Mock(address=self.ib_address, + extra={'client-id': self.client_id}, + spec=['address', 'extra']) + self.port = mock.Mock(address='aa:bb:cc:dd:ee:ff', + extra={}, spec=['address', 'extra']) + self.ports = [self.ib_port, self.port] + self.expected_rmac = '02:00:00:61:00:02' + self.fileobj = mock.mock_open(read_data=self.ib_data) + + def test_matching_ib(self): + with mock.patch('builtins.open', self.fileobj, + create=True) as mock_open: + pxe_filter._ib_mac_to_rmac_mapping(self.ports) + + self.assertEqual(self.expected_rmac, self.ib_port.address) + self.assertEqual(self.ports, [self.ib_port, self.port]) + mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', + 'r') + + def test_ib_not_match(self): + self.ports[0].extra['client-id'] = 'foo' + with mock.patch('builtins.open', self.fileobj, + create=True) as mock_open: + pxe_filter._ib_mac_to_rmac_mapping(self.ports) + + self.assertEqual(self.ib_address, self.ib_port.address) + self.assertEqual(self.ports, [self.ib_port, self.port]) + mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', + 'r') + + def test_open_no_such_file(self): + with mock.patch('builtins.open', + side_effect=IOError(), autospec=True) as mock_open: + pxe_filter._ib_mac_to_rmac_mapping(self.ports) + + self.assertEqual(self.ib_address, self.ib_port.address) + self.assertEqual(self.ports, [self.ib_port, self.port]) + mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', + 'r') + + def test_no_interfaces(self): + CONF.set_override('ethoib_interfaces', [], 'iptables') + with mock.patch('builtins.open', self.fileobj, + create=True) as mock_open: + pxe_filter._ib_mac_to_rmac_mapping(self.ports) + + self.assertEqual(self.ib_address, self.ib_port.address) + self.assertEqual(self.ports, [self.ib_port, self.port]) + mock_open.assert_not_called() + + +class TestGetInactiveMacs(test_base.BaseTest): + def setUp(self): + super(TestGetInactiveMacs, self).setUp() + self.mock__ib_mac_to_rmac_mapping = self.useFixture( + fixtures.MockPatchObject(pxe_filter, + '_ib_mac_to_rmac_mapping')).mock + self.mock_active_macs = self.useFixture( + fixtures.MockPatchObject(node_cache, 'active_macs')).mock + self.mock_ironic = mock.Mock() + + def test_inactive_port(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.return_value = mock_ports_list + self.mock_active_macs.return_value = {'foo'} + + ports = pxe_filter.get_inactive_macs(self.mock_ironic) + self.assertEqual({'bar'}, ports) + self.mock_ironic.ports.assert_called_once_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + [mock_ports_list[1]]) + + @mock.patch('time.sleep', lambda _x: None) + def test_retry_on_port_list_failure(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.side_effect = [ + os_exc.SDKException('boom'), + mock_ports_list + ] + self.mock_active_macs.return_value = {'foo'} + + ports = pxe_filter.get_inactive_macs(self.mock_ironic) + self.assertEqual({'bar'}, ports) + self.mock_ironic.ports.assert_called_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + [mock_ports_list[1]]) + + +class TestGetActiveMacs(test_base.BaseTest): + def setUp(self): + super(TestGetActiveMacs, self).setUp() + self.mock__ib_mac_to_rmac_mapping = self.useFixture( + fixtures.MockPatchObject(pxe_filter, + '_ib_mac_to_rmac_mapping')).mock + self.mock_active_macs = self.useFixture( + fixtures.MockPatchObject(node_cache, 'active_macs')).mock + self.mock_ironic = mock.Mock() + + def test_active_port(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.return_value = mock_ports_list + self.mock_active_macs.return_value = {'foo'} + + ports = pxe_filter.get_active_macs(self.mock_ironic) + self.assertEqual({'foo'}, ports) + self.mock_ironic.ports.assert_called_once_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + [mock_ports_list[0]]) + + @mock.patch('time.sleep', lambda _x: None) + def test_retry_on_port_list_failure(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.side_effect = [ + os_exc.SDKException('boom'), + mock_ports_list + ] + self.mock_active_macs.return_value = {'foo'} + + ports = pxe_filter.get_active_macs(self.mock_ironic) + self.assertEqual({'foo'}, ports) + self.mock_ironic.ports.assert_called_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + [mock_ports_list[0]]) + + +class TestGetIronicMacs(test_base.BaseTest): + def setUp(self): + super(TestGetIronicMacs, self).setUp() + self.mock__ib_mac_to_rmac_mapping = self.useFixture( + fixtures.MockPatchObject(pxe_filter, + '_ib_mac_to_rmac_mapping')).mock + self.mock_ironic = mock.Mock() + + def test_active_port(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.return_value = mock_ports_list + + ports = pxe_filter.get_ironic_macs(self.mock_ironic) + self.assertEqual({'foo', 'bar'}, ports) + self.mock_ironic.ports.assert_called_once_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + mock_ports_list) + + @mock.patch('time.sleep', lambda _x: None) + def test_retry_on_port_list_failure(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.side_effect = [ + os_exc.SDKException('boom'), + mock_ports_list + ] + + ports = pxe_filter.get_ironic_macs(self.mock_ironic) + self.assertEqual({'foo', 'bar'}, ports) + self.mock_ironic.ports.assert_called_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + mock_ports_list) diff --git a/releasenotes/notes/dnsmasq-pxe-filter-eoib-mac-support-7567bbc7c6bf1878.yaml b/releasenotes/notes/dnsmasq-pxe-filter-eoib-mac-support-7567bbc7c6bf1878.yaml new file mode 100644 index 000000000..9768dfa18 --- /dev/null +++ b/releasenotes/notes/dnsmasq-pxe-filter-eoib-mac-support-7567bbc7c6bf1878.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + The dnsmasq pxe-filter now supports mapping between host InfiniBand MAC to + EthernetOverInfiniBand MAC. (This was previously only supported by the + iptables pxe-filter.) diff --git a/releasenotes/notes/pxe-filter-add-deny-unknown-host-option-b84b2aa1f7f49a17.yaml b/releasenotes/notes/pxe-filter-add-deny-unknown-host-option-b84b2aa1f7f49a17.yaml new file mode 100644 index 000000000..265e38b26 --- /dev/null +++ b/releasenotes/notes/pxe-filter-add-deny-unknown-host-option-b84b2aa1f7f49a17.yaml @@ -0,0 +1,17 @@ +--- +features: + - | + By default the DHCP filtering will open the DHCP server for any node when + introspection is active. It will only block DHCP for enrolled nodes that + are not being introspected. Doing so is required to support interface + discovery (which by default will enroll the pxe port to ironic if not + present). This behaviour is not always wanted, as nodes not managed by + ironic may boot the inspection image. + + A new option was added ``[pxe_filter]deny_unknown_macs`` which allow + changeing this behaviour so that the DHCP server only allow enrolled nodes + being introspected and deny everything else. + + .. Note:: If this option is ``True``, nodes must have at least one + enrolled port prior to introspection. +