* [iptables PATCH] nft-arp: fix inversion of matches
@ 2014-11-08 21:35 Arturo Borrero Gonzalez
2014-11-10 17:30 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Arturo Borrero Gonzalez @ 2014-11-08 21:35 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
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 <arturo.borrero.glez@gmail.com>
---
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(-)
diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
index f45ad0f..cb3623d 100644
--- a/iptables/nft-arp.c
+++ b/iptables/nft-arp.c
@@ -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) {
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [iptables PATCH] nft-arp: fix inversion of matches
2014-11-08 21:35 [iptables PATCH] nft-arp: fix inversion of matches Arturo Borrero Gonzalez
@ 2014-11-10 17:30 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2014-11-10 17:30 UTC (permalink / raw)
To: Arturo Borrero Gonzalez; +Cc: netfilter-devel
On Sat, Nov 08, 2014 at 10:35:49PM +0100, Arturo Borrero Gonzalez wrote:
> 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 <arturo.borrero.glez@gmail.com>
> ---
> 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 | 41 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 36 insertions(+), 5 deletions(-)
>
> diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
> index f45ad0f..cb3623d 100644
> --- a/iptables/nft-arp.c
> +++ b/iptables/nft-arp.c
> @@ -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.
> + 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);
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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-11-10 17:28 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-08 21:35 [iptables PATCH] nft-arp: fix inversion of matches Arturo Borrero Gonzalez
2014-11-10 17:30 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).