Re: Use of oifname in input chains

Re: Use of oifname in input chains

[Pablo Neira Ayuso]

On Tue, Jun 25, 2019 at 02:29:54PM +0200, Phil Sutter wrote:
> Hi Pablo,
> 
> I have a ticket complaining that it is possible to use 'oifname' match
> within a base chain hooked into input. Nftables behaves correctly, the
> statement simply won't ever match. Question is whether this is a bug or
> feature:

Matching oifname on input does not make sense. You can just narrow
down this from the control plane patch via kernel patch to reject
this.

> Bug because the rule clearly won't ever match and so does not make sense
> when used in a base chain.
> 
> Feature because one could add the rule to a non-base chain and jump to
> it from any hook to reduce duplication in ruleset. We would have to
> check rules in the target chain while validating the rule containing the
> jump.
> 
> What do you think?

How does this behave in iptables BTW? I think iptables simply allows
this, but it won't ever match obviously.

Re: Use of oifname in input chains

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Feature because one could add the rule to a non-base chain and jump to
> > it from any hook to reduce duplication in ruleset. We would have to
> > check rules in the target chain while validating the rule containing the
> > jump.
> > 
> > What do you think?
> 
> How does this behave in iptables BTW? I think iptables simply allows
> this, but it won't ever match obviously.

iptables userspace will reject iptables -A INPUT -o foo.
-A FOO -o foo will "work", even if we only have a -j FOO from INPUT.

I don't think its worth to add tracking for this to kernel:

new chain C
meta oifname bla added to C
jump added from output to C
jump added from input to C   # should this fail? why?

new chain C
jump added from input to C
meta oifname added to C	     # same q: why should this fail?

Re: Use of oifname in input chains

[Pablo Neira Ayuso]

On Wed, Jun 26, 2019 at 12:32:30PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > Feature because one could add the rule to a non-base chain and jump to
> > > it from any hook to reduce duplication in ruleset. We would have to
> > > check rules in the target chain while validating the rule containing the
> > > jump.
> > > 
> > > What do you think?
> > 
> > How does this behave in iptables BTW? I think iptables simply allows
> > this, but it won't ever match obviously.
> 
> iptables userspace will reject iptables -A INPUT -o foo.
> -A FOO -o foo will "work", even if we only have a -j FOO from INPUT.
> 
> I don't think its worth to add tracking for this to kernel:
> 
> new chain C
> meta oifname bla added to C
> jump added from output to C
> jump added from input to C   # should this fail? why?
> 
> new chain C
> jump added from input to C
> meta oifname added to C	     # same q: why should this fail?

There's tracking infrastructure for this already in place, right? It's
just a matter to check for this from nft_meta_get_validate().

Re: Use of oifname in input chains

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > new chain C
> > meta oifname bla added to C
> > jump added from output to C
> > jump added from input to C   # should this fail? why?
> > 
> > new chain C
> > jump added from input to C
> > meta oifname added to C	     # same q: why should this fail?
> 
> There's tracking infrastructure for this already in place, right? It's
> just a matter to check for this from nft_meta_get_validate().

But what semantics would you add?
It seems it would 100% break existing rulesets.

new chain C
jump added from ouput to C
meta oifname added to C	   	# allowed? jump from output exists
jump added from input to C	# disallow this? Why?

..
delete jump from output		# disallow?

This seems rather suicidal to me.

Re: Use of oifname in input chains

[Pablo Neira Ayuso]

On Wed, Jun 26, 2019 at 12:42:54PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > new chain C
> > > meta oifname bla added to C
> > > jump added from output to C
> > > jump added from input to C   # should this fail? why?
> > > 
> > > new chain C
> > > jump added from input to C
> > > meta oifname added to C	     # same q: why should this fail?
> > 
> > There's tracking infrastructure for this already in place, right? It's
> > just a matter to check for this from nft_meta_get_validate().
> 
> But what semantics would you add?
> It seems it would 100% break existing rulesets.
> 
> new chain C
> jump added from ouput to C
> meta oifname added to C	   	# allowed? jump from output exists
> jump added from input to C	# disallow this? Why?

To me, it makes no sense to use oifname from the input chain
indirectly, even if this is being used from the C chain.

> ..
> delete jump from output		# disallow?
> 
> This seems rather suicidal to me.

