nft: ah expression doesn't work for IPv6

nft: ah expression doesn't work for IPv6

[Phil Sutter]

Hi,

There is currently an open issue with nft in that ah expression doesn't
work for IPv6 since it creates a payload expression which doesn't find
the AH IPv6 extension header.

There has been a discussion about this problem off-list in which two
alternative "solutions" were named, both involve checking the context
to find out whether one is trying to match an IPv4 or IPv6 packet and
then create either payload or exthdr expression. Though this solution is
not optimal, as in (at least) inet table the address family is not given
per se.

I had the idea of creating a "real" solution to the problem which
consists of a payload/exthdr hybrid in kernel space searching for the
given header in different locations depending on the family of the
actual package being handled. Ideally this should reuse as much code as
possible, so maybe "just" create both expressions in user space and in
kernel space only branch between the two.

This is merely a rough idea and I probably miss some obstacles which
have to be overcome in order to make this work. What are your opinions
on that? Do you think something like that could work, or do you have a
better idea to fix this problem?

Thanks, Phil

Re: nft: ah expression doesn't work for IPv6

[Pablo Neira Ayuso]

On Thu, Mar 02, 2017 at 08:56:52PM +0100, Phil Sutter wrote:
> Hi,
> 
> There is currently an open issue with nft in that ah expression doesn't
> work for IPv6 since it creates a payload expression which doesn't find
> the AH IPv6 extension header.
> 
> There has been a discussion about this problem off-list in which two
> alternative "solutions" were named, both involve checking the context
> to find out whether one is trying to match an IPv4 or IPv6 packet and
> then create either payload or exthdr expression. Though this solution is
> not optimal, as in (at least) inet table the address family is not given
> per se.
> 
> I had the idea of creating a "real" solution to the problem which
> consists of a payload/exthdr hybrid in kernel space searching for the
> given header in different locations depending on the family of the
> actual package being handled. Ideally this should reuse as much code as
> possible, so maybe "just" create both expressions in user space and in
> kernel space only branch between the two.

If the problem is the inet chain, I would prefer we request explicit
dependencies for ah so we generate the right bytecode depending on the
family. Yes, I mean we would need two different rules for each case by
now.

On top of that, do you have a real usecase having both AH traffic for
IPv4 and IPv6 traffic? If you don't I would prefer you just fix what
we have and focus on a different task, we have plenty of work to do
ahead. We can hand over you more useful tasks.

Re: nft: ah expression doesn't work for IPv6

[Pablo Neira Ayuso]

On Thu, Mar 02, 2017 at 10:01:29PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Mar 02, 2017 at 08:56:52PM +0100, Phil Sutter wrote:
> > Hi,
> > 
> > There is currently an open issue with nft in that ah expression doesn't
> > work for IPv6 since it creates a payload expression which doesn't find
> > the AH IPv6 extension header.
> > 
> > There has been a discussion about this problem off-list in which two
> > alternative "solutions" were named, both involve checking the context
> > to find out whether one is trying to match an IPv4 or IPv6 packet and
> > then create either payload or exthdr expression. Though this solution is
> > not optimal, as in (at least) inet table the address family is not given
> > per se.
> > 
> > I had the idea of creating a "real" solution to the problem which
> > consists of a payload/exthdr hybrid in kernel space searching for the
> > given header in different locations depending on the family of the
> > actual package being handled. Ideally this should reuse as much code as
> > possible, so maybe "just" create both expressions in user space and in
> > kernel space only branch between the two.
> 
> If the problem is the inet chain, I would prefer we request explicit
> dependencies for ah so we generate the right bytecode depending on the
> family. Yes, I mean we would need two different rules for each case by
> now.
> 
> On top of that, do you have a real usecase having both AH traffic for
> IPv4 and IPv6 traffic? If you don't I would prefer you just fix what
> we have and focus on a different task, we have plenty of work to do
> ahead. We can hand over you more useful tasks.

Sorry, I didn't mean to sound rude. But I would like to see more
usecases that would fit into such hybrid. I think we need more
usecases (not only AH) to justify this extra complexity that this
hybrid (or probably something else we can come up with) requires.

Re: nft: ah expression doesn't work for IPv6

