xt_addrtype limit-iface-in is broken for ipv6

xt_addrtype limit-iface-in is broken for ipv6

[Florian Westphal]

quoting

https://bugzilla.netfilter.org/show_bug.cgi?id=812

[..]
When I tried to use in the nat/PREROUTING it messes up the
routing cache even if the rule didn't matched at all.
[..]
If I remove the --limit-iface-in from the non-working scenario, so just
use the -m addrtype --dst-type LOCAL it works!

This happens when LOCAL type matching is requested with
--limit-iface-in, and the default route is via the interface the packet
we test arrived on.

Because xt_addrtype uses ip6_route_output, the ipv6 routing
implementation creates an unwanted cached entry, and the packet
won't make it to the real/expected destination.

Silently ignoring --limit-iface-in makes the routing work
but it breaks rule matching (--dst-type LOCAL with limit-iface-in
is supposed to only match if the dst address is configured on
the incoming interface; without --limit-iface-in it will match if
the address is reachable via lo).

AFAIU the only solution is to use ipv6_chk_addr() when
LOCAL is requested instead of a route lookup.

Since this would create a dependeny on ipv6 its a no-go.
So, it boils down to two possible solutions:

a), extend struct nf_afinfo to also register
    ipv6_chk_addr(), OR
b), revert the commit that moved ipt_addrtype to xt_addrtype,
    and keep the ipv6 code in ip6t_addrtype.

IMO, the latter seems to be preferable.

http://git.breakpoint.cc/cgit.cgi/fw/nf-next.git/commit/?h=addrtype_2&id=3b30d692a351237f73b37b218f5310c5dc29cec0

implements this.

I plan to test this some more and will submit a patch once -next opens.

If someone has a better solution, please let me know.

Regards,
Florian

Re: xt_addrtype limit-iface-in is broken for ipv6

[Pablo Neira Ayuso]

On Sun, May 05, 2013 at 11:36:10AM +0200, Florian Westphal wrote:
> quoting
> 
> https://bugzilla.netfilter.org/show_bug.cgi?id=812
> 
> [..]
> When I tried to use in the nat/PREROUTING it messes up the
> routing cache even if the rule didn't matched at all.
> [..]
> If I remove the --limit-iface-in from the non-working scenario, so just
> use the -m addrtype --dst-type LOCAL it works!
> 
> This happens when LOCAL type matching is requested with
> --limit-iface-in, and the default route is via the interface the packet
> we test arrived on.
> 
> Because xt_addrtype uses ip6_route_output, the ipv6 routing
> implementation creates an unwanted cached entry, and the packet
> won't make it to the real/expected destination.
> 
> Silently ignoring --limit-iface-in makes the routing work
> but it breaks rule matching (--dst-type LOCAL with limit-iface-in
> is supposed to only match if the dst address is configured on
> the incoming interface; without --limit-iface-in it will match if
> the address is reachable via lo).
> 
> AFAIU the only solution is to use ipv6_chk_addr() when
> LOCAL is requested instead of a route lookup.
> 
> Since this would create a dependeny on ipv6 its a no-go.
> So, it boils down to two possible solutions:
> 
> a), extend struct nf_afinfo to also register
>     ipv6_chk_addr(), OR
> b), revert the commit that moved ipt_addrtype to xt_addrtype,
>     and keep the ipv6 code in ip6t_addrtype.

I'd prefer something smaller  so I can pass a fix to -stable. We
cannot pass patches bigger than 100 lines including context.

Re: xt_addrtype limit-iface-in is broken for ipv6

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Because xt_addrtype uses ip6_route_output, the ipv6 routing
> > implementation creates an unwanted cached entry, and the packet
> > won't make it to the real/expected destination.
> > 
> > Silently ignoring --limit-iface-in makes the routing work
> > but it breaks rule matching (--dst-type LOCAL with limit-iface-in
> > is supposed to only match if the dst address is configured on
> > the incoming interface; without --limit-iface-in it will match if
> > the address is reachable via lo).
> > 
> > AFAIU the only solution is to use ipv6_chk_addr() when
> > LOCAL is requested instead of a route lookup.
> > 
> > Since this would create a dependeny on ipv6 its a no-go.
> > So, it boils down to two possible solutions:
> > 
> > a), extend struct nf_afinfo to also register
> >     ipv6_chk_addr(), OR
> > b), revert the commit that moved ipt_addrtype to xt_addrtype,
> >     and keep the ipv6 code in ip6t_addrtype.
> 
> I'd prefer something smaller  so I can pass a fix to -stable. We
> cannot pass patches bigger than 100 lines including context.

This will be tough.  Extending struct nf_afinfo for
ipv6_chk_addr MIGHT come in just under 100 lines.  I'll have go at this.

[ i don't like this solution because we add a something for the sake
  of a single ipv6 special case ].