mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2024-12-23 06:05:27 +00:00
event: filter global events by domain:getattr ACL [CVE-2014-0028]
Ever since ACL filtering was added in commit7639736
(v1.1.1), a user could still use event registration to obtain access to a domain that they could not normally access via virDomainLookup* or virConnectListAllDomains and friends. We already have the framework in the RPC generator for creating the filter, and previous cleanup patches got us to the point that we can now wire the filter through the entire object event stack. Furthermore, whether or not domain:getattr is honored, use of global events is a form of obtaining a list of networks, which is covered by connect:search_domains added ina93cd08
(v1.1.0). Ideally, we'd have a way to enforce connect:search_domains when doing global registrations while omitting that check on a per-domain registration. But this patch just unconditionally requires connect:search_domains, even when no list could be obtained, based on the following observations: 1. Administrators are unlikely to grant domain:getattr for one or all domains while still denying connect:search_domains - a user that is able to manage domains will want to be able to manage them efficiently, but efficient management includes being able to list the domains they can access. The idea of denying connect:search_domains while still granting access to individual domains is therefore not adding any real security, but just serves as a layer of obscurity to annoy the end user. 2. In the current implementation, domain events are filtered on the client; the server has no idea if a domain filter was requested, and must therefore assume that all domain event requests are global. Even if we fix the RPC protocol to allow for server-side filtering for newer client/server combos, making the connect:serach_domains ACL check conditional on whether the domain argument was NULL won't benefit older clients. Therefore, we choose to document that connect:search_domains is a pre-requisite to any domain event management. Network events need the same treatment, with the obvious change of using connect:search_networks and network:getattr. * src/access/viraccessperm.h (VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS) (VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional effect of the permission. * src/conf/domain_event.h (virDomainEventStateRegister) (virDomainEventStateRegisterID): Add new parameter. * src/conf/network_event.h (virNetworkEventStateRegisterID): Likewise. * src/conf/object_event_private.h (virObjectEventStateRegisterID): Likewise. * src/conf/object_event.c (_virObjectEventCallback): Track a filter. (virObjectEventDispatchMatchCallback): Use filter. (virObjectEventCallbackListAddID): Register filter. * src/conf/domain_event.c (virDomainEventFilter): New function. (virDomainEventStateRegister, virDomainEventStateRegisterID): Adjust callers. * src/conf/network_event.c (virNetworkEventFilter): New function. (virNetworkEventStateRegisterID): Adjust caller. * src/remote/remote_protocol.x (REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER) (REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY) (REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a filter, and require connect:search_domains instead of weaker connect:read. * src/test/test_driver.c (testConnectDomainEventRegister) (testConnectDomainEventRegisterAny) (testConnectNetworkEventRegisterAny): Update callers. * src/remote/remote_driver.c (remoteConnectDomainEventRegister) (remoteConnectDomainEventRegisterAny): Likewise. * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister) (xenUnifiedConnectDomainEventRegisterAny): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise. * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister) (libxlConnectDomainEventRegisterAny): Likewise. * src/qemu/qemu_driver.c (qemuConnectDomainEventRegister) (qemuConnectDomainEventRegisterAny): Likewise. * src/uml/uml_driver.c (umlConnectDomainEventRegister) (umlConnectDomainEventRegisterAny): Likewise. * src/network/bridge_driver.c (networkConnectNetworkEventRegisterAny): Likewise. * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister) (lxcConnectDomainEventRegisterAny): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> (cherry picked from commitf9f5634053
) Conflicts: src/conf/object_event.c - not backporting event refactoring src/conf/object_event_private.h - likewise src/conf/network_event.c - not backporting network events src/conf/network_event.h - likewise src/network/bridge_driver.c - likewise src/access/viraccessperm.h - likewise src/remote/remote_protocol.x - likewise src/conf/domain_event.c - includes code that upstream has in object_event src/conf/domain_event.h - context src/libxl/libxl_driver.c - context src/lxc/lxc_driver.c - context src/remote/remote_driver.c - context, not backporting network events src/test/test_driver.c - context, not backporting network events src/uml/uml_driver.c - context src/xen/xen_driver.c - context
This commit is contained in:
parent
e122c603df
commit
7ccc135996
@ -1,7 +1,7 @@
|
||||
/*
|
||||
* viraccessperm.h: access control permissions
|
||||
*
|
||||
* Copyright (C) 2012-2013 Red Hat, Inc.
|
||||
* Copyright (C) 2012-2014 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
|
||||
@ -47,7 +47,7 @@ typedef enum {
|
||||
|
||||
/**
|
||||
* @desc: List domains
|
||||
* @message: Listing domains requires authorization
|
||||
* @message: Listing domains or using domain events requires authorization
|
||||
* @anonymous: 1
|
||||
*/
|
||||
VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS,
|
||||
|
@ -32,6 +32,20 @@
|
||||
|
||||
#define VIR_FROM_THIS VIR_FROM_NONE
|
||||
|
||||
/**
|
||||
* virObjectEventCallbackFilter:
|
||||
* @conn: the connection pointer
|
||||
* @event: the event about to be dispatched
|
||||
* @opaque: opaque data registered with the filter
|
||||
*
|
||||
* Callback to do final filtering for a reason not tracked directly by
|
||||
* virObjectEventStateRegisterID(). Return false if @event must not
|
||||
* be sent to @conn.
|
||||
*/
|
||||
typedef bool (*virObjectEventCallbackFilter)(virConnectPtr conn,
|
||||
virDomainEventPtr event,
|
||||
void *opaque);
|
||||
|
||||
struct _virDomainMeta {
|
||||
int id;
|
||||
char *name;
|
||||
@ -68,6 +82,8 @@ struct _virDomainEventCallback {
|
||||
int eventID;
|
||||
virConnectPtr conn;
|
||||
virDomainMetaPtr dom;
|
||||
virObjectEventCallbackFilter filter;
|
||||
void *filter_opaque;
|
||||
virConnectDomainEventGenericCallback cb;
|
||||
void *opaque;
|
||||
virFreeCallback freecb;
|
||||
@ -344,6 +360,9 @@ virDomainEventCallbackListPurgeMarked(virDomainEventCallbackListPtr cbList)
|
||||
* virDomainEventCallbackListAddID:
|
||||
* @conn: pointer to the connection
|
||||
* @cbList: the list
|
||||
* @dom: optional domain to filter on
|
||||
* @filter optional last-ditch filter callback
|
||||
* @filter_opaque: opaque data to pass to @filter
|
||||
* @eventID: the event ID
|
||||
* @callback: the callback to add
|
||||
* @opaque: opaque data tio pass to callback
|
||||
@ -355,6 +374,8 @@ static int
|
||||
virDomainEventCallbackListAddID(virConnectPtr conn,
|
||||
virDomainEventCallbackListPtr cbList,
|
||||
virDomainPtr dom,
|
||||
virObjectEventCallbackFilter filter,
|
||||
void *filter_opaque,
|
||||
int eventID,
|
||||
virConnectDomainEventGenericCallback callback,
|
||||
void *opaque,
|
||||
@ -401,6 +422,8 @@ virDomainEventCallbackListAddID(virConnectPtr conn,
|
||||
memcpy(event->dom->uuid, dom->uuid, VIR_UUID_BUFLEN);
|
||||
event->dom->id = dom->id;
|
||||
}
|
||||
event->filter = filter;
|
||||
event->filter_opaque = filter_opaque;
|
||||
|
||||
/* Make space on list */
|
||||
if (VIR_REALLOC_N(cbList->callbacks, cbList->count + 1) < 0)
|
||||
@ -440,6 +463,8 @@ error:
|
||||
* virDomainEventCallbackListAdd:
|
||||
* @conn: pointer to the connection
|
||||
* @cbList: the list
|
||||
* @filter optional last-ditch filter callback
|
||||
* @filter_opaque: opaque data to pass to @filter
|
||||
* @callback: the callback to add
|
||||
* @opaque: opaque data tio pass to callback
|
||||
*
|
||||
@ -448,11 +473,14 @@ error:
|
||||
static int
|
||||
virDomainEventCallbackListAdd(virConnectPtr conn,
|
||||
virDomainEventCallbackListPtr cbList,
|
||||
virObjectEventCallbackFilter filter,
|
||||
void *filter_opaque,
|
||||
virConnectDomainEventCallback callback,
|
||||
void *opaque,
|
||||
virFreeCallback freecb)
|
||||
{
|
||||
return virDomainEventCallbackListAddID(conn, cbList, NULL,
|
||||
filter, filter_opaque,
|
||||
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
|
||||
VIR_DOMAIN_EVENT_CALLBACK(callback),
|
||||
opaque, freecb, NULL);
|
||||
@ -680,6 +708,32 @@ static virDomainEventPtr virDomainEventNewInternal(int eventID,
|
||||
return event;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* virDomainEventFilter:
|
||||
* @conn: pointer to the connection
|
||||
* @event: the event to check
|
||||
* @opaque: opaque data holding ACL filter to use
|
||||
*
|
||||
* Internal function to run ACL filtering before dispatching an event
|
||||
*/
|
||||
static bool
|
||||
virDomainEventFilter(virConnectPtr conn, virDomainEventPtr event,
|
||||
void *opaque)
|
||||
{
|
||||
virDomainDef dom;
|
||||
virDomainObjListFilter filter = opaque;
|
||||
|
||||
/* For now, we just create a virDomainDef with enough contents to
|
||||
* satisfy what viraccessdriverpolkit.c references. This is a bit
|
||||
* fragile, but I don't know of anything better. */
|
||||
dom.name = event->dom.name;
|
||||
memcpy(dom.uuid, event->dom.uuid, VIR_UUID_BUFLEN);
|
||||
|
||||
return (filter)(conn, &dom);
|
||||
}
|
||||
|
||||
|
||||
virDomainEventPtr virDomainEventNew(int id, const char *name,
|
||||
const unsigned char *uuid,
|
||||
int type, int detail)
|
||||
@ -1381,6 +1435,9 @@ static int virDomainEventDispatchMatchCallback(virDomainEventPtr event,
|
||||
if (cb->eventID != event->eventID)
|
||||
return 0;
|
||||
|
||||
if (cb->filter && !(cb->filter)(cb->conn, event, cb->filter_opaque))
|
||||
return 0;
|
||||
|
||||
if (cb->dom) {
|
||||
/* Deliberately ignoring 'id' for matching, since that
|
||||
* will cause problems when a domain switches between
|
||||
@ -1510,6 +1567,7 @@ virDomainEventStateFlush(virDomainEventStatePtr state)
|
||||
* virDomainEventStateRegister:
|
||||
* @conn: connection to associate with callback
|
||||
* @state: domain event state
|
||||
* @filter: optional ACL filter to limit which events can be sent
|
||||
* @callback: function to remove from event
|
||||
* @opaque: data blob to pass to callback
|
||||
* @freecb: callback to free @opaque
|
||||
@ -1522,6 +1580,7 @@ virDomainEventStateFlush(virDomainEventStatePtr state)
|
||||
int
|
||||
virDomainEventStateRegister(virConnectPtr conn,
|
||||
virDomainEventStatePtr state,
|
||||
virDomainObjListFilter filter,
|
||||
virConnectDomainEventCallback callback,
|
||||
void *opaque,
|
||||
virFreeCallback freecb)
|
||||
@ -1542,7 +1601,8 @@ virDomainEventStateRegister(virConnectPtr conn,
|
||||
}
|
||||
|
||||
ret = virDomainEventCallbackListAdd(conn, state->callbacks,
|
||||
callback, opaque, freecb);
|
||||
filter ? virDomainEventFilter : NULL,
|
||||
filter, callback, opaque, freecb);
|
||||
|
||||
if (ret == -1 &&
|
||||
state->callbacks->count == 0 &&
|
||||
@ -1561,6 +1621,7 @@ cleanup:
|
||||
* virDomainEventStateRegisterID:
|
||||
* @conn: connection to associate with callback
|
||||
* @state: domain event state
|
||||
* @filter: optional ACL filter to limit which events can be sent
|
||||
* @eventID: ID of the event type to register for
|
||||
* @cb: function to remove from event
|
||||
* @opaque: data blob to pass to callback
|
||||
@ -1575,6 +1636,7 @@ cleanup:
|
||||
int
|
||||
virDomainEventStateRegisterID(virConnectPtr conn,
|
||||
virDomainEventStatePtr state,
|
||||
virDomainObjListFilter filter,
|
||||
virDomainPtr dom,
|
||||
int eventID,
|
||||
virConnectDomainEventGenericCallback cb,
|
||||
@ -1597,8 +1659,9 @@ virDomainEventStateRegisterID(virConnectPtr conn,
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
ret = virDomainEventCallbackListAddID(conn, state->callbacks,
|
||||
dom, eventID, cb, opaque, freecb,
|
||||
ret = virDomainEventCallbackListAddID(conn, state->callbacks, dom,
|
||||
filter ? virDomainEventFilter : NULL,
|
||||
filter, eventID, cb, opaque, freecb,
|
||||
callbackID);
|
||||
|
||||
if (ret == -1 &&
|
||||
|
@ -1,7 +1,7 @@
|
||||
/*
|
||||
* domain_event.h: domain event queue processing helpers
|
||||
*
|
||||
* Copyright (C) 2012 Red Hat, Inc.
|
||||
* Copyright (C) 2012-2014 Red Hat, Inc.
|
||||
* Copyright (C) 2008 VirtualIron
|
||||
*
|
||||
* This library is free software; you can redistribute it and/or
|
||||
@ -149,19 +149,21 @@ virDomainEventStateQueue(virDomainEventStatePtr state,
|
||||
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
|
||||
int virDomainEventStateRegister(virConnectPtr conn,
|
||||
virDomainEventStatePtr state,
|
||||
virDomainObjListFilter filter,
|
||||
virConnectDomainEventCallback callback,
|
||||
void *opaque,
|
||||
virFreeCallback freecb)
|
||||
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
|
||||
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
|
||||
int virDomainEventStateRegisterID(virConnectPtr conn,
|
||||
virDomainEventStatePtr state,
|
||||
virDomainObjListFilter filter,
|
||||
virDomainPtr dom,
|
||||
int eventID,
|
||||
virConnectDomainEventGenericCallback cb,
|
||||
void *opaque,
|
||||
virFreeCallback freecb,
|
||||
int *callbackID)
|
||||
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5);
|
||||
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(6);
|
||||
int
|
||||
virDomainEventStateDeregister(virConnectPtr conn,
|
||||
virDomainEventStatePtr state,
|
||||
|
@ -3527,6 +3527,7 @@ libxlConnectDomainEventRegister(virConnectPtr conn,
|
||||
|
||||
ret = virDomainEventStateRegister(conn,
|
||||
driver->domainEventState,
|
||||
virConnectDomainEventRegisterCheckACL,
|
||||
callback, opaque, freecb);
|
||||
|
||||
return ret;
|
||||
@ -4126,6 +4127,7 @@ libxlConnectDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom, int eve
|
||||
|
||||
if (virDomainEventStateRegisterID(conn,
|
||||
driver->domainEventState,
|
||||
virConnectDomainEventRegisterAnyCheckACL,
|
||||
dom, eventID, callback, opaque,
|
||||
freecb, &ret) < 0)
|
||||
ret = -1;
|
||||
|
@ -1294,6 +1294,7 @@ lxcConnectDomainEventRegister(virConnectPtr conn,
|
||||
|
||||
ret = virDomainEventStateRegister(conn,
|
||||
driver->domainEventState,
|
||||
virConnectDomainEventRegisterCheckACL,
|
||||
callback, opaque, freecb);
|
||||
|
||||
return ret;
|
||||
@ -1334,6 +1335,7 @@ lxcConnectDomainEventRegisterAny(virConnectPtr conn,
|
||||
|
||||
if (virDomainEventStateRegisterID(conn,
|
||||
driver->domainEventState,
|
||||
virConnectDomainEventRegisterAnyCheckACL,
|
||||
dom, eventID,
|
||||
callback, opaque, freecb, &ret) < 0)
|
||||
ret = -1;
|
||||
|
@ -9878,6 +9878,7 @@ qemuConnectDomainEventRegister(virConnectPtr conn,
|
||||
|
||||
if (virDomainEventStateRegister(conn,
|
||||
driver->domainEventState,
|
||||
virConnectDomainEventRegisterCheckACL,
|
||||
callback, opaque, freecb) < 0)
|
||||
goto cleanup;
|
||||
|
||||
@ -9926,6 +9927,7 @@ qemuConnectDomainEventRegisterAny(virConnectPtr conn,
|
||||
|
||||
if (virDomainEventStateRegisterID(conn,
|
||||
driver->domainEventState,
|
||||
virConnectDomainEventRegisterAnyCheckACL,
|
||||
dom, eventID,
|
||||
callback, opaque, freecb, &ret) < 0)
|
||||
ret = -1;
|
||||
|
@ -4309,7 +4309,7 @@ static int remoteConnectDomainEventRegister(virConnectPtr conn,
|
||||
|
||||
remoteDriverLock(priv);
|
||||
|
||||
if ((count = virDomainEventStateRegister(conn, priv->domainEventState,
|
||||
if ((count = virDomainEventStateRegister(conn, priv->domainEventState, NULL,
|
||||
callback, opaque, freecb)) < 0) {
|
||||
virReportError(VIR_ERR_RPC, "%s", _("adding cb to list"));
|
||||
goto done;
|
||||
@ -5095,7 +5095,7 @@ static int remoteConnectDomainEventRegisterAny(virConnectPtr conn,
|
||||
remoteDriverLock(priv);
|
||||
|
||||
if ((count = virDomainEventStateRegisterID(conn,
|
||||
priv->domainEventState,
|
||||
priv->domainEventState, NULL,
|
||||
dom, eventID,
|
||||
callback, opaque, freecb,
|
||||
&callbackID)) < 0) {
|
||||
|
@ -1955,7 +1955,7 @@ struct remote_node_device_destroy_args {
|
||||
|
||||
/*
|
||||
* Events Register/Deregister:
|
||||
* It would seem rpcgen does not like both args, and ret
|
||||
* It would seem rpcgen does not like both args and ret
|
||||
* to be null. It will not generate the prototype otherwise.
|
||||
* Pass back a redundant boolean to force prototype generation.
|
||||
*/
|
||||
@ -3620,7 +3620,8 @@ enum remote_procedure {
|
||||
/**
|
||||
* @generate: none
|
||||
* @priority: high
|
||||
* @acl: connect:read
|
||||
* @acl: connect:search_domains
|
||||
* @aclfilter: domain:getattr
|
||||
*/
|
||||
REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER = 105,
|
||||
|
||||
@ -4052,7 +4053,8 @@ enum remote_procedure {
|
||||
/**
|
||||
* @generate: none
|
||||
* @priority: high
|
||||
* @acl: connect:read
|
||||
* @acl: connect:search_domains
|
||||
* @aclfilter: domain:getattr
|
||||
*/
|
||||
REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY = 167,
|
||||
|
||||
|
@ -5948,7 +5948,7 @@ testConnectDomainEventRegister(virConnectPtr conn,
|
||||
|
||||
testDriverLock(driver);
|
||||
ret = virDomainEventStateRegister(conn,
|
||||
driver->domainEventState,
|
||||
driver->domainEventState, NULL,
|
||||
callback, opaque, freecb);
|
||||
testDriverUnlock(driver);
|
||||
|
||||
@ -5986,7 +5986,7 @@ testConnectDomainEventRegisterAny(virConnectPtr conn,
|
||||
|
||||
testDriverLock(driver);
|
||||
if (virDomainEventStateRegisterID(conn,
|
||||
driver->domainEventState,
|
||||
driver->domainEventState, NULL,
|
||||
dom, eventID,
|
||||
callback, opaque, freecb, &ret) < 0)
|
||||
ret = -1;
|
||||
|
@ -2618,6 +2618,7 @@ umlConnectDomainEventRegister(virConnectPtr conn,
|
||||
umlDriverLock(driver);
|
||||
ret = virDomainEventStateRegister(conn,
|
||||
driver->domainEventState,
|
||||
virConnectDomainEventRegisterCheckACL,
|
||||
callback, opaque, freecb);
|
||||
umlDriverUnlock(driver);
|
||||
|
||||
@ -2660,6 +2661,7 @@ umlConnectDomainEventRegisterAny(virConnectPtr conn,
|
||||
umlDriverLock(driver);
|
||||
if (virDomainEventStateRegisterID(conn,
|
||||
driver->domainEventState,
|
||||
virConnectDomainEventRegisterAnyCheckACL,
|
||||
dom, eventID,
|
||||
callback, opaque, freecb, &ret) < 0)
|
||||
ret = -1;
|
||||
|
@ -7240,7 +7240,7 @@ static int vboxConnectDomainEventRegister(virConnectPtr conn,
|
||||
* later you can iterate over them
|
||||
*/
|
||||
|
||||
ret = virDomainEventStateRegister(conn, data->domainEvents,
|
||||
ret = virDomainEventStateRegister(conn, data->domainEvents, NULL,
|
||||
callback, opaque, freecb);
|
||||
VIR_DEBUG("virDomainEventStateRegister (ret = %d) (conn: %p, "
|
||||
"callback: %p, opaque: %p, "
|
||||
@ -7332,7 +7332,7 @@ static int vboxConnectDomainEventRegisterAny(virConnectPtr conn,
|
||||
* later you can iterate over them
|
||||
*/
|
||||
|
||||
if (virDomainEventStateRegisterID(conn, data->domainEvents,
|
||||
if (virDomainEventStateRegisterID(conn, data->domainEvents, NULL,
|
||||
dom, eventID,
|
||||
callback, opaque, freecb, &ret) < 0)
|
||||
ret = -1;
|
||||
|
@ -2326,6 +2326,7 @@ xenUnifiedConnectDomainEventRegister(virConnectPtr conn,
|
||||
}
|
||||
|
||||
ret = virDomainEventStateRegister(conn, priv->domainEvents,
|
||||
virConnectDomainEventRegisterCheckACL,
|
||||
callback, opaque, freefunc);
|
||||
|
||||
xenUnifiedUnlock(priv);
|
||||
@ -2383,6 +2384,7 @@ xenUnifiedConnectDomainEventRegisterAny(virConnectPtr conn,
|
||||
}
|
||||
|
||||
if (virDomainEventStateRegisterID(conn, priv->domainEvents,
|
||||
virConnectDomainEventRegisterAnyCheckACL,
|
||||
dom, eventID,
|
||||
callback, opaque, freefunc, &ret) < 0)
|
||||
ret = -1;
|
||||
|
Loading…
Reference in New Issue
Block a user