From f3e3de8097f05cc830768da7d3f3e9eae04b40a1 Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Wed, 24 Aug 2022 10:15:27 -0700
Subject: [PATCH] Fix software raid output poisoning

In the event a device name is set to contain a raid device path,
it is possible for the Name and Events field values of mdadm's
detailed output to contain text which inadvertently gets captured and
mapped as component data for the "holder" devices of the RAID set.

This would cause invalid values to get passed to UEFI methods
which would cause a deployment to fail under these circumstances.

We now ignore the Name and Events fields in mdadm output.

Change-Id: If721dfe1caa5915326482969e55fbf4697538231
---
 ironic_python_agent/hardware.py               |  3 +-
 .../tests/unit/samples/hardware_samples.py    | 28 +++++++++++++++++++
 .../tests/unit/test_hardware.py               |  6 ++++
 ...traid-name-poisoning-4e934dd4e60830b1.yaml |  7 +++++
 4 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/fix-softraid-name-poisoning-4e934dd4e60830b1.yaml

diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index ebd480982..6c3b4184b 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -382,9 +382,10 @@ def get_holder_disks(raid_device):
 
     holder_parts = []
     for line in lines[1:]:
+        if 'Events' in line or 'Name' in line:
+            continue
         device = re.findall(r'/dev/\w+', line)
         holder_parts += device
-
     for part in holder_parts:
         # NOTE(mnaser): If the last character is not a digit and it is a valid
         #               device, this means that instead of a partition, it's a
diff --git a/ironic_python_agent/tests/unit/samples/hardware_samples.py b/ironic_python_agent/tests/unit/samples/hardware_samples.py
index 82f29eb70..f9635e432 100644
--- a/ironic_python_agent/tests/unit/samples/hardware_samples.py
+++ b/ironic_python_agent/tests/unit/samples/hardware_samples.py
@@ -1049,6 +1049,34 @@ MDADM_DETAIL_OUTPUT_BROKEN_RAID0 = ("""/dev/md126:
        -       8        2        -        /dev/sda2
 """)
 
+# NOTE(TheJulia): The name and events field, in some cases can
+# match the regex causing parsing of the text to fail.
+MDADM_DETAIL_POISONED = ("""/dev/md0:
+        Version : 1.2
+  Creation Time : Wed Aug 17 16:09:19 2022
+     Raid Level : raid1
+     Array Size : 4673536 (4.46 GiB 4.79 GB)
+  Used Dev Size : 4673536 (4.46 GiB 4.79 GB)
+   Raid Devices : 2
+  Total Devices : 2
+    Persistence : Superblock is persistent
+
+    Update Time : Wed Aug 17 16:10:03 2022
+          State : clean
+ Active Devices : 2
+Working Devices : 2
+ Failed Devices : 0
+  Spare Devices : 0
+
+           Name : box:/dev/md0  (local to host box)
+           UUID : e50fb152:aa80db1d:3c901b03:dd280e35
+         Events : 21/dev/md/dev/md
+
+    Number   Major   Minor   RaidDevice State
+       0     251        1        0      active sync   /dev/vda1
+       1     251       17        1      active sync   /dev/vdb1
+""")
+
 MDADM_EXAMINE_OUTPUT_MEMBER = ("""/dev/sda1:
           Magic : a92b4efc
         Version : 1.2
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index 1e2c6f8b9..57a2330a5 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -4450,6 +4450,12 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         holder_disks = hardware.get_holder_disks('/dev/md126')
         self.assertEqual(['/dev/sda'], holder_disks)
 
+    @mock.patch.object(il_utils, 'execute', autospec=True)
+    def test_get_holder_disks_poisoned_output(self, mocked_execute):
+        mocked_execute.side_effect = [(hws.MDADM_DETAIL_POISONED, '')]
+        holder_disks = hardware.get_holder_disks('/dev/md0')
+        self.assertEqual(['/dev/vda', '/dev/vdb'], holder_disks)
+
     @mock.patch.object(hardware, 'get_holder_disks', autospec=True)
     @mock.patch.object(hardware, 'get_component_devices', autospec=True)
     @mock.patch.object(hardware, 'list_all_block_devices', autospec=True)
diff --git a/releasenotes/notes/fix-softraid-name-poisoning-4e934dd4e60830b1.yaml b/releasenotes/notes/fix-softraid-name-poisoning-4e934dd4e60830b1.yaml
new file mode 100644
index 000000000..9b80e30c6
--- /dev/null
+++ b/releasenotes/notes/fix-softraid-name-poisoning-4e934dd4e60830b1.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes handling of Software RAID device discovery so RAID device ``Names``
+    and ``Events`` field values do not inadvertently cause the command to
+    return unexpected output. Previously this could cause a deployment to
+    when handling UEFI partitions.