mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2024-12-24 06:35:24 +00:00
api: require write permission for guest agent interaction
I noticed that we allow virDomainGetVcpusFlags even for read-only connections, but that with a flag, it can require guest agent interaction. It is feasible that a malicious guest could intentionally abuse the replies it sends over the guest agent connection to possibly trigger a bug in libvirt's JSON parser, or withhold an answer so as to prevent the use of the agent in a later command such as a shutdown request. Although we don't know of any such exploits now (and therefore don't mind posting this patch publicly without trying to get a CVE assigned), it is better to err on the side of caution and explicitly require full access to any domain where the API requires guest interaction to operate correctly. I audited all commands that are marked as conditionally using a guest agent. Note that at least virDomainFSTrim is documented as needing a guest agent, but that such use is unconditional depending on the hypervisor (so the existing domain:fs_trim ACL should be sufficient there, rather than also requirng domain:write). But when designing future APIs, such as the plans for obtaining a domain's IP addresses, we should copy the approach of this patch in making interaction with the guest be specified via a flag, and use that flag to also require stricter access checks. * src/libvirt.c (virDomainGetVcpusFlags): Forbid guest interaction on read-only connection. (virDomainShutdownFlags, virDomainReboot): Improve docs on agent interaction. * src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_SNAPSHOT_CREATE_XML) (REMOTE_PROC_DOMAIN_SET_VCPUS_FLAGS) (REMOTE_PROC_DOMAIN_GET_VCPUS_FLAGS, REMOTE_PROC_DOMAIN_REBOOT) (REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS): Require domain:write for any conditional use of a guest agent. * src/xen/xen_driver.c: Fix clients. * src/libxl/libxl_driver.c: Likewise. * src/uml/uml_driver.c: Likewise. * src/qemu/qemu_driver.c: Likewise. * src/lxc/lxc_driver.c: Likewise. Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
parent
bb85da2cb1
commit
7f2d27d1e3
@ -3134,6 +3134,9 @@ error:
|
||||
* in which the hypervisor tries each shutdown method is undefined,
|
||||
* and a hypervisor is not required to support all methods.
|
||||
*
|
||||
* To use guest agent (VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) the domain XML
|
||||
* must have <channel> configured.
|
||||
*
|
||||
* Returns 0 in case of success and -1 in case of failure.
|
||||
*/
|
||||
int
|
||||
@ -3180,7 +3183,7 @@ error:
|
||||
*
|
||||
* If @flags is set to zero, then the hypervisor will choose the
|
||||
* method of shutdown it considers best. To have greater control
|
||||
* pass one or more of the virDomainShutdownFlagValues. The order
|
||||
* pass one or more of the virDomainRebootFlagValues. The order
|
||||
* in which the hypervisor tries each shutdown method is undefined,
|
||||
* and a hypervisor is not required to support all methods.
|
||||
*
|
||||
@ -9347,7 +9350,7 @@ error:
|
||||
* current virtual CPU count.
|
||||
*
|
||||
* If @flags includes VIR_DOMAIN_VCPU_GUEST, then the state of the processors
|
||||
* is modified in the guest instead of the hypervisor. This flag is only usable
|
||||
* is queried in the guest instead of the hypervisor. This flag is only usable
|
||||
* on live domains. Guest agent may be needed for this flag to be available.
|
||||
*
|
||||
* Returns the number of vCPUs in case of success, -1 in case of failure.
|
||||
@ -9362,6 +9365,10 @@ virDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
|
||||
virResetLastError();
|
||||
|
||||
virCheckDomainReturn(domain, -1);
|
||||
conn = domain->conn;
|
||||
|
||||
if (flags & VIR_DOMAIN_VCPU_GUEST)
|
||||
virCheckReadOnlyGoto(conn->flags, error);
|
||||
|
||||
/* At most one of these two flags should be set. */
|
||||
if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
|
||||
@ -9372,7 +9379,6 @@ virDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
|
||||
__FUNCTION__);
|
||||
goto error;
|
||||
}
|
||||
conn = domain->conn;
|
||||
|
||||
if (conn->driver->domainGetVcpusFlags) {
|
||||
int ret;
|
||||
|
@ -1409,7 +1409,7 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
|
||||
if (!(vm = libxlDomObjFromDomain(dom)))
|
||||
goto cleanup;
|
||||
|
||||
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def) < 0)
|
||||
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
if (!virDomainObjIsActive(vm)) {
|
||||
@ -1456,7 +1456,7 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
|
||||
if (!(vm = libxlDomObjFromDomain(dom)))
|
||||
goto cleanup;
|
||||
|
||||
if (virDomainRebootEnsureACL(dom->conn, vm->def) < 0)
|
||||
if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
if (!virDomainObjIsActive(vm)) {
|
||||
@ -2316,7 +2316,7 @@ libxlDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
|
||||
if (!(vm = libxlDomObjFromDomain(dom)))
|
||||
goto cleanup;
|
||||
|
||||
if (virDomainGetVcpusFlagsEnsureACL(dom->conn, vm->def) < 0)
|
||||
if (virDomainGetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
active = virDomainObjIsActive(vm);
|
||||
|
@ -3335,7 +3335,7 @@ lxcDomainShutdownFlags(virDomainPtr dom,
|
||||
|
||||
priv = vm->privateData;
|
||||
|
||||
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def) < 0)
|
||||
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
if (!virDomainObjIsActive(vm)) {
|
||||
@ -3415,7 +3415,7 @@ lxcDomainReboot(virDomainPtr dom,
|
||||
|
||||
priv = vm->privateData;
|
||||
|
||||
if (virDomainRebootEnsureACL(dom->conn, vm->def) < 0)
|
||||
if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
if (!virDomainObjIsActive(vm)) {
|
||||
|
@ -1835,7 +1835,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) {
|
||||
if (agentRequested || (!flags && priv->agent))
|
||||
useAgent = true;
|
||||
|
||||
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def) < 0)
|
||||
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
if (priv->agentError) {
|
||||
@ -1936,7 +1936,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
|
||||
|
||||
priv = vm->privateData;
|
||||
|
||||
if (virDomainRebootEnsureACL(dom->conn, vm->def) < 0)
|
||||
if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
if ((flags & VIR_DOMAIN_REBOOT_GUEST_AGENT) ||
|
||||
@ -4898,7 +4898,7 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
|
||||
|
||||
priv = vm->privateData;
|
||||
|
||||
if (virDomainGetVcpusFlagsEnsureACL(dom->conn, vm->def) < 0)
|
||||
if (virDomainGetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
|
||||
@ -13090,7 +13090,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
|
||||
|
||||
cfg = virQEMUDriverGetConfig(driver);
|
||||
|
||||
if (virDomainSnapshotCreateXMLEnsureACL(domain->conn, vm->def) < 0)
|
||||
if (virDomainSnapshotCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
|
||||
|
@ -3105,6 +3105,7 @@ enum remote_procedure {
|
||||
/**
|
||||
* @generate: both
|
||||
* @acl: domain:init_control
|
||||
* @acl: domain:write:VIR_DOMAIN_REBOOT_GUEST_AGENT
|
||||
*/
|
||||
REMOTE_PROC_DOMAIN_REBOOT = 27,
|
||||
|
||||
@ -4198,6 +4199,7 @@ enum remote_procedure {
|
||||
/**
|
||||
* @generate: both
|
||||
* @acl: domain:snapshot
|
||||
* @acl: domain:write:VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE
|
||||
*/
|
||||
REMOTE_PROC_DOMAIN_SNAPSHOT_CREATE_XML = 185,
|
||||
|
||||
@ -4290,12 +4292,14 @@ enum remote_procedure {
|
||||
* @acl: domain:write
|
||||
* @acl: domain:save:!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE
|
||||
* @acl: domain:save:VIR_DOMAIN_AFFECT_CONFIG
|
||||
* @acl: domain:write:VIR_DOMAIN_VCPU_GUEST
|
||||
*/
|
||||
REMOTE_PROC_DOMAIN_SET_VCPUS_FLAGS = 199,
|
||||
|
||||
/**
|
||||
* @generate: both
|
||||
* @acl: domain:read
|
||||
* @acl: domain:write:VIR_DOMAIN_VCPU_GUEST
|
||||
*/
|
||||
REMOTE_PROC_DOMAIN_GET_VCPUS_FLAGS = 200,
|
||||
|
||||
@ -4682,6 +4686,7 @@ enum remote_procedure {
|
||||
/**
|
||||
* @generate: both
|
||||
* @acl: domain:init_control
|
||||
* @acl: domain:write:VIR_DOMAIN_SHUTDOWN_GUEST_AGENT
|
||||
*/
|
||||
REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 258,
|
||||
|
||||
|
@ -1635,7 +1635,7 @@ static int umlDomainShutdownFlags(virDomainPtr dom,
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def) < 0)
|
||||
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
#if 0
|
||||
|
@ -952,7 +952,7 @@ xenUnifiedDomainShutdownFlags(virDomainPtr dom,
|
||||
if (!(def = xenGetDomainDefForDom(dom)))
|
||||
goto cleanup;
|
||||
|
||||
if (virDomainShutdownFlagsEnsureACL(dom->conn, def) < 0)
|
||||
if (virDomainShutdownFlagsEnsureACL(dom->conn, def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
ret = xenDaemonDomainShutdown(dom->conn, def);
|
||||
@ -979,7 +979,7 @@ xenUnifiedDomainReboot(virDomainPtr dom, unsigned int flags)
|
||||
if (!(def = xenGetDomainDefForDom(dom)))
|
||||
goto cleanup;
|
||||
|
||||
if (virDomainRebootEnsureACL(dom->conn, def) < 0)
|
||||
if (virDomainRebootEnsureACL(dom->conn, def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
ret = xenDaemonDomainReboot(dom->conn, def);
|
||||
@ -1526,7 +1526,7 @@ xenUnifiedDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
|
||||
if (!(def = xenGetDomainDefForDom(dom)))
|
||||
goto cleanup;
|
||||
|
||||
if (virDomainGetVcpusFlagsEnsureACL(dom->conn, def) < 0)
|
||||
if (virDomainGetVcpusFlagsEnsureACL(dom->conn, def, flags) < 0)
|
||||
goto cleanup;
|
||||
|
||||
ret = xenUnifiedDomainGetVcpusFlagsInternal(dom, def, flags);
|
||||
|
Loading…
Reference in New Issue
Block a user