netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Phil Sutter <phil@nwl.cc>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>, Eric Garver <e@erig.me>,
	netfilter-devel@vger.kernel.org
Subject: [iptables PATCH] nft: Fix for broken address mask match detection
Date: Mon, 28 Sep 2020 19:05:47 +0200	[thread overview]
Message-ID: <20200928170547.13857-1-phil@nwl.cc> (raw)

Trying to decide whether a bitwise expression is needed to match parts
of a source or destination address only, add_addr() checks if all bytes
in 'mask' are 0xff or not. The check is apparently broken though as each
byte in 'mask' is cast to a signed char before comparing against 0xff,
therefore the bitwise is always added:

| # ./bad/iptables-nft -A foo -s 10.0.0.1 -j ACCEPT
| # ./good/iptables-nft -A foo -s 10.0.0.2 -j ACCEPT
| # nft --debug=netlink list chain ip filter foo
| ip filter foo 5
|   [ payload load 4b @ network header + 12 => reg 1 ]
|   [ bitwise reg 1 = (reg=1 & 0xffffffff ) ^ 0x00000000 ]
|   [ cmp eq reg 1 0x0100000a ]
|   [ counter pkts 0 bytes 0 ]
|   [ immediate reg 0 accept ]
|
| ip filter foo 6 5
|   [ payload load 4b @ network header + 12 => reg 1 ]
|   [ cmp eq reg 1 0x0200000a ]
|   [ counter pkts 0 bytes 0 ]
|   [ immediate reg 0 accept ]
|
| table ip filter {
| 	chain foo {
| 		ip saddr 10.0.0.1 counter packets 0 bytes 0 accept
| 		ip saddr 10.0.0.2 counter packets 0 bytes 0 accept
| 	}
| }

Fix the cast, safe an extra op and gain 100% performance in ideal cases.

Fixes: 56859380eb328 ("xtables-compat: avoid unneeded bitwise ops")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-shared.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index c5a8f3fcc051d..7741d23befc5a 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -165,7 +165,7 @@ void add_outiface(struct nftnl_rule *r, char *iface, uint32_t op)
 void add_addr(struct nftnl_rule *r, int offset,
 	      void *data, void *mask, size_t len, uint32_t op)
 {
-	const char *m = mask;
+	const unsigned char *m = mask;
 	int i;
 
 	add_payload(r, offset, len, NFT_PAYLOAD_NETWORK_HEADER);
-- 
2.28.0


             reply	other threads:[~2020-09-28 16:50 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-28 17:05 Phil Sutter [this message]
2020-09-30  9:58 ` [iptables PATCH] nft: Fix for broken address mask match detection Pablo Neira Ayuso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200928170547.13857-1-phil@nwl.cc \
    --to=phil@nwl.cc \
    --cc=e@erig.me \
    --cc=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).