[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing |
Date: |
Wed, 05 Nov 2014 09:39:03 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Kevin Wolf <address@hidden> writes:
> Am 04.11.2014 um 16:25 hat Stefan Hajnoczi geschrieben:
>> On Tue, Nov 04, 2014 at 11:11:33AM +0100, Kevin Wolf wrote:
>> > Am 03.11.2014 um 16:05 hat Stefan Hajnoczi geschrieben:
>> > > The argument that there might not be a traditional filename doesn't make
>> > > sense to me. When there is no filename the command-line is already
>> > > sufficiently complex and usage is fancy enough that probing adds no
>> > > convenience, the user can just specify the format.
>> >
>> > -hda nbd://localhost
>> > -drive file=nbd://localhost,format=raw
>> >
>> > Almost double the length, and I don't see anything fancy in the first
>> > line.
>> >
>> > > Anyway, does this sound reasonable:
>> > >
>> > > In QEMU 3.0, require the format= option for -drive. Keep probing the
>> > > way it is for non-drive options because they are used for convenience by
>> > > local users.
>> >
>> > And being hacked while using -hda is better in which way?
>>
>> Markus is proposing that we look at the filename extension. In that
>> case QEMU cannot be tricked by the contents of a raw image.
>>
>> That makes -hda perfectly safe although there are cases where QEMU
>> doesn't know what to do and requires format=.
>
> Wait, by "keep probing the way it is" you mean implementing one of the
> other proposals? So you're only suggesting being stricter on -drive as
> an additional measure?
>
>> I do worry that changing QEMU's probing behavior drastically can lead to
>> consistencies where libvirt does its own probing :(. Haven't thought
>> through the bug scenarios but that could be a security problem in
>> itself.
>
> 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.
I don't understand VIR_STORAGE_FILE_AUTO_SAFE offhand.
VIR_STORAGE_FILE_AUTO means probing. Its use appears to be deprecated.
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.
Another reason for having its own probing might be providing a secure
replacement for QEMU's insecure probing.
> 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.
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing, (continued)
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing, Markus Armbruster, 2014/11/03
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing, Max Reitz, 2014/11/03
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing, Kevin Wolf, 2014/11/03
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing, Stefan Hajnoczi, 2014/11/03
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing, Max Reitz, 2014/11/03
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing, Markus Armbruster, 2014/11/04
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing, Kevin Wolf, 2014/11/04
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing, Stefan Hajnoczi, 2014/11/04
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing, Kevin Wolf, 2014/11/04
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH RFC 2/2] block: Warn on insecure format probing, Eric Blake, 2014/11/05