evaluate: don't assert on net/transport header conflict

commit 3a734d60813193974a4a0e8ed0af3349f8857ec9 upstream.

before:
nft: evaluate.c:467: conflict_resolution_gen_dependency: Assertion `expr->payload.base == PROTO_BASE_LL_HDR' failed.
Aborted (core dumped)

conflict_resolution_gen_dependency() can only handle linklayer
conflicts, hence the assert.

Rename it accordingly.  Also rename resolve_protocol_conflict, it doesn't
do anything for != PROTO_BASE_LL_HDR and extend the assertion to that
function too.

Callers now enforce PROTO_BASE_LL_HDR prerequisite.

after:
Error: conflicting transport layer protocols specified: comp vs. udp
 ip6 nexthdr comp udp dport 4789
                  ^^^^^^^^^

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

diff --git a/src/evaluate.c b/src/evaluate.c
index 5027524ebf86baca2ddece7ff655953ce238c01c..ba40808e1def6154f788415620c97ea5ccdd397f 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -472,9 +472,9 @@ static int expr_evaluate_primary(struct eval_ctx *ctx, struct expr **expr)
 }
 
 static int
-conflict_resolution_gen_dependency(struct eval_ctx *ctx, int protocol,
-                                  const struct expr *expr,
-                                  struct stmt **res)
+ll_conflict_resolution_gen_dependency(struct eval_ctx *ctx, int protocol,
+                                     const struct expr *expr,
+                                     struct stmt **res)
 {
        enum proto_bases base = expr->payload.base;
        const struct proto_hdr_template *tmpl;
@@ -727,49 +727,43 @@ static bool proto_is_dummy(const struct proto_desc *desc)
        return desc == &proto_inet || desc == &proto_netdev;
 }
 
-static int resolve_protocol_conflict(struct eval_ctx *ctx,
-                                    const struct proto_desc *desc,
-                                    struct expr *payload)
+static int resolve_ll_protocol_conflict(struct eval_ctx *ctx,
+                                       const struct proto_desc *desc,
+                                       struct expr *payload)
 {
        enum proto_bases base = payload->payload.base;
        struct stmt *nstmt = NULL;
+       unsigned int i;
        int link, err;
 
-       if (payload->payload.base == PROTO_BASE_LL_HDR) {
-               if (proto_is_dummy(desc)) {
-                       err = meta_iiftype_gen_dependency(ctx, payload, &nstmt);
-                       if (err < 0)
-                               return err;
+       assert(base == PROTO_BASE_LL_HDR);
 
-                       desc = payload->payload.desc;
-                       rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
-               } else {
-                       unsigned int i;
+       if (proto_is_dummy(desc)) {
+               err = meta_iiftype_gen_dependency(ctx, payload, &nstmt);
+               if (err < 0)
+                       return err;
 
-                       /* payload desc stored in the L2 header stack? No conflict. */
-                       for (i = 0; i < ctx->pctx.stacked_ll_count; i++) {
-                               if (ctx->pctx.stacked_ll[i] == payload->payload.desc)
-                                       return 0;
-                       }
+               desc = payload->payload.desc;
+               rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+       } else {
+               /* payload desc stored in the L2 header stack? No conflict. */
+               for (i = 0; i < ctx->pctx.stacked_ll_count; i++) {
+                       if (ctx->pctx.stacked_ll[i] == payload->payload.desc)
+                               return 0;
                }
        }
 
-       assert(base <= PROTO_BASE_MAX);
        /* This payload and the existing context don't match, conflict. */
        if (ctx->pctx.protocol[base + 1].desc != NULL)
                return 1;
 
        link = proto_find_num(desc, payload->payload.desc);
        if (link < 0 ||
-           conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
+           ll_conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
                return 1;
 
-       if (base == PROTO_BASE_LL_HDR) {
-               unsigned int i;
-
-               for (i = 0; i < ctx->pctx.stacked_ll_count; i++)
-                       payload->payload.offset += ctx->pctx.stacked_ll[i]->length;
-       }
+       for (i = 0; i < ctx->pctx.stacked_ll_count; i++)
+               payload->payload.offset += ctx->pctx.stacked_ll[i]->length;
 
        rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
 
@@ -808,7 +802,7 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
 
                        link = proto_find_num(desc, payload->payload.desc);
                        if (link < 0 ||
-                           conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
+                           ll_conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
                                return expr_error(ctx->msgs, payload,
                                                  "conflicting protocols specified: %s vs. %s",
                                                  desc->name,
@@ -863,8 +857,8 @@ check_icmp:
        /* If we already have context and this payload is on the same
         * base, try to resolve the protocol conflict.
         */
-       if (payload->payload.base == desc->base) {
-               err = resolve_protocol_conflict(ctx, desc, payload);
+       if (base == PROTO_BASE_LL_HDR) {
+               err = resolve_ll_protocol_conflict(ctx, desc, payload);
                if (err <= 0)
                        return err;
 
@@ -873,7 +867,8 @@ check_icmp:
                        return 0;
        }
        return expr_error(ctx->msgs, payload,
-                         "conflicting protocols specified: %s vs. %s",
+                         "conflicting %s protocols specified: %s vs. %s",
+                         proto_base_names[base],
                          ctx->pctx.protocol[base].desc->name,
                          payload->payload.desc->name);
 }

diff --git a/tests/shell/testcases/bogons/nft-f/evaluate_conflict_resolution_gen_dependency_base_ll_hdr_assert b/tests/shell/testcases/bogons/nft-f/evaluate_conflict_resolution_gen_dependency_base_ll_hdr_assert
new file mode 100644 (file)
index 0000000..43d72c4
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/evaluate_conflict_resolution_gen_dependency_base_ll_hdr_assert
@@ -0,0 +1,5 @@
+table ip6 t {
+       chain c {
+               ip6 nexthdr comp udp dport 4789
+       }
+}