Merge pull request #1772 from brauner/2017-08-31/ensure_lxc_user_nic_tests_privilege_over_netns

src/lxc/confile_utils.c

@@ -261,6 +261,12 @@ void lxc_log_configured_netdevs(const struct lxc_conf *conf)
 			if (netdev->priv.veth_attr.pair)
 				TRACE("veth pair: %s",
 				      netdev->priv.veth_attr.pair);
+			if (netdev->priv.veth_attr.veth1[0] != '\0')
+				TRACE("veth1 : %s",
+				      netdev->priv.veth_attr.veth1);
+			if (netdev->priv.veth_attr.ifindex > 0)
+				TRACE("host side ifindex for veth device: %d",
+				      netdev->priv.veth_attr.ifindex);
 			break;
 		case LXC_NET_MACVLAN:
 			TRACE("type: macvlan");

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);
@@ -976,13 +976,89 @@ struct user_nic_args {
 #define LXC_USERNIC_CREATE 0
 #define LXC_USERNIC_DELETE 1
 
+static bool is_privileged_over_netns(int netns_fd)
+{
+	int ret;
+	uid_t euid, ruid, suid;
+	bool bret = false;
+	int ofd = -1;
+
+	ofd = lxc_preserve_ns(getpid(), "net");
+	if (ofd < 0) {
+		usernic_error("Failed opening network namespace path for %d", getpid());
+		return false;
+	}
+
+	ret = getresuid(&ruid, &euid, &suid);
+	if (ret < 0) {
+		usernic_error("Failed to retrieve real, effective, and saved "
+			      "user IDs: %s\n",
+			      strerror(errno));
+		goto do_partial_cleanup;
+	}
+
+	ret = setns(netns_fd, CLONE_NEWNET);
+	if (ret < 0) {
+		usernic_error("Failed to setns() to network namespace %s\n",
+			      strerror(errno));
+		goto do_partial_cleanup;
+	}
+
+	ret = setresuid(ruid, ruid, 0);
+	if (ret < 0) {
+		usernic_error("Failed to drop privilege by setting effective "
+			      "user id and real user id to %d, and saved user "
+			      "ID to 0: %s\n",
+			      ruid, strerror(errno));
+		/* It's ok to jump to do_full_cleanup here since setresuid()
+		 * will succeed when trying to set real, effective, and saved to
+		 * values they currently have.
+		 */
+		goto do_full_cleanup;
+	}
+
+	/* Test whether we are privileged over the network namespace. To do this
+	 * we try to delete the loopback interface which is not possible. If we
+	 * are privileged over the network namespace we will get ENOTSUP. If we
+	 * are not privileged over the network namespace we will get EPERM.
+	 */
+	ret = lxc_netdev_delete_by_name("lo");
+	if (ret == -ENOTSUP)
+		bret = true;
+
+do_full_cleanup:
+	ret = setresuid(ruid, euid, suid);
+	if (ret < 0) {
+		usernic_error("Failed to restore privilege by setting "
+			      "effective user id to %d, real user id to %d, "
+			      "and saved user ID to %d: %s\n", ruid, euid, suid,
+			      strerror(errno));
+
+		bret = false;
+	}
+
+	ret = setns(ofd, CLONE_NEWNET);
+	if (ret < 0) {
+		usernic_error("Failed to setns() to original network namespace "
+			      "of PID %d: %s\n", ofd, strerror(errno));
+
+		bret = false;
+	}
+
+do_partial_cleanup:
+
+	close(ofd);
+	return bret;
+}
+
 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;
 	char *cnic = NULL, *nicname = NULL;
 	struct alloted_s *alloted = NULL;
-	struct user_nic_args args;
 
 	if (argc < 7 || argc > 8) {
 		usage(argv[0], true);
@@ -1027,26 +1103,50 @@ int main(int argc, char *argv[])
 		exit(EXIT_FAILURE);
 	}
 
-	ret = lxc_safe_int(args.pid, &pid);
-	if (ret < 0) {
-		usernic_error("Could not read pid: %s\n", args.pid);
-		exit(EXIT_FAILURE);
+	if (request == LXC_USERNIC_CREATE) {
+		ret = lxc_safe_int(args.pid, &pid);
+		if (ret < 0) {
+			usernic_error("Could not read pid: %s\n", args.pid);
+			exit(EXIT_FAILURE);
+		}
+	} else if (request == LXC_USERNIC_DELETE) {
+		netns_fd = open(args.pid, O_RDONLY);
+		if (netns_fd < 0) {
+			usernic_error("Could not open \"%s\": %s\n", args.pid,
+				      strerror(errno));
+			exit(EXIT_FAILURE);
+		}
 	}
 
 	if (!create_db_dir(LXC_USERNIC_DB)) {
 		usernic_error("%s", "Failed to create directory for db file\n");
+		if (netns_fd >= 0)
+			close(netns_fd);
 		exit(EXIT_FAILURE);
 	}
 
 	fd = open_and_lock(LXC_USERNIC_DB);
 	if (fd < 0) {
 		usernic_error("Failed to lock %s\n", LXC_USERNIC_DB);
+		if (netns_fd >= 0)
+			close(netns_fd);
 		exit(EXIT_FAILURE);
 	}
 
-	if (!may_access_netns(pid)) {
-		usernic_error("User %s may not modify netns for pid %d\n", me, pid);
-		exit(EXIT_FAILURE);
+	if (request == LXC_USERNIC_CREATE) {
+		if (!may_access_netns(pid)) {
+			usernic_error("User %s may not modify netns for pid %d\n", me, pid);
+			exit(EXIT_FAILURE);
+		}
+	} else if (request == LXC_USERNIC_DELETE) {
+		bool has_priv;
+		has_priv = is_privileged_over_netns(netns_fd);
+		close(netns_fd);
+		if (!has_priv) {
+			usernic_error("%s", "Process is not privileged over "
+					    "network namespace\n");
+			exit(EXIT_FAILURE);
+		}
 	}
 
 	n = get_alloted(me, args.type, args.link, &alloted);
@@ -1104,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);
@@ -1113,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);

src/lxc/network.h

@@ -79,9 +79,21 @@ struct lxc_route6 {
 	struct in6_addr addr;
 };
 
+/* Contains information about the host side veth device.
+ * @pair    : Name of the host side veth device.
+ *            If the user requested that the host veth device be created with a
+ *            specific names this field will be set. If this field is set @veth1
+ *            is not set.
+ * @veth1   : Name of the host side veth device.
+ *            If the user did not request that the host veth device be created
+ *            with a specific name this field will be set. If this field is set
+ *            @pair is not set.
+ * @ifindex : Ifindex of the network device.
+ */
 struct ifla_veth {
-	char *pair; /* pair name */
-	char veth1[IFNAMSIZ]; /* needed for deconf */
+	char *pair;
+	char veth1[IFNAMSIZ];
+	int ifindex;
 };
 
 struct ifla_vlan {
@@ -103,20 +115,42 @@ union netdev_p {
 
 /*
  * Defines a structure to configure a network device
- * @link       : lxc.net.[i].link, name of bridge or host iface to attach if any
- * @name       : lxc.net.[i].name, name of iface on the container side
- * @flags      : flag of the network device (IFF_UP, ... )
- * @ipv4       : a list of ipv4 addresses to be set on the network device
- * @ipv6       : a list of ipv6 addresses to be set on the network device
- * @upscript   : a script filename to be executed during interface configuration
- * @downscript : a script filename to be executed during interface destruction
- * @idx        : network counter
+ * @idx               : network counter
+ * @ifindex           : ifindex of the network device
+ *                      Note that this is the ifindex of the network device in
+ *                      the container's network namespace. If the network device
+ *                      consists of a pair of network devices (e.g. veth pairs
+ *                      attached to a network bridge) then this index cannot be
+ *                      used to identify or modify the host veth device. See
+ *                      struct ifla_veth for the host side information.
+ * @type              : network type (veth, macvlan, vlan, ...)
+ * @flags             : flag of the network device (IFF_UP, ... )
+ * @link              : lxc.net.[i].link, name of bridge or host iface to attach
+ *                      if any
+ * @name              : lxc.net.[i].name, name of iface on the container side
+ * @hwaddr            : mac address
+ * @mtu               : maximum transmission unit
+ * @priv              : information specific to the specificed network type
+ *                      Note that this is a union so whether accessing a struct
+ *                      is possible is dependent on the network type.
+ * @ipv4              : a list of ipv4 addresses to be set on the network device
+ * @ipv6              : a list of ipv6 addresses to be set on the network device
+ * @ipv4_gateway_auto : whether the ipv4 gateway is to be automatically gathered
+ *                      from the associated @link
+ * @ipv4_gateway      : ipv4 gateway
+ * @ipv6_gateway_auto : whether the ipv6 gateway is to be automatically gathered
+ *                      from the associated @link
+ * @ipv6_gateway      : ipv6 gateway
+ * @upscript          : a script filename to be executed during interface
+ *                      configuration
+ * @downscript        : a script filename to be executed during interface
+ *                      destruction
  */
 struct lxc_netdev {
 	ssize_t idx;
+	int ifindex;
 	int type;
 	int flags;
-	int ifindex;
 	char *link;
 	char *name;
 	char *hwaddr;
@@ -124,10 +158,10 @@ struct lxc_netdev {
 	union netdev_p priv;
 	struct lxc_list ipv4;
 	struct lxc_list ipv6;
-	struct in_addr *ipv4_gateway;
 	bool ipv4_gateway_auto;
-	struct in6_addr *ipv6_gateway;
+	struct in_addr *ipv4_gateway;
 	bool ipv6_gateway_auto;
+	struct in6_addr *ipv6_gateway;
 	char *upscript;
 	char *downscript;
 };
@@ -224,10 +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 bool lxc_delete_network(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,

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;
@@ -1312,6 +1317,7 @@ static int lxc_spawn(struct lxc_handler *handler)
 		SYSERROR("Failed to clone a new set of namespaces.");
 		goto out_delete_net;
 	}
+
 	for (i = 0; i < LXC_NS_MAX; i++)
 		if (flags & ns_info[i].clone_flag)
 			INFO("Cloned %s.", ns_info[i].flag_name);
@@ -1363,13 +1369,34 @@ static int lxc_spawn(struct lxc_handler *handler)
 	if (failed_before_rename)
 		goto out_delete_net;
 
+	handler->netnsfd = lxc_preserve_ns(handler->pid, "net");
+	if (handler->netnsfd < 0) {
+		ERROR("Failed to preserve network namespace");
+		goto out_delete_net;
+	}
+
 	/* 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) {
@@ -1437,15 +1464,22 @@ static int lxc_spawn(struct lxc_handler *handler)
 	}
 
 	lxc_sync_fini(handler);
-	handler->netnsfd = lxc_preserve_ns(handler->pid, "net");
 
 	return 0;
 
 out_delete_net:
 	if (cgroups_connected)
 		cgroup_disconnect();
-	if (handler->clone_flags & CLONE_NEWNET)
-		lxc_delete_network(handler);
+
+	if (handler->clone_flags & CLONE_NEWNET) {
+		DEBUG("Tearing down network devices");
+		if (!lxc_delete_network_priv(handler))
+			DEBUG("Failed tearing down network devices");
+
+		if (!lxc_delete_network_unpriv(handler))
+			DEBUG("Failed tearing down network devices");
+	}
+
 out_abort:
 	lxc_abort(name, handler);
 	lxc_sync_fini(handler);
@@ -1454,6 +1488,11 @@ out_abort:
 		handler->pinfd = -1;
 	}
 
+	if (handler->netnsfd >= 0) {
+		close(handler->netnsfd);
+		handler->netnsfd = -1;
+	}
+
 	return -1;
 }
 
@@ -1463,7 +1502,6 @@ int __lxc_start(const char *name, struct lxc_handler *handler,
 {
 	int status;
 	int err = -1;
-	bool removed_all_netdevs = true;
 	struct lxc_conf *conf = handler->conf;
 
 	if (lxc_init(name, handler) < 0) {
@@ -1509,10 +1547,6 @@ int __lxc_start(const char *name, struct lxc_handler *handler,
 	err = lxc_poll(name, handler);
 	if (err) {
 		ERROR("LXC mainloop exited with error: %d.", err);
-		if (handler->netnsfd >= 0) {
-			close(handler->netnsfd);
-			handler->netnsfd = -1;
-		}
 		goto out_abort;
 	}
 
@@ -1544,9 +1578,6 @@ int __lxc_start(const char *name, struct lxc_handler *handler,
 	DEBUG("Pushing physical nics back to host namespace");
 	lxc_restore_phys_nics_to_netns(handler->netnsfd, handler->conf);
 
-	DEBUG("Tearing down virtual network devices used by container \"%s\".", name);
-	removed_all_netdevs = lxc_delete_network(handler);
-
 	if (handler->pinfd >= 0) {
 		close(handler->pinfd);
 		handler->pinfd = -1;
@@ -1554,12 +1585,18 @@ int __lxc_start(const char *name, struct lxc_handler *handler,
 
 	lxc_monitor_send_exit_code(name, status, handler->lxcpath);
 	err =  lxc_error_set_and_log(handler->pid, status);
+
 out_fini:
-	if (!removed_all_netdevs) {
-		DEBUG("Failed tearing down network devices used by container. Trying again!");
-		removed_all_netdevs = lxc_delete_network(handler);
-		if (!removed_all_netdevs)
-			DEBUG("Failed tearing down network devices used by container. Not trying again!");
+	DEBUG("Tearing down network devices");
+	if (!lxc_delete_network_priv(handler))
+		DEBUG("Failed tearing down network devices");
+
+	if (!lxc_delete_network_unpriv(handler))
+		DEBUG("Failed tearing down network devices");
+
+	if (handler->netnsfd >= 0) {
+		close(handler->netnsfd);
+		handler->netnsfd = -1;
 	}
 
 out_detach_blockdev: