pfctl: fix recursive printing of NAT rules

pfctl_show_nat() is called recursively to print nat anchors. This passes the
anchor path, but this path was modified by pfctl_show_nat(), leading to issues
printing the anchors.

Make a copy of the path ('npath') before we modify it. Ensure we do this
correctly by sprinking in 'const', and add a test case to verify that we do now
print things correctly.

Reported by:	Thomas Pasqualini <thomas.pasqualini@orange.com>
MFC after:	2 weeks
Sponsored by:	Rubicon Communications, LLC ("Netgate")

(cherry picked from commit 58164dcb55d62ca73b5e550b8344bf61e2d8a47a)

sbin/pfctl/pfctl.c

@@ -93,12 +93,12 @@ int	 pfctl_load_hostid(struct pfctl *, u_int32_t);
 int	 pfctl_load_reassembly(struct pfctl *, u_int32_t);
 int	 pfctl_load_syncookies(struct pfctl *, u_int8_t);
 int	 pfctl_get_pool(int, struct pfctl_pool *, u_int32_t, u_int32_t, int,
-	    char *);
+	    const char *);
 void	 pfctl_print_eth_rule_counters(struct pfctl_eth_rule *, int);
 void	 pfctl_print_rule_counters(struct pfctl_rule *, int);
 int	 pfctl_show_eth_rules(int, char *, int, enum pfctl_show, char *, int, int);
 int	 pfctl_show_rules(int, char *, int, enum pfctl_show, char *, int, int);
-int	 pfctl_show_nat(int, char *, int, char *, int, int);
+int	 pfctl_show_nat(int, const char *, int, char *, int, int);
 int	 pfctl_show_src_nodes(int, int);
 int	 pfctl_show_states(int, const char *, int);
 int	 pfctl_show_status(int, int);
@@ -946,7 +946,7 @@ pfctl_id_kill_states(int dev, const char *iface, int opts)
 
 int
 pfctl_get_pool(int dev, struct pfctl_pool *pool, u_int32_t nr,
-    u_int32_t ticket, int r_action, char *anchorname)
+    u_int32_t ticket, int r_action, const char *anchorname)
 {
 	struct pfioc_pooladdr pp;
 	struct pf_pooladdr *pa;
@@ -1398,7 +1398,7 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format,
 }
 
 int
-pfctl_show_nat(int dev, char *path, int opts, char *anchorname, int depth,
+pfctl_show_nat(int dev, const char *path, int opts, char *anchorname, int depth,
     int wildcard)
 {
 	struct pfctl_rules_info ri;
@@ -1421,16 +1421,17 @@ pfctl_show_nat(int dev, char *path, int opts, char *anchorname, int depth,
 		p[0] = '\0';
 	}
 
+	if ((npath = calloc(1, MAXPATHLEN)) == NULL)
+		errx(1, "pfctl_rules: calloc");
+
 	if (anchorname[0] == '/') {
-		if ((npath = calloc(1, MAXPATHLEN)) == NULL)
-			errx(1, "pfctl_rules: calloc");
 		snprintf(npath, MAXPATHLEN, "%s", anchorname);
 	} else {
-		if (path[0])
-			snprintf(&path[len], MAXPATHLEN - len, "/%s", anchorname);
+		snprintf(npath, MAXPATHLEN, "%s", path);
+		if (npath[0])
+			snprintf(&npath[len], MAXPATHLEN - len, "/%s", anchorname);
 		else
-			snprintf(&path[len], MAXPATHLEN - len, "%s", anchorname);
-		npath = path;
+			snprintf(&npath[len], MAXPATHLEN - len, "%s", anchorname);
 	}
 
 	/*
@@ -1463,12 +1464,12 @@ pfctl_show_nat(int dev, char *path, int opts, char *anchorname, int depth,
 			INDENT(depth, !(opts & PF_OPT_VERBOSE));
 			printf("}\n");
 		}
-		path[len] = '\0';
+		npath[len] = '\0';
 		return (0);
 	}
 
 	for (i = 0; i < 3; i++) {
-		ret = pfctl_get_rules_info(dev, &ri, nattype[i], path);
+		ret = pfctl_get_rules_info(dev, &ri, nattype[i], npath);
 		if (ret != 0) {
 			warn("DIOCGETRULES");
 			return (-1);
@@ -1476,13 +1477,13 @@ pfctl_show_nat(int dev, char *path, int opts, char *anchorname, int depth,
 		for (nr = 0; nr < ri.nr; ++nr) {
 			INDENT(depth, !(opts & PF_OPT_VERBOSE));
 
-			if (pfctl_get_rule(dev, nr, ri.ticket, path,
+			if (pfctl_get_rule(dev, nr, ri.ticket, npath,
 			    nattype[i], &rule, anchor_call)) {
 				warn("DIOCGETRULE");
 				return (-1);
 			}
 			if (pfctl_get_pool(dev, &rule.rpool, nr,
-			    ri.ticket, nattype[i], path) != 0)
+			    ri.ticket, nattype[i], npath) != 0)
 				return (-1);
 
 			if (dotitle) {

tests/sys/netpfil/pf/anchor.sh

@@ -161,10 +161,56 @@ wildcard_cleanup()
 	pft_cleanup
 }
 
+atf_test_case "nat" "cleanup"
+nat_head()
+{
+	atf_set descr 'Test nested nat anchors'
+	atf_set require.user root
+}
+
+nat_body()
+{
+	pft_init
+
+	epair=$(vnet_mkepair)
+	vnet_mkjail alcatraz ${epair}a
+
+	ifconfig ${epair}b 192.0.2.2/24 up
+	jexec alcatraz ifconfig ${epair}a 192.0.2.1/24 up
+
+	# Sanity check
+	atf_check -s exit:0 -o ignore ping -c 1 192.0.2.1
+
+	jexec alcatraz pfctl -e
+	pft_set_rules alcatraz \
+	    "nat-anchor \"foo/*\"" \
+	    "pass"
+
+	echo "nat log on ${epair}a inet from 192.0.2.0/24 to any port = 53 -> 192.0.2.1" \
+	    | jexec alcatraz pfctl -a "foo/bar" -g -f -
+	echo "rdr on ${epair}a proto tcp to port echo -> 127.0.0.1 port echo" \
+	    | jexec alcatraz pfctl -a "foo/baz" -g -f -
+
+	jexec alcatraz pfctl -sn -a "*"
+	jexec alcatraz pfctl -sn -a "foo/bar"
+	jexec alcatraz pfctl -sn -a "foo/baz"
+
+	atf_check -s exit:0 -o match:"nat log on epair0a inet from 192.0.2.0/24 to any port = domain -> 192.0.2.1" \
+	    jexec alcatraz pfctl -sn -a "*"
+	atf_check -s exit:0 -o match:"rdr on epair0a inet proto tcp from any to any port = echo -> 127.0.0.1 port 7" \
+	    jexec alcatraz pfctl -sn -a "*"
+}
+
+nat_cleanup()
+{
+	pft_cleanup
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case "pr183198"
 	atf_add_test_case "pr279225"
 	atf_add_test_case "nested_anchor"
 	atf_add_test_case "wildcard"
+	atf_add_test_case "nat"
 }