network: bridge: Don't start network if it collides with host routing

Fedora bug https://bugzilla.redhat.com/show_bug.cgi?id=235961

If using the default virtual network, an easy way to lose guest network
connectivity is to install libvirt inside the VM. The autostarted
default network inside the guest collides with host virtual network
routing. This is a long standing issue that has caused users quite a
bit of pain and confusion.

On network startup, parse /proc/net/route and compare the requested
IP+netmask against host routing destinations: if any matches are found,
refuse to start the network.

v2: Drop sscanf, fix a comment typo, comment that function could use
    libnl instead of /proc

v3: Consider route netmask. Compare binary data rather than convert to
    string.

v4: Return to using sscanf, drop inet functions in favor of virSocket,
    parsing safety checks. Don't make parse failures fatal, in case
    expected format changes.

v5: Try and continue if we receive unexpected. Delimit parsed lines to
    prevent scanning past newline

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5d7ef19241b12b14901ff07d53bb8e7527e92c3c..7ab3f3eec49cfed46d956269eb4ef2965cd3e3a7 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -57,6 +57,7 @@
 #include "bridge.h"
 #include "logging.h"
 #include "dnsmasq.h"
+#include "util/network.h"
 
 #define NETWORK_PID_DIR LOCAL_STATE_DIR "/run/libvirt/network"
 #define NETWORK_STATE_DIR LOCAL_STATE_DIR "/lib/libvirt/network"
@@ -908,6 +909,114 @@ cleanup:
     return ret;
 }
 
+#define PROC_NET_ROUTE "/proc/net/route"
+
+/* XXX: This function can be a lot more exhaustive, there are certainly
+ *      other scenarios where we can ruin host network connectivity.
+ * XXX: Using a proper library is preferred over parsing /proc
+ */
+static int networkCheckRouteCollision(virNetworkObjPtr network)
+{
+    int ret = -1, len;
+    unsigned int net_dest;
+    char *cur, *buf = NULL;
+    enum {MAX_ROUTE_SIZE = 1024*64};
+    virSocketAddr inaddress, innetmask;
+
+    if (!network->def->ipAddress || !network->def->netmask)
+        return 0;
+
+    if (virSocketParseAddr(network->def->ipAddress, &inaddress, 0) < 0) {
+        networkReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("cannot parse IP address '%s'"),
+                           network->def->ipAddress);
+        goto error;
+    }
+
+    if (virSocketParseAddr(network->def->netmask, &innetmask, 0) < 0) {
+        networkReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("cannot parse netmask '%s'"),
+                           network->def->netmask);
+        goto error;
+    }
+
+    if (inaddress.stor.ss_family != AF_INET ||
+        innetmask.stor.ss_family != AF_INET) {
+        /* Only support collision check for IPv4 */
+        goto out;
+    }
+
+    net_dest = (inaddress.inet4.sin_addr.s_addr &
+                innetmask.inet4.sin_addr.s_addr);
+
+    /* Read whole routing table into memory */
+    if ((len = virFileReadAll(PROC_NET_ROUTE, MAX_ROUTE_SIZE, &buf)) < 0)
+        goto error;
+
+    /* Dropping the last character shouldn't hurt */
+    if (len > 0)
+        buf[len-1] = '\0';
+
+    VIR_DEBUG("%s output:\n%s", PROC_NET_ROUTE, buf);
+
+    if (!STRPREFIX (buf, "Iface"))
+        goto out;
+
+    /* First line is just headings, skip it */
+    cur = strchr(buf, '\n');
+    if (cur)
+        cur++;
+
+    while (cur) {
+        char iface[17], dest[128], mask[128];
+        unsigned int addr_val, mask_val;
+        int num;
+
+        /* NUL-terminate the line, so sscanf doesn't go beyond a newline.  */
+        char *nl = strchr(cur, '\n');
+        if (nl) {
+            *nl++ = '\0';
+        }
+
+        num = sscanf(cur, "%16s %127s %*s %*s %*s %*s %*s %127s",
+                     iface, dest, mask);
+        cur = nl;
+
+        if (num != 3) {
+            VIR_DEBUG("Failed to parse %s", PROC_NET_ROUTE);
+            continue;
+        }
+
+        if (virStrToLong_ui(dest, NULL, 16, &addr_val) < 0) {
+            VIR_DEBUG("Failed to convert network address %s to uint", dest);
+            continue;
+        }
+
+        if (virStrToLong_ui(mask, NULL, 16, &mask_val) < 0) {
+            VIR_DEBUG("Failed to convert network mask %s to uint", mask);
+            continue;
+        }
+
+        addr_val &= mask_val;
+
+        if ((net_dest == addr_val) &&
+            (innetmask.inet4.sin_addr.s_addr == mask_val)) {
+            networkReportError(VIR_ERR_INTERNAL_ERROR,
+                              _("Network %s/%s is already in use by "
+                                "interface %s"),
+                                network->def->ipAddress,
+                                network->def->netmask, iface);
+            goto error;
+        }
+    }
+
+out:
+    ret = 0;
+error:
+    VIR_FREE(buf);
+    return ret;
+}
+
 static int networkStartNetworkDaemon(struct network_driver *driver,
                                      virNetworkObjPtr network)
 {
@@ -919,6 +1028,10 @@ static int networkStartNetworkDaemon(struct network_driver *driver,
         return -1;
     }
 
+    /* Check to see if network collides with an existing route */
+    if (networkCheckRouteCollision(network) < 0)
+        return -1;
+
     if ((err = brAddBridge(driver->brctl, network->def->bridge))) {
         virReportSystemError(err,
                              _("cannot create bridge '%s'"),