[libvirt] [PATCH 15/19] Add ACL checks into the interface driver

Daniel P. Berrange berrange at redhat.com
Thu May 9 13:26:18 UTC 2013


From: "Daniel P. Berrange" <berrange at redhat.com>

Insert calls to the ACL checking APIs in all interface
driver entrypoints.

Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
---
 src/interface/interface_backend_netcf.c | 114 ++++++++++++++++++++++++++++++++
 src/interface/interface_backend_udev.c  |  85 +++++++++++++++++++++---
 2 files changed, 191 insertions(+), 8 deletions(-)

diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c
index cbba4fd..4707e3e 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -31,6 +31,7 @@
 #include "interface_conf.h"
 #include "viralloc.h"
 #include "virlog.h"
+#include "access/viraccessapicheck.h"
 
 #define VIR_FROM_THIS VIR_FROM_INTERFACE
 
@@ -52,6 +53,36 @@ static void interfaceDriverUnlock(struct interface_driver *driver)
     virMutexUnlock(&driver->lock);
 }
 
+/*
+ * Get a minimal virInterfaceDef containing enough metadata
+ * for access control checks to be performed. Currently
+ * this implies existance of name and mac address attributes
+ */
+static virInterfaceDef * ATTRIBUTE_NONNULL(1)
+netcfGetMinimalDefForDevice(struct netcf_if *iface)
+{
+    virInterfaceDef *def;
+
+    /* Allocate our interface definition structure */
+    if (VIR_ALLOC(def) < 0) {
+        virReportOOMError();
+        return NULL;
+    }
+
+    if (VIR_STRDUP(def->name, ncf_if_name(iface)) < 0)
+        goto cleanup;
+
+    if (VIR_STRDUP(def->mac, ncf_if_mac_string(iface)) < 0)
+        goto cleanup;
+
+    return def;
+
+cleanup:
+    virInterfaceDefFree(def);
+    return NULL;
+}
+
+
 static int netcf_to_vir_err(int netcf_errcode)
 {
     switch (netcf_errcode)
@@ -182,6 +213,9 @@ static int netcfConnectNumOfInterfaces(virConnectPtr conn)
     int count;
     struct interface_driver *driver = conn->interfacePrivateData;
 
+    if (virConnectNumOfInterfacesEnsureACL(conn) < 0)
+        return -1;
+
     interfaceDriverLock(driver);
     count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_ACTIVE);
     if (count < 0) {
@@ -201,6 +235,9 @@ static int netcfConnectListInterfaces(virConnectPtr conn, char **const names, in
     struct interface_driver *driver = conn->interfacePrivateData;
     int count;
 
+    if (virConnectListInterfacesEnsureACL(conn) < 0)
+        return -1;
+
     interfaceDriverLock(driver);
 
     count = ncf_list_interfaces(driver->netcf, nnames, names, NETCF_IFACE_ACTIVE);
@@ -223,6 +260,9 @@ static int netcfConnectNumOfDefinedInterfaces(virConnectPtr conn)
     int count;
     struct interface_driver *driver = conn->interfacePrivateData;
 
+    if (virConnectNumOfDefinedInterfacesEnsureACL(conn) < 0)
+        return -1;
+
     interfaceDriverLock(driver);
     count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_INACTIVE);
     if (count < 0) {
@@ -243,6 +283,9 @@ static int netcfConnectListDefinedInterfaces(virConnectPtr conn, char **const na
     struct interface_driver *driver = conn->interfacePrivateData;
     int count;
 
+    if (virConnectListDefinedInterfacesEnsureACL(conn) < 0)
+        return -1;
+
     interfaceDriverLock(driver);
 
     count = ncf_list_interfaces(driver->netcf, nnames, names, NETCF_IFACE_INACTIVE);
@@ -279,6 +322,9 @@ netcfConnectListAllInterfaces(virConnectPtr conn,
     virCheckFlags(VIR_CONNECT_LIST_INTERFACES_ACTIVE |
                   VIR_CONNECT_LIST_INTERFACES_INACTIVE, -1);
 
+    if (virConnectListAllInterfacesEnsureACL(conn) < 0)
+        return -1;
+
     interfaceDriverLock(driver);
 
     /* List all interfaces, in case of we might support new filter flags
@@ -413,6 +459,7 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn,
     struct interface_driver *driver = conn->interfacePrivateData;
     struct netcf_if *iface;
     virInterfacePtr ret = NULL;
+    virInterfaceDefPtr def = NULL;
 
     interfaceDriverLock(driver);
     iface = ncf_lookup_by_name(driver->netcf, name);
@@ -431,10 +478,17 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn,
         goto cleanup;
     }
 
+    if (!(def = netcfGetMinimalDefForDevice(iface)))
+        goto cleanup;
+
+    if (virInterfaceLookupByNameEnsureACL(conn, def) < 0)
+       goto cleanup;
+
     ret = virGetInterface(conn, ncf_if_name(iface), ncf_if_mac_string(iface));
 
 cleanup:
     ncf_if_free(iface);
+    virInterfaceDefFree(def);
     interfaceDriverUnlock(driver);
     return ret;
 }
@@ -446,6 +500,7 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr conn,
     struct netcf_if *iface;
     int niface;
     virInterfacePtr ret = NULL;
+    virInterfaceDefPtr def = NULL;
 
     interfaceDriverLock(driver);
     niface = ncf_lookup_by_mac_string(driver->netcf, macstr, 1, &iface);
@@ -471,10 +526,18 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr conn,
         goto cleanup;
     }
 
+
+    if (!(def = netcfGetMinimalDefForDevice(iface)))
+        goto cleanup;
+
+    if (virInterfaceLookupByMACStringEnsureACL(conn, def) < 0)
+       goto cleanup;
+
     ret = virGetInterface(conn, ncf_if_name(iface), ncf_if_mac_string(iface));
 
 cleanup:
     ncf_if_free(iface);
+    virInterfaceDefFree(def);
     interfaceDriverUnlock(driver);
     return ret;
 }
@@ -519,6 +582,9 @@ static char *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo,
         goto cleanup;
     }
 
+    if (virInterfaceGetXMLDescEnsureACL(ifinfo->conn, ifacedef) < 0)
+        goto cleanup;
+
     ret = virInterfaceDefFormat(ifacedef);
     if (!ret) {
         /* error was already reported */
@@ -553,6 +619,9 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn,
         goto cleanup;
     }
 
+    if (virInterfaceDefineXMLEnsureACL(conn, ifacedef) < 0)
+        goto cleanup;
+
     xmlstr = virInterfaceDefFormat(ifacedef);
     if (!xmlstr) {
         /* error was already reported */
@@ -583,6 +652,7 @@ cleanup:
 static int netcfInterfaceUndefine(virInterfacePtr ifinfo) {
     struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
     struct netcf_if *iface = NULL;
+    virInterfaceDefPtr def = NULL;
     int ret = -1;
 
     interfaceDriverLock(driver);
@@ -593,6 +663,13 @@ static int netcfInterfaceUndefine(virInterfacePtr ifinfo) {
         goto cleanup;
     }
 
+
+    if (!(def = netcfGetMinimalDefForDevice(iface)))
+        goto cleanup;
+
+    if (virInterfaceUndefineEnsureACL(ifinfo->conn, def) < 0)
+       goto cleanup;
+
     ret = ncf_if_undefine(iface);
     if (ret < 0) {
         const char *errmsg, *details;
@@ -606,6 +683,7 @@ static int netcfInterfaceUndefine(virInterfacePtr ifinfo) {
 
 cleanup:
     ncf_if_free(iface);
+    virInterfaceDefFree(def);
     interfaceDriverUnlock(driver);
     return ret;
 }
@@ -615,6 +693,7 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo,
 {
     struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
     struct netcf_if *iface = NULL;
+    virInterfaceDefPtr def = NULL;
     int ret = -1;
 
     virCheckFlags(0, -1);
@@ -627,6 +706,13 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo,
         goto cleanup;
     }
 
+
+    if (!(def = netcfGetMinimalDefForDevice(iface)))
+        goto cleanup;
+
+    if (virInterfaceCreateEnsureACL(ifinfo->conn, def) < 0)
+       goto cleanup;
+
     ret = ncf_if_up(iface);
     if (ret < 0) {
         const char *errmsg, *details;
@@ -640,6 +726,7 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo,
 
 cleanup:
     ncf_if_free(iface);
+    virInterfaceDefFree(def);
     interfaceDriverUnlock(driver);
     return ret;
 }
@@ -649,6 +736,7 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo,
 {
     struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
     struct netcf_if *iface = NULL;
+    virInterfaceDefPtr def = NULL;
     int ret = -1;
 
     virCheckFlags(0, -1);
@@ -661,6 +749,13 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo,
         goto cleanup;
     }
 
+
+    if (!(def = netcfGetMinimalDefForDevice(iface)))
+        goto cleanup;
+
+    if (virInterfaceDestroyEnsureACL(ifinfo->conn, def) < 0)
+       goto cleanup;
+
     ret = ncf_if_down(iface);
     if (ret < 0) {
         const char *errmsg, *details;
@@ -674,6 +769,7 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo,
 
 cleanup:
     ncf_if_free(iface);
+    virInterfaceDefFree(def);
     interfaceDriverUnlock(driver);
     return ret;
 }
@@ -683,6 +779,7 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo)
     struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
     struct netcf_if *iface = NULL;
     unsigned int flags = 0;
+    virInterfaceDefPtr def = NULL;
     int ret = -1;
 
     interfaceDriverLock(driver);
@@ -693,6 +790,13 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo)
         goto cleanup;
     }
 
+
+    if (!(def = netcfGetMinimalDefForDevice(iface)))
+        goto cleanup;
+
+    if (virInterfaceIsActiveEnsureACL(ifinfo->conn, def) < 0)
+       goto cleanup;
+
     if (ncf_if_status(iface, &flags) < 0) {
         const char *errmsg, *details;
         int errcode = ncf_error(driver->netcf, &errmsg, &details);
@@ -707,6 +811,7 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo)
 
 cleanup:
     ncf_if_free(iface);
+    virInterfaceDefFree(def);
     interfaceDriverUnlock(driver);
     return ret;
 }
@@ -719,6 +824,9 @@ static int netcfInterfaceChangeBegin(virConnectPtr conn, unsigned int flags)
 
     virCheckFlags(0, -1); /* currently flags must be 0 */
 
+    if (virInterfaceChangeBeginEnsureACL(conn) < 0)
+        return -1;
+
     interfaceDriverLock(driver);
 
     ret = ncf_change_begin(driver->netcf, 0);
@@ -742,6 +850,9 @@ static int netcfInterfaceChangeCommit(virConnectPtr conn, unsigned int flags)
 
     virCheckFlags(0, -1); /* currently flags must be 0 */
 
+    if (virInterfaceChangeCommitEnsureACL(conn) < 0)
+        return -1;
+
     interfaceDriverLock(driver);
 
     ret = ncf_change_commit(driver->netcf, 0);
@@ -765,6 +876,9 @@ static int netcfInterfaceChangeRollback(virConnectPtr conn, unsigned int flags)
 
     virCheckFlags(0, -1); /* currently flags must be 0 */
 
+    if (virInterfaceChangeRollbackEnsureACL(conn) < 0)
+        return -1;
+
     interfaceDriverLock(driver);
 
     ret = ncf_change_rollback(driver->netcf, 0);
diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
index f9a179f..a6e7bd5 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -31,6 +31,7 @@
 #include "interface_conf.h"
 #include "viralloc.h"
 #include "virstring.h"
+#include "access/viraccessapicheck.h"
 
 #define VIR_FROM_THIS VIR_FROM_INTERFACE
 
@@ -61,6 +62,36 @@ virUdevStatusString(virUdevStatus status)
     return "";
 }
 
+/*
+ * Get a minimal virInterfaceDef containing enough metadata
+ * for access control checks to be performed. Currently
+ * this implies existance of name and mac address attributes
+ */
+static virInterfaceDef * ATTRIBUTE_NONNULL(1)
+udevGetMinimalDefForDevice(struct udev_device *dev)
+{
+    virInterfaceDef *def;
+
+    /* Allocate our interface definition structure */
+    if (VIR_ALLOC(def) < 0) {
+        virReportOOMError();
+        return NULL;
+    }
+
+    if (VIR_STRDUP(def->name, udev_device_get_sysname(dev)) < 0)
+        goto cleanup;
+
+    if (VIR_STRDUP(def->mac, udev_device_get_sysattr_value(dev, "address")) < 0)
+        goto cleanup;
+
+    return def;
+
+cleanup:
+    virInterfaceDefFree(def);
+    return NULL;
+}
+
+
 static struct udev_enumerate * ATTRIBUTE_NONNULL(1)
 udevGetDevices(struct udev *udev, virUdevStatus status)
 {
@@ -255,6 +286,9 @@ error:
 static int
 udevConnectNumOfInterfaces(virConnectPtr conn)
 {
+    if (virConnectNumOfInterfacesEnsureACL(conn) < 0)
+        return -1;
+
     return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_ACTIVE);
 }
 
@@ -263,6 +297,9 @@ udevConnectListInterfaces(virConnectPtr conn,
                           char **const names,
                           int names_len)
 {
+    if (virConnectListInterfacesEnsureACL(conn) < 0)
+        return -1;
+
     return udevListInterfacesByStatus(conn, names, names_len,
                                       VIR_UDEV_IFACE_ACTIVE);
 }
@@ -270,6 +307,9 @@ udevConnectListInterfaces(virConnectPtr conn,
 static int
 udevConnectNumOfDefinedInterfaces(virConnectPtr conn)
 {
+    if (virConnectNumOfDefinedInterfacesEnsureACL(conn) < 0)
+        return -1;
+
     return udevNumOfInterfacesByStatus(conn, VIR_UDEV_IFACE_INACTIVE);
 }
 
@@ -278,6 +318,9 @@ udevConnectListDefinedInterfaces(virConnectPtr conn,
                                  char **const names,
                                  int names_len)
 {
+    if (virConnectListDefinedInterfacesEnsureACL(conn) < 0)
+        return -1;
+
     return udevListInterfacesByStatus(conn, names, names_len,
                                       VIR_UDEV_IFACE_INACTIVE);
 }
@@ -302,6 +345,9 @@ udevConnectListAllInterfaces(virConnectPtr conn,
     virCheckFlags(VIR_CONNECT_LIST_INTERFACES_ACTIVE |
                   VIR_CONNECT_LIST_INTERFACES_INACTIVE, -1);
 
+    if (virConnectListAllInterfacesEnsureACL(conn) < 0)
+        return -1;
+
     /* Grab a udev reference */
     udev = udev_ref(driverState->udev);
 
@@ -413,8 +459,8 @@ udevInterfaceLookupByName(virConnectPtr conn, const char *name)
     struct udev_iface_driver *driverState = conn->interfacePrivateData;
     struct udev *udev = udev_ref(driverState->udev);
     struct udev_device *dev;
-    const char *macaddr;
     virInterfacePtr ret = NULL;
+    virInterfaceDefPtr def = NULL;
 
     /* get a device reference based on the device name */
     dev = udev_device_new_from_subsystem_sysname(udev, "net", name);
@@ -425,12 +471,18 @@ udevInterfaceLookupByName(virConnectPtr conn, const char *name)
         goto cleanup;
     }
 
-    macaddr = udev_device_get_sysattr_value(dev, "address");
-    ret = virGetInterface(conn, name, macaddr);
+    if (!(def = udevGetMinimalDefForDevice(dev)))
+        goto err;
+
+    if (virInterfaceLookupByNameEnsureACL(conn, def) < 0)
+       goto err;
+
+    ret = virGetInterface(conn, def->name, def->mac);
     udev_device_unref(dev);
 
 cleanup:
     udev_unref(udev);
+    virInterfaceDefFree(def);
 
     return ret;
 }
@@ -443,7 +495,7 @@ udevInterfaceLookupByMACString(virConnectPtr conn, const char *macstr)
     struct udev_enumerate *enumerate = NULL;
     struct udev_list_entry *dev_entry;
     struct udev_device *dev;
-    const char *name;
+    virInterfaceDefPtr def = NULL;
     virInterfacePtr ret = NULL;
 
     enumerate = udevGetDevices(udev, VIR_UDEV_IFACE_ALL);
@@ -481,14 +533,21 @@ udevInterfaceLookupByMACString(virConnectPtr conn, const char *macstr)
     }
 
     dev = udev_device_new_from_syspath(udev, udev_list_entry_get_name(dev_entry));
