From: Christian Brauner Date: Thu, 31 Aug 2017 13:30:39 +0000 (+0200) Subject: network: rework network creation X-Git-Tag: lxc-2.0.9~48^2~30 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=96416905e79d09e3cd572c1b3ccbb8323d43cffa;p=thirdparty%2Flxc.git 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 --- diff --git a/src/lxc/lxc_user_nic.c b/src/lxc/lxc_user_nic.c index 4c446f7c3..1853e0412 100644 --- 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 6461b03b9..8d565ed53 100644 --- 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 4c359dc6c..3ef016642 100644 --- 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 cb9a68cf6..0ced4e918 100644 --- 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) {