rework ATA secure erase

hdparm versions prior to 9.51 interpret the value, NULL, as a
password with string value: "NULL".

Example output of hdparm with NULL password:

    [root@localhost ~]# hdparm --user-master u --security-unlock NULL /dev/sda
    security_password="NULL"

    /dev/sda:
    Issuing SECURITY_UNLOCK command, password="NULL", user=user
    SECURITY_UNLOCK: Input/output error

Example output of hdparm with "" as password:

    [root@localhost ~]# hdparm --user-master u --security-unlock "" /dev/sda
    security_password=""

    /dev/sda:
     Issuing SECURITY_UNLOCK command, password="", user=user

Note the values of security_password in the output above. The output
was observed on a CentOS 7 system, which ships hdparm 9.43 in the
offical repositories.

This change attempts to unlock the drive with the empty string if an
unlock with NULL was unsucessful.

Issuing a security-unlock will cause a state transition from SEC4
(security enabled, locked, not frozen) to SEC5 (security enabled,
unlocked, not frozen). In order to check that a password unlock attempt
was successful it makes sense to check that the drive is in the unlocked
state (a necessary condition for SEC5). Only after all unlock attempts
fail, do we consider the drive out of our control.

The conditions to check the drive is in the right state have been
adjusted to ensure that the drive is in the SEC5 state prior to issuing
a secure erase. Previously, on the "recovery from previous fail" path,
the security state was asserted to be "not enabled" after an unlock -
this could never have been the case.

A good overview of the ATA security states can be found here:

  http://www.admin-magazine.com/Archive/2014/19/Using-the-ATA-security-features-of-modern-hard-disks-and-SSDs

Change-Id: Ic24b706a04ff6c08d750b9e3d79eb79eab2952ad
Story: 2001762
Task: 12161
Story: 2001763
Task: 12162

1 files changed, 59 insertions, 21 deletions

diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index 95c22fc1..55548745 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -113,7 +113,7 @@ HDPARM_INFO_TEMPLATE = (
     '\tMaster password revision code = 65534\n'
     '\t%(supported)s\n'
     '\t%(enabled)s\n'
-    '\tnot\tlocked\n'
+    '\t%(locked)s\n'
     '\t%(frozen)s\n'
     '\tnot\texpired: security count\n'
     '\t%(enhanced_erase)s\n'
@@ -1441,28 +1441,65 @@ class TestGenericHardwareManager(base.IronicAgentTest):
             'shred', '--force', '--zero', '--verbose', '--iterations', '1',
             '/dev/sda')
 
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_erase_block_device_ata_security_unlock_fallback_pass(
+            self, mocked_execute):
+        hdparm_output = create_hdparm_info(
+            supported=True, enabled=True, locked=True
+        )
+        hdparm_output_unlocked = create_hdparm_info(
+            supported=True, enabled=True, frozen=False, enhanced_erase=False)
+        hdparm_output_not_enabled = create_hdparm_info(
+            supported=True, enabled=False, frozen=False, enhanced_erase=False)
+        mocked_execute.side_effect = [
+            (hdparm_output, ''),
+            processutils.ProcessExecutionError(),  # NULL fails to unlock
+            (hdparm_output, ''),  # recheck security lines
+            None,  # security unlock with ""
+            (hdparm_output_unlocked, ''),
+            '',
+            (hdparm_output_not_enabled, '')
+        ]
+
+        block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,
+                                            True)
+
+        self.hardware.erase_block_device(self.node, block_device)
+
+        mocked_execute.assert_any_call('hdparm', '--user-master', 'u',
+                                       '--security-unlock', '', '/dev/sda')
+
     @mock.patch.object(hardware.GenericHardwareManager, '_shred_block_device',
                        autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test_erase_block_device_ata_security_enabled(
             self, mocked_execute, mock_shred):
+        # Tests that an exception is thrown if all of the recovery passwords
+        # fail to unlock the device without throwing exception
         hdparm_output = create_hdparm_info(
-            supported=True, enabled=True, frozen=False, enhanced_erase=False)
+            supported=True, enabled=True, locked=True)
 
         mocked_execute.side_effect = [
             (hdparm_output, ''),
             None,
+            (hdparm_output, ''),
+            None,
+            (hdparm_output, ''),
+            None,
             (hdparm_output, '')
         ]
 
         block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,
                                             True)
-
         self.assertRaises(
             errors.IncompatibleHardwareMethodError,
             self.hardware.erase_block_device,
             self.node,
             block_device)
