Merge "Fix Cinder Integration fallout from CVE-2023-2088" into stable…

…/zed

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

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:

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)

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)
 
 

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 <https://launchpad.net/bugs/2004555>`_. The work in Ironic
+    to track this fix was logged in
+    `bug 2019892 <https://bugs.launchpad.net/ironic/+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.