Codebase list cinder / 8251c62
* CVE-2023-2088: Unauthorized volume access through deleted volume attachments. Applied upstream patch: Reject unsafe delete attachment calls (Closes: #1035961). Thomas Goirand 11 months ago
3 changed file(s) with 994 addition(s) and 0 deletion(s). Raw diff Collapse all Expand all
+8
-0
debian/changelog less more
0 cinder (2:22.0.0-3) experimental; urgency=medium
1
2 * CVE-2023-2088: Unauthorized volume access through deleted volume
3 attachments. Applied upstream patch: Reject unsafe delete attachment calls
4 (Closes: #1035961).
5
6 -- Thomas Goirand <zigo@debian.org> Fri, 12 May 2023 12:22:24 +0200
7
08 cinder (2:22.0.0-2) experimental; urgency=medium
19
210 * Build-depends on openstack-pkg-tools (>= 123~).
+985
-0
debian/patches/CVE-2023-2088_Reject_unsafe_delete_attachment_calls.patch less more
0 Author: Gorka Eguileor <geguileo@redhat.com>
1 Date: Thu, 16 Feb 2023 15:57:15 +0100
2 Description: CVE-2023-2088 Reject unsafe delete attachment calls
3 Due to how the Linux SCSI kernel driver works there are some storage
4 systems, such as iSCSI with shared targets, where a normal user can
5 access other projects' volume data connected to the same compute host
6 using the attachments REST API.
7 .
8 This affects both single and multi-pathed connections.
9 .
10 To prevent users from doing this, unintentionally or maliciously,
11 cinder-api will now reject some delete attachment requests that are
12 deemed unsafe.
13 .
14 Cinder will process the delete attachment request normally in the
15 following cases:
16 .
17 - The request comes from an OpenStack service that is sending the
18 service token that has one of the roles in `service_token_roles`.
19 - Attachment doesn't have an instance_uuid value
20 - The instance for the attachment doesn't exist in Nova
21 - According to Nova the volume is not connected to the instance
22 - Nova is not using this attachment record
23 .
24 There are 3 operations in the actions REST API endpoint that can be used
25 for an attack:
26 .
27 - `os-terminate_connection`: Terminate volume attachment
28 - `os-detach`: Detach a volume
29 - `os-force_detach`: Force detach a volume
30 .
31 In this endpoint we just won't allow most requests not coming from a
32 service. The rules we apply are the same as for attachment delete
33 explained earlier, but in this case we may not have the attachment id
34 and be more restrictive. This should not be a problem for normal
35 operations because:
36 .
37 - Cinder backup doesn't use the REST API but RPC calls via RabbitMQ
38 - Glance doesn't use this interface
39 .
40 Checking whether it's a service or not is done at the cinder-api level
41 by checking that the service user that made the call has at least one of
42 the roles in the `service_token_roles` configuration. These roles are
43 retrieved from keystone by the keystone middleware using the value of
44 the "X-Service-Token" header.
45 .
46 If Cinder is configured with `service_token_roles_required = true` and
47 an attacker provides non-service valid credentials the service will
48 return a 401 error, otherwise it'll return 409 as if a normal user had
49 made the call without the service token.
50 Bug: https://launchpad.net/bugs/2004555
51 Change-Id: I612905a1bf4a1706cce913c0d8a6df7a240d599a
52 Bug-Debian: https://bugs.debian.org/1035961
53 Origin: upstream, https://review.opendev.org/c/openstack/cinder/+/882836
54 Last-Update: 2023-05-12
55
56 diff --git a/api-ref/source/v3/attachments.inc b/api-ref/source/v3/attachments.inc
57 index 87b57d6..cb37848 100644
58 --- a/api-ref/source/v3/attachments.inc
59 +++ b/api-ref/source/v3/attachments.inc
60 @@ -41,6 +41,20 @@
61
62 Deletes an attachment.
63
64 +For security reasons (see bug `#2004555
65 +<https://bugs.launchpad.net/nova/+bug/2004555>`_) the Block Storage API rejects
66 +REST API calls manually made from users with a 409 status code if there is a
67 +Nova instance currently using the attachment, which happens when all the
68 +following conditions are met:
69 +
70 +- Attachment has an instance uuid
71 +- VM exists in Nova
72 +- Instance has the volume attached
73 +- Attached volume in instance is using the attachment
74 +
75 +Calls coming from other OpenStack services (like the Compute Service) are
76 +always accepted.
77 +
78 Available starting in the 3.27 microversion.
79
80 Response codes
81 @@ -54,6 +68,7 @@
82
83 - 400
84 - 404
85 + - 409
86
87
88 Request
89 diff --git a/api-ref/source/v3/volumes-v3-volumes-actions.inc b/api-ref/source/v3/volumes-v3-volumes-actions.inc
90 index 808dcda..bb79e30 100644
91 --- a/api-ref/source/v3/volumes-v3-volumes-actions.inc
92 +++ b/api-ref/source/v3/volumes-v3-volumes-actions.inc
93 @@ -337,6 +337,21 @@
94
95 - Volume status must be ``in-use``.
96
97 +For security reasons (see bug `#2004555
98 +<https://bugs.launchpad.net/nova/+bug/2004555>`_), regardless of the policy
99 +defaults, the Block Storage API rejects REST API calls manually made from
100 +users with a 409 status code if completing the request could pose a risk, which
101 +happens if all of these happen:
102 +
103 +- The request comes from a user
104 +- There's an instance uuid in provided attachment or in the volume's attachment
105 +- VM exists in Nova
106 +- Instance has the volume attached
107 +- Attached volume in instance is using the attachment
108 +
109 +Calls coming from other OpenStack services (like the Compute Service) are
110 +always accepted.
111 +
112 Response codes
113 --------------
114
115 @@ -344,6 +359,9 @@
116
117 - 202
118
119 +.. rest_status_code:: error ../status.yaml
120 +
121 + - 409
122
123 Request
124 -------
125 @@ -415,6 +433,21 @@
126 through the ``volume_extension:volume_admin_actions:force_detach`` rule in
127 the policy configuration file.
128
129 +For security reasons (see bug `#2004555
130 +<https://bugs.launchpad.net/nova/+bug/2004555>`_), regardless of the policy
131 +defaults, the Block Storage API rejects REST API calls manually made from
132 +users with a 409 status code if completing the request could pose a risk, which
133 +happens if all of these happen:
134 +
135 +- The request comes from a user
136 +- There's an instance uuid in provided attachment or in the volume's attachment
137 +- VM exists in Nova
138 +- Instance has the volume attached
139 +- Attached volume in instance is using the attachment
140 +
141 +Calls coming from other OpenStack services (like the Compute Service) are
142 +always accepted.
143 +
144 Response codes
145 --------------
146
147 @@ -422,6 +455,9 @@
148
149 - 202
150
151 +.. rest_status_code:: error ../status.yaml
152 +
153 + - 409
154
155 Request
156 -------
157 @@ -883,6 +919,22 @@
158
159 - Volume status must be ``in-use``.
160
161 +For security reasons (see bug `#2004555
162 +<https://bugs.launchpad.net/nova/+bug/2004555>`_), regardless of the policy
163 +defaults, the Block Storage API rejects REST API calls manually made from
164 +users with a 409 status code if completing the request could pose a risk, which
165 +happens if all of these happen:
166 +
167 +- The request comes from a user
168 +- There's an instance uuid in the volume's attachment
169 +- VM exists in Nova
170 +- Instance has the volume attached
171 +- Attached volume in instance is using the attachment
172 +
173 +Calls coming from other OpenStack services (like the Compute Service) are
174 +always accepted.
175 +
176 +
177 Response codes
178 --------------
179
180 @@ -890,6 +942,9 @@
181
182 - 202
183
184 +.. rest_status_code:: error ../status.yaml
185 +
186 + - 409
187
188 Request
189 -------
190 diff --git a/cinder/compute/nova.py b/cinder/compute/nova.py
191 index b0a0952..3e87009 100644
192 --- a/cinder/compute/nova.py
193 +++ b/cinder/compute/nova.py
194 @@ -133,6 +133,7 @@
195
196 class API(base.Base):
197 """API for interacting with novaclient."""
198 + NotFound = nova_exceptions.NotFound
199
200 def __init__(self):
201 self.message_api = message_api.API()
202 @@ -237,3 +238,9 @@
203 resource_uuid=volume_id,
204 detail=message_field.Detail.REIMAGE_VOLUME_FAILED)
205 return result
206 +
207 + @staticmethod
208 + def get_server_volume(context, server_id, volume_id):
209 + # Use microversion that includes attachment_id
210 + nova = novaclient(context, api_version='2.89')
211 + return nova.volumes.get_server_volume(server_id, volume_id)
212 diff --git a/cinder/exception.py b/cinder/exception.py
213 index 906ed39..47d6d3d 100644
214 --- a/cinder/exception.py
215 +++ b/cinder/exception.py
216 @@ -1092,3 +1092,10 @@
217 "Driver initiator data for initiator '%(initiator)s' and backend "
218 "'%(namespace)s' with key '%(key)s' already exists."
219 )
220 +
221 +
222 +class ConflictNovaUsingAttachment(CinderException):
223 + message = _("Detach volume from instance %(instance_id)s using the "
224 + "Compute API")
225 + code = 409
226 + safe = True
227 diff --git a/cinder/tests/unit/api/contrib/test_admin_actions.py b/cinder/tests/unit/api/contrib/test_admin_actions.py
228 index e56b112..2224686 100644
229 --- a/cinder/tests/unit/api/contrib/test_admin_actions.py
230 +++ b/cinder/tests/unit/api/contrib/test_admin_actions.py
231 @@ -1037,6 +1037,8 @@
232 super(AdminActionsAttachDetachTest, self).setUp()
233 # start service to handle rpc messages for attach requests
234 self.svc = self.start_service('volume', host='test')
235 + self.mock_deletion_allowed = self.mock_object(
236 + volume_api.API, 'attachment_deletion_allowed', return_value=None)
237
238 def tearDown(self):
239 self.svc.stop()
240 @@ -1092,6 +1094,16 @@
241 admin_metadata = volume.admin_metadata
242 self.assertEqual(1, len(admin_metadata))
243 self.assertEqual('False', admin_metadata['readonly'])
244 + # One call is for the terminate_connection and the other is for the
245 + # detach
246 + self.assertEqual(2, self.mock_deletion_allowed.call_count)
247 + self.mock_deletion_allowed.assert_has_calls(
248 + [mock.call(self.ctx, None, mock.ANY),
249 + mock.call(self.ctx, attachment['id'], mock.ANY)])
250 + for i in (0, 1):
251 + self.assertIsInstance(
252 + self.mock_deletion_allowed.call_args_list[i][0][2],
253 + objects.Volume)
254
255 def test_force_detach_host_attached_volume(self):
256 # current status is available
257 @@ -1143,6 +1155,16 @@
258 admin_metadata = volume['admin_metadata']
259 self.assertEqual(1, len(admin_metadata))
260 self.assertEqual('False', admin_metadata['readonly'])
261 + # One call is for the terminate_connection and the other is for the
262 + # detach
263 + self.assertEqual(2, self.mock_deletion_allowed.call_count)
264 + self.mock_deletion_allowed.assert_has_calls(
265 + [mock.call(self.ctx, None, mock.ANY),
266 + mock.call(self.ctx, attachment['id'], mock.ANY)])
267 + for i in (0, 1):
268 + self.assertIsInstance(
269 + self.mock_deletion_allowed.call_args_list[i][0][2],
270 + objects.Volume)
271
272 def test_volume_force_detach_raises_remote_error(self):
273 # current status is available
274 @@ -1186,6 +1208,10 @@
275 resp = req.get_response(app())
276 self.assertEqual(HTTPStatus.BAD_REQUEST, resp.status_int)
277
278 + self.mock_deletion_allowed.assert_called_once_with(
279 + self.ctx, None, volume)
280 + self.mock_deletion_allowed.reset_mock()
281 +
282 # test for VolumeBackendAPIException
283 volume_remote_error = (
284 messaging.RemoteError(exc_type='VolumeBackendAPIException'))
285 @@ -1205,6 +1231,8 @@
286 self.assertRaises(messaging.RemoteError,
287 req.get_response,
288 app())
289 + self.mock_deletion_allowed.assert_called_once_with(
290 + self.ctx, None, volume)
291
292 def test_volume_force_detach_raises_db_error(self):
293 # In case of DB error 500 error code is returned to user
294 @@ -1250,6 +1278,8 @@
295 self.assertRaises(messaging.RemoteError,
296 req.get_response,
297 app())
298 + self.mock_deletion_allowed.assert_called_once_with(
299 + self.ctx, None, volume)
300
301 def test_volume_force_detach_missing_connector(self):
302 # current status is available
303 @@ -1290,6 +1320,8 @@
304 # make request
305 resp = req.get_response(app())
306 self.assertEqual(HTTPStatus.ACCEPTED, resp.status_int)
307 + self.mock_deletion_allowed.assert_called_once_with(
308 + self.ctx, None, volume)
309
310 def test_attach_in_used_volume_by_instance(self):
311 """Test that attaching to an in-use volume fails."""
312 diff --git a/cinder/tests/unit/api/v3/test_attachments.py b/cinder/tests/unit/api/v3/test_attachments.py
313 index 6adf5a3..3525135 100644
314 --- a/cinder/tests/unit/api/v3/test_attachments.py
315 +++ b/cinder/tests/unit/api/v3/test_attachments.py
316 @@ -159,6 +159,8 @@
317 @ddt.data('reserved', 'attached')
318 @mock.patch.object(volume_rpcapi.VolumeAPI, 'attachment_delete')
319 def test_delete_attachment(self, status, mock_delete):
320 + self.patch('cinder.volume.api.API.attachment_deletion_allowed',
321 + return_value=None)
322 volume1 = self._create_volume(display_name='fake_volume_1',
323 project_id=fake.PROJECT_ID)
324 attachment = self._create_attachment(
325 diff --git a/cinder/tests/unit/attachments/test_attachments_api.py b/cinder/tests/unit/attachments/test_attachments_api.py
326 index 8f663f3..b956416 100644
327 --- a/cinder/tests/unit/attachments/test_attachments_api.py
328 +++ b/cinder/tests/unit/attachments/test_attachments_api.py
329 @@ -12,12 +12,14 @@
330
331 from unittest import mock
332
333 +from cinder.compute import nova
334 from cinder import context
335 from cinder import db
336 from cinder import exception
337 from cinder import objects
338 from cinder.tests.unit.api.v2 import fakes as v2_fakes
339 from cinder.tests.unit import fake_constants as fake
340 +from cinder.tests.unit import fake_volume
341 from cinder.tests.unit import test
342 from cinder.tests.unit import utils as tests_utils
343 from cinder.volume import api as volume_api
344 @@ -78,10 +80,13 @@
345 attachment.id)
346 self.assertEqual(connection_info, new_attachment.connection_info)
347
348 + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed')
349 @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_delete')
350 def test_attachment_delete_reserved(self,
351 - mock_rpc_attachment_delete):
352 + mock_rpc_attachment_delete,
353 + mock_allowed):
354 """Test attachment_delete with reserved."""
355 + mock_allowed.return_value = None
356 volume_params = {'status': 'available'}
357
358 vref = tests_utils.create_volume(self.context, **volume_params)
359 @@ -94,18 +99,22 @@
360 self.assertEqual(vref.id, aref.volume_id)
361 self.volume_api.attachment_delete(self.context,
362 aobj)
363 + mock_allowed.assert_called_once_with(self.context, aobj)
364
365 # Since it's just reserved and never finalized, we should never make an
366 # rpc call
367 mock_rpc_attachment_delete.assert_not_called()
368
369 + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed')
370 @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_delete')
371 @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_update')
372 def test_attachment_create_update_and_delete(
373 self,
374 mock_rpc_attachment_update,
375 - mock_rpc_attachment_delete):
376 + mock_rpc_attachment_delete,
377 + mock_allowed):
378 """Test attachment_delete."""
379 + mock_allowed.return_value = None
380 volume_params = {'status': 'available'}
381 connection_info = {'fake_key': 'fake_value',
382 'fake_key2': ['fake_value1', 'fake_value2']}
383 @@ -142,6 +151,7 @@
384 self.volume_api.attachment_delete(self.context,
385 aref)
386
387 + mock_allowed.assert_called_once_with(self.context, aref)
388 mock_rpc_attachment_delete.assert_called_once_with(self.context,
389 aref.id,
390 mock.ANY)
391 @@ -173,10 +183,13 @@
392 vref.id)
393 self.assertEqual(2, len(vref.volume_attachment))
394
395 + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed')
396 @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_update')
397 def test_attachment_create_reserve_delete(
398 self,
399 - mock_rpc_attachment_update):
400 + mock_rpc_attachment_update,
401 + mock_allowed):
402 + mock_allowed.return_value = None
403 volume_params = {'status': 'available'}
404 connector = {
405 "initiator": "iqn.1993-08.org.debian:01:cad181614cec",
406 @@ -211,12 +224,15 @@
407 # attachments reserve
408 self.volume_api.attachment_delete(self.context,
409 aref)
410 + mock_allowed.assert_called_once_with(self.context, aref)
411 vref = objects.Volume.get_by_id(self.context,
412 vref.id)
413 self.assertEqual('reserved', vref.status)
414
415 - def test_reserve_reserve_delete(self):
416 + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed')
417 + def test_reserve_reserve_delete(self, mock_allowed):
418 """Test that we keep reserved status across multiple reserves."""
419 + mock_allowed.return_value = None
420 volume_params = {'status': 'available'}
421
422 vref = tests_utils.create_volume(self.context, **volume_params)
423 @@ -235,6 +251,7 @@
424 self.assertEqual('reserved', vref.status)
425 self.volume_api.attachment_delete(self.context,
426 aref)
427 + mock_allowed.assert_called_once_with(self.context, aref)
428 vref = objects.Volume.get_by_id(self.context,
429 vref.id)
430 self.assertEqual('reserved', vref.status)
431 @@ -344,3 +361,169 @@
432 self.context,
433 vref,
434 fake.UUID1)
435 +
436 + def _get_attachment(self, with_instance_id=True):
437 + volume = fake_volume.fake_volume_obj(self.context, id=fake.VOLUME_ID)
438 + volume.volume_attachment = objects.VolumeAttachmentList()
439 + attachment = fake_volume.volume_attachment_ovo(
440 + self.context,
441 + volume_id=fake.VOLUME_ID,
442 + instance_uuid=fake.INSTANCE_ID if with_instance_id else None,
443 + connection_info='{"a": 1}')
444 + attachment.volume = volume
445 + return attachment
446 +
447 + @mock.patch('cinder.compute.nova.API.get_server_volume')
448 + def test_attachment_deletion_allowed_service_call(self, mock_get_server):
449 + """Service calls are never redirected."""
450 + self.context.service_roles = ['reader', 'service']
451 + attachment = self._get_attachment()
452 + self.volume_api.attachment_deletion_allowed(self.context, attachment)
453 + mock_get_server.assert_not_called()
454 +
455 + @mock.patch('cinder.compute.nova.API.get_server_volume')
456 + def test_attachment_deletion_allowed_service_call_different_service_name(
457 + self, mock_get_server):
458 + """Service calls are never redirected and role can be different.
459 +
460 + In this test we support 2 different service roles, the standard service
461 + and a custom one called captain_awesome, and passing the custom one
462 + works as expected.
463 + """
464 + self.override_config('service_token_roles',
465 + ['service', 'captain_awesome'],
466 + group='keystone_authtoken')
467 +
468 + self.context.service_roles = ['reader', 'captain_awesome']
469 + attachment = self._get_attachment()
470 + self.volume_api.attachment_deletion_allowed(self.context, attachment)
471 + mock_get_server.assert_not_called()
472 +
473 + @mock.patch('cinder.compute.nova.API.get_server_volume')
474 + def test_attachment_deletion_allowed_no_instance(self, mock_get_server):
475 + """Attachments with no instance id are never redirected."""
476 + attachment = self._get_attachment(with_instance_id=False)
477 + self.volume_api.attachment_deletion_allowed(self.context, attachment)
478 + mock_get_server.assert_not_called()
479 +
480 + @mock.patch('cinder.compute.nova.API.get_server_volume')
481 + def test_attachment_deletion_allowed_no_conn_info(self, mock_get_server):
482 + """Attachments with no connection information are never redirected."""
483 + attachment = self._get_attachment(with_instance_id=False)
484 + attachment.connection_info = None
485 + self.volume_api.attachment_deletion_allowed(self.context, attachment)
486 +
487 + mock_get_server.assert_not_called()
488 +
489 + def test_attachment_deletion_allowed_no_attachment(self):
490 + """For users don't allow operation with no attachment reference."""
491 + self.assertRaises(exception.ConflictNovaUsingAttachment,
492 + self.volume_api.attachment_deletion_allowed,
493 + self.context, None)
494 +
495 + @mock.patch('cinder.objects.VolumeAttachment.get_by_id',
496 + side_effect=exception.VolumeAttachmentNotFound())
497 + def test_attachment_deletion_allowed_attachment_id_not_found(self,
498 + mock_get):
499 + """For users don't allow if attachment cannot be found."""
500 + attachment = self._get_attachment(with_instance_id=False)
501 + attachment.connection_info = None
502 + self.assertRaises(exception.ConflictNovaUsingAttachment,
503 + self.volume_api.attachment_deletion_allowed,
504 + self.context, fake.ATTACHMENT_ID)
505 + mock_get.assert_called_once_with(self.context, fake.ATTACHMENT_ID)
506 +
507 + def test_attachment_deletion_allowed_volume_no_attachments(self):
508 + """For users allow if volume has no attachments."""
509 + volume = tests_utils.create_volume(self.context)
510 + self.volume_api.attachment_deletion_allowed(self.context, None, volume)
511 +
512 + def test_attachment_deletion_allowed_multiple_attachment(self):
513 + """For users don't allow if volume has multiple attachments."""
514 + attachment = self._get_attachment()
515 + volume = attachment.volume
516 + volume.volume_attachment = objects.VolumeAttachmentList(
517 + objects=[attachment, attachment])
518 + self.assertRaises(exception.ConflictNovaUsingAttachment,
519 + self.volume_api.attachment_deletion_allowed,
520 + self.context, None, volume)
521 +
522 + @mock.patch('cinder.compute.nova.API.get_server_volume')
523 + def test_attachment_deletion_allowed_vm_not_found(self, mock_get_server):
524 + """Don't reject if instance doesn't exist"""
525 + mock_get_server.side_effect = nova.API.NotFound(404)
526 + attachment = self._get_attachment()
527 + self.volume_api.attachment_deletion_allowed(self.context, attachment)
528 +
529 + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID,
530 + fake.VOLUME_ID)
531 +
532 + @mock.patch('cinder.compute.nova.API.get_server_volume')
533 + def test_attachment_deletion_allowed_attachment_from_volume(
534 + self, mock_get_server):
535 + """Don't reject if instance doesn't exist"""
536 + mock_get_server.side_effect = nova.API.NotFound(404)
537 + attachment = self._get_attachment()
538 + volume = attachment.volume
539 + volume.volume_attachment = objects.VolumeAttachmentList(
540 + objects=[attachment])
541 + self.volume_api.attachment_deletion_allowed(self.context, None, volume)
542 +
543 + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID,
544 + volume.id)
545 +
546 + @mock.patch('cinder.objects.VolumeAttachment.get_by_id')
547 + def test_attachment_deletion_allowed_mismatched_volume_and_attach_id(
548 + self, mock_get_attatchment):
549 + """Reject if volume and attachment don't match."""
550 + attachment = self._get_attachment()
551 + volume = attachment.volume
552 + volume.volume_attachment = objects.VolumeAttachmentList(
553 + objects=[attachment])
554 + attachment2 = self._get_attachment()
555 + attachment2.volume_id = attachment.volume.id = fake.VOLUME2_ID
556 + self.assertRaises(exception.InvalidInput,
557 + self.volume_api.attachment_deletion_allowed,
558 + self.context, attachment2.id, volume)
559 + mock_get_attatchment.assert_called_once_with(self.context,
560 + attachment2.id)
561 +
562 + @mock.patch('cinder.objects.VolumeAttachment.get_by_id')
563 + @mock.patch('cinder.compute.nova.API.get_server_volume')
564 + def test_attachment_deletion_allowed_not_found_attachment_id(
565 + self, mock_get_server, mock_get_attachment):
566 + """Don't reject if instance doesn't exist"""
567 + mock_get_server.side_effect = nova.API.NotFound(404)
568 + mock_get_attachment.return_value = self._get_attachment()
569 +
570 + self.volume_api.attachment_deletion_allowed(self.context,
571 + fake.ATTACHMENT_ID)
572 +
573 + mock_get_attachment.assert_called_once_with(self.context,
574 + fake.ATTACHMENT_ID)
575 +
576 + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID,
577 + fake.VOLUME_ID)
578 +
579 + @mock.patch('cinder.compute.nova.API.get_server_volume')
580 + def test_attachment_deletion_allowed_mismatch_id(self, mock_get_server):
581 + """Don't reject if attachment id on nova doesn't match"""
582 + mock_get_server.return_value.attachment_id = fake.ATTACHMENT2_ID
583 + attachment = self._get_attachment()
584 + self.volume_api.attachment_deletion_allowed(self.context, attachment)
585 +
586 + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID,
587 + fake.VOLUME_ID)
588 +
589 + @mock.patch('cinder.compute.nova.API.get_server_volume')
590 + def test_attachment_deletion_allowed_user_call_fails(self,
591 + mock_get_server):
592 + """Fail user calls"""
593 + attachment = self._get_attachment()
594 + mock_get_server.return_value.attachment_id = attachment.id
595 + self.assertRaises(exception.ConflictNovaUsingAttachment,
596 + self.volume_api.attachment_deletion_allowed,
597 + self.context, attachment)
598 +
599 + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID,
600 + fake.VOLUME_ID)
601 diff --git a/cinder/tests/unit/policies/test_attachments.py b/cinder/tests/unit/policies/test_attachments.py
602 index 7307a83..8d3040b 100644
603 --- a/cinder/tests/unit/policies/test_attachments.py
604 +++ b/cinder/tests/unit/policies/test_attachments.py
605 @@ -62,6 +62,9 @@
606
607 self.api_path = '/v3/%s/attachments' % (self.project_id)
608 self.api_version = mv.NEW_ATTACH
609 + self.mock_is_service = self.patch(
610 + 'cinder.volume.api.API.is_service_request',
611 + return_value=True)
612
613 def _initialize_connection(self, volume, connector):
614 return {'data': connector}
615 diff --git a/cinder/tests/unit/policies/test_volume_actions.py b/cinder/tests/unit/policies/test_volume_actions.py
616 index d9b2edc..17dacd5 100644
617 --- a/cinder/tests/unit/policies/test_volume_actions.py
618 +++ b/cinder/tests/unit/policies/test_volume_actions.py
619 @@ -89,6 +89,9 @@
620 self._initialize_connection)
621 self.api_path = '/v3/%s/volumes' % (self.project_id)
622 self.api_version = mv.BASE_VERSION
623 + self.mock_is_service = self.patch(
624 + 'cinder.volume.api.API.is_service_request',
625 + return_value=True)
626
627 def _initialize_connection(self, volume, connector):
628 return {'data': connector}
629 @@ -946,6 +949,7 @@
630 self.assertEqual(HTTPStatus.ACCEPTED, response.status_int)
631
632 body = {"os-detach": {}}
633 + # Detach for user call succeeds because the volume has no attachments
634 response = self._get_request_response(admin_context, path, 'POST',
635 body=body)
636 self.assertEqual(HTTPStatus.ACCEPTED, response.status_int)
637 @@ -966,6 +970,7 @@
638 body=body)
639 self.assertEqual(HTTPStatus.ACCEPTED, response.status_int)
640
641 + # Succeeds for a user call because there are no attachments
642 body = {"os-detach": {}}
643 response = self._get_request_response(user_context, path, 'POST',
644 body=body)
645 @@ -1062,6 +1067,7 @@
646 'terminate_connection')
647 def test_admin_can_initialize_terminate_conn(self, mock_t, mock_i):
648 admin_context = self.admin_context
649 + admin_context.service_roles = ['service']
650
651 volume = self._create_fake_volume(admin_context)
652 path = '/v3/%(project_id)s/volumes/%(volume_id)s/action' % {
653 @@ -1084,6 +1090,7 @@
654 'terminate_connection')
655 def test_owner_can_initialize_terminate_conn(self, mock_t, mock_i):
656 user_context = self.user_context
657 + user_context.service_roles = ['service']
658
659 volume = self._create_fake_volume(user_context)
660 path = '/v3/%(project_id)s/volumes/%(volume_id)s/action' % {
661 diff --git a/cinder/tests/unit/volume/test_connection.py b/cinder/tests/unit/volume/test_connection.py
662 index 0d472d6..6bd9b89 100644
663 --- a/cinder/tests/unit/volume/test_connection.py
664 +++ b/cinder/tests/unit/volume/test_connection.py
665 @@ -1334,7 +1334,8 @@
666 self.context,
667 volume, None, None, None, None)
668
669 - def test_volume_detach_in_maintenance(self):
670 + @mock.patch('cinder.volume.api.API.attachment_deletion_allowed')
671 + def test_volume_detach_in_maintenance(self, mock_attachment_deletion):
672 """Test detach the volume in maintenance."""
673 test_meta1 = {'fake_key1': 'fake_value1', 'fake_key2': 'fake_value2'}
674 volume = tests_utils.create_volume(self.context, metadata=test_meta1,
675 @@ -1345,3 +1346,5 @@
676 volume_api.detach,
677 self.context,
678 volume, None)
679 + mock_attachment_deletion.assert_called_once_with(self.context,
680 + None, volume)
681 diff --git a/cinder/volume/api.py b/cinder/volume/api.py
682 index c36d171..3ea48b0 100644
683 --- a/cinder/volume/api.py
684 +++ b/cinder/volume/api.py
685 @@ -34,6 +34,7 @@
686
687 from cinder.api import common
688 from cinder.common import constants
689 +from cinder import compute
690 from cinder import context
691 from cinder import coordination
692 from cinder import db
693 @@ -862,11 +863,14 @@
694 attachment_id: str) -> None:
695 context.authorize(vol_action_policy.DETACH_POLICY,
696 target_obj=volume)
697 + self.attachment_deletion_allowed(context, attachment_id, volume)
698 +
699 if volume['status'] == 'maintenance':
700 LOG.info('Unable to detach volume, '
701 'because it is in maintenance.', resource=volume)
702 msg = _("The volume cannot be detached in maintenance mode.")
703 raise exception.InvalidVolume(reason=msg)
704 +
705 detach_results = self.volume_rpcapi.detach_volume(context, volume,
706 attachment_id)
707 LOG.info("Detach volume completed successfully.",
708 @@ -893,6 +897,19 @@
709 resource=volume)
710 return init_results
711
712 + @staticmethod
713 + def is_service_request(ctxt: 'context.RequestContext') -> bool:
714 + """Check if a request is coming from a service
715 +
716 + A request is coming from a service if it has a service token and the
717 + service user has one of the roles configured in the
718 + `service_token_roles` configuration option in the
719 + `[keystone_authtoken]` section (defaults to `service`).
720 + """
721 + roles = ctxt.service_roles
722 + service_roles = set(CONF.keystone_authtoken.service_token_roles)
723 + return bool(roles and service_roles.intersection(roles))
724 +
725 def terminate_connection(self,
726 context: context.RequestContext,
727 volume: objects.Volume,
728 @@ -900,6 +917,8 @@
729 force: bool = False) -> None:
730 context.authorize(vol_action_policy.TERMINATE_POLICY,
731 target_obj=volume)
732 + self.attachment_deletion_allowed(context, None, volume)
733 +
734 self.volume_rpcapi.terminate_connection(context,
735 volume,
736 connector,
737 @@ -2521,11 +2540,90 @@
738 attachment_ref.save()
739 return attachment_ref
740
741 + def attachment_deletion_allowed(self,
742 + ctxt: context.RequestContext,
743 + attachment_or_attachment_id,
744 + volume=None):
745 + """Check if deleting an attachment is allowed (Bug #2004555)
746 +
747 + Allowed is based on the REST API policy, the status of the attachment,
748 + where it is used, and who is making the request.
749 +
750 + Deleting an attachment on the Cinder side while leaving the volume
751 + connected to the nova host results in leftover devices that can lead to
752 + data leaks/corruption.
753 +
754 + OS-Brick may have code to detect it, but in some cases it is detected
755 + after it has already been exposed, so it's better to prevent users from
756 + being able to intentionally triggering the issue.
757 + """
758 + # It's ok to delete an attachment if the request comes from a service
759 + if self.is_service_request(ctxt):
760 + return
761 +
762 + if not attachment_or_attachment_id and volume:
763 + if not volume.volume_attachment:
764 + return
765 + if len(volume.volume_attachment) == 1:
766 + attachment_or_attachment_id = volume.volume_attachment[0]
767 +
768 + if isinstance(attachment_or_attachment_id, str):
769 + try:
770 + attachment = objects.VolumeAttachment.get_by_id(
771 + ctxt, attachment_or_attachment_id)
772 + except exception.VolumeAttachmentNotFound:
773 + attachment = None
774 + else:
775 + attachment = attachment_or_attachment_id
776 +
777 + if attachment:
778 + if volume:
779 + if volume.id != attachment.volume_id:
780 + raise exception.InvalidInput(
781 + reason='Mismatched volume and attachment')
782 +
783 + server_id = attachment.instance_uuid
784 + # It's ok to delete if it's not connected to a vm.
785 + if not server_id or not attachment.connection_info:
786 + return
787 +
788 + volume = volume or attachment.volume
789 + nova = compute.API()
790 + LOG.info('Attachment connected to vm %s, checking data on nova',
791 + server_id)
792 + # If nova is down the client raises 503 and we report that
793 + try:
794 + nova_volume = nova.get_server_volume(ctxt, server_id,
795 + volume.id)
796 + except nova.NotFound:
797 + LOG.warning('Instance or volume not found on Nova, deleting '
798 + 'attachment locally, which may leave leftover '
799 + 'devices on Nova compute')
800 + return
801 +
802 + if nova_volume.attachment_id != attachment.id:
803 + LOG.warning('Mismatch! Nova has different attachment id (%s) '
804 + 'for the volume, deleting attachment locally. '
805 + 'May leave leftover devices in a compute node',
806 + nova_volume.attachment_id)
807 + return
808 + else:
809 + server_id = ''
810 +
811 + LOG.error('Detected user call to delete in-use attachment. Call must '
812 + 'come from the nova service and nova must be configured to '
813 + 'send the service token. Bug #2004555')
814 + raise exception.ConflictNovaUsingAttachment(instance_id=server_id)
815 +
816 def attachment_delete(self,
817 ctxt: context.RequestContext,
818 attachment) -> objects.VolumeAttachmentList:
819 + # Check if policy allows user to delete attachment
820 ctxt.authorize(attachment_policy.DELETE_POLICY,
821 target_obj=attachment)
822 +
823 + self.attachment_deletion_allowed(ctxt, attachment)
824 +
825 volume = attachment.volume
826
827 if attachment.attach_status == fields.VolumeAttachStatus.RESERVED:
828 diff --git a/doc/source/configuration/block-storage/service-token.rst b/doc/source/configuration/block-storage/service-token.rst
829 index 1c48552..a04aa05 100644
830 --- a/doc/source/configuration/block-storage/service-token.rst
831 +++ b/doc/source/configuration/block-storage/service-token.rst
832 @@ -1,6 +1,6 @@
833 -=========================================================
834 -Using service tokens to prevent long-running job failures
835 -=========================================================
836 +====================
837 +Using service tokens
838 +====================
839
840 When a user initiates a request whose processing involves multiple services
841 (for example, a boot-from-volume request to the Compute Service will require
842 @@ -8,20 +8,32 @@
843 Image Service), the user's token is handed from service to service. This
844 ensures that the requestor is tracked correctly for audit purposes and also
845 guarantees that the requestor has the appropriate permissions to do what needs
846 -to be done by the other services. If the chain of operations takes a long
847 -time, however, the user's token may expire before the action is completed,
848 -leading to the failure of the user's original request.
849 +to be done by the other services.
850
851 -One way to deal with this is to set a long token life in Keystone, and this may
852 -be what you are currently doing. But this can be problematic for installations
853 -whose security policies prefer short user token lives. Beginning with the
854 -Queens release, an alternative solution is available. You have the ability to
855 -configure some services (particularly Nova and Cinder) to send a "service
856 -token" along with the user's token. When properly configured, the Identity
857 -Service will validate an expired user token *when it is accompanied by a valid
858 -service token*. Thus if the user's token expires somewhere during a long
859 -running chain of operations among various OpenStack services, the operations
860 -can continue.
861 +There are several instances where we want to differentiate between a request
862 +coming from the user to one coming from another OpenStack service on behalf of
863 +the user:
864 +
865 +- **For security reasons** There are some operations in the Block Storage
866 + service, required for normal operations, that could be exploited by a
867 + malicious user to gain access to resources belonging to other users. By
868 + differentiating when the request comes directly from a user and when from
869 + another OpenStack service the Cinder service can protect the deployment.
870 +
871 +- To prevent long-running job failures: If the chain of operations takes a long
872 + time, the user's token may expire before the action is completed, leading to
873 + the failure of the user's original request.
874 +
875 + One way to deal with this is to set a long token life in Keystone, and this
876 + may be what you are currently doing. But this can be problematic for
877 + installations whose security policies prefer short user token lives.
878 + Beginning with the Queens release, an alternative solution is available. You
879 + have the ability to configure some services (particularly Nova and Cinder) to
880 + send a "service token" along with the user's token. When properly
881 + configured, the Identity Service will validate an expired user token *when it
882 + is accompanied by a valid service token*. Thus if the user's token expires
883 + somewhere during a long running chain of operations among various OpenStack
884 + services, the operations can continue.
885
886 .. note::
887 There's nothing special about a service token. It's a regular token
888 diff --git a/doc/source/configuration/index.rst b/doc/source/configuration/index.rst
889 index 002469f..ed599de 100644
890 --- a/doc/source/configuration/index.rst
891 +++ b/doc/source/configuration/index.rst
892 @@ -6,6 +6,7 @@
893 :maxdepth: 1
894
895 block-storage/block-storage-overview.rst
896 + block-storage/service-token.rst
897 block-storage/volume-drivers.rst
898 block-storage/backup-drivers.rst
899 block-storage/schedulers.rst
900 @@ -15,10 +16,16 @@
901 block-storage/policy-config-HOWTO.rst
902 block-storage/fc-zoning.rst
903 block-storage/volume-encryption.rst
904 - block-storage/service-token.rst
905 block-storage/config-options.rst
906 block-storage/samples/index.rst
907
908 +.. warning::
909 +
910 + For security reasons **Service Tokens must to be configured** in OpenStack
911 + for Cinder to operate securely. Pay close attention to the :doc:`specific
912 + section describing it: <block-storage/service-token>`. See
913 + https://bugs.launchpad.net/nova/+bug/2004555 for details.
914 +
915 .. note::
916
917 The examples of common configurations for shared
918 diff --git a/doc/source/install/index.rst b/doc/source/install/index.rst
919 index 931ea78..ae1732a 100644
920 --- a/doc/source/install/index.rst
921 +++ b/doc/source/install/index.rst
922 @@ -35,6 +35,13 @@
923
924 The following links describe how to install the Cinder Block Storage Service:
925
926 +.. warning::
927 +
928 + For security reasons **Service Tokens must to be configured** in OpenStack
929 + for Cinder to operate securely. Pay close attention to the :doc:`specific
930 + section describing it: <../configuration/block-storage/service-token>`. See
931 + https://bugs.launchpad.net/nova/+bug/2004555 for details.
932 +
933 .. toctree::
934
935 get-started-block-storage
936 diff --git a/releasenotes/notes/redirect-detach-nova-4b7b7902d7d182e0.yaml b/releasenotes/notes/redirect-detach-nova-4b7b7902d7d182e0.yaml
937 new file mode 100644
938 index 0000000..dd3ea4f
939 --- /dev/null
940 +++ b/releasenotes/notes/redirect-detach-nova-4b7b7902d7d182e0.yaml
941 @@ -0,0 +1,43 @@
942 +---
943 +critical:
944 + - |
945 + Detaching volumes will fail if Nova is not `configured to send service
946 + tokens <https://docs.openstack.org/cinder/latest/configuration/block-storage/service-token.html>`_,
947 + please read the upgrade section for more information. (`Bug #2004555
948 + <https://bugs.launchpad.net/cinder/+bug/2004555>`_).
949 +upgrade:
950 + - |
951 + Nova must be `configured to send service tokens
952 + <https://docs.openstack.org/cinder/latest/configuration/block-storage/service-token.html>`_
953 + **and** cinder must be configured to recognize at least one of the roles
954 + that the nova service user has been assigned in keystone. By default,
955 + cinder will recognize the ``service`` role, so if the nova service user
956 + is assigned a differently named role in your cloud, you must adjust your
957 + cinder configuration file (``service_token_roles`` configuration option
958 + in the ``keystone_authtoken`` section). If nova and cinder are not
959 + configured correctly in this regard, detaching volumes will no longer
960 + work (`Bug #2004555 <https://bugs.launchpad.net/cinder/+bug/2004555>`_).
961 +security:
962 + - |
963 + As part of the fix for `Bug #2004555
964 + <https://bugs.launchpad.net/cinder/+bug/2004555>`_, cinder now rejects
965 + user attachment delete requests for attachments that are being used by nova
966 + instances to ensure that no leftover devices are produced on the compute
967 + nodes which could be used to access another project's volumes. Terminate
968 + connection, detach, and force detach volume actions (calls that are not
969 + usually made by users directly) are, in most cases, not allowed for users.
970 +fixes:
971 + - |
972 + `Bug #2004555 <https://bugs.launchpad.net/cinder/+bug/2004555>`_: Fixed
973 + issue where a user manually deleting an attachment, calling terminate
974 + connection, detach, or force detach, for a volume that is still used by a
975 + nova instance resulted in leftover devices on the compute node. These
976 + operations will now fail when it is believed to be a problem.
977 +issues:
978 + - |
979 + For security reasons (`Bug #2004555
980 + <https://bugs.launchpad.net/cinder/+bug/2004555>`_) manually deleting an
981 + attachment, manually doing the ``os-terminate_connection``, ``os-detach``
982 + or ``os-force_detach`` actions will no longer be allowed in most cases
983 + unless the request is coming from another OpenStack service on behalf of a
984 + user.
+1
-0
debian/patches/series less more
11 do-not-use-urllib3-from-botocore.patch
22 remove-test_backup_s3.patch
33 py3.11-test_rbd_iscsi_Make_tests_compatible_with_python.patch
4 CVE-2023-2088_Reject_unsafe_delete_attachment_calls.patch