network: rework network creation

- On unprivileged veth network creation have lxc-user-nic send the names of the
  veth devices and their respective ifindeces. The advantage of retrieving this
  information from lxc-user-nic is that we spare us sending around more stuff
  via the netpipe in start.c. Also, lxc-user-nic operates in both namespaces
  (the container's namespace and the hosts's namespace) via setns and so is
  guaranteed to retrieve the correct ifindex via if_nametoindex() which is an
  network namespace aware ioctl() call. While I'm pretty sure the ifindeces for
  veth devices are identical across network namespaces I'm weary to rely on
  this. We need the ifindexes to guarantee safe deletion of unprivileged
  network devices via lxc-user-nic later on since we use them to identify the
  network devices in their corresponding network namespaces.
- Move the network device logging from the child to the parent. The child does
  not have all of the information about the network devices available only the
  few bits it actually needs to now. The monitor process is the only process
  that needs all this information.
- The network creation code for privileged and unprivileged networks was
  previously mangled into one single function but at the same time some of the
  privileged code had additional functions that were called in other places in
  start.c. Let's divide and conquer and split out the privileged and
  unprivileged network creation into completely separate functions. This makes
  what's happening way more clear. This will also have no performance impact
  since either you are privileged and only execute the privileged network
  creation functions or you are unprivileged and only execute the unprivileged
  network creation functions.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/lxc_user_nic.c b/src/lxc/lxc_user_nic.c
