[OSSA 2015-011] Format-guessing and file disclosure in image convert (CVE-2015-1850, CVE-2015-1851)

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.

Feels legit to me, could a cinder-coresec person confirm ?

yep, that's nasty... Mike has verified as well

Sorry for the late response on this (midcycle meetup last week and catching up with emails). I was definitely able to verify this with the reproduce steps. Thanks for reporting this Bastian.

On Mon, Feb 02, 2015 at 06:23:00PM -0000, Mike Perez wrote:
> Sorry for the late response on this (midcycle meetup last week and
> catching up with emails). I was definitely able to verify this with the
> reproduce steps. Thanks for reporting this Bastian.

While you are at it, there may be more surprises.
- cinder/nova calls "qemu-img info" without format. While this only
  leads to missidentification, I don't know if this could be used
  further.
- nova/cinder calls "qemu-img resize" without format. Unlikely to be
  exploitable.
- nova calls "qemu-img convert" without input format. This should be
  exploitable if instance storage uses raw images (raw, rbd, lvm).

Bastian

--
The face of war has never changed. Surely it is more logical to heal
than to kill.
  -- Surak of Vulcan, "The Savage Curtain", stardate 5906.5

Adding nova-coresec to investigate the last case mentioned in comment 5

I've set the priority to medium to show that it's "next on the list".

Hi

It's been almost four weeks. I've seen no proposed patches or pointers to any activity. I think it is time to publish this vulnerability.

Bastian

Tony has agreed to look into this from the Nova side after he finishes up some other work he's doing.

Looking through the Nova code it seems that all of our uses of qemu-img create pass -f and an input format. Is this exploitable with just -O being left off?

A quick grep on nova 2014.2.2 shows:

| nova/virt/images.py: cmd = ('qemu-img', 'convert', '-O', out_format, source, dest)

Ahh, good catch. I glossed over that one somehow. And that is used in the code path for creating a snapshot to upload.

FYI If cinder (or any part of Nova/Glance/etc) ever creates qcow2 files with backing files, it must also make sure to use the '-obacking_fmt=FORMATNAME' option when creating them, to stop QEMU probiing formats when opening backing files.

@waldi: I couldn't find your affiliation, any company you want to be mentioned with this advisory ?

@nova-coresec: Can you check if other vulnerable image conversions exist (beside the snapshot upload) ?

Assuming this affects all cinder backend, here is impact description draft #1:

Title: Host file disclosure through qcow2 backing file
Reporter: Bastian Blank
Products: Cinder and Nova
Affects: up to 2014.1.3 and 2014.2 versions through 2014.2.2

Description:
Bastian Blank reported a vulnerability in Cinder and Nova. By creating a qcow2 image with an arbitrary backing file, an authenticated user may mislead Cinder upload-to-image action, resulting in disclosure of any file from the Cinder server. A similar vulnerability in Nova can also be used by an authenticated user to trick Nova during a snapshot upload, resulting in disclosure of any file for which the Nova process user has access to. All Cinder setups and all Nova setups with force_raw_image (which is set by default) are affected.

impact desc lgtm

Thanks!

@nova-coresec please confirm that description before we request a CVE with it.

@ttx nova is certainly vulnerable in the snapshot case. From what I see nova may *also* be vulnerable (at least in theory) when creating instances.

As Tony said, Nova is vulnerable in the snapshot case. I'm not an expert in the libvirt driver and the complexity of the image handling code makes it hard to say with certainty that the vulnerability exists during instance creation, but it does appear to me that it uses the unprotected 'qemu-img create' with an LVM image type. I can't say at this point whether or not there are safeguards in place that means that is ok.

On Thu, Feb 26, 2015 at 04:29:50PM -0000, Tristan Cacqueray wrote:
> @waldi: I couldn't find your affiliation, any company you want to be
> mentioned with this advisory ?

This would be "credativ".

