Skip to contentSkip to conversation

nft-arp: fix inversion of matches

PATCH In review Arturo Borrero Gonzalez 2 participants 2 messages Nov 8, 2014 – Nov 10, 2014 netfilter

lore
Share & tools
lore

Arturo Borrero Gonzalez

author
#0 lore raw

PATCH 1/1 nft-arp: fix inversion of matches

Inversion of matches is failing because NFT_CMP_EQ is used unconditionally.

The family agnostic functions don't need this fix, because arp inv flags are translated to ipt inv flags, and these flags are well handled there.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---
NOTES: This patch is for the master branch of iptables tree.
Compile-tested only. Please comment.

 iptables/nft-arp.c |   41 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)
iptables/nft-arp.c +36 -5
index f45ad0f..cb3623d 100644
@@ -164,6 +164,7 @@ static int nft_arp_add(struct nft_rule *r, void *data)
 	struct arptables_command_state *cs = data;
 	struct arpt_entry *fw = &cs->fw;
 	uint8_t flags = arpt_to_ipt_flags(fw->arp.invflags);
+	uint32_t op = NFT_CMP_EQ;
 	int ret = 0;
 
 	if (fw->arp.iniface[0] != '\0')

@@ -174,12 +175,24 @@ static int nft_arp_add(struct nft_rule *r, void *data)
 
 	if (fw->arp.arhrd != 0) {
 		add_payload(r, offsetof(struct arphdr, ar_hrd), 2);
-		add_cmp_u16(r, fw->arp.arhrd, NFT_CMP_EQ);
+
+		if (fw->arp.invflags & ARPT_INV_ARPHRD)
+			op = NFT_CMP_NEQ;
+		else
+			op = NFT_CMP_EQ;
+
+		add_cmp_u16(r, fw->arp.arhrd, op);
 	}
 
 	if (fw->arp.arpro != 0) {
 	        add_payload(r, offsetof(struct arphdr, ar_pro), 2);
-		add_cmp_u16(r, fw->arp.arpro, NFT_CMP_EQ);
+
+		if (fw->arp.invflags & ARPT_INV_ARPPRO)
+			op = NFT_CMP_NEQ;
+		else
+			op = NFT_CMP_EQ;
+
+		add_cmp_u16(r, fw->arp.arpro, op);
 	}
 
 	if (fw->arp.arhln != 0)

@@ -190,12 +203,24 @@ static int nft_arp_add(struct nft_rule *r, void *data)
 
 	if (fw->arp.arpop != 0) {
 		add_payload(r, offsetof(struct arphdr, ar_op), 2);
-		add_cmp_u16(r, fw->arp.arpop, NFT_CMP_EQ);
+
+		if (fw->arp.invflags & ARPT_INV_ARPOP)
+			op = NFT_CMP_NEQ;
+		else
+			op = NFT_CMP_EQ;
+
+		add_cmp_u16(r, fw->arp.arpop, op);
 	}
 
 	if (fw->arp.src_devaddr.addr[0] != '\0') {
 		add_payload(r, sizeof(struct arphdr), fw->arp.arhln);
-		add_cmp_ptr(r, NFT_CMP_EQ, fw->arp.src_devaddr.addr, fw->arp.arhln);
+
+		if (fw->arp.invflags & ARPT_INV_SRCDEVADDR)
+			op = NFT_CMP_NEQ;
+		else
+			op = NFT_CMP_EQ;
+
+		add_cmp_ptr(r, op, fw->arp.src_devaddr.addr, fw->arp.arhln);
 	}
 
 	if (fw->arp.src.s_addr != 0) {

@@ -206,7 +231,13 @@ static int nft_arp_add(struct nft_rule *r, void *data)
 
 	if (fw->arp.tgt_devaddr.addr[0] != '\0') {
 		add_payload(r, sizeof(struct arphdr) + fw->arp.arhln + 4, fw->arp.arhln);
-		add_cmp_ptr(r, NFT_CMP_EQ, fw->arp.tgt_devaddr.addr, fw->arp.arhln);
+
+		if (fw->arp.invflags & ARPT_INV_TGTDEVADDR)
+			op = NFT_CMP_NEQ;
+		else
+			op = NFT_CMP_EQ;
+
+		add_cmp_ptr(r, op, fw->arp.tgt_devaddr.addr, fw->arp.arhln);
 	}
 
 	if (fw->arp.tgt.s_addr != 0) {
maintainer
#1 lore raw
Quoted 9 lines · Arturo Borrero Gonzalez

Inversion of matches is failing because NFT_CMP_EQ is used unconditionally.

The family agnostic functions don't need this fix, because arp inv flags are translated to ipt inv flags, and these flags are well handled there.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---
NOTES: This patch is for the master branch of iptables tree.
Compile-tested only. Please comment.

Please, no compile-tested patches, submit RFC or tested stuff.

iptables/nft-arp.c static int nft_arp_add(struct nft_rule *r, void *data) · line 166

Quoted 11 lines
 iptables/nft-arp.c |   41 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)
iptables/nft-arp.c +0 -0
index f45ad0f..cb3623d 100644
@@ -164,6 +164,7 @@ static int nft_arp_add(struct nft_rule *r, void *data)
 	struct arptables_command_state *cs = data;
 	struct arpt_entry *fw = &cs->fw;
 	uint8_t flags = arpt_to_ipt_flags(fw->arp.invflags);

I prefer if you kill arpt_to_ipt_flags(), so add_iniface() uses native NFT_CMP_{EQ,NEQ} as third parameter. Thus, add_iniface() will be consistent with other existing add_cmp_*() functions.

This will require some extra code in nft-ipv4 and nft-ipv6 though, but I'd like to avoid this arpt to ipt flags conversion.

Quoted 6 lines
iptables/nft-arp.c +1 -0
+	uint32_t op = NFT_CMP_EQ;
 	int ret = 0;
 
 	if (fw->arp.iniface[0] != '\0')

@@ -174,12 +175,24 @@ static int nft_arp_add(struct nft_rule *r, void *data)

iptables/nft-arp.c static int nft_arp_add(struct nft_rule *r, void *data) · line 184

Quoted 10 lines
iptables/nft-arp.c +7 -1
 	if (fw->arp.arhrd != 0) {
 		add_payload(r, offsetof(struct arphdr, ar_hrd), 2);
-		add_cmp_u16(r, fw->arp.arhrd, NFT_CMP_EQ);
+
+		if (fw->arp.invflags & ARPT_INV_ARPHRD)
+			op = NFT_CMP_NEQ;
+		else
+			op = NFT_CMP_EQ;
+
+		add_cmp_u16(r, fw->arp.arhrd, op);

This is a good example of what I would like to see in the family specific nft code, use this pattern consistently in this code. Thanks.