conf: Don't be strict about exclusivity of forwarding mode

Currently as well as building the forwarding tables, conf() maintains a
"forwarding mode" value for each protocol and direction.  This prevents,
for example "-t all" and "-t 40000" being given on the same command line.

This restriction predates the forwarding table and is no longer really
necessary.  Remove the restriction, instead doing our best to apply all the
given options simultaneously.

 * Many combinations previously disallowed will still be disallowed because
   of conflicts between the specific generated rules, e.g.
        -t all -t 8888
   (because -t all already listens on port 8888)
 * Some new combinations are now allowed and will work, e.g.
        -t all -t 40000
   because 'all' excludes ephemeral ports (which includes 40000 on default
   Linux configurations).
 * We remove our mode variables, but keep boolean variables to track if
   any forwarding config option has been given.  This is needed in order to
   correctly default to -t auto -T auto -u auto -U auto for pasta.
 * -[tTuU] none after any other rules is still considered an error.
   However -t none *before* other rules is allowed.  This is potentially
   confusing, but is awkward to avoid for the time being.

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

1 files changed, 27 insertions, 70 deletions

diff --git a/conf.c b/conf.c
index 375b9a4..18d9223 100644
--- a/conf.c
+++ b/conf.c
@@ -233,22 +233,6 @@ fail:
 }
 
 /**
- * enum fwd_mode - Overall forwarding mode for a direction and protocol
- * @FWD_MODE_UNSET	Initial value, not parsed/configured yet
- * @FWD_MODE_SPEC	Forward specified ports
- * @FWD_MODE_NONE	No forwarded ports
- * @FWD_MODE_AUTO	Automatic detection and forwarding based on bound ports
- * @FWD_MODE_ALL	Bind all free ports
- */
-enum fwd_mode {
-	FWD_MODE_UNSET = 0,
-	FWD_MODE_SPEC,
-	FWD_MODE_NONE,
-	FWD_MODE_AUTO,
-	FWD_MODE_ALL,
-};
-
-/**
  * conf_ports_spec() - Parse port range(s) specifier
  * @c:		Execution context
  * @optname:	Short option name, t, T, u, or U
@@ -350,13 +334,12 @@ bad:
  * @optname:	Short option name, t, T, u, or U
  * @optarg:	Option argument (port specification)
  * @fwd:	Forwarding table to be updated
- * @mode:	Overall port forwarding mode (updated)
  */
 static void conf_ports(const struct ctx *c, char optname, const char *optarg,
-		       struct fwd_table *fwd, enum fwd_mode *mode)
+		       struct fwd_table *fwd)
 {
 	union inany_addr addr_buf = inany_any6, *addr = &addr_buf;
-	char buf[BUFSIZ], *spec, *ifname = NULL, *p;
+	char buf[BUFSIZ], *spec, *ifname = NULL;
 	uint8_t proto;
 
 	if (optname == 't' || optname == 'T')
@@ -367,10 +350,14 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		assert(0);
 
 	if (!strcmp(optarg, "none")) {
-		if (*mode)
-			goto mode_conflict;
+		unsigned i;
 
-		*mode = FWD_MODE_NONE;
+		for (i = 0; i < fwd->count; i++) {
+			if (fwd->rules[i].proto == proto) {
+				die("-%c none conflicts with previous options",
+					optname);
+			}
+		}
 		return;
 	}
 
@@ -380,14 +367,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		die("UDP port forwarding requested but UDP is disabled");
 
 	if (!strcmp(optarg, "auto")) {
-		if (*mode)
-			goto mode_conflict;
-
 		if (c->mode != MODE_PASTA)
 			die("'auto' port forwarding is only allowed for pasta");
 
-		*mode = FWD_MODE_AUTO;
-
 		conf_ports_range_except(c, optname, optarg, fwd,
 					proto, NULL, NULL,
 					1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
@@ -398,11 +380,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 	if (!strcmp(optarg, "all")) {
 		uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
 
-		if (*mode)
-			goto mode_conflict;
-
-		*mode = FWD_MODE_ALL;
-
 		/* Exclude ephemeral ports */
 		fwd_port_map_ephemeral(exclude);
 
@@ -413,11 +390,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		return;
 	}
 
-	if (*mode > FWD_MODE_SPEC)
-		die("Specific ports cannot be specified together with all/none/auto");
-
-	*mode = FWD_MODE_SPEC;
-
 	strncpy(buf, optarg, sizeof(buf) - 1);
 
 	if ((spec = strchr(buf, '/'))) {
@@ -445,7 +417,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		if (ifname == buf + 1) {	/* Interface without address */
 			addr = NULL;
 		} else {
-			p = buf;
+			char *p = buf;
 
 			/* Allow square brackets for IPv4 too for convenience */
 			if (*p == '[' && p[strlen(p) - 1] == ']') {
@@ -482,10 +454,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,
 		ifname = "lo";
 
 	conf_ports_spec(c, optname, optarg, fwd, proto, addr, ifname, spec);
-	return;
-
-mode_conflict:
-	die("Port forwarding mode '%s' conflicts with previous mode", optarg);
 }
 
 /**
@@ -1602,12 +1570,9 @@ void conf(struct ctx *c, int argc, char **argv)
 	};
 	const char *optstring = "+dqfel:hs:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:";
 	const char *logname = (c->mode == MODE_PASTA) ? "pasta" : "passt";
+	bool opt_t = false, opt_T = false, opt_u = false, opt_U = false;
 	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 };
 	bool copy_addrs_opt = false, copy_routes_opt = false;
-	enum fwd_mode tcp_out_mode = FWD_MODE_UNSET;
-	enum fwd_mode udp_out_mode = FWD_MODE_UNSET;
-	enum fwd_mode tcp_in_mode = FWD_MODE_UNSET;
-	enum fwd_mode udp_in_mode = FWD_MODE_UNSET;
 	bool v4_only = false, v6_only = false;
 	unsigned dns4_idx = 0, dns6_idx = 0;
 	unsigned long max_mtu = IP_MAX_MTU;
@@ -2212,17 +2177,17 @@ void conf(struct ctx *c, int argc, char **argv)
 		name = getopt_long(argc, argv, optstring, options, NULL);
 
 		if (name == 't') {
-			conf_ports(c, name, optarg, c->fwd[PIF_HOST],
-				   &tcp_in_mode);
+			opt_t = true;
+			conf_ports(c, name, optarg, c->fwd[PIF_HOST]);
 		} else if (name == 'u') {
-			conf_ports(c, name, optarg, c->fwd[PIF_HOST],
-				   &udp_in_mode);
+			opt_u = true;
+			conf_ports(c, name, optarg, c->fwd[PIF_HOST]);
 		} else if (name == 'T') {
-			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
-				   &tcp_out_mode);
+			opt_T = true;
+			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE]);
 		} else if (name == 'U') {
-			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
-				   &udp_out_mode);
+			opt_U = true;
+			conf_ports(c, name, optarg, c->fwd[PIF_SPLICE]);
 		}
 	} while (name != -1);
 
@@ -2273,22 +2238,14 @@ void conf(struct ctx *c, int argc, char **argv)
 	}
 
 	if (c->mode == MODE_PASTA) {
-		if (!tcp_in_mode) {
-			conf_ports(c, 't', "auto",
-				   c->fwd[PIF_HOST], &tcp_in_mode);
-		}
-		if (!tcp_out_mode) {
-			conf_ports(c, 'T', "auto",
-				   c->fwd[PIF_SPLICE], &tcp_out_mode);
-		}
-		if (!udp_in_mode) {
-			conf_ports(c, 'u', "auto",
-				   c->fwd[PIF_HOST], &udp_in_mode);
-		}
-		if (!udp_out_mode) {
-			conf_ports(c, 'U', "auto",
-				   c->fwd[PIF_SPLICE], &udp_out_mode);
-		}
+		if (!opt_t)
+			conf_ports(c, 't', "auto", c->fwd[PIF_HOST]);
+		if (!opt_T)
+			conf_ports(c, 'T', "auto", c->fwd[PIF_SPLICE]);
+		if (!opt_u)
+			conf_ports(c, 'u', "auto", c->fwd[PIF_HOST]);
+		if (!opt_U)
+			conf_ports(c, 'U', "auto", c->fwd[PIF_SPLICE]);
 	}
 
 	if (!c->quiet)