From 92e26b01e97418f3f491216eb193124703befd3d Mon Sep 17 00:00:00 2001 From: Arne Wiebalck Date: Wed, 11 Nov 2020 11:22:31 +0100 Subject: [PATCH] Add clean step 'erase_pstore' Add an automatic clean step to clean the Linux kernel's pstore. The step is disabled by default. Story: #2008317 Task: #41214 Change-Id: Ie1a42dfff4c7e1c7abeaf39feca956bb9e2ea497 --- doc/source/admin/hardware_managers.rst | 3 + ironic_python_agent/hardware.py | 39 +++++++++++ .../tests/unit/test_hardware.py | 64 +++++++++++++++++++ .../add_erase_pstore-b109c58ed8f5d351.yaml | 11 ++++ 4 files changed, 117 insertions(+) create mode 100644 releasenotes/notes/add_erase_pstore-b109c58ed8f5d351.yaml diff --git a/doc/source/admin/hardware_managers.rst b/doc/source/admin/hardware_managers.rst index a65818c14..6649a47ba 100644 --- a/doc/source/admin/hardware_managers.rst +++ b/doc/source/admin/hardware_managers.rst @@ -35,6 +35,9 @@ Clean steps ``deploy.erase_devices_metadata`` Erases partition tables from all recognized disk devices. Can be used as an alternative to the much longer ``erase_devices`` step. +``deploy.erase_pstore`` + Erases entries from pstore, the kernel's oops/panic logger. Disabled by + default. Can be enabled via priority overrides. ``raid.create_configuration`` Create a RAID configuration. This step belongs to the ``raid`` interface and must be used through the :ironic-doc:`ironic RAID feature diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index dc7c1662f..9858f0c6a 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -22,6 +22,7 @@ from multiprocessing.pool import ThreadPool import os import re import shlex +import shutil import time from ironic_lib import disk_utils @@ -1314,6 +1315,37 @@ class GenericHardwareManager(HardwareManager): for k, v in erase_errors.items()])) raise errors.BlockDeviceEraseError(excpt_msg) + def _find_pstore_mount_point(self): + """Find the pstore mount point by scanning /proc/mounts. + + :returns: The pstore mount if existing, none otherwise. + """ + with open("/proc/mounts", "r") as mounts: + for line in mounts: + # /proc/mounts format is: "device mountpoint fstype ..." + m = re.match(r'^pstore (\S+) pstore', line) + if m: + return m.group(1) + + def erase_pstore(self, node, ports): + """Attempt to erase the kernel pstore. + + :param node: Ironic node object + :param ports: list of Ironic port objects + """ + pstore_path = self._find_pstore_mount_point() + if not pstore_path: + LOG.debug("No pstore found") + return + + LOG.info("Cleaning up pstore in %s", pstore_path) + for file in os.listdir(pstore_path): + filepath = os.path.join(pstore_path, file) + try: + shutil.rmtree(filepath) + except OSError: + os.remove(filepath) + def _shred_block_device(self, node, block_device): """Erase a block device using shred. @@ -1680,6 +1712,13 @@ class GenericHardwareManager(HardwareManager): 'reboot_requested': False, 'abortable': True }, + { + 'step': 'erase_pstore', + 'priority': 0, + 'interface': 'deploy', + 'reboot_requested': False, + 'abortable': True + }, { 'step': 'delete_configuration', 'priority': 0, diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 0fce12efe..77f6ff27c 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -14,6 +14,7 @@ import binascii import os +import shutil import time from unittest import mock @@ -859,6 +860,24 @@ MDADM_EXAMINE_OUTPUT_NON_MEMBER = ("""/dev/sdz1: """) +PROC_MOUNTS_OUTPUT = (""" +debugfs /sys/kernel/debug debugfs rw,relatime 0 0 +/dev/sda2 / ext4 rw,relatime,errors=remount-ro 0 0 +tmpfs /run/user/1000 tmpfs rw,nosuid,nodev,relatime 0 0 +pstore /sys/fs/pstore pstore rw,nosuid,nodev,noexec,relatime 0 0 +/dev/loop19 /snap/core/10126 squashfs ro,nodev,relatime 0 0 +""") + + +PROC_MOUNTS_OUTPUT_NO_PSTORE = (""" +debugfs /sys/kernel/debug debugfs rw,relatime 0 0 +/dev/sda2 / ext4 rw,relatime,errors=remount-ro 0 0 +tmpfs /run/user/1000 tmpfs rw,nosuid,nodev,relatime 0 0 +pstore /sys/fs/pstore qstore rw,nosuid,nodev,noexec,relatime 0 0 +/dev/loop19 /snap/core/10126 squashfs ro,nodev,relatime 0 0 +""") + + class FakeHardwareManager(hardware.GenericHardwareManager): def __init__(self, hardware_support): self._hardware_support = hardware_support @@ -921,6 +940,13 @@ class TestGenericHardwareManager(base.IronicAgentTest): 'reboot_requested': False, 'abortable': True }, + { + 'step': 'erase_pstore', + 'priority': 0, + 'interface': 'deploy', + 'reboot_requested': False, + 'abortable': True + }, { 'step': 'delete_configuration', 'priority': 0, @@ -2631,6 +2657,44 @@ class TestGenericHardwareManager(base.IronicAgentTest): test_security_erase_option( self, False, '--security-erase') + def test__find_pstore_mount_point(self): + with mock.patch('builtins.open', + mock.mock_open(), + create=True) as mocked_open: + mocked_open.return_value.__iter__ = \ + lambda self: iter(PROC_MOUNTS_OUTPUT.splitlines()) + + self.assertEqual(self.hardware._find_pstore_mount_point(), + "/sys/fs/pstore") + mocked_open.assert_called_once_with('/proc/mounts', 'r') + + def test__find_pstore_mount_point_no_pstore(self): + with mock.patch('builtins.open', + mock.mock_open(), + create=True) as mocked_open: + mocked_open.return_value.__iter__.return_value = \ + PROC_MOUNTS_OUTPUT_NO_PSTORE.splitlines() + self.assertIsNone(self.hardware._find_pstore_mount_point()) + mocked_open.assert_called_once_with('/proc/mounts', 'r') + + @mock.patch('os.listdir', autospec=True) + @mock.patch.object(shutil, 'rmtree', autospec=True) + @mock.patch.object(hardware.GenericHardwareManager, + '_find_pstore_mount_point', autospec=True) + def test_erase_pstore(self, mocked_find_pstore, mocked_rmtree, + mocked_listdir): + mocked_find_pstore.return_value = '/sys/fs/pstore' + pstore_entries = ['dmesg-erst-663482778', + 'dmesg-erst-663482779'] + mocked_listdir.return_value = pstore_entries + self.hardware.erase_pstore(self.node, []) + mocked_listdir.assert_called_once() + self.assertEqual(mocked_rmtree.call_count, + len(pstore_entries)) + mocked_rmtree.assert_has_calls([ + mock.call('/sys/fs/pstore/' + arg) for arg in pstore_entries + ]) + @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(hardware.GenericHardwareManager, '_is_virtual_media_device', autospec=True) diff --git a/releasenotes/notes/add_erase_pstore-b109c58ed8f5d351.yaml b/releasenotes/notes/add_erase_pstore-b109c58ed8f5d351.yaml new file mode 100644 index 000000000..dd22fe201 --- /dev/null +++ b/releasenotes/notes/add_erase_pstore-b109c58ed8f5d351.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + Adds a clean step to erase the Linux kernel's pstore. The step is disabled + by default. +security: + - | + If enabled, the new clean step 'erase_pstore' removes all pstore entries + (the oops/panic logs from a failing kernel) upon cleaning. This is to + reduce the risk that potentially sensitive data is preserved across + instantiations (and therefore different users) of a bare metal node. \ No newline at end of file