7:34 a.m.
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(a)suse.com>, Tel. +49 (0)911 74053 2107
SUSE Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer
8:46 a.m.
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(a)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.
9:34 a.m.
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(a)suse.com>, Tel. +49 (0)911 74053 2107
SUSE Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer
9:49 a.m.
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 :|
8:47 a.m.
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(a)suse.com>, Tel. +49 (0)911 74053 2107
SUSE Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer
10:05 a.m.
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.
8:34 a.m.
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 :|
8:47 a.m.
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(a)suse.com>, Tel. +49 (0)911 74053 2107
SUSE Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer
9:03 a.m.
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 :|
9:14 a.m.
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.
8:48 a.m.
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(a)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?
1783
days inactive
1784
days old
10 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
Martin Wilck -
Peter Krempa