Note that this is not finished yet, but allows to test the image
detection patches:

Prepare few images:
qemu-img create -f qcow2 /tmp/base.qcow2 10M
qemu-img create -f qcow2          -b /tmp/base.qcow2 /tmp/overlay1-noformat.qcow2
qemu-img create -f qcow2 -F qcow2 -b /tmp/base.qcow2 /tmp/overlay1-format.qcow2
qemu-img create -f qcow2 -F qcow2 -b /tmp/overlay1-format.qcow2 /tmp/overlay2-format.qcow2
qemu-img create -f qcow2          -b /tmp/overlay1-noformat.qcow2 /tmp/overlay2-noformat.qcow2
qemu-img creage -f qcow2 -b nbd://example/asdf /tmp/nbd-noformat.qcow2 10M

(Note that the last one prints error, but that's expected)

Probe images:

$ ./tests/qemublockprobe -f qcow2 -p /tmp/overlay1-noformat.qcow2
type: file (1)
path: /tmp/overlay1-noformat.qcow2
format: qcow2 (14)
protocol: none' (0)
backing store raw: /tmp/base.qcow2

type: file (1)
path: /tmp/base.qcow2
format: qcow2 (14)
protocol: none' (0)

type: none (0)
path: (null)
format: none (0)
protocol: none' (0)

$ ./tests/qemublockprobe -f qcow2 -p /tmp/overlay2-format.qcow2
type: file (1)
path: /tmp/overlay2-format.qcow2
format: qcow2 (14)
protocol: none' (0)
backing store raw: /tmp/overlay1-format.qcow2

type: file (1)
path: /tmp/overlay1-format.qcow2
format: qcow2 (14)
protocol: none' (0)
backing store raw: /tmp/base.qcow2

type: file (1)
path: /tmp/base.qcow2
format: qcow2 (14)
protocol: none' (0)

type: none (0)
path: (null)
format: none (0)
protocol: none' (0)

$ ./tests/qemublockprobe -f qcow2 -p /tmp/overlay2-noformat.qcow2
/home/pipo/build/libvirt/gcc/tests/.libs/lt-qemublockprobe: libvirt error: Requested operation is not valid: format of backing image '/tmp/overlay1-noformat.qcow2' of image '/tmp/overlay2-noformat.qcow2' was not specified in the image metadata (See https://libvirt.org/kbase/backing_chains.html for troubleshooting)

$ ./tests/qemublockprobe -f qcow2 -p /tmp/nbd-noformat.qcow2
/home/pipo/build/libvirt/gcc/tests/.libs/lt-qemublockprobe: libvirt error: Requested operation is not valid: format of backing image 'nbd://example/asdf' of image '/tmp/nbd-noformat.qcow2' was not specified in the image metadata (See https://libvirt.org/kbase/backing_chains.html for troubleshooting)
---
 tests/Makefile.am      |  13 ++++-
 tests/qemublockprobe.c | 130 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemublockprobe.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index ed5255b62d..a47c7eda22 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -291,7 +291,9 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \
 	qemufirmwaretest \
 	qemuvhostusertest \
 	$(NULL)
-test_helpers += qemucapsprobe
+test_helpers += qemucapsprobe \
+		qemublockprobe \
+		$(NULL)
 test_libraries += libqemumonitortestutils.la \
 		libqemutestdriver.la \
 		libqemuxml2argvmock.la \
@@ -652,6 +654,14 @@ qemublocktest_LDADD = \
 	$(qemu_LDADDS) \
 	$(NULL)

+qemublockprobe_SOURCES = \
+	qemublockprobe.c \
+	$(NULL)
+qemublockprobe_LDADD = \
+	../src/libvirt.la \
+	$(qemu_LDADDS) \
+	$(NULL)
+
 qemudomaincheckpointxml2xmltest_SOURCES = \
 	qemudomaincheckpointxml2xmltest.c testutilsqemu.c testutilsqemu.h \
 	testutils.c testutils.h
@@ -707,6 +717,7 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c \
 	qemucaps2xmltest.c qemucommandutiltest.c \
 	qemumemlocktest.c qemucpumock.c testutilshostcpus.h \
 	qemublocktest.c \
+	qemublockprobe.c \
 	qemumigparamstest.c \
 	qemusecuritytest.c qemusecuritytest.h \
 	qemusecuritymock.c \
diff --git a/tests/qemublockprobe.c b/tests/qemublockprobe.c
new file mode 100644
index 0000000000..0dbc31028c
--- /dev/null
+++ b/tests/qemublockprobe.c
@@ -0,0 +1,130 @@
+/*
+ * qemublockprobe.c: image backing chain prober
+ *
+ * Copyright (C) 2019 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdbool.h>
+
+#include "util/virfile.h"
+#include "util/virlog.h"
+#include "util/virstoragefile.h"
+
+#include "virgettext.h"
+
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+
+static void
+print_source(virStorageSourcePtr src)
+{
+    size_t i;
+
+    g_print("type: %s (%d)\n", virStorageTypeToString(src->type), src->type);
+    g_print("path: %s\n", src->path);
+    g_print("format: %s (%d)\n", virStorageFileFormatTypeToString(src->format), src->format);
+    g_print("protocol: %s' (%d)\n", virStorageNetProtocolTypeToString(src->protocol), src->protocol);
+    for (i = 0; i < src->nhosts; i++) {
+        virStorageNetHostDefPtr h = src->hosts + i;
+
+        g_print("host %zu: name: '%s', port: '%u', transport: '%s'(%d), socket: '%s'\n",
+                i, h->name, h->port, virStorageNetHostTransportTypeToString(h->transport),
+                h->transport, h->socket);
+    }
+    if (src->sliceStorage)
+        g_print("slice type: storage, offset: %llu, size: %llu\n",
+                src->sliceStorage->offset, src->sliceStorage->size);
+    if (src->backingStoreRaw)
+        g_print("backing store raw: %s\n", src->backingStoreRaw);
+    if (src->externalDataStoreRaw)
+        g_print("external store raw: %s\n", src->externalDataStoreRaw);
+    if (src->relPath)
+        g_print("relative path: %s\n", src->relPath);
+
+    g_print("\n");
+}
+
+
+int main(int argc, char **argv)
+{
+    g_autofree char *path = NULL;
+    g_autofree char *format = NULL;
+    g_autoptr(GError) error = NULL;
+    bool verbose = false;
+    g_autoptr(virStorageSource) src = NULL;
+    GOptionContext *ctx;
+    virStorageSourcePtr n;
+    int ret = 1;
+
+    GOptionEntry entries[] = {
+        { "path", 'p', 0, G_OPTION_ARG_STRING, &path, "path to image", "DIR" },
+        { "format", 'f', 0, G_OPTION_ARG_STRING, &format, "format of image", "DIR" },
+        { "verbose", 'v', 0, G_OPTION_ARG_NONE, &verbose, "Verbose output", NULL },
+        { 0 }
+    };
+
+
+    ctx = g_option_context_new("- inspect an image");
+    g_option_context_add_main_entries(ctx, entries, PACKAGE);
+    if (!g_option_context_parse(ctx, &argc, &argv, &error)) {
+        g_printerr("%s: option parsing failed: %s\n",
+                   argv[0], error->message);
+        return 1;
+    }
+
+    if (!path) {
+        g_printerr("%s: missing path\n", argv[0]);
+        return 1;
+    }
+
+    if (virErrorInitialize() < 0) {
+        g_printerr("%s: failed to initialize error handling\n", argv[0]);
+        return 1;
+    }
+
+    virLogSetFromEnv();
+    virFileActivateDirOverrideForProg(argv[0]);
+
+    if (!(src = virStorageSourceNew()))
+        goto cleanup;
+
+    src->path = g_steal_pointer(&path);
+    src->type = VIR_STORAGE_TYPE_FILE;
+
+    if (format &&
+        (src->format = virStorageFileFormatTypeFromString(format)) < 0) {
+        g_printerr("%s: unknown format '%s'\n", argv[0], format);
+        goto cleanup;
+    }
+
+    if (virStorageFileGetMetadata(src, -1, -1, true) < 0)
+        goto cleanup;
+
+    for (n = src; n; n = n->backingStore)
+        print_source(n);
+
+    ret = 0;
+
+ cleanup:
+    if (virGetLastErrorCode() != VIR_ERR_OK)
+        g_printerr("%s: libvirt error: %s\n", argv[0], virGetLastErrorMessage());
+
+    return ret;
+}
-- 
2.24.1

On 2/17/20 11:13 AM, Peter Krempa wrote:
> Note that this is not finished yet, but allows to test the image
> detection patches:

"allows to ${verb}" is not idiomatic; you want "allows ${verb}ing" or 
"allows $subject to ${verb}".  Here, I would go with "allows testing of 
the image detection patches".

> 
> Prepare few images:

Prepare a few images:

> qemu-img create -f qcow2 /tmp/base.qcow2 10M
> qemu-img create -f qcow2          -b /tmp/base.qcow2 /tmp/overlay1-noformat.qcow2
> qemu-img create -f qcow2 -F qcow2 -b /tmp/base.qcow2 /tmp/overlay1-format.qcow2
> qemu-img create -f qcow2 -F qcow2 -b /tmp/overlay1-format.qcow2 /tmp/overlay2-format.qcow2
> qemu-img create -f qcow2          -b /tmp/overlay1-noformat.qcow2 /tmp/overlay2-noformat.qcow2
> qemu-img creage -f qcow2 -b nbd://example/asdf /tmp/nbd-noformat.qcow2 10M
> 

/tmp/overlay1-noformat.qcow2 is inherently unsafe.  The probe of 
/tmp/base.qcow2 returns qcow2, but we cannot trust whether that was 
because /tmp/base.qcow2 was actually qcow2 or if it was because 
/tmp/base.qcow2 was raw where the guest wrote a qcow2 header; in the 
former case our guess is correct, but in the latter case, even though we 
avoid a security issue of chasing further files under guest control, we 
do NOT avoid the issue of corrupting guest data (serving the qcow2 
payload rather than the qcow2 metadata that the guest wrote in a raw 
file is guest-visible data corruption).

> (Note that the last one prints error, but that's expected)
> 
> Probe images:
> 
> $ ./tests/qemublockprobe -f qcow2 -p /tmp/overlay1-noformat.qcow2
> type: file (1)
> path: /tmp/overlay1-noformat.qcow2
> format: qcow2 (14)
> protocol: none' (0)

Why the mismatched '?

> backing store raw: /tmp/base.qcow2
> 
> type: file (1)
> path: /tmp/base.qcow2
> format: qcow2 (14)
> protocol: none' (0)
> 
> type: none (0)
> path: (null)
> format: none (0)
> protocol: none' (0)
> 

The tool needs to report that this image as potentially corrupt (our 
probe of qcow2 may be correct, or it may be a mistake for what was 
really raw, and without an explicit backing format, we are unwilling to 
hand the image to qemu for fear of data corruption visible to the guest, 
even if we have avoided a security hole of chasing files under guest 
control).

> $ ./tests/qemublockprobe -f qcow2 -p /tmp/overlay2-format.qcow2
> type: file (1)
> path: /tmp/overlay2-format.qcow2
> format: qcow2 (14)
> protocol: none' (0)
> backing store raw: /tmp/overlay1-format.qcow2
> 
> type: file (1)
> path: /tmp/overlay1-format.qcow2
> format: qcow2 (14)
> protocol: none' (0)
> backing store raw: /tmp/base.qcow2
> 
> type: file (1)
> path: /tmp/base.qcow2
> format: qcow2 (14)
> protocol: none' (0)
> 
> type: none (0)
> path: (null)
> format: none (0)
> protocol: none' (0)

This image is safe.

> 
> $ ./tests/qemublockprobe -f qcow2 -p /tmp/overlay2-noformat.qcow2
> /home/pipo/build/libvirt/gcc/tests/.libs/lt-qemublockprobe: libvirt error: Requested operation is not valid: format of backing image '/tmp/overlay1-noformat.qcow2' of image '/tmp/overlay2-noformat.qcow2' was not specified in the image metadata (See https://libvirt.org/kbase/backing_chains.html for troubleshooting)

This image is correctly identified as unsafe.

> 
> $ ./tests/qemublockprobe -f qcow2 -p /tmp/nbd-noformat.qcow2
> /home/pipo/build/libvirt/gcc/tests/.libs/lt-qemublockprobe: libvirt error: Requested operation is not valid: format of backing image 'nbd://example/asdf' of image '/tmp/nbd-noformat.qcow2' was not specified in the image metadata (See https://libvirt.org/kbase/backing_chains.html for troubleshooting)

This image is correctly identified as potentially unsafe because we were 
unable to probe nbd://example/asdf (had the probe been successful, AND 
returned a result of raw, then this image would be safe; had the probe 
been successful but returned anything other than raw, it is no different 
than the existing failure of the probe being unsuccessful)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On Wed, Feb 19, 2020 at 10:31:18 -0600, Eric Blake wrote:
> On 2/17/20 11:13 AM, Peter Krempa wrote:
> > Note that this is not finished yet, but allows to test the image
> > detection patches:
> 
> "allows to ${verb}" is not idiomatic; you want "allows ${verb}ing" or
> "allows $subject to ${verb}".  Here, I would go with "allows testing of the
> image detection patches".
> 
> > 
> > Prepare few images:
> 
> Prepare a few images:
> 
> > qemu-img create -f qcow2 /tmp/base.qcow2 10M
> > qemu-img create -f qcow2          -b /tmp/base.qcow2 /tmp/overlay1-noformat.qcow2
> > qemu-img create -f qcow2 -F qcow2 -b /tmp/base.qcow2 /tmp/overlay1-format.qcow2
> > qemu-img create -f qcow2 -F qcow2 -b /tmp/overlay1-format.qcow2 /tmp/overlay2-format.qcow2
> > qemu-img create -f qcow2          -b /tmp/overlay1-noformat.qcow2 /tmp/overlay2-noformat.qcow2
> > qemu-img creage -f qcow2 -b nbd://example/asdf /tmp/nbd-noformat.qcow2 10M
> > 
> 
> /tmp/overlay1-noformat.qcow2 is inherently unsafe.  The probe of
> /tmp/base.qcow2 returns qcow2, but we cannot trust whether that was because
> /tmp/base.qcow2 was actually qcow2 or if it was because /tmp/base.qcow2 was
> raw where the guest wrote a qcow2 header; in the former case our guess is
> correct, but in the latter case, even though we avoid a security issue of
> chasing further files under guest control, we do NOT avoid the issue of
> corrupting guest data (serving the qcow2 payload rather than the qcow2
> metadata that the guest wrote in a raw file is guest-visible data
> corruption).
> 
> > (Note that the last one prints error, but that's expected)
> > 
> > Probe images:
> > 
> > $ ./tests/qemublockprobe -f qcow2 -p /tmp/overlay1-noformat.qcow2
> > type: file (1)
> > path: /tmp/overlay1-noformat.qcow2
> > format: qcow2 (14)
> > protocol: none' (0)
> 
> Why the mismatched '?
> 
> > backing store raw: /tmp/base.qcow2
> > 
> > type: file (1)
> > path: /tmp/base.qcow2
> > format: qcow2 (14)
> > protocol: none' (0)
> > 
> > type: none (0)
> > path: (null)
> > format: none (0)
> > protocol: none' (0)
> > 
> 
> The tool needs to report that this image as potentially corrupt (our probe
> of qcow2 may be correct, or it may be a mistake for what was really raw, and
> without an explicit backing format, we are unwilling to hand the image to
> qemu for fear of data corruption visible to the guest, even if we have
> avoided a security hole of chasing files under guest control).

As noted previously, we've used these semantics forever. Prior to
introduction of blockdev, we probed the format, but assumed that the
image is 'raw' in such case. But we've never reported an error or done
anything. We started the VM and let qemu re-probe.

This meant that both 'raw' and 'qcow2' images without a backing file
were working without a hitch even with sVirt. Users not having the
benefit of sVirt were also possibly still seeing possible unwanted
access.

I'm specifically interrested in keeping both 'raw' and single layer
'qcow2' work as they did before because in my opinion it's not worse
than it was before when running qemu where libvirt wouldn't use blockdev
and _strictly_ better in case when we do use -blockdev, while not making
many users unhappy.

And there were already a lot of unhappy users.