+        mocked_execute.assert_any_call('hdparm', '--user-master', 'u',
+                                       '--security-unlock', '', '/dev/sda')
+        mocked_execute.assert_any_call('hdparm', '--user-master', 'u',
+                                       '--security-unlock', 'NULL', '/dev/sda')
         self.assertFalse(mock_shred.called)
 
     @mock.patch.object(hardware.GenericHardwareManager, '_shred_block_device',
@@ -1471,7 +1508,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
     def test_erase_block_device_ata_security_enabled_unlock_attempt(
             self, mocked_execute, mock_shred):
         hdparm_output = create_hdparm_info(
-            supported=True, enabled=True, frozen=False, enhanced_erase=False)
+            supported=True, enabled=True, locked=True)
         hdparm_output_not_enabled = create_hdparm_info(
             supported=True, enabled=False, frozen=False, enhanced_erase=False)
 
@@ -1493,34 +1530,36 @@ class TestGenericHardwareManager(base.IronicAgentTest):
     @mock.patch.object(utils, 'execute', autospec=True)
     def test__ata_erase_security_enabled_unlock_exception(
             self, mocked_execute):
+        # test that an exception is thrown when security unlock fails with
+        # ProcessExecutionError
         hdparm_output = create_hdparm_info(
-            supported=True, enabled=True, frozen=False, enhanced_erase=False)
-
+            supported=True, enabled=True, locked=True)
         mocked_execute.side_effect = [
             (hdparm_output, ''),
-            processutils.ProcessExecutionError()
+            processutils.ProcessExecutionError(),
+            (hdparm_output, ''),
+            processutils.ProcessExecutionError(),
+            (hdparm_output, ''),
         ]
 
         block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,
                                             True)
-
         self.assertRaises(errors.BlockDeviceEraseError,
                           self.hardware._ata_erase,
                           block_device)
+        mocked_execute.assert_any_call('hdparm', '--user-master', 'u',
+                                       '--security-unlock', '', '/dev/sda')
+        mocked_execute.assert_any_call('hdparm', '--user-master', 'u',
+                                       '--security-unlock', 'NULL', '/dev/sda')
 
     @mock.patch.object(utils, 'execute', autospec=True)
     def test__ata_erase_security_enabled_set_password_exception(
             self, mocked_execute):
         hdparm_output = create_hdparm_info(
-            supported=True, enabled=True, frozen=False, enhanced_erase=False)
-        hdparm_output_not_enabled = create_hdparm_info(
             supported=True, enabled=False, frozen=False, enhanced_erase=False)
 
         mocked_execute.side_effect = [
             (hdparm_output, ''),
-            '',
-            (hdparm_output_not_enabled, ''),
-            '',
             processutils.ProcessExecutionError()
         ]
 
@@ -1534,17 +1573,14 @@ class TestGenericHardwareManager(base.IronicAgentTest):
     @mock.patch.object(utils, 'execute', autospec=True)
     def test__ata_erase_security_erase_exec_exception(
             self, mocked_execute):
+        # Exception on security erase
         hdparm_output = create_hdparm_info(
-            supported=True, enabled=True, frozen=False, enhanced_erase=False)
-        hdparm_output_not_enabled = create_hdparm_info(
             supported=True, enabled=False, frozen=False, enhanced_erase=False)
 
         mocked_execute.side_effect = [
             (hdparm_output, '', '-1'),
-            '',
-            (hdparm_output_not_enabled, ''),
-            '',
-            processutils.ProcessExecutionError()
+            '',  # security-set-pass
+            processutils.ProcessExecutionError()  # security-erase
         ]
 
         block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,
@@ -2003,8 +2039,8 @@ class TestModuleFunctions(base.IronicAgentTest):
             mock.call('iscsistart', '-f')])
 
 
-def create_hdparm_info(supported=False, enabled=False, frozen=False,
-                       enhanced_erase=False):
+def create_hdparm_info(supported=False, enabled=False, locked=False,
+                       frozen=False, enhanced_erase=False):
 
     def update_values(values, state, key):
         if not state:
@@ -2013,12 +2049,14 @@ def create_hdparm_info(supported=False, enabled=False, frozen=False,
     values = {
         'supported': '\tsupported',
         'enabled': '\tenabled',
+        'locked': '\tlocked',
         'frozen': '\tfrozen',
         'enhanced_erase': '\tsupported: enhanced erase',
     }
 
     update_values(values, supported, 'supported')
     update_values(values, enabled, 'enabled')
+    update_values(values, locked, 'locked')
     update_values(values, frozen, 'frozen')
     update_values(values, enhanced_erase, 'enhanced_erase')