From d36bd6f74aa12f6d460fbf086893150a2dc92d63 Mon Sep 17 00:00:00 2001 From: Shivanand Tendulker Date: Wed, 6 Jul 2016 21:26:33 -0700 Subject: [PATCH] Support to validate iLO SSL certificate in iLO drivers iLO drivers do not validate iLO SSL certificate. This commit adds support in iLO drivers to validate iLO SSL certificate. Change-Id: Iff0d02799d3d9338b7dbdd77eab1f12f709a7765 Closes-Bug: #1599710 --- doc/source/drivers/ilo.rst | 3 ++ etc/ironic/ironic.conf.sample | 22 +++++++++----- ironic/conf/ilo.py | 2 ++ ironic/drivers/modules/ilo/common.py | 10 ++++++- ironic/tests/unit/conductor/test_manager.py | 7 +++-- .../unit/drivers/modules/ilo/test_common.py | 30 +++++++++++++++++-- ...ate-ilo-certificates-3ab98bb8cfad7d60.yaml | 8 +++++ 7 files changed, 69 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/validate-ilo-certificates-3ab98bb8cfad7d60.yaml diff --git a/doc/source/drivers/ilo.rst b/doc/source/drivers/ilo.rst index 352eaea7e9..7d181cf079 100644 --- a/doc/source/drivers/ilo.rst +++ b/doc/source/drivers/ilo.rst @@ -288,6 +288,7 @@ Nodes configured for iLO driver should have the ``driver`` property set to - ``ilo_username``: Username for the iLO with administrator privileges. - ``ilo_password``: Password for the above iLO user. - ``ilo_deploy_iso``: The glance UUID of the deploy ramdisk ISO image. +- ``ilo_ca_file``: (optional) CA certificate file to validate iLO. - ``client_port``: (optional) Port to be used for iLO operations if you are using a custom port on the iLO. Default port used is 443. - ``client_timeout``: (optional) Timeout for iLO operations. Default timeout @@ -425,6 +426,7 @@ Nodes configured for iLO driver should have the ``driver`` property set to - ``ilo_username``: Username for the iLO with administrator privileges. - ``ilo_password``: Password for the above iLO user. - ``ilo_deploy_iso``: The glance UUID of the deploy ramdisk ISO image. +- ``ilo_ca_file``: (optional) CA certificate file to validate iLO. - ``client_port``: (optional) Port to be used for iLO operations if you are using a custom port on the iLO. Default port used is 443. - ``client_timeout``: (optional) Timeout for iLO operations. Default timeout @@ -543,6 +545,7 @@ Nodes configured for iLO driver should have the ``driver`` property set to - ``ilo_password``: Password for the above iLO user. - ``deploy_kernel``: The glance UUID of the deployment kernel. - ``deploy_ramdisk``: The glance UUID of the deployment ramdisk. +- ``ilo_ca_file``: (optional) CA certificate file to validate iLO. - ``client_port``: (optional) Port to be used for iLO operations if you are using a custom port on the iLO. Default port used is 443. - ``client_timeout``: (optional) Timeout for iLO operations. Default timeout diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 10e7b8ac7b..a0f8cca2a6 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -37,7 +37,7 @@ # recommended set of production-oriented network interfaces. A # complete list of network interfaces present on your system # may be found by enumerating the -# "ironic.hardware.interfaces.network" entrypoint.This value +# "ironic.hardware.interfaces.network" entrypoint. This value # must be the same on all ironic-conductor and ironic-api # services, because it is used by ironic-api service to # validate a new or updated node's network_interface value. @@ -156,6 +156,7 @@ # is set in the configuration file and other logging # configuration options are ignored (for example, # logging_context_format_string). (string value) +# Note: This option can be changed without restarting. # Deprecated group/name - [DEFAULT]/log_config #log_config_append = @@ -635,13 +636,13 @@ # From ironic # -# Path to serial console terminal program. Used only by -# Shell In A Box console. (string value) +# Path to serial console terminal program. Used only by Shell +# In A Box console. (string value) #terminal = shellinaboxd -# Directory containing the terminal SSL cert(PEM) for serial -# console access. Used only by Shell In A Box console. -# (string value) +# Directory containing the terminal SSL cert (PEM) for serial +# console access. Used only by Shell In A Box console. (string +# value) #terminal_cert_dir = # Directory for holding terminal pid files. If not specified, @@ -736,8 +737,12 @@ # From oslo.db # -# The file name to use with SQLite. (string value) +# DEPRECATED: The file name to use with SQLite. (string value) # Deprecated group/name - [DEFAULT]/sqlite_db +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +# Reason: Should use config option connection or +# slave_connection to connect the database. #sqlite_db = oslo.sqlite # If True, SQLite uses synchronous mode. (boolean value) @@ -1153,6 +1158,9 @@ # operations (integer value) #power_wait = 2 +# CA certificate file to validate iLO. (string value) +#ca_file = + [inspector] diff --git a/ironic/conf/ilo.py b/ironic/conf/ilo.py index 54f86cbaf2..172b138d99 100644 --- a/ironic/conf/ilo.py +++ b/ironic/conf/ilo.py @@ -78,6 +78,8 @@ opts = [ default=2, help=_('Amount of time in seconds to wait in between power ' 'operations')), + cfg.StrOpt('ca_file', + help=_('CA certificate file to validate iLO.')), ] diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index f48407a705..69060f41c5 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -59,6 +59,7 @@ REQUIRED_PROPERTIES = { OPTIONAL_PROPERTIES = { 'client_port': _("port to be used for iLO operations. Optional."), 'client_timeout': _("timeout (in seconds) for iLO operations. Optional."), + 'ca_file': _("CA certificate file to validate iLO. optional"), } CONSOLE_PROPERTIES = { 'console_port': _("node's UDP port to connect to. Only required for " @@ -211,6 +212,12 @@ def parse_driver_info(node): value = info.get(param, CONF.ilo.get(param)) if param == "client_port": d_info[param] = utils.validate_network_port(value, param) + elif param == "ca_file": + if value and not os.path.isfile(value): + raise exception.InvalidParameterValue(_( + '%(param)s "%(value)s" is not found.') % + {'param': param, 'value': value}) + d_info[param] = value else: try: d_info[param] = int(value) @@ -250,7 +257,8 @@ def get_ilo_object(node): driver_info['ilo_username'], driver_info['ilo_password'], driver_info['client_timeout'], - driver_info['client_port']) + driver_info['client_port'], + cacert=driver_info.get('ca_file')) return ilo_object diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 6553afa7dc..75c194751d 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -4022,21 +4022,22 @@ class ManagerTestProperties(tests_db_base.DbTestCase): def test_driver_properties_fake_ilo(self): expected = ['ilo_address', 'ilo_username', 'ilo_password', - 'client_port', 'client_timeout', 'ilo_change_password'] + 'client_port', 'client_timeout', 'ilo_change_password', + 'ca_file'] self._check_driver_properties("fake_ilo", expected) def test_driver_properties_ilo_iscsi(self): expected = ['ilo_address', 'ilo_username', 'ilo_password', 'client_port', 'client_timeout', 'ilo_deploy_iso', 'console_port', 'ilo_change_password', - 'deploy_forces_oob_reboot'] + 'deploy_forces_oob_reboot', 'ca_file'] self._check_driver_properties("iscsi_ilo", expected) def test_driver_properties_agent_ilo(self): expected = ['ilo_address', 'ilo_username', 'ilo_password', 'client_port', 'client_timeout', 'ilo_deploy_iso', 'console_port', 'ilo_change_password', - 'deploy_forces_oob_reboot'] + 'deploy_forces_oob_reboot', 'ca_file'] self._check_driver_properties("agent_ilo", expected) def test_driver_properties_fail(self): diff --git a/ironic/tests/unit/drivers/modules/ilo/test_common.py b/ironic/tests/unit/drivers/modules/ilo/test_common.py index 5dc4929733..cde0808a39 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_common.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_common.py @@ -61,7 +61,9 @@ class IloValidateParametersTestCase(db_base.DbTestCase): self.context, driver='fake_ilo', driver_info=INFO_DICT) - def test_parse_driver_info(self): + @mock.patch.object(os.path, 'isfile', return_value=True, autospec=True) + def _test_parse_driver_info(self, isFile_mock): + info = ilo_common.parse_driver_info(self.node) self.assertEqual(INFO_DICT['ilo_address'], info['ilo_address']) @@ -69,6 +71,15 @@ class IloValidateParametersTestCase(db_base.DbTestCase): self.assertEqual(INFO_DICT['ilo_password'], info['ilo_password']) self.assertEqual(60, info['client_timeout']) self.assertEqual(443, info['client_port']) + self.assertEqual('/home/user/cafile.pem', info['ca_file']) + + def test_parse_driver_info_ca_file_in_driver_info(self): + self.node.driver_info['ca_file'] = '/home/user/cafile.pem' + self._test_parse_driver_info() + + def test_parse_driver_info_ca_file_in_conf_file(self): + self.config(ca_file='/home/user/cafile.pem', group='ilo') + self._test_parse_driver_info() def test_parse_driver_info_missing_address(self): del self.node.driver_info['ilo_address'] @@ -85,6 +96,13 @@ class IloValidateParametersTestCase(db_base.DbTestCase): self.assertRaises(exception.MissingParameterValue, ilo_common.parse_driver_info, self.node) + @mock.patch.object(os.path, 'isfile', return_value=False, autospec=True) + def test_parse_driver_info_invalid_cafile(self, isFile_mock): + self.node.driver_info['ca_file'] = '/home/missing.pem' + self.assertRaisesRegex(exception.InvalidParameterValue, + 'ca_file "/home/missing.pem" is not found.', + ilo_common.parse_driver_info, self.node) + def test_parse_driver_info_invalid_timeout(self): self.node.driver_info['client_timeout'] = 'qwe' self.assertRaises(exception.InvalidParameterValue, @@ -132,11 +150,13 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): self.node = obj_utils.create_test_node( self.context, driver='fake_ilo', driver_info=self.info) + @mock.patch.object(os.path, 'isfile', return_value=True, autospec=True) @mock.patch.object(ilo_client, 'IloClient', spec_set=True, autospec=True) - def test_get_ilo_object(self, ilo_client_mock): + def _test_get_ilo_object(self, ilo_client_mock, isFile_mock, ca_file=None): self.info['client_timeout'] = 60 self.info['client_port'] = 443 + self.info['ca_file'] = ca_file ilo_client_mock.return_value = 'ilo_object' returned_ilo_object = ilo_common.get_ilo_object(self.node) ilo_client_mock.assert_called_with( @@ -147,6 +167,12 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): self.info['client_port']) self.assertEqual('ilo_object', returned_ilo_object) + def test_get_ilo_object_cafile(self): + self._test_get_ilo_object(ca_file='/home/user/ilo.pem') + + def test_get_ilo_object_no_cafile(self): + self._test_get_ilo_object() + @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True, autospec=True) def test_get_ilo_license(self, get_ilo_object_mock): diff --git a/releasenotes/notes/validate-ilo-certificates-3ab98bb8cfad7d60.yaml b/releasenotes/notes/validate-ilo-certificates-3ab98bb8cfad7d60.yaml new file mode 100644 index 0000000000..9d9d7d1abf --- /dev/null +++ b/releasenotes/notes/validate-ilo-certificates-3ab98bb8cfad7d60.yaml @@ -0,0 +1,8 @@ +--- +features: + - Added support to validate iLO SSL certificate in iLO + drivers. + New config parameter '[ilo]/ca_file' added to + specify iLO CA certificate file. + If 'ca_file' is specified, iLO drivers will validate + iLO SSL certificates.