Merge "rework ATA secure erase"
This commit is contained in:
Zuul
Gerrit Code Review
commit
696d1cf421
|
|
@ -898,39 +898,57 @@ class GenericHardwareManager(HardwareManager):
|
|||
if 'supported' not in security_lines:
|
||||
return False
|
||||
|
||||
if 'enabled' in security_lines:
|
||||
# Attempt to unlock the drive in the event it has already been
|
||||
# locked by a previous failed attempt.
|
||||
try:
|
||||
utils.execute('hdparm', '--user-master', 'u',
|
||||
'--security-unlock', 'NULL', block_device.name)
|
||||
security_lines = self._get_ata_security_lines(block_device)
|
||||
except processutils.ProcessExecutionError as e:
|
||||
raise errors.BlockDeviceEraseError('Security password set '
|
||||
'failed for device '
|
||||
'%(name)s: %(err)s' %
|
||||
{'name': block_device.name,
|
||||
'err': e})
|
||||
|
||||
if 'enabled' in security_lines:
|
||||
raise errors.BlockDeviceEraseError(
|
||||
('Block device {} already has a security password set'
|
||||
).format(block_device.name))
|
||||
# At this point, we could be SEC1,2,4,5,6
|
||||
|
||||
if 'not frozen' not in security_lines:
|
||||
# In SEC2 or 6
|
||||
raise errors.BlockDeviceEraseError(
|
||||
('Block device {} is frozen and cannot be erased'
|
||||
).format(block_device.name))
|
||||
|
||||
try:
|
||||
utils.execute('hdparm', '--user-master', 'u',
|
||||
'--security-set-pass', 'NULL', block_device.name)
|
||||
except processutils.ProcessExecutionError as e:
|
||||
raise errors.BlockDeviceEraseError('Security password set '
|
||||
'failed for device '
|
||||
'%(name)s: %(err)s' %
|
||||
{'name': block_device.name,
|
||||
'err': e})
|
||||
# At this point, we could be in SEC1,4,5
|
||||
|
||||
# Attempt to unlock the drive in the event it has already been
|
||||
# locked by a previous failed attempt. We try the empty string as
|
||||
# versions of hdparm < 9.51, interpreted NULL as the literal string,
|
||||
# "NULL", as opposed to the empty string.
|
||||
unlock_passwords = ['NULL', '']
|
||||
for password in unlock_passwords:
|
||||
if 'not locked' in security_lines:
|
||||
break
|
||||
try:
|
||||
utils.execute('hdparm', '--user-master', 'u',
|
||||
'--security-unlock', password,
|
||||
block_device.name)
|
||||
except processutils.ProcessExecutionError as e:
|
||||
LOG.info('Security unlock failed for device '
|
||||
'%(name)s using password "%(password)s": %(err)s',
|
||||
{'name': block_device.name,
|
||||
'password': password,
|
||||
'err': e})
|
||||
security_lines = self._get_ata_security_lines(block_device)
|
||||
|
||||
# If the unlock failed we will still be in SEC4, otherwise, we will be
|
||||
# in SEC1 or SEC5
|
||||
|
||||
if 'not locked' not in security_lines:
|
||||
# In SEC4
|
||||
raise errors.BlockDeviceEraseError(
|
||||
('Block device {} already has a security password set'
|
||||
).format(block_device.name))
|
||||
|
||||
# At this point, we could be in SEC1 or 5
|
||||
if 'not enabled' in security_lines:
|
||||
# SEC1. Try to transition to SEC5 by setting empty user
|
||||
# password.
|
||||
try:
|
||||
utils.execute('hdparm', '--user-master', 'u',
|
||||
'--security-set-pass', 'NULL', block_device.name)
|
||||
except processutils.ProcessExecutionError as e:
|
||||
error_msg = ('Security password set failed for device '
|
||||
'{name}: {err}'
|
||||
).format(name=block_device.name, err=e)
|
||||
raise errors.BlockDeviceEraseError(error_msg)
|
||||
|
||||
# Use the 'enhanced' security erase option if it's supported.
|
||||
erase_option = '--security-erase'
|
||||
|
|
@ -949,10 +967,12 @@ class GenericHardwareManager(HardwareManager):
|
|||
# Verify that security is now 'not enabled'
|
||||
security_lines = self._get_ata_security_lines(block_device)
|
||||
if 'not enabled' not in security_lines:
|
||||
# Not SEC1 - fail
|
||||
raise errors.BlockDeviceEraseError(
|
||||
('An unknown error occurred erasing block device {}'
|
||||
).format(block_device.name))
|
||||
|
||||
# In SEC1 security state
|
||||
return True
|
||||
|
||||
def get_bmc_address(self):
|
||||
|
|
|
|||
|
|
@ -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,15 +1441,49 @@ 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, '')
|
||||
|
|
@ -1457,12 +1491,15 @@ class TestGenericHardwareManager(base.IronicAgentTest):
|
|||
|
||||
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')
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue