[OSSA 2013-001] No authentication on block device used for os-volume_boot

The connection code attaches to the volume using api commands to cinder. Volumes belonging to other users will not be visible, so this shouldn't be possible in folsom.

@Vish: would Essex be affected ?

Also would Folsom with nova-volumes be affected ?

attach_volume is definitely not affected. It appears that attach_volume_boot has an elevated context so it may be possible to boot off of other peoples volumes. This appears to affect folsom/nova-volumes as well, although it should be verified. A simple fix would be to make sure we don't have an elevated context before doing the volume_api.get and volume_api.get_snapshot code in stable/essex here:

 375 # TODO(yamahata): default name and description
 376 snapshot = self.volume_api.get_snapshot(context,
 377 bdm['snapshot_id'])
 378 vol = self.volume_api.create(context, bdm['volume_size'],
 379 '', '', snapshot)
 380 # TODO(yamahata): creating volume simultaneously
 381 # reduces creation time?
 382 self.volume_api.wait_creation(context, vol)
 383 self.db.block_device_mapping_update(
 384 context, bdm['id'], {'volume_id': vol['id']})
 385 bdm['volume_id'] = vol['id']
 386
 387 if bdm['volume_id'] is not None:
 388 volume = self.volume_api.get(context, bdm['volume_id'])
 389 self.volume_api.check_attach(context, volume)
 390 cinfo = self._attach_volume_boot(context,
 391 instance,
 392 volume,
 393 bdm['device_name'])

Looks like a deeper analysis is needed on this.. Vish, anyone you'd suggest would be a good fit to look into it ?

I know rmk has some essex installs with volumes. I will add him to the list to see if he can verify.

Verified with a trunk devstack using nova-volumes. Here is a fix for trunk.

Verified with a trunk devstack using nova-volumes. Here is a fix for trunk.

Now I'm confused. I understood that Grizzly was unaffected and now you say you have a trunk patch...
Which series are actually affected ?

Adding nova-core for security patch review.

ttx: only using nova-volumes is affected which means it doesn't affect grizzly. The fix involves checking at the api layer which is actually still valuable in trunk even though the security issue is not present. It should apply against folsom just fine. Rmk is working on a version for essex.

(the reason the patch is still valuable against trunk is it makes the failure happen at the api layer which causes the create to fail fast instead of putting the instance into error later in the process)

Patch mostly looks good to me ... my only comment/question is whether we could catch something more specific than just "Exception".

@vish: thx for the explanation. As far as this security bug goes, I'll just consider it doesn't apply to Grizzly (even if pushing the fix there can't hurt).

Adding Daviey so that he can look into stable/diablo being affected or not.

+2 on the patch. The Exception thing is just a potential cleanup, not a problem.

@nova-core: We need one more review here so we can proceed

@nova-core: one more review still needed

+2 from me

Refreshing patches: volume.patch does not apply cleanly to Grizzly
rmk: are you working on an Essex patch ?

@rmk: any progress on the Essex patch ?

Adding the full Diablo maintenance team (rather than just Dave) to evaluate Diablo's vulnerability, although having the Essex patch will help in assessing that.

My proposed impact description:

Title: Boot from volume allows access to random volumes
Reporter: Phil Day (HP)
Products: Nova
Affects: ??

Description:
Phil Day from HP reported a vulnerability in volume attachment in nova-volume, affecting the boot-from-volume feature. By passing a specific volume ID, an authenticated user may be able to boot from a volume he doesn't own, potentially resulting in full access to that 3rd-party volume contents. Folsom setups making use of Cinder are not affected.

Looks like rmk is not making progress on the essex patch. Anyone else in nova-core want to have a try ?

Here's the first pass at an essex patch. I haven't been able to test it though, so please review with care.

@nova-core: please have a look at proposed essex patch in comment 23

essex patch looks valid to me +1

Still need one more +1 on the Essex patch
Everyone alright with proposed impact description ?

Last chance to propose a patch for Diablo, otherwise we'll release this without the diablo fix...

+1 from me

Please validate impact description:

========
Title: Boot from volume allows access to random volumes
Reporter: Phil Day (HP)
Products: Nova
Affects: Essex, Folsom

Description:
Phil Day from HP reported a vulnerability in volume attachment in nova-volume, affecting the boot-from-volume feature. By passing a specific volume ID, an authenticated user may be able to boot from a volume he doesn't own, potentially resulting in full access to that 3rd-party volume contents. Folsom setups making use of Cinder are not affected.
========

New refresh of the Grizzly patch

Thierry - re the grizzly patch - it looks good although the same thing is done in compute manager's _setup_block_device_mapping.

Just a heads up: I am trying to rework BDMs at the moment as part of https://blueprints.launchpad.net/nova/+spec/improve-block-device-handling - and I am hoping to propose more thorough validations as a part of those changes. so I am likely to remove/rework the bit of nova.api. The patch should be ready during next week.

Remaining tasks before disclosure:
Anyone: please review impact statement

Notification sent out to stakeholders. Proposed CRD is Thursday, January 24th at 1500 UTC.

Bit confused as to why there's a grizzly patch if we're saying grizzly isn't affected?

@Mark: there is a bug in Grizzly, but without security consequences. See comment 10.

I just started looking at this for Ubuntu. The essex patch fails in its testsuite with various errors like this:
======================================================================
ERROR: test_run_with_snapshot (nova.tests.api.ec2.test_cloud.CloudTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/«BUILDDIR»/nova-2012.1.3+stable-20120827-4d2a4afe/nova/tests/api/ec2/test_cloud.py", line 2014, in test_run_with_snapshot
    ec2_instance_id = self._run_instance(**kwargs)
  File "/«BUILDDIR»/nova-2012.1.3+stable-20120827-4d2a4afe/nova/tests/api/ec2/test_cloud.py", line 1441, in _run_instance
    rv = self.cloud.run_instances(self.context, **kwargs)
  File "/«BUILDDIR»/nova-2012.1.3+stable-20120827-4d2a4afe/nova/api/ec2/cloud.py", line 1341, in run_instances
    block_device_mapping=kwargs.get('block_device_mapping', {}))
  File "/«BUILDDIR»/nova-2012.1.3+stable-20120827-4d2a4afe/nova/compute/api.py", line 690, in create
    scheduler_hints=scheduler_hints)
  File "/«BUILDDIR»/nova-2012.1.3+stable-20120827-4d2a4afe/nova/compute/api.py", line 395, in _create_instance
    if bdm.get('snapshot_id') is not None and bdm['volume_id'] is None:
KeyError: "'volume_id'\n-------------------- >> begin captured logging << --------------------\nnova.service: AUDIT: Starting compute node (version 2012.1.3-LOCALBRANCH:LOCALREVISION)\nnova.virt.fake: INFO: Compute_service record created for acde74615e9e4451837745036802057d \nnova.service: AUDIT: Starting scheduler node (version 2012.1.3-LOCALBRANCH:LOCALREVISION)\nnova.service: AUDIT: Starting network node (version 2012.1.3-LOCALBRANCH:LOCALREVISION)\nnova.service: AUDIT: Starting volume node (version 2012.1.3-LOCALBRANCH:LOCALREVISION)\nnova.api.ec2.cloud: AUDIT: Create snapshot of volume vol-00000001\nnova.volume.manager: INFO: snapshot snapshot-00000001: creating\nnova.api.ec2.cloud: AUDIT: Create snapshot of volume vol-00000001\nnova.volume.manager: INFO: snapshot snapshot-00000002: creating\n--------------------- >> end captured logging << ---------------------"

This is because of the check for "bdm['volume_id'] is None". We need to verify that 'volume_id' is a key in bdm before accessing the value for the key. Attached is an updated patch for essex that passes its test suite on Ubuntu.

FYI, the folsom patch suffers from the same problem. After adjusting it to use:
+ def _validate_bdm(self, context, instance):
+ for bdm in self.db.block_device_mapping_get_all_by_instance(
+ context, instance['uuid']):
+ # NOTE(vish): For now, just make sure the volumes are accessible.
+ if bdm['snapshot_id'] is not None and \
+ 'volume_id' in bdm and bdm['volume_id'] is None:
+ try:
+ self.volume_api.get_snapshot(context, bdm['snapshot_id'])
+ except Exception:
+ raise exception.InvalidBDMSnapshot(id=bdm['snapshot_id'])
+ elif 'volume_id' in bdm and bdm['volume_id'] is not None:
+ try:
+ self.volume_api.get(context, bdm['volume_id'])
+ except Exception:
+ raise exception.InvalidBDMVolume(id=bdm['volume_id'])
+

it passes its testsuite as well (I would attach the patch, but I had to massage it a little for the version of Folsom in Ubuntu).

