[libvirt] backing file chains, -blockdev, and storage file autodetection
The recent change in libvirt to pass storage arguments to qemu via "-blockdev", explicity passing backing file chain information rather than relying on qemu to figure it out, has bitten me quite painfully. I've had the habit for years to use qcow2 with backing chains without specifying the backing file format explicitly, as information like the following was enough both for me and for qemu to figure out that the backing file was in qcow2 format: # qemu-img info sles15-gm-cc.qcow2 image: sles15-gm-cc.qcow2 file format: qcow2 virtual size: 20 GiB (21474836480 bytes) disk size: 407 MiB cluster_size: 65536 backing file: /mnt/vms/sles15-gm-base.qcow2 But if libvirt encounters a qemu-img file like this, it passes the backing file to qemu with "driver";"raw": -blockdev '{"driver":"file","filename":"/mnt/vms/sles15-gm-base.qcow2","node-name":"libvirt-4-storage","auto-read-only":true,"discard": "unmap"}' \ -blockdev '{"node-name":"libvirt-4-format","read-only":true,"driver":"raw","file":"libvirt-4-storage"}' \ The effect was that some of my VMs would refuse to start with a "no bootable disk" error message from the VM's BIOS. Others did boot (I believe this was because the boot sector had been modified in the top image in the backing file chain), and I'm quite grateful I did not attempt to boot them fully, because god knows what might have happened if the OS had later encountered garbage data from the backing file at some random point. It took me half a day to figure out that this effect had been caused by the recent libvirt update. I'm aware that documentation about this has been added recently (https://libvirt.org/kbase/backing_chains.html (*)). Also I believe that current libvirt master (unlike 5.10.0) would refuse to start such images in the first place ( https://bugzilla.redhat.com/show_bug.cgi?id=1588373), perhaps providing users with a better clue than before what was going wrong. Meanwhile I've fixed my VM images by adding the backing file format tag, as suggested in the documentation. However I still think that this was quite a disruptive change and highly unexpected for users. IMO the default behavior shouldn't have been switched like this without appropriate warnings. The rationale given for not autodetecting the file format is "a malicious guest could rewrite the header of the disk leading to access of host files". I suppose a guest would need to manipulate a raw image to look like qcow2, qed or similar for this to happen (and set the backing file to "/etc/shadow", maybe?). Still the malicious guest would need to find a way to manipulate the data *on the backing store*, because the format of the topmost image is explicit anyway. Modifying the backing store could be difficult for the guest, because it's normally read-only and changes go only to the top layer. Or am I missing something? The opposite (manipulating qcow2 to look like raw) shouldn't be possible, IMO. While I can't deny that an attack like this might be feasible, I am still wondering why this hasn't been an issue in past years (with qemu auto-detecting the backing file format). More importantly, perhaps the disruption caused by this change could be mitigated by allowing autodetection in certain cases (e.g. if the file name of the backing file indicates it's qcow2, as in the example above), or by providing a configuration option to enable it in environments (like mine, developer test environment) where evil guests are very unlikely. Best regards, Martin (*) This page is quite hard to find, googling for "libvirt backing chain" does not pick it up prominently just yet. Actually I only found this information by running "git grep" on the libvirt git repo. -- Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer
On Wed, Jan 08, 2020 at 13:34:14 +0000, Martin Wilck wrote:
The recent change in libvirt to pass storage arguments to qemu via "-blockdev", explicity passing backing file chain information rather than relying on qemu to figure it out, has bitten me quite painfully.
I'm sorry for causing this. I'm also sorry for going to sound too dismissive later on.
I've had the habit for years to use qcow2 with backing chains without specifying the backing file format explicitly, as information like the following was enough both for me and for qemu to figure out that the backing file was in qcow2 format:
# qemu-img info sles15-gm-cc.qcow2 image: sles15-gm-cc.qcow2 file format: qcow2 virtual size: 20 GiB (21474836480 bytes) disk size: 407 MiB cluster_size: 65536 backing file: /mnt/vms/sles15-gm-base.qcow2
But if libvirt encounters a qemu-img file like this, it passes the backing file to qemu with "driver";"raw":
-blockdev '{"driver":"file","filename":"/mnt/vms/sles15-gm-base.qcow2","node-name":"libvirt-4-storage","auto-read-only":true,"discard": "unmap"}' \ -blockdev '{"node-name":"libvirt-4-format","read-only":true,"driver":"raw","file":"libvirt-4-storage"}' \
The effect was that some of my VMs would refuse to start with a "no bootable disk" error message from the VM's BIOS. Others did boot (I believe this was because the boot sector had been modified in the top image in the backing file chain), and I'm quite grateful I did not attempt to boot them fully, because god knows what might have happened if the OS had later encountered garbage data from the backing file at some random point.
It took me half a day to figure out that this effect had been caused by the recent libvirt update.
I'm aware that documentation about this has been added recently (https://libvirt.org/kbase/backing_chains.html (*)). Also I believe that current libvirt master (unlike 5.10.0) would refuse to start such images in the first place ( https://bugzilla.redhat.com/show_bug.cgi?id=1588373), perhaps providing users with a better clue than before what was going wrong.
Meanwhile I've fixed my VM images by adding the backing file format tag, as suggested in the documentation. However I still think that this was quite a disruptive change and highly unexpected for users. IMO the default behavior shouldn't have been switched like this without appropriate warnings.
This is a good sounding idea but mostly pointless when attempting to pull off. If we'd add notes in the documentation, most people wouldn't read them until stuff breaks. Similarly for putting warnings into the code. They end up in the log file and not many people even bother to read them. The only recourse would be: "we told you so a year ago in this document you didn't read". The end result would be the same.
The rationale given for not autodetecting the file format is "a malicious guest could rewrite the header of the disk leading to access of host files". I suppose a guest would need to manipulate a raw image to look like qcow2, qed or similar for this to happen (and set the backing file to "/etc/shadow", maybe?). Still the malicious guest would need to find a way to manipulate the data *on the backing store*, because the format of the topmost image is explicit anyway. Modifying the backing store could be difficult for the guest, because it's normally read-only and changes go only to the top layer. Or am I
You are right in cases when the backing image is always a backing image. That can't be guaranteed though because the VM may use the image as the only image in raw mode. Then when an overlay is created the image may already be compromised. Obviously this requires the permission to do the overlay, which can arguably not be granted. In general though this scenario is possible. In libvirt we already always record the image format when creating the overlay for many years now.
missing something? The opposite (manipulating qcow2 to look like raw) shouldn't be possible, IMO.
No this is impossible to do without compromising qemu or misconfiguring the VM, so you are right.
While I can't deny that an attack like this might be feasible, I am still wondering why this hasn't been an issue in past years (with qemu auto-detecting the backing file format).
Well, for distros using selinux this attack is mitigated by selinux usage. That was also the reason why "assuming raw" was good enough. It was also 'good enough' because there wasn't any other option. It additionally created a whole lot of users which were already bitten by this many years ago and now use the format correctly since we'd not allow access otherwise when selinux is used.
More importantly, perhaps the disruption caused by this change could be mitigated by allowing autodetection in certain cases (e.g. if the file name of the backing file indicates it's qcow2, as in the example above), or by providing a configuration option to enable it in environments (like mine, developer test environment) where evil guests are very unlikely.
The option to allow autodetection was removed some time ago with the rationale that it's not worth even allowing reverting to an insecure configuration. While this will cause a disruption during upgrade, users will need to modify the custom scripts. Unfortunately that would have to hapen in one way or another anyways. Ideally users should take advantage libvirt APIs to create overlays since they always record the image format. Also I'm not really sold on the idea of matching qcow2 in the image name. It's just another bit of metadata, you can record it properly in the overlay. (*) Additionally there's a lot of wrong documentation on the internet which doesn't call for specifying the backing format. Thus there's always a group of things that will break. Again, I'm sorry for sounding too dismissive, but in one way or another I don't think this was avoidable. It could be partially mitigated if I remembered to force the error when backing store format is missing in 5.10.
Best regards, Martin
(*) This page is quite hard to find, googling for "libvirt backing chain" does not pick it up prominently just yet. Actually I only found
I added it just recently. Possibly the search engines didn't pick it up.
this information by running "git grep" on the libvirt git repo.
-- Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer
* I will need to think about this for a bit first.
Hi Peter, On Wed, 2020-01-08 at 15:46 +0100, Peter Krempa wrote:
On Wed, Jan 08, 2020 at 13:34:14 +0000, Martin Wilck wrote:
The recent change in libvirt to pass storage arguments to qemu via "-blockdev", explicity passing backing file chain information rather than relying on qemu to figure it out, has bitten me quite painfully.
I'm sorry for causing this. I'm also sorry for going to sound too dismissive later on.
Thanks for your reply anyway.
Meanwhile I've fixed my VM images by adding the backing file format tag, as suggested in the documentation. However I still think that this was quite a disruptive change and highly unexpected for users. IMO the default behavior shouldn't have been switched like this without appropriate warnings.
This is a good sounding idea but mostly pointless when attempting to pull off.
If we'd add notes in the documentation, most people wouldn't read them until stuff breaks. Similarly for putting warnings into the code. They end up in the log file and not many people even bother to read them.
Well, as soon as things break, some people start reading the log files. The release notes problem is familiar of course. I guess we all have it, both sides (neither do people read my release notes, nor do I read theirs). Part of the reason I posted this was hope that it will help others figure out the issue more quickly in the future.
The rationale given for not autodetecting the file format is "a malicious guest could rewrite the header of the disk leading to access of host files". I suppose a guest would need to manipulate a raw image to look like qcow2, qed or similar for this to happen (and set the backing file to "/etc/shadow", maybe?). Still the malicious guest would need to find a way to manipulate the data *on the backing store*, because the format of the topmost image is explicit anyway. Modifying the backing store could be difficult for the guest, because it's normally read-only and changes go only to the top layer. Or am I
You are right in cases when the backing image is always a backing image.
That can't be guaranteed though because the VM may use the image as the only image in raw mode. Then when an overlay is created the image may already be compromised. Obviously this requires the permission to do the overlay, which can arguably not be granted. In general though this scenario is possible.
OK, I didn't think of that. My personal use case is to create af fresh install and use that as read-only base image for various VMs, never using it directly any more. I think this attack would be possible if autodetection was very basic (e.g. just looked at the file magic). Trying to fool a more sophisticated format autodetection without shooting itself into the foot (by destroying its own file system) would be quite hard for a guest, I believe. But I lack knowledge about the qcow2 format to really judge that.
In libvirt we already always record the image format when creating the overlay for many years now.
Unfortunately almost all tutorials about backing chains on the web just teach qemu-img usage. In my case I guess it's just an old habit.
While I can't deny that an attack like this might be feasible, I am
still wondering why this hasn't been an issue in past years (with qemu auto-detecting the backing file format).
Well, for distros using selinux this attack is mitigated by selinux usage. That was also the reason why "assuming raw" was good enough.
Not sure I'm getting that. Assuming "raw" wrongly could lead to massive data corruption in the guest, unless I'm mistaken. I wonder how SELinux would prevent that...
The option to allow autodetection was removed some time ago with the rationale that it's not worth even allowing reverting to an insecure configuration.
While this will cause a disruption during upgrade, users will need to modify the custom scripts. Unfortunately that would have to hapen in one way or another anyways.
Hm, this doesn't make me happy but I guess it's how it is.
Also I'm not really sold on the idea of matching qcow2 in the image name. It's just another bit of metadata, you can record it properly in the overlay. (*)
* I will need to think about this for a bit first.
Thanks. Martin -- Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer
On Wed, Jan 08, 2020 at 03:34:02PM +0000, Martin Wilck wrote:
Hi Peter,
On Wed, 2020-01-08 at 15:46 +0100, Peter Krempa wrote:
On Wed, Jan 08, 2020 at 13:34:14 +0000, Martin Wilck wrote:
The recent change in libvirt to pass storage arguments to qemu via "-blockdev", explicity passing backing file chain information rather than relying on qemu to figure it out, has bitten me quite painfully.
snip
In libvirt we already always record the image format when creating the overlay for many years now.
Unfortunately almost all tutorials about backing chains on the web just teach qemu-img usage. In my case I guess it's just an old habit.
I wonder if there's any scope for improving qemu-img here so that it can 'do the right thing', or at least issue a warning if the suer doesn't set a backing format. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, 2020-01-08 at 15:49 +0000, Daniel P. Berrangé wrote:
I wonder if there's any scope for improving qemu-img here so that it can 'do the right thing', or at least issue a warning if the suer doesn't set a backing format.
Yes, someone should send a patch that would make qemu-img print a big fat warning when a backing file was specified without "-F". Regards, Martin -- Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer
On Wed, Jan 08, 2020 at 15:34:02 +0000, Martin Wilck wrote:
Hi Peter,
On Wed, 2020-01-08 at 15:46 +0100, Peter Krempa wrote:
On Wed, Jan 08, 2020 at 13:34:14 +0000, Martin Wilck wrote:
[...]
You are right in cases when the backing image is always a backing image.
That can't be guaranteed though because the VM may use the image as the only image in raw mode. Then when an overlay is created the image may already be compromised. Obviously this requires the permission to do the overlay, which can arguably not be granted. In general though this scenario is possible.
OK, I didn't think of that. My personal use case is to create af fresh install and use that as read-only base image for various VMs, never using it directly any more.
Well yeah. The thing is that if you want to allow format probing you must declare somehow that you trust the image itself. The simplest way is just to declare the format in the first place to avoid any out-of-band metadata. In fact, with the new libvirt you can override the format of the backing image by directly specifying it in the XML which is basically out-of-band metadata storage.
I think this attack would be possible if autodetection was very basic (e.g. just looked at the file magic). Trying to fool a more sophisticated format autodetection without shooting itself into the foot (by destroying its own file system) would be quite hard for a guest, I believe. But I lack knowledge about the qcow2 format to really judge that.
Well, in the general scenario, you can e.g. assume that the VM has another disk where the malicious guest could do shenanigans without breaking itself.
In libvirt we already always record the image format when creating the overlay for many years now.
Unfortunately almost all tutorials about backing chains on the web just teach qemu-img usage. In my case I guess it's just an old habit.
While I can't deny that an attack like this might be feasible, I am
still wondering why this hasn't been an issue in past years (with qemu auto-detecting the backing file format).
Well, for distros using selinux this attack is mitigated by selinux usage. That was also the reason why "assuming raw" was good enough.
Not sure I'm getting that. Assuming "raw" wrongly could lead to massive data corruption in the guest, unless I'm mistaken. I wonder how SELinux would prevent that...
The failure with selinux is a bit different. The backing file is not labelled and thus can't be opened resulting in an hard failure during qemu startup. I agree completely that the fact that we didn't reject if the backing file didn't have a format right away in 5.10 was a rather big overlook from my side. Especially since there were already bugs and described failure scenarios.
On Wed, Jan 08, 2020 at 03:46:59PM +0100, Peter Krempa wrote:
On Wed, Jan 08, 2020 at 13:34:14 +0000, Martin Wilck wrote:
The recent change in libvirt to pass storage arguments to qemu via "-blockdev", explicity passing backing file chain information rather than relying on qemu to figure it out, has bitten me quite painfully.
I'm sorry for causing this. I'm also sorry for going to sound too dismissive later on.
snip
While I can't deny that an attack like this might be feasible, I am still wondering why this hasn't been an issue in past years (with qemu auto-detecting the backing file format).
Well, for distros using selinux this attack is mitigated by selinux usage. That was also the reason why "assuming raw" was good enough. It was also 'good enough' because there wasn't any other option.
It additionally created a whole lot of users which were already bitten by this many years ago and now use the format correctly since we'd not allow access otherwise when selinux is used.
IIUC there are three scenarios at play here 1. qcow2 file, first level raw backing 2. qcow2 file, first level qcow2 backing, no backing 3. qcow2 file, first level qcow2 backing, with second level backing Historically with the SELinux driver, if no backing_fmt is set, then we blindly assume that scenario (1) is in effect. We cannot tell QEMU that we treated it as scenario (1) though, so QEMU will still probe format and potentially open the first level backing as qcow2 still. IOW, despite our SELinux & QEMU driver assumption for scenario (1), scenario (2) would still suceed in the past. Scenario (3) would always have failed, because SELinux won't have labelled the second level backing. Essentially scenario (2) worked by accident, not design, but IIUC we have not been warning users about this. With new libvirt and -blockdev we still assume the backing_fmt is raw if not set in the SELinux driver, but we now have the ability to tell QEMU about our assumption via -blockdev. As such we've not ensured scenario (2) fails. Users who were silently relying on scenario (2) are now broken and have three options IIUC - Run qemu-img rebase to set the backing_fmt or - Update the guest XML to set the <backingStore> format value or - Update the /etc/libvirt/qemu.conf to set capability_filters to disable "blockdev" Always assuming the format is raw certainly avoids the security danger, but is unhelpful to users who relied on scenario (2). I'm inclined to say we should make scenario (2)/(3) into a hard error. Only allow scenario (1) to run normally. ie, we should probe the disk format for the backing file, and if it is *not* raw, then report an immediate error, with the error message pointing to the kbase page explaining how to fix this. This will help the 99% common case identify the fix more quickly, with no obvious downside that I see. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, 2020-01-09 at 14:34 +0000, Daniel P. Berrangé wrote:
IIUC there are three scenarios at play here
1. qcow2 file, first level raw backing 2. qcow2 file, first level qcow2 backing, no backing 3. qcow2 file, first level qcow2 backing, with second level backing
Historically with the SELinux driver, if no backing_fmt is set, then we blindly assume that scenario (1) is in effect.
We cannot tell QEMU that we treated it as scenario (1) though, so QEMU will still probe format and potentially open the first level backing as qcow2 still.
IOW, despite our SELinux & QEMU driver assumption for scenario (1), scenario (2) would still suceed in the past. Scenario (3) would always have failed, because SELinux won't have labelled the second level backing.
But it only failed with SELinux, right? Not every distro is mandating SELinux usage with libvirt. SUSE distros don't, for example.
Essentially scenario (2) worked by accident, not design, but IIUC we have not been warning users about this.
With new libvirt and -blockdev we still assume the backing_fmt is raw if not set in the SELinux driver, but we now have the ability to tell QEMU about our assumption via -blockdev. As such we've not ensured scenario (2) fails.
Users who were silently relying on scenario (2) are now broken and have three options IIUC
- Run qemu-img rebase to set the backing_fmt
or
- Update the guest XML to set the <backingStore> format value
or
- Update the /etc/libvirt/qemu.conf to set capability_filters to disable "blockdev"
I wasn't aware of this option, thanks. I'd actually looked for a way to revert libvirt's behavior to what it did in previous versions.
Always assuming the format is raw certainly avoids the security danger, but is unhelpful to users who relied on scenario (2).
I'm inclined to say we should make scenario (2)/(3) into a hard error. Only allow scenario (1) to run normally.
ie, we should probe the disk format for the backing file, and if it is *not* raw, then report an immediate error, with the error message pointing to the kbase page explaining how to fix this. This will help the 99% common case identify the fix more quickly, with no obvious downside that I see.
Sounds good. I'd appreciate mention of the capability_filters option on the kbase page. Thanks, Martin -- Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 SUSE Software Solutions Germany GmbH HRB 36809, AG Nürnberg GF: Felix Imendörffer
On Thu, Jan 09, 2020 at 02:47:22PM +0000, Martin Wilck wrote:
On Thu, 2020-01-09 at 14:34 +0000, Daniel P. Berrangé wrote:
IIUC there are three scenarios at play here
1. qcow2 file, first level raw backing 2. qcow2 file, first level qcow2 backing, no backing 3. qcow2 file, first level qcow2 backing, with second level backing
Historically with the SELinux driver, if no backing_fmt is set, then we blindly assume that scenario (1) is in effect.
We cannot tell QEMU that we treated it as scenario (1) though, so QEMU will still probe format and potentially open the first level backing as qcow2 still.
IOW, despite our SELinux & QEMU driver assumption for scenario (1), scenario (2) would still suceed in the past. Scenario (3) would always have failed, because SELinux won't have labelled the second level backing.
But it only failed with SELinux, right? Not every distro is mandating SELinux usage with libvirt. SUSE distros don't, for example.
No, that's just one example. It should also fail with AppArmor, and should also fail with system libvirtd, since libvirt DAC driver chowns files from root -> qemu user ID. Only case where all three scenarios would work are if using the session libvirtd as an unprivileged user, or if using systemd libvirtd with QEMU configured to run as root (a really bad idea).
Essentially scenario (2) worked by accident, not design, but IIUC we have not been warning users about this.
With new libvirt and -blockdev we still assume the backing_fmt is raw if not set in the SELinux driver, but we now have the ability to tell QEMU about our assumption via -blockdev. As such we've not ensured scenario (2) fails.
Users who were silently relying on scenario (2) are now broken and have three options IIUC
- Run qemu-img rebase to set the backing_fmt
or
- Update the guest XML to set the <backingStore> format value
or
- Update the /etc/libvirt/qemu.conf to set capability_filters to disable "blockdev"
I wasn't aware of this option, thanks. I'd actually looked for a way to revert libvirt's behavior to what it did in previous versions.
Note, that we call that a hack. There's no guarantee it will continue working correctly long term & there's no testing of it in general. So at most it should be used as a short term solution until you can use qemu-img rebase or update the XML backingStore for all affected VM.
Always assuming the format is raw certainly avoids the security danger, but is unhelpful to users who relied on scenario (2).
I'm inclined to say we should make scenario (2)/(3) into a hard error. Only allow scenario (1) to run normally.
ie, we should probe the disk format for the backing file, and if it is *not* raw, then report an immediate error, with the error message pointing to the kbase page explaining how to fix this. This will help the 99% common case identify the fix more quickly, with no obvious downside that I see.
Sounds good. I'd appreciate mention of the capability_filters option on the kbase page.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Jan 09, 2020 at 14:47:22 +0000, Martin Wilck wrote:
On Thu, 2020-01-09 at 14:34 +0000, Daniel P. Berrangé wrote:
[...]
- Run qemu-img rebase to set the backing_fmt
or
- Update the guest XML to set the <backingStore> format value
or
- Update the /etc/libvirt/qemu.conf to set capability_filters to disable "blockdev"
I wasn't aware of this option, thanks. I'd actually looked for a way to revert libvirt's behavior to what it did in previous versions.
Please DO NOT use this in production. This is for debugging and workarounds. This may break stuff in the future and is not officially supported. The comment in the file should say so.
Always assuming the format is raw certainly avoids the security danger, but is unhelpful to users who relied on scenario (2).
I'm inclined to say we should make scenario (2)/(3) into a hard error. Only allow scenario (1) to run normally.
ie, we should probe the disk format for the backing file, and if it is *not* raw, then report an immediate error, with the error message pointing to the kbase page explaining how to fix this. This will help the 99% common case identify the fix more quickly, with no obvious downside that I see.
Sounds good. I'd appreciate mention of the capability_filters option on the kbase page.
No. That is not a proper fix unfortunately.
On Thu, Jan 09, 2020 at 14:34:08 +0000, Daniel Berrange wrote:
On Wed, Jan 08, 2020 at 03:46:59PM +0100, Peter Krempa wrote:
On Wed, Jan 08, 2020 at 13:34:14 +0000, Martin Wilck wrote:
The recent change in libvirt to pass storage arguments to qemu via "-blockdev", explicity passing backing file chain information rather than relying on qemu to figure it out, has bitten me quite painfully.
I'm sorry for causing this. I'm also sorry for going to sound too dismissive later on.
snip
While I can't deny that an attack like this might be feasible, I am still wondering why this hasn't been an issue in past years (with qemu auto-detecting the backing file format).
Well, for distros using selinux this attack is mitigated by selinux usage. That was also the reason why "assuming raw" was good enough. It was also 'good enough' because there wasn't any other option.
It additionally created a whole lot of users which were already bitten by this many years ago and now use the format correctly since we'd not allow access otherwise when selinux is used.
IIUC there are three scenarios at play here
1. qcow2 file, first level raw backing 2. qcow2 file, first level qcow2 backing, no backing 3. qcow2 file, first level qcow2 backing, with second level backing
Historically with the SELinux driver, if no backing_fmt is set, then we blindly assume that scenario (1) is in effect.
We cannot tell QEMU that we treated it as scenario (1) though, so QEMU will still probe format and potentially open the first level backing as qcow2 still.
IOW, despite our SELinux & QEMU driver assumption for scenario (1), scenario (2) would still suceed in the past. Scenario (3) would always have failed, because SELinux won't have labelled the second level backing.
Essentially scenario (2) worked by accident, not design, but IIUC we have not been warning users about this.
Exactly.
With new libvirt and -blockdev we still assume the backing_fmt is raw if not set in the SELinux driver, but we now have the ability to tell QEMU about our assumption via -blockdev. As such we've not ensured scenario (2) fails.
Users who were silently relying on scenario (2) are now broken and have three options IIUC
- Run qemu-img rebase to set the backing_fmt
or
- Update the guest XML to set the <backingStore> format value
or
- Update the /etc/libvirt/qemu.conf to set capability_filters to disable "blockdev"
This thing is only for experimenting though. I don't think we should suggest it as a solution as it may cause problems in the future.
Always assuming the format is raw certainly avoids the security danger, but is unhelpful to users who relied on scenario (2).
I'm inclined to say we should make scenario (2)/(3) into a hard
They are a hard error now: commit 3615e8b39badf2a526996a69dc91a92b04cf262e Author: Peter Krempa <pkrempa@redhat.com> Date: Tue Dec 17 17:04:04 2019 +0100 util: storage: Don't treat files with missing backing store format as 'raw' Assuming that the backing image format is raw is wrong when doing image detection: 1) In -drive mode qemu will still probe the image format of the backing image. This means it will try to open a backing file of the image which will fail if a more advanced security model is in use. 2) In blockdev mode the image will be opened as raw actually which is wrong since it might be qcow. Not opening the backing images will also end up in the guest seeing corrupted data. Rather than attempt to solve various corner cases when us assuming the storage file being raw and actually being right forbid startup when the guest image doesn't have the format specified in the metadata.
error. Only allow scenario (1) to run normally.
but that makes also scenario 1 an error.
ie, we should probe the disk format for the backing file, and if it is *not* raw, then report an immediate error, with the
My only grief here is that we'd actually have to run format detection at all. That code was deleted from the startup path long time ago and I don't feel particularly good about re-adding it.
error message pointing to the kbase page explaining how to fix this. This will help the 99% common case identify the fix more quickly, with no obvious downside that I see.
This would be the first error message to contain URI. Or do you have any other suggestion how to word it?
participants (3)
-
Daniel P. Berrangé -
Martin Wilck -
Peter Krempa