[Phil Sutter]

On Thu, Mar 02, 2017 at 10:25:22PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Mar 02, 2017 at 10:01:29PM +0100, Pablo Neira Ayuso wrote:
> > On Thu, Mar 02, 2017 at 08:56:52PM +0100, Phil Sutter wrote:
> > > Hi,
> > > 
> > > There is currently an open issue with nft in that ah expression doesn't
> > > work for IPv6 since it creates a payload expression which doesn't find
> > > the AH IPv6 extension header.
> > > 
> > > There has been a discussion about this problem off-list in which two
> > > alternative "solutions" were named, both involve checking the context
> > > to find out whether one is trying to match an IPv4 or IPv6 packet and
> > > then create either payload or exthdr expression. Though this solution is
> > > not optimal, as in (at least) inet table the address family is not given
> > > per se.
> > > 
> > > I had the idea of creating a "real" solution to the problem which
> > > consists of a payload/exthdr hybrid in kernel space searching for the
> > > given header in different locations depending on the family of the
> > > actual package being handled. Ideally this should reuse as much code as
> > > possible, so maybe "just" create both expressions in user space and in
> > > kernel space only branch between the two.
> > 
> > If the problem is the inet chain, I would prefer we request explicit
> > dependencies for ah so we generate the right bytecode depending on the
> > family. Yes, I mean we would need two different rules for each case by
> > now.

Well, in bison there would have to be a common entrance point, which
checks the scope and then generates either payload expr, exthdr expr or
an error depending on what it is. Correct?

> > On top of that, do you have a real usecase having both AH traffic for
> > IPv4 and IPv6 traffic? If you don't I would prefer you just fix what
> > we have and focus on a different task, we have plenty of work to do
> > ahead. We can hand over you more useful tasks.

Not sure if I understand you correctly, but the use-case is matching on
some AH header field in IPv6 traffic? But sure, if you think that's too
exotic I could maybe just add some check somewhere which causes an error
if the family is not specified or not IPv4 and ah expression is being
used.

> Sorry, I didn't mean to sound rude. But I would like to see more
> usecases that would fit into such hybrid. I think we need more
> usecases (not only AH) to justify this extra complexity that this
> hybrid (or probably something else we can come up with) requires.

No problem! Didn't sound rude to me, just inappropriate - it's not up to
you what I'm wasting my time for. :)

Cheers, Phil

Re: nft: ah expression doesn't work for IPv6

[Pablo Neira Ayuso]

On Fri, Mar 03, 2017 at 03:01:57PM +0100, Phil Sutter wrote:
> On Thu, Mar 02, 2017 at 10:25:22PM +0100, Pablo Neira Ayuso wrote:
> > On Thu, Mar 02, 2017 at 10:01:29PM +0100, Pablo Neira Ayuso wrote:
> > > On Thu, Mar 02, 2017 at 08:56:52PM +0100, Phil Sutter wrote:
[...]
> > > If the problem is the inet chain, I would prefer we request explicit
> > > dependencies for ah so we generate the right bytecode depending on the
> > > family. Yes, I mean we would need two different rules for each case by
> > > now.
> 
> Well, in bison there would have to be a common entrance point, which
> checks the scope and then generates either payload expr, exthdr expr or
> an error depending on what it is. Correct?

That's right.

> > > On top of that, do you have a real usecase having both AH traffic for
> > > IPv4 and IPv6 traffic? If you don't I would prefer you just fix what
> > > we have and focus on a different task, we have plenty of work to do
> > > ahead. We can hand over you more useful tasks.
> 
> Not sure if I understand you correctly, but the use-case is matching on
> some AH header field in IPv6 traffic? But sure, if you think that's too
> exotic I could maybe just add some check somewhere which causes an error
> if the family is not specified or not IPv4 and ah expression is being
> used.

As you indicate the AH handling in nft is inconsistent, just like it
happens in iptables and ip6tables, look:

1) in iptables we can do -p ah, then use an extension to match AH
   header from transport.
2) in ip6tables we cannot do -p ah. We use a specific ah extension for
   IPv6 that uses ipv6_find_hdr().

This is a mess.

Please, develop your hybrid idea.