Add access control filtering of interface objects

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@redhat.com>

diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h
index ed6986c5d859b194ba46c44f1ab63294cd10ac39..d60867bd1bd5511e80655216c9b1bc00cced72d4 100644 (file)
--- 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 9aa673d20a2aefded48b944acee498c70af3ca6b..760540ab6307b060066bb5572688a75d5abfddd5 100644 (file)
--- 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 we might support new filter flags
+     * beyond 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,7 +575,18 @@ netcfConnectListAllInterfaces(virConnectPtr conn,
             goto cleanup;
         }
 
-        /* XXX: Filter the result, need to be splitted once new filter flags
+        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 split once new filter flags
          * except active|inactive are supported.
          */
         if (MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) &&

diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
index 95034bfd517090e0eae13827f076a93bd8a13261..6270c9dc170d8e4d3f9af6d41d64c12e5afc6e13 100644 (file)
--- 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) ||