index 4c446f7c36c6a29e654b26cc2439a20b766c8e9e..1853e0412c9acc8dfcd8df21f499090d86d7f090 100644 (file)
--- a/src/lxc/lxc_user_nic.c
+++ b/src/lxc/lxc_user_nic.c
@@ -797,7 +797,7 @@ again:
 }
 
 static char *lxc_secure_rename_in_ns(int pid, char *oldname, char *newname,
-                                    int *ifidx)
+                                    int *container_veth_ifidx)
 {
        int ret;
        uid_t ruid, suid, euid;
@@ -881,7 +881,7 @@ static char *lxc_secure_rename_in_ns(int pid, char *oldname, char *newname,
        /* Allocation failure for strdup() is checked below. */
        name = strdup(ifname);
        string_ret = name;
-       *ifidx = ifindex;
+       *container_veth_ifidx = ifindex;
 
 do_full_cleanup:
        ret = setresuid(ruid, euid, suid);
@@ -1053,7 +1053,7 @@ do_partial_cleanup:
 
 int main(int argc, char *argv[])
 {
-       int fd, ifindex, n, pid, request, ret;
+       int container_veth_ifidx, fd, host_veth_ifidx, n, pid, request, ret;
        char *me, *newname;
        struct user_nic_args args;
        int netns_fd = -1;
@@ -1204,7 +1204,8 @@ int main(int argc, char *argv[])
        }
 
        /* Now rename the link. */
-       newname = lxc_secure_rename_in_ns(pid, cnic, args.veth_name, &ifindex);
+       newname = lxc_secure_rename_in_ns(pid, cnic, args.veth_name,
+                                         &container_veth_ifidx);
        if (!newname) {
                usernic_error("%s", "Failed to rename the link\n");
                ret = lxc_netdev_delete_by_name(cnic);
@@ -1213,9 +1214,13 @@ int main(int argc, char *argv[])
                free(nicname);
                exit(EXIT_FAILURE);
        }
+       host_veth_ifidx = if_nametoindex(nicname);
 
-       /* Write the name of the interface pair to the stdout: eth0:veth9MT2L4 */
-       fprintf(stdout, "%s:%s:%d\n", newname, nicname, ifindex);
+       /* Write names of veth pairs and their ifindeces to stout:
+        * (e.g. eth0:731:veth9MT2L4:730)
+        */
+       fprintf(stdout, "%s:%d:%s:%d\n", newname, container_veth_ifidx, nicname,
+               host_veth_ifidx);
        free(newname);
        free(nicname);
        exit(EXIT_SUCCESS);

diff --git a/src/lxc/network.c b/src/lxc/network.c
index 315b44d7964412a655db07c8c05992fe49843496..6c867c65488106c1fdaecfbe44c95a1e6b32f92d 100644 (file)
--- a/src/lxc/network.c
+++ b/src/lxc/network.c
@@ -47,7 +47,6 @@
 
 #include "conf.h"
 #include "config.h"
-#include "confile_utils.h"
 #include "log.h"
 #include "network.h"
 #include "nl.h"
@@ -2033,8 +2032,8 @@ int lxc_find_gateway_addresses(struct lxc_handler *handler)
 }
 
 #define LXC_USERNIC_PATH LIBEXECDIR "/lxc/lxc-user-nic"
-static int lxc_create_network_unpriv(const char *lxcpath, char *lxcname,
-                                    struct lxc_netdev *netdev, pid_t pid)
+static int lxc_create_network_unpriv_exec(const char *lxcpath, char *lxcname,
+                                         struct lxc_netdev *netdev, pid_t pid)
 {
        int ret;
        pid_t child;
@@ -2107,7 +2106,7 @@ static int lxc_create_network_unpriv(const char *lxcpath, char *lxcname,
 
        bytes = read(pipefd[0], &buffer, MAXPATHLEN);
        if (bytes < 0) {
-               SYSERROR("Failed to read from pipe file descriptor.");
+               SYSERROR("Failed to read from pipe file descriptor");
                close(pipefd[0]);
                return -1;
        }
@@ -2124,36 +2123,58 @@ static int lxc_create_network_unpriv(const char *lxcpath, char *lxcname,
 
        /* netdev->name */
        token = strtok_r(buffer, ":", &saveptr);
-       if (!token)
+       if (!token) {
+               ERROR("Failed to parse lxc-user-nic output");
                return -1;
+       }
 
        netdev->name = malloc(IFNAMSIZ + 1);
        if (!netdev->name) {
-               SYSERROR("Failed to allocate memory.");
+               SYSERROR("Failed to allocate memory");
                return -1;
        }
        memset(netdev->name, 0, IFNAMSIZ + 1);
        strncpy(netdev->name, token, IFNAMSIZ);
 
-       /* netdev->priv.veth_attr.pair */
+       /* netdev->ifindex */
        token = strtok_r(NULL, ":", &saveptr);
-       if (!token)
+       if (!token) {
+               ERROR("Failed to parse lxc-user-nic output");
                return -1;
+       }
 
-       netdev->priv.veth_attr.pair = strdup(token);
-       if (!netdev->priv.veth_attr.pair) {
-               ERROR("Failed to allocate memory.");
+       ret = lxc_safe_int(token, &netdev->ifindex);
+       if (ret < 0) {
+               ERROR("%s - Failed to convert string \"%s\" to integer",
+                     strerror(-ret), token);
                return -1;
        }
 
-       /* netdev->ifindex */
+       /* netdev->priv.veth_attr.veth1 */
        token = strtok_r(NULL, ":", &saveptr);
-       if (!token)
+       if (!token) {
+               ERROR("Failed to parse lxc-user-nic output");
                return -1;
+       }
 
-       ret = lxc_safe_int(token, &netdev->ifindex);
+       if (strlen(token) >= IFNAMSIZ) {
+               ERROR("Host side veth device name returned by lxc-user-nic is "
+                     "too long");
+               return -E2BIG;
+       }
+       strcpy(netdev->priv.veth_attr.veth1, token);
+
+       /* netdev->priv.veth_attr.ifindex */
+       token = strtok_r(NULL, ":", &saveptr);
+       if (!token) {
+               ERROR("Failed to parse lxc-user-nic output");
+               return -1;
+       }
+
+       ret = lxc_safe_int(token, &netdev->priv.veth_attr.ifindex);
        if (ret < 0) {
-               ERROR("Failed to parse ifindex for network device \"%s\"", netdev->name);
+               ERROR("%s - Failed to convert string \"%s\" to integer",
+                     strerror(-ret), token);
                return -1;
        }
 
@@ -2202,15 +2223,22 @@ static int lxc_delete_network_unpriv_exec(const char *lxcpath, char *lxcname,
                        exit(EXIT_FAILURE);
                }
 
-               if (!netdev->link)
+               if (netdev->priv.veth_attr.veth1[0] == '\0') {
+                       SYSERROR("Host side veth device name is missing");
+                       exit(EXIT_FAILURE);
+               }
+
+               if (!netdev->link) {
                        SYSERROR("Network link for network device \"%s\" is "
-                                "missing", netdev->priv.veth_attr.pair);
+                                "missing", netdev->priv.veth_attr.veth1);
+                       exit(EXIT_FAILURE);
+               }
 
                INFO("Execing lxc-user-nic delete %s %s %s veth %s %s", lxcpath,
-                    lxcname, netns_path, netdev->link, netdev->priv.veth_attr.pair);
+                    lxcname, netns_path, netdev->link, netdev->priv.veth_attr.veth1);
                execlp(LXC_USERNIC_PATH, LXC_USERNIC_PATH, "delete", lxcpath,
                       lxcname, netns_path, "veth", netdev->link,
-                      netdev->priv.veth_attr.pair, (char *)NULL);
+                      netdev->priv.veth_attr.veth1, (char *)NULL);
                SYSERROR("Failed to exec lxc-user-nic.");
                exit(EXIT_FAILURE);
        }
@@ -2312,7 +2340,7 @@ bool lxc_delete_network_unpriv(struct lxc_handler *handler)
                        deleted_all = false;
                        WARN("Failed to remove port \"%s\" from openvswitch "
                             "bridge \"%s\"",
-                            netdev->priv.veth_attr.pair, netdev->link);
+                            netdev->priv.veth_attr.veth1, netdev->link);
                        continue;
                }
                INFO("Removed interface \"%s\" from \"%s\"", hostveth,
@@ -2351,33 +2379,19 @@ int lxc_create_network_priv(struct lxc_handler *handler)
        return 0;
 }
 
