api: disallow virConnect*HypervisorCPU on read-only connections

These APIs can be used to execute arbitrary emulators.
Forbid them on read-only connections.

Fixes: CVE-2019-10168
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit bf6c2830b6)
Signed-off-by: Ján Tomko <jtomko@redhat.com>

docs/news.xml

@@ -33,7 +33,7 @@
      -->
 
 <libvirt>
-  <release version="v4.8.0" date="unreleased">
+  <release version="v4.8.0" date="2018-10-01">
     <section title="New features">
       <change>
         <summary>
@@ -55,6 +55,15 @@
           Drop support for these older versions and require Xen >= 4.6.
         </description>
       </change>
+      <change>
+        <summary>
+          nwfilter: Disallow binding creation in session mode
+        </summary>
+        <description>
+          Ensure that a filter binding creation is not attempted in session
+          mode and generates a proper error message.
+        </description>
+      </change>
     </section>
     <section title="Improvements">
       <change>
@@ -68,8 +77,46 @@
           Guest Agent.
         </description>
       </change>
+      <change>
+        <summary>
+          virsh: Implement vsh-table in virsh and virsh-admin
+        </summary>
+        <description>
+          The new API fixes problems with table-alignment, making the tables
+          more readable and deals with unicode.
+        </description>
+      </change>
     </section>
     <section title="Bug fixes">
+      <change>
+        <summary>
+          storage: Allow inputvol to be encrypted
+        </summary>
+        <description>
+          When creating a storage volume based on another volume, the base
+          input volume is allowed to be encrypted.
+        </description>
+      </change>
+      <change>
+        <summary>
+          virsh: Require explicit --domain for domxml-to-native
+        </summary>
+        <description>
+          The --domain option for domxml-to-native virsh command has always
+          been documented as required, but commit v4.3.0-127-gd86531daf2
+          accidentally made it optional.
+        </description>
+      </change>
+      <change>
+        <summary>
+          lxc_monitor: Avoid AB / BA lock race
+        </summary>
+        <description>
+          A deadlock situation could occur when autostarting a LXC domain
+          'guest' due to two threads attempting to take opposing locks while
+          holding opposing locks (AB BA problem).
+        </description>
+      </change>
     </section>
   </release>
   <release version="v4.7.0" date="2018-09-03">

src/admin/admin_server_dispatch.c