> Description:
> Bastian Blank reported a vulnerability in Cinder and Nova. By creating a qcow2 image with an arbitrary backing file, an authenticated user may mislead Cinder upload-to-image action, resulting in disclosure of any file from the Cinder server. A similar vulnerability in Nova can also be used by an authenticated user to trick Nova during a snapshot upload, resulting in disclosure of any file for which the Nova process user has access to. All Cinder setups and all Nova setups with force_raw_image (which is set by default) are affected.

I find that description not well formulated. For me "creating an image"
implies that I have a new file after that. But in this case there
already exists the volume or instance disk and I have to use them for
the attack.

Also what is "force_raw_image"?

Bastian

--
It would be illogical to kill without reason.
  -- Spock, "Journey to Babel", stardate 3842.4

On Fri, Feb 27, 2015 at 04:52:32AM -0000, Tony Breeds wrote:
> @ttx nova is certainly vulnerable in the snapshot case. From what I see
> nova may *also* be vulnerable (at least in theory) when creating
> instances.

I tried that and was unable to get past the initial check. However this
code does a blacklist test instead of a whitelist tests of features the
image may have.

Okay, it parses the output of "qemu-img info", which looks more like it
is created for human, not machine consumption.

Bastian

--
The heart is not a logical organ.
  -- Dr. Janet Wallace, "The Deadly Years", stardate 3479.4

Hum, upon a secondary review it seems that force_raw_image only prevents the convertion from fetch_image, but not from the image backend. Nevermind that note, I guess every nova setups are affected.

Thanks Bastian, is it more clear like that ?

Title: Host file disclosure through qcow2 backing file
Reporter: Bastian Blank (credativ)
Products: Cinder and Nova
Affects: up to 2014.1.3 and 2014.2 versions through 2014.2.2

Description:
Bastian Blank from credativ reported a vulnerability in Cinder and Nova. By overwriting an image with a malicious qcow2 header, an authenticated user may mislead Cinder upload-to-image action, resulting in disclosure of any file from the Cinder server. A similar vulnerability in Nova can also be used by an authenticated user to trick Nova during a snapshot upload, resulting in disclosure of any file for which the Nova process user has access to. All Cinder and Nova setups are affected.

On Fri, Feb 27, 2015 at 11:09:56PM -0000, Tristan Cacqueray wrote:
> Hum, upon a secondary review it seems that force_raw_image only prevents
> the convertion from fetch_image, but not from the image backend.
> Nevermind that note, I guess every nova setups are affected.

Ah, I missed that this is a Nova-only setting. Every call to
"convert_image" with something that may be a raw-image is affected.

> Thanks Bastian, is it more clear like that ?

This looks better, thanks.

Bastian

--
If some day we are defeated, well, war has its fortunes, good and bad.
  -- Commander Kor, "Errand of Mercy", stardate 3201.7

Impact desc +1

Grant will have a try at patching this.

Thought I'd get the patch conversation started with this.

This change explicitly requires all calls to convert_image to supply an image format. It touches a lot of drivers so may not be desirable. The alternative will need to use qemu-img info for each call to derive the image type.

*Note this only covers the qemu-img convert cases in cinder. I will take a look at required changes in Nova next.

On Mon, Mar 02, 2015 at 11:24:02PM -0000, Grant Murphy wrote:
> This change explicitly requires all calls to convert_image to supply an
> image format. It touches a lot of drivers so may not be desirable.

Well, you mix in correct but independent cleanup changes (renaming of
variables).

> The
> alternative will need to use qemu-img info for each call to derive the
> image type.

And this would fix the problem how? The format guessing qemu-img does
is the reason for problem.

Bastian

--

Thanks for the review Bastain. I'll try to cleanup some of my clean ups.. but I'm not convinced I have the driver code changes right anyway.

Can we get @cinder-coresec and @nova-coresec to come up something decent for this please? I'd like to get this moving along !

From the @nova-coresec POV this bug is important but behind others so while you have some momentum please keep it going Grant!

