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@xxxxxxxxxxxxx>
---
v2: adjust error reporting path too.
src/cmd.c | 3 +++
src/evaluate.c | 27 ++++++++++---------
src/mnl.c | 8 ++++--
src/parser_bison.y | 1 +
.../bogons/nft-f/basechain_bad_policy | 2 ++
.../bogons/nft-f/unexisting_chain_set_policy | 5 ++++
6 files changed, 32 insertions(+), 14 deletions(-)
create mode 100644 tests/shell/testcases/bogons/nft-f/basechain_bad_policy
create mode 100644 tests/shell/testcases/bogons/nft-f/unexisting_chain_set_policy
diff --git a/src/cmd.c b/src/cmd.c
index ff634af2ac24..9d5544f03c32 100644
--- 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 8f037601c45f..7b524b418eb7 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -5772,18 +5772,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 43229f2498e5..ceb43b06690c 100644
--- 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 0b1ea699c610..1e4b3f8a50c5 100644
--- 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
index 000000000000..998e423cb7b4
--- /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
index 000000000000..088955996490
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/unexisting_chain_set_policy
@@ -0,0 +1,5 @@
+table ip x {
+ chain y {
+ policy drop;
+ }
+}
--
2.30.2
