image_utils: xen server image handling issue

[Brian Rosmaita]

This may be something that can be worked on in the open as a normal bug/hardening opportunity, but I want to get some VMT feedback before making it public.

What complicates this a bit is that the issue occurs in some old code (introduced in Havana) that implements Xenserver image support [0]. Nova removed xenserver support in victoria [1], so there's no reason to have it in cinder any more ... though you never know what users are doing, so we'll have to discuss its removal in public, which will bring attention to the issue I'm about to describe. The primary thing that concerns me is that once you see this in the code, you realize that there is no way for an operator to turn it off ... all you need to do are set some specific image properties on an image payload in glance and you can force the behavior.

Here's the issue. Xenserver used the VHD disk format in an OVF container fomat, which is basically a VHD chain inside a tgz file, where the VHDs followed a specific naming convention. These would normally be created by the nova image-create server action. The cinder use is that if a user requests to create a volume from such an image, cinder has to unpack and then coalesce the VHDs into a single image that's written to the volume. So the potential vulnerabilty is that a bad actor uploads a gzip bomb or some kind of nasty gzipped tarfile to glance, sets the image properties, and then requests cinder to create a volume from it.

The cinder situation is:
- this happens in an operator-configurable directory [2], so the consumed space can be isolated on a system running the cinder-volume service
- there's no way to turn off this behavior; cinder just looks to see if the image metadata has disk_format==vhd and container_format==ovf, and if so, unpacks and coalesces it [3,4]
- before this would be called, there is a check to make sure the glance image metadata 'size' would fit in the available space [5] (but this is the number of bytes the thing takes up in glance, not its uncompressed size)
- the un-tgz is done by calling 'tar' using oslo processutils (eventually) with no processlimits [6] (so the good news is that as long as the server's tar has been kept up to date, it should be resistant to the known tar attacks)

So someone could slip a gzip bomb into glance, set the requisite metadata on the image, and then cinder would happily 'tar -zxf' it. But what I think would happen in that case is that you'd get a failure in the gunzip phase, and the process would error out without leaving anything behind in the tmp space.

tar itself isn't good about cleaning up when it runs out of space, though. I think it just reports an error and leaves whatever it untarred in the filesystem in whatever state it was when space was exhausted. I don't know enough about tar exploits to know whether it's possible to produce a tgz that doesn't run out of space in the gunzip phase but then does something bad in the untar phase that could fill up the isolated tmp space and make it unavailable for other processes.

In summary, I'm not sure how big a deal this issue is and whether it can be simply be worked on in the open as a normal bug/hardening opportunity, or whether it should be handled first as a private security issue.

[0] https://opendev.org/openstack/cinder/commit/0e5cb31af
[1] https://opendev.org/openstack/nova/commit/adb28f5
[2] https://opendev.org/openstack/cinder/src/commit/d6fd72b47d2b19794afb0865f4c9cdabc299788b/cinder/image/image_utils.py#L62-L65
[3] https://opendev.org/openstack/cinder/src/commit/d6fd72b47d2b19794afb0865f4c9cdabc299788b/cinder/image/image_utils.py#L927-L928
[4] https://opendev.org/openstack/cinder/src/commit/d6fd72b47d2b19794afb0865f4c9cdabc299788b/cinder/image/image_utils.py#L1099-L1103
[5] https://opendev.org/openstack/cinder/src/commit/d6fd72b47d2b19794afb0865f4c9cdabc299788b/cinder/volume/flows/manager/create_volume.py#L913-L931
[6] https://opendev.org/openstack/cinder/src/commit/d6fd72b47d2b19794afb0865f4c9cdabc299788b/cinder/image/image_utils.py#L1110-L1111

[Brian Rosmaita (brian-rosmaita)]

Adding Nick Tait, who I've already discussed this with.

[Jeremy Stanley (fungi)]

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.

At first blush, I'm leaning toward considering this impractical to exploit for the reasons you cite. If there's no disagreement from Cinder security reviewers, I'm inclined to treat it as class C1 per our taxonomy (that is, switch to public and fix but no need for an advisory, even though someone might still assign a CVE for tracking it out of thoroughness): https://security.openstack.org/vmt-process.html#report-taxonomy

[Brian Rosmaita (brian-rosmaita)]

As far as I can tell, nobody thinks this needs to be kept private (and there's been plenty of time for feedback). Let's go ahead and make it public and it can be worked on like a normal bug.

[Jeremy Stanley (fungi)]

Based on Brian's comment, I'm treating this as a class C1 report (previously suggested), basically a security hardening opportunity due to being fairly impractical to exploit.