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 6461b03b99b9457a92534bb73c800149eb24421b..8d565ed531dc9865a482366e45b2e76d8cab9796 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"
@@ -2005,8 +2004,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;
@@ -2079,7 +2078,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;
        }
@@ -2096,36 +2095,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;
        }
 
@@ -2174,15 +2195,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);
        }
@@ -2284,7 +2312,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,
@@ -2323,33 +2351,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;
 
@@ -2376,6 +2390,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;
@@ -2826,7 +2874,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;
 }
@@ -2837,8 +2885,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 4c359dc6cddf91a69728a3f660a609f5245731ce..3ef016642cbe358c2d60d7efa87cf325a9049600 100644 (file)
--- a/src/lxc/network.h
+++ b/src/lxc/network.h
@@ -291,11 +291,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 cb9a68cf65333f2ef4c8b3ce7735ef515a00e12d..0ced4e9186ce99db788f5e79454da821d61bfa50 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -60,6 +60,7 @@
 #include "commands.h"
 #include "commands_utils.h"
 #include "conf.h"
+#include "confile_utils.h"
 #include "console.h"
 #include "error.h"
 #include "log.h"
@@ -875,15 +876,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;
@@ -1437,11 +1442,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) {