Skip to contentSkip to conversation

nft-bridge: fix inversion of matches

PATCH Applied Arturo Borrero Gonzalez 2 participants 4 messages Nov 8, 2014 – Nov 10, 2014 netfilter

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 nft-bridge: fix inversion of matches

The inversion in bridge specific matches is failing because before this patch NFT_CMP_EQ is used unconditionally.

No need to change the invesion in family-agnostic functions, given ebt inv flags are translated to ipt inv flags and inversion is properly calculated there.

---
NOTE: I think the previous patch to fix this isse was wrong in several aspects.
This is a new approach. Compile-tested only. Please comment.

 iptables/nft-bridge.c |   25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)
iptables/nft-bridge.c +22 -3
index 0e21b46..66bbefd 100644
@@ -165,6 +165,7 @@ static int nft_bridge_add(struct nft_rule *r, void *data)
 	struct ebtables_command_state *cs = data;
 	struct ebt_entry *fw = &cs->fw;
 	uint8_t flags = ebt_to_ipt_flags(fw->invflags);
+	uint32_t op = NFT_CMP_EQ;
 	char *addr;
 
 	if (fw->in[0] != '\0')
@@ -182,18 +183,36 @@ static int nft_bridge_add(struct nft_rule *r, void *data)
 	addr = ether_ntoa((struct ether_addr *) fw->sourcemac);
 	if (strcmp(addr, "0:0:0:0:0:0") != 0) {
 		add_payload(r, offsetof(struct ethhdr, h_source), 6);
-		add_cmp_ptr(r, NFT_CMP_EQ, fw->sourcemac, 6);
+
+		if (fw->invflags & EBT_ISOURCE)
+			op = NFT_CMP_NEQ;
+		else
+			op = NFT_CMP_EQ;
+
+		add_cmp_ptr(r, op, fw->sourcemac, 6);
 	}
 
 	addr = ether_ntoa((struct ether_addr *) fw->destmac);
 	if (strcmp(addr, "0:0:0:0:0:0") != 0) {
 		add_payload(r, offsetof(struct ethhdr, h_dest), 6);
-		add_cmp_ptr(r, NFT_CMP_EQ, fw->destmac, 6);
+
+		if (fw->invflags & EBT_IDEST)
+			op = NFT_CMP_NEQ;
+		else
+			op = NFT_CMP_EQ;
+
+		add_cmp_ptr(r, op, fw->destmac, 6);
 	}
 
 	if (fw->ethproto != 0) {
 		add_payload(r, offsetof(struct ethhdr, h_proto), 2);
-		add_cmp_u16(r, fw->ethproto, NFT_CMP_EQ);
+
+		if (fw->invflags & EBT_IPROTO)
+			op = NFT_CMP_NEQ;
+		else
+			op = NFT_CMP_EQ;
+
+		add_cmp_u16(r, fw->ethproto, op);
 	}
 
 	return _add_action(r, cs);
#1 lore raw

PATCH 2/2 nft-bridge: fix printing of inverted protocols, addresses

Previous to this patch, no '!' is printed in payload comparisions. This patch solves it, so we can print for example inverted protocols:

% ebtables-compat -L
[...]
-p ! 0x800 -j ACCEPT

---
NOTE: This patch was already sent. No changes to it, just resending.

 iptables/nft-bridge.c |    6 ++++++
 1 file changed, 6 insertions(+)
iptables/nft-bridge.c +6 -0
index 66bbefd..7f0c578 100644
@@ -287,15 +287,21 @@ static void nft_bridge_parse_payload(struct nft_xt_ctx *ctx,
 		get_cmp_data(e, addr, sizeof(addr), &inv);
 		for (i = 0; i < ETH_ALEN; i++)
 			fw->destmac[i] = addr[i];
+		if (inv)
+			fw->invflags |= EBT_IDEST;
 		break;
 	case offsetof(struct ethhdr, h_source):
 		get_cmp_data(e, addr, sizeof(addr), &inv);
 		for (i = 0; i < ETH_ALEN; i++)
 			fw->sourcemac[i] = addr[i];
+		if (inv)
+			fw->invflags |= EBT_ISOURCE;
 		break;
 	case offsetof(struct ethhdr, h_proto):
 		get_cmp_data(e, &ethproto, sizeof(ethproto), &inv);
 		fw->ethproto = ethproto;
+		if (inv)
+			fw->invflags |= EBT_IPROTO;
 		break;
 	}
 }
maintainer Applied
#2 lore raw

Re: [ebtables-compat-experimental3 PATCH 2/2] nft-bridge: fix printing of inverted protocols, addresses

Quoted 6 lines · Arturo Borrero Gonzalez

Previous to this patch, no '!' is printed in payload comparisions. This patch solves it, so we can print for example inverted protocols:

% ebtables-compat -L
[...]
-p ! 0x800 -j ACCEPT

Applied, thanks.

#3 lore raw
In reply to Arturo Borrero Gonzalez
Quoted 29 lines · Arturo Borrero Gonzalez

The inversion in bridge specific matches is failing because before this patch NFT_CMP_EQ is used unconditionally.

No need to change the invesion in family-agnostic functions, given ebt inv flags are translated to ipt inv flags and inversion is properly calculated there.

---
NOTE: I think the previous patch to fix this isse was wrong in several aspects.
This is a new approach. Compile-tested only. Please comment.

 iptables/nft-bridge.c |   25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)
iptables/nft-bridge.c +1 -0
index 0e21b46..66bbefd 100644
@@ -165,6 +165,7 @@ static int nft_bridge_add(struct nft_rule *r, void *data)
 	struct ebtables_command_state *cs = data;
 	struct ebt_entry *fw = &cs->fw;
 	uint8_t flags = ebt_to_ipt_flags(fw->invflags);
+	uint32_t op = NFT_CMP_EQ;
 	char *addr;
 
 	if (fw->in[0] != '\0')
@@ -182,18 +183,36 @@ static int nft_bridge_add(struct nft_rule *r, void *data)
 	addr = ether_ntoa((struct ether_addr *) fw->sourcemac);

iptables/nft-bridge.c static int nft_bridge_add(struct nft_rule *r, void *data) · line 192

Quoted 10 lines
iptables/nft-bridge.c +7 -1
 	if (strcmp(addr, "0:0:0:0:0:0") != 0) {
 		add_payload(r, offsetof(struct ethhdr, h_source), 6);
-		add_cmp_ptr(r, NFT_CMP_EQ, fw->sourcemac, 6);
+
+		if (fw->invflags & EBT_ISOURCE)
+			op = NFT_CMP_NEQ;
+		else
+			op = NFT_CMP_EQ;
+
+		add_cmp_ptr(r, op, fw->sourcemac, 6);

Please, use ETH_ALEN instead of hardcoded values. I know this code is full of this, but when it comes to source code readability, it's always better to use something meaningful.

Apart from that, looks good. Please test and resubmit. Thanks.