-int lxc_create_network(const char *lxcpath, char *lxcname,
-                      struct lxc_list *network, pid_t pid)
+int lxc_network_move_created_netdev_priv(const char *lxcpath, char *lxcname,
+                                        struct lxc_list *network, pid_t pid)
 {
        int err;
-       bool am_root;
        char ifname[IFNAMSIZ];
        struct lxc_list *iterator;
 
-       am_root = (getuid() == 0);
+       if (am_unpriv())
+               return 0;
 
        lxc_list_for_each(iterator, network) {
                struct lxc_netdev *netdev = iterator->elem;
 
-               if (netdev->type == LXC_NET_VETH && !am_root) {
-                       if (netdev->mtu)
-                               INFO("mtu ignored due to insufficient privilege");
-                       if (lxc_create_network_unpriv(lxcpath, lxcname, netdev, pid))
-                               return -1;
-                       /* lxc-user-nic has moved the nic to the new ns.
-                        * unpriv_assign_nic() fills in netdev->name.
-                        * netdev->ifindex will be filled in at
-                        * lxc_setup_netdev_in_child_namespaces().
-                        */
-                       continue;
-               }
-
-               /* empty network namespace, nothing to move */
                if (!netdev->ifindex)
                        continue;
 
@@ -2404,6 +2418,40 @@ int lxc_create_network(const char *lxcpath, char *lxcname,
        return 0;
 }
 
+int lxc_create_network_unpriv(const char *lxcpath, char *lxcname,
+                             struct lxc_list *network, pid_t pid)
+{
+       struct lxc_list *iterator;
+
+       if (!am_unpriv())
+               return 0;
+
+       lxc_list_for_each(iterator, network) {
+               struct lxc_netdev *netdev = iterator->elem;
+
+               if (netdev->type == LXC_NET_EMPTY)
+                       continue;
+
+               if (netdev->type == LXC_NET_NONE)
+                       continue;
+
+               if (netdev->type != LXC_NET_VETH) {
+                       ERROR("Networks of type %s are not supported by "
+                             "unprivileged containers",
+                             lxc_net_type_to_str(netdev->type));
+                       return -1;
+               }
+
+               if (netdev->mtu)
+                       INFO("mtu ignored due to insufficient privilege");
+
+               if (lxc_create_network_unpriv_exec(lxcpath, lxcname, netdev, pid))
+                       return -1;
+       }
+
+       return 0;
+}
+
 bool lxc_delete_network_priv(struct lxc_handler *handler)
 {
        int ret;
@@ -2854,7 +2902,7 @@ static int lxc_setup_netdev_in_child_namespaces(struct lxc_netdev *netdev)
                }
        }
 
-       DEBUG("Network devie \"%s\" has been setup", current_ifname);
+       DEBUG("Network device \"%s\" has been setup", current_ifname);
 
        return 0;
 }
