WIP: cli: add --no-safeguards

enter.c

@@ -93,8 +93,28 @@ int enter(struct entry_settings *opts)
 	enum nsaction nsactions[MAX_NS];
 	opts_to_nsactions(opts->shares, nsactions);
 
-	if (nsactions[NS_NET] != NSACTION_UNSHARE && opts->nnics > 0) {
-		errx(1, "cannot create NICs when not in a network namespace");
+	/* Don't shoot ourselves in the foot. It's technically possible to
+	   let users mount things in the host mount namespace but in practice
+	   it's a terrible idea due to the sheer amount of things that can go
+	   wrong, like "what do I do if one of the mounts failed but the previous
+	   ones didn't?", or "how do I clean up things that I've (re)mounted?".
+	   The same argument applies for other system resources, like network
+	   interfaces.
+
+	   Instead of allowing this by default, we only skip validation when
+	   the user explicitly passed --no-safeguards. This is because there
+	   are legitimate use-cases in changing non-unshared namespaces, like
+	   using multiple calls to bst to setup an environment in multiple
+	   stages. */
+
+	if (!opts->no_safeguards) {
+		if (nsactions[NS_NET] != NSACTION_UNSHARE && opts->nnics > 0) {
+			errx(1, "cannot create NICs when not in a network namespace");
+		}
+
+		if (nsactions[NS_MNT] != NSACTION_UNSHARE && opts->nmounts > 0) {
+			errx(1, "cannot create mounts when not in a mount namespace");
+		}
 	}
 
 	struct outer_helper outer_helper;
@@ -279,15 +299,9 @@ int enter(struct entry_settings *opts)
 	int rtnl = init_rtnetlink_socket();
 
 	/* Rename interfaces according to their specifications */
-	if (net_unshare) {
-		for (size_t i = 0; i < opts->nnics; ++i) {
-			if (i > (size_t) INT_MAX - 2) {
-				errx(1, "cannot iterate over more than %d interfaces", INT_MAX - 2);
-			}
-			/* interface indices start from 1, and we want to ignore interface 1 (lo),
-			   so we slide our indices by 2. */
-			net_if_rename(rtnl, (int)i + 2, opts->nics[i].name);
-		}
+	for (size_t i = 0; i < outer_helper.nnics; ++i) {
+		struct nic_options *nic = &outer_helper.nics[i];
+		net_if_rename(rtnl, nic->idx, nic->name);
 	}
 
 	if (opts->setup_program) {
@@ -391,11 +405,11 @@ int enter(struct entry_settings *opts)
 		if (!opts->no_loopback_setup) {
 			net_if_up(rtnl, "lo");
 		}
+	}
 
-		/* Bring up the rest of the nics */
-		for (size_t i = 0; i < opts->nnics; ++i) {
-			net_if_up(rtnl, opts->nics[i].name);
-		}
+	/* Bring up the rest of the nics */
+	for (size_t i = 0; i < opts->nnics; ++i) {
+		net_if_up(rtnl, opts->nics[i].name);
 	}
 
 	/* We have a special case for pivot_root: the syscall wants the
@@ -410,15 +424,6 @@ int enter(struct entry_settings *opts)
 	   tmpfses in user namespaces forces the options uid=<real-uid> and
 	   gid=<real-gid>. */
 	if (opts->nmounts > 0) {
-		/* Don't shoot ourselves in the foot. It's technically possible to
-		   let users mount things in the host mount namespace but in practice
-		   it's a terrible idea due to the sheer amount of things that can go
-		   wrong, like "what do I do if one of the mounts failed but the previous
-		   ones didn't?", or "how do I clean up things that I've (re)mounted?". */
-		if (!mnt_unshare) {
-			errx(1, "attempted to mount things in an existing mount namespace.");
-		}
-
 		if (!opts->no_fake_devtmpfs) {
 			for (struct mount_entry *mnt = opts->mounts; mnt < opts->mounts + opts->nmounts; ++mnt) {
 				if (strcmp(mnt->type, "devtmpfs") == 0) {

enter.h

@@ -82,6 +82,7 @@ struct entry_settings {
 	int no_init;
 	int no_loopback_setup;
 	int no_env;
+	int no_safeguards;
 };
 
 int enter(struct entry_settings *opts);

main.c

@@ -58,6 +58,7 @@ enum {
 	OPTION_NO_INIT,
 	OPTION_NO_ENV,
 	OPTION_NO_COPY_HARD_LIMITS,
+	OPTION_NO_SAFEGUARDS,
 };
 
 static void process_nslist_entry(const char **out, const char *share, const char *path, int append_nsname)
@@ -281,6 +282,7 @@ int main(int argc, char *argv[], char *envp[])
 		{ "no-loopback-setup",   no_argument, NULL, OPTION_NO_LOOPBACK_SETUP    },
 		{ "no-init",             no_argument, NULL, OPTION_NO_INIT              },
 		{ "no-env",              no_argument, NULL, OPTION_NO_ENV               },
+		{ "no-safeguards",       no_argument, NULL, OPTION_NO_SAFEGUARDS        },
 
 		{ 0, 0, 0, 0 }
 	};
@@ -558,6 +560,10 @@ int main(int argc, char *argv[], char *envp[])
 				opts.no_env = 1;
 				break;
 
+			case OPTION_NO_SAFEGUARDS:
+				opts.no_safeguards = 1;
+				break;
+
 			case 'r':
 				opts.root = optarg;
 				break;

net.c

@@ -40,7 +40,7 @@ int init_rtnetlink_socket()
 	return sockfd;
 }
 
-static int nl_sendmsg(int sockfd, const struct iovec *iov, size_t iovlen)
+static int nl_sendmsg(int sockfd, struct ifinfomsg *out, const struct iovec *iov, size_t iovlen)
 {
 	struct sockaddr_nl addr = {
 		.nl_family = AF_NETLINK,
@@ -62,14 +62,22 @@ static int nl_sendmsg(int sockfd, const struct iovec *iov, size_t iovlen)
 		struct nlmsgerr err;
 	} resp;
 
-	if (recv(sockfd, &resp, sizeof (resp), MSG_TRUNC) == -1) {
+	ssize_t size = recv(sockfd, &resp, sizeof (resp), MSG_TRUNC);
+	if (size == -1) {
 		err(1, "nl_sendmsg: recv");
 	}
+	if (size < sizeof (resp)) {
+		errx(1, "nl_sendmsg: recv: response too short");
+	}
 
 	if (resp.hdr.nlmsg_type == NLMSG_ERROR && resp.err.error != 0) {
 		errno = -resp.err.error;
 		return -1;
 	}
+
+	if (out) {
+		//*out = resp.ifinfo;
+	}
 	return 0;
 }
 
@@ -210,7 +218,7 @@ static struct nic_handler nic_handlers[] = {
 	{ NULL, NULL },
 };
 
-void net_if_add(int sockfd, const struct nic_options *nicopts)
+unsigned net_if_add(int sockfd, const struct nic_options *nicopts)
 {
 	struct nlpkt pkt;
 	nlpkt_init(&pkt);
@@ -229,13 +237,17 @@ void net_if_add(int sockfd, const struct nic_options *nicopts)
 	}
 	handler(&pkt, nicopts);
 
+	struct ifinfomsg ifinfo;
 	struct iovec iov = { .iov_base = pkt.data, .iov_len = pkt.hdr->nlhdr.nlmsg_len };
 
-	if (nl_sendmsg(sockfd, &iov, 1) == -1) {
+	if (nl_sendmsg(sockfd, &ifinfo, &iov, 1) == -1) {
 		err(1, "if_add %s %.*s", nicopts->type, IF_NAMESIZE, nicopts->name);
 	}
 
 	nlpkt_close(&pkt);
+
+	printf("idx for %s: %d\n", nicopts->name, ifinfo.ifi_index);
+	return (int) ifinfo.ifi_index;
 }
 
 void net_if_rename(int sockfd, int link, const char *to)
@@ -251,7 +263,7 @@ void net_if_rename(int sockfd, int link, const char *to)
 
 	struct iovec iov = { .iov_base = pkt.data, .iov_len = pkt.hdr->nlhdr.nlmsg_len };
 
-	if (nl_sendmsg(sockfd, &iov, 1) == -1) {
+	if (nl_sendmsg(sockfd, NULL, &iov, 1) == -1) {
 		char name[IF_NAMESIZE];
 		if_indextoname((unsigned int)link, name);
 		err(1, "if_rename %.*s -> %.*s", IF_NAMESIZE, name, IF_NAMESIZE, to);
@@ -283,7 +295,7 @@ void net_if_up(int sockfd, const char *name)
 		{ .iov_base = &ifinfo, .iov_len = sizeof (ifinfo) },
 	};
 
-	if (nl_sendmsg(sockfd, iov, sizeof (iov) / sizeof (struct iovec)) == -1) {
+	if (nl_sendmsg(sockfd, NULL, iov, sizeof (iov) / sizeof (struct iovec)) == -1) {
 		err(1, "if_up %.*s", IF_NAMESIZE, name);
 	}
 }

net.h

@@ -19,6 +19,7 @@ struct ipvlan {
 };
 
 struct nic_options {
+	unsigned idx;
 	char type[16];
 	char name[IF_NAMESIZE];
 	unsigned link_idx;
@@ -31,7 +32,7 @@ struct nic_options {
 
 int init_rtnetlink_socket();
 
-void net_if_add(int sockfd, const struct nic_options *nicopts);
+unsigned net_if_add(int sockfd, const struct nic_options *nicopts);
 void net_if_rename(int sockfd, int link, const char *to);
 void net_if_up(int sockfd, const char *name);
 

outer.c

@@ -138,7 +138,7 @@ static void create_nics(pid_t child_pid, struct nic_options *nics, size_t nnics)
 
 	for (size_t i = 0; i < nnics; ++i) {
 		nics[i].netns_pid = child_pid;
-		net_if_add(rtnl, &nics[i]);
+		nics[i].idx = net_if_add(rtnl, &nics[i]);
 	}
 
 	reset_capabilities();
@@ -267,6 +267,18 @@ void outer_helper_spawn(struct outer_helper *helper)
 		create_nics(child_pid, helper->nics, helper->nnics);
 	}
 
+	/* Send some context back to the parent process */
+	size_t remain = sizeof (helper->nics[0]) * helper->nnics;
+	char *ptr = (char *) helper->nics;
+	while (remain > 0) {
+		ssize_t size = write(pipefds_in[1], ptr, remain);
+		if (size == -1) {
+			err(1, "outer_helper: send context to parent");
+		}
+		ptr += remain;
+		remain -= size;
+	}
+
 	/* Notify sibling that we're done persisting their proc files
 	   and/or changing their [ug]id map */
 	int ok = 1;
@@ -286,10 +298,22 @@ void outer_helper_sendpid(const struct outer_helper *helper, pid_t pid)
 
 void outer_helper_sync(const struct outer_helper *helper)
 {
+	/* Copy back some context from the outer helper */
+	size_t remain = sizeof (helper->nics[0]) * helper->nnics;
+	char *ptr = (char *) helper->nics;
+	while (remain > 0) {
+		ssize_t size = read(helper->in, ptr, remain);
+		if (size == -1) {
+			err(1, "outer_helper_sync: read");
+		}
+		ptr += remain;
+		remain -= size;
+	}
+
 	int ok;
 	switch (read(helper->in, &ok, sizeof (ok))) {
 	case -1:
-		err(1, "outer_helper_wait: read");
+		err(1, "outer_helper_sync: read");
 	case 0:
 		/* Outer helper died before setting all of our attributes. */
 		exit(1);