Re: [PATCH nf-next] netfilter: x_tables: add context to know if extension runs from nft_compat

[Patrick McHardy <kaber@trash.net>]

On 02.03, Pablo Neira Ayuso wrote:
> On Mon, Mar 02, 2015 at 02:19:00PM +0000, Patrick McHardy wrote:
> > On 02.03, Pablo Neira Ayuso wrote:
> > > On Mon, Mar 02, 2015 at 01:19:01PM +0000, Patrick McHardy wrote:
> > > > > 
> > > > > 3) SYMPROXY6. Don't check for e->ipv6.flags, we can instead check
> > > > >    for e->ipv6.proto as other extensions do, if zero then it doesn't
> > > > >    fulfill the dependency.
> > > > 
> > > > But we don't perform a protocol match in ip6_tables if the IP6T_F_PROTO
> > > > flag is not given. ip6_tables differs from ip_tables in this regard.
> > > 
> > > This just makes sure that SYNPROXY6 is not called for non-tcp traffic
> > > in the rule loading path, which should be OK.
> > 
> > Yeah, but for ip6_tables we actually need the check the way it is,
> > without IP6T_F_PROTO we will not perform the protocol match.
> 
>         if (!(e->ipv6.flags & IP6T_F_PROTO) ||
>             e->ipv6.proto != IPPROTO_TCP ||
>             e->ipv6.invflags & XT_INV_PROTO)
>                 return -EINVAL;
> 
> e->ipv6.flags & IP6T_F_PROTO seems redundant to me, it just says
> e->ipv6.proto is set.

No, it also instructs ip6_tables to actually match on that value.

> If that flag is not set, then e->ipv6.proto is left unset. But the
> effect should be the same since 0 != IPPROTO_TCP.
> 
> This is just relaxing the validation in SYNPROXY6 to only check
> e->ipv6.proto which is what nft_compat sets.
> 
> Am I missing anything?

Yes, ip6_tables only performs the protocol match when that flag is set.
Look:

        /* look for the desired protocol header */
        if((ip6info->flags & IP6T_F_PROTO)) {
		...
		if (ip6info->proto == protohdr) {
	
This is where ip6_tables and ip_tables are different, ip_tables
takes ipinfo->proto as an indicator, ip6_tables the flag.