src: ensure chain policy evaluation when specified

Set on CHAIN_F_BASECHAIN when policy is specified in chain, otherwise
chain priority is not evaluated.

Toggling this flag requires needs three adjustments to work though:

1) chain_evaluate() needs skip evaluation of hook name and priority if
   not specified to allow for updating the default chain policy, e.g.

	chain ip x y { policy accept; }

2) update netlink bytecode generation for chain to skip NFTA_CHAIN_HOOK
   so update path is exercised in the kernel.

3) error reporting needs to check if basechain priority and type is
   set on, otherwise skip further hints.

Fixes: acdfae9c3126 ("src: allow to specify the default policy for base chains")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/cmd.c b/src/cmd.c
index ff634af2ac24945e5082113ca839d0ad4ae2ef4f..9d5544f03c32a3e44fe62e45f91de22d2de7734f 100644 (file)
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -282,6 +282,9 @@ static int nft_cmd_chain_error(struct netlink_ctx *ctx, struct cmd *cmd,
                if (!(chain->flags & CHAIN_F_BASECHAIN))
                        break;
 
+               if (!chain->priority.expr || !chain->type.str)
+                       break;
+
                mpz_export_data(&priority, chain->priority.expr->value,
                                BYTEORDER_HOST_ENDIAN, sizeof(int));
                if (priority <= -200 && !strcmp(chain->type.str, "nat"))

diff --git a/src/evaluate.c b/src/evaluate.c
index 0a430c827210179e4954c6d6ceb3da19c8c6d50f..1696f2b8a855927174b1bde6bb27b99fe67489f2 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -5776,18 +5776,21 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
        }
 
        if (chain->flags & CHAIN_F_BASECHAIN) {
-               chain->hook.num = str2hooknum(chain->handle.family,
-                                             chain->hook.name);
-               if (chain->hook.num == NF_INET_NUMHOOKS)
-                       return __stmt_binary_error(ctx, &chain->hook.loc, NULL,
-                                                  "The %s family does not support this hook",
-                                                  family2str(chain->handle.family));
-
-               if (!evaluate_priority(ctx, &chain->priority,
-                                      chain->handle.family, chain->hook.num))
-                       return __stmt_binary_error(ctx, &chain->priority.loc, NULL,
-                                                  "invalid priority expression %s in this context.",
-                                                  expr_name(chain->priority.expr));
+               if (chain->hook.name) {
+                       chain->hook.num = str2hooknum(chain->handle.family,
+                                                     chain->hook.name);
+                       if (chain->hook.num == NF_INET_NUMHOOKS)
+                               return __stmt_binary_error(ctx, &chain->hook.loc, NULL,
+                                                          "The %s family does not support this hook",
+                                                          family2str(chain->handle.family));
+               }
+               if (chain->priority.expr) {
+                       if (!evaluate_priority(ctx, &chain->priority,
+                                              chain->handle.family, chain->hook.num))
+                               return __stmt_binary_error(ctx, &chain->priority.loc, NULL,
+                                                          "invalid priority expression %s in this context.",
+                                                          expr_name(chain->priority.expr));
+               }
                if (chain->policy) {
                        expr_set_context(&ctx->ectx, &policy_type,
                                         NFT_NAME_MAXLEN * BITS_PER_BYTE);

diff --git a/src/mnl.c b/src/mnl.c
index 43229f2498e553f63ee2d1f11d785039b8bcd0fb..ceb43b06690c01c5511abe235512992c10393a67 100644 (file)
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -897,7 +897,9 @@ int mnl_nft_chain_add(struct netlink_ctx *ctx, struct cmd *cmd,
                        mnl_attr_put_strz(nlh, NFTA_CHAIN_TYPE, cmd->chain->type.str);
                }
 
-               nest = mnl_attr_nest_start(nlh, NFTA_CHAIN_HOOK);
+               if (cmd->chain->type.str ||
+                   (cmd->chain && cmd->chain->dev_expr))
+                       nest = mnl_attr_nest_start(nlh, NFTA_CHAIN_HOOK);
 
                if (cmd->chain->type.str) {
                        mnl_attr_put_u32(nlh, NFTA_HOOK_HOOKNUM, htonl(cmd->chain->hook.num));
@@ -909,7 +911,9 @@ int mnl_nft_chain_add(struct netlink_ctx *ctx, struct cmd *cmd,
                if (cmd->chain && cmd->chain->dev_expr)
                        mnl_nft_chain_devs_build(nlh, cmd);
 
-               mnl_attr_nest_end(nlh, nest);
+               if (cmd->chain->type.str ||
+                   (cmd->chain && cmd->chain->dev_expr))
+                       mnl_attr_nest_end(nlh, nest);
        }
 
        nftnl_chain_free(nlc);

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 0b1ea699c6102023f925a8d3ae9b512ec67ed0dd..1e4b3f8a50c59aaab848d8eb297b30e9de62f9f7 100644 (file)
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2834,6 +2834,7 @@ policy_spec               :       POLICY          policy_expr     close_scope_policy
                                }
                                $<chain>0->policy               = $2;
                                $<chain>0->policy->location     = @$;
+                               $<chain>0->flags                |= CHAIN_F_BASECHAIN;
                        }
                        ;
 

diff --git a/tests/shell/testcases/bogons/nft-f/basechain_bad_policy b/tests/shell/testcases/bogons/nft-f/basechain_bad_policy
new file mode 100644 (file)
index 0000000..998e423
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/basechain_bad_policy
@@ -0,0 +1,2 @@
+define MY_POLICY = deny
+table T { chain C { policy $MY_POLICY; };};

diff --git a/tests/shell/testcases/bogons/nft-f/unexisting_chain_set_policy b/tests/shell/testcases/bogons/nft-f/unexisting_chain_set_policy
new file mode 100644 (file)
index 0000000..0889559
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/unexisting_chain_set_policy
@@ -0,0 +1,5 @@
+table ip x {
+        chain y {
+                policy drop;
+        }
+}