netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iptables-nftables] nft: fix interface wildcard matching
@ 2013-10-16 14:07 Pablo Neira Ayuso
  2013-10-17  8:39 ` Anand Raj Manickam
  0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-10-16 14:07 UTC (permalink / raw)
  To: netfilter-devel

In (73ea1cc nft: convert rule into a command state structure), the
interface wildcard matching got broken. The previous handling was
flawed by the use of ifnametoindex in scenario where the interface
may vanished after a rule was added.

This approach relies on the trailing '\0' to identify if this is
an exact or wildcard matching, based on discussion with Florian.

Based on initial patch from Anand Raj Manickam.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 iptables/nft-shared.c |   38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 3987f74..e0eaa17 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -124,13 +124,11 @@ void add_iniface(struct nft_rule *r, char *iface, int invflags)
 	else
 		op = NFT_CMP_EQ;
 
-	if (iface[iface_len - 1] == '+') {
-		add_meta(r, NFT_META_IIFNAME);
+	add_meta(r, NFT_META_IIFNAME);
+	if (iface[iface_len - 1] == '+')
 		add_cmp_ptr(r, op, iface, iface_len - 1);
-	} else {
-		add_meta(r, NFT_META_IIF);
-		add_cmp_u32(r, if_nametoindex(iface), op);
-	}
+	else
+		add_cmp_ptr(r, op, iface, iface_len + 1);
 }
 
 void add_outiface(struct nft_rule *r, char *iface, int invflags)
@@ -145,13 +143,11 @@ void add_outiface(struct nft_rule *r, char *iface, int invflags)
 	else
 		op = NFT_CMP_EQ;
 
-	if (iface[iface_len - 1] == '+') {
-		add_meta(r, NFT_META_OIFNAME);
+	add_meta(r, NFT_META_OIFNAME);
+	if (iface[iface_len - 1] == '+')
 		add_cmp_ptr(r, op, iface, iface_len - 1);
-	} else {
-		add_meta(r, NFT_META_OIF);
-		add_cmp_u32(r, if_nametoindex(iface), op);
-	}
+	else
+		add_cmp_ptr(r, op, iface, iface_len + 1);
 }
 
 void add_addr(struct nft_rule *r, int offset,
@@ -251,15 +247,14 @@ void parse_meta(struct nft_rule_expr *e, uint8_t key, char *iniface,
 			*invflags |= IPT_INV_VIA_IN;
 
 		memcpy(iniface, ifname, len);
-		iniface[len] = '\0';
 
-		/* If zero, then this is an interface mask */
-		if (if_nametoindex(iniface) == 0) {
+		if (iniface[len] == '\0')
+			memset(iniface_mask, 0xff, len);
+		else {
 			iniface[len] = '+';
 			iniface[len+1] = '\0';
+			memset(iniface_mask, 0xff, len + 1);
 		}
-
-		memset(iniface_mask, 0xff, len);
 		break;
 	case NFT_META_OIFNAME:
 		ifname = nft_rule_expr_get(e, NFT_EXPR_CMP_DATA, &len);
@@ -267,15 +262,14 @@ void parse_meta(struct nft_rule_expr *e, uint8_t key, char *iniface,
 			*invflags |= IPT_INV_VIA_OUT;
 
 		memcpy(outiface, ifname, len);
-		outiface[len] = '\0';
 
-		/* If zero, then this is an interface mask */
-		if (if_nametoindex(outiface) == 0) {
+		if (outiface[len] == '\0')
+			memset(outiface_mask, 0xff, len);
+		else {
 			outiface[len] = '+';
 			outiface[len+1] = '\0';
+			memset(outiface_mask, 0xff, len + 1);
 		}
-
-		memset(outiface_mask, 0xff, len);
 		break;
 	default:
 		DEBUGP("unknown meta key %d\n", key);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH iptables-nftables] nft: fix interface wildcard matching
  2013-10-16 14:07 [PATCH iptables-nftables] nft: fix interface wildcard matching Pablo Neira Ayuso
@ 2013-10-17  8:39 ` Anand Raj Manickam
  2013-10-17  8:43   ` Pablo Neira Ayuso
  0 siblings, 1 reply; 5+ messages in thread
From: Anand Raj Manickam @ 2013-10-17  8:39 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On Wed, Oct 16, 2013 at 7:37 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> In (73ea1cc nft: convert rule into a command state structure), the
> interface wildcard matching got broken. The previous handling was
> flawed by the use of ifnametoindex in scenario where the interface
> may vanished after a rule was added.
>
> This approach relies on the trailing '\0' to identify if this is
> an exact or wildcard matching, based on discussion with Florian.
>
> Based on initial patch from Anand Raj Manickam.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  iptables/nft-shared.c |   38 ++++++++++++++++----------------------
>  1 file changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
> index 3987f74..e0eaa17 100644
> --- a/iptables/nft-shared.c
> +++ b/iptables/nft-shared.c
> @@ -124,13 +124,11 @@ void add_iniface(struct nft_rule *r, char *iface, int invflags)
>         else
>                 op = NFT_CMP_EQ;
>
> -       if (iface[iface_len - 1] == '+') {
> -               add_meta(r, NFT_META_IIFNAME);
> +       add_meta(r, NFT_META_IIFNAME);
> +       if (iface[iface_len - 1] == '+')
>                 add_cmp_ptr(r, op, iface, iface_len - 1);
> -       } else {
> -               add_meta(r, NFT_META_IIF);
> -               add_cmp_u32(r, if_nametoindex(iface), op);
> -       }
> +       else
> +               add_cmp_ptr(r, op, iface, iface_len + 1);
>  }
>
>  void add_outiface(struct nft_rule *r, char *iface, int invflags)
> @@ -145,13 +143,11 @@ void add_outiface(struct nft_rule *r, char *iface, int invflags)
>         else
>                 op = NFT_CMP_EQ;
>
> -       if (iface[iface_len - 1] == '+') {
> -               add_meta(r, NFT_META_OIFNAME);
> +       add_meta(r, NFT_META_OIFNAME);
> +       if (iface[iface_len - 1] == '+')
>                 add_cmp_ptr(r, op, iface, iface_len - 1);
> -       } else {
> -               add_meta(r, NFT_META_OIF);
> -               add_cmp_u32(r, if_nametoindex(iface), op);
> -       }
> +       else
> +               add_cmp_ptr(r, op, iface, iface_len + 1);
>  }
>
>  void add_addr(struct nft_rule *r, int offset,
> @@ -251,15 +247,14 @@ void parse_meta(struct nft_rule_expr *e, uint8_t key, char *iniface,
>                         *invflags |= IPT_INV_VIA_IN;
>
>                 memcpy(iniface, ifname, len);
> -               iniface[len] = '\0';
>
> -               /* If zero, then this is an interface mask */
> -               if (if_nametoindex(iniface) == 0) {
> +               if (iniface[len] == '\0')
> +                       memset(iniface_mask, 0xff, len);
> +               else {
>                         iniface[len] = '+';
>                         iniface[len+1] = '\0';
> +                       memset(iniface_mask, 0xff, len + 1);
>                 }
> -
> -               memset(iniface_mask, 0xff, len);
>                 break;
>         case NFT_META_OIFNAME:
>                 ifname = nft_rule_expr_get(e, NFT_EXPR_CMP_DATA, &len);
> @@ -267,15 +262,14 @@ void parse_meta(struct nft_rule_expr *e, uint8_t key, char *iniface,
>                         *invflags |= IPT_INV_VIA_OUT;
>
>                 memcpy(outiface, ifname, len);
> -               outiface[len] = '\0';
>
> -               /* If zero, then this is an interface mask */
> -               if (if_nametoindex(outiface) == 0) {
> +               if (outiface[len] == '\0')
> +                       memset(outiface_mask, 0xff, len);
> +               else {
>                         outiface[len] = '+';
>                         outiface[len+1] = '\0';
> +                       memset(outiface_mask, 0xff, len + 1);
>                 }
> -
> -               memset(outiface_mask, 0xff, len);
>                 break;
>         default:
>                 DEBUGP("unknown meta key %d\n", key);

Pablo,
This again breaks the delete functionality .



> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH iptables-nftables] nft: fix interface wildcard matching
  2013-10-17  8:39 ` Anand Raj Manickam
