* [PATCH nf] netfilter: nf_tables: validate NFPROTO_{IPV4,IPV6,INET} family
@ 2024-01-23 16:13 Pablo Neira Ayuso
2024-01-23 17:10 ` Florian Westphal
0 siblings, 1 reply; 2+ messages in thread
From: Pablo Neira Ayuso @ 2024-01-23 16:13 UTC (permalink / raw)
To: netfilter-devel
Several expressions explicitly refer to NF_INET_* hook definitions
from expr->ops->validate, however, family is not validated.
Bail out with EOPNOTSUPP in case they are used from unsupported
families.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
This is v1:
NF_INET_* and NF_NETDEV_* hook definitions overlap, and .validate refers to
hooks only in several expressions, not families.
- synproxy refers to NFPROTO_BRIDGE from eval path, however, .validate does
not refer to bridge hooks.
- rt and socket might be already used from the ingress hooks, another
possibility would be to update .validate to refer to these hooks explicitly.
- nft_compat's xt_target_check() and xt_match_check() calls already validate
the family, but better reject this unfront.
compile tested only.
net/netfilter/nft_compat.c | 10 ++++++++++
net/netfilter/nft_flow_offload.c | 5 +++++
net/netfilter/nft_nat.c | 5 +++++
net/netfilter/nft_rt.c | 5 +++++
net/netfilter/nft_socket.c | 5 +++++
net/netfilter/nft_synproxy.c | 5 +++++
net/netfilter/nft_tproxy.c | 5 +++++
7 files changed, 40 insertions(+)
diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 5284cd2ad532..61f5498b73ef 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -350,6 +350,11 @@ static int nft_target_validate(const struct nft_ctx *ctx,
unsigned int hook_mask = 0;
int ret;
+ if (ctx->family != NFPROTO_IPV4 &&
+ ctx->family != NFPROTO_IPV6 &&
+ ctx->family != NFPROTO_BRIDGE)
+ return -EOPNOTSUPP;
+
if (nft_is_base_chain(ctx->chain)) {
const struct nft_base_chain *basechain =
nft_base_chain(ctx->chain);
@@ -595,6 +600,11 @@ static int nft_match_validate(const struct nft_ctx *ctx,
unsigned int hook_mask = 0;
int ret;
+ if (ctx->family != NFPROTO_IPV4 &&
+ ctx->family != NFPROTO_IPV6 &&
+ ctx->family != NFPROTO_BRIDGE)
+ return -EOPNOTSUPP;
+
if (nft_is_base_chain(ctx->chain)) {
const struct nft_base_chain *basechain =
nft_base_chain(ctx->chain);
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index ab3362c483b4..397351fa4d5f 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -384,6 +384,11 @@ static int nft_flow_offload_validate(const struct nft_ctx *ctx,
{
unsigned int hook_mask = (1 << NF_INET_FORWARD);
+ if (ctx->family != NFPROTO_IPV4 &&
+ ctx->family != NFPROTO_IPV6 &&
+ ctx->family != NFPROTO_INET)
+ return -EOPNOTSUPP;
+
return nft_chain_validate_hooks(ctx->chain, hook_mask);
}
diff --git a/net/netfilter/nft_nat.c b/net/netfilter/nft_nat.c
index 583885ce7232..808f5802c270 100644
--- a/net/netfilter/nft_nat.c
+++ b/net/netfilter/nft_nat.c
@@ -143,6 +143,11 @@ static int nft_nat_validate(const struct nft_ctx *ctx,
struct nft_nat *priv = nft_expr_priv(expr);
int err;
+ if (ctx->family != NFPROTO_IPV4 &&
+ ctx->family != NFPROTO_IPV6 &&
+ ctx->family != NFPROTO_INET)
+ return -EOPNOTSUPP;
+
err = nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
if (err < 0)
return err;
diff --git a/net/netfilter/nft_rt.c b/net/netfilter/nft_rt.c
index 35a2c28caa60..24d977138572 100644
--- a/net/netfilter/nft_rt.c
+++ b/net/netfilter/nft_rt.c
@@ -166,6 +166,11 @@ static int nft_rt_validate(const struct nft_ctx *ctx, const struct nft_expr *exp
const struct nft_rt *priv = nft_expr_priv(expr);
unsigned int hooks;
+ if (ctx->family != NFPROTO_IPV4 &&
+ ctx->family != NFPROTO_IPV6 &&
+ ctx->family != NFPROTO_INET)
+ return -EOPNOTSUPP;
+
switch (priv->key) {
case NFT_RT_NEXTHOP4:
case NFT_RT_NEXTHOP6:
diff --git a/net/netfilter/nft_socket.c b/net/netfilter/nft_socket.c
index 9ed85be79452..f30163e2ca62 100644
--- a/net/netfilter/nft_socket.c
+++ b/net/netfilter/nft_socket.c
@@ -242,6 +242,11 @@ static int nft_socket_validate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nft_data **data)
{
+ if (ctx->family != NFPROTO_IPV4 &&
+ ctx->family != NFPROTO_IPV6 &&
+ ctx->family != NFPROTO_INET)
+ return -EOPNOTSUPP;
+
return nft_chain_validate_hooks(ctx->chain,
(1 << NF_INET_PRE_ROUTING) |
(1 << NF_INET_LOCAL_IN) |
diff --git a/net/netfilter/nft_synproxy.c b/net/netfilter/nft_synproxy.c
index 13da882669a4..ebdb81eaafb0 100644
--- a/net/netfilter/nft_synproxy.c
+++ b/net/netfilter/nft_synproxy.c
@@ -253,6 +253,11 @@ static int nft_synproxy_validate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nft_data **data)
{
+ if (ctx->family != NFPROTO_IPV4 &&
+ ctx->family != NFPROTO_IPV6 &&
+ ctx->family != NFPROTO_INET)
+ return -EOPNOTSUPP;
+
return nft_chain_validate_hooks(ctx->chain, (1 << NF_INET_LOCAL_IN) |
(1 << NF_INET_FORWARD));
}
diff --git a/net/netfilter/nft_tproxy.c b/net/netfilter/nft_tproxy.c
index ae15cd693f0e..71412adb73d4 100644
--- a/net/netfilter/nft_tproxy.c
+++ b/net/netfilter/nft_tproxy.c
@@ -316,6 +316,11 @@ static int nft_tproxy_validate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nft_data **data)
{
+ if (ctx->family != NFPROTO_IPV4 &&
+ ctx->family != NFPROTO_IPV6 &&
+ ctx->family != NFPROTO_INET)
+ return -EOPNOTSUPP;
+
return nft_chain_validate_hooks(ctx->chain, 1 << NF_INET_PRE_ROUTING);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH nf] netfilter: nf_tables: validate NFPROTO_{IPV4,IPV6,INET} family
2024-01-23 16:13 [PATCH nf] netfilter: nf_tables: validate NFPROTO_{IPV4,IPV6,INET} family Pablo Neira Ayuso
@ 2024-01-23 17:10 ` Florian Westphal
0 siblings, 0 replies; 2+ messages in thread
From: Florian Westphal @ 2024-01-23 17:10 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Several expressions explicitly refer to NF_INET_* hook definitions
> from expr->ops->validate, however, family is not validated.
>
> Bail out with EOPNOTSUPP in case they are used from unsupported
> families.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> This is v1:
>
> NF_INET_* and NF_NETDEV_* hook definitions overlap, and .validate refers to
> hooks only in several expressions, not families.
>
> - synproxy refers to NFPROTO_BRIDGE from eval path, however, .validate does
> not refer to bridge hooks.
Probably those NFPROTO_BRIDGE should be removed then.
> diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
> index 5284cd2ad532..61f5498b73ef 100644
> --- a/net/netfilter/nft_compat.c
> +++ b/net/netfilter/nft_compat.c
> @@ -350,6 +350,11 @@ static int nft_target_validate(const struct nft_ctx *ctx,
> unsigned int hook_mask = 0;
> int ret;
>
> + if (ctx->family != NFPROTO_IPV4 &&
> + ctx->family != NFPROTO_IPV6 &&
> + ctx->family != NFPROTO_BRIDGE)
> + return -EOPNOTSUPP;
> +
> if (nft_is_base_chain(ctx->chain)) {
> const struct nft_base_chain *basechain =
> nft_base_chain(ctx->chain);
> @@ -595,6 +600,11 @@ static int nft_match_validate(const struct nft_ctx *ctx,
> unsigned int hook_mask = 0;
> int ret;
>
> + if (ctx->family != NFPROTO_IPV4 &&
> + ctx->family != NFPROTO_IPV6 &&
> + ctx->family != NFPROTO_BRIDGE)
> + return -EOPNOTSUPP;
> +
Both need to permit NFPROTO_ARP too.
LGTM otherwise.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-01-23 17:10 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-23 16:13 [PATCH nf] netfilter: nf_tables: validate NFPROTO_{IPV4,IPV6,INET} family Pablo Neira Ayuso
2024-01-23 17:10 ` Florian Westphal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).