From 59e25afcdc0a415c8b1bb6fff5fc14985d79e06b Mon Sep 17 00:00:00 2001

From: Phil Sutter <phil@nwl.cc>

Date: Thu, 31 Jan 2019 16:12:54 +0100

Subject: [PATCH] arptables-nft-save: Fix position of -j option

Legacy arptables-save (just like arptables itself) prints verdict as

first option, then matches and finally any target options.

To achieve this without introducing double/trailing spaces everywhere,

integrate target ('-j') option printing into

nft_arp_print_rule_details() and make it print separating whitespace

before each option.

In nft_arp_save_rule(), replace the call to save_matches_and_target() by

by a direct call to cs->target->save() since the former prints '-j'

option itself. Since there are no match extensions in arptables, any

other code from that function is not needed.

Signed-off-by: Phil Sutter <phil@nwl.cc>

Signed-off-by: Florian Westphal <fw@strlen.de>

(cherry picked from commit 2c3f7a2cd6fd8325b3a84e280cce945c6c20b87f)

Signed-off-by: Phil Sutter <psutter@redhat.com>

---

 iptables/nft-arp.c                            | 65 +++++++++++--------

 .../arptables/0001-arptables-save-restore_0   | 32 ++++-----

 .../0002-arptables-restore-defaults_0         |  6 +-

 3 files changed, 58 insertions(+), 45 deletions(-)

diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c

index f357fc4a43c4c..2cbdf23214049 100644

--- a/iptables/nft-arp.c

+++ b/iptables/nft-arp.c

@@ -434,14 +434,21 @@ static void nft_arp_print_header(unsigned int format, const char *chain,

 	}

 }

