Skip to content

Commit

Permalink
netfilter: nf_tables: validate hooks in NAT expressions
Browse files Browse the repository at this point in the history 
The user can crash the kernel if it uses any of the existing NAT
expressions from the wrong hook, so add some code to validate this
when loading the rule.

This patch introduces nft_chain_validate_hooks() which is based on
an existing function in the bridge version of the reject expression.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
  • Loading branch information
Pablo Neira Ayuso committed Jan 19, 2015
1 parent 2061dcd commit 75e8d06
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 50 deletions.
2 changes: 2 additions & 0 deletions include/net/netfilter/nf_tables.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,8 @@ enum nft_chain_type {

int nft_chain_validate_dependency(const struct nft_chain *chain,
enum nft_chain_type type);
int nft_chain_validate_hooks(const struct nft_chain *chain,
unsigned int hook_flags);

struct nft_stats {
u64 bytes;
Expand Down
29 changes: 6 additions & 23 deletions net/bridge/netfilter/nft_reject_bridge.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,22 +265,12 @@ static void nft_reject_bridge_eval(const struct nft_expr *expr,
data[NFT_REG_VERDICT].verdict = NF_DROP;
}

static int nft_reject_bridge_validate_hooks(const struct nft_chain *chain)
static int nft_reject_bridge_validate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nft_data **data)
{
struct nft_base_chain *basechain;

if (chain->flags & NFT_BASE_CHAIN) {
basechain = nft_base_chain(chain);

switch (basechain->ops[0].hooknum) {
case NF_BR_PRE_ROUTING:
case NF_BR_LOCAL_IN:
break;
default:
return -EOPNOTSUPP;
}
}
return 0;
return nft_chain_validate_hooks(ctx->chain, (1 << NF_BR_PRE_ROUTING) |
(1 << NF_BR_LOCAL_IN));
}

static int nft_reject_bridge_init(const struct nft_ctx *ctx,
Expand All @@ -290,7 +280,7 @@ static int nft_reject_bridge_init(const struct nft_ctx *ctx,
struct nft_reject *priv = nft_expr_priv(expr);
int icmp_code, err;

err = nft_reject_bridge_validate_hooks(ctx->chain);
err = nft_reject_bridge_validate(ctx, expr, NULL);
if (err < 0)
return err;

Expand Down Expand Up @@ -341,13 +331,6 @@ static int nft_reject_bridge_dump(struct sk_buff *skb,
return -1;
}

static int nft_reject_bridge_validate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nft_data **data)
{
return nft_reject_bridge_validate_hooks(ctx->chain);
}

static struct nft_expr_type nft_reject_bridge_type;
static const struct nft_expr_ops nft_reject_bridge_ops = {
.type = &nft_reject_bridge_type,
Expand Down
18 changes: 18 additions & 0 deletions net/netfilter/nf_tables_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3753,6 +3753,24 @@ int nft_chain_validate_dependency(const struct nft_chain *chain,
}
EXPORT_SYMBOL_GPL(nft_chain_validate_dependency);

int nft_chain_validate_hooks(const struct nft_chain *chain,
unsigned int hook_flags)
{
struct nft_base_chain *basechain;

if (chain->flags & NFT_BASE_CHAIN) {
basechain = nft_base_chain(chain);

if ((1 << basechain->ops[0].hooknum) & hook_flags)
return 0;

return -EOPNOTSUPP;
}

return 0;
}
EXPORT_SYMBOL_GPL(nft_chain_validate_hooks);

/*
* Loop detection - walk through the ruleset beginning at the destination chain
* of a new jump until either the source chain is reached (loop) or all
Expand Down
26 changes: 17 additions & 9 deletions net/netfilter/nft_masq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,30 @@ const struct nla_policy nft_masq_policy[NFTA_MASQ_MAX + 1] = {
};
EXPORT_SYMBOL_GPL(nft_masq_policy);

int nft_masq_validate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nft_data **data)
{
int err;

err = nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
if (err < 0)
return err;

return nft_chain_validate_hooks(ctx->chain,
(1 << NF_INET_POST_ROUTING));
}
EXPORT_SYMBOL_GPL(nft_masq_validate);

int nft_masq_init(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nlattr * const tb[])
{
struct nft_masq *priv = nft_expr_priv(expr);
int err;

err = nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
if (err < 0)
err = nft_masq_validate(ctx, expr, NULL);
if (err)
return err;

if (tb[NFTA_MASQ_FLAGS] == NULL)
Expand Down Expand Up @@ -60,12 +75,5 @@ int nft_masq_dump(struct sk_buff *skb, const struct nft_expr *expr)
}
EXPORT_SYMBOL_GPL(nft_masq_dump);

int nft_masq_validate(const struct nft_ctx *ctx, const struct nft_expr *expr,
const struct nft_data **data)
{
return nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
}
EXPORT_SYMBOL_GPL(nft_masq_validate);

MODULE_LICENSE("GPL");
MODULE_AUTHOR("Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>");
40 changes: 30 additions & 10 deletions net/netfilter/nft_nat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,40 @@ static const struct nla_policy nft_nat_policy[NFTA_NAT_MAX + 1] = {
[NFTA_NAT_FLAGS] = { .type = NLA_U32 },
};

static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
const struct nlattr * const tb[])
static int nft_nat_validate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nft_data **data)
{
struct nft_nat *priv = nft_expr_priv(expr);
u32 family;
int err;

err = nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
if (err < 0)
return err;

switch (priv->type) {
case NFT_NAT_SNAT:
err = nft_chain_validate_hooks(ctx->chain,
(1 << NF_INET_POST_ROUTING) |
(1 << NF_INET_LOCAL_IN));
break;
case NFT_NAT_DNAT:
err = nft_chain_validate_hooks(ctx->chain,
(1 << NF_INET_PRE_ROUTING) |
(1 << NF_INET_LOCAL_OUT));
break;
}

return err;
}

static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
const struct nlattr * const tb[])
{
struct nft_nat *priv = nft_expr_priv(expr);
u32 family;
int err;

if (tb[NFTA_NAT_TYPE] == NULL ||
(tb[NFTA_NAT_REG_ADDR_MIN] == NULL &&
tb[NFTA_NAT_REG_PROTO_MIN] == NULL))
Expand All @@ -115,6 +138,10 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
return -EINVAL;
}

err = nft_nat_validate(ctx, expr, NULL);
if (err < 0)
return err;

if (tb[NFTA_NAT_FAMILY] == NULL)
return -EINVAL;

Expand Down Expand Up @@ -219,13 +246,6 @@ static int nft_nat_dump(struct sk_buff *skb, const struct nft_expr *expr)
return -1;
}

static int nft_nat_validate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nft_data **data)
{
return nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
}

static struct nft_expr_type nft_nat_type;
static const struct nft_expr_ops nft_nat_ops = {
.type = &nft_nat_type,
Expand Down
25 changes: 17 additions & 8 deletions net/netfilter/nft_redir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,30 @@ const struct nla_policy nft_redir_policy[NFTA_REDIR_MAX + 1] = {
};
EXPORT_SYMBOL_GPL(nft_redir_policy);

int nft_redir_validate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nft_data **data)
{
int err;

err = nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
if (err < 0)
return err;

return nft_chain_validate_hooks(ctx->chain,
(1 << NF_INET_PRE_ROUTING) |
(1 << NF_INET_LOCAL_OUT));
}
EXPORT_SYMBOL_GPL(nft_redir_validate);

int nft_redir_init(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nlattr * const tb[])
{
struct nft_redir *priv = nft_expr_priv(expr);
int err;

err = nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
err = nft_redir_validate(ctx, expr, NULL);
if (err < 0)
return err;

Expand Down Expand Up @@ -88,12 +104,5 @@ int nft_redir_dump(struct sk_buff *skb, const struct nft_expr *expr)
}
EXPORT_SYMBOL_GPL(nft_redir_dump);

int nft_redir_validate(const struct nft_ctx *ctx, const struct nft_expr *expr,
const struct nft_data **data)
{
return nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
}
EXPORT_SYMBOL_GPL(nft_redir_validate);

MODULE_LICENSE("GPL");
MODULE_AUTHOR("Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>");

0 comments on commit 75e8d06

Please sign in to comment.