[PATCH nft 0/3] parser: allow to define maps that contain ct objects

[PATCH nft 0/3] parser: allow to define maps that contain ct objects

[Florian Westphal]

This allows to store ct object references in maps.
This is a userspace only change.

The only kernel patch required to make the test case work on nf.git
is netfilter: nft_ct: fix l3num expectations with inet pseudo family,
else ct expectations in inet family get rejected.

Note that the second patch is the opposite of what we agreed on at
NFWS, this is also why its both separate and has a very large
wall-of-text commit message.

I did not find a way to solve this without kernel patches in a sane
way.

I could still make it work with the new postfix keywords mentioned
in patch 2 commit message, but it would need a lot of guesswork and
special casing to figure out when to add the "right" postfix keyword.

Please see patch 2 for details and rationale wrt. chosen solution.

Florian Westphal (3):
  parser: allow to define maps that contain timeouts and expectations
  parser: allow to define maps that contain ct helpers
  tests: add test case for named ct objects

 src/parser_bison.y                            | 25 ++++-
 .../testcases/maps/dumps/named_ct_objects.nft | 71 ++++++++++++++
 tests/shell/testcases/maps/named_ct_objects   | 94 +++++++++++++++++++
 3 files changed, 185 insertions(+), 5 deletions(-)
 create mode 100644 tests/shell/testcases/maps/dumps/named_ct_objects.nft
 create mode 100755 tests/shell/testcases/maps/named_ct_objects

-- 
2.43.0

[PATCH nft 1/3] parser: allow to define maps that contain timeouts and expectations

[Florian Westphal]

Its currently not possible to use ct timeouts/expectations/helpers
in objref maps, bison parser lacks the relevant keywords.

This change adds support for timeouts and expectations.
Ct helpers are more problematic, this will come in a different change.

Support is only added for the "typeof" keyword, otherwise we'd
need to add pseudo-datatypes as well, but making "ct expectation"
available as "type" as well might be confusing.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_bison.y | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index cd1dc658882d..05861c3e2f75 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -751,7 +751,7 @@ int nft_lex(void *, void *, void *);
 
 %type <set>			map_block_alloc map_block
 %destructor { set_free($$); }	map_block_alloc
-%type <val>			map_block_obj_type map_block_data_interval
+%type <val>			map_block_obj_type map_block_obj_typeof map_block_data_interval
 
 %type <flowtable>		flowtable_block_alloc flowtable_block
 %destructor { flowtable_free($$); }	flowtable_block_alloc
@@ -988,7 +988,7 @@ int nft_lex(void *, void *, void *);
 %destructor { expr_free($$); }	exthdr_exists_expr
 %type <val>			exthdr_key
 
-%type <val>			ct_l4protoname ct_obj_type ct_cmd_type
+%type <val>			ct_l4protoname ct_obj_type ct_cmd_type ct_obj_type_map
 
 %type <timeout_state>		timeout_state
 %destructor { timeout_state_free($$); }		timeout_state
@@ -2269,6 +2269,10 @@ map_block_alloc		:	/* empty */
 			}
 			;
 
+ct_obj_type_map		: 	TIMEOUT		{ $$ = NFT_OBJECT_CT_TIMEOUT; }
+			|	EXPECTATION	{ $$ = NFT_OBJECT_CT_EXPECT; }
+			;
+
 map_block_obj_type	:	COUNTER	close_scope_counter { $$ = NFT_OBJECT_COUNTER; }
 			|	QUOTA	close_scope_quota { $$ = NFT_OBJECT_QUOTA; }
 			|	LIMIT	close_scope_limit { $$ = NFT_OBJECT_LIMIT; }
@@ -2276,6 +2280,10 @@ map_block_obj_type	:	COUNTER	close_scope_counter { $$ = NFT_OBJECT_COUNTER; }
 			|	SYNPROXY close_scope_synproxy { $$ = NFT_OBJECT_SYNPROXY; }
 			;
 
+map_block_obj_typeof	:	map_block_obj_type
+			|	CT	ct_obj_type_map	close_scope_ct	{ $$ = $2; }
+			;
+
 map_block_data_interval :	INTERVAL { $$ = EXPR_F_INTERVAL; }
 			|	{ $$ = 0; }
 			;
@@ -2341,7 +2349,7 @@ map_block		:	/* empty */	{ $$ = $<set>-1; }
 				$$ = $1;
 			}
 			|	map_block	TYPEOF
