conf: Bind inbound ports with CAP_NET_BIND_SERVICE before isolate_user()

Even if CAP_NET_BIND_SERVICE is granted, we'll lose the capability in
the target user namespace as we isolate the process, which means
we're unable to bind to low ports at that point.

Bind inbound ports, and only those, before isolate_user(). Keep the
handling of outbound ports (for pasta mode only) after the setup of
the namespace, because that's where we'll bind them.

To this end, initialise the netlink socket for the init namespace
before isolate_user() as well, as we actually need to know the
addresses of the upstream interface before binding ports, in case
they're not explicitly passed by the user.

As we now call nl_sock_init() twice, checking its return code from
conf() twice looks a bit heavy: make it exit(), instead, as we
can't do much if we don't have netlink sockets.

While at it:

- move the v4_only && v6_only options check just after the first
  option processing loop, as this is more strictly related to
  option parsing proper

- update the man page, explaining that CAP_NET_BIND_SERVICE is
  *not* the preferred way to bind ports, because passt and pasta
  can be abused to allow other processes to make effective usage
  of it. Add a note about the recommended sysctl instead

- simplify nl_sock_init_do() now that it's called once for each
  case

Reported-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

1 files changed, 21 insertions, 19 deletions

diff --git a/netlink.c b/netlink.c
index 6e5a96b..0850cbe 100644
--- a/netlink.c
+++ b/netlink.c
@@ -19,6 +19,7 @@
 #include <sys/types.h>
 #include <limits.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <unistd.h>
 #include <arpa/inet.h>
@@ -39,57 +40,58 @@ static int nl_sock_ns	= -1;
 static int nl_seq;
 
 /**
- * nl_sock_init_do() - Set up netlink sockets in init and target namespace
- * @arg:	Execution context
+ * nl_sock_init_do() - Set up netlink sockets in init or target namespace
+ * @arg:	Execution context, if running from namespace, NULL otherwise
  *
  * Return: 0
  */
 static int nl_sock_init_do(void *arg)
 {
 	struct sockaddr_nl addr = { .nl_family = AF_NETLINK, };
-	int *s = &nl_sock;
+	int *s = arg ? &nl_sock_ns : &nl_sock;
 #ifdef NETLINK_GET_STRICT_CHK
 	int y = 1;
 #endif
 
-ns:
+	if (arg)
+		ns_enter((struct ctx *)arg);
+
 	if (((*s) = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE)) < 0 ||
-	    bind(*s, (struct sockaddr *)&addr, sizeof(addr)))
+	    bind(*s, (struct sockaddr *)&addr, sizeof(addr))) {
 		*s = -1;
-
-	if (*s == -1 || !arg || s == &nl_sock_ns)
 		return 0;
+	}
 
 #ifdef NETLINK_GET_STRICT_CHK
 	if (setsockopt(*s, SOL_NETLINK, NETLINK_GET_STRICT_CHK, &y, sizeof(y)))
 		debug("netlink: cannot set NETLINK_GET_STRICT_CHK on %i", *s);
 #endif
-
-	ns_enter((struct ctx *)arg);
-	s = &nl_sock_ns;
-	goto ns;
+	return 0;
 }
 
 /**
- * nl_sock_init() - Call nl_sock_init_do() and check for failures
+ * nl_sock_init() - Call nl_sock_init_do(), won't return on failure
  * @c:		Execution context
- *
- * Return: -EIO if sockets couldn't be set up, 0 otherwise
+ * @ns:		Get socket in namespace, not in init
  */
-int nl_sock_init(const struct ctx *c)
+void nl_sock_init(const struct ctx *c, bool ns)
 {
-	if (c->mode == MODE_PASTA) {
+	if (ns) {
 		NS_CALL(nl_sock_init_do, c);
 		if (nl_sock_ns == -1)
-			return -EIO;
+			goto fail;
 	} else {
 		nl_sock_init_do(NULL);
 	}
 
 	if (nl_sock == -1)
-		return -EIO;
+		goto fail;
 
-	return 0;
+	return;
+
+fail:
+	err("Failed to get netlink socket");
+	exit(EXIT_FAILURE);
 }
 
 /**