[PATCH] selftests: nft_queue: conntrack expiration requeue

[PATCH] selftests: nft_queue: conntrack expiration requeue

[Antonio Ojea]

The nfqueue mechanism allows userspace daemons to implement complex,
dynamic filtering rules. This is particularly useful in distributed
platforms like Kubernetes, where security policies may be too numerous
or change too frequently (in the order of seconds) to be implemented
efficiently in the dataplane.

To avoid the performance penalty of crossing between kernel and
userspace for every packet, a common optimization is to use stateful
nftables rules (e.g., ct state established,related accept) to bypass the
queue for packets belonging to known flows.

However, if there is the need to reevaluate the established connections
using the existing rules, we should have a way to stop tracking the
connections so they are sent back to the queue for reevaluation.

Simply flushing the conntrack entries does not work for TCP if tcp_loose
is enabled, since the conntrack stack will recover the connection
state. Setting the conntrack entry timeout to 0 allows to remove the state
and the packet is sent to the queue.

This tests validates this scenario, it establish a TCP connection,
confirms that established packets bypass the queue, and that after
setting the conntrack entry timeout to 0 subsequent packets are
requeued.

Signed-off-by: Antonio Ojea <aojea@google.com>
---
 .../selftests/net/netfilter/nft_queue.sh      | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/tools/testing/selftests/net/netfilter/nft_queue.sh b/tools/testing/selftests/net/netfilter/nft_queue.sh
index 6136ceec45e0..b25f0e23ce3d 100755
--- a/tools/testing/selftests/net/netfilter/nft_queue.sh
+++ b/tools/testing/selftests/net/netfilter/nft_queue.sh
@@ -28,8 +28,10 @@ cleanup()
 
 checktool "nft --version" "test without nft tool"
 checktool "socat -h" "run test without socat"
+checktool "conntrack -V" "run test without conntrack tool"
 
 modprobe -q sctp
+modprobe -q nf_conntrack
 
 trap cleanup EXIT
 
@@ -353,6 +355,78 @@ EOF
 	echo "PASS: tcp via loopback and re-queueing"
 }
 