@@ -2865,8 +2913,6 @@ int lxc_setup_network_in_child_namespaces(const struct lxc_conf *conf,
        struct lxc_list *iterator;
        struct lxc_netdev *netdev;
 
-       lxc_log_configured_netdevs(conf);
-
        lxc_list_for_each(iterator, network) {
                netdev = iterator->elem;
 

diff --git a/src/lxc/network.h b/src/lxc/network.h
index 0bea49448eb68e9de2c4683d870522bc190d5710..17c095802e072764f5c55a998166091824130ff7 100644 (file)
--- a/src/lxc/network.h
+++ b/src/lxc/network.h
@@ -258,11 +258,15 @@ extern const char *lxc_net_type_to_str(int type);
 extern int setup_private_host_hw_addr(char *veth1);
 extern int netdev_get_mtu(int ifindex);
 extern int lxc_create_network_priv(struct lxc_handler *handler);
+extern int lxc_network_move_created_netdev_priv(const char *lxcpath,
+                                               char *lxcname,
+                                               struct lxc_list *network,
+                                               pid_t pid);
 extern bool lxc_delete_network_priv(struct lxc_handler *handler);
 extern bool lxc_delete_network_unpriv(struct lxc_handler *handler);
 extern int lxc_find_gateway_addresses(struct lxc_handler *handler);
-extern int lxc_create_network(const char *lxcpath, char *lxcname,
-                             struct lxc_list *network, pid_t pid);
+extern int lxc_create_network_unpriv(const char *lxcpath, char *lxcname,
+                                    struct lxc_list *network, pid_t pid);
 extern int lxc_requests_empty_network(struct lxc_handler *handler);
 extern void lxc_restore_phys_nics_to_netns(int netnsfd, struct lxc_conf *conf);
 extern int lxc_setup_network_in_child_namespaces(const struct lxc_conf *conf,

diff --git a/src/lxc/start.c b/src/lxc/start.c
index c41b8dfdd8da5878ae4d66499dcfd5bf03a4236f..bc5daa764a77c4e6c80baabe118a5362119652d4 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -68,6 +68,7 @@
 #include "commands.h"
 #include "commands_utils.h"
 #include "conf.h"
+#include "confile_utils.h"
 #include "console.h"
 #include "error.h"
 #include "log.h"
@@ -809,15 +810,19 @@ static int read_unpriv_netifindex(struct lxc_list *network)
 
        if (netpipe == -1)
                return 0;
+
        lxc_list_for_each(iterator, network) {
                netdev = iterator->elem;
                if (netdev->type != LXC_NET_VETH)
                        continue;
-               if (!(netdev->name = malloc(IFNAMSIZ))) {
+
+               netdev->name = malloc(IFNAMSIZ);
+               if (!netdev->name) {
                        ERROR("Out of memory.");
                        close(netpipe);
                        return -1;
                }
+
                if (read(netpipe, netdev->name, IFNAMSIZ) != IFNAMSIZ) {
                        close(netpipe);
                        return -1;
@@ -1372,11 +1377,26 @@ static int lxc_spawn(struct lxc_handler *handler)
 
        /* Create the network configuration. */
        if (handler->clone_flags & CLONE_NEWNET) {
-               if (lxc_create_network(handler->lxcpath, handler->name,
-                                      &handler->conf->network, handler->pid)) {
+               if (lxc_network_move_created_netdev_priv(handler->lxcpath,
+                                                        handler->name,
+                                                        &handler->conf->network,
+                                                        handler->pid)) {
                        ERROR("Failed to create the configured network.");
                        goto out_delete_net;
                }
+
+               if (lxc_create_network_unpriv(handler->lxcpath, handler->name,
+                                             &handler->conf->network,
+                                             handler->pid)) {
+                       ERROR("Failed to create the configured network.");
+                       goto out_delete_net;
+               }
+
+               /* Now all networks are created and moved into place. The
+                * corresponding structs have now all been filled. So log them
+                * for debugging purposes.
+                */
+               lxc_log_configured_netdevs(handler->conf);
        }
 
        if (netpipe != -1) {