openstack
/
ironic
Code Issues Proposed changes

Enhance certificate verification for ilo harware type 

This patch enhance certificate verification for hardware
type ilo and ilo5 by adding attribute ilo_verify_ca which can take
directory and bolean values apart from file.

Change-Id: Ic48bf53097635498a8461be049ee5d2a50c6fe2a
This commit is contained in:
ankit 2020-07-28 07:23:25 +00:00
parent 11aa5f6639
commit 778d715b5f
5 changed files with 194 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
ironic/conf/ilo.py
View File

@ -77,7 +77,20 @@ opts = [
'300GB SSD with default pattern "block" would take '
'approx. 30 seconds to complete sanitize disk erase.')),
cfg.StrOpt('ca_file',
deprecated_for_removal=True,
deprecated_reason=_('Its being replaced by new configuration '
'parameter "verify_ca".'),
help=_('CA certificate file to validate iLO.')),
cfg.StrOpt('verify_ca',
default='True',
help=_('CA certificate to validate iLO. This can be either '
'a Boolean value, a path to a CA_BUNDLE file or '
'directory with certificates of trusted CAs. If set '
'to True the driver will verify the host certificates; '
'if False the driver will ignore verifying the SSL '
'certificate. If it\'s a path the driver will use the '
'specified certificate or one of the certificates in '
'the directory. Defaults to True.')),
cfg.StrOpt('default_boot_mode',
default='auto',
choices=[('auto', _('based on boot mode settings on the '

107
ironic/drivers/modules/ilo/common.py
View File

@ -25,6 +25,7 @@ from ironic_lib import utils as ironic_utils
from oslo_log import log as logging
from oslo_utils import fileutils
from oslo_utils import importutils
from oslo_utils import strutils
from ironic.common import boot_devices
from ironic.common import exception
@ -56,7 +57,17 @@ 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")
'ca_file': _("CA certificate file to validate iLO. This "
"attibute is deprecated and will be removed in "
"future release. Optional"),
'ilo_verify_ca': _("Either a Boolean value, a path to a CA_BUNDLE "
"file or directory with certificates of trusted "
"CAs. If set to True the driver will verify the "
"host certificates; if False the driver will ignore "
"verifying the SSL certificate. If it\'s a path the "
"driver will use the specified certificate or one of "
"the certificates in the directory. Defaults to True. "
"Optional")
}
SNMP_PROPERTIES = {
@ -255,22 +266,8 @@ def parse_driver_info(node):
"The following required iLO parameters are missing from the "
"node's driver_info: %s") % missing_info)
not_integers = []
for param in OPTIONAL_PROPERTIES:
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)
except ValueError:
not_integers.append(param)
optional_info = _parse_optional_driver_info(node)
d_info.update(optional_info)
snmp_info = _parse_snmp_driver_info(info)
if snmp_info:
@ -284,11 +281,6 @@ def parse_driver_info(node):
if param == "console_port":
d_info[param] = utils.validate_network_port(value, param)
if not_integers:
raise exception.InvalidParameterValue(_(
"The following iLO parameters from the node's driver_info "
"should be integers: %s") % not_integers)
return d_info
@ -335,6 +327,75 @@ def _parse_snmp_driver_info(info):
return snmp_info
def _parse_optional_driver_info(node):
"""Parses the optional driver_info parameters.
:param node: an ironic Node object.
:returns: a dictionary containing information of optional properties.
:raises: InvalidParameterValue if any parameters are incorrect
"""
info = node.driver_info
optional_info = {}
not_integers = []
for param in OPTIONAL_PROPERTIES:
if param != 'ilo_verify_ca':
value = info.get(param, CONF.ilo.get(param))
if param == "client_port":
optional_info[param] = utils.validate_network_port(value, param)
elif param == "client_timeout":
try:
optional_info[param] = int(value)
except ValueError:
not_integers.append(param)
if not_integers:
raise exception.InvalidParameterValue(_(
"The following iLO parameters from the node's driver_info "
"should be integers: %s") % not_integers)
ca_file_value = info.get('ca_file', CONF.ilo.get('ca_file'))
verify_ca = info.get('ilo_verify_ca', CONF.ilo.get('verify_ca'))
if ca_file_value:
LOG.warning("The `driver_info/ca_file` parameter is deprecated "
"in favor of `driver_info/ilo_verify_ca` parameter. "
"Please use `driver_info/ilo_verify_ca` instead of "
"`driver_info/ca_file` in node %(node)s "
"configuration.", {'node': node.uuid})
if not os.path.isfile(ca_file_value):
raise exception.InvalidParameterValue(_(
'ca_file "%(value)s" is not found.') %
{'value': ca_file_value})
optional_info['verify_ca'] = ca_file_value
else:
# Check if ilo_verify_ca is a Boolean or a file/directory
# in the file-system
if isinstance(verify_ca, str):
if not os.path.isdir(verify_ca) and not os.path.isfile(verify_ca):
try:
verify_ca = strutils.bool_from_string(verify_ca,
strict=True)
except ValueError:
raise exception.InvalidParameterValue(
_('Invalid value type set in driver_info/'
'ilo_verify_ca on node %(node)s. '
'The value should be a Boolean or the path '
'to a file/directory, not "%(value)s"'
) % {'value': verify_ca, 'node': node.uuid})
elif not isinstance(verify_ca, bool):
raise exception.InvalidParameterValue(
_('Invalid value type set in driver_info/ilo_verify_ca '
'on node %(node)s. The value should be a Boolean '
'or the path to a file/directory, not "%(value)s"'
) % {'value': verify_ca, 'node': node.uuid})
optional_info['verify_ca'] = verify_ca
return optional_info
def get_ilo_object(node):
"""Gets an IloClient object from proliantutils library.
@ -369,7 +430,7 @@ def get_ilo_object(node):
driver_info['ilo_password'],
driver_info['client_timeout'],
driver_info['client_port'],
cacert=driver_info.get('ca_file'),
cacert=driver_info['verify_ca'],
snmp_credentials=info)
return ilo_object

3
ironic/tests/unit/conductor/test_manager.py
View File

@ -5832,7 +5832,8 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
'ilo_deploy_iso', 'console_port', 'ilo_change_password',
'ca_file', 'snmp_auth_user', 'snmp_auth_prot_password',
'snmp_auth_priv_password', 'snmp_auth_protocol',
'snmp_auth_priv_protocol', 'deploy_forces_oob_reboot']
'snmp_auth_priv_protocol', 'deploy_forces_oob_reboot',
'ilo_verify_ca']
self._check_driver_properties("ilo", expected)
def test_driver_properties_fail(self):

91
ironic/tests/unit/drivers/modules/ilo/test_common.py
View File

@ -83,7 +83,7 @@ class IloValidateParametersTestCase(BaseIloTest):
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'])
self.assertEqual('/home/user/cafile.pem', info['verify_ca'])
self.assertEqual('user', info['snmp_auth_user'])
self.assertEqual('1234', info['snmp_auth_prot_password'])
self.assertEqual('4321', info['snmp_auth_priv_password'])
@ -99,6 +99,81 @@ class IloValidateParametersTestCase(BaseIloTest):
self.assertEqual(60, info['client_timeout'])
self.assertEqual(443, info['client_port'])
@mock.patch.object(os.path, 'isdir', autospec=True)
def test_parse_driver_info_path_verify_ca_dir(self, mock_isdir):
mock_isdir.return_value = True
fake_path = '/path/to/a/valid/CA'
self.node.driver_info['ilo_verify_ca'] = fake_path
info = ilo_common.parse_driver_info(self.node)
self.assertEqual(INFO_DICT['ilo_address'], info['ilo_address'])
self.assertEqual(INFO_DICT['ilo_username'], info['ilo_username'])
self.assertEqual(INFO_DICT['ilo_password'], info['ilo_password'])
self.assertEqual(60, info['client_timeout'])
self.assertEqual(443, info['client_port'])
self.assertEqual(fake_path, info['verify_ca'])
mock_isdir.assert_called_once_with(fake_path)
@mock.patch.object(os.path, 'isfile', autospec=True)
def test_parse_driver_info_path_verify_ca_file(self, mock_isfile):
mock_isfile.return_value = True
fake_path = '/path/to/a/valid/CA.pem'
self.node.driver_info['ilo_verify_ca'] = fake_path
info = ilo_common.parse_driver_info(self.node)
self.assertEqual(INFO_DICT['ilo_address'], info['ilo_address'])
self.assertEqual(INFO_DICT['ilo_username'], info['ilo_username'])
self.assertEqual(INFO_DICT['ilo_password'], info['ilo_password'])
self.assertEqual(60, info['client_timeout'])
self.assertEqual(443, info['client_port'])
self.assertEqual(fake_path, info['verify_ca'])
mock_isfile.assert_called_once_with(fake_path)
def test_parse_driver_info_verify_ca_default_value(self):
info = ilo_common.parse_driver_info(self.node)
self.assertEqual(INFO_DICT['ilo_address'], info['ilo_address'])
self.assertEqual(INFO_DICT['ilo_username'], info['ilo_username'])
self.assertEqual(INFO_DICT['ilo_password'], info['ilo_password'])
self.assertEqual(60, info['client_timeout'])
self.assertEqual(443, info['client_port'])
self.assertEqual(True, info['verify_ca'])
def test_parse_driver_info_verify_ca_string_false(self):
self.config(verify_ca="False", group='ilo')
info = ilo_common.parse_driver_info(self.node)
self.assertEqual(INFO_DICT['ilo_address'], info['ilo_address'])
self.assertEqual(INFO_DICT['ilo_username'], info['ilo_username'])
self.assertEqual(INFO_DICT['ilo_password'], info['ilo_password'])
self.assertEqual(60, info['client_timeout'])
self.assertEqual(443, info['client_port'])
self.assertEqual(False, info['verify_ca'])
def test_parse_driver_info_verify_ca_boolean_true(self):
self.config(verify_ca=True, group='ilo')
info = ilo_common.parse_driver_info(self.node)
self.assertEqual(INFO_DICT['ilo_address'], info['ilo_address'])
self.assertEqual(INFO_DICT['ilo_username'], info['ilo_username'])
self.assertEqual(INFO_DICT['ilo_password'], info['ilo_password'])
self.assertEqual(60, info['client_timeout'])
self.assertEqual(443, info['client_port'])
self.assertEqual(True, info['verify_ca'])
def test_parse_driver_info_verify_ca_boolean_false(self):
self.config(verify_ca=False, group='ilo')
info = ilo_common.parse_driver_info(self.node)
self.assertEqual(INFO_DICT['ilo_address'], info['ilo_address'])
self.assertEqual(INFO_DICT['ilo_username'], info['ilo_username'])
self.assertEqual(INFO_DICT['ilo_password'], info['ilo_password'])
self.assertEqual(60, info['client_timeout'])
self.assertEqual(443, info['client_port'])
self.assertEqual(False, info['verify_ca'])
def test_parse_driver_info_invalid_value_verify_ca(self):
# Integers are not supported
self.node.driver_info['ilo_verify_ca'] = 123456
self.assertRaisesRegex(exception.InvalidParameterValue,
'Invalid value type',
ilo_common._parse_optional_driver_info,
self.node)
@mock.patch.object(os.path, 'isfile', return_value=True, autospec=True)
def test_parse_driver_info_snmp_true_no_auth_priv_protocols(self,
isFile_mock):
@ -113,7 +188,7 @@ class IloValidateParametersTestCase(BaseIloTest):
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'])
self.assertEqual('/home/user/cafile.pem', info['verify_ca'])
self.assertEqual('user', info['snmp_auth_user'])
self.assertEqual('1234', info['snmp_auth_prot_password'])
self.assertEqual('4321', info['snmp_auth_priv_password'])
@ -230,7 +305,7 @@ class IloCommonMethodsTestCase(BaseIloTest):
def _test_get_ilo_object(self, ilo_client_mock, isFile_mock, ca_file=None):
self.info['client_timeout'] = 600
self.info['client_port'] = 4433
self.info['ca_file'] = ca_file
self.info['ilo_verify_ca'] = ca_file
self.node.driver_info = self.info
ilo_client_mock.return_value = 'ilo_object'
returned_ilo_object = ilo_common.get_ilo_object(self.node)
@ -240,7 +315,7 @@ class IloCommonMethodsTestCase(BaseIloTest):
self.info['ilo_password'],
self.info['client_timeout'],
self.info['client_port'],
cacert=self.info['ca_file'],
cacert=self.info['ilo_verify_ca'],
snmp_credentials=None)
self.assertEqual('ilo_object', returned_ilo_object)
@ -256,7 +331,7 @@ class IloCommonMethodsTestCase(BaseIloTest):
'snmp_inspection': True}
d_info = {'client_timeout': 600,
'client_port': 4433,
'ca_file': 'ca_file',
'verify_ca': 'True',
'snmp_auth_user': 'user',
'snmp_auth_prot_password': '1234',
'snmp_auth_priv_password': '4321',
@ -272,15 +347,15 @@ class IloCommonMethodsTestCase(BaseIloTest):
self.info['ilo_password'],
self.info['client_timeout'],
self.info['client_port'],
cacert=self.info['ca_file'],
cacert=self.info['verify_ca'],
snmp_credentials=info)
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()
def test_get_ilo_object_cafile_boolean(self):
self._test_get_ilo_object(ca_file=True)
def test_update_ipmi_properties(self):
with task_manager.acquire(self.context, self.node.uuid,

12
releasenotes/notes/ilo-certificate-verification-enhancement-8eefd541cfc2a9da.yaml Normal file
View File

@ -0,0 +1,12 @@
---
features:
- |
Adds a new configuration option ``[ilo]verify_ca`` and a new
``driver_info`` parameter ``ilo_verify_ca`` to enhance certificate
verification for hardware type ilo and ilo5 which can take
directory and bolean values apart from file.
deprecations:
- The ``[ilo]ca_file`` configuration option is deprecated for
removal, please use ``[ilo]verify_ca`` instead which can take
directory and boolean values apart from file for certificate
verification.