Re: [iptables PATCH] xtables: Fix for matching rules with wildcard interfaces

2018-10-30 Thread Pablo Neira Ayuso
On Tue, Oct 30, 2018 at 06:45:20PM +0100, Phil Sutter wrote:
> Hi Pablo,
> 
> On Tue, Oct 30, 2018 at 06:01:19PM +0100, Pablo Neira Ayuso wrote:
> > On Tue, Oct 30, 2018 at 05:57:53PM +0100, Phil Sutter wrote:
> > > Due to xtables_parse_interface() and parse_ifname() being misaligned
> > > regarding interface mask setting, rules containing a wildcard interface
> > > added with iptables-nft could neither be checked nor deleted.
> > > 
> > > Signed-off-by: Phil Sutter 
> > > ---
> > >  iptables/nft-shared.c|  2 +-
> > >  .../shell/testcases/nft-only/0004wildcard-iface_0| 12 
> > >  2 files changed, 13 insertions(+), 1 deletion(-)
> > >  create mode 100755 
> > > iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0
> > > 
> > > diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
> > > index 492e4ec124a79..7b8ca5e4becaf 100644
> > > --- a/iptables/nft-shared.c
> > > +++ b/iptables/nft-shared.c
> > > @@ -249,7 +249,7 @@ static void parse_ifname(const char *name, unsigned 
> > > int len, char *dst, unsigned
> > >   return;
> > >   dst[len++] = 0;
> > >   if (mask)
> > > - memset(mask, 0xff, len + 1);
> > > + memset(mask, 0xff, len - 2);
> > >  }
> > >  
> > >  int parse_meta(struct nftnl_expr *e, uint8_t key, char *iniface,
> > > diff --git a/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0 
> > > b/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0
> > > new file mode 100755
> > > index 0..b7c398ecbb29c
> > > --- /dev/null
> > > +++ b/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0
> > > @@ -0,0 +1,12 @@
> > > +#!/bin/bash
> > > +
> > > +# Make sure rules containing wildcard interfaces are found again.
> > > +
> > > +set -e
> > > +
> > > +[[ $XT_MULTI == */xtables-nft-multi ]] || { echo "skip $XT_MULTI"; exit 
> > > 0; }
> > > +
> > > +lname='alongifacename+'
> > > +$XT_MULTI iptables -A FORWARD -i eth+ -o $lname -j ACCEPT
> > > +$XT_MULTI iptables -C FORWARD -i eth+ -o $lname -j ACCEPT
> > > +$XT_MULTI iptables -D FORWARD -i eth+ -o $lname -j ACCEPT
> > 
> > Suggestion: Probably we can catch this through tests/py/, just a
> > suggestion. -C and -D operations, very much look the same from
> > interface perspective, so just checking for -I then -D should be fine
> > as tests/py.
> 
> Yes, testing for -C and -D is kind of redundant, though shouldn't matter
> much as it's just one more command. What do you mean with tests/py?
> There's no such thing in iptables repository? :)

Refering to iptables-tests.py, eg. extensions/libxt_mark.t, we can add
a iptables.t file to test for built-in selectors, eg. -s, -d, -i, -o
and so on. Also update iptables-tests.py to check for iptables.t.

If you don't like iptables/iptables.t, just pick a better name /
location in the tree :)


Re: [iptables PATCH] xtables: Fix for matching rules with wildcard interfaces

2018-10-30 Thread Phil Sutter
Hi Pablo,