Visually inspecting Diablo, it seems affected too (the original report was against Diablo, so I am not sure why it is marked Incomplete...).

Attached is a patch for Diablo backported from Essex with my changes above. It does pass the testsuite included in Diablo on Ubuntu, but beyond that I haven't tested it.

Argh. We really need that private Gerrit and tests to validate patches better. Thanks for catching that. I suggest we push back publication to next Tuesday (this was a really short embargo period) and communicate the new set of patches to the stakeholders. Russell, thoughts?

About Diablo, we were waiting for a confirmation since the Diablo code base mentioned in the report is based on an early and heavily patched milestone :) The diablo-maint team didn't really pick it up though, hence the "Incomplete" status.

Red Hat are OK with this going out today at least. The ammendments simple.
Though when verifiying them I noticed the logic could be simplified somewhat.
Also the existing patches didn't contain commit messages.
Also all didn't apply cleanly to the latest trees.
Also the essex and diablo ones duplicated the InvalidSnapshot exception class.

So I redid all the patches against the latest trees , and collated all the info
from this bug report into the commit messages, and verified pertinent tests pass.

patches following...

redone diablo patch

redone essex patch

redone folsom patch

redone grizzly patch

DISCLOSURE DELAYED to Tuesday, January 29th at 1500 UTC.

Updated Essex patch to fix pep8 error

Updated Folsom patch to fix a pep8 issue

And updated Grizzly patch to fix pep8

It would be good to get another ACK on this

+2 on russellb's patch, looks good to me

+2

Note I'll push these patches to E,F,G tomorrow at 1500 UTC,
and approve them immediately as preapproved.

I want to amend the Author to Vish (who I double checked with).

Fix proposed to branch: master
Review: https://review.openstack.org/20698

Fix proposed to branch: stable/folsom
Review: https://review.openstack.org/20699

Fix proposed to branch: stable/essex
Review: https://review.openstack.org/20700

Fix proposed to branch: stable/diablo
Review: https://review.openstack.org/20703

Reviewed: https://review.openstack.org/20698
Committed: http://github.com/openstack/nova/commit/24fffd9d8b77e9b71e8013fc22c172f76bb4e84c
Submitter: Jenkins
Branch: master

commit 24fffd9d8b77e9b71e8013fc22c172f76bb4e84c
Author: Vishvananda Ishaya <email address hidden>
Date: Thu Jan 24 10:07:33 2013 +0000

    validate specified volumes to boot from at the API layer

    This causes the create to fail fast instead of putting
    the instance into error later in the process.

    Related to bug: 1069904
    Change-Id: I5f7c8d20d3ebf33ce1ce64bf0a8418bd2b5a6411

Reviewed: https://review.openstack.org/20700
Committed: http://github.com/openstack/nova/commit/243d516cea9d3caa5a8267b12d2f577dcb24193b
Submitter: Jenkins
Branch: stable/essex

commit 243d516cea9d3caa5a8267b12d2f577dcb24193b
Author: Vishvananda Ishaya <email address hidden>
Date: Thu Jan 24 10:45:19 2013 +0000

    disallow boot from volume from specifying arbitrary volumes

    Fix a vulnerability in volume attachment in nova-volume, affecting the
    boot-from-volume feature. By passing a specific volume ID, an
    authenticated user may be able to boot from a volume they don't own,
    potentially resulting in full access to that 3rd-party volume.

    Fixes bug: 1069904, CVE-2013-0208
    Change-Id: I5f7c8d20d3ebf33ce1ce64bf0a8418bd2b5a6411

Reviewed: https://review.openstack.org/20699
Committed: http://github.com/openstack/nova/commit/317cc0af385536dee43ef2addad50a91357fc1ad
Submitter: Jenkins
Branch: stable/folsom

commit 317cc0af385536dee43ef2addad50a91357fc1ad
Author: Vishvananda Ishaya <email address hidden>
Date: Thu Jan 24 10:07:33 2013 +0000

    disallow boot from volume from specifying arbitrary volumes

    Fix a vulnerability in volume attachment in nova-volume, affecting the
    boot-from-volume feature. By passing a specific volume ID, an
    authenticated user may be able to boot from a volume they don't own,
    potentially resulting in full access to that 3rd-party volume.
    Folsom setups making use of Cinder are not affected.

    Fixes bug: 1069904, CVE-2013-0208
    Change-Id: I5f7c8d20d3ebf33ce1ce64bf0a8418bd2b5a6411

OSSA 2013-001