netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions
@ 2020-03-07  2:26 Phil Sutter
  2020-03-07  2:26 ` [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges Phil Sutter
  2020-03-09 13:32 ` [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Eric Garver
  0 siblings, 2 replies; 4+ messages in thread
From: Phil Sutter @ 2020-03-07  2:26 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Eric Garver, netfilter-devel

Duplicate commit 8ac2f3b2fca38's changes to bison parser into JSON
parser by introducing a new context flag signalling we're parsing
concatenated expressions.

Fixes: 8ac2f3b2fca38 ("src: Add support for concatenated set ranges")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Fix accidental reject of concat expressions on LHS.
---
 src/parser_json.c | 47 +++++++++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/src/parser_json.c b/src/parser_json.c
index 85082ccee7ef6..480b15aeb2507 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -40,6 +40,7 @@
 #define CTX_F_MANGLE	(1 << 5)
 #define CTX_F_SES	(1 << 6)	/* set_elem_expr_stmt */
 #define CTX_F_MAP	(1 << 7)	/* LHS of map_expr */
+#define CTX_F_CONCAT	(1 << 8)	/* inside concat_expr */
 
 struct json_ctx {
 	struct input_descriptor indesc;
@@ -99,6 +100,7 @@ static struct expr *json_parse_primary_expr(struct json_ctx *ctx, json_t *root);
 static struct expr *json_parse_set_rhs_expr(struct json_ctx *ctx, json_t *root);
 static struct expr *json_parse_set_elem_expr_stmt(struct json_ctx *ctx, json_t *root);
 static struct expr *json_parse_map_lhs_expr(struct json_ctx *ctx, json_t *root);
+static struct expr *json_parse_concat_elem_expr(struct json_ctx *ctx, json_t *root);
 static struct stmt *json_parse_stmt(struct json_ctx *ctx, json_t *root);
 
 /* parsing helpers */
@@ -1058,7 +1060,7 @@ static struct expr *json_parse_concat_expr(struct json_ctx *ctx,
 	}
 
 	json_array_foreach(root, index, value) {
-		tmp = json_parse_primary_expr(ctx, value);
+		tmp = json_parse_concat_elem_expr(ctx, value);
 		if (!tmp) {
 			json_error(ctx, "Parsing expr at index %zd failed.", index);
 			expr_free(expr);
@@ -1354,28 +1356,28 @@ static struct expr *json_parse_expr(struct json_ctx *ctx, json_t *root)
 		{ "set", json_parse_set_expr, CTX_F_RHS | CTX_F_STMT }, /* allow this as stmt expr because that allows set references */
 		{ "map", json_parse_map_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS },
 		/* below three are multiton_rhs_expr */
-		{ "prefix", json_parse_prefix_expr, CTX_F_RHS | CTX_F_STMT },
-		{ "range", json_parse_range_expr, CTX_F_RHS | CTX_F_STMT },
-		{ "payload", json_parse_payload_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP },
-		{ "exthdr", json_parse_exthdr_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
+		{ "prefix", json_parse_prefix_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_CONCAT },
+		{ "range", json_parse_range_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_CONCAT },
+		{ "payload", json_parse_payload_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "exthdr", json_parse_exthdr_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
 		{ "tcp option", json_parse_tcp_option_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES },
 		{ "ip option", json_parse_ip_option_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES },
-		{ "meta", json_parse_meta_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP },
-		{ "osf", json_parse_osf_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_MAP },
-		{ "ipsec", json_parse_xfrm_expr, CTX_F_PRIMARY | CTX_F_MAP },
-		{ "socket", json_parse_socket_expr, CTX_F_PRIMARY },
-		{ "rt", json_parse_rt_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "ct", json_parse_ct_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP },
-		{ "numgen", json_parse_numgen_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
+		{ "meta", json_parse_meta_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "osf", json_parse_osf_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_MAP | CTX_F_CONCAT },
+		{ "ipsec", json_parse_xfrm_expr, CTX_F_PRIMARY | CTX_F_MAP | CTX_F_CONCAT },
+		{ "socket", json_parse_socket_expr, CTX_F_PRIMARY | CTX_F_CONCAT },
+		{ "rt", json_parse_rt_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "ct", json_parse_ct_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "numgen", json_parse_numgen_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
 		/* below two are hash expr */
-		{ "jhash", json_parse_hash_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "symhash", json_parse_hash_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "fib", json_parse_fib_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "|", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "^", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "&", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ ">>", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
-		{ "<<", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
+		{ "jhash", json_parse_hash_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "symhash", json_parse_hash_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "fib", json_parse_fib_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "|", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "^", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "&", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ ">>", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "<<", json_parse_binop_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
 		{ "accept", json_parse_verdict_expr, CTX_F_RHS | CTX_F_SET_RHS },
 		{ "drop", json_parse_verdict_expr, CTX_F_RHS | CTX_F_SET_RHS },
 		{ "continue", json_parse_verdict_expr, CTX_F_RHS | CTX_F_SET_RHS },
@@ -1500,6 +1502,11 @@ static struct expr *json_parse_map_lhs_expr(struct json_ctx *ctx, json_t *root)
 	return json_parse_flagged_expr(ctx, CTX_F_MAP, root);
 }
 
+static struct expr *json_parse_concat_elem_expr(struct json_ctx *ctx, json_t *root)
+{
+	return json_parse_flagged_expr(ctx, CTX_F_CONCAT, root);
+}
+
 static struct expr *json_parse_dtype_expr(struct json_ctx *ctx, json_t *root)
 {
 	if (json_is_string(root)) {
-- 
2.25.1


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

* [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges
  2020-03-07  2:26 [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Phil Sutter
@ 2020-03-07  2:26 ` Phil Sutter
  2020-03-07 10:16   ` Stefano Brivio
  2020-03-09 13:32 ` [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Eric Garver
  1 sibling, 1 reply; 4+ messages in thread
From: Phil Sutter @ 2020-03-07  2:26 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Eric Garver, netfilter-devel

Very basic testing, just a set definition, a rule which references it
and another one with an anonymous set.

Sadly this is already enough to expose some pending issues:

* Payload dependency killing ignores the concatenated IP header
  expressions on LHS, so rule output is asymmetric.

* Anonymous sets don't accept concatenated ranges yet, so the second
  rule is manually disabled for now.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- New patch.
---
 tests/py/inet/sets.t                |  6 +++++
 tests/py/inet/sets.t.json           | 35 +++++++++++++++++++++++++++++
 tests/py/inet/sets.t.payload.bridge | 13 +++++++++++
 tests/py/inet/sets.t.payload.inet   | 11 +++++++++
 tests/py/inet/sets.t.payload.netdev | 12 ++++++++++
 5 files changed, 77 insertions(+)

diff --git a/tests/py/inet/sets.t b/tests/py/inet/sets.t
index daf8f2d6ca302..e0b0ee867f9b7 100644
--- a/tests/py/inet/sets.t
+++ b/tests/py/inet/sets.t
@@ -16,3 +16,9 @@ ip saddr != @set2 drop;fail
 
 ip6 daddr != @set2 accept;ok
 ip6 daddr @set1 drop;fail
+
+!set3 type ipv4_addr . ipv4_addr . inet_service flags interval;ok
+?set3 10.0.0.0/8 . 192.168.1.3-192.168.1.9 . 1024-65535;ok
+
+ip saddr . ip daddr . tcp dport @set3 accept;ok
+-ip daddr . tcp dport { 10.0.0.0/8 . 10-23, 192.168.1.1-192.168.3.8 . 80-443 } accept;ok
diff --git a/tests/py/inet/sets.t.json b/tests/py/inet/sets.t.json
index bcb638f2664d5..58e19ef647058 100644
--- a/tests/py/inet/sets.t.json
+++ b/tests/py/inet/sets.t.json
@@ -36,3 +36,38 @@
     }
 ]
 
+# ip saddr . ip daddr . tcp dport @set3 accept
+[
+    {
+        "match": {
+            "left": {
+                "concat": [
+                    {
+                        "payload": {
+                            "field": "saddr",
+                            "protocol": "ip"
+                        }
+                    },
+                    {
+                        "payload": {
+                            "field": "daddr",
+                            "protocol": "ip"
+                        }
+                    },
+                    {
+                        "payload": {
+                            "field": "dport",
+                            "protocol": "tcp"
+                        }
+                    }
+                ]
+            },
+            "op": "==",
+            "right": "@set3"
+        }
+    },
+    {
+        "accept": null
+    }
+]
+
diff --git a/tests/py/inet/sets.t.payload.bridge b/tests/py/inet/sets.t.payload.bridge
index f5aaab1d79bc6..089d9dd7a28dd 100644
--- a/tests/py/inet/sets.t.payload.bridge
+++ b/tests/py/inet/sets.t.payload.bridge
@@ -13,3 +13,16 @@ bridge test-inet input
   [ payload load 16b @ network header + 24 => reg 1 ]
   [ lookup reg 1 set set2 0x1 ]
   [ immediate reg 0 accept ]
+
+# ip saddr . ip daddr . tcp dport @set3 accept
+bridge 
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ payload load 4b @ network header + 16 => reg 9 ]
+  [ payload load 2b @ transport header + 2 => reg 10 ]
+  [ lookup reg 1 set set3 ]
+  [ immediate reg 0 accept ]
+
diff --git a/tests/py/inet/sets.t.payload.inet b/tests/py/inet/sets.t.payload.inet
index 1584fc07451eb..c5acd6103a038 100644
--- a/tests/py/inet/sets.t.payload.inet
+++ b/tests/py/inet/sets.t.payload.inet
@@ -14,4 +14,15 @@ inet test-inet input
   [ lookup reg 1 set set2 0x1 ]
   [ immediate reg 0 accept ]
 
+# ip saddr . ip daddr . tcp dport @set3 accept
+inet 
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ payload load 4b @ network header + 16 => reg 9 ]
+  [ payload load 2b @ transport header + 2 => reg 10 ]
+  [ lookup reg 1 set set3 ]
+  [ immediate reg 0 accept ]
 
diff --git a/tests/py/inet/sets.t.payload.netdev b/tests/py/inet/sets.t.payload.netdev
index 9c94e38429fb7..82994eabf48b7 100644
--- a/tests/py/inet/sets.t.payload.netdev
+++ b/tests/py/inet/sets.t.payload.netdev
@@ -14,3 +14,15 @@ netdev test-netdev ingress
   [ lookup reg 1 set set2 0x1 ]
   [ immediate reg 0 accept ]
 
+# ip saddr . ip daddr . tcp dport @ set3 accept
+inet 
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ payload load 4b @ network header + 16 => reg 9 ]
+  [ payload load 2b @ transport header + 2 => reg 10 ]
+  [ lookup reg 1 set set3 ]
+  [ immediate reg 0 accept ]
+
-- 
2.25.1


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

* Re: [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges
  2020-03-07  2:26 ` [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges Phil Sutter
@ 2020-03-07 10:16   ` Stefano Brivio
  0 siblings, 0 replies; 4+ messages in thread
From: Stefano Brivio @ 2020-03-07 10:16 UTC (permalink / raw)
  To: Phil Sutter; +Cc: Pablo Neira Ayuso, Eric Garver, netfilter-devel

On Sat,  7 Mar 2020 03:26:33 +0100
Phil Sutter <phil@nwl.cc> wrote:

> * Anonymous sets don't accept concatenated ranges yet, so the second
>   rule is manually disabled for now.

By the way, I'm looking into this these days.

-- 
Stefano


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

* Re: [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions
  2020-03-07  2:26 [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Phil Sutter
  2020-03-07  2:26 ` [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges Phil Sutter
@ 2020-03-09 13:32 ` Eric Garver
  1 sibling, 0 replies; 4+ messages in thread
From: Eric Garver @ 2020-03-09 13:32 UTC (permalink / raw)
  To: Phil Sutter; +Cc: Pablo Neira Ayuso, netfilter-devel

On Sat, Mar 07, 2020 at 03:26:32AM +0100, Phil Sutter wrote:
> Duplicate commit 8ac2f3b2fca38's changes to bison parser into JSON
> parser by introducing a new context flag signalling we're parsing
> concatenated expressions.
> 
> Fixes: 8ac2f3b2fca38 ("src: Add support for concatenated set ranges")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v1:
> - Fix accidental reject of concat expressions on LHS.
> ---

Thanks Phil. This passes all my tests and the patch looks good.

Acked-by: Eric Garver <eric@garver.life>


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

end of thread, other threads:[~2020-03-09 13:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-07  2:26 [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Phil Sutter
2020-03-07  2:26 ` [nft PATCH v2 2/2] tests/py: Add tests involving concatenated ranges Phil Sutter
2020-03-07 10:16   ` Stefano Brivio
2020-03-09 13:32 ` [nft PATCH v2 1/2] parser_json: Support ranges in concat expressions Eric Garver

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).