Since there hasn't been much other movement on this I've updated a new patch that hopefully addresses the variable renaming complaints. As we can't rely on calls to qemu-img info to determine the source format explicit decisions need to be made in the driver code in cases that call image_utils.convert_image(). I couldn't really find a case where I could reliably discover this information from the database.. so I've added a configuration setting for the affected drivers. I can't be certain if the default values I've supplied here are correct or even sensible. So I would again ask cinder devs to please take a look at this and suggest a viable alternative.

Thanks.

Hi Grant, thanks for the patch. I'll test this out today and let you know.

I don't think this patch will work reliably. Coding the expected format in the configuration for images coming from Glance doesn't work because because the images could be a number of different formats -- we need to query Glance for that information.

I've taken a look at a different instance of this same issue in this spec [1], which revolves around similar problems guessing formats of volume/snapshot files. It's a different scenario, but the gist of it is, at any point that you are handling an image or file from the "outside", you have to have a separate identifier indicating what format it is in, which isn't the file itself.

For this bug, Glance should be able to tell us that. But coding expected formats in the configuration isn't viable since it restricts us to only one format, and opens up holes such as someone supplying a raw image with a malicious qcow2 header written into it, which is accepted because the configuration by default says that it is qcow2.

[1] https://review.openstack.org/#/c/103750/

Thanks Grant. I agree with Eric that this patch does solve this particular case, but not all.

Thanks for the review. I was hoping my crummy patch would inspire one of you guys to write something that actually worked. @thingee @eharney Do you think you could take a look at this?

Can we user the same trick Eric used fixing the related CVE, and analyse the apparent structure of any qcow2 file provided to see if it is backed by something it shouldn't be?

On Tue, Mar 31, 2015 at 02:45:58PM -0000, Duncan Thomas wrote:
> Can we user the same trick Eric used fixing the related CVE, and analyse
> the apparent structure of any qcow2 file provided to see if it is backed
> by something it shouldn't be?

Not sure which trick you mean. But the only way to properly fix this is
to actually supply the input format, as only the storage layer knows.
If you resort to guessing, this will get you a nice way to hide
information, including unusable backups.

Bastian

--
The sight of death frightens them [Earthers].
  -- Kras the Klingon, "Friday's Child", stardate 3497.2

Spoke to Eric about the approach. He'll be working on a patch tonight.

This patch handles this scenario by stopping the upload process if the volume data contains a header referencing a backing file.

This is an intermediate solution until we can implement full format tracking of volume data, but it prevents this bug from causing a data leak by failing safe.

Bastian: The problem is, in a few cases, we don't know the storage format for sure. For filesystem based starge, nova can and does change the format during certain operations, without telling cinder. This is something we're working on fixing, and I agree it is a better fix, but it isn't going to be quick or simple.

On Wed, Apr 01, 2015 at 10:00:13AM -0000, Duncan Thomas wrote:
> Bastian: The problem is, in a few cases, we don't know the storage
> format for sure. For filesystem based starge, nova can and does change
> the format during certain operations, without telling cinder.

This are "few cases". So you can fix the known cases, properly document
the cases where you may not know. But I want to have some informations:
- the code location of this stabbing cinder in the back and
- the db location nova itself stores this information (or does it always
  fall back to sniffing, which means all qemu calls needs to be audited
  as well).

> This is
> something we're working on fixing, and I agree it is a better fix, but
> it isn't going to be quick or simple.

So fix the locations that are easy and properly set the type to
undefined if you don't know. I don't really think lvm/iscsi backed
volumes will be changed in this way.

It's been over two months now without any real progress. Now you claim
it can't be fixed, only worked around. So I don't consider waiting any
longer will help and think about publishing this issue after easter.
So, if there have been CVE assigned please document them, otherwise I
will take care of that.

Regards,
Bastian

--
Most legends have their basis in facts.
  -- Kirk, "And The Children Shall Lead", stardate 5029.5

