diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 08cccce7af..7a4b11ccd8 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1625,7 +1625,7 @@ EOF function configure_ironic_api { iniset $IRONIC_CONF_FILE DEFAULT auth_strategy $IRONIC_AUTH_STRATEGY configure_keystone_authtoken_middleware $IRONIC_CONF_FILE ironic - + iniset $IRONIC_CONF_FILE keystone_authtoken service_token_roles_required True if [[ "$IRONIC_USE_WSGI" == "True" ]]; then iniset $IRONIC_CONF_FILE oslo_middleware enable_proxy_headers_parsing True elif is_service_enabled tls-proxy; then diff --git a/ironic/common/cinder.py b/ironic/common/cinder.py index cc140547ed..e0cbd17472 100644 --- a/ironic/common/cinder.py +++ b/ironic/common/cinder.py @@ -19,6 +19,7 @@ from cinderclient.v3 import client from oslo_log import log +from ironic.common import context as ironic_context from ironic.common import exception from ironic.common.i18n import _ from ironic.common import keystone @@ -39,30 +40,67 @@ def _get_cinder_session(): return _CINDER_SESSION -def get_client(context): +def get_client(context, auth_from_config=False): """Get a cinder client connection. :param context: request context, instance of ironic.common.context.RequestContext + :param auth_from_config: (boolean) When True, use auth values from + conf parameters :returns: A cinder client. """ service_auth = keystone.get_auth('cinder') session = _get_cinder_session() + # Used the service cached session to get the endpoint + # because getting an endpoint requires auth! + endpoint = keystone.get_endpoint('cinder', session=session, + auth=service_auth) + project_id = None + if hasattr(service_auth, 'get_project_id'): + # Being moderately defensive so we don't have to mock this in + # every cinder unit test. + project_id = service_auth.get_project_id(session) + if project_id and project_id in str(endpoint): + # We've found the project ID in the endpoint URL due to the + # endpoint configuration, however this is not required in + # more modern versions of cinder, and if you attempt to access + # a resource with a project ID in the URL, and with a service + # token, the request gets a Bad Request response. + # This works starting at Cinder microversion 3.67 - Yoga. + endpoint = endpoint.replace("/%s" % project_id, "") + + if not context: + context = ironic_context.RequestContext(auth_token=None) + + user_auth = None + if CONF.cinder.auth_type != 'none' and context.auth_token: + user_auth = keystone.get_service_auth( + context, endpoint, service_auth, + only_service_auth=auth_from_config) + + if auth_from_config: + # If we are here, then we've been requested to *only* use our supplied + sess = keystone.get_session('cinder', timeout=CONF.cinder.timeout, + auth=service_auth) + else: + sess = keystone.get_session('cinder', timeout=CONF.cinder.timeout, + auth=user_auth or service_auth) + + # Re-determine the endpoint so we can work with versions prior to + # Yoga, becuase the endpoint, based upon configuration, may require + # project_id specific URLs. + if user_auth: + endpoint = keystone.get_endpoint('cinder', session=sess, + auth=user_auth) - # TODO(pas-ha) use versioned endpoint data to select required - # cinder api version - cinder_url = keystone.get_endpoint('cinder', session=session, - auth=service_auth) - # TODO(pas-ha) investigate possibility of passing a user context here, - # similar to what neutron/glance-related code does # NOTE(pas-ha) cinderclient has both 'connect_retries' (passed to # ksa.Adapter) and 'retries' (used in its subclass of ksa.Adapter) options. # The first governs retries on establishing the HTTP connection, # the second governs retries on OverLimit exceptions from API. # The description of [cinder]/retries fits the first, # so this is what we pass. - return client.Client(session=session, auth=service_auth, - endpoint_override=cinder_url, + return client.Client(session=sess, auth=user_auth, + endpoint_override=endpoint, connect_retries=CONF.cinder.retries, global_request_id=context.global_id) @@ -134,17 +172,21 @@ def _create_metadata_dictionary(node, action): return {label: json.dumps(data)} -def _init_client(task): +def _init_client(task, auth_from_config=False): """Obtain cinder client and return it for use. :param task: TaskManager instance representing the operation. + :param auth_from_config: If we should source our authentication parameters + from the configured service as opposed to request + context. :returns: A cinder client. :raises: StorageError If an exception is encountered creating the client. """ node = task.node try: - return get_client(task.context) + return get_client(task.context, + auth_from_config=auth_from_config) except Exception as e: msg = (_('Failed to initialize cinder client for operations on node ' '%(uuid)s: %(err)s') % {'uuid': node.uuid, 'err': e}) @@ -238,8 +280,9 @@ def attach_volumes(task, volume_list, connector): }] """ node = task.node + LOG.debug('Initializing volume attach for node %(node)s.', + {'node': node.uuid}) client = _init_client(task) - connected = [] for volume_id in volume_list: try: @@ -367,8 +410,10 @@ def _handle_errors(msg): LOG.error(msg) raise exception.StorageError(msg) - client = _init_client(task) + client = _init_client(task, auth_from_config=False) node = task.node + LOG.debug('Initializing volume detach for node %(node)s.', + {'node': node.uuid}) for volume_id in volume_list: try: diff --git a/ironic/common/keystone.py b/ironic/common/keystone.py index fae8d90f38..e8351f2b5f 100644 --- a/ironic/common/keystone.py +++ b/ironic/common/keystone.py @@ -125,7 +125,8 @@ def get_endpoint(group, **adapter_kwargs): return result -def get_service_auth(context, endpoint, service_auth): +def get_service_auth(context, endpoint, service_auth, + only_service_auth=False): """Create auth plugin wrapping both user and service auth. When properly configured and using auth_token middleware, @@ -134,8 +135,25 @@ def get_service_auth(context, endpoint, service_auth): Ideally we would use the plugin provided by auth_token middleware however this plugin isn't serialized yet. + + :param context: The RequestContext instance from which the user + auth_token is extracted. + :param endpoint: The requested endpoint to be utilized. + :param service_auth: The service authenticaiton credentals to be + used. + :param only_service_auth: Boolean, default False. When set to True, + the resulting Service token pair is generated + as if it originates from the user itself. + Useful to cast admin level operations which are + launched by Ironic itself, as opposed to user + initiated requests. + :returns: Returns a service token via the ServiceTokenAuthWrapper + class. """ - # TODO(pas-ha) use auth plugin from context when it is available - user_auth = token_endpoint.Token(endpoint, context.auth_token) + user_auth = None + if not only_service_auth: + user_auth = token_endpoint.Token(endpoint, context.auth_token) + else: + user_auth = service_auth return service_token.ServiceTokenAuthWrapper(user_auth=user_auth, service_auth=service_auth) diff --git a/ironic/tests/unit/common/test_cinder.py b/ironic/tests/unit/common/test_cinder.py index f3bd7ae771..f1f5c1824b 100644 --- a/ironic/tests/unit/common/test_cinder.py +++ b/ironic/tests/unit/common/test_cinder.py @@ -58,8 +58,7 @@ def test__get_cinder_session(self, mock_keystone_session, mock_auth): @mock.patch('ironic.common.keystone.get_adapter', autospec=True) @mock.patch('ironic.common.keystone.get_service_auth', autospec=True, return_value=mock.sentinel.sauth) -@mock.patch('ironic.common.keystone.get_auth', autospec=True, - return_value=mock.sentinel.auth) +@mock.patch('ironic.common.keystone.get_auth', autospec=True) @mock.patch('ironic.common.keystone.get_session', autospec=True, return_value=mock.sentinel.session) @mock.patch.object(cinderclient.Client, '__init__', autospec=True, @@ -74,8 +73,11 @@ def setUp(self): cinder._CINDER_SESSION = None self.context = context.RequestContext(global_request_id='global') - def _assert_client_call(self, init_mock, url, auth=mock.sentinel.auth): - cinder.get_client(self.context) + def _assert_client_call(self, init_mock, url, auth=mock.sentinel.auth, + auth_from_config=None): + if not auth_from_config: + self.context.auth_token = 'meow' + cinder.get_client(self.context, auth_from_config=auth_from_config) init_mock.assert_called_once_with( mock.ANY, session=mock.sentinel.session, @@ -86,15 +88,45 @@ def _assert_client_call(self, init_mock, url, auth=mock.sentinel.auth): def test_get_client(self, mock_client_init, mock_session, mock_auth, mock_sauth, mock_adapter): - + mock_auth.return_value = mock_auth_obj = mock.Mock() + mock_auth_obj.get_project_id.return_value = '1111' + mock_adapter.return_value = mock_adapter_obj = mock.Mock() + mock_adapter_obj.get_endpoint.side_effect = iter([ + 'cinder_url/1111', + 'cinder_url']) + self._assert_client_call(mock_client_init, 'cinder_url', + auth=mock.sentinel.sauth) + mock_session.assert_has_calls([ + mock.call('cinder'), + mock.call('cinder', timeout=1, auth=mock.sentinel.sauth)]) + mock_auth.assert_called_once_with('cinder') + mock_adapter.assert_has_calls([ + mock.call('cinder', session=mock.sentinel.session, + auth=mock_auth_obj), + + mock.call('cinder', session=mock.sentinel.session, + auth=mock.sentinel.sauth)]) + self.assertTrue(mock_sauth.called) + + def test_get_client_service_token(self, mock_client_init, mock_session, + mock_auth, mock_sauth, mock_adapter): + mock_auth.return_value = mock_auth_obj = mock.Mock() + mock_auth_obj.get_project_id.return_value = '1111' mock_adapter.return_value = mock_adapter_obj = mock.Mock() - mock_adapter_obj.get_endpoint.return_value = 'cinder_url' - self._assert_client_call(mock_client_init, 'cinder_url') - mock_session.assert_called_once_with('cinder') + mock_adapter_obj.get_endpoint.side_effect = iter([ + 'cinder_url/1111', + 'cinder_url']) + self._assert_client_call( + mock_client_init, + 'cinder_url', + auth=None, + auth_from_config=True) + mock_session.assert_has_calls([ + mock.call('cinder'), + mock.call('cinder', timeout=1, auth=mock_auth_obj)]) mock_auth.assert_called_once_with('cinder') - mock_adapter.assert_called_once_with('cinder', - session=mock.sentinel.session, - auth=mock.sentinel.auth) + mock_adapter.assert_called_once_with( + 'cinder', session=mock.sentinel.session, auth=mock_auth_obj) self.assertFalse(mock_sauth.called) diff --git a/releasenotes/notes/cinder-2019892-6b5a9de5c5f05aa6.yaml b/releasenotes/notes/cinder-2019892-6b5a9de5c5f05aa6.yaml new file mode 100644 index 0000000000..0c73cdad2b --- /dev/null +++ b/releasenotes/notes/cinder-2019892-6b5a9de5c5f05aa6.yaml @@ -0,0 +1,16 @@ +--- +fixes: + - | + Fixes Ironic integration with Cinder because of changes which resulted as + part of the recent Security related fix in + `bug 2004555 `_. The work in Ironic + to track this fix was logged in + `bug 2019892 `_. + Ironic now sends a service token to Cinder, which allows for access + restrictions added as part of the original CVE-2023-2088 + fix to be appropriately bypassed. Ironic was not vulnerable, + but the restrictions added as a result did impact Ironic's usage. + This is because Ironic volume attachments are not on a shared + "compute node", but instead mapped to the physical machines + and Ironic handles the attachment life-cycle after initial + attachment.