@ 2013-10-17  8:43   ` Pablo Neira Ayuso
  2013-10-17 12:47     ` Anand Raj Manickam
  0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-10-17  8:43 UTC (permalink / raw)
  To: Anand Raj Manickam; +Cc: netfilter-devel

On Thu, Oct 17, 2013 at 02:09:05PM +0530, Anand Raj Manickam wrote:
> On Wed, Oct 16, 2013 at 7:37 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > In (73ea1cc nft: convert rule into a command state structure), the
> > interface wildcard matching got broken. The previous handling was
> > flawed by the use of ifnametoindex in scenario where the interface
> > may vanished after a rule was added.
> >
> > This approach relies on the trailing '\0' to identify if this is
> > an exact or wildcard matching, based on discussion with Florian.
> >
> > Based on initial patch from Anand Raj Manickam.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> >  iptables/nft-shared.c |   38 ++++++++++++++++----------------------
> >  1 file changed, 16 insertions(+), 22 deletions(-)
> >
> > diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
> > index 3987f74..e0eaa17 100644
> > --- a/iptables/nft-shared.c
> > +++ b/iptables/nft-shared.c
> > @@ -124,13 +124,11 @@ void add_iniface(struct nft_rule *r, char *iface, int invflags)
> >         else
> >                 op = NFT_CMP_EQ;
> >
> > -       if (iface[iface_len - 1] == '+') {
> > -               add_meta(r, NFT_META_IIFNAME);
> > +       add_meta(r, NFT_META_IIFNAME);
> > +       if (iface[iface_len - 1] == '+')
> >                 add_cmp_ptr(r, op, iface, iface_len - 1);
> > -       } else {
> > -               add_meta(r, NFT_META_IIF);
> > -               add_cmp_u32(r, if_nametoindex(iface), op);
> > -       }
> > +       else
> > +               add_cmp_ptr(r, op, iface, iface_len + 1);
> >  }
> >
> >  void add_outiface(struct nft_rule *r, char *iface, int invflags)
> > @@ -145,13 +143,11 @@ void add_outiface(struct nft_rule *r, char *iface, int invflags)
> >         else
> >                 op = NFT_CMP_EQ;
> >
> > -       if (iface[iface_len - 1] == '+') {
> > -               add_meta(r, NFT_META_OIFNAME);
> > +       add_meta(r, NFT_META_OIFNAME);
> > +       if (iface[iface_len - 1] == '+')
> >                 add_cmp_ptr(r, op, iface, iface_len - 1);
> > -       } else {
> > -               add_meta(r, NFT_META_OIF);
> > -               add_cmp_u32(r, if_nametoindex(iface), op);
> > -       }
> > +       else
> > +               add_cmp_ptr(r, op, iface, iface_len + 1);
> >  }
> >
> >  void add_addr(struct nft_rule *r, int offset,
> > @@ -251,15 +247,14 @@ void parse_meta(struct nft_rule_expr *e, uint8_t key, char *iniface,
> >                         *invflags |= IPT_INV_VIA_IN;
> >
> >                 memcpy(iniface, ifname, len);
> > -               iniface[len] = '\0';
> >
> > -               /* If zero, then this is an interface mask */
> > -               if (if_nametoindex(iniface) == 0) {
> > +               if (iniface[len] == '\0')
> > +                       memset(iniface_mask, 0xff, len);
> > +               else {
> >                         iniface[len] = '+';
> >                         iniface[len+1] = '\0';
> > +                       memset(iniface_mask, 0xff, len + 1);
> >                 }
> > -
> > -               memset(iniface_mask, 0xff, len);
> >                 break;
> >         case NFT_META_OIFNAME:
> >                 ifname = nft_rule_expr_get(e, NFT_EXPR_CMP_DATA, &len);
> > @@ -267,15 +262,14 @@ void parse_meta(struct nft_rule_expr *e, uint8_t key, char *iniface,
> >                         *invflags |= IPT_INV_VIA_OUT;
> >
> >                 memcpy(outiface, ifname, len);
> > -               outiface[len] = '\0';
> >
> > -               /* If zero, then this is an interface mask */
> > -               if (if_nametoindex(outiface) == 0) {
> > +               if (outiface[len] == '\0')
> > +                       memset(outiface_mask, 0xff, len);
> > +               else {
> >                         outiface[len] = '+';
> >                         outiface[len+1] = '\0';
> > +                       memset(outiface_mask, 0xff, len + 1);
> >                 }
> > -
> > -               memset(outiface_mask, 0xff, len);
> >                 break;
> >         default:
> >                 DEBUGP("unknown meta key %d\n", key);
> 
> Pablo,
> This again breaks the delete functionality .

This is working here with a fresh compilation:

# xtables -I INPUT -i eth+
# xtables -D INPUT -i eth+
# xtables -I INPUT -i eth0
# xtables -D INPUT -i eth0

# which xtables
/usr/sbin/xtables
# ls -la /usr/sbin/xtables
lrwxrwxrwx 1 root root 13 oct 17 10:42 /usr/sbin/xtables -> xtables-multi

What problem are you noticing?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH iptables-nftables] nft: fix interface wildcard matching
  2013-10-17  8:43   ` Pablo Neira Ayuso
@ 2013-10-17 12:47     ` Anand Raj Manickam
  2013-10-17 13:27       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 5+ messages in thread
From: Anand Raj Manickam @ 2013-10-17 12:47 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On Thu, Oct 17, 2013 at 2:13 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Oct 17, 2013 at 02:09:05PM +0530, Anand Raj Manickam wrote:
>> On Wed, Oct 16, 2013 at 7:37 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> > In (73ea1cc nft: convert rule into a command state structure), the
>> > interface wildcard matching got broken. The previous handling was
>> > flawed by the use of ifnametoindex in scenario where the interface
>> > may vanished after a rule was added.
>> >
>> > This approach relies on the trailing '\0' to identify if this is
>> > an exact or wildcard matching, based on discussion with Florian.
>> >
>> > Based on initial patch from Anand Raj Manickam.
>> >
>> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>> > ---
>> >  iptables/nft-shared.c |   38 ++++++++++++++++----------------------
>> >  1 file changed, 16 insertions(+), 22 deletions(-)
>> >
>> > diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
>> > index 3987f74..e0eaa17 100644
>> > --- a/iptables/nft-shared.c
>> > +++ b/iptables/nft-shared.c
>> > @@ -124,13 +124,11 @@ void add_iniface(struct nft_rule *r, char *iface, int invflags)
>> >         else
>> >                 op = NFT_CMP_EQ;
>> >
>> > -       if (iface[iface_len - 1] == '+') {
>> > -               add_meta(r, NFT_META_IIFNAME);
>> > +       add_meta(r, NFT_META_IIFNAME);
>> > +       if (iface[iface_len - 1] == '+')
>> >                 add_cmp_ptr(r, op, iface, iface_len - 1);
>> > -       } else {
>> > -               add_meta(r, NFT_META_IIF);
>> > -               add_cmp_u32(r, if_nametoindex(iface), op);
>> > -       }
>> > +       else
>> > +               add_cmp_ptr(r, op, iface, iface_len + 1);
>> >  }
>> >
>> >  void add_outiface(struct nft_rule *r, char *iface, int invflags)
>> > @@ -145,13 +143,11 @@ void add_outiface(struct nft_rule *r, char *iface, int invflags)
>> >         else
>> >                 op = NFT_CMP_EQ;
>> >
>> > -       if (iface[iface_len - 1] == '+') {
>> > -               add_meta(r, NFT_META_OIFNAME);
>> > +       add_meta(r, NFT_META_OIFNAME);
>> > +       if (iface[iface_len - 1] == '+')
>> >                 add_cmp_ptr(r, op, iface, iface_len - 1);
>> > -       } else {
>> > -               add_meta(r, NFT_META_OIF);
>> > -               add_cmp_u32(r, if_nametoindex(iface), op);
>> > -       }
>> > +       else
>> > +               add_cmp_ptr(r, op, iface, iface_len + 1);
>> >  }
>> >
>> >  void add_addr(struct nft_rule *r, int offset,
>> > @@ -251,15 +247,14 @@ void parse_meta(struct nft_rule_expr *e, uint8_t key, char *iniface,
>> >                         *invflags |= IPT_INV_VIA_IN;
>> >
>> >                 memcpy(iniface, ifname, len);
>> > -               iniface[len] = '\0';
>> >
>> > -               /* If zero, then this is an interface mask */
>> > -               if (if_nametoindex(iniface) == 0) {
>> > +               if (iniface[len] == '\0')
>> > +                       memset(iniface_mask, 0xff, len);
>> > +               else {
>> >                         iniface[len] = '+';
>> >                         iniface[len+1] = '\0';
>> > +                       memset(iniface_mask, 0xff, len + 1);
>> >                 }
>> > -
>> > -               memset(iniface_mask, 0xff, len);
>> >                 break;
>> >         case NFT_META_OIFNAME:
>> >                 ifname = nft_rule_expr_get(e, NFT_EXPR_CMP_DATA, &len);
>> > @@ -267,15 +262,14 @@ void parse_meta(struct nft_rule_expr *e, uint8_t key, char *iniface,
>> >                         *invflags |= IPT_INV_VIA_OUT;
>> >
>> >                 memcpy(outiface, ifname, len);
>> > -               outiface[len] = '\0';
>> >
>> > -               /* If zero, then this is an interface mask */
>> > -               if (if_nametoindex(outiface) == 0) {
>> > +               if (outiface[len] == '\0')
>> > +                       memset(outiface_mask, 0xff, len);
>> > +               else {
>> >                         outiface[len] = '+';
>> >                         outiface[len+1] = '\0';
>> > +                       memset(outiface_mask, 0xff, len + 1);
>> >                 }
>> > -
>> > -               memset(outiface_mask, 0xff, len);
>> >                 break;
>> >         default:
>> >                 DEBUGP("unknown meta key %d\n", key);
>>
>> Pablo,
>> This again breaks the delete functionality .
>
> This is working here with a fresh compilation:
>
> # xtables -I INPUT -i eth+
> # xtables -D INPUT -i eth+
> # xtables -I INPUT -i eth0
> # xtables -D INPUT -i eth0
>
> # which xtables
> /usr/sbin/xtables
> # ls -la /usr/sbin/xtables
> lrwxrwxrwx 1 root root 13 oct 17 10:42 /usr/sbin/xtables -> xtables-multi
>
> What problem are you noticing?
Sorry about it , i guess i had  the wrong build .. works great ..
Do you still want to maintain the refrence for  NFT_META_IIF / NFT_META_OIF ?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH iptables-nftables] nft: fix interface wildcard matching
  2013-10-17 12:47     ` Anand Raj Manickam
@ 2013-10-17 13:27       ` Pablo Neira Ayuso
  0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-10-17 13:27 UTC (permalink / raw)
  To: Anand Raj Manickam; +Cc: netfilter-devel

On Thu, Oct 17, 2013 at 06:17:25PM +0530, Anand Raj Manickam wrote:
> On Thu, Oct 17, 2013 at 2:13 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Thu, Oct 17, 2013 at 02:09:05PM +0530, Anand Raj Manickam wrote:
> >> On Wed, Oct 16, 2013 at 7:37 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
[...]
> >> This again breaks the delete functionality .
> >
> > This is working here with a fresh compilation:
> >
> > # xtables -I INPUT -i eth+
> > # xtables -D INPUT -i eth+
> > # xtables -I INPUT -i eth0
> > # xtables -D INPUT -i eth0
> >
> > # which xtables
> > /usr/sbin/xtables
> > # ls -la /usr/sbin/xtables
> > lrwxrwxrwx 1 root root 13 oct 17 10:42 /usr/sbin/xtables -> xtables-multi
> >
> > What problem are you noticing?
>
> Sorry about it , i guess i had  the wrong build .. works great ..

No problem.

> Do you still want to maintain the refrence for  NFT_META_IIF / NFT_META_OIF ?

Yes, currently you cannot use nftables and xtables at the same time,
but some degree of interaction is desired. This should allow xtables
to interpret add rule added using ifindex from nft.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-10-17 13:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-16 14:07 [PATCH iptables-nftables] nft: fix interface wildcard matching Pablo Neira Ayuso
2013-10-17  8:39 ` Anand Raj Manickam
2013-10-17  8:43   ` Pablo Neira Ayuso
2013-10-17 12:47     ` Anand Raj Manickam
2013-10-17 13:27       ` 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).