qemu: snapshot: Do ACL check prior to checkpoint interlocking

[libvirt] [PATCH] qemu: snapshot: Do ACL check prior to checkpoint interlocking

Posted by Peter Krempa 4 years, 7 months ago

Commit 7efe930ec3c introduced interlock of snapshots and checkpoints,
but the check is executed prior to the snapshot API ACL check. This
means that an unauthorized user can see whether a VM exists if it has a
checkpoint.

Move the checks to proper places.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---

Given that currently checkpoints by themselves are not very useful I
doubt that there are users which could hit this. Thus I'm sending it
also directly to the public mailing list for faster turnaround.

 src/qemu/qemu_driver.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0753904472..f7f059b6d6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15902,18 +15902,18 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
     if (!(vm = qemuDomObjFromDomain(domain)))
         goto cleanup;

-    if (virDomainListCheckpoints(vm->checkpoints, NULL, domain, NULL, 0) > 0) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-                       _("cannot create snapshot while checkpoint exists"));
-        goto cleanup;
-    }
-
     priv = vm->privateData;
     cfg = virQEMUDriverGetConfig(driver);

     if (virDomainSnapshotCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0)
         goto cleanup;

+    if (virDomainListCheckpoints(vm->checkpoints, NULL, domain, NULL, 0) > 0) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("cannot create snapshot while checkpoint exists"));
+        goto cleanup;
+    }
+
     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
         goto cleanup;

-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Libvirt-Security] [PATCH] qemu: snapshot: Do ACL check prior to checkpoint interlocking

Posted by Eric Blake 4 years, 7 months ago

On 9/23/19 8:52 AM, Peter Krempa wrote:
> Commit 7efe930ec3c introduced interlock of snapshots and checkpoints,
> but the check is executed prior to the snapshot API ACL check. This
> means that an unauthorized user can see whether a VM exists if it has a
> checkpoint.
> 
> Move the checks to proper places.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> 
> Given that currently checkpoints by themselves are not very useful I
> doubt that there are users which could hit this. Thus I'm sending it
> also directly to the public mailing list for faster turnaround.
> 
>  src/qemu/qemu_driver.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list