[PATCH nft] payload: don't kill dependency for proto_th

[PATCH nft] payload: don't kill dependency for proto_th

[Florian Westphal]

proto_th carries no information about the proto number, we need to
preserve the L4 protocol expression unless we can be sure that

For example, if "meta l4proto 91 @th,0,16 0" is simplified to
"th sport 0", the information of protocol number is lost.

Based on initial patch from Xiao Liang.

Signed-off-by: Xiao Liang <shaw.leon@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/netlink_delinearize.c         |  1 +
 src/payload.c                     | 16 ++++++++++++++--
 tests/py/any/rawpayload.t         |  1 +
 tests/py/any/rawpayload.t.json    | 31 +++++++++++++++++++++++++++++++
 tests/py/any/rawpayload.t.payload |  8 ++++++++
 5 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 86c8602860f6..b629916ebff8 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2102,6 +2102,7 @@ static void payload_match_expand(struct rule_pp_ctx *ctx,
 		 */
 		payload_dependency_kill(&dl->pdctx, nexpr->left,
 					dl->pctx.family);
+		expr_set_type(tmp, nexpr->left->dtype, nexpr->byteorder);
 		if (expr->op == OP_EQ && left->flags & EXPR_F_PROTOCOL)
 			payload_dependency_store(&dl->pdctx, nstmt, base);
 	}
diff --git a/src/payload.c b/src/payload.c
index ee6b39a34cb4..018719751103 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -812,7 +812,7 @@ static bool icmp_dep_type_match(enum icmp_hdr_field_type t, uint8_t type)
 	BUG("Missing icmp type mapping");
 }
 
-static bool payload_may_dependency_kill_icmp(struct payload_dep_ctx *ctx, struct expr *expr)
+static bool payload_may_dependency_kill_icmp(struct payload_dep_ctx *ctx, const struct expr *expr)
 {
 	const struct expr *dep = payload_dependency_get(ctx, expr->payload.base);
 	enum icmp_hdr_field_type icmp_dep;
@@ -832,7 +832,7 @@ static bool payload_may_dependency_kill_icmp(struct payload_dep_ctx *ctx, struct
 	return ctx->icmp_type == icmp_dep_to_type(icmp_dep);
 }
 
-static bool payload_may_dependency_kill_ll(struct payload_dep_ctx *ctx, struct expr *expr)
+static bool payload_may_dependency_kill_ll(struct payload_dep_ctx *ctx, const struct expr *expr)
 {
 	const struct expr *dep = payload_dependency_get(ctx, expr->payload.base);
 
@@ -894,6 +894,18 @@ static bool payload_may_dependency_kill(struct payload_dep_ctx *ctx,
 	if (expr->payload.base != PROTO_BASE_TRANSPORT_HDR)
 		return true;
 
+	if (expr->payload.desc == &proto_th) {
+		/* &proto_th could mean any of udp, tcp, dccp, ... so we
+		 * cannot remove the dependency.
+		 *
+		 * Also prefer raw payload @th syntax, there is no
+		 * 'source/destination port' protocol here.
+		 */
+		expr->payload.desc = &proto_unknown;
+		expr->dtype = &xinteger_type;
+		return false;
+	}
+
 	if (dep->left->etype != EXPR_PAYLOAD ||
 	    dep->left->payload.base != PROTO_BASE_TRANSPORT_HDR)
 		return true;
diff --git a/tests/py/any/rawpayload.t b/tests/py/any/rawpayload.t
index 745b4a615e6c..118f58fd0f75 100644
--- a/tests/py/any/rawpayload.t
+++ b/tests/py/any/rawpayload.t
@@ -21,6 +21,7 @@ meta l4proto tcp @th,16,16 { 22, 23, 80};ok;tcp dport { 22, 23, 80}
 @ll,0,128 0xfedcba987654321001234567890abcde;ok
 
 meta l4proto 91 @th,400,16 0x0 accept;ok
+meta l4proto 91 @th,0,16 0x0 accept;ok
 
 @ih,32,32 0x14000000;ok
 @ih,58,6 set 0 @ih,86,6 set 0 @ih,170,22 set 0;ok;@ih,58,6 set 0x0 @ih,86,6 set 0x0 @ih,170,22 set 0x0
diff --git a/tests/py/any/rawpayload.t.json b/tests/py/any/rawpayload.t.json
index 4a06c5987a7b..04ed0acf1ed0 100644
--- a/tests/py/any/rawpayload.t.json
+++ b/tests/py/any/rawpayload.t.json
@@ -187,6 +187,37 @@
     }
 ]
 
+# meta l4proto 91 @th,0,16 0x0 accept
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "l4proto"
+                }
+            },
+            "op": "==",
+            "right": 91
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "base": "th",
+                    "len": 16,
+                    "offset": 0
+                }
+            },
+            "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 8984eef6a481..c093d5d8932f 100644
--- a/tests/py/any/rawpayload.t.payload
+++ b/tests/py/any/rawpayload.t.payload
@@ -56,6 +56,14 @@ inet test-inet input
   [ cmp eq reg 1 0x00000000 ]
   [ immediate reg 0 accept ]
 
+# meta l4proto 91 @th,0,16 0x0 accept
+inet test-inet input
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x0000005b ]
+  [ payload load 2b @ transport header + 0 => 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.45.3

Re: [PATCH nft] payload: don't kill dependency for proto_th

[Pablo Neira Ayuso]

On Thu, Feb 27, 2025 at 11:47:02AM +0100, Florian Westphal wrote:
> proto_th carries no information about the proto number, we need to
> preserve the L4 protocol expression unless we can be sure that
> 
> For example, if "meta l4proto 91 @th,0,16 0" is simplified to
> "th sport 0", the information of protocol number is lost.
> 
> Based on initial patch from Xiao Liang.
> 
> Signed-off-by: Xiao Liang <shaw.leon@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>

Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>