All of lore.kernel.org
 help / color / mirror / Atom feed
* nftables queue target aborts rules processing unconditionally
@ 2017-03-03 15:01 Andreas Schultz
  2017-03-03 15:41 ` Florian Westphal
  0 siblings, 1 reply; 5+ messages in thread
From: Andreas Schultz @ 2017-03-03 15:01 UTC (permalink / raw)
  To: netfilter-devel

Hi,

The nft queueing seems to have broken the continuation of
rule processing when NF_ACCEPT is returned.

If have the following rules:

table ip filter {
	map client_to_any {
		type ipv4_addr : verdict
		elements = { 10.180.200.72 : goto CIn_1}
	}

	chain FORWARD {
		type filter hook forward priority 0; policy accept;
		iif { "eth0"} counter packets 1 bytes 84 goto client_to_any

	chain client_to_any {
		nftrace set 1 # handle 11
		counter packets 1 bytes 84 ip saddr vmap @client_to_any # handle 12
		counter packets 1 bytes 84 queue num 0 # handle 13
		counter packets 0 bytes 0 # handle 14
		counter packets 0 bytes 0 ip saddr vmap @client_to_any # handle 16
		goto DENY # handle 17
	}

}

The idea is that the first packet for an yet unknown client will
bypass rules #12, match rule 13 and land in queue 0. The userspace
process then generates the appropriate rules and return an NF_ACCEPT
on the queue.

This should continue the rule processing at rule #14 and finally
match on the update vmap in rule #16.

The problem is that the rule processing is not continuing as
you can see on the counters.

http://www.netfilter.org/documentation/HOWTO/netfilter-hacking-HOWTO.txt
clearly states:

> 1. NF_ACCEPT: continue traversal as normal.

So, why is the processing aborted?

It also appears as if the nft trace infrastructure does not now how to
deal with queues. The above rules lead to this annotated trace output:

> trace id 10d53daf ip filter client_to_any packet: iif "upstream" oif "ens256" ether saddr 00:50:56:96:9b:1c ether daddr 00:0c:29:46:1f:53 ether type ip6 

That's rule #11... Where is the hit on the queue rule and the return??

> add chain ip filter CIn_1
> add rule ip filter CIn_1 counter name "CIn_1_Session"
> add rule ip filter CIn_1 ip daddr 172.20.16.0/24 counter packets 0 bytes 0 accept
> add rule ip filter CIn_1 ip daddr 8.0.0.0/8 counter packets 0 bytes 0 accept
> add chain ip filter COut_1
> add rule ip filter COut_1 counter name "COut_1_Session"
> add rule ip filter COut_1 ip saddr 172.20.16.0/24 counter packets 0 bytes 0 accept
> add rule ip filter COut_1 ip saddr 8.0.0.0/8 counter packets 0 bytes 0 accept

This is the userspace process updating the rules. But the update of the
client_to_any verdict map is missing.

> trace id 10d53daf ip mangle POSTROUTING verdict continue 
> trace id 10d53daf ip mangle POSTROUTING 

The packets skipped the filter table processing completely. Not even the
application of the default accept policy is logged....

Also of this have been tested on net-next and nftables snapshot from 2017-03-01.

The missing trace are only cosmetic (albeit confusing during debugging), but
that the queue aborts the rule processing seems to be a bug.

Regards
Andreas

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: nftables queue target aborts rules processing unconditionally
  2017-03-03 15:01 nftables queue target aborts rules processing unconditionally Andreas Schultz
@ 2017-03-03 15:41 ` Florian Westphal
  2017-03-03 15:57   ` Andreas Schultz
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Westphal @ 2017-03-03 15:41 UTC (permalink / raw)
  To: Andreas Schultz; +Cc: netfilter-devel

Andreas Schultz <aschultz@tpip.net> wrote:
> Hi,
> 
> The nft queueing seems to have broken the continuation of
> rule processing when NF_ACCEPT is returned.

No, see below

> If have the following rules:
> 
> table ip filter {
> 	map client_to_any {
> 		type ipv4_addr : verdict
> 		elements = { 10.180.200.72 : goto CIn_1}
> 	}
> 
> 	chain FORWARD {
> 		type filter hook forward priority 0; policy accept;
> 		iif { "eth0"} counter packets 1 bytes 84 goto client_to_any
> 
> 	chain client_to_any {
> 		nftrace set 1 # handle 11
> 		counter packets 1 bytes 84 ip saddr vmap @client_to_any # handle 12
> 		counter packets 1 bytes 84 queue num 0 # handle 13
> 		counter packets 0 bytes 0 # handle 14
> 		counter packets 0 bytes 0 ip saddr vmap @client_to_any # handle 16
> 		goto DENY # handle 17
> 	}
> 
> }
> The idea is that the first packet for an yet unknown client will
> bypass rules #12, match rule 13 and land in queue 0. The userspace
> process then generates the appropriate rules and return an NF_ACCEPT
> on the queue.
> 
> This should continue the rule processing at rule #14 and finally
> match on the update vmap in rule #16.

No, unfortunately thats not how NF_QUEUE operates.

On a QUEUE verdict the packet leaves the rule set context,
both in iptables and nftables.

> The problem is that the rule processing is not continuing as
> you can see on the counters.
> 
> http://www.netfilter.org/documentation/HOWTO/netfilter-hacking-HOWTO.txt
> clearly states:
> 
> > 1. NF_ACCEPT: continue traversal as normal.
> 
> So, why is the processing aborted?

NF_ACCEPT makes packets move to the next *netfilter hook*,
but thats not the same as the next (nf|ip)tables rule.

e.g. in iptables if you NFQUEUE in mangle input packet re-appears
in filter input after an ACCEPT reinject.

> It also appears as if the nft trace infrastructure does not now how to
> deal with queues. The above rules lead to this annotated trace output:
> 
> > trace id 10d53daf ip filter client_to_any packet: iif "upstream" oif "ens256" ether saddr 00:50:56:96:9b:1c ether daddr 00:0c:29:46:1f:53 ether type ip6 
> 
> That's rule #11... Where is the hit on the queue rule and the return??

No idea, I will have a closer look next week.
Glancing at the code it should work just fine.

> The missing trace are only cosmetic (albeit confusing during debugging), but
> that the queue aborts the rule processing seems to be a bug.

Unfortunately no, this is how it has always been.

I think we could make it work better in nftables but it would require
a lot more work and it would leak nf_tables details into the generic
core.

We would have to
1. store a pointer to the rule head that caused the queueing in
nf_queue_entry struct
2. also store the current generation counter of the table
3. on reinject we'd have to check that rule head pointer is nonzero
(i.e. queued from nftables), then call into an nftables specific
reinject function that would check if the generation counter is
identical (to detect when rules might have been changed in meantime).

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: nftables queue target aborts rules processing unconditionally
  2017-03-03 15:41 ` Florian Westphal
