Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3049 bytes --]

On 11/05/2014 09:39 AM, Markus Armbruster wrote:

>> Hm... In which cases does libvirt probe the image format? And is it even
>> consistent with qemu today?
> 
> I had a quick look at the source.  Eric, please correct
> misunderstandings.
> 
> Enumation type virStorageFileProbeFormat enumerates supported formats.
> It has pseudo-formats VIR_STORAGE_FILE_AUTO, VIR_STORAGE_FILE_AUTO_SAFE.

VIR_STORAGE_FILE_AUTO is the format that libvirt assigns an image where
the libvirt XML was not explicit.  VIR_STORAGE_FILE_AUTO_SAFE is what
the image gets reassigned to for QED (as the only format where the
backing format is mandatory as part of the backing chain), which
basically says: trust the backing chain if the XML omitted a type,
instead of doing the normal rules of auto.

> 
> I don't understand VIR_STORAGE_FILE_AUTO_SAFE offhand.
> 
> VIR_STORAGE_FILE_AUTO means probing.  Its use appears to be deprecated.

Close.  It actually means: "the user did not specify a format in their
XML, so it is now up to a configuration knob whether we will probe or
whether we will forcefully error out and tell the user to fix their
XML".  The configuration knob (allow_disk_format_probing in
/etc/libvirt/qemu.conf) defaults to 0 by default (error out and tell the
user to fix their XML) but can be overridden to 1 by someone that knows
what they are doing (probes are allowed at the user's own risk).

> Actual probing happens in virStorageFileProbeFormatFromBuf():
> 
>     For all formats:
>         if magic and version match, pick this format
> 
>     If some magic matched, but not the version: warn
> 
>     For all formats:
>         if file name extension matches, pick this format
> 
>     Pick raw.
> 
> The formats' magic, version and extension are defined in fileTypeInfo[].
> 
> If I remember correctly, libvirt has its own probing because running an
> external program just to probe is too slow.

Correct.  And while the libvirt probing was originally modeled after
qemu, the two approaches have probably diverged a bit over time.  An
obvious difference: qemu only probes 512? bytes (maybe 4096?), but
libvirt probes deep enough to determine the .iso file format (a 5-byte
magic number at decimal offset 32769).  See VIR_STORAGE_MAX_HEADER
0x8200 in src/util/virstoragefile.h.

> 
> Another reason for having its own probing might be providing a secure
> replacement for QEMU's insecure probing.

While that may be a possible outcome, I'm not sure it was an intentional
design.

> 
>> If you can get libvirt to explicitly pass the wrong format=... option
>> because it did its own probing, we have a problem no matter what we
>> change in qemu.
> 
> Yes, but that would be a libvirt problem.  No excuse for us to ignore
> our own problems.

Correct - for the purposes of this thread, we need only figure out how
to make qemu closer to being secure by default.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]