-static void nft_arp_print_rule_details(const struct arpt_entry *fw,

+static void nft_arp_print_rule_details(const struct iptables_command_state *cs,

 				       unsigned int format)

 {

+	const struct arpt_entry *fw = &cs->arp;

 	char buf[BUFSIZ];

 	char iface[IFNAMSIZ+2];

+	const char *sep = "";

 	int print_iface = 0;

 	int i;

+	if (strlen(cs->jumpto)) {

+		printf("%s-j %s", sep, cs->jumpto);

+		sep = " ";

+	}

+

 	iface[0] = '\0';

 	if (fw->arp.iniface[0] != '\0') {

@@ -453,9 +460,11 @@ static void nft_arp_print_rule_details(const struct arpt_entry *fw,

 		if (format & FMT_NUMERIC) strcat(iface, "*");

 		else strcat(iface, "any");

 	}

-	if (print_iface)

-		printf("%s-i %s ", fw->arp.invflags & ARPT_INV_VIA_IN ?

+	if (print_iface) {

+		printf("%s%s-i %s", sep, fw->arp.invflags & ARPT_INV_VIA_IN ?

 				   "! " : "", iface);

+		sep = " ";

+	}

 	print_iface = 0;

 	iface[0] = '\0';

@@ -469,12 +478,14 @@ static void nft_arp_print_rule_details(const struct arpt_entry *fw,

 		if (format & FMT_NUMERIC) strcat(iface, "*");

 		else strcat(iface, "any");

 	}

-	if (print_iface)

-		printf("%s-o %s ", fw->arp.invflags & ARPT_INV_VIA_OUT ?

+	if (print_iface) {

+		printf("%s%s-o %s", sep, fw->arp.invflags & ARPT_INV_VIA_OUT ?

 				   "! " : "", iface);

+		sep = " ";

+	}

 	if (fw->arp.smsk.s_addr != 0L) {

-		printf("%s", fw->arp.invflags & ARPT_INV_SRCIP

+		printf("%s%s", sep, fw->arp.invflags & ARPT_INV_SRCIP

 			? "! " : "");

 		if (format & FMT_NUMERIC)

 			sprintf(buf, "%s", addr_to_dotted(&(fw->arp.src)));

@@ -482,7 +493,8 @@ static void nft_arp_print_rule_details(const struct arpt_entry *fw,

 			sprintf(buf, "%s", addr_to_anyname(&(fw->arp.src)));

 		strncat(buf, mask_to_dotted(&(fw->arp.smsk)),

 			sizeof(buf) - strlen(buf) - 1);

-		printf("-s %s ", buf);

+		printf("-s %s", buf);

+		sep = " ";

 	}

 	for (i = 0; i < ARPT_DEV_ADDR_LEN_MAX; i++)

@@ -490,16 +502,16 @@ static void nft_arp_print_rule_details(const struct arpt_entry *fw,

 			break;

 	if (i == ARPT_DEV_ADDR_LEN_MAX)

 		goto after_devsrc;

-	printf("%s", fw->arp.invflags & ARPT_INV_SRCDEVADDR

+	printf("%s%s", sep, fw->arp.invflags & ARPT_INV_SRCDEVADDR

 		? "! " : "");

 	printf("--src-mac ");

 	print_mac_and_mask((unsigned char *)fw->arp.src_devaddr.addr,

 		(unsigned char *)fw->arp.src_devaddr.mask, ETH_ALEN);

-	printf(" ");

+	sep = " ";

 after_devsrc:

 	if (fw->arp.tmsk.s_addr != 0L) {

-		printf("%s", fw->arp.invflags & ARPT_INV_TGTIP

+		printf("%s%s", sep, fw->arp.invflags & ARPT_INV_TGTIP

 			? "! " : "");

 		if (format & FMT_NUMERIC)

 			sprintf(buf, "%s", addr_to_dotted(&(fw->arp.tgt)));

@@ -507,7 +519,8 @@ after_devsrc:

 			sprintf(buf, "%s", addr_to_anyname(&(fw->arp.tgt)));

 		strncat(buf, mask_to_dotted(&(fw->arp.tmsk)),

 			sizeof(buf) - strlen(buf) - 1);

-		printf("-d %s ", buf);

+		printf("-d %s", buf);

+		sep = " ";

 	}

 	for (i = 0; i 

@@ -515,28 +528,28 @@ after_devsrc:

 			break;

 	if (i == ARPT_DEV_ADDR_LEN_MAX)

 		goto after_devdst;

-	printf("%s", fw->arp.invflags & ARPT_INV_TGTDEVADDR

+	printf("%s%s", sep, fw->arp.invflags & ARPT_INV_TGTDEVADDR

 		? "! " : "");

 	printf("--dst-mac ");

 	print_mac_and_mask((unsigned char *)fw->arp.tgt_devaddr.addr,

 		(unsigned char *)fw->arp.tgt_devaddr.mask, ETH_ALEN);

-	printf(" ");

+	sep = " ";

 after_devdst:

 	if (fw->arp.arhln_mask != 0) {

-		printf("%s", fw->arp.invflags & ARPT_INV_ARPHLN

+		printf("%s%s", sep, fw->arp.invflags & ARPT_INV_ARPHLN

 			? "! " : "");

 		printf("--h-length %d", fw->arp.arhln);

 		if (fw->arp.arhln_mask != 255)

 			printf("/%d", fw->arp.arhln_mask);

-		printf(" ");

+		sep = " ";

 	}

 	if (fw->arp.arpop_mask != 0) {

 		int tmp = ntohs(fw->arp.arpop);

-		printf("%s", fw->arp.invflags & ARPT_INV_ARPOP

+		printf("%s%s", sep, fw->arp.invflags & ARPT_INV_ARPOP

 			? "! " : "");

 		if (tmp <= NUMOPCODES && !(format & FMT_NUMERIC))

 			printf("--opcode %s", opcodes[tmp-1]);

@@ -545,13 +558,13 @@ after_devdst:

 		if (fw->arp.arpop_mask != 65535)

 			printf("/%d", ntohs(fw->arp.arpop_mask));

-		printf(" ");

+		sep = " ";

 	}

 	if (fw->arp.arhrd_mask != 0) {

 		uint16_t tmp = ntohs(fw->arp.arhrd);

-		printf("%s", fw->arp.invflags & ARPT_INV_ARPHRD

+		printf("%s%s", sep, fw->arp.invflags & ARPT_INV_ARPHRD

 			? "! " : "");

 		if (tmp == 1 && !(format & FMT_NUMERIC))

 			printf("--h-type %s", "Ethernet");

@@ -559,13 +572,13 @@ after_devdst:

 			printf("--h-type %u", tmp);

 		if (fw->arp.arhrd_mask != 65535)

 			printf("/%d", ntohs(fw->arp.arhrd_mask));

-		printf(" ");

+		sep = " ";

 	}

 	if (fw->arp.arpro_mask != 0) {

 		int tmp = ntohs(fw->arp.arpro);

-		printf("%s", fw->arp.invflags & ARPT_INV_ARPPRO

+		printf("%s%s", sep, fw->arp.invflags & ARPT_INV_ARPPRO

 			? "! " : "");

 		if (tmp == 0x0800 && !(format & FMT_NUMERIC))

 			printf("--proto-type %s", "IPv4");

@@ -573,7 +586,7 @@ after_devdst:

 			printf("--proto-type 0x%x", tmp);

 		if (fw->arp.arpro_mask != 65535)

 			printf("/%x", ntohs(fw->arp.arpro_mask));

-		printf(" ");

+		sep = " ";

 	}

 }

@@ -584,8 +597,10 @@ nft_arp_save_rule(const void *data, unsigned int format)

 	format |= FMT_NUMERIC;

-	nft_arp_print_rule_details(&cs->arp, format);

-	save_matches_and_target(cs, false, &cs->arp, format);

+	nft_arp_print_rule_details(cs, format);

+	if (cs->target && cs->target->save)

+		cs->target->save(&cs->fw, cs->target->t);

+	printf("\n");

 }

 static void

@@ -598,9 +613,7 @@ nft_arp_print_rule(struct nftnl_rule *r, unsigned int num, unsigned int format)

 	nft_rule_to_iptables_command_state(r, &cs);

-	if (strlen(cs.jumpto))

-		printf("-j %s ", cs.jumpto);

-	nft_arp_print_rule_details(&cs.arp, format);

+	nft_arp_print_rule_details(&cs, format);

 	print_matches_and_target(&cs, format);

 	if (!(format & FMT_NOCOUNTS)) {

diff --git a/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0 b/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0

index f8629551b0ba9..0664e3b38d5e8 100755

--- a/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0

+++ b/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0

@@ -35,22 +35,22 @@ DUMP='*filter

 :INPUT ACCEPT

 :OUTPUT DROP

 :foo -

--A INPUT -s 10.0.0.0/8 --h-length 6 --h-type 1 -j ACCEPT

--A INPUT -d 192.168.123.1 --h-length 6 --h-type 1 -j ACCEPT

--A INPUT --src-mac fe:ed:ba:be:00:01 --h-length 6 --h-type 1 -j ACCEPT

--A INPUT --dst-mac fe:ed:ba:be:00:01 --h-length 6 --h-type 1 -j ACCEPT

--A INPUT --h-length 6 --h-type 1 -j foo

--A INPUT --h-length 6 --h-type 1 

--A OUTPUT -o lo --h-length 6 --h-type 1 -j ACCEPT

--A OUTPUT -o eth134 --h-length 6 --h-type 1 -j mangle --mangle-ip-s 10.0.0.1

--A OUTPUT -o eth432 --h-length 6 --h-type 1 -j CLASSIFY --set-class feed:babe

--A OUTPUT -o eth432 --h-length 6 --opcode 1 --h-type 1 -j CLASSIFY --set-class feed:babe

--A foo -i lo --h-length 6 --h-type 1 -j ACCEPT

--A foo --h-length 6 --h-type 1 -j ACCEPT

--A foo --h-length 6 --h-type 1 -j MARK --set-mark 12345

--A foo --h-length 6 --opcode 1 --h-type 1 -j ACCEPT

--A foo --h-length 6 --h-type 1 --proto-type 0x800 -j ACCEPT

--A foo -i lo --h-length 6 --opcode 1 --h-type 1 --proto-type 0x800 -j ACCEPT

+-A INPUT -j ACCEPT -s 10.0.0.0/8 --h-length 6 --h-type 1

+-A INPUT -j ACCEPT -d 192.168.123.1 --h-length 6 --h-type 1

+-A INPUT -j ACCEPT --src-mac fe:ed:ba:be:00:01 --h-length 6 --h-type 1

+-A INPUT -j ACCEPT --dst-mac fe:ed:ba:be:00:01 --h-length 6 --h-type 1

+-A INPUT -j foo --h-length 6 --h-type 1

+-A INPUT  --h-length 6 --h-type 1

+-A OUTPUT -j ACCEPT -o lo --h-length 6 --h-type 1

+-A OUTPUT -j mangle -o eth134 --h-length 6 --h-type 1 --mangle-ip-s 10.0.0.1

+-A OUTPUT -j CLASSIFY -o eth432 --h-length 6 --h-type 1 --set-class feed:babe

+-A OUTPUT -j CLASSIFY -o eth432 --h-length 6 --opcode 1 --h-type 1 --set-class feed:babe

+-A foo -j ACCEPT -i lo --h-length 6 --h-type 1

+-A foo -j ACCEPT --h-length 6 --h-type 1

+-A foo -j MARK --h-length 6 --h-type 1 --set-mark 12345

+-A foo -j ACCEPT --h-length 6 --opcode 1 --h-type 1

+-A foo -j ACCEPT --h-length 6 --h-type 1 --proto-type 0x800

+-A foo -j ACCEPT -i lo --h-length 6 --opcode 1 --h-type 1 --proto-type 0x800

 '

 diff -u <(echo -e "$DUMP") <($XT_MULTI arptables-save)

diff --git a/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0 b/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0

index ee17da0023b82..d742c3d506305 100755

--- a/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0

+++ b/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0

@@ -11,7 +11,7 @@ set -e

 DUMP='*filter

 :OUTPUT ACCEPT

 -A OUTPUT -j mangle --mangle-ip-s 10.0.0.1

--A OUTPUT --h-length 6 --h-type 1 -j mangle --mangle-ip-d 10.0.0.2

+-A OUTPUT -j mangle --h-length 6 --h-type 1 --mangle-ip-d 10.0.0.2

 '

 # note how mangle-ip-s is unset in second rule

@@ -19,8 +19,8 @@ DUMP='*filter

 EXPECT='*filter

 :INPUT ACCEPT

 :OUTPUT ACCEPT

--A OUTPUT --h-length 6 --h-type 1 -j mangle --mangle-ip-s 10.0.0.1

--A OUTPUT --h-length 6 --h-type 1 -j mangle --mangle-ip-d 10.0.0.2

+-A OUTPUT -j mangle --h-length 6 --h-type 1 --mangle-ip-s 10.0.0.1

+-A OUTPUT -j mangle --h-length 6 --h-type 1 --mangle-ip-d 10.0.0.2

 '

 $XT_MULTI arptables -F

-- 

2.20.1