diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index a287f9c293..fdf36003d3 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -907,9 +907,19 @@ # set to 0, will not run during cleaning. (integer value) #erase_devices_priority = -# Number of iterations to be run for erasing devices. (integer -# value) -#erase_devices_iterations = 1 +# During shred, overwrite all block devices N times with +# random data. This is only used if a device could not be ATA +# Secure Erased. Defaults to 1. (integer value) +# Minimum value: 0 +# Deprecated group/name - [deploy]/erase_devices_iterations +#shred_random_overwrite_iterations = 1 + +# Whether to write zeros to a node's block devices after +# writing random data. This will write zeros to the device +# even when deploy.shred_random_overwrite_interations is 0. +# This option is only used if a device could not be ATA Secure +# Erased. Defaults to True. (boolean value) +#shred_final_overwrite_with_zeros = true # Whether to power off a node after deploy failure. Defaults # to True. (boolean value) diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 0d6819dd90..b0b2dc93ee 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -49,12 +49,12 @@ from ironic import objects deploy_opts = [ cfg.StrOpt('http_url', - help='ironic-conductor node\'s HTTP server URL. ' - 'Example: http://192.1.2.3:8080', + help=_("ironic-conductor node's HTTP server URL. " + "Example: http://192.1.2.3:8080"), deprecated_group='pxe'), cfg.StrOpt('http_root', default='/httpboot', - help='ironic-conductor node\'s HTTP root path.', + help=_("ironic-conductor node's HTTP root path."), deprecated_group='pxe'), cfg.IntOpt('erase_devices_priority', help=_('Priority to run in-band erase devices via the Ironic ' @@ -62,9 +62,23 @@ deploy_opts = [ 'set in the ramdisk (defaults to 10 for the ' 'GenericHardwareManager). If set to 0, will not run ' 'during cleaning.')), - cfg.IntOpt('erase_devices_iterations', + # TODO(mmitchell): Remove the deprecated name/group during Ocata cycle. + cfg.IntOpt('shred_random_overwrite_iterations', + deprecated_name='erase_devices_iterations', + deprecated_group='deploy', default=1, - help=_('Number of iterations to be run for erasing devices.')), + min=0, + help=_('During shred, overwrite all block devices N times with ' + 'random data. This is only used if a device could not ' + 'be ATA Secure Erased. Defaults to 1.')), + cfg.BoolOpt('shred_final_overwrite_with_zeros', + default=True, + help=_("Whether to write zeros to a node's block devices " + "after writing random data. This will write zeros to " + "the device even when " + "deploy.shred_random_overwrite_interations is 0. This " + "option is only used if a device could not be ATA " + "Secure Erased. Defaults to True.")), cfg.BoolOpt('power_off_after_deploy_failure', default=True, help=_('Whether to power off a node after deploy failure. ' @@ -97,6 +111,19 @@ SUPPORTED_CAPABILITIES = { DISK_LAYOUT_PARAMS = ('root_gb', 'swap_mb', 'ephemeral_gb') + +def warn_about_unsafe_shred_parameters(): + iterations = CONF.deploy.shred_random_overwrite_iterations + overwrite_with_zeros = CONF.deploy.shred_final_overwrite_with_zeros + if iterations == 0 and overwrite_with_zeros is False: + LOG.warning(_LW('With shred_random_overwrite_iterations set to 0 and ' + 'shred_final_overwrite_with_zeros set to False, disks ' + 'may NOT be shredded at all, unless they support ATA ' + 'Secure Erase. This is a possible SECURITY ISSUE!')) + + +warn_about_unsafe_shred_parameters() + # All functions are called from deploy() directly or indirectly. # They are split for stub-out. @@ -629,8 +656,12 @@ def agent_add_clean_params(task): :param task: a TaskManager instance. """ info = task.node.driver_internal_info - passes = CONF.deploy.erase_devices_iterations - info['agent_erase_devices_iterations'] = passes + + random_iterations = CONF.deploy.shred_random_overwrite_iterations + info['agent_erase_devices_iterations'] = random_iterations + zeroize = CONF.deploy.shred_final_overwrite_with_zeros + info['agent_erase_devices_zeroize'] = zeroize + task.node.driver_internal_info = info task.node.save() diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 18bcfcab8c..372479a4df 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -1287,7 +1287,8 @@ class OtherFunctionTestCase(db_base.DbTestCase): log_calls=calls) def test_set_failed_state_no_poweroff(self): - cfg.CONF.deploy.power_off_after_deploy_failure = False + cfg.CONF.set_override('power_off_after_deploy_failure', False, + 'deploy') exc_state = exception.InvalidState('invalid state') exc_param = exception.InvalidParameterValue('invalid parameter') mock_call = mock.call(mock.ANY) @@ -1343,6 +1344,37 @@ class OtherFunctionTestCase(db_base.DbTestCase): mock_clean_up_caches.assert_called_once_with(None, 'master_dir', [('uuid', 'path')]) + @mock.patch.object(utils, 'LOG', autospec=True) + def test_warn_about_unsafe_shred_parameters_defaults(self, log_mock): + utils.warn_about_unsafe_shred_parameters() + self.assertFalse(log_mock.warning.called) + + @mock.patch.object(utils, 'LOG', autospec=True) + def test_warn_about_unsafe_shred_parameters_zeros(self, log_mock): + cfg.CONF.set_override('shred_random_overwrite_iterations', 0, 'deploy') + cfg.CONF.set_override('shred_final_overwrite_with_zeros', True, + 'deploy') + utils.warn_about_unsafe_shred_parameters() + self.assertFalse(log_mock.warning.called) + + @mock.patch.object(utils, 'LOG', autospec=True) + def test_warn_about_unsafe_shred_parameters_random_no_zeros(self, + log_mock): + cfg.CONF.set_override('shred_random_overwrite_iterations', 1, 'deploy') + cfg.CONF.set_override('shred_final_overwrite_with_zeros', False, + 'deploy') + utils.warn_about_unsafe_shred_parameters() + self.assertFalse(log_mock.warning.called) + + @mock.patch.object(utils, 'LOG', autospec=True) + def test_warn_about_unsafe_shred_parameters_produces_a_warning(self, + log_mock): + cfg.CONF.set_override('shred_random_overwrite_iterations', 0, 'deploy') + cfg.CONF.set_override('shred_final_overwrite_with_zeros', False, + 'deploy') + utils.warn_about_unsafe_shred_parameters() + self.assertTrue(log_mock.warning.called) + class VirtualMediaDeployUtilsTestCase(db_base.DbTestCase): @@ -1680,12 +1712,16 @@ class AgentMethodsTestCase(db_base.DbTestCase): self.assertEqual(states.CLEANWAIT, response) def test_agent_add_clean_params(self): - cfg.CONF.deploy.erase_devices_iterations = 2 + cfg.CONF.set_override('shred_random_overwrite_iterations', 2, 'deploy') + cfg.CONF.set_override('shred_final_overwrite_with_zeros', False, + 'deploy') with task_manager.acquire( self.context, self.node.uuid, shared=False) as task: utils.agent_add_clean_params(task) - self.assertEqual(task.node.driver_internal_info.get( - 'agent_erase_devices_iterations'), 2) + self.assertEqual(2, task.node.driver_internal_info.get( + 'agent_erase_devices_iterations')) + self.assertEqual(False, task.node.driver_internal_info.get( + 'agent_erase_devices_zeroize')) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports', autospec=True) @@ -1749,8 +1785,10 @@ class AgentMethodsTestCase(db_base.DbTestCase): utils.prepare_inband_cleaning(task, manage_boot=manage_boot)) prepare_cleaning_ports_mock.assert_called_once_with(task) power_mock.assert_called_once_with(task, states.REBOOT) - self.assertEqual(task.node.driver_internal_info.get( - 'agent_erase_devices_iterations'), 1) + self.assertEqual(1, task.node.driver_internal_info.get( + 'agent_erase_devices_iterations')) + self.assertEqual(True, task.node.driver_internal_info.get( + 'agent_erase_devices_zeroize')) if manage_boot: prepare_ramdisk_mock.assert_called_once_with( mock.ANY, mock.ANY, {'a': 'b', 'c': 'd'}) diff --git a/releasenotes/notes/shred-final-overwrite-with-zeros-50b5ba5b19c0da27.yaml b/releasenotes/notes/shred-final-overwrite-with-zeros-50b5ba5b19c0da27.yaml new file mode 100644 index 0000000000..1d836dc33a --- /dev/null +++ b/releasenotes/notes/shred-final-overwrite-with-zeros-50b5ba5b19c0da27.yaml @@ -0,0 +1,14 @@ +--- +features: + - A new configuration option, `shred_final_overwrite_with_zeros` is now + available. This option controls the final overwrite with zeros done on + all block devices for a node under cleaning. This feature was previously + always enabled and not configurable. This option is only used when a + block device could not be ATA Secure Erased. +deprecations: + - The [deploy]/erase_devices_iterations config is deprecated and will + be removed in the Ocata cycle. It has been replaced by the + [deploy]/shred_random_overwrite_iterations config. This configuration + option controls the number of times block devices are overwritten with + random data. This option is only used when a block device could not be + ATA Secure Erased.