On Tue, Oct 30, 2018 at 06:01:19PM +0100, Pablo Neira Ayuso wrote:
> On Tue, Oct 30, 2018 at 05:57:53PM +0100, Phil Sutter wrote:
> > Due to xtables_parse_interface() and parse_ifname() being misaligned
> > regarding interface mask setting, rules containing a wildcard interface
> > added with iptables-nft could neither be checked nor deleted.
> > 
> > Signed-off-by: Phil Sutter 
> > ---
> >  iptables/nft-shared.c|  2 +-
> >  .../shell/testcases/nft-only/0004wildcard-iface_0| 12 
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> >  create mode 100755 
> > iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0
> > 
> > diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
> > index 492e4ec124a79..7b8ca5e4becaf 100644
> > --- a/iptables/nft-shared.c
> > +++ b/iptables/nft-shared.c
> > @@ -249,7 +249,7 @@ static void parse_ifname(const char *name, unsigned int 
> > len, char *dst, unsigned
> > return;
> > dst[len++] = 0;
> > if (mask)
> > -   memset(mask, 0xff, len + 1);
> > +   memset(mask, 0xff, len - 2);
> >  }
> >  
> >  int parse_meta(struct nftnl_expr *e, uint8_t key, char *iniface,
> > diff --git a/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0 
> > b/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0
> > new file mode 100755
> > index 0..b7c398ecbb29c
> > --- /dev/null
> > +++ b/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0
> > @@ -0,0 +1,12 @@
> > +#!/bin/bash
> > +
> > +# Make sure rules containing wildcard interfaces are found again.
> > +
> > +set -e
> > +
> > +[[ $XT_MULTI == */xtables-nft-multi ]] || { echo "skip $XT_MULTI"; exit 0; 
> > }
> > +
> > +lname='alongifacename+'
> > +$XT_MULTI iptables -A FORWARD -i eth+ -o $lname -j ACCEPT
> > +$XT_MULTI iptables -C FORWARD -i eth+ -o $lname -j ACCEPT
> > +$XT_MULTI iptables -D FORWARD -i eth+ -o $lname -j ACCEPT
> 
> Suggestion: Probably we can catch this through tests/py/, just a
> suggestion. -C and -D operations, very much look the same from
> interface perspective, so just checking for -I then -D should be fine
> as tests/py.

Yes, testing for -C and -D is kind of redundant, though shouldn't matter
much as it's just one more command. What do you mean with tests/py?
There's no such thing in iptables repository? :)

Thanks, Phil


Re: [iptables PATCH] xtables: Fix for matching rules with wildcard interfaces

2018-10-30 Thread Pablo Neira Ayuso
On Tue, Oct 30, 2018 at 05:57:53PM +0100, Phil Sutter wrote:
> Due to xtables_parse_interface() and parse_ifname() being misaligned
> regarding interface mask setting, rules containing a wildcard interface
> added with iptables-nft could neither be checked nor deleted.
> 
> Signed-off-by: Phil Sutter 
> ---
>  iptables/nft-shared.c|  2 +-
>  .../shell/testcases/nft-only/0004wildcard-iface_0| 12 
>  2 files changed, 13 insertions(+), 1 deletion(-)
>  create mode 100755 
> iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0
> 
> diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
> index 492e4ec124a79..7b8ca5e4becaf 100644
> --- a/iptables/nft-shared.c
> +++ b/iptables/nft-shared.c
> @@ -249,7 +249,7 @@ static void parse_ifname(const char *name, unsigned int 
> len, char *dst, unsigned
>   return;
>   dst[len++] = 0;
>   if (mask)
> - memset(mask, 0xff, len + 1);
> + memset(mask, 0xff, len - 2);
>  }
>  
>  int parse_meta(struct nftnl_expr *e, uint8_t key, char *iniface,
> diff --git a/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0 
> b/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0
> new file mode 100755
> index 0..b7c398ecbb29c
> --- /dev/null
> +++ b/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0
> @@ -0,0 +1,12 @@
> +#!/bin/bash
> +
> +# Make sure rules containing wildcard interfaces are found again.
> +
> +set -e
> +
> +[[ $XT_MULTI == */xtables-nft-multi ]] || { echo "skip $XT_MULTI"; exit 0; }
> +
> +lname='alongifacename+'
> +$XT_MULTI iptables -A FORWARD -i eth+ -o $lname -j ACCEPT
> +$XT_MULTI iptables -C FORWARD -i eth+ -o $lname -j ACCEPT
> +$XT_MULTI iptables -D FORWARD -i eth+ -o $lname -j ACCEPT

Suggestion: Probably we can catch this through tests/py/, just a
suggestion. -C and -D operations, very much look the same from
interface perspective, so just checking for -I then -D should be fine
as tests/py.


[iptables PATCH] xtables: Fix for matching rules with wildcard interfaces

2018-10-30 Thread Phil Sutter
Due to xtables_parse_interface() and parse_ifname() being misaligned
regarding interface mask setting, rules containing a wildcard interface
added with iptables-nft could neither be checked nor deleted.

Signed-off-by: Phil Sutter 
---
 iptables/nft-shared.c|  2 +-
 .../shell/testcases/nft-only/0004wildcard-iface_0| 12 
 2 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100755 iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 492e4ec124a79..7b8ca5e4becaf 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -249,7 +249,7 @@ static void parse_ifname(const char *name, unsigned int 
len, char *dst, unsigned
return;
dst[len++] = 0;
if (mask)
-   memset(mask, 0xff, len + 1);
+   memset(mask, 0xff, len - 2);
 }
 
 int parse_meta(struct nftnl_expr *e, uint8_t key, char *iniface,
diff --git a/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0 
b/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0
new file mode 100755
index 0..b7c398ecbb29c
--- /dev/null
+++ b/iptables/tests/shell/testcases/nft-only/0004wildcard-iface_0
@@ -0,0 +1,12 @@
+#!/bin/bash
+
+# Make sure rules containing wildcard interfaces are found again.
+
+set -e
+
+[[ $XT_MULTI == */xtables-nft-multi ]] || { echo "skip $XT_MULTI"; exit 0; }
+
+lname='alongifacename+'
+$XT_MULTI iptables -A FORWARD -i eth+ -o $lname -j ACCEPT
+$XT_MULTI iptables -C FORWARD -i eth+ -o $lname -j ACCEPT
+$XT_MULTI iptables -D FORWARD -i eth+ -o $lname -j ACCEPT
-- 
2.19.0