11:57 a.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The current ACL checks validate access to the object being
passed in to the API calls.
There are a few APIs (all the virConnectList* / virConnectNum*
ones) which are used to get lists of objects in the first
place. Currently you could find out that there is a VM called
"foo", but you can't then do virDomainLookupByName since the
ACL check may block it.
This series introduces filtering in the object list APIs,
so you can't even see the existance of an object called
"foo", if you don't have permission over it.
This is not yet filtering the legacy Xen driver.
Daniel P. Berrange (8):
Add access control filtering of domain objects
Add access control filtering of network objects
Add access control filtering of node device objects
Add access control filtering of storage objects
Add access control filtering of secret objects
Add access control filtering of nwfilter objects
Add access control filtering of interface objects
Extend the ACL test case to validate filter rule checks
src/Makefile.am | 1 +
src/check-aclrules.pl | 97 ++++++++++++
src/conf/domain_conf.c | 91 +++++++----
src/conf/domain_conf.h | 17 ++-
src/conf/interface_conf.h | 3 +
src/conf/network_conf.c | 12 +-
src/conf/network_conf.h | 13 +-
src/conf/node_device_conf.c | 12 +-
src/conf/node_device_conf.h | 12 +-
src/conf/storage_conf.c | 12 +-
src/conf/storage_conf.h | 11 +-
src/interface/interface_backend_netcf.c | 262 +++++++++++++++++++++++++++-----
src/interface/interface_backend_udev.c | 56 +++++--
src/libvirt_private.syms | 6 +-
src/libxl/libxl_driver.c | 15 +-
src/lxc/lxc_driver.c | 15 +-
src/network/bridge_driver.c | 44 +++---
src/node_device/node_device_driver.c | 28 ++--
src/nwfilter/nwfilter_driver.c | 39 +++--
src/openvz/openvz_driver.c | 7 +-
src/parallels/parallels_driver.c | 14 +-
src/parallels/parallels_network.c | 2 +-
src/qemu/qemu_driver.c | 24 +--
src/rpc/gendispatch.pl | 42 +++--
src/secret/secret_driver.c | 14 +-
src/storage/storage_driver.c | 62 +++++---
src/test/test_driver.c | 18 ++-
src/uml/uml_driver.c | 15 +-
src/vmware/vmware_driver.c | 12 +-
29 files changed, 716 insertions(+), 240 deletions(-)
--
1.8.1.4
11:57 a.m.
New subject: [libvirt] [PATCH 1/8] Add access control filtering of domain objects
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list domain objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/conf/domain_conf.c | 91 +++++++++++++++++++++++++++-------------
src/conf/domain_conf.h | 17 ++++++--
src/libxl/libxl_driver.c | 15 ++++---
src/lxc/lxc_driver.c | 15 ++++---
src/openvz/openvz_driver.c | 7 ++--
src/parallels/parallels_driver.c | 14 ++++---
src/qemu/qemu_driver.c | 24 ++++++-----
src/rpc/gendispatch.pl | 42 ++++++++++++-------
src/test/test_driver.c | 13 +++---
src/uml/uml_driver.c | 15 ++++---
src/vmware/vmware_driver.c | 12 +++---
11 files changed, 172 insertions(+), 93 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e41dfa2..f5cfb26 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16984,47 +16984,51 @@ virDomainGetRootFilesystem(virDomainDefPtr def)
}
-static void
-virDomainObjListCountActive(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *data)
-{
- virDomainObjPtr obj = payload;
- int *count = data;
- virObjectLock(obj);
- if (virDomainObjIsActive(obj))
- (*count)++;
- virObjectUnlock(obj);
-}
+struct virDomainObjListData {
+ virDomainObjListFilter filter;
+ virConnectPtr conn;
+ bool active;
+ int count;
+};
static void
-virDomainObjListCountInactive(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *data)
+virDomainObjListCount(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
{
virDomainObjPtr obj = payload;
- int *count = data;
+ struct virDomainObjListData *data = opaque;
virObjectLock(obj);
- if (!virDomainObjIsActive(obj))
- (*count)++;
+ if (data->filter &&
+ !data->filter(data->conn, obj->def))
+ goto cleanup;
+ if (virDomainObjIsActive(obj)) {
+ if (data->active)
+ data->count++;
+ } else {
+ if (!data->active)
+ data->count++;
+ }
+cleanup:
virObjectUnlock(obj);
}
int
virDomainObjListNumOfDomains(virDomainObjListPtr doms,
- int active)
+ bool active,
+ virDomainObjListFilter filter,
+ virConnectPtr conn)
{
- int count = 0;
+ struct virDomainObjListData data = { filter, conn, active, 0 };
virObjectLock(doms);
- if (active)
- virHashForEach(doms->objs, virDomainObjListCountActive, &count);
- else
- virHashForEach(doms->objs, virDomainObjListCountInactive, &count);
+ virHashForEach(doms->objs, virDomainObjListCount, &data);
virObjectUnlock(doms);
- return count;
+ return data.count;
}
struct virDomainIDData {
+ virDomainObjListFilter filter;
+ virConnectPtr conn;
int numids;
int maxids;
int *ids;
@@ -17038,17 +17042,24 @@ virDomainObjListCopyActiveIDs(void *payload,
virDomainObjPtr obj = payload;
struct virDomainIDData *data = opaque;
virObjectLock(obj);
+ if (data->filter &&
+ !data->filter(data->conn, obj->def))
+ goto cleanup;
if (virDomainObjIsActive(obj) && data->numids < data->maxids)
data->ids[data->numids++] = obj->def->id;
+cleanup:
virObjectUnlock(obj);
}
int
virDomainObjListGetActiveIDs(virDomainObjListPtr doms,
int *ids,
- int maxids)
+ int maxids,
+ virDomainObjListFilter filter,
+ virConnectPtr conn)
{
- struct virDomainIDData data = { 0, maxids, ids };
+ struct virDomainIDData data = { filter, conn,
+ 0, maxids, ids };
virObjectLock(doms);
virHashForEach(doms->objs, virDomainObjListCopyActiveIDs, &data);
virObjectUnlock(doms);
@@ -17056,6 +17067,8 @@ virDomainObjListGetActiveIDs(virDomainObjListPtr doms,
}
struct virDomainNameData {
+ virDomainObjListFilter filter;
+ virConnectPtr conn;
int oom;
int numnames;
int maxnames;
@@ -17074,12 +17087,16 @@ virDomainObjListCopyInactiveNames(void *payload,
return;
virObjectLock(obj);
+ if (data->filter &&
+ !data->filter(data->conn, obj->def))
+ goto cleanup;
if (!virDomainObjIsActive(obj) && data->numnames < data->maxnames)
{
if (VIR_STRDUP(data->names[data->numnames], obj->def->name) < 0)
data->oom = 1;
else
data->numnames++;
}
+cleanup:
virObjectUnlock(obj);
}
@@ -17087,9 +17104,12 @@ virDomainObjListCopyInactiveNames(void *payload,
int
virDomainObjListGetInactiveNames(virDomainObjListPtr doms,
char **const names,
- int maxnames)
+ int maxnames,
+ virDomainObjListFilter filter,
+ virConnectPtr conn)
{
- struct virDomainNameData data = { 0, 0, maxnames, names };
+ struct virDomainNameData data = { filter, conn,
+ 0, 0, maxnames, names };
int i;
virObjectLock(doms);
virHashForEach(doms->objs, virDomainObjListCopyInactiveNames, &data);
@@ -17805,6 +17825,7 @@ cleanup:
struct virDomainListData {
virConnectPtr conn;
virDomainPtr *domains;
+ virDomainObjListFilter filter;
unsigned int flags;
int ndomains;
bool error;
@@ -17826,6 +17847,11 @@ virDomainListPopulate(void *payload,
virObjectLock(vm);
/* check if the domain matches the filter */
+ /* filter by the callback function (access control checks) */
+ if (data->filter != NULL &&
+ !data->filter(data->conn, vm->def))
+ goto cleanup;
+
/* filter by active state */
if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) &&
!((MATCH(VIR_CONNECT_LIST_DOMAINS_ACTIVE) &&
@@ -17905,12 +17931,17 @@ int
virDomainObjListExport(virDomainObjListPtr doms,
virConnectPtr conn,
virDomainPtr **domains,
+ virDomainObjListFilter filter,
unsigned int flags)
{
int ret = -1;
int i;
- struct virDomainListData data = { conn, NULL, flags, 0, false };
+ struct virDomainListData data = {
+ conn, NULL,
+ filter,
+ flags, 0, false
+ };
virObjectLock(doms);
if (domains) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3817e37..7310c1c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2051,6 +2051,9 @@ struct _virDomainObj {
typedef struct _virDomainObjList virDomainObjList;
typedef virDomainObjList *virDomainObjListPtr;
+typedef bool (*virDomainObjListFilter)(virConnectPtr conn,
+ virDomainDefPtr def);
+
/* This structure holds various callbacks and data needed
* while parsing and creating domain XMLs */
@@ -2404,14 +2407,21 @@ int virDomainFSIndexByName(virDomainDefPtr def, const char
*name);
int virDomainVideoDefaultType(virDomainDefPtr def);
int virDomainVideoDefaultRAM(virDomainDefPtr def, int type);
-int virDomainObjListNumOfDomains(virDomainObjListPtr doms, int active);
+int virDomainObjListNumOfDomains(virDomainObjListPtr doms,
+ bool active,
+ virDomainObjListFilter filter,
+ virConnectPtr conn);
int virDomainObjListGetActiveIDs(virDomainObjListPtr doms,
int *ids,
- int maxids);
+ int maxids,
+ virDomainObjListFilter filter,
+ virConnectPtr conn);
int virDomainObjListGetInactiveNames(virDomainObjListPtr doms,
char **const names,
- int maxnames);
+ int maxnames,
+ virDomainObjListFilter filter,
+ virConnectPtr conn);
typedef int (*virDomainObjListIterator)(virDomainObjPtr dom,
void *opaque);
@@ -2621,6 +2631,7 @@ VIR_ENUM_DECL(virDomainStartupPolicy)
int virDomainObjListExport(virDomainObjListPtr doms,
virConnectPtr conn,
virDomainPtr **domains,
+ virDomainObjListFilter filter,
unsigned int flags);
virDomainVcpuPinDefPtr virDomainLookupVcpuPin(virDomainDefPtr def,
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9f52394..eee42fa 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1574,7 +1574,8 @@ libxlConnectListDomains(virConnectPtr conn, int *ids, int nids)
return -1;
libxlDriverLock(driver);
- n = virDomainObjListGetActiveIDs(driver->domains, ids, nids);
+ n = virDomainObjListGetActiveIDs(driver->domains, ids, nids,
+ virConnectListDomainsCheckACL, conn);
libxlDriverUnlock(driver);
return n;
@@ -1590,7 +1591,8 @@ libxlConnectNumOfDomains(virConnectPtr conn)
return -1;
libxlDriverLock(driver);
- n = virDomainObjListNumOfDomains(driver->domains, 1);
+ n = virDomainObjListNumOfDomains(driver->domains, true,
+ virConnectNumOfDomainsCheckACL, conn);
libxlDriverUnlock(driver);
return n;
@@ -3202,7 +3204,8 @@ libxlConnectListDefinedDomains(virConnectPtr conn,
return -1;
libxlDriverLock(driver);
- n = virDomainObjListGetInactiveNames(driver->domains, names, nnames);
+ n = virDomainObjListGetInactiveNames(driver->domains, names, nnames,
+ virConnectListDefinedDomainsCheckACL, conn);
libxlDriverUnlock(driver);
return n;
}
@@ -3217,7 +3220,8 @@ libxlConnectNumOfDefinedDomains(virConnectPtr conn)
return -1;
libxlDriverLock(driver);
- n = virDomainObjListNumOfDomains(driver->domains, 0);
+ n = virDomainObjListNumOfDomains(driver->domains, false,
+ virConnectNumOfDefinedDomainsCheckACL, NULL);
libxlDriverUnlock(driver);
return n;
@@ -4609,7 +4613,8 @@ libxlConnectListAllDomains(virConnectPtr conn,
return -1;
libxlDriverLock(driver);
- ret = virDomainObjListExport(driver->domains, conn, domains, flags);
+ ret = virDomainObjListExport(driver->domains, conn, domains,
+ virConnectListAllDomainsCheckACL, flags);
libxlDriverUnlock(driver);
return ret;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 8d02c52..1a6d086 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -392,7 +392,8 @@ static int lxcConnectListDomains(virConnectPtr conn, int *ids, int
nids) {
return -1;
lxcDriverLock(driver);
- n = virDomainObjListGetActiveIDs(driver->domains, ids, nids);
+ n = virDomainObjListGetActiveIDs(driver->domains, ids, nids,
+ virConnectListDomainsCheckACL, conn);
lxcDriverUnlock(driver);
return n;
@@ -406,7 +407,8 @@ static int lxcConnectNumOfDomains(virConnectPtr conn) {
return -1;
lxcDriverLock(driver);
- n = virDomainObjListNumOfDomains(driver->domains, 1);
+ n = virDomainObjListNumOfDomains(driver->domains, true,
+ virConnectNumOfDomainsCheckACL, conn);
lxcDriverUnlock(driver);
return n;
@@ -421,7 +423,8 @@ static int lxcConnectListDefinedDomains(virConnectPtr conn,
return -1;
lxcDriverLock(driver);
- n = virDomainObjListGetInactiveNames(driver->domains, names, nnames);
+ n = virDomainObjListGetInactiveNames(driver->domains, names, nnames,
+ virConnectListDefinedDomainsCheckACL, conn);
lxcDriverUnlock(driver);
return n;
@@ -436,7 +439,8 @@ static int lxcConnectNumOfDefinedDomains(virConnectPtr conn) {
return -1;
lxcDriverLock(driver);
- n = virDomainObjListNumOfDomains(driver->domains, 0);
+ n = virDomainObjListNumOfDomains(driver->domains, false,
+ virConnectNumOfDefinedDomainsCheckACL, conn);
lxcDriverUnlock(driver);
return n;
@@ -2829,7 +2833,8 @@ lxcConnectListAllDomains(virConnectPtr conn,
return -1;
lxcDriverLock(driver);
- ret = virDomainObjListExport(driver->domains, conn, domains, flags);
+ ret = virDomainObjListExport(driver->domains, conn, domains,
+ virConnectListAllDomainsCheckACL, flags);
lxcDriverUnlock(driver);
return ret;
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index d04e3ba..7af0349 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1566,7 +1566,7 @@ static int openvzConnectNumOfDomains(virConnectPtr conn) {
int n;
openvzDriverLock(driver);
- n = virDomainObjListNumOfDomains(driver->domains, 1);
+ n = virDomainObjListNumOfDomains(driver->domains, true, NULL, NULL);
openvzDriverUnlock(driver);
return n;
@@ -1678,7 +1678,7 @@ static int openvzConnectNumOfDefinedDomains(virConnectPtr conn) {
int n;
openvzDriverLock(driver);
- n = virDomainObjListNumOfDomains(driver->domains, 0);
+ n = virDomainObjListNumOfDomains(driver->domains, false, NULL, NULL);
openvzDriverUnlock(driver);
return n;
@@ -2122,7 +2122,8 @@ openvzConnectListAllDomains(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1);
openvzDriverLock(driver);
- ret = virDomainObjListExport(driver->domains, conn, domains, flags);
+ ret = virDomainObjListExport(driver->domains, conn, domains,
+ NULL, flags);
openvzDriverUnlock(driver);
return ret;
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index b7c4ec4..d5e0ea3 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -1045,7 +1045,8 @@ parallelsConnectListDomains(virConnectPtr conn, int *ids, int
maxids)
int n;
parallelsDriverLock(privconn);
- n = virDomainObjListGetActiveIDs(privconn->domains, ids, maxids);
+ n = virDomainObjListGetActiveIDs(privconn->domains, ids, maxids,
+ NULL, NULL);
parallelsDriverUnlock(privconn);
return n;
@@ -1058,7 +1059,8 @@ parallelsConnectNumOfDomains(virConnectPtr conn)
int count;
parallelsDriverLock(privconn);
- count = virDomainObjListNumOfDomains(privconn->domains, 1);
+ count = virDomainObjListNumOfDomains(privconn->domains, true,
+ NULL, NULL);
parallelsDriverUnlock(privconn);
return count;
@@ -1073,7 +1075,7 @@ parallelsConnectListDefinedDomains(virConnectPtr conn, char **const
names, int m
parallelsDriverLock(privconn);
memset(names, 0, sizeof(*names) * maxnames);
n = virDomainObjListGetInactiveNames(privconn->domains, names,
- maxnames);
+ maxnames, NULL, NULL);
parallelsDriverUnlock(privconn);
return n;
@@ -1086,7 +1088,8 @@ parallelsConnectNumOfDefinedDomains(virConnectPtr conn)
int count;
parallelsDriverLock(privconn);
- count = virDomainObjListNumOfDomains(privconn->domains, 0);
+ count = virDomainObjListNumOfDomains(privconn->domains, false,
+ NULL, NULL);
parallelsDriverUnlock(privconn);
return count;
@@ -1102,7 +1105,8 @@ parallelsConnectListAllDomains(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1);
parallelsDriverLock(privconn);
- ret = virDomainObjListExport(privconn->domains, conn, domains, flags);
+ ret = virDomainObjListExport(privconn->domains, conn, domains,
+ NULL, flags);
parallelsDriverUnlock(privconn);
return ret;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f51e766..593f532 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1484,7 +1484,8 @@ static int qemuConnectListDomains(virConnectPtr conn, int *ids, int
nids) {
if (virConnectListDomainsEnsureACL(conn) < 0)
return -1;
- n = virDomainObjListGetActiveIDs(driver->domains, ids, nids);
+ n = virDomainObjListGetActiveIDs(driver->domains, ids, nids,
+ virConnectListDomainsCheckACL, conn);
return n;
}
@@ -1496,7 +1497,8 @@ static int qemuConnectNumOfDomains(virConnectPtr conn) {
if (virConnectNumOfDomainsEnsureACL(conn) < 0)
return -1;
- n = virDomainObjListNumOfDomains(driver->domains, 1);
+ n = virDomainObjListNumOfDomains(driver->domains, true,
+ virConnectNumOfDomainsCheckACL, conn);
return n;
}
@@ -5711,7 +5713,8 @@ static int qemuConnectListDefinedDomains(virConnectPtr conn,
if (virConnectListDefinedDomainsEnsureACL(conn) < 0)
goto cleanup;
- ret = virDomainObjListGetInactiveNames(driver->domains, names, nnames);
+ ret = virDomainObjListGetInactiveNames(driver->domains, names, nnames,
+ virConnectListDefinedDomainsCheckACL, NULL);
cleanup:
return ret;
@@ -5724,7 +5727,8 @@ static int qemuConnectNumOfDefinedDomains(virConnectPtr conn) {
if (virConnectNumOfDefinedDomainsEnsureACL(conn) < 0)
goto cleanup;
- ret = virDomainObjListNumOfDomains(driver->domains, 0);
+ ret = virDomainObjListNumOfDomains(driver->domains, false,
+ virConnectNumOfDefinedDomainsCheckACL, NULL);
cleanup:
return ret;
@@ -12654,8 +12658,7 @@ static int qemuDomainSnapshotListNames(virDomainPtr domain, char
**names,
if (virDomainSnapshotListNamesEnsureACL(domain->conn, vm->def) < 0)
goto cleanup;
- n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen,
- flags);
+ n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen,
flags);
cleanup:
if (vm)
@@ -12732,8 +12735,7 @@ qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr
snapshot,
if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
goto cleanup;
- n = virDomainSnapshotObjListGetNames(vm->snapshots, snap, names, nameslen,
- flags);
+ n = virDomainSnapshotObjListGetNames(vm->snapshots, snap, names, nameslen,
flags);
cleanup:
if (vm)
@@ -12790,8 +12792,7 @@ qemuDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot,
if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
goto cleanup;
- n = virDomainListSnapshots(vm->snapshots, snap, snapshot->domain, snaps,
- flags);
+ n = virDomainListSnapshots(vm->snapshots, snap, snapshot->domain, snaps,
flags);
cleanup:
if (vm)
@@ -15614,7 +15615,8 @@ qemuConnectListAllDomains(virConnectPtr conn,
if (virConnectListAllDomainsEnsureACL(conn) < 0)
goto cleanup;
- ret = virDomainObjListExport(driver->domains, conn, domains, flags);
+ ret = virDomainObjListExport(driver->domains, conn, domains,
+ virConnectListAllDomainsCheckACL, flags);
cleanup:
return ret;
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index ff15474..fdf5a79 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -1762,8 +1762,21 @@ elsif ($mode eq "client") {
push @argdecls, "unsigned int flags";
}
+ my $ret;
+ my $pass;
+ my $fail;
+ if ($action eq "Check") {
+ $ret = "bool";
+ $pass = "true";
+ $fail = "false";
+ } else {
+ $ret = "int";
+ $pass = "0";
+ $fail = "-1";
+ }
+
if ($mode eq "aclheader") {
- print "extern int $apiname(" . join(", ", @argdecls)
. ");\n";
+ print "extern $ret $apiname(" . join(", ", @argdecls)
. ");\n";
} else {
my @argvars;
push @argvars, "mgr";
@@ -1775,18 +1788,18 @@ elsif ($mode eq "client") {
push @argvars, $arg;
}
- if ($action eq "Check") {
- print "/* Returns: -1 on error, 0 on denied, 1 on allowed
*/\n";
- } else {
- print "/* Returns: -1 on error (denied==error), 0 on allowed
*/\n";
- }
- print "int $apiname(" . join(", ", @argdecls) .
")\n";
+ print "/* Returns: $fail on error/denied, $pass on allowed
*/\n";
+ print "$ret $apiname(" . join(", ", @argdecls) .
")\n";
print "{\n";
print " virAccessManagerPtr mgr;\n";
print " int rv;\n";
print "\n";
- print " if (!(mgr = virAccessManagerGetDefault()))\n";
- print " return -1;\n";
+ print " if (!(mgr = virAccessManagerGetDefault())) {\n";
+ if ($action eq "Check") {
+ print " virResetLastError();\n";
+ }
+ print " return $fail;\n";
+ print " }\n";
print "\n";
foreach my $acl (@acl) {
@@ -1811,20 +1824,17 @@ elsif ($mode eq "client") {
if ($action eq "Ensure") {
print " if (rv == 0)\n";
print " virReportError(VIR_ERR_ACCESS_DENIED,
NULL);\n";
- print " return -1;\n";
+ print " return $fail;\n";
} else {
- print " return rv;\n";
+ print " virResetLastError();\n";
+ print " return $fail;\n";
}
print " }";
print "\n";
}
print " virObjectUnref(mgr);\n";
- if ($action eq "Check") {
- print " return 1;\n";
- } else {
- print " return 0;\n";
- }
+ print " return $pass;\n";
print "}\n\n";
}
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 30c2194..88e23a3 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1274,7 +1274,7 @@ static int testConnectNumOfDomains(virConnectPtr conn)
int count;
testDriverLock(privconn);
- count = virDomainObjListNumOfDomains(privconn->domains, 1);
+ count = virDomainObjListNumOfDomains(privconn->domains, true, NULL, NULL);
testDriverUnlock(privconn);
return count;
@@ -1463,7 +1463,7 @@ static int testConnectListDomains(virConnectPtr conn,
int n;
testDriverLock(privconn);
- n = virDomainObjListGetActiveIDs(privconn->domains, ids, maxids);
+ n = virDomainObjListGetActiveIDs(privconn->domains, ids, maxids, NULL, NULL);
testDriverUnlock(privconn);
return n;
@@ -2475,7 +2475,7 @@ static int testConnectNumOfDefinedDomains(virConnectPtr conn) {
int count;
testDriverLock(privconn);
- count = virDomainObjListNumOfDomains(privconn->domains, 0);
+ count = virDomainObjListNumOfDomains(privconn->domains, false, NULL, NULL);
testDriverUnlock(privconn);
return count;
@@ -2490,7 +2490,8 @@ static int testConnectListDefinedDomains(virConnectPtr conn,
testDriverLock(privconn);
memset(names, 0, sizeof(*names)*maxnames);
- n = virDomainObjListGetInactiveNames(privconn->domains, names, maxnames);
+ n = virDomainObjListGetInactiveNames(privconn->domains, names, maxnames,
+ NULL, NULL);
testDriverUnlock(privconn);
return n;
@@ -5688,6 +5689,7 @@ static int testNWFilterClose(virConnectPtr conn) {
return 0;
}
+
static int testConnectListAllDomains(virConnectPtr conn,
virDomainPtr **domains,
unsigned int flags)
@@ -5698,7 +5700,8 @@ static int testConnectListAllDomains(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1);
testDriverLock(privconn);
- ret = virDomainObjListExport(privconn->domains, conn, domains, flags);
+ ret = virDomainObjListExport(privconn->domains, conn, domains,
+ NULL, flags);
testDriverUnlock(privconn);
return ret;
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 25b9748..df98eb8 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1545,7 +1545,8 @@ static int umlConnectListDomains(virConnectPtr conn, int *ids, int
nids) {
return -1;
umlDriverLock(driver);
- n = virDomainObjListGetActiveIDs(driver->domains, ids, nids);
+ n = virDomainObjListGetActiveIDs(driver->domains, ids, nids,
+ virConnectListDomainsCheckACL, conn);
umlDriverUnlock(driver);
return n;
@@ -1558,7 +1559,8 @@ static int umlConnectNumOfDomains(virConnectPtr conn) {
return -1;
umlDriverLock(driver);
- n = virDomainObjListNumOfDomains(driver->domains, 1);
+ n = virDomainObjListNumOfDomains(driver->domains, true,
+ virConnectNumOfDomainsCheckACL, conn);
umlDriverUnlock(driver);
return n;
@@ -1965,7 +1967,8 @@ static int umlConnectListDefinedDomains(virConnectPtr conn,
return -1;
umlDriverLock(driver);
- n = virDomainObjListGetInactiveNames(driver->domains, names, nnames);
+ n = virDomainObjListGetInactiveNames(driver->domains, names, nnames,
+ virConnectListDefinedDomainsCheckACL, conn);
umlDriverUnlock(driver);
return n;
@@ -1979,7 +1982,8 @@ static int umlConnectNumOfDefinedDomains(virConnectPtr conn) {
return -1;
umlDriverLock(driver);
- n = virDomainObjListNumOfDomains(driver->domains, 0);
+ n = virDomainObjListNumOfDomains(driver->domains, false,
+ virConnectNumOfDefinedDomainsCheckACL, conn);
umlDriverUnlock(driver);
return n;
@@ -2710,7 +2714,8 @@ static int umlConnectListAllDomains(virConnectPtr conn,
return -1;
umlDriverLock(driver);
- ret = virDomainObjListExport(driver->domains, conn, domains, flags);
+ ret = virDomainObjListExport(driver->domains, conn, domains,
+ virConnectListAllDomainsCheckACL, flags);
umlDriverUnlock(driver);
return ret;
diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index 8a3fc99..ca6615f 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -987,7 +987,7 @@ vmwareConnectNumOfDefinedDomains(virConnectPtr conn)
vmwareDriverLock(driver);
vmwareDomainObjListUpdateAll(driver->domains, driver);
- n = virDomainObjListNumOfDomains(driver->domains, 0);
+ n = virDomainObjListNumOfDomains(driver->domains, false, NULL, NULL);
vmwareDriverUnlock(driver);
return n;
@@ -1001,7 +1001,7 @@ vmwareConnectNumOfDomains(virConnectPtr conn)
vmwareDriverLock(driver);
vmwareDomainObjListUpdateAll(driver->domains, driver);
- n = virDomainObjListNumOfDomains(driver->domains, 1);
+ n = virDomainObjListNumOfDomains(driver->domains, true, NULL, NULL);
vmwareDriverUnlock(driver);
return n;
@@ -1016,7 +1016,7 @@ vmwareConnectListDomains(virConnectPtr conn, int *ids, int nids)
vmwareDriverLock(driver);
vmwareDomainObjListUpdateAll(driver->domains, driver);
- n = virDomainObjListGetActiveIDs(driver->domains, ids, nids);
+ n = virDomainObjListGetActiveIDs(driver->domains, ids, nids, NULL, NULL);
vmwareDriverUnlock(driver);
return n;
@@ -1031,7 +1031,8 @@ vmwareConnectListDefinedDomains(virConnectPtr conn,
vmwareDriverLock(driver);
vmwareDomainObjListUpdateAll(driver->domains, driver);
- n = virDomainObjListGetInactiveNames(driver->domains, names, nnames);
+ n = virDomainObjListGetInactiveNames(driver->domains, names, nnames,
+ NULL, NULL);
vmwareDriverUnlock(driver);
return n;
}
@@ -1121,7 +1122,8 @@ vmwareConnectListAllDomains(virConnectPtr conn,
vmwareDriverLock(driver);
vmwareDomainObjListUpdateAll(driver->domains, driver);
- ret = virDomainObjListExport(driver->domains, conn, domains, flags);
+ ret = virDomainObjListExport(driver->domains, conn, domains,
+ NULL, flags);
vmwareDriverUnlock(driver);
return ret;
}
--
1.8.1.4
2:17 p.m.
New subject: [libvirt] [PATCH 1/8] Add access control filtering of domain objects
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list domain objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/conf/domain_conf.c | 91 +++++++++++++++++++++++++++-------------
src/conf/domain_conf.h | 17 ++++++--
src/libxl/libxl_driver.c | 15 ++++---
src/lxc/lxc_driver.c | 15 ++++---
src/openvz/openvz_driver.c | 7 ++--
src/parallels/parallels_driver.c | 14 ++++---
src/qemu/qemu_driver.c | 24 ++++++-----
src/rpc/gendispatch.pl | 42 ++++++++++++-------
src/test/test_driver.c | 13 +++---
src/uml/uml_driver.c | 15 ++++---
src/vmware/vmware_driver.c | 12 +++---
11 files changed, 172 insertions(+), 93 deletions(-)
Big, but that's to be expected given our track record of multiple
listing APIs.
int
virDomainObjListNumOfDomains(virDomainObjListPtr doms,
- int active)
+ bool active,
Nice conversion of int->bool on something that was already used as a bool.
@@ -12654,8 +12658,7 @@ static int
qemuDomainSnapshotListNames(virDomainPtr domain, char **names,
if (virDomainSnapshotListNamesEnsureACL(domain->conn, vm->def) < 0)
goto cleanup;
- n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen,
- flags);
+ n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen,
flags);
A bit of spurious reformatting here; you touch the same code later in
the series, so you may want to rebase to avoid the churn, although I
won't insist.
@@ -12732,8 +12735,7 @@
qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
goto cleanup;
- n = virDomainSnapshotObjListGetNames(vm->snapshots, snap, names, nameslen,
- flags);
+ n = virDomainSnapshotObjListGetNames(vm->snapshots, snap, names, nameslen,
flags);
and again
cleanup:
if (vm)
@@ -12790,8 +12792,7 @@ qemuDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot,
if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
goto cleanup;
- n = virDomainListSnapshots(vm->snapshots, snap, snapshot->domain, snaps,
- flags);
+ n = virDomainListSnapshots(vm->snapshots, snap, snapshot->domain, snaps,
flags);
Here too. :)
@@ -1775,18 +1788,18 @@ elsif ($mode eq "client") {
push @argvars, $arg;
}
- if ($action eq "Check") {
- print "/* Returns: -1 on error, 0 on denied, 1 on allowed
*/\n";
- } else {
- print "/* Returns: -1 on error (denied==error), 0 on allowed
*/\n";
- }
- print "int $apiname(" . join(", ", @argdecls) .
")\n";
+ print "/* Returns: $fail on error/denied, $pass on allowed
*/\n";
+ print "$ret $apiname(" . join(", ", @argdecls) .
")\n";
This is a semantic change for Check functions, but I can agree with it
(in the security world, telling a person that they were denied due to
security as opposed to some other reason is sometimes in itself a
security hole due to the information leak). But is it worth splitting
into a separate patch, since it is an independent improvement used by
all of the rest of the series, and not something fundamental to listing
domains?
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:23 a.m.
New subject: [libvirt] [PATCH 1/8] Add access control filtering of domain objects
On Tue, Jul 02, 2013 at 01:17:44PM -0600, Eric Blake wrote:
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> Ensure that all APIs which list domain objects filter
> them against the access control system.
>
> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> ---
> src/conf/domain_conf.c | 91 +++++++++++++++++++++++++++-------------
> src/conf/domain_conf.h | 17 ++++++--
> src/libxl/libxl_driver.c | 15 ++++---
> src/lxc/lxc_driver.c | 15 ++++---
> src/openvz/openvz_driver.c | 7 ++--
> src/parallels/parallels_driver.c | 14 ++++---
> src/qemu/qemu_driver.c | 24 ++++++-----
> src/rpc/gendispatch.pl | 42 ++++++++++++-------
> src/test/test_driver.c | 13 +++---
> src/uml/uml_driver.c | 15 ++++---
> src/vmware/vmware_driver.c | 12 +++---
> 11 files changed, 172 insertions(+), 93 deletions(-)
Big, but that's to be expected given our track record of multiple
listing APIs.
>
> int
> virDomainObjListNumOfDomains(virDomainObjListPtr doms,
> - int active)
> + bool active,
Nice conversion of int->bool on something that was already used as a bool.
> @@ -12654,8 +12658,7 @@ static int qemuDomainSnapshotListNames(virDomainPtr domain,
char **names,
> if (virDomainSnapshotListNamesEnsureACL(domain->conn, vm->def) < 0)
> goto cleanup;
>
> - n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen,
> - flags);
> + n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen,
flags);
A bit of spurious reformatting here; you touch the same code later in
the series, so you may want to rebase to avoid the churn, although I
won't insist.
> @@ -12732,8 +12735,7 @@ qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr
snapshot,
> if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
> goto cleanup;
>
> - n = virDomainSnapshotObjListGetNames(vm->snapshots, snap, names, nameslen,
> - flags);
> + n = virDomainSnapshotObjListGetNames(vm->snapshots, snap, names, nameslen,
flags);
and again
>
> cleanup:
> if (vm)
> @@ -12790,8 +12792,7 @@ qemuDomainSnapshotListAllChildren(virDomainSnapshotPtr
snapshot,
> if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
> goto cleanup;
>
> - n = virDomainListSnapshots(vm->snapshots, snap, snapshot->domain, snaps,
> - flags);
> + n = virDomainListSnapshots(vm->snapshots, snap, snapshot->domain, snaps,
flags);
Here too. :)
Opps, left over from code I backed out.
> @@ -1775,18 +1788,18 @@ elsif ($mode eq "client") {
> push @argvars, $arg;
> }
>
> - if ($action eq "Check") {
> - print "/* Returns: -1 on error, 0 on denied, 1 on allowed
*/\n";
> - } else {
> - print "/* Returns: -1 on error (denied==error), 0 on
allowed */\n";
> - }
> - print "int $apiname(" . join(", ", @argdecls) .
")\n";
> + print "/* Returns: $fail on error/denied, $pass on allowed
*/\n";
> + print "$ret $apiname(" . join(", ", @argdecls)
. ")\n";
This is a semantic change for Check functions, but I can agree with it
(in the security world, telling a person that they were denied due to
security as opposed to some other reason is sometimes in itself a
security hole due to the information leak). But is it worth splitting
into a separate patch, since it is an independent improvement used by
all of the rest of the series, and not something fundamental to listing
domains?
Yeah should be moved to a dedicated patch.
ACK.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:57 a.m.
New subject: [libvirt] [PATCH 2/8] Add access control filtering of network objects
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list network objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/conf/network_conf.c | 12 ++++++-----
src/conf/network_conf.h | 13 ++++++++----
src/libvirt_private.syms | 2 +-
src/network/bridge_driver.c | 44 ++++++++++++++++++++++++---------------
src/parallels/parallels_network.c | 2 +-
src/test/test_driver.c | 2 +-
6 files changed, 46 insertions(+), 29 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 2b4845c..64fd581 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -4289,10 +4289,11 @@ virNetworkMatch(virNetworkObjPtr netobj,
#undef MATCH
int
-virNetworkList(virConnectPtr conn,
- virNetworkObjList netobjs,
- virNetworkPtr **nets,
- unsigned int flags)
+virNetworkObjListExport(virConnectPtr conn,
+ virNetworkObjList netobjs,
+ virNetworkPtr **nets,
+ virNetworkObjListFilter filter,
+ unsigned int flags)
{
virNetworkPtr *tmp_nets = NULL;
virNetworkPtr net = NULL;
@@ -4310,7 +4311,8 @@ virNetworkList(virConnectPtr conn,
for (i = 0; i < netobjs.count; i++) {
virNetworkObjPtr netobj = netobjs.objs[i];
virNetworkObjLock(netobj);
- if (virNetworkMatch(netobj, flags)) {
+ if ((!filter || filter(conn, netobj->def)) &&
+ virNetworkMatch(netobj, flags)) {
if (nets) {
if (!(net = virGetNetwork(conn,
netobj->def->name,
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 43f80d4..a1d3282 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -296,6 +296,10 @@ void virNetworkDefFree(virNetworkDefPtr def);
void virNetworkObjFree(virNetworkObjPtr net);
void virNetworkObjListFree(virNetworkObjListPtr vms);
+
+typedef bool (*virNetworkObjListFilter)(virConnectPtr conn,
+ virNetworkDefPtr def);
+
virNetworkObjPtr virNetworkAssignDef(virNetworkObjListPtr nets,
const virNetworkDefPtr def,
bool live);
@@ -417,9 +421,10 @@ VIR_ENUM_DECL(virNetworkForward)
VIR_CONNECT_LIST_NETWORKS_FILTERS_PERSISTENT | \
VIR_CONNECT_LIST_NETWORKS_FILTERS_AUTOSTART)
-int virNetworkList(virConnectPtr conn,
- virNetworkObjList netobjs,
- virNetworkPtr **nets,
- unsigned int flags);
+int virNetworkObjListExport(virConnectPtr conn,
+ virNetworkObjList netobjs,
+ virNetworkPtr **nets,
+ virNetworkObjListFilter filter,
+ unsigned int flags);
#endif /* __NETWORK_CONF_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f08ac64..bd52b3d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -495,13 +495,13 @@ virNetworkFindByUUID;
virNetworkForwardTypeToString;
virNetworkIpDefNetmask;
virNetworkIpDefPrefix;
-virNetworkList;
virNetworkLoadAllConfigs;
virNetworkLoadAllState;
virNetworkObjAssignDef;
virNetworkObjFree;
virNetworkObjGetPersistentDef;
virNetworkObjIsDuplicate;
+virNetworkObjListExport;
virNetworkObjListFree;
virNetworkObjLock;
virNetworkObjReplacePersistentDef;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index fb1741f..742b492 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2899,10 +2899,12 @@ static int networkConnectNumOfNetworks(virConnectPtr conn) {
networkDriverLock(driver);
for (i = 0; i < driver->networks.count; i++) {
- virNetworkObjLock(driver->networks.objs[i]);
- if (virNetworkObjIsActive(driver->networks.objs[i]))
+ virNetworkObjPtr obj = driver->networks.objs[i];
+ virNetworkObjLock(obj);
+ if (virConnectNumOfNetworksCheckACL(conn, obj->def) &&
+ virNetworkObjIsActive(obj))
nactive++;
- virNetworkObjUnlock(driver->networks.objs[i]);
+ virNetworkObjUnlock(obj);
}
networkDriverUnlock(driver);
@@ -2918,15 +2920,17 @@ static int networkConnectListNetworks(virConnectPtr conn, char
**const names, in
networkDriverLock(driver);
for (i = 0; i < driver->networks.count && got < nnames; i++) {
- virNetworkObjLock(driver->networks.objs[i]);
- if (virNetworkObjIsActive(driver->networks.objs[i])) {
- if (VIR_STRDUP(names[got], driver->networks.objs[i]->def->name) <
0) {
- virNetworkObjUnlock(driver->networks.objs[i]);
+ virNetworkObjPtr obj = driver->networks.objs[i];
+ virNetworkObjLock(obj);
+ if (virConnectListNetworksCheckACL(conn, obj->def) &&
+ virNetworkObjIsActive(obj)) {
+ if (VIR_STRDUP(names[got], obj->def->name) < 0) {
+ virNetworkObjUnlock(obj);
goto cleanup;
}
got++;
}
- virNetworkObjUnlock(driver->networks.objs[i]);
+ virNetworkObjUnlock(obj);
}
networkDriverUnlock(driver);
@@ -2948,10 +2952,12 @@ static int networkConnectNumOfDefinedNetworks(virConnectPtr conn)
{
networkDriverLock(driver);
for (i = 0; i < driver->networks.count; i++) {
- virNetworkObjLock(driver->networks.objs[i]);
- if (!virNetworkObjIsActive(driver->networks.objs[i]))
+ virNetworkObjPtr obj = driver->networks.objs[i];
+ virNetworkObjLock(obj);
+ if (virConnectNumOfDefinedNetworksCheckACL(conn, obj->def) &&
+ !virNetworkObjIsActive(obj))
ninactive++;
- virNetworkObjUnlock(driver->networks.objs[i]);
+ virNetworkObjUnlock(obj);
}
networkDriverUnlock(driver);
@@ -2967,15 +2973,17 @@ static int networkConnectListDefinedNetworks(virConnectPtr conn,
char **const na
networkDriverLock(driver);
for (i = 0; i < driver->networks.count && got < nnames; i++) {
- virNetworkObjLock(driver->networks.objs[i]);
- if (!virNetworkObjIsActive(driver->networks.objs[i])) {
- if (VIR_STRDUP(names[got], driver->networks.objs[i]->def->name) <
0) {
- virNetworkObjUnlock(driver->networks.objs[i]);
+ virNetworkObjPtr obj = driver->networks.objs[i];
+ virNetworkObjLock(obj);
+ if (virConnectListDefinedNetworksCheckACL(conn, obj->def) &&
+ !virNetworkObjIsActive(obj)) {
+ if (VIR_STRDUP(names[got], obj->def->name) < 0) {
+ virNetworkObjUnlock(obj);
goto cleanup;
}
got++;
}
- virNetworkObjUnlock(driver->networks.objs[i]);
+ virNetworkObjUnlock(obj);
}
networkDriverUnlock(driver);
return got;
@@ -3001,7 +3009,9 @@ networkConnectListAllNetworks(virConnectPtr conn,
goto cleanup;
networkDriverLock(driver);
- ret = virNetworkList(conn, driver->networks, nets, flags);
+ ret = virNetworkObjListExport(conn, driver->networks, nets,
+ virConnectListAllNetworksCheckACL,
+ flags);
networkDriverUnlock(driver);
cleanup:
diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
index c126e31..26a3f13 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -463,7 +463,7 @@ static int parallelsConnectListAllNetworks(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
parallelsDriverLock(privconn);
- ret = virNetworkList(conn, privconn->networks, nets, flags);
+ ret = virNetworkObjListExport(conn, privconn->networks, nets, NULL, flags);
parallelsDriverUnlock(privconn);
return ret;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 88e23a3..d4c339e 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3092,7 +3092,7 @@ testConnectListAllNetworks(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
testDriverLock(privconn);
- ret = virNetworkList(conn, privconn->networks, nets, flags);
+ ret = virNetworkObjListExport(conn, privconn->networks, nets, NULL, flags);
testDriverUnlock(privconn);
return ret;
--
1.8.1.4
2:36 p.m.
New subject: [libvirt] [PATCH 2/8] Add access control filtering of network objects
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list network objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/conf/network_conf.c | 12 ++++++-----
src/conf/network_conf.h | 13 ++++++++----
src/libvirt_private.syms | 2 +-
src/network/bridge_driver.c | 44 ++++++++++++++++++++++++---------------
src/parallels/parallels_network.c | 2 +-
src/test/test_driver.c | 2 +-
6 files changed, 46 insertions(+), 29 deletions(-)
Gets easier on other objects, after seeing how it was done for domains :)
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:57 a.m.
New subject: [libvirt] [PATCH 3/8] Add access control filtering of node device objects
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list node device objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/conf/node_device_conf.c | 12 +++++++-----
src/conf/node_device_conf.h | 12 ++++++++----
src/node_device/node_device_driver.c | 28 +++++++++++++++++-----------
3 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 96742ef..edcfa1f 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1591,10 +1591,11 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj,
#undef MATCH
int
-virNodeDeviceList(virConnectPtr conn,
- virNodeDeviceObjList devobjs,
- virNodeDevicePtr **devices,
- unsigned int flags)
+virNodeDeviceObjListExport(virConnectPtr conn,
+ virNodeDeviceObjList devobjs,
+ virNodeDevicePtr **devices,
+ virNodeDeviceObjListFilter filter,
+ unsigned int flags)
{
virNodeDevicePtr *tmp_devices = NULL;
virNodeDevicePtr device = NULL;
@@ -1612,7 +1613,8 @@ virNodeDeviceList(virConnectPtr conn,
for (i = 0; i < devobjs.count; i++) {
virNodeDeviceObjPtr devobj = devobjs.objs[i];
virNodeDeviceObjLock(devobj);
- if (virNodeDeviceMatch(devobj, flags)) {
+ if ((!filter || filter(conn, devobj->def)) &&
+ virNodeDeviceMatch(devobj, flags)) {
if (devices) {
if (!(device = virGetNodeDevice(conn,
devobj->def->name))) {
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index ec35da2..1fa61b5 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -280,9 +280,13 @@ void virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj);
VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS | \
VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC)
-int virNodeDeviceList(virConnectPtr conn,
- virNodeDeviceObjList devobjs,
- virNodeDevicePtr **devices,
- unsigned int flags);
+typedef bool (*virNodeDeviceObjListFilter)(virConnectPtr conn,
+ virNodeDeviceDefPtr def);
+
+int virNodeDeviceObjListExport(virConnectPtr conn,
+ virNodeDeviceObjList devobjs,
+ virNodeDevicePtr **devices,
+ virNodeDeviceObjListFilter filter,
+ unsigned int flags);
#endif /* __VIR_NODE_DEVICE_CONF_H__ */
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 67e90a1..1512d26 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -135,11 +135,13 @@ nodeNumOfDevices(virConnectPtr conn,
nodeDeviceLock(driver);
for (i = 0; i < driver->devs.count; i++) {
- virNodeDeviceObjLock(driver->devs.objs[i]);
- if ((cap == NULL) ||
- virNodeDeviceHasCap(driver->devs.objs[i], cap))
+ virNodeDeviceObjPtr obj = driver->devs.objs[i];
+ virNodeDeviceObjLock(obj);
+ if (virNodeNumOfDevicesCheckACL(conn, obj->def) &&
+ ((cap == NULL) ||
+ virNodeDeviceHasCap(obj, cap)))
++ndevs;
- virNodeDeviceObjUnlock(driver->devs.objs[i]);
+ virNodeDeviceObjUnlock(obj);
}
nodeDeviceUnlock(driver);
@@ -163,15 +165,17 @@ nodeListDevices(virConnectPtr conn,
nodeDeviceLock(driver);
for (i = 0; i < driver->devs.count && ndevs < maxnames; i++) {
- virNodeDeviceObjLock(driver->devs.objs[i]);
- if (cap == NULL ||
- virNodeDeviceHasCap(driver->devs.objs[i], cap)) {
- if (VIR_STRDUP(names[ndevs++], driver->devs.objs[i]->def->name) <
0) {
- virNodeDeviceObjUnlock(driver->devs.objs[i]);
+ virNodeDeviceObjPtr obj = driver->devs.objs[i];
+ virNodeDeviceObjLock(obj);
+ if (virNodeListDevicesCheckACL(conn, obj->def) &&
+ (cap == NULL ||
+ virNodeDeviceHasCap(obj, cap))) {
+ if (VIR_STRDUP(names[ndevs++], obj->def->name) < 0) {
+ virNodeDeviceObjUnlock(obj);
goto failure;
}
}
- virNodeDeviceObjUnlock(driver->devs.objs[i]);
+ virNodeDeviceObjUnlock(obj);
}
nodeDeviceUnlock(driver);
@@ -199,7 +203,9 @@ nodeConnectListAllNodeDevices(virConnectPtr conn,
return -1;
nodeDeviceLock(driver);
- ret = virNodeDeviceList(conn, driver->devs, devices, flags);
+ ret = virNodeDeviceObjListExport(conn, driver->devs, devices,
+ virConnectListAllNodeDevicesCheckACL,
+ flags);
nodeDeviceUnlock(driver);
return ret;
}
--
1.8.1.4
3:11 p.m.
New subject: [libvirt] [PATCH 3/8] Add access control filtering of node device objects
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list node device objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/conf/node_device_conf.c | 12 +++++++-----
src/conf/node_device_conf.h | 12 ++++++++----
src/node_device/node_device_driver.c | 28 +++++++++++++++++-----------
3 files changed, 32 insertions(+), 20 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:57 a.m.
New subject: [libvirt] [PATCH 4/8] Add access control filtering of storage objects
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list storage objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/conf/storage_conf.c | 12 +++++----
src/conf/storage_conf.h | 11 +++++---
src/libvirt_private.syms | 4 +--
src/storage/storage_driver.c | 62 +++++++++++++++++++++++++++++---------------
src/test/test_driver.c | 3 ++-
5 files changed, 59 insertions(+), 33 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 288e265..2539c45 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -2203,10 +2203,11 @@ virStoragePoolMatch(virStoragePoolObjPtr poolobj,
#undef MATCH
int
-virStoragePoolList(virConnectPtr conn,
- virStoragePoolObjList poolobjs,
- virStoragePoolPtr **pools,
- unsigned int flags)
+virStoragePoolObjListExport(virConnectPtr conn,
+ virStoragePoolObjList poolobjs,
+ virStoragePoolPtr **pools,
+ virStoragePoolObjListFilter filter,
+ unsigned int flags)
{
virStoragePoolPtr *tmp_pools = NULL;
virStoragePoolPtr pool = NULL;
@@ -2224,7 +2225,8 @@ virStoragePoolList(virConnectPtr conn,
for (i = 0; i < poolobjs.count; i++) {
virStoragePoolObjPtr poolobj = poolobjs.objs[i];
virStoragePoolObjLock(poolobj);
- if (virStoragePoolMatch(poolobj, flags)) {
+ if ((!filter || filter(conn, poolobj->def)) &&
+ virStoragePoolMatch(poolobj, flags)) {
if (pools) {
if (!(pool = virGetStoragePool(conn,
poolobj->def->name,
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 3af59df..c183427 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -357,6 +357,8 @@ struct _virStoragePoolSourceList {
virStoragePoolSourcePtr sources;
};
+typedef bool (*virStoragePoolObjListFilter)(virConnectPtr conn,
+ virStoragePoolDefPtr def);
static inline int
virStoragePoolObjIsActive(virStoragePoolObjPtr pool)
@@ -570,9 +572,10 @@ VIR_ENUM_DECL(virStoragePartedFsType)
VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_AUTOSTART | \
VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE)
-int virStoragePoolList(virConnectPtr conn,
- virStoragePoolObjList poolobjs,
- virStoragePoolPtr **pools,
- unsigned int flags);
+int virStoragePoolObjListExport(virConnectPtr conn,
+ virStoragePoolObjList poolobjs,
+ virStoragePoolPtr **pools,
+ virStoragePoolObjListFilter filter,
+ unsigned int flags);
#endif /* __VIR_STORAGE_CONF_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bd52b3d..4d64a97 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -532,7 +532,7 @@ virNodeDeviceFindBySysfsPath;
virNodeDeviceGetParentHost;
virNodeDeviceGetWWNs;
virNodeDeviceHasCap;
-virNodeDeviceList;
+virNodeDeviceObjListExport;
virNodeDeviceObjListFree;
virNodeDeviceObjLock;
virNodeDeviceObjRemove;
@@ -650,7 +650,6 @@ virStoragePoolDefParseString;
virStoragePoolFormatDiskTypeToString;
virStoragePoolFormatFileSystemNetTypeToString;
virStoragePoolFormatFileSystemTypeToString;
-virStoragePoolList;
virStoragePoolLoadAllConfigs;
virStoragePoolObjAssignDef;
virStoragePoolObjClearVols;
@@ -658,6 +657,7 @@ virStoragePoolObjDeleteDef;
virStoragePoolObjFindByName;
virStoragePoolObjFindByUUID;
virStoragePoolObjIsDuplicate;
+virStoragePoolObjListExport;
virStoragePoolObjListFree;
virStoragePoolObjLock;
virStoragePoolObjRemove;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index cc8eaa9..02f7b69 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -348,10 +348,12 @@ storageConnectNumOfStoragePools(virConnectPtr conn) {
storageDriverLock(driver);
for (i = 0; i < driver->pools.count; i++) {
- virStoragePoolObjLock(driver->pools.objs[i]);
- if (virStoragePoolObjIsActive(driver->pools.objs[i]))
+ virStoragePoolObjPtr obj = driver->pools.objs[i];
+ virStoragePoolObjLock(obj);
+ if (virConnectNumOfStoragePoolsCheckACL(conn, obj->def) &&
+ virStoragePoolObjIsActive(obj))
nactive++;
- virStoragePoolObjUnlock(driver->pools.objs[i]);
+ virStoragePoolObjUnlock(obj);
}
storageDriverUnlock(driver);
@@ -370,15 +372,17 @@ storageConnectListStoragePools(virConnectPtr conn,
storageDriverLock(driver);
for (i = 0; i < driver->pools.count && got < nnames; i++) {
- virStoragePoolObjLock(driver->pools.objs[i]);
- if (virStoragePoolObjIsActive(driver->pools.objs[i])) {
- if (VIR_STRDUP(names[got], driver->pools.objs[i]->def->name) < 0)
{
- virStoragePoolObjUnlock(driver->pools.objs[i]);
+ virStoragePoolObjPtr obj = driver->pools.objs[i];
+ virStoragePoolObjLock(obj);
+ if (virConnectListStoragePoolsCheckACL(conn, obj->def) &&
+ virStoragePoolObjIsActive(obj)) {
+ if (VIR_STRDUP(names[got], obj->def->name) < 0) {
+ virStoragePoolObjUnlock(obj);
goto cleanup;
}
got++;
}
- virStoragePoolObjUnlock(driver->pools.objs[i]);
+ virStoragePoolObjUnlock(obj);
}
storageDriverUnlock(driver);
return got;
@@ -401,10 +405,12 @@ storageConnectNumOfDefinedStoragePools(virConnectPtr conn) {
storageDriverLock(driver);
for (i = 0; i < driver->pools.count; i++) {
- virStoragePoolObjLock(driver->pools.objs[i]);
- if (!virStoragePoolObjIsActive(driver->pools.objs[i]))
+ virStoragePoolObjPtr obj = driver->pools.objs[i];
+ virStoragePoolObjLock(obj);
+ if (virConnectNumOfDefinedStoragePoolsCheckACL(conn, obj->def) &&
+ !virStoragePoolObjIsActive(obj))
nactive++;
- virStoragePoolObjUnlock(driver->pools.objs[i]);
+ virStoragePoolObjUnlock(obj);
}
storageDriverUnlock(driver);
@@ -423,15 +429,17 @@ storageConnectListDefinedStoragePools(virConnectPtr conn,
storageDriverLock(driver);
for (i = 0; i < driver->pools.count && got < nnames; i++) {
- virStoragePoolObjLock(driver->pools.objs[i]);
- if (!virStoragePoolObjIsActive(driver->pools.objs[i])) {
- if (VIR_STRDUP(names[got], driver->pools.objs[i]->def->name) < 0)
{
- virStoragePoolObjUnlock(driver->pools.objs[i]);
+ virStoragePoolObjPtr obj = driver->pools.objs[i];
+ virStoragePoolObjLock(obj);
+ if (virConnectListDefinedStoragePoolsCheckACL(conn, obj->def) &&
+ !virStoragePoolObjIsActive(obj)) {
+ if (VIR_STRDUP(names[got], obj->def->name) < 0) {
+ virStoragePoolObjUnlock(obj);
goto cleanup;
}
got++;
}
- virStoragePoolObjUnlock(driver->pools.objs[i]);
+ virStoragePoolObjUnlock(obj);
}
storageDriverUnlock(driver);
return got;
@@ -1152,7 +1160,7 @@ static int
storagePoolNumOfVolumes(virStoragePoolPtr obj) {
virStorageDriverStatePtr driver = obj->conn->storagePrivateData;
virStoragePoolObjPtr pool;
- int ret = -1;
+ int ret = -1, i;
storageDriverLock(driver);
pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
@@ -1172,7 +1180,12 @@ storagePoolNumOfVolumes(virStoragePoolPtr obj) {
_("storage pool '%s' is not active"),
pool->def->name);
goto cleanup;
}
- ret = pool->volumes.count;
+ ret = 0;
+ for (i = 0; i < pool->volumes.count; i++) {
+ if (virStoragePoolNumOfVolumesCheckACL(obj->conn, pool->def,
+ pool->volumes.objs[i]))
+ ret++;
+ }
cleanup:
if (pool)
@@ -1210,6 +1223,9 @@ storagePoolListVolumes(virStoragePoolPtr obj,
}
for (i = 0; i < pool->volumes.count && n < maxnames; i++) {
+ if (!virStoragePoolListVolumesCheckACL(obj->conn, pool->def,
+ pool->volumes.objs[i]))
+ continue;
if (VIR_STRDUP(names[n++], pool->volumes.objs[i]->name) < 0)
goto cleanup;
}
@@ -1268,11 +1284,14 @@ storagePoolListAllVolumes(virStoragePoolPtr pool,
}
if (VIR_ALLOC_N(tmp_vols, obj->volumes.count + 1) < 0) {
- virReportOOMError();
- goto cleanup;
+ virReportOOMError();
+ goto cleanup;
}
for (i = 0; i < obj->volumes.count; i++) {
+ if (!virStoragePoolListAllVolumesCheckACL(pool->conn, obj->def,
+ obj->volumes.objs[i]))
+ continue;
if (!(vol = virGetStorageVol(pool->conn, obj->def->name,
obj->volumes.objs[i]->name,
obj->volumes.objs[i]->key,
@@ -2511,7 +2530,8 @@ storageConnectListAllStoragePools(virConnectPtr conn,
goto cleanup;
storageDriverLock(driver);
- ret = virStoragePoolList(conn, driver->pools, pools, flags);
+ ret = virStoragePoolObjListExport(conn, driver->pools, pools,
+ virConnectListAllStoragePoolsCheckACL, flags);
storageDriverUnlock(driver);
cleanup:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d4c339e..80a84d5 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4052,7 +4052,8 @@ testConnectListAllStoragePools(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL, -1);
testDriverLock(privconn);
- ret = virStoragePoolList(conn, privconn->pools, pools, flags);
+ ret = virStoragePoolObjListExport(conn, privconn->pools, pools,
+ NULL, flags);
testDriverUnlock(privconn);
return ret;
--
1.8.1.4
3:20 p.m.
New subject: [libvirt] [PATCH 4/8] Add access control filtering of storage objects
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list storage objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/conf/storage_conf.c | 12 +++++----
src/conf/storage_conf.h | 11 +++++---
src/libvirt_private.syms | 4 +--
src/storage/storage_driver.c | 62 +++++++++++++++++++++++++++++---------------
src/test/test_driver.c | 3 ++-
5 files changed, 59 insertions(+), 33 deletions(-)
+++ b/src/libvirt_private.syms
@@ -532,7 +532,7 @@ virNodeDeviceFindBySysfsPath;
virNodeDeviceGetParentHost;
virNodeDeviceGetWWNs;
virNodeDeviceHasCap;
-virNodeDeviceList;
+virNodeDeviceObjListExport;
This hunk belongs in a different patch.
ACK with that fixed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:57 a.m.
New subject: [libvirt] [PATCH 5/8] Add access control filtering of secret objects
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list secret objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/secret/secret_driver.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index fbe49d7..71b3fe7 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -566,8 +566,11 @@ secretConnectNumOfSecrets(virConnectPtr conn)
secretDriverLock(driver);
i = 0;
- for (secret = driver->secrets; secret != NULL; secret = secret->next)
- i++;
+ for (secret = driver->secrets; secret != NULL; secret = secret->next) {
+ if (virConnectNumOfSecretsCheckACL(conn,
+ secret->def))
+ i++;
+ }
secretDriverUnlock(driver);
return i;
@@ -590,6 +593,9 @@ secretConnectListSecrets(virConnectPtr conn, char **uuids, int
maxuuids)
i = 0;
for (secret = driver->secrets; secret != NULL; secret = secret->next) {
char *uuidstr;
+ if (!virConnectListSecretsCheckACL(conn,
+ secret->def))
+ continue;
if (i == maxuuids)
break;
if (VIR_ALLOC_N(uuidstr, VIR_UUID_STRING_BUFLEN) < 0) {
@@ -666,6 +672,10 @@ secretConnectListAllSecrets(virConnectPtr conn,
}
for (entry = driver->secrets; entry != NULL; entry = entry->next) {
+ if (!virConnectListAllSecretsCheckACL(conn,
+ entry->def))
+ continue;
+
/* filter by whether it's ephemeral */
if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL) &&
!((MATCH(VIR_CONNECT_LIST_SECRETS_EPHEMERAL) &&
--
1.8.1.4
3:55 p.m.
New subject: [libvirt] [PATCH 5/8] Add access control filtering of secret objects
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list secret objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/secret/secret_driver.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:57 a.m.
New subject: [libvirt] [PATCH 6/8] Add access control filtering of nwfilter objects
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list nwfilter objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/nwfilter/nwfilter_driver.c | 39 +++++++++++++++++++++++++++------------
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 7e8e202..0fbc940 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -441,11 +441,21 @@ nwfilterClose(virConnectPtr conn) {
static int
nwfilterConnectNumOfNWFilters(virConnectPtr conn) {
virNWFilterDriverStatePtr driver = conn->nwfilterPrivateData;
+ int i, n;
if (virConnectNumOfNWFiltersEnsureACL(conn) < 0)
return -1;
- return driver->nwfilters.count;
+ n = 0;
+ for (i = 0; i < driver->nwfilters.count; i++) {
+ virNWFilterObjPtr obj = driver->nwfilters.objs[i];
+ virNWFilterObjLock(obj);
+ if (virConnectNumOfNWFiltersCheckACL(conn, obj->def))
+ n++;
+ virNWFilterObjUnlock(obj);
+ }
+
+ return n;
}
@@ -461,13 +471,16 @@ nwfilterConnectListNWFilters(virConnectPtr conn,
nwfilterDriverLock(driver);
for (i = 0; i < driver->nwfilters.count && got < nnames; i++) {
- virNWFilterObjLock(driver->nwfilters.objs[i]);
- if (VIR_STRDUP(names[got], driver->nwfilters.objs[i]->def->name) < 0)
{
- virNWFilterObjUnlock(driver->nwfilters.objs[i]);
- goto cleanup;
+ virNWFilterObjPtr obj = driver->nwfilters.objs[i];
+ virNWFilterObjLock(obj);
+ if (virConnectListNWFiltersCheckACL(conn, obj->def)) {
+ if (VIR_STRDUP(names[got], obj->def->name) < 0) {
+ virNWFilterObjUnlock(obj);
+ goto cleanup;
+ }
+ got++;
}
- got++;
- virNWFilterObjUnlock(driver->nwfilters.objs[i]);
+ virNWFilterObjUnlock(obj);
}
nwfilterDriverUnlock(driver);
return got;
@@ -513,13 +526,15 @@ nwfilterConnectListAllNWFilters(virConnectPtr conn,
for (i = 0; i < driver->nwfilters.count; i++) {
obj = driver->nwfilters.objs[i];
virNWFilterObjLock(obj);
- if (!(filter = virGetNWFilter(conn, obj->def->name,
- obj->def->uuid))) {
- virNWFilterObjUnlock(obj);
- goto cleanup;
+ if (virConnectListAllNWFiltersCheckACL(conn, obj->def)) {
+ if (!(filter = virGetNWFilter(conn, obj->def->name,
+ obj->def->uuid))) {
+ virNWFilterObjUnlock(obj);
+ goto cleanup;
+ }
+ tmp_filters[nfilters++] = filter;
}
virNWFilterObjUnlock(obj);
- tmp_filters[nfilters++] = filter;
}
*filters = tmp_filters;
--
1.8.1.4
4:27 p.m.
New subject: [libvirt] [PATCH 6/8] Add access control filtering of nwfilter objects
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list nwfilter objects filter
them against the access control system.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/nwfilter/nwfilter_driver.c | 39 +++++++++++++++++++++++++++------------
1 file changed, 27 insertions(+), 12 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:57 a.m.
New subject: [libvirt] [PATCH 7/8] Add access control filtering of interface objects
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list interface objects filter
them against the access control system.
This makes the APIs for listing names and counting devices
slightly less efficient, since we can't use the direct
netcf APIs for these tasks. Instead we have to ask netcf
for the full list of objects & iterate over the list
filtering them out.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/conf/interface_conf.h | 3 +
src/interface/interface_backend_netcf.c | 262 +++++++++++++++++++++++++++-----
src/interface/interface_backend_udev.c | 56 +++++--
3 files changed, 270 insertions(+), 51 deletions(-)
diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h
index ed6986c..d60867b 100644
--- a/src/conf/interface_conf.h
+++ b/src/conf/interface_conf.h
@@ -211,6 +211,9 @@ char *virInterfaceDefFormat(const virInterfaceDefPtr def);
void virInterfaceObjLock(virInterfaceObjPtr obj);
void virInterfaceObjUnlock(virInterfaceObjPtr obj);
+typedef bool (*virInterfaceObjListFilter)(virConnectPtr conn,
+ virInterfaceDefPtr def);
+
# define VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE \
(VIR_CONNECT_LIST_INTERFACES_ACTIVE | \
VIR_CONNECT_LIST_INTERFACES_INACTIVE)
diff --git a/src/interface/interface_backend_netcf.c
b/src/interface/interface_backend_netcf.c
index a995816..abd11e2 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -209,48 +209,233 @@ static int netcfInterfaceClose(virConnectPtr conn)
return 0;
}
-static int netcfConnectNumOfInterfaces(virConnectPtr conn)
+static int netcfConnectNumOfInterfacesImpl(virConnectPtr conn,
+ int status,
+ virInterfaceObjListFilter filter)
{
- int count;
struct interface_driver *driver = conn->interfacePrivateData;
+ int count;
+ int want = 0;
+ int ret = -1;
+ int i;
+ char **names = NULL;
- if (virConnectNumOfInterfacesEnsureACL(conn) < 0)
- return -1;
-
- interfaceDriverLock(driver);
- count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_ACTIVE);
+ /* List all interfaces, in case of we might support new filter flags
+ * except active|inactive in future.
+ */
+ count = ncf_num_of_interfaces(driver->netcf, status);
if (count < 0) {
const char *errmsg, *details;
int errcode = ncf_error(driver->netcf, &errmsg, &details);
virReportError(netcf_to_vir_err(errcode),
- _("failed to get number of interfaces on host:
%s%s%s"),
- errmsg, details ? " - " : "", details ?
details : "");
+ _("failed to get number of host interfaces: %s%s%s"),
+ errmsg, details ? " - " : "",
+ details ? details : "");
+ goto cleanup;
}
- interfaceDriverUnlock(driver);
- return count;
+ if (count == 0) {
+ ret = 0;
+ goto cleanup;
+ }
+
+ if (VIR_ALLOC_N(names, count) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if ((count = ncf_list_interfaces(driver->netcf, count, names, status)) < 0) {
+ const char *errmsg, *details;
+ int errcode = ncf_error(driver->netcf, &errmsg, &details);
+ virReportError(netcf_to_vir_err(errcode),
+ _("failed to list host interfaces: %s%s%s"),
+ errmsg, details ? " - " : "",
+ details ? details : "");
+ goto cleanup;
+ }
+
+ for (i = 0; i < count; i++) {
+ virInterfaceDefPtr def;
+ struct netcf_if *iface;
+
+ iface = ncf_lookup_by_name(driver->netcf, names[i]);
+ if (!iface) {
+ const char *errmsg, *details;
+ int errcode = ncf_error(driver->netcf, &errmsg, &details);
+ if (errcode != NETCF_NOERROR) {
+ virReportError(netcf_to_vir_err(errcode),
+ _("couldn't find interface named '%s':
%s%s%s"),
+ names[i], errmsg,
+ details ? " - " : "", details ?
details : "");
+ goto cleanup;
+ } else {
+ /* Ignore the NETCF_NOERROR, as the interface is very likely
+ * deleted by other management apps (e.g. virt-manager).
+ */
+ VIR_WARN("couldn't find interface named '%s', might be
"
+ "deleted by other process", names[i]);
+ continue;
+ }
+ }
+
+ if (!(def = netcfGetMinimalDefForDevice(iface))) {
+ ncf_if_free(iface);
+ goto cleanup;
+ }
+ ncf_if_free(iface);
+
+ if (!filter(conn, def)) {
+ virInterfaceDefFree(def);
+ continue;
+ }
+ virInterfaceDefFree(def);
+
+ want++;
+ }
+
+ ret = want;
+
+cleanup:
+ if (names)
+ for (i = 0; i < count; i++)
+ VIR_FREE(names[i]);
+ VIR_FREE(names);
+ return ret;
}
-static int netcfConnectListInterfaces(virConnectPtr conn, char **const names, int
nnames)
+
+static int netcfConnectListInterfacesImpl(virConnectPtr conn,
+ int status,
+ char **const names, int nnames,
+ virInterfaceObjListFilter filter)
{
struct interface_driver *driver = conn->interfacePrivateData;
- int count;
+ int count = 0;
+ int want = 0;
+ int ret = -1;
+ int i;
+ char **allnames = NULL;
- if (virConnectListInterfacesEnsureACL(conn) < 0)
- return -1;
+ count = ncf_num_of_interfaces(driver->netcf, status);
+ if (count < 0) {
+ const char *errmsg, *details;
+ int errcode = ncf_error(driver->netcf, &errmsg, &details);
+ virReportError(netcf_to_vir_err(errcode),
+ _("failed to get number of host interfaces: %s%s%s"),
+ errmsg, details ? " - " : "",
+ details ? details : "");
+ goto cleanup;
+ }
- interfaceDriverLock(driver);
+ if (count == 0) {
+ ret = 0;
+ goto cleanup;
+ }
- count = ncf_list_interfaces(driver->netcf, nnames, names, NETCF_IFACE_ACTIVE);
- if (count < 0) {
+ if (VIR_ALLOC_N(allnames, count) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if ((count = ncf_list_interfaces(driver->netcf, count, allnames, status)) < 0)
{
const char *errmsg, *details;
int errcode = ncf_error(driver->netcf, &errmsg, &details);
virReportError(netcf_to_vir_err(errcode),
_("failed to list host interfaces: %s%s%s"),
errmsg, details ? " - " : "",
details ? details : "");
+ goto cleanup;
+ }
+
+ if (count == 0) {
+ ret = 0;
+ goto cleanup;
+ }
+
+ for (i = 0; i < count && want < nnames; i++) {
+ virInterfaceDefPtr def;
+ struct netcf_if *iface;
+
+ iface = ncf_lookup_by_name(driver->netcf, allnames[i]);
+ if (!iface) {
+ const char *errmsg, *details;
+ int errcode = ncf_error(driver->netcf, &errmsg, &details);
+ if (errcode != NETCF_NOERROR) {
+ virReportError(netcf_to_vir_err(errcode),
+ _("couldn't find interface named '%s':
%s%s%s"),
+ allnames[i], errmsg,
+ details ? " - " : "", details ?
details : "");
+ goto cleanup;
+ } else {
+ /* Ignore the NETCF_NOERROR, as the interface is very likely
+ * deleted by other management apps (e.g. virt-manager).
+ */
+ VIR_WARN("couldn't find interface named '%s', might be
"
+ "deleted by other process", allnames[i]);
+ continue;
+ }
+ }
+
+ if (!(def = netcfGetMinimalDefForDevice(iface))) {
+ ncf_if_free(iface);
+ goto cleanup;
+ }
+ ncf_if_free(iface);
+
+ if (!filter(conn, def)) {
+ virInterfaceDefFree(def);
+ continue;
+ }
+ virInterfaceDefFree(def);
+
+ names[want++] = allnames[i];
+ allnames[i] = NULL;
+ }
+
+ ret = want;
+
+cleanup:
+ if (allnames)
+ for (i = 0; i < count; i++)
+ VIR_FREE(allnames[i]);
+ VIR_FREE(allnames);
+ if (ret < 0) {
+ for (i = 0; i < nnames; i++)
+ VIR_FREE(names[i]);
}
+ return ret;
+}
+
+
+static int netcfConnectNumOfInterfaces(virConnectPtr conn)
+{
+ int count;
+ struct interface_driver *driver = conn->interfacePrivateData;
+
+ if (virConnectNumOfInterfacesEnsureACL(conn) < 0)
+ return -1;
+
+ interfaceDriverLock(driver);
+ count = netcfConnectNumOfInterfacesImpl(conn,
+ NETCF_IFACE_ACTIVE,
+ virConnectNumOfInterfacesCheckACL);
+ interfaceDriverUnlock(driver);
+ return count;
+}
+
+static int netcfConnectListInterfaces(virConnectPtr conn, char **const names, int
nnames)
+{
+ struct interface_driver *driver = conn->interfacePrivateData;
+ int count;
+
+ if (virConnectListInterfacesEnsureACL(conn) < 0)
+ return -1;
+ interfaceDriverLock(driver);
+ count = netcfConnectListInterfacesImpl(conn,
+ NETCF_IFACE_ACTIVE,
+ names, nnames,
+ virConnectListInterfacesCheckACL);
interfaceDriverUnlock(driver);
return count;
@@ -265,16 +450,9 @@ static int netcfConnectNumOfDefinedInterfaces(virConnectPtr conn)
return -1;
interfaceDriverLock(driver);
- count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_INACTIVE);
- if (count < 0) {
- const char *errmsg, *details;
- int errcode = ncf_error(driver->netcf, &errmsg, &details);
- virReportError(netcf_to_vir_err(errcode),
- _("failed to get number of defined interfaces on host:
%s%s%s"),
- errmsg, details ? " - " : "",
- details ? details : "");
- }
-
+ count = netcfConnectNumOfInterfacesImpl(conn,
+ NETCF_IFACE_INACTIVE,
+ virConnectNumOfDefinedInterfacesCheckACL);
interfaceDriverUnlock(driver);
return count;
}
@@ -288,17 +466,10 @@ static int netcfConnectListDefinedInterfaces(virConnectPtr conn,
char **const na
return -1;
interfaceDriverLock(driver);
-
- count = ncf_list_interfaces(driver->netcf, nnames, names, NETCF_IFACE_INACTIVE);
- if (count < 0) {
- const char *errmsg, *details;
- int errcode = ncf_error(driver->netcf, &errmsg, &details);
- virReportError(netcf_to_vir_err(errcode),
- _("failed to list host defined interfaces: %s%s%s"),
- errmsg, details ? " - " : "",
- details ? details : "");
- }
-
+ count = netcfConnectListInterfacesImpl(conn,
+ NETCF_IFACE_INACTIVE,
+ names, nnames,
+ virConnectListDefinedInterfacesCheckACL);
interfaceDriverUnlock(driver);
return count;
@@ -373,6 +544,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn,
}
for (i = 0; i < count; i++) {
+ virInterfaceDefPtr def;
iface = ncf_lookup_by_name(driver->netcf, names[i]);
if (!iface) {
const char *errmsg, *details;
@@ -403,6 +575,17 @@ netcfConnectListAllInterfaces(virConnectPtr conn,
goto cleanup;
}
+ if (!(def = netcfGetMinimalDefForDevice(iface)))
+ goto cleanup;
+
+ if (!virConnectListAllInterfacesCheckACL(conn, def)) {
+ ncf_if_free(iface);
+ iface = NULL;
+ virInterfaceDefFree(def);
+ continue;
+ }
+ virInterfaceDefFree(def);
+
/* XXX: Filter the result, need to be splitted once new filter flags
* except active|inactive are supported.
*/
@@ -412,6 +595,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn,
(MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) &&
(status & NETCF_IFACE_INACTIVE)))) {
ncf_if_free(iface);
+ iface = NULL;
continue;
}
diff --git a/src/interface/interface_backend_udev.c
b/src/interface/interface_backend_udev.c
index 68e1e2f..e063702 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -181,7 +181,8 @@ udevInterfaceClose(virConnectPtr conn)
}
static int
-udevNumOfInterfacesByStatus(virConnectPtr conn, virUdevStatus status)
+udevNumOfInterfacesByStatus(virConnectPtr conn, virUdevStatus status,
+ virInterfaceObjListFilter filter)
{
struct udev_iface_driver *driverState = conn->interfacePrivateData;
struct udev *udev = udev_ref(driverState->udev);
@@ -208,7 +209,18 @@ udevNumOfInterfacesByStatus(virConnectPtr conn, virUdevStatus
status)
/* For each item so we can count */
udev_list_entry_foreach(dev_entry, devices) {
- count++;
+ struct udev_device *dev;
+ const char *path;
+ virInterfaceDefPtr def;
+
+ path = udev_list_entry_get_name(dev_entry);
+ dev = udev_device_new_from_syspath(udev, path);
+
+ def = udevGetMinimalDefForDevice(dev);
+ if (filter(conn, def))
+ count++;
+ udev_device_unref(dev);
+ virInterfaceDefFree(def);
}
cleanup:
@@ -223,7 +235,8 @@ static int
udevListInterfacesByStatus(virConnectPtr conn,
char **const names,
int names_len,
- virUdevStatus status)
+ virUdevStatus status,
+ virInterfaceObjListFilter filter)
{
struct udev_iface_driver *driverState = conn->interfacePrivateData;
struct udev *udev = udev_ref(driverState->udev);
@@ -251,6 +264,7 @@ udevListInterfacesByStatus(virConnectPtr conn,
udev_list_entry_foreach(dev_entry, devices) {
struct udev_device *dev;
const char *path;
+ virInterfaceDefPtr def;
/* Ensure we won't exceed the size of our array */
if (count > names_len)
@@ -258,13 +272,18 @@ udevListInterfacesByStatus(virConnectPtr conn,
path = udev_list_entry_get_name(dev_entry);
dev = udev_device_new_from_syspath(udev, path);
- if (VIR_STRDUP(names[count], udev_device_get_sysname(dev)) < 0) {
- udev_device_unref(dev);
- goto error;
+
+ def = udevGetMinimalDefForDevice(dev);
+ if (filter(conn, def)) {
+ if (VIR_STRDUP(names[count], udev_device_get_sysname(dev)) < 0) {
+ udev_device_unref(dev);
+ virInterfaceDefFree(def);
+ goto error;
+ }
+ count++;
}
udev_device_unref(dev);
-
- count++;
+ virInterfaceDefFree(def);
}
udev_enumerate_unref(enumerate);
@@ -289,7 +308,8 @@ udevConnectNumOfInterfaces(virConnectPtr conn)
if (virConnectNumOfInterfacesEnsureACL(conn) < 0)
return -1;
- return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_ACTIVE);
+ return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_ACTIVE,
+ virConnectNumOfInterfacesCheckACL);
}
static int
@@ -301,7 +321,8 @@ udevConnectListInterfaces(virConnectPtr conn,
return -1;
return udevListInterfacesByStatus(conn, names, names_len,
- VIR_UDEV_IFACE_ACTIVE);
+ VIR_UDEV_IFACE_ACTIVE,
+ virConnectListInterfacesCheckACL);
}
static int
@@ -310,7 +331,8 @@ udevConnectNumOfDefinedInterfaces(virConnectPtr conn)
if (virConnectNumOfDefinedInterfacesEnsureACL(conn) < 0)
return -1;
- return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_INACTIVE);
+ return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_INACTIVE,
+ virConnectNumOfDefinedInterfacesCheckACL);
}
static int
@@ -322,7 +344,8 @@ udevConnectListDefinedInterfaces(virConnectPtr conn,
return -1;
return udevListInterfacesByStatus(conn, names, names_len,
- VIR_UDEV_IFACE_INACTIVE);
+ VIR_UDEV_IFACE_INACTIVE,
+ virConnectListDefinedInterfacesCheckACL);
}
#define MATCH(FLAG) (flags & (FLAG))
@@ -400,6 +423,7 @@ udevConnectListAllInterfaces(virConnectPtr conn,
const char *path;
const char *name;
const char *macaddr;
+ virInterfaceDefPtr def;
path = udev_list_entry_get_name(dev_entry);
dev = udev_device_new_from_syspath(udev, path);
@@ -407,6 +431,14 @@ udevConnectListAllInterfaces(virConnectPtr conn,
macaddr = udev_device_get_sysattr_value(dev, "address");
status = STREQ(udev_device_get_sysattr_value(dev, "operstate"),
"up");
+ def = udevGetMinimalDefForDevice(dev);
+ if (!virConnectListAllInterfacesCheckACL(conn, def)) {
+ udev_device_unref(dev);
+ virInterfaceDefFree(def);
+ continue;
+ }
+ virInterfaceDefFree(def);
+
/* Filter the results */
if (MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) &&
!((MATCH(VIR_CONNECT_LIST_INTERFACES_ACTIVE) && status) ||
--
1.8.1.4
5:02 p.m.
New subject: [libvirt] [PATCH 7/8] Add access control filtering of interface objects
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Ensure that all APIs which list interface objects filter
them against the access control system.
This makes the APIs for listing names and counting devices
slightly less efficient, since we can't use the direct
netcf APIs for these tasks. Instead we have to ask netcf
for the full list of objects & iterate over the list
filtering them out.
Such is life with security filtering.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/conf/interface_conf.h | 3 +
src/interface/interface_backend_netcf.c | 262 +++++++++++++++++++++++++++-----
src/interface/interface_backend_udev.c | 56 +++++--
3 files changed, 270 insertions(+), 51 deletions(-)
+++ b/src/interface/interface_backend_netcf.c
@@ -209,48 +209,233 @@ static int netcfInterfaceClose(virConnectPtr conn)
return 0;
}
-static int netcfConnectNumOfInterfaces(virConnectPtr conn)
+static int netcfConnectNumOfInterfacesImpl(virConnectPtr conn,
+ int status,
+ virInterfaceObjListFilter filter)
{
- int count;
struct interface_driver *driver = conn->interfacePrivateData;
+ int count;
+ int want = 0;
+ int ret = -1;
+ int i;
+ char **names = NULL;
- if (virConnectNumOfInterfacesEnsureACL(conn) < 0)
- return -1;
-
- interfaceDriverLock(driver);
- count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_ACTIVE);
+ /* List all interfaces, in case of we might support new filter flags
+ * except active|inactive in future.
s/case of/case/
s/except/beyond/
+
+ if (VIR_ALLOC_N(names, count) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
[Uggh - we still need to revisit the goal to hoist OOM reporting into
VIR_ALLOC and friends - nothing magic happened during my 3-week vacation
on that front. But that's a completely separate project.]
+static int netcfConnectListInterfacesImpl(virConnectPtr conn,
+ int status,
+ char **const names, int nnames,
+ virInterfaceObjListFilter filter)
{
struct interface_driver *driver = conn->interfacePrivateData;
- int count;
+ int count = 0;
+ int want = 0;
+ int ret = -1;
+ int i;
+ char **allnames = NULL;
- if (virConnectListInterfacesEnsureACL(conn) < 0)
- return -1;
+ count = ncf_num_of_interfaces(driver->netcf, status);
Can any of this be shared? You have:
long setup
foreach element
if not filtered
count++
and
long setup
foreach element
if not filtered
append object
where the long setup is the same. I think some of the other filtering
code has used a single implementation, where passing in NULL for the
**names parameter implies just counting, and passing in non-NULL implies
object appending. Then you wouldn't be duplicating as much code, making
it easier to add new filters in one place in the future.
+static int netcfConnectListInterfaces(virConnectPtr conn, char
**const names, int nnames)
+{
+ struct interface_driver *driver = conn->interfacePrivateData;
+ int count;
+
+ if (virConnectListInterfacesEnsureACL(conn) < 0)
+ return -1;
+ interfaceDriverLock(driver);
+ count = netcfConnectListInterfacesImpl(conn,
+ NETCF_IFACE_ACTIVE,
+ names, nnames,
+ virConnectListInterfacesCheckACL);
Indentation is off.
@@ -403,6 +575,17 @@ netcfConnectListAllInterfaces(virConnectPtr
conn,
goto cleanup;
}
+ if (!(def = netcfGetMinimalDefForDevice(iface)))
+ goto cleanup;
+
+ if (!virConnectListAllInterfacesCheckACL(conn, def)) {
+ ncf_if_free(iface);
+ iface = NULL;
+ virInterfaceDefFree(def);
+ continue;
+ }
+ virInterfaceDefFree(def);
+
/* XXX: Filter the result, need to be splitted once new filter flags
* except active|inactive are supported.
Pre-existing, but as long as you are here, s/splitted/split/. Or maybe
even nuke the comment, now that you have set up the framework for
additional filtering so it no longer applies.
@@ -412,6 +595,7 @@ netcfConnectListAllInterfaces(virConnectPtr
conn,
(MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) &&
(status & NETCF_IFACE_INACTIVE)))) {
ncf_if_free(iface);
+ iface = NULL;
continue;
Oooh tricky - you posted the CVE-2013-2218 patch embedded inside this
patch, prior to the embargo date, but no one picked up on it until now :)
At any rate, a consolidation patch between the count vs. list function
is probably worth a separate patch, at which point your code as-is looks
mostly okay. ACK with grammar/spacing nits fixed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:57 a.m.
New subject: [libvirt] [PATCH 8/8] Extend the ACL test case to validate filter rule checks
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The 'check-aclrules' test case validates that there are ACL
checks in each method. This extends it so that it can also
validate that methods which return info about lists of objects,
will filter their returned info throw an ACL check.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/Makefile.am | 1 +
src/check-aclrules.pl | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 98 insertions(+)
diff --git a/src/Makefile.am b/src/Makefile.am
index e6b1927..5057277 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -500,6 +500,7 @@ check-driverimpls:
check-aclrules:
$(AM_V_GEN)$(PERL) $(srcdir)/check-aclrules.pl \
+ $(addprefix $(srcdir)/,$(filter-out /%,$(REMOTE_PROTOCOL))) \
$(addprefix $(srcdir)/,$(filter-out /%,$(STATEFUL_DRIVER_SOURCE_FILES)))
EXTRA_DIST += check-driverimpls.pl check-aclrules.pl
diff --git a/src/check-aclrules.pl b/src/check-aclrules.pl
index 336b2fe..057517e 100644
--- a/src/check-aclrules.pl
+++ b/src/check-aclrules.pl
@@ -42,6 +42,7 @@ my $intable = 0;
my $table;
my %acls;
+my %aclfilters;
my %whitelist = (
"connectClose" => 1,
@@ -73,9 +74,80 @@ my %implwhitelist = (
"xenUnifiedDomainIsUpdated" => 1,
"xenUnifiedDomainOpenConsole" => 1,
);
+my %filterimplwhitelist = (
+ "xenUnifiedConnectListDomains" => 1,
+ "xenUnifiedConnectNumOfDomains" => 1,
+ "xenUnifiedConnectListDefinedDomains" => 1,
+ "xenUnifiedConnectNumOfDefinedDomains" => 1,
+ );
my $lastfile;
+sub fixup_name {
+ my $name = shift;
+
+ $name =~ s/Nwfilter/NWFilter/;
+ $name =~ s/Xml$/XML/;
+ $name =~ s/Uri$/URI/;
+ $name =~ s/Uuid$/UUID/;
+ $name =~ s/Id$/ID/;
+ $name =~ s/Mac$/MAC/;
+ $name =~ s/Cpu$/CPU/;
+ $name =~ s/Os$/OS/;
+ $name =~ s/Nmi$/NMI/;
+ $name =~ s/Pm/PM/;
+ $name =~ s/Fstrim$/FSTrim/;
+ $name =~ s/Scsi/SCSI/;
+ $name =~ s/Wwn$/WWN/;
+
+ return $name;
+}
+
+sub name_to_ProcName {
+ my $name = shift;
+
+ my @elems;
+ if ($name =~ /_/ || (lc $name) eq "open" || (lc $name) eq
"close") {
+ @elems = split /_/, $name;
+ @elems = map lc, @elems;
+ @elems = map ucfirst, @elems;
+ } else {
+ @elems = $name;
+ }
+ @elems = map { fixup_name($_) } @elems;
+ my $procname = join "", @elems;
+
+ $procname =~ s/^([A-Z])/lc $1/e;
+
+ return $procname;
+}
+
+
+my $proto = shift @ARGV;
+
+open PROTO, "<$proto" or die "cannot read $proto";
+
+my %filtered;
+my $incomment = 0;
+my $filtered = 0;
+while (<PROTO>) {
+ if (m,/\*\*,) {
+ $incomment = 1;
+ $filtered = 0;
+ } elsif ($incomment) {
+ if (m,\*\s\@aclfilter,) {
+ $filtered = 1;
+ } elsif ($filtered &&
+ m,REMOTE_PROC_(.*)\s+=\s*\d+,) {
+ my $api = name_to_ProcName($1);
+ $filtered{$api} = 1;
+ $incomment = 0;
+ }
+ }
+}
+
+close PROTO;
+
while (<>) {
if (!defined $lastfile ||
$lastfile ne $ARGV) {
@@ -107,6 +179,21 @@ while (<>) {
}
}
$acls{$maybefunc} = 1;
+ } elsif (m,(\w+)CheckACL,) {
+ # Record the fact that maybefunc contains an
+ # ACL filter call, and make sure it is the right call!
+ my $func = $1;
+ $func =~ s/^vir//;
+ if (!defined $maybefunc) {
+ print "$ARGV:$. Unexpected check '$func' outside
function\n";
+ $status = 1;
+ } else {
+ unless ($maybefunc =~ /$func$/i) {
+ print "$ARGV:$. Mismatch check 'vir${func}CheckACL' for
function '$maybefunc'\n";
+ $status = 1;
+ }
+ }
+ $aclfilters{$maybefunc} = 1;
} elsif (m,\b(\w+)\(,) {
# Handles case where we replaced an API with a new
# one which adds new parameters, and we're left with
@@ -115,6 +202,9 @@ while (<>) {
if (exists $acls{$callfunc}) {
$acls{$maybefunc} = 1;
}
+ if (exists $aclfilters{$callfunc}) {
+ $aclfilters{$maybefunc} = 1;
+ }
}
}
@@ -138,6 +228,13 @@ while (<>) {
print "$ARGV:$. Missing ACL check in function '$impl' for
'$api'\n";
$status = 1;
}
+
+ if (exists $filtered{$api} &&
+ !exists $aclfilters{$impl} &&
+ !exists $filterimplwhitelist{$impl}) {
+ print "$ARGV:$. Missing ACL filter in function '$impl' for
'$api'\n";
+ $status = 1;
+ }
}
} elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+/) {
if ($1 ne "virNWFilterCallbackDriver" &&
--
1.8.1.4
5:21 p.m.
New subject: [libvirt] [PATCH 8/8] Extend the ACL test case to validate filter rule checks
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The 'check-aclrules' test case validates that there are ACL
checks in each method. This extends it so that it can also
validate that methods which return info about lists of objects,
will filter their returned info throw an ACL check.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/Makefile.am | 1 +
src/check-aclrules.pl | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 98 insertions(+)
Seems reasonable; my perl is not as strong as my C, but it looks like
you were able to apply this patch out of order to test that it worked
correctly. ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:29 a.m.
New subject: [libvirt] [PATCH 8/8] Extend the ACL test case to validate filter rule checks
On Tue, Jul 02, 2013 at 04:21:30PM -0600, Eric Blake wrote:
On 06/27/2013 10:57 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> The 'check-aclrules' test case validates that there are ACL
> checks in each method. This extends it so that it can also
> validate that methods which return info about lists of objects,
> will filter their returned info throw an ACL check.
>
> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> ---
> src/Makefile.am | 1 +
> src/check-aclrules.pl | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 98 insertions(+)
>
Seems reasonable; my perl is not as strong as my C, but it looks like
you were able to apply this patch out of order to test that it worked
correctly. ACK.
Yes, exactly. I wrote this test case first, and then fixed up all the
flaws it identified.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:26 a.m.
On Thu, Jun 27, 2013 at 05:57:17PM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The current ACL checks validate access to the object being
passed in to the API calls.
There are a few APIs (all the virConnectList* / virConnectNum*
ones) which are used to get lists of objects in the first
place. Currently you could find out that there is a VM called
"foo", but you can't then do virDomainLookupByName since the
ACL check may block it.
This series introduces filtering in the object list APIs,
so you can't even see the existance of an object called
"foo", if you don't have permission over it.
This is not yet filtering the legacy Xen driver.
Daniel P. Berrange (8):
Add access control filtering of domain objects
Add access control filtering of network objects
Add access control filtering of node device objects
Add access control filtering of storage objects
Add access control filtering of secret objects
Add access control filtering of nwfilter objects
Add access control filtering of interface objects
Extend the ACL test case to validate filter rule checks
This series is a candidate for merging now the 1.1.0 release
is out, if someone can review it.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4160
days inactive
4166
days old
19 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Eric Blake