From a42980a016ca3bd184eaa677ee9ac13d00f4e5e6 Mon Sep 17 00:00:00 2001 From: cid Date: Wed, 12 Feb 2025 21:42:58 +0100 Subject: [PATCH] Ensure IPA is locked down in rescue mode Securely handle state transition by locking down IPA at the final stage of rescue operation to prevent restarts on tenant networks. Closes-Bug: #2086865 Change-Id: I8e1be8da93a8c3fdf3cff7ad386c702d970d15f1 --- ironic_python_agent/agent.py | 15 ++++++++++++++- ironic_python_agent/extensions/rescue.py | 6 ++++++ .../tests/unit/extensions/test_rescue.py | 19 ++++++++++++++++++- ...restart-after-rescue-2cdd9cb03c0efb1b.yaml | 5 +++++ 4 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/prevent-restart-after-rescue-2cdd9cb03c0efb1b.yaml diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 1d4b25254..3cbba6bec 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -15,6 +15,7 @@ import collections import importlib.metadata import ipaddress +import os import random import socket import threading @@ -564,7 +565,19 @@ class IronicPythonAgent(base.ExecuteCommandMixin): LOG.error('Neither ipa-api-url nor inspection_callback_url' 'found, please check your pxe append parameters.') - self.serve_ipa_api() + in_rescued_mode = os.path.exists('/etc/.rescued') + if not in_rescued_mode: + self.serve_ipa_api() + else: + # NOTE(cid): In rescued state, we don't call _lockdown_system() as + # it brings down network interfaces which should be preserved for + # rescue operations. + LOG.info('Found rescue state marker file, locking down IPA ' + 'in disabled mode') + self.heartbeater.stop() + self.serve_api = False + while True: + time.sleep(100) if not self.standalone and self.api_urls: self.heartbeater.stop() diff --git a/ironic_python_agent/extensions/rescue.py b/ironic_python_agent/extensions/rescue.py index a9eafd434..40704123e 100644 --- a/ironic_python_agent/extensions/rescue.py +++ b/ironic_python_agent/extensions/rescue.py @@ -57,6 +57,12 @@ class RescueExtension(base.BaseAgentExtension): def finalize_rescue(self, rescue_password="", hashed=False): """Sets the rescue password for the rescue user.""" self.write_rescue_password(rescue_password, hashed) + + try: + open('/etc/.rescued', 'w') + except Exception as exc: + LOG.warning('Failed to create rescue state file marker: %s', exc) + # IPA will terminate after the result of finalize_rescue is returned to # ironic to avoid exposing the IPA API to a tenant or public network self.agent.serve_api = False diff --git a/ironic_python_agent/tests/unit/extensions/test_rescue.py b/ironic_python_agent/tests/unit/extensions/test_rescue.py index 7f59982bd..87f3cac0b 100644 --- a/ironic_python_agent/tests/unit/extensions/test_rescue.py +++ b/ironic_python_agent/tests/unit/extensions/test_rescue.py @@ -84,12 +84,29 @@ class TestRescueExtension(test_base.BaseTestCase): for passwd in passwds: self._write_password_hashed_test(passwd) + @mock.patch('builtins.open', autospec=True) @mock.patch('ironic_python_agent.extensions.rescue.RescueExtension.' 'write_rescue_password', autospec=True) - def test_finalize_rescue(self, mock_write_rescue_password): + def test_finalize_rescue(self, mock_write_rescue_password, mock_open): self.agent_extension.agent.serve_api = True self.agent_extension.finalize_rescue(rescue_password='password') mock_write_rescue_password.assert_called_once_with( mock.ANY, rescue_password='password', hashed=False) self.assertFalse(self.agent_extension.agent.serve_api) + mock_open.assert_called_once_with('/etc/.rescued', 'w') + + @mock.patch('builtins.open', autospec=True) + @mock.patch('ironic_python_agent.extensions.rescue.RescueExtension.' + 'write_rescue_password', autospec=True) + def test_finalize_rescue_write_failure(self, mock_write_rescue_password, + mock_open): + """Test that finalize_rescue handles file write failure or no file.""" + mock_open.side_effect = IOError("Failed to write file") + self.agent_extension.agent.serve_api = True + self.agent_extension.finalize_rescue(rescue_password='password') + mock_write_rescue_password.assert_called_once_with( + mock.ANY, + rescue_password='password', hashed=False) + self.assertFalse(self.agent_extension.agent.serve_api) + mock_open.assert_called_once_with('/etc/.rescued', 'w') diff --git a/releasenotes/notes/prevent-restart-after-rescue-2cdd9cb03c0efb1b.yaml b/releasenotes/notes/prevent-restart-after-rescue-2cdd9cb03c0efb1b.yaml new file mode 100644 index 000000000..db814e535 --- /dev/null +++ b/releasenotes/notes/prevent-restart-after-rescue-2cdd9cb03c0efb1b.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Prevents IPA from restarting on tenant networks during rescue + operations by adding proper lockdown.