-						typeof_expr 	COLON	map_block_obj_type
+						typeof_expr 	COLON	map_block_obj_typeof
 						stmt_separator
 			{
 				$1->key = $3;
-- 
2.43.0

[PATCH nft 2/3] parser: allow to define maps that contain ct helpers

[Florian Westphal]

Its currently not possible to use ct helpers in objref maps.

Simply adding "CT HELPER" to "map_block_obj_type" does not work
due to a conflict with the "ct helper" ct_expr block.

  map m {
    type ipv4_addr : ct helper ..

... declares a map storing ip addresses and conntrack helper names
(string type).  This does not make sense, there is no way
to use the associated value (the names) in any sensible way:

ct helper "ftp" - this matches if the packet has a conntrack entry that
was accepted via the "ftp" conntrack helper. In nft vm terms, this is
translated to:

 [ ct load helper => reg 1 ]
 [ cmp eq reg 1 0x00707466 0x00000000 0x00000000 0x00000000 ]

Or one can query a set, e.g. 'ct helper { "ftp", "sip" }'.
"ftp" and "sip" are the kernel-defined names of these connection
tracking helpers.

ct helper set "ftp" is something else, however:

This is used to assign a *userspace defined helper objrect reference*.
Nftables will translate this to:

 [ objref type 3 name ftp ]

.. where "ftp" is a arbitrary user-chosen name.

  ct helper "ftp" {
    type "ftp" protocol tcp
    l3proto ip
  }

IOW, "ct helper" is ambiguous.  Without the "set" keyword (first case),
it places the kernel-defined name of the active connection tracking helper
in the chosen register (or it will cancel rule evaluation if no helper was
active).

With the set keyword (second case), the expected argument is a user-defined
object reference which will then tell the connection tracking engine to
monitor all further packets of the new flow with the given helper template.

This change makes it so that

  map m {
    type ipv4_addr : ct helper ..

declares a map storing ct helper object references suitable for
'ct helper set'.

The better alternative would be to resolve the ambiguity
by adding an additional postfix keyword, for example

 ct helper name (case one)
 ct helper object (case two).

But this needs a kernel change that adds
NFT_CT_HELPER_NAME and NFT_CT_HELPER_OBJECT to enum nft_ct_keys.

While a new kernel could handle old nftables binaries that still use
NFT_CT_HELPER key, new nftables would need to probe support first.

Furthermore,

 ct helper name set "foo"

... would make no sense, as the kernel-defined helper names are
readonly.

 ct helper object "foo"

... would make no sense, unless we extend the kernel to store
the nftables userspace-defined name in a well-known location
in the kernel.  Userdata area cannot work for this, because the
nft conntrack expression in the kernel would need to know how to
retrieve this info again.

Also, I cannot think of a sensible use case for this.
So the only remaining, useful commands are:

ct helper name "ftp"
ct helper object set "foo"

... which is identical to what we already support, just with
extra keyword.

So a much simpler solution that does not need any kernel changes
is make "ct helper" have different meaning depending on wheter it
is placed on the key side, i.e.:

    "typeof ct helper", "typeof ct helper : $value"

versus when its on placed on the data (value) side of maps:

    "typeof $key : ct helper".

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_bison.y | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 05861c3e2f75..bdb73911759c 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2329,9 +2329,16 @@ map_block		:	/* empty */	{ $$ = $<set>-1; }
 				}
 
 				$1->key = $3;
-				$1->data = $5;
 
-				$1->flags |= NFT_SET_MAP;
+				if ($5->etype == EXPR_CT && $5->ct.key == NFT_CT_HELPER) {
+					$1->objtype = NFT_OBJECT_CT_HELPER;
+					$1->flags  |= NFT_SET_OBJECT;
+					expr_free($5);
+				} else {
+					$1->data = $5;
+					$1->flags |= NFT_SET_MAP;
+				}
+
 				$$ = $1;
 			}
 			|	map_block	TYPE
-- 
2.43.0

[PATCH nft 3/3] tests: add test case for named ct objects

[Florian Westphal]

Add a dedicated test for named conntrack objects:
timeouts, helpers and expectations.

A json dump file is not added because the json input
code does not support "typeof" declarations for sets/maps.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 .../testcases/maps/dumps/named_ct_objects.nft | 71 ++++++++++++++
 tests/shell/testcases/maps/named_ct_objects   | 94 +++++++++++++++++++
 2 files changed, 165 insertions(+)
 create mode 100644 tests/shell/testcases/maps/dumps/named_ct_objects.nft
 create mode 100755 tests/shell/testcases/maps/named_ct_objects

diff --git a/tests/shell/testcases/maps/dumps/named_ct_objects.nft b/tests/shell/testcases/maps/dumps/named_ct_objects.nft
new file mode 100644
index 000000000000..59f18932b28a
--- /dev/null
+++ b/tests/shell/testcases/maps/dumps/named_ct_objects.nft
@@ -0,0 +1,71 @@
+table inet t {
+	ct expectation exp1 {
+		protocol tcp
+		dport 9876
+		timeout 1m
+		size 12
+		l3proto ip
+	}
+
+	ct expectation exp2 {
+		protocol tcp
+		dport 9876
+		timeout 3s
+		size 13
+		l3proto ip6
+	}
+
+	ct helper myftp {
+		type "ftp" protocol tcp
+		l3proto inet
+	}
+
+	ct timeout dns {
+		protocol tcp
+		l3proto ip
+		policy = { established : 3s, close : 1s }
+	}
+
+	map exp {
+		typeof ip saddr : ct expectation
+		elements = { 192.168.2.2 : "exp1" }
+	}
+
+	map exp6 {
+		typeof ip6 saddr : ct expectation
+		flags interval
+		elements = { dead:beef::/64 : "exp2" }
+	}
+
+	map helpobj {
+		typeof ip6 saddr : ct helper
+		flags interval
+		elements = { dead:beef::/64 : "myftp" }
+	}
+
+	map timeoutmap {
+		typeof ip daddr : ct timeout
+		elements = { 192.168.0.1 : "dns" }
+	}
+
+	set helpname {
+		typeof ct helper
+		elements = { "sip",
+			     "ftp" }
+	}
+
+	chain y {
+		ct expectation set ip saddr map @exp
+		ct expectation set ip6 saddr map { dead::beef : "exp2" }
+		ct expectation set ip6 daddr map { dead::beef : "exp2", feed::17 : "exp2" }
+		ct expectation set ip6 daddr . tcp dport map { feed::17 . 512 : "exp2", dead::beef . 123 : "exp2" }
+		ct helper set ip6 saddr map { 1c3::c01d : "myftp", dead::beef : "myftp" }
+		ct helper set ip6 saddr map @helpobj
+		ct timeout set ip daddr map @timeoutmap
+		ct timeout set ip daddr map { 1.2.3.4 : "dns", 5.6.7.8 : "dns", 192.168.8.0/24 : "dns" }
+		ct timeout set ip daddr map { 1.2.3.4-1.2.3.8 : "dns" }
+		ct timeout set ip6 daddr map { 1ce::/64 : "dns", dead::beef : "dns" }
+		ct helper @helpname accept
+		ip saddr 192.168.1.1 ct timeout set "dns"
+	}
+}
diff --git a/tests/shell/testcases/maps/named_ct_objects b/tests/shell/testcases/maps/named_ct_objects
new file mode 100755
index 000000000000..61b87c1ab14a
--- /dev/null
+++ b/tests/shell/testcases/maps/named_ct_objects
@@ -0,0 +1,94 @@
+#!/bin/bash
+
+# NFT_TEST_REQUIRES(NFT_TEST_HAVE_cttimeout)
+
+$NFT -f /dev/stdin <<EOF || exit 1
+table inet t {
+	ct expectation exp1 {
+		protocol tcp
+		dport 9876
+		timeout 1m
+		size 12
+		l3proto ip
+	}
+
+	ct expectation exp2 {
+		protocol tcp
+		dport 9876
+		timeout 3s
+		size 13
+		l3proto ip6
+	}
+
+	ct helper myftp {
+		type "ftp" protocol tcp
+	}
+
+	ct timeout dns {
+		protocol tcp
+		l3proto ip
+		policy = { established : 3, close : 1 }
+	}
+
+	map exp {
+		typeof ip saddr : ct expectation
+		elements = { 192.168.2.2 : "exp1" }
+	}
+
+	map exp6 {
+		typeof ip6 saddr : ct expectation
+		flags interval
+		elements = { dead:beef::/64 : "exp2" }
+	}
+
+	map helpobj {
+		typeof ip6 saddr : ct helper
+		flags interval
+		elements = { dead:beef::/64 : "myftp" }
+	}
+
+	map timeoutmap {
+		typeof ip daddr : ct timeout
+		elements = { 192.168.0.1 : "dns" }
+	}
+
+	set helpname {
+		typeof ct helper
+		elements = { "ftp", "sip" }
+	}
+
+	chain y {
+		ct expectation set ip saddr map @exp
+		ct expectation set ip6 saddr map { dead::beef : "exp2" }
+		ct expectation set ip6 daddr map { dead::beef : "exp2", feed::17 : "exp2" }
+		ct expectation set ip6 daddr . tcp dport map { dead::beef . 123 : "exp2", feed::17 . 512 : "exp2" }
+		ct helper set ip6 saddr map { dead::beef : "myftp", 1c3::c01d : "myftp" }
+		ct helper set ip6 saddr map @helpobj
+		ct timeout set ip daddr map @timeoutmap
+		ct timeout set ip daddr map { 1.2.3.4 : "dns", 5.6.7.8 : "dns", 192.168.8.0/24 : "dns" }
+		ct timeout set ip daddr map { 1.2.3.4-1.2.3.8 : "dns" }
+		ct timeout set ip6 daddr map { dead::beef : "dns", 1ce::/64 : "dns" }
+		ct helper @helpname accept
+	}
+}
+EOF
+
+must_fail()
+{
+	echo "Command should have failed: $1"
+	exit 111
+}
+
+
+must_work()
+{
+	echo "Command should have succeeded: $1"
+	exit 111
+}
+
+$NFT 'add rule inet t y ip saddr 192.168.1.1 ct timeout set "dns"' || must_work "dns timeout"
+
+$NFT 'add rule inet t y ct helper set ip saddr map @helpobj' && must_fail "helper assignment, map key is ipv6_addr"
+$NFT 'add rule inet t y ct helper set ip6 saddr map @helpname' && must_fail "helper assignment, not a map with objects"
+
+exit 0
-- 
2.43.0