Bastian: the security fix will likely be a workaround -- that doesn't prevent the issue from being fully fixed in a future version (by tracking format appropriately). Agree that this is taking way too long.

@Cinder-coresec: please review proposed patch
@Nova-coresec: please propose patch to cover for the case @ comment #10

We'll get a CVE based on the impact description at comment #20 for you to include in your public disclosure.

CVE have been requested with this description (same as comment #20, including 2014.1.4 in affected versions):

Title: Host file disclosure through qcow2 backing file
Reporter: Bastian Blank (credativ)
Products: Cinder and Nova
Affects: up to 2014.1.4 and 2014.2 versions through 2014.2.2

Description:
Bastian Blank from credativ reported a vulnerability in Cinder and Nova. By overwriting an image with a malicious qcow2 header, an authenticated user may mislead Cinder upload-to-image action, resulting in disclosure of any file from the Cinder server. A similar vulnerability in Nova can also be used by an authenticated user to trick Nova during a snapshot upload, resulting in disclosure of any file for which the Nova process user has access to. All Cinder and Nova setups are affected.

Apologies on the time it took to verify Eric's patch. I have verified this workaround works. It stops all uploads that have a backing file as mentioned in the commit message.

@Nova-coresec: please propose patch to cover for the case @ comment #10

Updating affected version to include recent stable release.

Title: Host file disclosure through qcow2 backing file
Reporter: Bastian Blank (credativ)
Products: Cinder and Nova
Affects: versions through 2014.1.4, and 2014.2 versions through 2014.2.3, and version 2015.1.0

Description:
Bastian Blank from credativ reported a vulnerability in Cinder and Nova. By overwriting an image with a malicious qcow2 header, an authenticated user may mislead Cinder upload-to-image action, resulting in disclosure of any file from the Cinder server. A similar vulnerability in Nova can also be used by an authenticated user to trick Nova during a snapshot upload, resulting in disclosure of any file for which the Nova process user has access to. All Cinder and Nova setups are affected.

Eric's patch in comment #36 looks good to me. Tested that it blocks upload of constructed QCOWs.

I tested patch from comment #36 and it worked fine for me as well.

@Tony, any progress on the Nova patch ?

I didn't make as much progress as I'd hoped today.

I have a work in process patch and I'm working on refining my repro case to make iterating on the patch quicker.

Embargo lifted now that this is reported in public: http://www.openwall.com/lists/oss-security/2015/06/13/1

My understanding from the history is that Cinder's patch is ready and reviewed but the Nova issue is still a problem, is this correct?

Now this issue is disclosed and public, I think we need to raise the urgency of some code mitigation/workaround?

Eric, as the issue is now public - worth pushing your patch to gerrit?

On the nova side, the recent slowness (to my understanding) is that we've had a lot of trouble actually finding an exploit for this. We agree there is a theoretical problem, but we haven't been able to actually replicate a working attack.

So... Nova needs to do some defensive work here, but that effort is not yet complete.

I tried to recreate this myself.. TL;DR I didn't succeed.

I created a cinder (LVM backend) volume, attached it as vdb to a guest and within the guest ran "qemu-img create -f qcow2 -b /etc/passwd /dev/vdb"

Introspecting on the underlying host, it looks like things are progressing:
# qemu-img info /dev/mapper/stack--volumes--lvmdriver--1-volume--6c76f71d--6ccb--4a95--845c--40fd9af1a12d | grep backing
backing file: /etc/passwd

I then created a second guest, and made the root boot from volume the cinder volume from above. Then following running "nova image-create" a reference does appear in Glance with the correct name set, but a null file size.

Attempting to retrieve the image through glance a 0 byte file is returned.

Mikal is correct nova has a theoretical issue but I've been unable to actually exploit it..

I've certainly been able to create a guest with an image that shows the problem BUT as yet I haven't been able to get nova to run convert on it.

A related issue is that nova blindly trusts qemu-img info which is itself susceptible to a similar issue.

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

Fix proposed to branch: stable/kilo
Review: https://review.openstack.org/191786

@cinder-coresec, I submited the proposed patch for review in master and kilo, but the cherry-pick failed for Juno and Icehouse, can you please fix this ?

@nova-coresec, are we confident this does not affect any stable release starting from Icehouse ?

Until Nova is confirmed, here is the impact description of Cinder:

Title: Cinder host file disclosure through qcow2 backing file
Reporter: Bastian Blank (credativ)
Products: Cinder
Affects: versions through 2014.1.4, and 2014.2 versions through 2014.2.3, and version 2015.1.0

Description:
Bastian Blank from credativ reported a vulnerability in Cinder. By overwriting an image with a malicious qcow2 header, an authenticated user may mislead Cinder upload-to-image action, resulting in disclosure of any file from the Cinder server. All Cinder and Nova setups are affected.

Oups, last sentence is "All Cinder setups are affected"

Yep, with that correction from comment #57, Tristan's updated impact Cinder-specific description in comment #56 looks good to me.

Fix proposed to branch: stable/juno
Review: https://review.openstack.org/191865

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/191871

Can we get series tasks for kilo/juno/icehouse ?

Reviewed: https://review.openstack.org/191785
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=b1143ee45323e63b965a3710f9063e65b252c978
Submitter: Jenkins
Branch: master

commit b1143ee45323e63b965a3710f9063e65b252c978
Author: Eric Harney <email address hidden>
Date: Tue Mar 31 19:48:17 2015 -0400

    Disallow backing files when uploading volumes to image

    Volumes with a header referencing a backing file can leak
    file data into the destination image when uploading a
    volume to an image.

    Halt the upload process if the volume data references a
    backing file to prevent this.

    Closes-Bug: #1415087
    Change-Id: Iab9718794e7f7e8444015712cfa08c46848ebf78

Reviewed: https://review.openstack.org/191786
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=9634b76ba5886d6c2f2128d550cb005dabf48213
Submitter: Jenkins
Branch: stable/kilo

commit 9634b76ba5886d6c2f2128d550cb005dabf48213
Author: Eric Harney <email address hidden>
Date: Tue Mar 31 19:48:17 2015 -0400

    Disallow backing files when uploading volumes to image

    Volumes with a header referencing a backing file can leak
    file data into the destination image when uploading a
    volume to an image.

    Halt the upload process if the volume data references a
    backing file to prevent this.

    Closes-Bug: #1415087
    Change-Id: Iab9718794e7f7e8444015712cfa08c46848ebf78
    (cherry picked from commit b1143ee45323e63b965a3710f9063e65b252c978)

Reviewed: https://review.openstack.org/191865
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=d31c937c566005dedf41a60c6b5bd5e7b26f221b
Submitter: Jenkins
Branch: stable/juno

commit d31c937c566005dedf41a60c6b5bd5e7b26f221b
Author: Eric Harney <email address hidden>
Date: Tue Mar 31 19:48:17 2015 -0400

    Disallow backing files when uploading volumes to image

    Volumes with a header referencing a backing file can leak
    file data into the destination image when uploading a
    volume to an image.

    Halt the upload process if the volume data references a
    backing file to prevent this.

    Closes-Bug: #1415087
    Change-Id: Iab9718794e7f7e8444015712cfa08c46848ebf78
    (cherry picked from commit 9634b76ba5886d6c2f2128d550cb005dabf48213)
    Conflicts:
        cinder/tests/test_image_utils.py (backport to old tests)

Reviewed: https://review.openstack.org/191871
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=bc0549e08b010edb863d409d80114aa78d317a61
Submitter: Jenkins
Branch: stable/icehouse

commit bc0549e08b010edb863d409d80114aa78d317a61
Author: Eric Harney <email address hidden>
Date: Tue Mar 31 19:48:17 2015 -0400

    Disallow backing files when uploading volumes to image

    Volumes with a header referencing a backing file can leak
    file data into the destination image when uploading a
    volume to an image.

    Halt the upload process if the volume data references a
    backing file to prevent this.

    Closes-Bug: #1415087
    Change-Id: Iab9718794e7f7e8444015712cfa08c46848ebf78
    (cherry picked from commit 9634b76ba5886d6c2f2128d550cb005dabf48213)
    Conflicts:
        cinder/tests/test_image_utils.py (backport to old tests)
    (cherry picked from commit d31c937c566005dedf41a60c6b5bd5e7b26f221b)
    Conflicts:
        cinder/tests/test_image_utils.py

The OSSA task is now completed for Cinder. Does someone still trying to reproduce the bug on Nova or should we close the OSSA task ?

There was an error in CVE assignment. The Cinder issue is CVE-2015-1851. Nova has been assigned CVE-2015-1850.

Tony, in the end, were you able to reproduce this in Nova ?

@Tony & nova-coresec please respond to Tristan's last question.

The OSSA tasks is now closed. If Nova turns out to be affected, a new OSSA will be required anyway.

What are the plans for nova fix? Is nova affected or not affected?
The CVE record is still opened in MITRE: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2015-1850

A CVE was requested for a suspected vulnerability in Nova, but was never confirmed exploitable. Unless someone comes forward soon with a reproducible case there, I recommend we request a reject of CVE 2015-1850 for completeness.

Anna, since nobody have been able to reproduce this bug in Nova, it seems like there will be no fix for it.

Someone from Nova team needs to mark this bug as Invalid.

Given nova.virt.libvirt.utils.create_cow_image which is calling qemu-img create with a backing file but not specifying the backing file format, I'd consider nova affected by this issue:

https://github.com/openstack/nova/blob/master/nova/virt/libvirt/utils.py#L75

Thanks, Matt. Even if we couldn't find a way to exploit this, that doesn't mean there isn't one or that a fix isn't needed. Nova should not be doing what is described in comment 74.

I agree this is at least a security hardening opportunity in Nova, and would be great to patch if someone is interested in writing that patch. However the decision to issue a security hinges on having a clearer explanation of _how_ this makes users of Nova vulnerable, and would still need the aforementioned patch (and would need to be the sort we could safely provide to users of supported stable release branches without introducing new configuration or backward-incompatible behavior changes).

Sorry, I meant "the decision to issue a security *advisory*" above.

If we're just adding the hardening and tests then we should be able to do that in a backport compatible way.

Are there any updates on nova fix?

I'm going to mark the nova part of this as incomplete since no one has said they know how to demonstrate a vulnerability in nova for this. The CVE for nova is marked as reserved:

https://bugs.launchpad.net/bugs/cve/2015-1850

And the OSSA was released for the cinder CVE:

https://security.openstack.org/ossa/OSSA-2015-011.html

Furthermore, distros like Red Hat have already been posting that this is something to be aware of but does not have a fix and is considered low severity:

https://access.redhat.com/security/cve/CVE-2015-1850

Same with Canonical:

http://people.canonical.com/~ubuntu-security/cve/2015/CVE-2015-1850.html

OSSA-2015-011 let the cat out of the bag on CVE-2015-1850, and then so have RedHat and Canonical with their statements as well, so while it's technically still reserved in the mitre system, that's really an inaccuracy at this point. The information is out there.

Since CVE-2015-1850 is marked as incomplete, will the nova bug be fixed(or hardened) with a new advisory?

To reiterate Matt's comment #80, "no one has said they know how to demonstrate a vulnerability in nova for this." If someone steps up to fix the underlying bug, I doubt there will be any objection, but the OpenStack Vulnerability Management Team won't issue any official advisory statement without documented impact.

Tony, we talked at the summit. I believe you said you were working on nova changes to address this, and would throw up a WIP patch set folks could track. I haven't seen that patch set linked here. Can you give a status update?

As there has been no demonstration of an actual Nova exposure, this is closed out for Nova. If someone finds a real exposure there, please reopen.