Re: [PATCH 3/3] qemu-img: Deprecate use of -b without -F

On Sat, Feb 22, 2020 at 05:23:41 -0600, Eric Blake wrote: I disagree. If you are creating an overlay on top of an existing image it's not safe to probe the format any more generally. (obviously you'd have to trust the image and express the trust somehow) The image may have been used in a VM as raw and that means that the VM might have recorded a valid qcow2 header into it. Creating the overlay with probing would legitimize this. Let's assume we have a malicious image written by the guest but we simulate it by: $ qemu-img create -f qcow2 -F raw -b /etc/passwd /tmp/malicious Formatting '/tmp/malicious', fmt=qcow2 size=2560 backing_file=/etc/passwd backing_fmt=raw cluster_size=65536 lazy_refcounts=off refcount_bits=16 Now we want to create an overlay. a) without this patchset: $ qemu-img create -f qcow2 -b /tmp/malicious /tmp/pre-patch.qcow2 Formatting '/tmp/pre-patch.qcow2', fmt=qcow2 size=2560 backing_file=/tmp/malicious cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ qemu-img info /tmp/pre-patch.qcow2 image: /tmp/pre-patch.qcow2 file format: qcow2 virtual size: 2.5 KiB (2560 bytes) disk size: 196 KiB cluster_size: 65536 backing file: /tmp/malicious Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false There's no 'backing file format'. When used by libvirt we'd not allow the VM to touch the backing file of /tmp/malicious in pre-blockdev era and in libvirt-6.0 we'd report an error right away. b) Now with this patchset: $ ./qemu-img create -f qcow2 -b /tmp/malicious /tmp/post-patch.qcow2 qemu-img: warning: Deprecated use of non-raw backing file without explicit backing format, using detected format of qcow2 Formatting '/tmp/post-patch.qcow2', fmt=qcow2 size=2560 backing_file=/tmp/malicious backing_fmt=qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ qemu-img info /tmp/post-patch.qcow2 image: /tmp/post-patch.qcow2 file format: qcow2 virtual size: 2.5 KiB (2560 bytes) disk size: 196 KiB cluster_size: 65536 backing file: /tmp/malicious backing file format: qcow2 Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false You now get a warning, but "backing file format" is now recorded in the overlay. Now this is WAY worse than it was before. The overlay now legitimizes the format recorded by the malicious guest which circumvents libvirt's protections. The warning is very easy to miss, and if you run it in scripts you might never get to see it. We can't allow that. While it's safe I don't think it should be encouraged. IMO -F should be made mandatory with -b. Not necessarily. The backing store string can be e.g.: $ ./qemu-img create -f qcow1 -b 'json:{"driver":"file","filename":"/tmp/malicious"}' /tmp/json.qcow2 Formatting '/tmp/json.qcow1', fmt=qcow2 size=197120 backing_file=json:{"driver":"file",,"filename":"/tmp/malicious"} cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ qemu-img info /tmp/json.qcow1 image: /tmp/json.qcow1 file format: qcow1 virtual size: 191 KiB (197120 bytes) disk size: 195 KiB cluster_size: 65535 backing file: json:{"driver":"file","filename":"/tmp/malicious"} Format specific information: compat: 0.1 lazy refcounts: false refcount bits: 15 corrupt: false Now this has the old semantics but we didn't even get the warning. But at least the backing file format is not written into the overlay. We must never write the detected format into the overlay. Not even when we print a warning. This can legitimize a malicious file if the user mises the warning.