+test_tcp_conntrack_timeout_requeue()
+{
+	# Set up initial nftables ruleset.
+	ip netns exec "$nsrouter" nft -f /dev/stdin <<EOF
+flush ruleset
+table inet filter {
+	chain post {
+		type filter hook postrouting priority 0; policy accept;
+		ct state established,related counter accept
+		tcp dport 12345 counter queue num 10
+	}
+}
+EOF
+	if [ $? -ne 0 ]; then
+		echo "FAIL: Failed to apply initial nftables rules."
+		exit 1
+	fi
+
+	# Start server in the background
+	ip netns exec "$ns2" socat -u TCP4-LISTEN:12345,fork STDOUT > "$TMPFILE1" 2>/dev/null &
+	local server_pid=$!
+	busywait "$BUSYWAIT_TIMEOUT" listener_ready "$ns2" "12345"
+
+	# Start nf_queue listener in ACCEPT mode
+	ip netns exec "$nsrouter" ./nf_queue -q 10 -c > "$TMPFILE2" &
+	local nfq_accept_pid=$!
+	busywait "$BUSYWAIT_TIMEOUT" nf_queue_wait "$nsrouter" 10
+
+	# Establish the connection and send the first message
+	tail -f "$TMPFILE0" | ip netns exec "$ns1" socat STDIO TCP:10.0.2.99:12345 &
+	echo "message1" >> "$TMPFILE0"
+	if ! busywait "$BUSYWAIT_TIMEOUT" grep -q "message1" "$TMPFILE1"; then
+		echo "FAIL: Did not receive first message."
+		ret=1
+		return
+	fi
+
+	# Switch nfqueue listener to DROP mode
+	kill "$nfq_accept_pid"; wait "$nfq_accept_pid" 2>/dev/null
+	ip netns exec "$nsrouter" ./nf_queue -q 10 -c -Q 0 > "$TMPFILE3" &
+	local nfq_drop_pid=$!
+	busywait "$BUSYWAIT_TIMEOUT" nf_queue_wait "$nsrouter" 10
+
+	# Send another message; it should be accepted by the 'ct state established' rule
+	echo "message2" >> "$TMPFILE0"
+	if ! busywait "$BUSYWAIT_TIMEOUT" grep -q "message2" "$TMPFILE1"; then
+		echo "FAIL: Did not receive second message."
+		ret=1
+		return
+	fi
+
+	# Set conntrack timeout to 0 to force re-evaluation
+	ip netns exec "$nsrouter" conntrack -U -p tcp --dport 12345 -d 10.0.2.99 -s 10.0.1.99 -t 0
+
+	# Send a final message. It should be queued and then dropped.
+	echo "message3" >> "$TMPFILE0"
+	if busywait "$BUSYWAIT_TIMEOUT" grep -q "message3" "$TMPFILE1"; then
+		echo "FAIL: Third message was received, but should have been dropped."
+		ret=1
+		return
+	fi
+
+	kill "$server_pid"
+	wait "$server_pid" 2>/dev/null
+	kill "$nfq_drop_pid"
+	wait "$nfq_drop_pid" 2>/dev/null
+
+	echo "PASS: tcp established re-queueing on conntrack timeout"
+
+	return 0
+}
+
 test_icmp_vrf() {
 	if ! ip -net "$ns1" link add tvrf type vrf table 9876;then
 		echo "SKIP: Could not add vrf device"
@@ -661,6 +735,7 @@ test_tcp_forward
 test_tcp_localhost
 test_tcp_localhost_connectclose
 test_tcp_localhost_requeue
+test_tcp_conntrack_timeout_requeue
 test_sctp_forward
 test_sctp_output
 test_udp_ct_race
-- 
2.51.0.869.ge66316f041-goog

Re: [PATCH] selftests: nft_queue: conntrack expiration requeue

[Florian Westphal]

Antonio Ojea <aojea@google.com> wrote:
> The nfqueue mechanism allows userspace daemons to implement complex,
> dynamic filtering rules. This is particularly useful in distributed
> platforms like Kubernetes, where security policies may be too numerous
> or change too frequently (in the order of seconds) to be implemented
> efficiently in the dataplane.
> 
> To avoid the performance penalty of crossing between kernel and
> userspace for every packet, a common optimization is to use stateful
> nftables rules (e.g., ct state established,related accept) to bypass the
> queue for packets belonging to known flows.
> 
> However, if there is the need to reevaluate the established connections
> using the existing rules, we should have a way to stop tracking the
> connections so they are sent back to the queue for reevaluation.
> 
> Simply flushing the conntrack entries does not work for TCP if tcp_loose
> is enabled, since the conntrack stack will recover the connection

you mean disabled?
loose tracking (midstream pickup) is on by default.

> state. Setting the conntrack entry timeout to 0 allows to remove the state
> and the packet is sent to the queue.

But its the same as --delete, entry gets tossed (its timed out) and is
re-created from scratch.

> This tests validates this scenario, it establish a TCP connection,
> confirms that established packets bypass the queue, and that after
> setting the conntrack entry timeout to 0 subsequent packets are
> requeued.

That zaps the entry and re-creates it, all state is lost.
Wouldn't it make more sense to bypass based on connmark or ctlabels?

I'm not sure what the test is supposed to assert.

That setting timeout via ctnetlink to 0 kicks the entry out of the ct hash?

Re: [PATCH] selftests: nft_queue: conntrack expiration requeue

[Antonio Ojea]

> >
> > Simply flushing the conntrack entries does not work for TCP if tcp_loose
> > is enabled, since the conntrack stack will recover the connection
>
> you mean disabled?
> loose tracking (midstream pickup) is on by default.

yeah, sorry, as you say, I meant disabled

> > state. Setting the conntrack entry timeout to 0 allows to remove the state
> > and the packet is sent to the queue.
>
> But its the same as --delete, entry gets tossed (its timed out) and is
> re-created from scratch.
>

The behavior is not the same as deleting the entry, I tried both and
only works by setting the timeout, I tried to follow the codepath but
I'm very unfamiliar with the codebase to understand why delete is
different from updating with timeout 0.

> > This tests validates this scenario, it establish a TCP connection,
> > confirms that established packets bypass the queue, and that after
> > setting the conntrack entry timeout to 0 subsequent packets are
> > requeued.
>
> That zaps the entry and re-creates it, all state is lost.
> Wouldn't it make more sense to bypass based on connmark or ctlabels?

This simplifies the datapath considerably and avoids the risk of
having to coordinate marks with other components, but there is also
some ignorance on my side on how to use all netfilter features
efficiently.

The use case I have is to only process the first packet of each
connection in user space BUT also be able to scan the conntrack table
to match some connections that despite being established I want to
reevaluate only once, so I have something like:

ct state established,related accept
queue flags bypass to 98

And then I scan the conntrack table, and for each flow I need to
reevaluate once , I just reset the timeout and it ignores the rule "ct
state established,related accept" , then if the verdict is accepted it
keeps skipping the queue.

However, if there is a more elegant way that does not depend on this
"hack" please let me know
Can I apply a connmark or ctlabel via netlink in an existing flow?
If so, how to make it so it only is enqueued once, do I need to mark
and unmark the flow?

> I'm not sure what the test is supposed to assert.
>
> That setting timeout via ctnetlink to 0 kicks the entry out of the ct hash?

The behavior I'd like to assert is if this behavior is just some side
effect I found or something it will be "stable" , since I'm trying to
build the firewall on this behavior and if changes it will be very
disruptive

Re: [PATCH] selftests: nft_queue: conntrack expiration requeue

[Florian Westphal]

Antonio Ojea <aojea@google.com> wrote:
> yeah, sorry, as you say, I meant disabled
> 
> > > state. Setting the conntrack entry timeout to 0 allows to remove the state
> > > and the packet is sent to the queue.
> >
> > But its the same as --delete, entry gets tossed (its timed out) and is
> > re-created from scratch.
> >
> 
> The behavior is not the same as deleting the entry, I tried both and
> only works by setting the timeout, I tried to follow the codepath but
> I'm very unfamiliar with the codebase to understand why delete is
> different from updating with timeout 0.

It should be the same.

Only difference:

ctnetlink delete zaps entry right away, timeout-0 zaps it on the next
(matching) packet or when gc finds the entry (or on next conntrack -L),
whatever comes first.

> > That zaps the entry and re-creates it, all state is lost.
> > Wouldn't it make more sense to bypass based on connmark or ctlabels?
> 
> This simplifies the datapath considerably and avoids the risk of
> having to coordinate marks with other components, but there is also
> some ignorance on my side on how to use all netfilter features
> efficiently.
> 
> The use case I have is to only process the first packet of each
> connection in user space BUT also be able to scan the conntrack table
> to match some connections that despite being established I want to
> reevaluate only once, so I have something like:

> ct state established,related accept
> queue flags bypass to 98

Understood.

> And then I scan the conntrack table, and for each flow I need to
> reevaluate once , I just reset the timeout and it ignores the rule "ct
> state established,related accept" , then if the verdict is accepted it
> keeps skipping the queue.

Yes, but thats because the packet that hits the timed-out flow zaps it
and is 'new' (for tcp_loose=1).

> However, if there is a more elegant way that does not depend on this
> "hack" please let me know
> Can I apply a connmark or ctlabel via netlink in an existing flow?

Yes, bypassing nfqueue was the original use-case that prompted the
connlabel feature.

> If so, how to make it so it only is enqueued once, do I need to mark
> and unmark the flow?

There are several ways to do what you want.

You can add a 'requeue' named set:
   ct id @myset queue ...

and then add/remove from that set to (re)enable via userspace.

Or you can use opt-in (or out, its simpler) via mark or label:
  ct state new ct label set shouldqueue ...
  ct label shouldqueue queue ...

or, without need to set initial state:
  ct label ! "wasqueued" queue ...

... and then use ctnetlink to set the label (or clear it).

Or, set/clear the label from the nfqueue program itself when setting accept
verdict (via nested NFQA_CT + CTA_MARK/LABELS).

Same with connmark, but I know that this might be impossible
due to those being used for policy routing etc.

> > I'm not sure what the test is supposed to assert.
> >
> > That setting timeout via ctnetlink to 0 kicks the entry out of the ct hash?
> 
> The behavior I'd like to assert is if this behavior is just some side
> effect I found or something it will be "stable" , since I'm trying to
> build the firewall on this behavior and if changes it will be very
> disruptive

Setting the timeout to 0 zaps the entry at earliest opportunity,
it will not be "reactivated" on next packet, its just
conntrack --delete with minor random delay.

Re: [PATCH] selftests: nft_queue: conntrack expiration requeue

[Antonio Ojea]

>
> Or, set/clear the label from the nfqueue program itself when setting accept
> verdict (via nested NFQA_CT + CTA_MARK/LABELS).
>
> Same with connmark, but I know that this might be impossible
> due to those being used for policy routing etc.
>

This works pretty good, and is really neat

ct mark 0x66000000 ct state established,related accept
queue flags bypass to 98

So I just need to set the mark/label on the verdict and clear the mark
or the label via netlink and it gets requeued.

I can make it work with connmark very easily but labels seem a better
fit because they allow me to set different values and avoid
compatibility issues.
However, I'm not able to make this work with labels, I do not know if
it is a problem on my side, I'm not able to use the userspace
conntrack tool too.
I think I'm setting the label correctly but the output of conntrack -L
or conntrack -L -o labels do not show anything.
If I try to set the label manually it also fails with ENOSPC

conntrack -U -d 10.244.2.2 --label-add net
conntrack v1.4.8 (conntrack-tools): Operation failed: No space left on device

Re: [PATCH] selftests: nft_queue: conntrack expiration requeue

[Florian Westphal]

Antonio Ojea <aojea@google.com> wrote:
> ct mark 0x66000000 ct state established,related accept
> queue flags bypass to 98
> 
> So I just need to set the mark/label on the verdict and clear the mark
> or the label via netlink and it gets requeued.
> 
> I can make it work with connmark very easily but labels seem a better
> fit because they allow me to set different values and avoid
> compatibility issues.
> However, I'm not able to make this work with labels, I do not know if
> it is a problem on my side, I'm not able to use the userspace
> conntrack tool too.
> I think I'm setting the label correctly but the output of conntrack -L
> or conntrack -L -o labels do not show anything.
> If I try to set the label manually it also fails with ENOSPC
> 
> conntrack -U -d 10.244.2.2 --label-add net
> conntrack v1.4.8 (conntrack-tools): Operation failed: No space left on device

No space for the label extension.

Does it start to work for new flows when you add a rule like
'ct label foo'?

Re: [PATCH] selftests: nft_queue: conntrack expiration requeue

[Antonio Ojea]

> > I think I'm setting the label correctly but the output of conntrack -L
> > or conntrack -L -o labels do not show anything.
> > If I try to set the label manually it also fails with ENOSPC
> >
> > conntrack -U -d 10.244.2.2 --label-add net
> > conntrack v1.4.8 (conntrack-tools): Operation failed: No space left on device
>
> No space for the label extension.
>
> Does it start to work for new flows when you add a rule like
> 'ct label foo'?

It does work, but it still shows the error

conntrack -U -d 10.244.2.2 --label-add test
tcp      6 86382 ESTABLISHED src=10.244.1.8 dst=10.244.2.2 sport=39133
dport=8080 src=10.244.2.2 dst=10.244.1.8 sport=8080 dport=39133
[ASSURED] mark=0 use=2 labels=test,net
conntrack v1.4.8 (conntrack-tools): Operation failed: No space left on device

the dump of the message that fails using strace is

sendto(4, [{nlmsg_len=80,
nlmsg_type=NFNL_SUBSYS_CTNETLINK<<8|IPCTNL_MSG_CT_NEW,
nlmsg_flags=NLM_F_REQUEST|NLM_F_ACK, nlmsg_seq=1761045207,
nlmsg_pid=0}, {nfgen_family=AF_INET, version=NFNETLINK_V0,
res_id=htons(0)}, [[{nla_len=52, nla_type=NLA_F_NESTED|0x1},
"\x14\x00\x01\x80\x08\x00\x01\x00\x0a\xf4\x01\x08\x08\x00\x02\x00\x0a\xf4\x02\x02\x1c\x00\x02\x80\x05\x00\x01\x00\x06\x00\x00\x00"...],
[{nla_len=8, nla_type=0x16}, "\x02\x00\x00\x00"]]], 80, 0,
{sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, 12) = 80
recvmsg(4, {msg_name={sa_family=AF_NETLINK, nl_pid=0,
nl_groups=00000000}, msg_namelen=12,
msg_iov=[{iov_base=[{nlmsg_len=100, nlmsg_type=NLMSG_ERROR,
nlmsg_flags=0, nlmsg_seq=1761045207, nlmsg_pid=-1573183269},
{error=-ENOSPC, msg=[{nlmsg_len=80,
nlmsg_type=NFNL_SUBSYS_CTNETLINK<<8|IPCTNL_MSG_CT_NEW,
nlmsg_flags=NLM_F_REQUEST|NLM_F_ACK, nlmsg_seq=1761045207,
nlmsg_pid=0}, {nfgen_family=AF_INET, version=NFNETLINK_V0,
res_id=htons(0)}, [[{nla_len=52, nla_type=NLA_F_NESTED|0x1},
"\x14\x00\x01\x80\x08\x00\x01\x00\x0a\xf4\x01\x08\x08\x00\x02\x00\x0a\xf4\x02\x02\x1c\x00\x02\x80\x05\x00\x01\x00\x06\x00\x00\x00"...],
[{nla_len=8, nla_type=0x16}, "\x02\x00\x00\x00"]]]}], iov_len=4096}],
msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 100

Re: [PATCH] selftests: nft_queue: conntrack expiration requeue

[Florian Westphal]

Antonio Ojea <aojea@google.com> wrote:
> > Does it start to work for new flows when you add a rule like
> > 'ct label foo'?
> 
> It does work, but it still shows the error
> 
> conntrack -U -d 10.244.2.2 --label-add test
> tcp      6 86382 ESTABLISHED src=10.244.1.8 dst=10.244.2.2 sport=39133
> dport=8080 src=10.244.2.2 dst=10.244.1.8 sport=8080 dport=39133
> [ASSURED] mark=0 use=2 labels=test,net
> conntrack v1.4.8 (conntrack-tools): Operation failed: No space left on device

Looks like one needs to set a label somewhere, no need for it to match.

chain never { ct label set foo }

makes this work for me.
We could change this so that *checking* a label also turns on the
extension infra.

Back then i did not want to allocate the extra space for
the extensions and i did not want to add to a new sysctl either.

So I went with 'no rules that adds one, no need for ct label
extension space allocation'.

Re: [PATCH] selftests: nft_queue: conntrack expiration requeue

[Antonio Ojea]

> Looks like one needs to set a label somewhere, no need for it to match.
>
> chain never { ct label set foo }
>
> makes this work for me.
> We could change this so that *checking* a label also turns on the
> extension infra.
>
> Back then i did not want to allocate the extra space for
> the extensions and i did not want to add to a new sysctl either.
>
> So I went with 'no rules that adds one, no need for ct label
> extension space allocation'.

But that does not consider the people that just use netlink to set the
labels ... from a 1k altitude , can you do a check on the first
update/create/delete label to initialize the extension?


Another question related, is it required for the label value to be
always 16 bytes?
I do not know if is the golang libraries I'm using but it seems it
does not work with other lengths

Re: [PATCH] selftests: nft_queue: conntrack expiration requeue

[Florian Westphal]

Antonio Ojea <aojea@google.com> wrote:
> > So I went with 'no rules that adds one, no need for ct label
> > extension space allocation'.
> 
> But that does not consider the people that just use netlink to set the
> labels ... from a 1k altitude , can you do a check on the first
> update/create/delete label to initialize the extension?

Yes, but in that case it is not possible to disable it again.

Rule based on/off means we can disable it once the rule is gone.

I'd propose to extend the label allocation to when a 'ct lablel'
rule is added rather than just 'ct label set'.

> Another question related, is it required for the label value to be
> always 16 bytes?

As far as I can see ctnetlink enforces the len must be a multiple of 4
and at most 16 bytes.

Re: [PATCH] selftests: nft_queue: conntrack expiration requeue

[Antonio Ojea]

>
> I'd propose to extend the label allocation to when a 'ct lablel'
> rule is added rather than just 'ct label set'.
>

+1 , thanks, selfishly, that will save me the need to add the "unused"
ct label set rule, since I only set via netlink.