The following ruleset:
add table ip filter
add chain ip filter input { type filter hook input priority 4; }
add chain ip filter ap
add rule ip filter input jump ap
add rule ip filter ap masquerade
results in a panic, because the masquerade extension should be rejected
from the filter chain. The existing validation is missing a chain
dependency check when the rule is added to the non-base chain.
This patch fixes the problem by walking down the rules from the
basechains, searching for either immediate or lookup expressions, then
jumping to non-base chains and again walking down the rules to perform
the expression validation, so we make sure the full ruleset graph is
validated.
As a result of this patch, nf_tables_check_loops() doesn't use
->validate to check for loops, instead it just checks for immediate
expressions.
Reported-by: Taehee Yoo <ap420073@xxxxxxxxx>
Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
---
v2: restore check_loop routine that broke in v1.
include/net/netfilter/nf_tables.h | 2 +
include/net/netfilter/nf_tables_core.h | 8 ++++
net/netfilter/nf_tables_api.c | 82 ++++++++++++++++++++++++----------
net/netfilter/nft_immediate.c | 27 +++++++----
net/netfilter/nft_lookup.c | 47 +++++++++++++++++++
5 files changed, 134 insertions(+), 32 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index a1e28dd5d0bf..7cb4c197b0d8 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -873,6 +873,8 @@ struct nft_chain {
char *name;
};
+int nft_chain_validate(const struct nft_ctx *ctx, const struct nft_chain *chain);
+
enum nft_chain_types {
NFT_CHAIN_T_DEFAULT = 0,
NFT_CHAIN_T_ROUTE,
diff --git a/include/net/netfilter/nf_tables_core.h b/include/net/netfilter/nf_tables_core.h
index ea5aab568be8..302913dfd0a2 100644
--- a/include/net/netfilter/nf_tables_core.h
+++ b/include/net/netfilter/nf_tables_core.h
@@ -2,6 +2,8 @@
#ifndef _NET_NF_TABLES_CORE_H
#define _NET_NF_TABLES_CORE_H
+#include <net/netfilter/nf_tables.h>
+
extern struct nft_expr_type nft_imm_type;
extern struct nft_expr_type nft_cmp_type;
extern struct nft_expr_type nft_lookup_type;
@@ -20,6 +22,12 @@ struct nft_cmp_fast_expr {
u8 len;
};
+struct nft_immediate_expr {
+ struct nft_data data;
+ enum nft_registers dreg:8;
+ u8 dlen;
+};
+
/* Calculate the mask for the nft_cmp_fast expression. On big endian the
* mask needs to include the *upper* bytes when interpreting that data as
* something smaller than the full u32, therefore a cpu_to_le32 is done.
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 91e80aa852d6..deb42fe18d5c 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1903,19 +1903,7 @@ static int nf_tables_newexpr(const struct nft_ctx *ctx,
goto err1;
}
- if (ops->validate) {
- const struct nft_data *data = NULL;
-
- err = ops->validate(ctx, expr, &data);
- if (err < 0)
- goto err2;
- }
-
return 0;
-
-err2:
- if (ops->destroy)
- ops->destroy(ctx, expr);
err1:
expr->ops = NULL;
return err;
@@ -2274,6 +2262,53 @@ static void nf_tables_rule_release(const struct nft_ctx *ctx,
nf_tables_rule_destroy(ctx, rule);
}
+int nft_chain_validate(const struct nft_ctx *ctx, const struct nft_chain *chain)
+{
+ struct nft_expr *expr, *last;
+ const struct nft_data *data;
+ struct nft_rule *rule;
+ int err;
+
+ list_for_each_entry(rule, &chain->rules, list) {
+ if (!nft_is_active_next(ctx->net, rule))
+ continue;
+
+ nft_rule_for_each_expr(expr, last, rule) {
+ if (!expr->ops->validate)
+ continue;
+
+ err = expr->ops->validate(ctx, expr, &data);
+ if (err < 0)
+ return err;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(nft_chain_validate);
+
+static int nf_tables_validate(struct net *net, const struct nft_table *table)
+{
+ struct nft_chain *chain;
+ struct nft_ctx ctx = {
+ .net = net,
+ .family = table->family,
+ };
+ int err;
+
+ list_for_each_entry(chain, &table->chains, list) {
+ if (!nft_is_base_chain(chain))
+ continue;
+
+ ctx.chain = chain;
+ err = nft_chain_validate(&ctx, chain);
+ if (err < 0)
+ return err;
+ }
+
+ return 0;
+}
+
#define NFT_RULE_MAXEXPRS 128
static struct nft_expr_info *info;
@@ -2435,7 +2470,8 @@ static int nf_tables_newrule(struct net *net, struct sock *nlsk,
}
}
chain->use++;
- return 0;
+
+ return nf_tables_validate(net, table);
err2:
nf_tables_rule_release(&ctx, rule);
@@ -4132,7 +4168,7 @@ static int nf_tables_newsetelem(struct net *net, struct sock *nlsk,
const struct nlattr *attr;
struct nft_set *set;
struct nft_ctx ctx;
- int rem, err = 0;
+ int rem, err;
if (nla[NFTA_SET_ELEM_LIST_ELEMENTS] == NULL)
return -EINVAL;
@@ -4152,9 +4188,10 @@ static int nf_tables_newsetelem(struct net *net, struct sock *nlsk,
nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) {
err = nft_add_set_elem(&ctx, set, attr, nlh->nlmsg_flags);
if (err < 0)
- break;
+ return err;
}
- return err;
+
+ return nf_tables_validate(net, ctx.table);
}
/**
@@ -6214,19 +6251,18 @@ static int nf_tables_check_loops(const struct nft_ctx *ctx,
list_for_each_entry(rule, &chain->rules, list) {
nft_rule_for_each_expr(expr, last, rule) {
- const struct nft_data *data = NULL;
+ struct nft_immediate_expr *priv;
+ const struct nft_data *data;
int err;
- if (!expr->ops->validate)
+ if (strcmp(expr->ops->type->name, "immediate"))
continue;
- err = expr->ops->validate(ctx, expr, &data);
- if (err < 0)
- return err;
-
- if (data == NULL)
+ priv = nft_expr_priv(expr);
+ if (priv->dreg != NFT_REG_VERDICT)
continue;
+ data = &priv->data;
switch (data->verdict.code) {
case NFT_JUMP:
case NFT_GOTO:
diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c
index aa87ff8beae8..15adf8ca82c3 100644
--- a/net/netfilter/nft_immediate.c
+++ b/net/netfilter/nft_immediate.c
@@ -17,12 +17,6 @@
#include <net/netfilter/nf_tables_core.h>
#include <net/netfilter/nf_tables.h>
-struct nft_immediate_expr {
- struct nft_data data;
- enum nft_registers dreg:8;
- u8 dlen;
-};
-
static void nft_immediate_eval(const struct nft_expr *expr,
struct nft_regs *regs,
const struct nft_pktinfo *pkt)
@@ -101,12 +95,27 @@ static int nft_immediate_dump(struct sk_buff *skb, const struct nft_expr *expr)
static int nft_immediate_validate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
- const struct nft_data **data)
+ const struct nft_data **d)
{
const struct nft_immediate_expr *priv = nft_expr_priv(expr);
+ const struct nft_data *data;
+ int err;
- if (priv->dreg == NFT_REG_VERDICT)
- *data = &priv->data;
+ if (priv->dreg != NFT_REG_VERDICT)
+ return 0;
+
+ data = &priv->data;
+
+ switch (data->verdict.code) {
+ case NFT_JUMP:
+ case NFT_GOTO:
+ err = nft_chain_validate(ctx, data->verdict.chain);
+ if (err < 0)
+ return err;
+ break;
+ default:
+ break;
+ }
return 0;
}
diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c
index f52da5e2199f..42e6fadf1417 100644
--- a/net/netfilter/nft_lookup.c
+++ b/net/netfilter/nft_lookup.c
@@ -149,6 +149,52 @@ static int nft_lookup_dump(struct sk_buff *skb, const struct nft_expr *expr)
return -1;
}
+static int nft_lookup_validate_setelem(const struct nft_ctx *ctx,
+ struct nft_set *set,
+ const struct nft_set_iter *iter,
+ struct nft_set_elem *elem)
+{
+ const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
+ const struct nft_data *data;
+
+ if (nft_set_ext_exists(ext, NFT_SET_EXT_FLAGS) &&
+ *nft_set_ext_flags(ext) & NFT_SET_ELEM_INTERVAL_END)
+ return 0;
+
+ data = nft_set_ext_data(ext);
+ switch (data->verdict.code) {
+ case NFT_JUMP:
+ case NFT_GOTO:
+ return nft_chain_validate(ctx, data->verdict.chain);
+ default:
+ return 0;
+ }
+}
+
+static int nft_lookup_validate(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nft_data **d)
+{
+ const struct nft_lookup *priv = nft_expr_priv(expr);
+ struct nft_set_iter iter;
+
+ if (!(priv->set->flags & NFT_SET_MAP) ||
+ priv->set->dtype != NFT_DATA_VERDICT)
+ return 0;
+
+ iter.genmask = nft_genmask_next(ctx->net);
+ iter.skip = 0;
+ iter.count = 0;
+ iter.err = 0;
+ iter.fn = nft_lookup_validate_setelem;
+
+ priv->set->ops->walk(ctx, priv->set, &iter);
+ if (iter.err < 0)
+ return iter.err;
+
+ return 0;
+}
+
static const struct nft_expr_ops nft_lookup_ops = {
.type = &nft_lookup_type,
.size = NFT_EXPR_SIZE(sizeof(struct nft_lookup)),
@@ -156,6 +202,7 @@ static const struct nft_expr_ops nft_lookup_ops = {
.init = nft_lookup_init,
.destroy = nft_lookup_destroy,
.dump = nft_lookup_dump,
+ .validate = nft_lookup_validate,
};
struct nft_expr_type nft_lookup_type __read_mostly = {
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
