[PATCH nft 1/2] netlink_delinearize: complete payload expression in payload statement

[PATCH nft 1/2] netlink_delinearize: complete payload expression in payload statement

[Pablo Neira Ayuso]

Call payload_expr_complete() to complete payload expression in payload
statement, otherwise expr->payload.desc is set to proto_unknown.

Call stmt_payload_binop_postprocess() introduced by 50ca788ca4d0
("netlink: decode payload statment") if payload_expr_complete() fails to
provide a protocol description (eg. ip dscp).

Follow up patch do not allow to remove redundant payload dependency if
proto_unknown is set to deal with the raw payload expression case.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/netlink_delinearize.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 828ad12d7536..0b6cf1072294 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2995,15 +2995,16 @@ static void stmt_payload_postprocess(struct rule_pp_ctx *ctx)
 {
 	struct stmt *stmt = ctx->stmt;
 
+	payload_expr_complete(stmt->payload.expr, &ctx->pctx);
+	if (!payload_is_known(stmt->payload.expr))
+		stmt_payload_binop_postprocess(ctx);
+
 	expr_postprocess(ctx, &stmt->payload.expr);
 
 	expr_set_type(stmt->payload.val,
 		      stmt->payload.expr->dtype,
 		      stmt->payload.expr->byteorder);
 
-	if (!payload_is_known(stmt->payload.expr))
-		stmt_payload_binop_postprocess(ctx);
-
 	expr_postprocess(ctx, &stmt->payload.val);
 }
 
-- 
2.30.2

[PATCH nft 2/2] payload: do not kill dependency for proto_unknown

[Pablo Neira Ayuso]

Unsupported meta match on layer 4 protocol sets on protocol context to
proto_unknown, handle anything coming after it as a raw expression in
payload_expr_expand().

Moreover, payload_dependency_kill() skips dependency removal if protocol
is unknown, so raw payload expression leaves meta layer 4 protocol
remains in place.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1641
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/payload.c                     |  6 ++++--
 tests/py/any/rawpayload.t         |  2 ++
 tests/py/any/rawpayload.t.json    | 31 +++++++++++++++++++++++++++++++
 tests/py/any/rawpayload.t.payload |  8 ++++++++
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/src/payload.c b/src/payload.c
index 2c0d0ac9e8ae..101bfbda5878 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -848,7 +848,8 @@ static bool payload_may_dependency_kill(struct payload_dep_ctx *ctx,
 void payload_dependency_kill(struct payload_dep_ctx *ctx, struct expr *expr,
 			     unsigned int family)
 {
-	if (payload_dependency_exists(ctx, expr->payload.base) &&
+	if (expr->payload.desc != &proto_unknown &&
+	    payload_dependency_exists(ctx, expr->payload.base) &&
 	    payload_may_dependency_kill(ctx, family, expr))
 		payload_dependency_release(ctx, expr->payload.base);
 }
@@ -1058,8 +1059,9 @@ void payload_expr_expand(struct list_head *list, struct expr *expr,
 	assert(expr->etype == EXPR_PAYLOAD);
 
 	desc = ctx->protocol[expr->payload.base].desc;
-	if (desc == NULL)
+	if (desc == NULL || desc == &proto_unknown)
 		goto raw;
+
 	assert(desc->base == expr->payload.base);
 
 	desc = get_stacked_desc(ctx, desc, expr, &total);
diff --git a/tests/py/any/rawpayload.t b/tests/py/any/rawpayload.t
index 128e8088c4e5..5bc9d35f7465 100644
--- a/tests/py/any/rawpayload.t
+++ b/tests/py/any/rawpayload.t
@@ -19,4 +19,6 @@ meta l4proto tcp @th,16,16 { 22, 23, 80};ok;tcp dport { 22, 23, 80}
 @ll,0,8 & 0x80 == 0x80;ok
 @ll,0,128 0xfedcba987654321001234567890abcde;ok
 
+meta l4proto 91 @th,400,16 0x0 accept;ok
+
 @ih,32,32 0x14000000;ok
diff --git a/tests/py/any/rawpayload.t.json b/tests/py/any/rawpayload.t.json
index b5115e0ddacf..4cae4d493da3 100644
--- a/tests/py/any/rawpayload.t.json
+++ b/tests/py/any/rawpayload.t.json
@@ -156,6 +156,37 @@
     }
 ]
 
+# meta l4proto 91 @th,400,16 0x0 accept
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "l4proto"
+                }
+            },
+            "op": "==",
+            "right": 91
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "base": "th",
+                    "len": 16,
+                    "offset": 400
+                }
+            },
+            "op": "==",
+            "right": 0
+        }
+    },
+    {
+        "accept": null
+    }
+]
+
 # @ih,32,32 0x14000000
 [
     {
diff --git a/tests/py/any/rawpayload.t.payload b/tests/py/any/rawpayload.t.payload
index 61c41cb976d6..fe2377e65a77 100644
--- a/tests/py/any/rawpayload.t.payload
+++ b/tests/py/any/rawpayload.t.payload
@@ -48,6 +48,14 @@ inet test-inet input
   [ payload load 16b @ link header + 0 => reg 1 ]
   [ cmp eq reg 1 0x98badcfe 0x10325476 0x67452301 0xdebc0a89 ]
 
+# meta l4proto 91 @th,400,16 0x0 accept
+inet test-inet input
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x0000005b ]
+  [ payload load 2b @ transport header + 50 => reg 1 ]
+  [ cmp eq reg 1 0x00000000 ]
+  [ immediate reg 0 accept ]
+
 # @ih,32,32 0x14000000
 inet test-inet input
   [ payload load 4b @ inner header + 4 => reg 1 ]
-- 
2.30.2

Re: [PATCH nft 2/2] payload: do not kill dependency for proto_unknown

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Unsupported meta match on layer 4 protocol sets on protocol context to
> proto_unknown, handle anything coming after it as a raw expression in
> payload_expr_expand().
> 
> Moreover, payload_dependency_kill() skips dependency removal if protocol
> is unknown, so raw payload expression leaves meta layer 4 protocol
> remains in place.
> 
> Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1641
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/payload.c                     |  6 ++++--
>  tests/py/any/rawpayload.t         |  2 ++
>  tests/py/any/rawpayload.t.json    | 31 +++++++++++++++++++++++++++++++
>  tests/py/any/rawpayload.t.payload |  8 ++++++++

LGTM, thanks for including testcases!