@ 2017-03-03 15:57   ` Andreas Schultz
  2017-03-03 16:01     ` Florian Westphal
  0 siblings, 1 reply; 5+ messages in thread
From: Andreas Schultz @ 2017-03-03 15:57 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

Hi,

----- On Mar 3, 2017, at 4:41 PM, Florian Westphal fw@strlen.de wrote:

> Andreas Schultz <aschultz@tpip.net> wrote:
>> Hi,
>> 
>> The nft queueing seems to have broken the continuation of
>> rule processing when NF_ACCEPT is returned.
> 
> No, see below
> 
>> If have the following rules:
>> 
>> table ip filter {
>> 	map client_to_any {
>> 		type ipv4_addr : verdict
>> 		elements = { 10.180.200.72 : goto CIn_1}
>> 	}
>> 
>> 	chain FORWARD {
>> 		type filter hook forward priority 0; policy accept;
>> 		iif { "eth0"} counter packets 1 bytes 84 goto client_to_any
>> 
>> 	chain client_to_any {
>> 		nftrace set 1 # handle 11
>> 		counter packets 1 bytes 84 ip saddr vmap @client_to_any # handle 12
>> 		counter packets 1 bytes 84 queue num 0 # handle 13
>> 		counter packets 0 bytes 0 # handle 14
>> 		counter packets 0 bytes 0 ip saddr vmap @client_to_any # handle 16
>> 		goto DENY # handle 17
>> 	}
>> 
>> }
>> The idea is that the first packet for an yet unknown client will
>> bypass rules #12, match rule 13 and land in queue 0. The userspace
>> process then generates the appropriate rules and return an NF_ACCEPT
>> on the queue.
>> 
>> This should continue the rule processing at rule #14 and finally
>> match on the update vmap in rule #16.
> 
> No, unfortunately thats not how NF_QUEUE operates.
> 
> On a QUEUE verdict the packet leaves the rule set context,
> both in iptables and nftables.
> 
>> The problem is that the rule processing is not continuing as
>> you can see on the counters.
>> 
>> http://www.netfilter.org/documentation/HOWTO/netfilter-hacking-HOWTO.txt
>> clearly states:
>> 
>> > 1. NF_ACCEPT: continue traversal as normal.
>> 
>> So, why is the processing aborted?
> 
> NF_ACCEPT makes packets move to the next *netfilter hook*,
> but thats not the same as the next (nf|ip)tables rule.
> 
> e.g. in iptables if you NFQUEUE in mangle input packet re-appears
> in filter input after an ACCEPT reinject.

ok, somewhat unexpected (or rather undocumented), but I can live
with that.

I've now experimented with NF_REPEAT to achieve something similar.
Can I assume that NF_REPEAT should restart the current "netfilter hook*?
e.g. when we are somewhere in FILTER FORWARD, it will restart with the
first rule of that hook?

My experiments show that this works with nft when I don't modify the
ruleset. However, when I modify the ruleset before returning NF_REPEAT,
the packet will skip the current hook completely.

I don't modify the chain the packet is currently traversing. I only add
new chains and modify the vmap.

>> It also appears as if the nft trace infrastructure does not now how to
>> deal with queues. The above rules lead to this annotated trace output:
>> 
>> > trace id 10d53daf ip filter client_to_any packet: iif "upstream" oif "ens256"
>> > ether saddr 00:50:56:96:9b:1c ether daddr 00:0c:29:46:1f:53 ether type ip6
>> 
>> That's rule #11... Where is the hit on the queue rule and the return??
> 
> No idea, I will have a closer look next week.
> Glancing at the code it should work just fine.

There might a event buffering issue. I have now sometimes seen the queueing
trace. At other times the event is lost. So maybe the netlink buffer is not
large enough?

Thanks
Andreas


>> The missing trace are only cosmetic (albeit confusing during debugging), but
>> that the queue aborts the rule processing seems to be a bug.
> 
> Unfortunately no, this is how it has always been.
> 
> I think we could make it work better in nftables but it would require
> a lot more work and it would leak nf_tables details into the generic
> core.
> 
> We would have to
> 1. store a pointer to the rule head that caused the queueing in
> nf_queue_entry struct
> 2. also store the current generation counter of the table
> 3. on reinject we'd have to check that rule head pointer is nonzero
> (i.e. queued from nftables), then call into an nftables specific
> reinject function that would check if the generation counter is
> identical (to detect when rules might have been changed in meantime).

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: nftables queue target aborts rules processing unconditionally
  2017-03-03 15:57   ` Andreas Schultz
@ 2017-03-03 16:01     ` Florian Westphal
  2017-03-03 16:24       ` Andreas Schultz
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Westphal @ 2017-03-03 16:01 UTC (permalink / raw)
  To: Andreas Schultz; +Cc: Florian Westphal, netfilter-devel

Andreas Schultz <aschultz@tpip.net> wrote:
> ok, somewhat unexpected (or rather undocumented), but I can live
> with that.
> 
> I've now experimented with NF_REPEAT to achieve something similar.
> Can I assume that NF_REPEAT should restart the current "netfilter hook*?

Yes.

> e.g. when we are somewhere in FILTER FORWARD, it will restart with the
> first rule of that hook?

It restarts the hook, yes.

> My experiments show that this works with nft when I don't modify the
> ruleset. However, when I modify the ruleset before returning NF_REPEAT,
> the packet will skip the current hook completely.

Hmm, that shouldn't happen.
REPEAT should always just re-start the current hook.
If that hook gets deleted (and possibly re-created) while packet was
queued the kernel is supposed to drop the packet.

> I don't modify the chain the packet is currently traversing. I only add
> new chains and modify the vmap.

The netfilter infrastructure is a layer below nftables/iptables so it
is not even aware of rule set modifications.

> >> It also appears as if the nft trace infrastructure does not now how to
> >> deal with queues. The above rules lead to this annotated trace output:
> >> 
> >> > trace id 10d53daf ip filter client_to_any packet: iif "upstream" oif "ens256"
> >> > ether saddr 00:50:56:96:9b:1c ether daddr 00:0c:29:46:1f:53 ether type ip6
> >> 
> >> That's rule #11... Where is the hit on the queue rule and the return??
> > 
> > No idea, I will have a closer look next week.
> > Glancing at the code it should work just fine.
> 
> There might a event buffering issue. I have now sometimes seen the queueing
> trace. At other times the event is lost. So maybe the netlink buffer is not
> large enough?

How many events are there...?
If there aren't hundreds of events going on that really should not be an
issue.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: nftables queue target aborts rules processing unconditionally
  2017-03-03 16:01     ` Florian Westphal
@ 2017-03-03 16:24       ` Andreas Schultz
  0 siblings, 0 replies; 5+ messages in thread
From: Andreas Schultz @ 2017-03-03 16:24 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

----- On Mar 3, 2017, at 5:01 PM, Florian Westphal fw@strlen.de wrote:

> Andreas Schultz <aschultz@tpip.net> wrote:
>> ok, somewhat unexpected (or rather undocumented), but I can live
>> with that.
>> 
>> I've now experimented with NF_REPEAT to achieve something similar.
>> Can I assume that NF_REPEAT should restart the current "netfilter hook*?
> 
> Yes.
> 
>> e.g. when we are somewhere in FILTER FORWARD, it will restart with the
>> first rule of that hook?
> 
> It restarts the hook, yes.
> 
>> My experiments show that this works with nft when I don't modify the
>> ruleset. However, when I modify the ruleset before returning NF_REPEAT,
>> the packet will skip the current hook completely.
> 
> Hmm, that shouldn't happen.
> REPEAT should always just re-start the current hook.
> If that hook gets deleted (and possibly re-created) while packet was
> queued the kernel is supposed to drop the packet.

I intentionally created an endless loop with NF_REPEAT. Without the nft
modification it goes into the expected loop, but with the nft modification
it does not (see below for log).
 
>> I don't modify the chain the packet is currently traversing. I only add
>> new chains and modify the vmap.
> 
> The netfilter infrastructure is a layer below nftables/iptables so it
> is not even aware of rule set modifications.
> 
>> >> It also appears as if the nft trace infrastructure does not now how to
>> >> deal with queues. The above rules lead to this annotated trace output:
>> >> 
>> >> > trace id 10d53daf ip filter client_to_any packet: iif "upstream" oif "ens256"
>> >> > ether saddr 00:50:56:96:9b:1c ether daddr 00:0c:29:46:1f:53 ether type ip6
>> >> 
>> >> That's rule #11... Where is the hit on the queue rule and the return??
>> > 
>> > No idea, I will have a closer look next week.
>> > Glancing at the code it should work just fine.
>> 
>> There might a event buffering issue. I have now sometimes seen the queueing
>> trace. At other times the event is lost. So maybe the netlink buffer is not
>> large enough?
> 
> How many events are there...?
> If there aren't hundreds of events going on that really should not be an
> issue.

The trace is really just the 10 or so events. Whether the queue event
is in there or not seems to be purely random:

trace id ed0492b0 ip filter client_to_any packet: iif "upstream" oif "ens256" ether saddr 00:50:56:96:9b:1c ether daddr 00:0c:29:46:1f:53 ether type ip6 
trace id ed0492b0 ip filter client_to_any rule nftrace set 1 (verdict continue)
trace id ed0492b0 ip filter client_to_any rule counter packets 0 bytes 0 queue num 0 (verdict queue)
add chain ip filter CIn_1
add rule ip filter CIn_1 counter name "CIn_1_Session"
add rule ip filter CIn_1 ip daddr 172.20.16.0/24 counter packets 0 bytes 0 accept
add rule ip filter CIn_1 ip daddr 8.0.0.0/8 counter packets 0 bytes 0 accept
add chain ip filter COut_1
add rule ip filter COut_1 counter name "COut_1_Session"
add rule ip filter COut_1 ip saddr 172.20.16.0/24 counter packets 0 bytes 0 accept
add rule ip filter COut_1 ip saddr 8.0.0.0/8 counter packets 0 bytes 0 accept
trace id ed0492b0 ip mangle POSTROUTING verdict continue 
trace id ed0492b0 ip mangle POSTROUTING 

Regards
Andreas

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-03-03 16:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-03 15:01 nftables queue target aborts rules processing unconditionally Andreas Schultz
2017-03-03 15:41 ` Florian Westphal
2017-03-03 15:57   ` Andreas Schultz
2017-03-03 16:01     ` Florian Westphal
2017-03-03 16:24       ` Andreas Schultz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.