-    name = udev_device_get_sysname(dev);
-    ret = virGetInterface(conn, name, macstr);
+
+    if (!(def = udevGetMinimalDefForDevice(dev)))
+        goto err;
+
+    if (virInterfaceLookupByMACStringEnsureACL(conn, def) < 0)
+       goto err;
+
+    ret = virGetInterface(conn, def->name, def->mac);
     udev_device_unref(dev);
 
 cleanup:
     if (enumerate)
         udev_enumerate_unref(enumerate);
     udev_unref(udev);
+    virInterfaceDefFree(def);
 
     return ret;
 }
@@ -1045,6 +1104,9 @@ udevInterfaceGetXMLDesc(virInterfacePtr ifinfo,
     if (!ifacedef)
         goto cleanup;
 
+    if (virInterfaceGetXMLDescEnsureACL(ifinfo->conn, ifacedef) < 0)
+        goto err;
+
     xmlstr = virInterfaceDefFormat(ifacedef);
 
     virInterfaceDefFree(ifacedef);
@@ -1062,7 +1124,8 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
     struct udev_iface_driver *driverState = ifinfo->conn->interfacePrivateData;
     struct udev *udev = udev_ref(driverState->udev);
     struct udev_device *dev;
-    int status;
+    virInterfaceDefPtr def = NULL;
+    int status = -1;
 
     dev = udev_device_new_from_subsystem_sysname(udev, "net",
                                                  ifinfo->name);
@@ -1070,10 +1133,15 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
         virReportError(VIR_ERR_NO_INTERFACE,
                        _("couldn't find interface named '%s'"),
                        ifinfo->name);
-        status = -1;
         goto cleanup;
     }
 
+    if (!(def = udevGetMinimalDefForDevice(dev)))
+        goto cleanup;
+
+    if (virInterfaceIsActiveEnsureACL(ifinfo->conn, def) < 0)
+       goto cleanup;
+
     /* Check if it's active or not */
     status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up");
 
@@ -1081,6 +1149,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
 
 cleanup:
     udev_unref(udev);
+    virInterfaceDefFree(def);
 
     return status;
 }
-- 
1.8.1.4




More information about the libvir-list mailing list