diff --git a/ironic_inspector/pxe_filter/dnsmasq.py b/ironic_inspector/pxe_filter/dnsmasq.py index e4acba7a2..a69c372a7 100644 --- a/ironic_inspector/pxe_filter/dnsmasq.py +++ b/ironic_inspector/pxe_filter/dnsmasq.py @@ -41,6 +41,19 @@ _EXCLUSIVE_WRITE_ATTEMPTS_DELAY = 0.01 _ROOTWRAP_COMMAND = 'sudo ironic-inspector-rootwrap {rootwrap_config!s}' _MACBL_LEN = len('ff:ff:ff:ff:ff:ff,ignore\n') +_UNKNOWN_HOSTS_FILE = 'unknown_hosts_filter' +_BLACKLIST_UNKNOWN_HOSTS = '*:*:*:*:*:*,ignore\n' +_WHITELIST_UNKNOWN_HOSTS = '*:*:*:*:*:*\n' + + +def _should_enable_unknown_hosts(): + """Check whether we should enable DHCP for unknown hosts + + We blacklist unknown hosts unless one or more nodes are on introspection + and node_not_found_hook is not set. + """ + return (node_cache.introspection_active() or + CONF.processing.node_not_found_hook is not None) class DnsmasqFilter(pxe_filter.BaseFilter): @@ -78,6 +91,9 @@ class DnsmasqFilter(pxe_filter.BaseFilter): # blacklist new ports that aren't being inspected for mac in ironic_macs - (blacklist_macs | active_macs): _blacklist_mac(mac) + + _configure_unknown_hosts() + timestamp_end = timeutils.utcnow() LOG.debug('The dnsmasq PXE filter was synchronized (took %s)', timestamp_end - timestamp_start) @@ -186,6 +202,41 @@ def _exclusive_write_or_pass(path, buf): return False +def _configure_unknown_hosts(): + """Manages a dhcp_hostsdir ignore/not-ignore record for unknown macs. + + :param allow: If True unknown hosts are whitelisted. If False unknown hosts + are blacklisted. + :raises: FileNotFoundError in case the dhcp_hostsdir is invalid, + IOError in case the dhcp host unknown file isn't writable. + :returns: None. + """ + path = os.path.join(CONF.dnsmasq_pxe_filter.dhcp_hostsdir, + _UNKNOWN_HOSTS_FILE) + + if _should_enable_unknown_hosts(): + wildcard_filter = _WHITELIST_UNKNOWN_HOSTS + log_wildcard_filter = 'whitelist' + else: + wildcard_filter = _BLACKLIST_UNKNOWN_HOSTS + log_wildcard_filter = 'blacklist' + + # Don't update if unknown hosts are already black/white-listed + try: + if os.stat(path).st_size == len(wildcard_filter): + return + except OSError as e: + if e.errno != os.errno.ENOENT: + raise + + if _exclusive_write_or_pass(path, '%s' % wildcard_filter): + LOG.debug('A %s record for all unknown hosts using wildcard mac ' + 'created', log_wildcard_filter) + else: + LOG.warning('Failed to %s unknown hosts using wildcard mac; ' + 'retrying next periodic sync time', log_wildcard_filter) + + def _blacklist_mac(mac): """Creates a dhcp_hostsdir ignore record for the MAC. diff --git a/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py b/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py index c97d27c3b..ce4ad406d 100644 --- a/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py +++ b/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py @@ -34,6 +34,21 @@ class DnsmasqTestBase(test_base.BaseTest): self.driver = dnsmasq.DnsmasqFilter() +class TestShouldEnableUnknownHosts(DnsmasqTestBase): + def setUp(self): + super(TestShouldEnableUnknownHosts, self).setUp() + self.mock_introspection_active = self.useFixture( + fixtures.MockPatchObject(node_cache, 'introspection_active')).mock + + def test_introspection_active(self): + self.mock_introspection_active.return_value = True + self.assertIs(True, dnsmasq._should_enable_unknown_hosts()) + + def test_introspection_not_active(self): + self.mock_introspection_active.return_value = False + self.assertIs(False, dnsmasq._should_enable_unknown_hosts()) + + class TestDnsmasqDriverAPI(DnsmasqTestBase): def setUp(self): super(TestDnsmasqDriverAPI, self).setUp() @@ -197,6 +212,40 @@ class TestMACHandlers(test_base.BaseTest): fixtures.MockPatchObject(os, 'listdir')).mock self.mock_remove = self.useFixture( fixtures.MockPatchObject(os, 'remove')).mock + self.mock_log = self.useFixture( + fixtures.MockPatchObject(dnsmasq, 'LOG')).mock + self.mock_introspection_active = self.useFixture( + fixtures.MockPatchObject(node_cache, 'introspection_active')).mock + + def test__whitelist_unknown_hosts(self): + self.mock_join.return_value = "%s/%s" % (self.dhcp_hostsdir, + dnsmasq._UNKNOWN_HOSTS_FILE) + self.mock_introspection_active.return_value = True + 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._WHITELIST_UNKNOWN_HOSTS) + self.mock_log.debug.assert_called_once_with( + 'A %s record for all unknown hosts using wildcard mac ' + 'created', 'whitelist') + + def test__blacklist_unknown_hosts(self): + self.mock_join.return_value = "%s/%s" % (self.dhcp_hostsdir, + dnsmasq._UNKNOWN_HOSTS_FILE) + self.mock_introspection_active.return_value = False + 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._BLACKLIST_UNKNOWN_HOSTS) + self.mock_log.debug.assert_called_once_with( + 'A %s record for all unknown hosts using wildcard mac ' + 'created', 'blacklist') def test__whitelist_mac(self): dnsmasq._whitelist_mac(self.mac) @@ -261,6 +310,9 @@ class TestSync(DnsmasqTestBase): fixtures.MockPatchObject(dnsmasq, '_whitelist_mac')).mock self.mock__blacklist_mac = self.useFixture( fixtures.MockPatchObject(dnsmasq, '_blacklist_mac')).mock + self.mock__configure_unknown_hosts = self.useFixture( + fixtures.MockPatchObject(dnsmasq, '_configure_unknown_hosts')).mock + self.mock_ironic = mock.Mock() self.mock_utcnow = self.useFixture( fixtures.MockPatchObject(dnsmasq.timeutils, 'utcnow')).mock @@ -283,6 +335,22 @@ class TestSync(DnsmasqTestBase): self.mock_ironic.port.list.return_value = [ mock.Mock(address=address) for address in self.ironic_macs] self.mock_active_macs.return_value = self.active_macs + self.mock_should_enable_unknown_hosts = self.useFixture( + fixtures.MockPatchObject(dnsmasq, + '_should_enable_unknown_hosts')).mock + self.mock_should_enable_unknown_hosts.return_value = True + + def test__sync_enable_unknown_hosts(self): + self.mock_should_enable_unknown_hosts.return_value = True + + self.driver._sync(self.mock_ironic) + self.mock__configure_unknown_hosts.assert_called_once_with() + + def test__sync_not_enable_unknown_hosts(self): + self.mock_should_enable_unknown_hosts.return_value = False + + self.driver._sync(self.mock_ironic) + self.mock__configure_unknown_hosts.assert_called_once_with() def test__sync(self): self.driver._sync(self.mock_ironic) @@ -296,6 +364,7 @@ class TestSync(DnsmasqTestBase): fields=['address']) self.mock_active_macs.assert_called_once_with() self.mock__get_blacklist.assert_called_once_with() + self.mock__configure_unknown_hosts.assert_called_once_with() self.mock_log.debug.assert_has_calls([ mock.call('Syncing the driver'), mock.call('The dnsmasq PXE filter was synchronized (took %s)', diff --git a/releasenotes/notes/pxe-filter-dnsmasq-not-known-hosts-filter-76ae5bd7a8db6f75.yaml b/releasenotes/notes/pxe-filter-dnsmasq-not-known-hosts-filter-76ae5bd7a8db6f75.yaml new file mode 100644 index 000000000..59056284d --- /dev/null +++ b/releasenotes/notes/pxe-filter-dnsmasq-not-known-hosts-filter-76ae5bd7a8db6f75.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + Adds wildcard ignore entry to ``dnsmasq`` PXE filter. When node + introspection is active, or if ``node_not_found_hook`` is set in the + configuration the ignore is removed from the wildcard entry. This ensures + that unknown nodes do not accidentally boot into the introspection image + when no node introspection is active. + + This brings ``dnsmasq`` PXE filter driver feature parity with the + ``iptables`` PXE filter driver, which uses a firewall rule to block any + DHCP request on the interface where Ironic Inspector's DHCP server is + listening.