From f5413a9bd5fe51c0d37ff7b7ed482f2ae6ed6b2a Mon Sep 17 00:00:00 2001 From: ankit Date: Tue, 23 Mar 2021 07:50:00 +0000 Subject: [PATCH] Add security dashboard clean steps to ilo drivers This commit adds new clean steps security_parameters_update, update_minimum_password_length and update_auth_failure_logging_threshold to allow users to edit following security parameters which fetched during node inspection - ``Password_Complexity``, ``RequiredLoginForiLORBSU``, ``RequireHostAuthentication``, ``MinPasswordLength``, ``IPMI/DCMI_Over_LAN``, ``Authentication_failure_Logging``, and ``Secure_Boot``. Story: 2008024 Task: 40736 Change-Id: I0dd9a83ee23c6b846eda3ff171ab7b3138b22fa7 --- doc/source/admin/drivers/ilo.rst | 131 ++++++++++++++++++ ironic/drivers/modules/ilo/common.py | 40 ++++++ ironic/drivers/modules/ilo/inspect.py | 27 ++++ ironic/drivers/modules/ilo/management.py | 127 +++++++++++++++++ .../unit/drivers/modules/ilo/test_common.py | 20 +++ .../unit/drivers/modules/ilo/test_inspect.py | 13 +- .../drivers/modules/ilo/test_management.py | 42 ++++++ ...ity-param-clean-step-00d5548072a397f2.yaml | 7 + 8 files changed, 404 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/adding-security-param-clean-step-00d5548072a397f2.yaml diff --git a/doc/source/admin/drivers/ilo.rst b/doc/source/admin/drivers/ilo.rst index 17d356d0e0..735ff97024 100644 --- a/doc/source/admin/drivers/ilo.rst +++ b/doc/source/admin/drivers/ilo.rst @@ -52,6 +52,9 @@ The hardware type ``ilo`` supports following HPE server features: * `Disk Erase Support`_ * `Initiating firmware update as manual clean step`_ * `Smart Update Manager (SUM) based firmware update`_ +* `Updating security parameters as manual clean step`_ +* `Update Minimum Password Length security parameter as manual clean step`_ +* `Update Authentication Failure Logging security parameter as manual clean step`_ * `Activating iLO Advanced license as manual clean step`_ * `Firmware based UEFI iSCSI boot from volume support`_ * `Certificate based validation in iLO`_ @@ -725,6 +728,17 @@ Supported **Manual** Cleaning Operations using Smart Update Manager (SUM). It is an inband step associated with the ``management`` interface. See `Smart Update Manager (SUM) based firmware update`_ for more information on usage. + ``security_parameters_update``: + Updates the Security Parameters. See `Updating security parameters as manual clean step`_ + for user guidance on usage. The supported security parameters for this clean step are: + ``Password_Complexity``, ``RequiredLoginForiLORBSU``, ``IPMI/DCMI_Over_LAN``, + ``RequireHostAuthentication`` and ``Secure_Boot``. + ``update_minimum_password_length``: + Updates the Minimum Password Length security parameter. See + `Update Minimum Password Length security parameter as manual clean step`_ for user guidance on usage. + ``update_auth_failure_logging_threshold``: + Updates the Authentication Failure Logging security parameter. See + `Update Authentication Failure Logging security parameter as manual clean step`_ for user guidance on usage. * iLO with firmware version 1.5 is minimally required to support all the operations. @@ -1646,6 +1660,123 @@ where things failed. You can then fix or work around and then try again. Refer `Guidelines for SPP ISO`_ for steps to get SPP (Service Pack for ProLiant) ISO. +Updating security parameters as manual clean step +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +iLO driver can invoke security parameters update as a manual clean step. Any +manual cleaning step can only be initiated when a node is in the ``manageable`` +state. Once the manual cleaning is finished, the node will be put in the +``manageable`` state again. A user can follow steps from :ref:`manual_cleaning` +to initiate manual cleaning operation on a node. This feature is only supported +for ``ilo5`` based hardware. + +An example of a manual clean step with ``security_parameters_update`` as the +only clean step could be:: + + "clean_steps": [{ + "interface": "management", + "step": "security_parameters_update", + "args": { + "security_parameters":[ + { + "param": "password_complexity", + "enable": "True", + "ignore": "False" + }, + { + "param": "require_login_for_ilo_rbsu", + "enable": "True", + "ignore": "False" + }, + { + "param": "ipmi_over_lan", + "enable": "True", + "ignore": "False" + }, + { + "param": "secure_boot", + "enable": "True", + "ignore": "False" + }, + { + "param": "require_host_authentication", + "enable": "True", + "ignore": "False" + } + ] + } + }] + +The different attributes of ``security_parameters_update`` clean step are as follows: + +.. csv-table:: + :header: "Attribute", "Description" + :widths: 30, 120 + + "``interface``", "Interface of clean step, here ``management``" + "``step``", "Name of clean step, here ``security_parameters_update``" + "``args``", "Keyword-argument entry (: ) being passed to clean step" + "``args.security_parameters``", "Ordered list of dictionaries of security parameters to be updated. This is mandatory." + +Each security parameter block is represented by a dictionary (JSON), in the form:: + + { + "param": "", + "enable": "security parameter to be enabled/disabled", + "ignore": "security parameter status to be ignored or not" + } + +In all of these fields, ``param`` field is mandatory. Remaining fields are boolean and are optional. If user doesn't +pass any value then for ``enable`` field the default will be True and for ``ignore`` field default will be False. + +* The Security Parameters which are supported for this clean step are: + ``Password_Complexity``, ``RequiredLoginForiLORBSU``, ``RequireHostAuthentication``, ``IPMI/DCMI_Over_LAN`` and ``Secure_Boot``. + +Update Minimum Password Length security parameter as manual clean step +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +iLO driver can invoke ``Minimum Password Length`` security parameter update as a manual clean +step. This feature is only supported for ``ilo5`` based hardware. + +An example of a manual clean step with ``update_minimum_password_length`` as the +only clean step could be:: + + "clean_steps": [{ + "interface": "management", + "step": "update_minimum_password_length", + "args": { + "password_length": "8", + "ignore": "False" + } + }] + +Both the arguments ``password_length`` and ``ignore`` are optional. The accepted values for password_length are +0 to 39. If user doesn't pass any value, the default value for password_length will be 8 and for ignore the default +value be False. + +Update Authentication Failure Logging security parameter as manual clean step +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +iLO driver can invoke ``Authentication Failure Logging`` security parameter update as a manual clean +step. This feature is only supported for ``ilo5`` based hardware. + +An example of a manual clean step with ``Authentication Failure Logging`` as the +only clean step could be:: + + "clean_steps": [{ + "interface": "management", + "step": "update_auth_failure_logging_threshold", + "args": { + "logging_threshold": "1", + "ignore": "False" + } + }] + +Both the arguments ``logging_threshold`` and ``ignore`` are optional. The accepted values for logging_threshold are +0 to 5. If user doesn't pass any value, the default value for logging_threshold will be 1 and for ignore the default +value be False. If user passes the value of logging_threshold as 0, the Authentication Failure Logging security +parameter will be disabled. + RAID Support ^^^^^^^^^^^^ diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index 8a390937a1..69161499b6 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -135,6 +135,46 @@ SUPPORTED_BOOT_MODE_UEFI_ONLY = 'uefi only' SUPPORTED_BOOT_MODE_LEGACY_BIOS_AND_UEFI = 'legacy bios and uefi' """ Node supports both legacy BIOS and UEFI boot mode.""" +SUPPORTED_SECURITY_PARAMETERS = ( + ['password_complexity', 'require_login_for_ilo_rbsu', 'ipmi_over_lan', + 'secure_boot', 'require_host_authentication']) + + +def validate_security_parameter_values(sec_param_info): + """Validate security parameter with valid values. + + :param sec_param_info: dict object containing the security parameter info + :raises: MissingParameterValue, for missing fields (or values) in + security parameter info. + :raises: InvalidParameterValue, for unsupported security parameter + :returns: tuple of security param, ignore and enable parameters. + """ + info = sec_param_info or {} + + LOG.debug("Validating security param info: %s in progress", info) + param = info.get('param') + + if not param: + msg = (_("Security parameter info: %(info)s is missing the " + "required 'param' field.") % {'info': info}) + LOG.error(msg) + raise exception.MissingParameterValue(msg) + + if param not in SUPPORTED_SECURITY_PARAMETERS: + msg = (_("Security parameter '%(param)s' is not a valid parameter. " + "Supported values are: %(supported_params)s") % + {'param': param, 'supported_params': ( + ", ".join(SUPPORTED_SECURITY_PARAMETERS))}) + LOG.error(msg) + raise exception.InvalidParameterValue(msg) + + ignored = info.get('ignore', False) + ignored = strutils.bool_from_string(ignored, default=False) + enable = info.get('enable', True) + enable = strutils.bool_from_string(enable, default=True) + + return param, enable, ignored + def copy_image_to_web_server(source_file_path, destination): """Copies the given image to the http web server. diff --git a/ironic/drivers/modules/ilo/inspect.py b/ironic/drivers/modules/ilo/inspect.py index 1e41a12261..7f79d3bcdb 100644 --- a/ironic/drivers/modules/ilo/inspect.py +++ b/ironic/drivers/modules/ilo/inspect.py @@ -155,6 +155,23 @@ def _get_capabilities(node, ilo_object): return capabilities +def _get_security_parameters(node, ilo_object): + """inspect hardware and gets security parameter information. + + :param node: Node object. + :param ilo_object: an instance of ilo drivers. + :returns: a dictionary of security parameters. + """ + security_params = None + try: + security_params = ilo_object.get_security_dashboard_values() + except ilo_error.IloError: + LOG.debug("Node %s did not return any security parameters.", + node.uuid) + + return security_params + + class IloInspect(base.InspectInterface): def get_properties(self): @@ -252,6 +269,16 @@ class IloInspect(base.InspectInterface): node_properties['capabilities'] = capabilities task.node.properties = node_properties + # Inspect the hardware for security parameters related information. + # Since it applies only for Gen10 based hardware, the method + # inspect_hardware() doesn't raise an error. + if model and 'Gen10' in model: + security_params = _get_security_parameters(task.node, ilo_object) + if security_params: + node_properties['security_parameters'] = ( + security_params.get('security_parameters')) + task.node.properties = node_properties + # RIBCL(Gen8) protocol cannot determine if a NIC # is physically connected with cable or not when the server # is not provisioned. RIS(Gen9) can detect the same for few NIC diff --git a/ironic/drivers/modules/ilo/management.py b/ironic/drivers/modules/ilo/management.py index 9787b2057d..28fef03bb5 100644 --- a/ironic/drivers/modules/ilo/management.py +++ b/ironic/drivers/modules/ilo/management.py @@ -22,6 +22,7 @@ from oslo_log import log as logging from oslo_service import loopingcall from oslo_utils import excutils from oslo_utils import importutils +from oslo_utils import strutils from ironic.common import boot_devices from ironic.common import boot_modes @@ -78,6 +79,62 @@ _RESET_ILO_CREDENTIALS_ARGSINFO = { } } +_SECURITY_PARAMETER_UPDATE_ARGSINFO = { + 'security_parameters': { + 'description': ( + "This argument represents the ordered list of JSON " + "dictionaries of security parameters. Each security " + "parameter consists of three fields, namely 'param', " + "'ignore' and 'enable' from which 'param' field will be " + "mandatory. These fields represent security parameter " + "name, ignore flag and state of the security parameter. " + "The supported security parameter names are " + "'password_complexity', 'require_login_for_ilo_rbsu', " + "'ipmi_over_lan', 'secure_boot', 'require_host_authentication'. " + "The security parameters will be updated (in the order given) " + "one by one on the baremetal server." + ), + 'required': True + } +} + +_MINIMUM_PASSWORD_LENGTH_UPDATE_ARGSINFO = { + 'password_length': { + 'description': ( + "This argument represents the minimum password length that can " + "be set for ilo. If not specified, default will be 8." + ), + 'required': False + }, + 'ignore': { + 'description': ( + "This argument represents boolean parameter. If set 'True' the " + "security parameters will be ignored. If not specified, default " + "will be 'False'." + ), + 'required': False + } +} + +_Auth_Failure_Logging_Threshold_ARGSINFO = { + 'logging_threshold': { + 'description': ( + "This argument represents the authentication failure " + "logging threshold that can be set for ilo. If not " + "specified, default will be 1." + ), + 'required': False + }, + 'ignore': { + 'description': ( + "This argument represents boolean parameter. If set 'True' the " + "security parameters will be ignored. If not specified, default " + "will be 'False'." + ), + 'required': False + } +} + _FIRMWARE_UPDATE_ARGSINFO = { 'firmware_update_mode': { 'description': ( @@ -432,6 +489,76 @@ class IloManagement(base.ManagementInterface): _execute_ilo_step(node, 'activate_license', ilo_license_key) LOG.info("iLO license activated for node %s.", node.uuid) + @METRICS.timer('IloManagement.security_parameters_update') + @base.clean_step(priority=0, abortable=False, + argsinfo=_SECURITY_PARAMETER_UPDATE_ARGSINFO) + def security_parameters_update(self, task, **kwargs): + """Updates the security parameters. + + :param task: a TaskManager object. + """ + node = task.node + security_parameter = kwargs.get('security_parameters') + try: + for sec_param_info in security_parameter: + param, enable, ignore = ( + ilo_common.validate_security_parameter_values( + sec_param_info)) + LOG.debug("Updating %(param)s security parameter for node " + "%(node)s ..", {'param': param, 'node': node.uuid}) + _execute_ilo_step(node, ('update_' + param), enable, ignore) + LOG.info("%(param)s security parameter for node %(node)s is " + "updated", {'param': param, 'node': node.uuid}) + except (exception.MissingParameterValue, + exception.InvalidParameterValue, + exception.NodeCleaningFailure): + LOG.error("%(param)s security parameter updation for " + "node: %(node)s failed.", + {'param': param, 'node': node.uuid}) + raise + + @METRICS.timer('IloManagement.update_minimum_password_length') + @base.clean_step(priority=0, abortable=False, + argsinfo=_MINIMUM_PASSWORD_LENGTH_UPDATE_ARGSINFO) + def update_minimum_password_length(self, task, **kwargs): + """Updates the Minimum Password Length security parameter. + + :param task: a TaskManager object. + """ + node = task.node + passwd_length = kwargs.get('password_length') + ignore = kwargs.get('ignore', False) + ignore = strutils.bool_from_string(ignore, default=False) + + LOG.debug("Updating minimum password length security parameter " + "for node %(node)s ..", {'node': node.uuid}) + _execute_ilo_step(node, 'update_minimum_password_length', + passwd_length, ignore) + LOG.info("Minimum password length security parameter for node " + "%(node)s is updated", {'node': node.uuid}) + + @METRICS.timer('IloManagement.update_auth_failure_logging_threshold') + @base.clean_step(priority=0, abortable=False, + argsinfo=_Auth_Failure_Logging_Threshold_ARGSINFO) + def update_auth_failure_logging_threshold(self, task, **kwargs): + """Updates the Auth Failure Logging Threshold security parameter. + + :param task: a TaskManager object. + """ + node = task.node + logging_threshold = kwargs.get('logging_threshold') + ignore = kwargs.get('ignore', False) + ignore = strutils.bool_from_string(ignore, default=False) + + LOG.debug("Updating authentication failure logging threshold " + "security parameter for node %(node)s ..", + {'node': node.uuid}) + _execute_ilo_step(node, 'update_authentication_failure_logging', + logging_threshold, ignore) + LOG.info("Authentication failure logging threshold security " + "parameter for node %(node)s is updated", + {'node': node.uuid}) + @METRICS.timer('IloManagement.update_firmware') @base.deploy_step(priority=0, argsinfo=_FIRMWARE_UPDATE_ARGSINFO) @base.clean_step(priority=0, abortable=False, diff --git a/ironic/tests/unit/drivers/modules/ilo/test_common.py b/ironic/tests/unit/drivers/modules/ilo/test_common.py index d8dfc71e01..f1d9cd241e 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_common.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_common.py @@ -904,6 +904,26 @@ class IloCommonMethodsTestCase(BaseIloTest): task, False) ilo_mock_object.set_secure_boot_mode.assert_called_once_with(False) + def test_validate_security_parameter_values(self): + param_info = {'param': 'password_complexity'} + param, enabled, ignored = ( + ilo_common.validate_security_parameter_values(param_info)) + self.assertEqual('password_complexity', param) + self.assertEqual(False, ignored) + self.assertEqual(True, enabled) + + def test_validate_security_parameter_values_no_param(self): + param_info = {'param': None} + self.assertRaises(exception.MissingParameterValue, + ilo_common.validate_security_parameter_values, + param_info) + + def test_validate_security_parameter_values_invalid_param(self): + param_info = {'param': 'minimum_password_length'} + self.assertRaises(exception.InvalidParameterValue, + ilo_common.validate_security_parameter_values, + param_info) + @mock.patch.object(os, 'chmod', spec_set=True, autospec=True) @mock.patch.object(shutil, 'copyfile', spec_set=True, diff --git a/ironic/tests/unit/drivers/modules/ilo/test_inspect.py b/ironic/tests/unit/drivers/modules/ilo/test_inspect.py index ce81a2e474..376c0d601d 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_inspect.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_inspect.py @@ -172,6 +172,8 @@ class IloInspectTestCase(test_common.BaseIloTest): @mock.patch.object(ilo_inspect.LOG, 'warning', spec_set=True, autospec=True) + @mock.patch.object(ilo_inspect, '_get_security_parameters', + spec_set=True, autospec=True) @mock.patch.object(ilo_inspect, '_get_capabilities', spec_set=True, autospec=True) @mock.patch.object(inspect_utils, 'create_ports_if_not_exist', @@ -187,21 +189,26 @@ class IloInspectTestCase(test_common.BaseIloTest): get_essential_mock, create_port_mock, get_capabilities_mock, + get_security_params_mock, log_mock): ilo_object_mock = get_ilo_object_mock.return_value properties = {'memory_mb': '512', 'local_gb': 10, 'cpus': '1', 'cpu_arch': 'x86_64'} macs = {'NIC.LOM.1.1': 'aa:aa:aa:aa:aa:aa'} capabilities = {'server_model': 'Gen10'} + security_params = ( + {'security_parameters': {'Password Complexity': 'ok'}}) result = {'properties': properties, 'macs': macs} get_essential_mock.return_value = result get_capabilities_mock.return_value = capabilities + get_security_params_mock.return_value = security_params power_mock.return_value = states.POWER_ON with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - expected_properties = {'memory_mb': '512', 'local_gb': 10, - 'cpus': '1', 'cpu_arch': 'x86_64', - 'capabilities': 'server_model:Gen10'} + expected_properties = { + 'memory_mb': '512', 'local_gb': 10, 'cpus': '1', + 'cpu_arch': 'x86_64', 'capabilities': 'server_model:Gen10', + 'security_parameters': {'Password Complexity': 'ok'}} task.driver.inspect.inspect_hardware(task) self.assertEqual(expected_properties, task.node.properties) power_mock.assert_called_once_with(mock.ANY, task) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_management.py b/ironic/tests/unit/drivers/modules/ilo/test_management.py index 1616f8ca04..07bb9f98b2 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_management.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_management.py @@ -382,6 +382,48 @@ class IloManagementTestCase(test_common.BaseIloTest): **activate_license_args) self.assertFalse(step_mock.called) + @mock.patch.object(ilo_management, '_execute_ilo_step', + spec_set=True, autospec=True) + def test_security_parameters_update(self, step_mock): + security_parameters = [ + { + "param": "password_complexity", + "enable": True, + "ignore": False + } + ] + security_parameters_args = { + 'security_parameters': security_parameters} + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.management.security_parameters_update( + task, **security_parameters_args) + step_mock.assert_called_once_with( + task.node, 'update_password_complexity', True, False) + + @mock.patch.object(ilo_management, '_execute_ilo_step', + spec_set=True, autospec=True) + def test_update_minimum_password_length(self, step_mock): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + minimum_password_args = {'password_length': '8'} + task.driver.management.update_minimum_password_length( + task, **minimum_password_args) + step_mock.assert_called_once_with( + task.node, 'update_minimum_password_length', '8', False) + + @mock.patch.object(ilo_management, '_execute_ilo_step', + spec_set=True, autospec=True) + def test_update_auth_failure_logging_threshold(self, step_mock): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + auth_failure_args = {'logging_threshold': '1'} + task.driver.management.update_auth_failure_logging_threshold( + task, **auth_failure_args) + step_mock.assert_called_once_with( + task.node, 'update_authentication_failure_logging', '1', False) + @mock.patch.object(deploy_utils, 'build_agent_options', spec_set=True, autospec=True) @mock.patch.object(ilo_boot.IloVirtualMediaBoot, 'clean_up_ramdisk', diff --git a/releasenotes/notes/adding-security-param-clean-step-00d5548072a397f2.yaml b/releasenotes/notes/adding-security-param-clean-step-00d5548072a397f2.yaml new file mode 100644 index 0000000000..2df38df6e2 --- /dev/null +++ b/releasenotes/notes/adding-security-param-clean-step-00d5548072a397f2.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Adding new clean steps to ``ilo`` hardware type - + ``security_parameters_update``, ``update_minimum_password_length``, + and ``update_auth_failure_logging_threshold`` which allows users to + modify ilo system security settings.