netlink_delinearize: don't kill dependencies accross statements

nft currently translates
ip protocol tcp meta mark set 1 tcp dport 22
to
mark set 0x00000001 tcp dport 22

This is wrong, the latter form is same as

mark set 0x00000001 ip protocol tcp tcp dport 22

and thats not correct (original rule sets mark for tcp packets only).

We need to clear the dependency stack whenever we see a statement other
than stmt_expr, as these will have side effects (counter, payload
mangling, logging and the like).

Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/payload.h b/include/payload.h
index a3d23095119c770b37e2e502d10b4d0937b7c1e0..8e357aef461e29864a9166e3491935ae342a9280 100644 (file)
--- a/include/payload.h
+++ b/include/payload.h
@@ -35,6 +35,8 @@ struct payload_dep_ctx {
 extern bool payload_is_known(const struct expr *expr);
 extern bool payload_is_stacked(const struct proto_desc *desc,
                               const struct expr *expr);
+
+void payload_dependency_reset(struct payload_dep_ctx *ctx);
 extern void payload_dependency_store(struct payload_dep_ctx *ctx,
                                     struct stmt *stmt,
                                     enum proto_bases base);

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 4a0b8dcc56516e4243e42417eb0928e728a51a2d..a65a97da89fbab48a869f1a53ceb1007cfd8354d 100644 (file)
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2153,6 +2153,27 @@ static void stmt_payload_postprocess(struct rule_pp_ctx *ctx)
        expr_postprocess(ctx, &stmt->payload.val);
 }
 
+/*
+ * We can only remove payload dependencies if they occur without
+ * a statment with side effects in between.
+ *
+ * For instance:
+ * 'ip protocol tcp tcp dport 22 counter' is same as
+ * 'tcp dport 22 counter'.
+ *
+ * 'ip protocol tcp counter tcp dport 22' cannot be written as
+ * 'counter tcp dport 22' (that would be counter ip protocol tcp, but
+ * that counts every packet, not just ip/tcp).
+ */
+static void
+rule_maybe_reset_payload_deps(struct payload_dep_ctx *pdctx, enum stmt_types t)
+{
+       if (t == STMT_EXPRESSION)
+               return;
+
+       payload_dependency_reset(pdctx);
+}
+
 static void rule_parse_postprocess(struct netlink_parse_ctx *ctx, struct rule *rule)
 {
        struct rule_pp_ctx rctx;
@@ -2162,9 +2183,11 @@ static void rule_parse_postprocess(struct netlink_parse_ctx *ctx, struct rule *r
        proto_ctx_init(&rctx.pctx, rule->handle.family);
 
        list_for_each_entry_safe(stmt, next, &rule->stmts, list) {
+               enum stmt_types type = stmt->ops->type;
+
                rctx.stmt = stmt;
 
-               switch (stmt->ops->type) {
+               switch (type) {
                case STMT_EXPRESSION:
                        stmt_expr_postprocess(&rctx);
                        break;
@@ -2217,7 +2240,10 @@ static void rule_parse_postprocess(struct netlink_parse_ctx *ctx, struct rule *r
                default:
                        break;
                }
+
                rctx.pdctx.prev = rctx.stmt;
+
+               rule_maybe_reset_payload_deps(&rctx.pdctx, type);
        }
 }
 

diff --git a/src/payload.c b/src/payload.c
index 169954bad207828f7dc47eef08d5853fae25a4d2..55128fee14986a0efe96b26fbb865545430b1e07 100644 (file)
--- a/src/payload.c
+++ b/src/payload.c
@@ -366,6 +366,11 @@ bool payload_is_stacked(const struct proto_desc *desc, const struct expr *expr)
        return next && next->base == desc->base;
 }
 
+void payload_dependency_reset(struct payload_dep_ctx *ctx)
+{
+       memset(ctx, 0, sizeof(*ctx));
+}
+
 /**
  * payload_dependency_store - store a possibly redundant protocol match
  *