@@ -66,6 +66,28 @@ remoteAdmClientNew(virNetServerClientPtr client ATTRIBUTE_UNUSED,
                    void *opaque)
 {
     struct daemonAdmClientPrivate *priv;
+    uid_t clientuid;
+    gid_t clientgid;
+    pid_t clientpid;
+    unsigned long long timestamp;
+
+    if (virNetServerClientGetUNIXIdentity(client,
+                                          &clientuid,
+                                          &clientgid,
+                                          &clientpid,
+                                          &timestamp) < 0)
+        return NULL;
+
+    VIR_DEBUG("New client pid %lld uid %lld",
+              (long long)clientpid,
+              (long long)clientuid);
+
+    if (geteuid() != clientuid) {
+        virReportRestrictedError(_("Disallowing client %lld with uid %lld"),
+                                 (long long)clientpid,
+                                 (long long)clientuid);
+        return NULL;
+    }
 
     if (VIR_ALLOC(priv) < 0)
         return NULL;

src/libvirt-domain.c

@@ -1073,9 +1073,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml,
  * previously by virDomainSave() or virDomainSaveFlags().
  *
  * No security-sensitive data will be included unless @flags contains
- * VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only
- * connections.  For this API, @flags should not contain either
- * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU.
+ * VIR_DOMAIN_XML_SECURE.
  *
  * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of
  * error.  The caller must free() the returned value.
@@ -1091,12 +1089,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *file,
 
     virCheckConnectReturn(conn, NULL);
     virCheckNonNullArgGoto(file, error);
-
-    if ((conn->flags & VIR_CONNECT_RO) && (flags & VIR_DOMAIN_XML_SECURE)) {
-        virReportError(VIR_ERR_OPERATION_DENIED, "%s",
-                       _("virDomainSaveImageGetXMLDesc with secure flag"));
-        goto error;
-    }
+    virCheckReadOnlyGoto(conn->flags, error);
 
     if (conn->driver->domainSaveImageGetXMLDesc) {
         char *ret;
@@ -9497,6 +9490,7 @@ virDomainManagedSaveDefineXML(virDomainPtr domain, const char *dxml,
 
     virCheckDomainReturn(domain, -1);
     conn = domain->conn;
+    virCheckReadOnlyGoto(conn->flags, error);
 
     if (conn->driver->domainManagedSaveDefineXML) {
         int ret;
@@ -11288,6 +11282,7 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
     virResetLastError();
 
     virCheckConnectReturn(conn, NULL);
+    virCheckReadOnlyGoto(conn->flags, error);
 
     if (conn->driver->connectGetDomainCapabilities) {
         char *ret;

src/libvirt-host.c

@@ -1041,6 +1041,7 @@ virConnectCompareHypervisorCPU(virConnectPtr conn,
 
     virCheckConnectReturn(conn, VIR_CPU_COMPARE_ERROR);
     virCheckNonNullArgGoto(xmlCPU, error);
+    virCheckReadOnlyGoto(conn->flags, error);
 
     if (conn->driver->connectCompareHypervisorCPU) {
         int ret;
@@ -1234,6 +1235,7 @@ virConnectBaselineHypervisorCPU(virConnectPtr conn,
 
     virCheckConnectReturn(conn, NULL);
     virCheckNonNullArgGoto(xmlCPUs, error);
+    virCheckReadOnlyGoto(conn->flags, error);
 
     if (conn->driver->connectBaselineHypervisorCPU) {
         char *cpu;

src/locking/virtlockd-admin.socket.in

@@ -5,6 +5,7 @@ Before=libvirtd.service
 [Socket]
 ListenStream=@localstatedir@/run/libvirt/virtlockd-admin-sock
 Service=virtlockd.service
+SocketMode=0600
 
 [Install]
 WantedBy=sockets.target

src/locking/virtlockd.socket.in

@@ -4,6 +4,7 @@ Before=libvirtd.service
 
 [Socket]
 ListenStream=@localstatedir@/run/libvirt/virtlockd-sock
+SocketMode=0600
 
 [Install]
 WantedBy=sockets.target

src/logging/virtlogd-admin.socket.in

@@ -5,6 +5,7 @@ Before=libvirtd.service
 [Socket]
 ListenStream=@localstatedir@/run/libvirt/virtlogd-admin-sock
 Service=virtlogd.service
+SocketMode=0600
 
 [Install]
 WantedBy=sockets.target

src/logging/virtlogd.socket.in

@@ -4,6 +4,7 @@ Before=libvirtd.service
 
 [Socket]
 ListenStream=@localstatedir@/run/libvirt/virtlogd-sock
+SocketMode=0600
 
 [Install]
 WantedBy=sockets.target

src/qemu/qemu_driver.c

@@ -6798,7 +6798,7 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path,
     if (fd < 0)
         goto cleanup;
 
-    if (virDomainSaveImageGetXMLDescEnsureACL(conn, def, flags) < 0)
+    if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0)
         goto cleanup;
 
     ret = qemuDomainDefFormatXML(driver, def, flags);

src/remote/remote_protocol.x

@@ -5226,8 +5226,7 @@ enum remote_procedure {
     /**
      * @generate: both
      * @priority: high
-     * @acl: domain:read
-     * @acl: domain:read_secure:VIR_DOMAIN_XML_SECURE
+     * @acl: domain:write
      */
     REMOTE_PROC_DOMAIN_SAVE_IMAGE_GET_XML_DESC = 235,
 

src/uml/uml_driver.c

@@ -1193,6 +1193,13 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn,
 {
     virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
 
+    /* URI was good, but driver isn't active */
+    if (uml_driver == NULL) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("uml state driver is not active"));
+        return VIR_DRV_OPEN_ERROR;
+    }
+
     /* Check path and tell them correct path if they made a mistake */
     if (uml_driver->privileged) {
         if (STRNEQ(conn->uri->path, "/system") &&
@@ -1211,13 +1218,6 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn,
         }
     }
 
-    /* URI was good, but driver isn't active */
-    if (uml_driver == NULL) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("uml state driver is not active"));
-        return VIR_DRV_OPEN_ERROR;
-    }
-
     if (virConnectOpenEnsureACL(conn) < 0)
         return VIR_DRV_OPEN_ERROR;
 

src/util/vircgroup.c

@@ -1055,7 +1055,6 @@ virCgroupNewMachineSystemd(const char *name,
     int rv;
     virCgroupPtr init;
     VIR_AUTOFREE(char *) path = NULL;
-    virErrorPtr saved = NULL;
 
     VIR_DEBUG("Trying to setup machine '%s' via systemd", name);
     if ((rv = virSystemdCreateMachine(name,
@@ -1088,24 +1087,20 @@ virCgroupNewMachineSystemd(const char *name,
 
     if (virCgroupEnableMissingControllers(path, pidleader,
                                           controllers, group) < 0) {
-        goto error;
+        return -1;
     }
 
-    if (virCgroupAddProcess(*group, pidleader) < 0)
-        goto error;
+    if (virCgroupAddProcess(*group, pidleader) < 0) {
+        virErrorPtr saved = virSaveLastError();
+        virCgroupRemove(*group);
+        virCgroupFree(group);
+        if (saved) {
+            virSetError(saved);
+            virFreeError(saved);
+        }
+    }
 
     return 0;
-
- error:
-    saved = virSaveLastError();
-    virCgroupRemove(*group);
-    virCgroupFree(group);
-    if (saved) {
-        virSetError(saved);
-        virFreeError(saved);
-    }
-
-    return -1;
 }
 
 

tests/libxlxml2domconfigdata/fullvirt-cpuid-legacy-nest.json

@@ -0,0 +1,60 @@
+{
+    "c_info": {
+        "type": "hvm",
+        "name": "XenGuest2",
+        "uuid": "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+    },
+    "b_info": {
+        "max_vcpus": 1,
+        "avail_vcpus": [
+            0
+        ],
+        "max_memkb": 592896,
+        "target_memkb": 403456,
+        "shadow_memkb": 5656,
+        "cpuid": [
+            {
+                "leaf": 1,
+                "ecx": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx0",
+                "edx": "xxxxxxxxxxxxxxxxxxxxxxxxxxx1xxxx"
+            }
+        ],
+        "sched_params": {
+        },
+        "type.hvm": {
+            "pae": "True",
+            "apic": "True",
+            "acpi": "True",
+            "nested_hvm": "False",
+            "nographic": "True",
+            "vnc": {
+                "enable": "False"
+            },
+            "sdl": {
+                "enable": "False"
+            },
+            "spice": {
+
+            },
+            "boot": "c",
+            "rdm": {
+
+            }
+        },
+        "arch_arm": {
+
+        }
+    },
+    "disks": [
+        {
+            "pdev_path": "/dev/HostVG/XenGuest2",
+            "vdev": "hda",
+            "backend": "phy",
+            "format": "raw",
+            "removable": 1,
+            "readwrite": 1
+        }
+    ],
+    "on_reboot": "restart",
+    "on_crash": "restart"
+}

tests/libxlxml2domconfigdata/fullvirt-cpuid-legacy-nest.xml

@@ -0,0 +1,34 @@
+<domain type='xen'>
+  <name>XenGuest2</name>
+  <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>592896</memory>
+  <currentMemory unit='KiB'>403456</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='xenfv'>hvm</type>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <cpu mode='host-passthrough'>
+    <feature policy='forbid' name='pni'/>
+    <feature policy='forbid' name='vmx'/>
+    <feature policy='require' name='tsc'/>
+  </cpu>
+  <clock offset='variable' adjustment='0' basis='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <disk type='block' device='disk'>
+      <driver name='phy' type='raw'/>
+      <source dev='/dev/HostVG/XenGuest2'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+  </devices>
+</domain>

tests/libxlxml2domconfigdata/vnuma-hvm-legacy-nest.json

@@ -0,0 +1,178 @@
+{
+    "c_info": {
+        "type": "hvm",
+        "name": "test-hvm",
+        "uuid": "2147d599-9cc6-c0dc-92ab-4064b5446e9b"
+    },
+    "b_info": {
+        "max_vcpus": 6,
+        "avail_vcpus": [
+            0,
+            1,
+            2,
+            3,
+            4,
+            5
+        ],
+        "vnuma_nodes": [
+            {
+                "memkb": 2097152,
+                "distances": [
+                    10,
+                    21,
+                    31,
+                    41,
+                    51,
+                    61
+                ],
+                "vcpus": [
+                    0
+                ]
+            },
+            {
+                "memkb": 2097152,
+                "distances": [
+                    21,
+                    10,
+                    21,
+                    31,
+                    41,
+                    51
+                ],
+                "vcpus": [
+                    1
+                ]
+            },
+            {
+                "memkb": 2097152,
+                "distances": [
+                    31,
+                    21,
+                    10,
+                    21,
+                    31,
+                    41
+                ],
+                "vcpus": [
+                    2
+                ]
+            },
+            {
+                "memkb": 2097152,
+                "distances": [
+                    41,
+                    31,
+                    21,
+                    10,
+                    21,
+                    31
+                ],
+                "vcpus": [
+                    3
+                ]
+            },
+            {
+                "memkb": 2097152,
+                "distances": [
+                    51,
+                    41,
+                    31,
+                    21,
+                    10,
+                    21
+                ],
+                "vcpus": [
+                    4
+                ]
+            },
+            {
+                "memkb": 2097152,
+                "distances": [
+                    61,
+                    51,
+                    41,
+                    31,
+                    21,
+                    10
+                ],
+                "vcpus": [
+                    5
+                ]
+            }
+        ],
+        "max_memkb": 1048576,
+        "target_memkb": 1048576,
+        "video_memkb": 8192,
+        "shadow_memkb": 14336,
+        "device_model_version": "qemu_xen",
+        "device_model": "/bin/true",
+        "sched_params": {
+
+        },
+        "type.hvm": {
+            "pae": "True",
+            "apic": "True",
+            "acpi": "True",
+            "nested_hvm": "True",
+            "vga": {
+                "kind": "cirrus"
+            },
+            "vnc": {
+                "enable": "True",
+                "listen": "0.0.0.0",
+                "findunused": "False"
+            },
+            "sdl": {
+                "enable": "False"
+            },
+            "spice": {
+
+            },
+            "boot": "c",
+            "rdm": {
+
+            }
+        },
+        "arch_arm": {
+
+        }
+    },
+    "disks": [
+        {
+            "pdev_path": "/var/lib/xen/images/test-hvm.img",
+            "vdev": "hda",
+            "backend": "qdisk",
+            "format": "raw",
+            "removable": 1,
+            "readwrite": 1
+        }
+    ],
+    "nics": [
+        {
+            "devid": 0,
+            "mac": "00:16:3e:66:12:b4",
+            "bridge": "br0",
+            "script": "/etc/xen/scripts/vif-bridge",
+            "nictype": "vif_ioemu"
+        }
+    ],
+    "vfbs": [
+        {
+	    "devid": -1,
+            "vnc": {
+                "enable": "True",
+                "listen": "0.0.0.0",
+                "findunused": "False"
+            },
+            "sdl": {
+                "enable": "False"
+            }
+        }
+    ],
+    "vkbs": [
+        {
+            "devid": -1
+        }
+    ],
+    "on_reboot": "restart"
+}

tests/libxlxml2domconfigdata/vnuma-hvm-legacy-nest.xml

@@ -0,0 +1,100 @@
+<domain type='xen'>
+  <name>test-hvm</name>
+  <description>None</description>
+  <uuid>2147d599-9cc6-c0dc-92ab-4064b5446e9b</uuid>
+  <memory>1048576</memory>
+  <currentMemory>1048576</currentMemory>
+  <vcpu>6</vcpu>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <clock offset='utc'/>
+  <os>
+    <type>hvm</type>
+    <loader>/usr/lib/xen/boot/hvmloader</loader>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <apic/>
+    <acpi/>
+    <pae/>
+  </features>
+  <cpu mode='host-passthrough'>
+    <numa>
+      <cell id='0' cpus='0' memory='2097152' unit='KiB'>
+        <distances>
+          <sibling id='0' value='10'/>
+          <sibling id='1' value='21'/>
+          <sibling id='2' value='31'/>
+          <sibling id='3' value='41'/>
+          <sibling id='4' value='51'/>
+          <sibling id='5' value='61'/>
+        </distances>
+      </cell>
+      <cell id='1' cpus='1' memory='2097152' unit='KiB'>
+        <distances>
+          <sibling id='0' value='21'/>
+          <sibling id='1' value='10'/>
+          <sibling id='2' value='21'/>
+          <sibling id='3' value='31'/>
+          <sibling id='4' value='41'/>
+          <sibling id='5' value='51'/>
+        </distances>
+      </cell>
+      <cell id='2' cpus='2' memory='2097152' unit='KiB'>
+        <distances>
+          <sibling id='0' value='31'/>
+          <sibling id='1' value='21'/>
+          <sibling id='2' value='10'/>
+          <sibling id='3' value='21'/>
+          <sibling id='4' value='31'/>
+          <sibling id='5' value='41'/>
+        </distances>
+      </cell>
+      <cell id='3' cpus='3' memory='2097152' unit='KiB'>
+        <distances>
+          <sibling id='0' value='41'/>
+          <sibling id='1' value='31'/>
+          <sibling id='2' value='21'/>
+          <sibling id='3' value='10'/>
+          <sibling id='4' value='21'/>
+          <sibling id='5' value='31'/>
+        </distances>
+      </cell>
+      <cell id='4' cpus='4' memory='2097152' unit='KiB'>
+        <distances>
+          <sibling id='0' value='51'/>
+          <sibling id='1' value='41'/>
+          <sibling id='2' value='31'/>
+          <sibling id='3' value='21'/>
+          <sibling id='4' value='10'/>
+          <sibling id='5' value='21'/>
+        </distances>
+      </cell>
+      <cell id='5' cpus='5' memory='2097152' unit='KiB'>
+        <distances>
+          <sibling id='0' value='61'/>
+          <sibling id='1' value='51'/>
+          <sibling id='2' value='41'/>
+          <sibling id='3' value='31'/>
+          <sibling id='4' value='21'/>
+          <sibling id='5' value='10'/>
+        </distances>
+      </cell>
+    </numa>
+  </cpu>
+  <devices>
+    <emulator>/bin/true</emulator>
+    <disk type='file' device='disk'>
+      <driver name='qemu'/>
+      <source file='/var/lib/xen/images/test-hvm.img'/>
+      <target dev='hda'/>
+    </disk>
+    <interface type='bridge'>
+      <source bridge='br0'/>
+      <mac address='00:16:3e:66:12:b4'/>
+      <script path='/etc/xen/scripts/vif-bridge'/>
+    </interface>
+    <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'/>
+  </devices>
+</domain>

tests/libxlxml2domconfigtest.c

@@ -212,6 +212,9 @@ mymain(void)
 # ifdef LIBXL_HAVE_BUILDINFO_NESTED_HVM
     DO_TEST("vnuma-hvm");
     DO_TEST("fullvirt-cpuid");
+# else
+    DO_TEST("vnuma-hvm-legacy-nest");
+    DO_TEST("fullvirt-cpuid-legacy-nest");
 # endif