OK, you think there may be people using oifname from the C chain, but
how so? To skip rules that are specific to the output path?

Anyway, I'm fine with leaving things as is, I don't need this. Just in
case you pass by here in the future, the tracking infrastructure
should allow for this.

Re: Use of oifname in input chains

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > delete jump from output		# disallow?
> > 
> > This seems rather suicidal to me.
> 
> OK, you think there may be people using oifname from the C chain, but
> how so? To skip rules that are specific to the output path?

Maybe, or just to consolidate rules, e.g.

chain C {
	[ some common rules ]
	meta oifname bla ...
	[ other common rules ]
}

After the proposed change, kernel refuses ruleset as soon as C is
or becomes reachable from a prerouting/input basechain.

(Alternatively, we could reject if not reachable from output/forward,
 but that seems even more crazy because we'd have to refuse ruleset
 that has unreachable chain with 'oifname' in it ...).

Re: Use of oifname in input chains

[Phil Sutter]

Hi,

On Wed, Jun 26, 2019 at 12:47:40PM +0200, Pablo Neira Ayuso wrote:
[...]
> OK, you think there may be people using oifname from the C chain, but
> how so? To skip rules that are specific to the output path?

My idea for how to use it was this:

| table ip t {
|     chain in {
|         type filter hook input priority 0; policy accept;
|         jump common
|     }
|
|     chain out {
|         type filter hook output priority 0; policy accept;
|         jump common
|     }
|
|     chain common {
|         iifname "eth0" tcp dport ssh counter packets 101 bytes 10149 accept
|         oifname "eth0" tcp sport ssh counter packets 65 bytes 8233 accept
|         counter packets 0 bytes 0 drop
|     }
| }

> Anyway, I'm fine with leaving things as is, I don't need this. Just in
> case you pass by here in the future, the tracking infrastructure
> should allow for this.

OK, cool. Thanks for clarifying upstream PoV. :)

Cheers, Phil

Re: Use of oifname in input chains

[Pablo Neira Ayuso]

On Wed, Jun 26, 2019 at 12:58:12PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > delete jump from output		# disallow?
> > > 
> > > This seems rather suicidal to me.
> > 
> > OK, you think there may be people using oifname from the C chain, but
> > how so? To skip rules that are specific to the output path?
> 
> Maybe, or just to consolidate rules, e.g.
> 
> chain C {
> 	[ some common rules ]
> 	meta oifname bla ...
> 	[ other common rules ]
> }
> 
> After the proposed change, kernel refuses ruleset as soon as C is
> or becomes reachable from a prerouting/input basechain.

I think it's more likely to misuse oifname from input path (eg. typo)
that finding someone with such usecase you describe above but...

> (Alternatively, we could reject if not reachable from output/forward,
>  but that seems even more crazy because we'd have to refuse ruleset
>  that has unreachable chain with 'oifname' in it ...).

... I have no problem whatsoever to leave the existing behaviour in place.

No need to keep spinning on this :-)

Re: Use of oifname in input chains

[Pablo Neira Ayuso]

On Wed, Jun 26, 2019 at 02:50:38PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Jun 26, 2019 at 12:58:12PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > delete jump from output		# disallow?
> > > > 
> > > > This seems rather suicidal to me.
> > > 
> > > OK, you think there may be people using oifname from the C chain, but
> > > how so? To skip rules that are specific to the output path?
> > 
> > Maybe, or just to consolidate rules, e.g.
> > 
> > chain C {
> > 	[ some common rules ]
> > 	meta oifname bla ...
> > 	[ other common rules ]
> > }
> > 
> > After the proposed change, kernel refuses ruleset as soon as C is
> > or becomes reachable from a prerouting/input basechain.
> 
> I think it's more likely to misuse oifname from input path (eg. typo)
> that finding someone with such usecase you describe above but...

For the usecase above, I would probably expose a 'meta hook' selector,
so you can restrict things depending on the path.

Anyway...

> > (Alternatively, we could reject if not reachable from output/forward,
> >  but that seems even more crazy because we'd have to refuse ruleset
> >  that has unreachable chain with 'oifname' in it ...).
> 
> ... I have no problem whatsoever to leave the existing behaviour in place.
> 
> No need to keep spinning on this :-)