diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst index dd19f8bde7..899ef98a26 100644 --- a/doc/source/admin/drivers/redfish.rst +++ b/doc/source/admin/drivers/redfish.rst @@ -87,8 +87,18 @@ field: The "auto" mode first tries "session" and falls back to "basic" if session authentication is not supported by the Redfish BMC. Default is set in ironic config - as ``[redfish]auth_type``. + as ``[redfish]auth_type``. Most operators should not + need to leverage this setting. Session based + authentication should generally be used in most + cases as it prevents re-authentication every time + a background task checks in with the BMC. +.. note:: + The ``redfish_address``, ``redfish_username``, ``redfish_password``, + and ``redfish_verify_ca`` fields, if changed, will trigger a new session + to be establsihed and cached with the BMC. The ``redfish_auth_type`` field + will only be used for the creation of a new cached session, or should + one be rejected by the BMC. The ``baremetal node create`` command can be used to enroll a node with the ``redfish`` driver. For example: @@ -620,6 +630,44 @@ Eject Virtual Media "boot_device (optional)", "body", "string", "Type of the device to eject (all devices by default)" +Internal Session Cache +====================== + +The ``redfish`` hardware type, and derived interfaces, utilizes a built-in +session cache which prevents Ironic from re-authenticating every time +Ironic attempts to connect to the BMC for any reason. + +This consists of cached connectors objects which are used and tracked by +a unique consideration of ``redfish_username``, ``redfish_password``, +``redfish_verify_ca``, and finally ``redfish_address``. Changing any one +of those values will trigger a new session to be created. +The ``redfish_system_id`` value is explicitly not considered as Redfish +has a model of use of one BMC to many systems, which is also a model +Ironic supports. + +The session cache default size is ``1000`` sessions per conductor. +If you are operating a deployment with a larger number of Redfish +BMCs, it is advised that you do appropriately tune that number. +This can be tuned via the API service configuration file, +``[redfish]connection_cache_size``. + +Session Cache Expiration +~~~~~~~~~~~~~~~~~~~~~~~~ + +By default, sessions remain cached for as long as possible in +memory, as long as they have not experienced an authentication, +connection, or other unexplained error. + +Under normal circumstances, the sessions will only be rolled out +of the cache in order of oldest first when the cache becomes full. +There is no time based expiration to entries in the session cache. + +Of course, the cache is only in memory, and restarting the +``ironic-conductor`` will also cause the cache to be rebuilt +from scratch. If this is due to any persistent connectivity issue, +this may be sign of an unexpected condition, and please consider +contacting the Ironic developer community for assistance. + .. _Redfish: http://redfish.dmtf.org/ .. _Sushy: https://opendev.org/openstack/sushy .. _TLS: https://en.wikipedia.org/wiki/Transport_Layer_Security diff --git a/ironic/drivers/modules/redfish/utils.py b/ironic/drivers/modules/redfish/utils.py index 40cf33bce8..e85e2ec6a6 100644 --- a/ironic/drivers/modules/redfish/utils.py +++ b/ironic/drivers/modules/redfish/utils.py @@ -15,6 +15,7 @@ # under the License. import collections +import hashlib import os from urllib import parse as urlparse @@ -198,43 +199,59 @@ class SessionCache(object): _sessions = collections.OrderedDict() def __init__(self, driver_info): + # Hash the password in the data structure, so we can + # include it in the session key. + # NOTE(TheJulia): Multiplying the address by 4, to ensure + # we meet a minimum of 16 bytes for salt. + pw_hash = hashlib.pbkdf2_hmac( + 'sha512', + driver_info.get('password').encode('utf-8'), + str(driver_info.get('address') * 4).encode('utf-8'), 40) self._driver_info = driver_info + # Assemble the session key and append the hashed password to it, + # which forces new sessions to be established when the saved password + # is changed, just like the username, or address. self._session_key = tuple( self._driver_info.get(key) for key in ('address', 'username', 'verify_ca') - ) + ) + (pw_hash.hex(),) def __enter__(self): try: return self.__class__._sessions[self._session_key] - except KeyError: - auth_type = self._driver_info['auth_type'] + LOG.debug('A cached redfish session for Redfish endpoint ' + '%(endpoint)s was not detected, initiating a session.', + {'endpoint': self._driver_info['address']}) - auth_class = self.AUTH_CLASSES[auth_type] + auth_type = self._driver_info['auth_type'] - authenticator = auth_class( - username=self._driver_info['username'], - password=self._driver_info['password'] - ) + auth_class = self.AUTH_CLASSES[auth_type] - sushy_params = {'verify': self._driver_info['verify_ca'], - 'auth': authenticator} - if 'root_prefix' in self._driver_info: - sushy_params['root_prefix'] = self._driver_info['root_prefix'] - conn = sushy.Sushy( - self._driver_info['address'], - **sushy_params - ) + authenticator = auth_class( + username=self._driver_info['username'], + password=self._driver_info['password'] + ) - if CONF.redfish.connection_cache_size: - self.__class__._sessions[self._session_key] = conn + sushy_params = {'verify': self._driver_info['verify_ca'], + 'auth': authenticator} + if 'root_prefix' in self._driver_info: + sushy_params['root_prefix'] = self._driver_info['root_prefix'] + conn = sushy.Sushy( + self._driver_info['address'], + **sushy_params + ) - if (len(self.__class__._sessions) - > CONF.redfish.connection_cache_size): - self._expire_oldest_session() + if CONF.redfish.connection_cache_size: + self.__class__._sessions[self._session_key] = conn + # Save a secure hash of the password into memory, so if we + # observe it change, we can detect the session is no longer valid. - return conn + if (len(self.__class__._sessions) + > CONF.redfish.connection_cache_size): + self._expire_oldest_session() + + return conn def __exit__(self, exc_type, exc_val, exc_tb): # NOTE(etingof): perhaps this session token is no good diff --git a/ironic/tests/unit/drivers/modules/redfish/test_utils.py b/ironic/tests/unit/drivers/modules/redfish/test_utils.py index ca8aba9da8..01b7089c7f 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_utils.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_utils.py @@ -252,6 +252,7 @@ class RedfishUtilsAuthTestCase(db_base.DbTestCase): redfish_utils.get_system(self.node) redfish_utils.get_system(self.node) self.assertEqual(1, mock_sushy.call_count) + self.assertEqual(len(redfish_utils.SessionCache._sessions), 1) @mock.patch.object(sushy, 'Sushy', autospec=True) def test_ensure_new_session_address(self, mock_sushy): @@ -269,6 +270,21 @@ class RedfishUtilsAuthTestCase(db_base.DbTestCase): redfish_utils.get_system(self.node) self.assertEqual(2, mock_sushy.call_count) + @mock.patch.object(sushy, 'Sushy', autospec=True) + def test_ensure_new_session_password(self, mock_sushy): + d_info = self.node.driver_info + d_info['redfish_username'] = 'foo' + d_info['redfish_password'] = 'bar' + self.node.driver_info = d_info + self.node.save() + redfish_utils.get_system(self.node) + d_info['redfish_password'] = 'foo' + self.node.driver_info = d_info + self.node.save() + redfish_utils.SessionCache._sessions = collections.OrderedDict() + redfish_utils.get_system(self.node) + self.assertEqual(2, mock_sushy.call_count) + @mock.patch.object(sushy, 'Sushy', autospec=True) @mock.patch('ironic.drivers.modules.redfish.utils.' 'SessionCache.AUTH_CLASSES', autospec=True) diff --git a/releasenotes/notes/redfish_consider_password_in_session_cache-1fa84234db179053.yaml b/releasenotes/notes/redfish_consider_password_in_session_cache-1fa84234db179053.yaml new file mode 100644 index 0000000000..af48b88faa --- /dev/null +++ b/releasenotes/notes/redfish_consider_password_in_session_cache-1fa84234db179053.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes an issue where the Redfish session cache would continue using an + old session when a password for a Redfish BMC was changed. Now the old + session will not be found in this case, and a new session will be created + with the latest credential information available.